#17783 common: Disallow calling IsArgSet() on ALLOW_LIST options
https://github.com/bitcoin/bitcoin/pull/17783 · · +1184/-207 in 15 files, 18 commits · labels: Utils/log/libs · draft
Goal
- Prevent negated or empty list options from mistakenly appearing as enabled
- Avoid subtle configuration and security bugs caused by misinterpreting command line options
This pull request disallows calling ArgsManager::IsArgSet() on arguments flagged with ALLOW_LIST, raising a logic error instead. It fixes call sites across the codebase to use GetArgs().empty() checks and normalizes -noonlynet handling.
Problem: When an ALLOW_LIST option is negated (such as -norpcwhitelist), IsArgSet() returns true because the option was present on the command line, but GetArgs() returns an empty list. Call sites that check IsArgSet() can mistakenly treat the option as enabled with empty configuration, leading to subtle configuration and security bugs.
Category: Utilities (logging, arguments, libraries) (#16 of 66)
P3 · bug fix
- P3 because it eliminates an internal hazard where empty list options falsely appear configured
- Systematically protects against recurring configuration and security mistakes across the codebase
P3 because it addresses an internal hazard in shared argument parsing where IsArgSet on list options returns true even when empty. Multiple bugs caused by this pattern were previously fixed in #30529, so disallowing the pattern systematically prevents recurring configuration errors.
Membership: Modifies ArgsManager parsing and validation in src/common/args.cpp and src/common/args.h.
Factors: security/stability 1, bug 1, performance 0, user value 0, leverage 1
Reviewability: Ready: Review #16545 first
- Review #16545 first
This PR is stacked on top of unmerged PRs #16545 and #17580, which introduce the type flags and ALLOW_LIST enforcement. The base PRs should be reviewed first.
Author status: active, periodically rebasing stack on main
Agreement: Positive
- Positive sentiment with minimal engagement while waiting on base pull requests
- Concept approval without stated reasons (promag)
Concept ACK from promag with no objections raised
The PR has had concept approval since 2019 without any objections, but has seen minimal reviewer engagement while waiting on its base PRs.
- promag gave a Concept ACK in December 2019
Review verdicts (DrahtBot): 0
- Concept ACK: promag
Dependencies
Depends on: #16545, #17580
Files
882 lines under test/bench/ci.
- src/test/argsman_tests.cpp +661/-71
- src/common/args.cpp +169/-49
- src/common/args.h +162/-21
- src/init.cpp +47/-39
- src/test/fuzz/system.cpp +60/-9
- src/test/getarg_tests.cpp +61/-3
- src/chainparamsbase.cpp +4/-4
- src/init/common.cpp +3/-3
- src/test/logging_tests.cpp +3/-3
- test/lint/check-doc.py +3/-2
- src/common/config.cpp +4/-0
- src/test/util/common.h +4/-0
- .github/workflows/ci.yml +1/-1
- src/wallet/init.cpp +1/-1
- test/functional/feature_config_args.py +1/-1
Card
This PR disallows calling ArgsManager::IsArgSet() on ALLOW_LIST options, throwing a logic error when attempted. It prevents bugs where a negated list option causes IsArgSet() to return true even though GetArgs() is empty, misleading callers into treating the setting as enabled. It is an internal cleanup that closes a bug hazard previously hit in RPC and network settings. The PR is stacked on top of open PRs #16545 and #17580, and has received concept support without detailed review.