descriptor, rpc, wallet: do not echo key material in error messages #36648

pull fametrano wants to merge 1 commits into bitcoin:master from fametrano:descriptor-errors-no-keys changing 11 files +163 −33

fametrano commented at 12:49 PM on October 24, 2026: none

Some descriptor error messages quote the string the caller sent, and a descriptor can hold a private key (a WIF, an xprv). The reply then carries the key into places the request may not go: an application's error log, an error reporter, a screenshot shared for help. Even the first characters of a key give away part of it.

This PR stops the quoting.

Parsing (descriptor.cpp, util/bip32.cpp):

before	after
`Key ' <key>' is invalid due to whitespace`	`Key is invalid due to whitespace`
`key '<key>' is not valid`	`key is not valid`
`Pubkey '<hex>' is invalid`	`Pubkey is invalid`
`Multi threshold '<text>' is not valid`	same for text of 11 characters or fewer, the length of `4294967295h`; `Multi threshold is not valid` for longer text
`Key path value '<text>' specifies multipath in a section where multipath is not allowed`	same rule: `Key path value specifies multipath in a section where multipath is not allowed` for longer text
`Key path value '<text>' is not a valid uint32`	same rule: `Key path value is not a valid uint32` for longer text
`'<expr>' is not a valid descriptor function`	`'<name>(...' is not a valid descriptor function`, where `<name>` is the text up to `(`; `Not a valid descriptor function` if there is no `(` or the name is longer than 16 characters

No key is 11 characters or fewer.

Callers:

Cannot derive script without private keys: '...' (rpc/util.cpp) and Could not add descriptor '...': ... (wallet/imports.cpp) now quote desc->ToString(), the public form with its checksum.
Unable to parse descriptor '...': ... (wallet/rpc/spend.cpp) no longer quotes the string: there is no parsed descriptor to print, and the parse error says what is wrong.

Existing assertions on these messages are updated. New tests check that no part of a key is in the parse errors and the caller errors, including a key with a typo in its last character. They also check the exact text of the caller errors.

This is one of several PRs from a large audit of Bitcoin Core that I did with the help of LLM tools. The audit produced its evidence all at once, which is why the PRs arrive together. I understand each change fully and will answer review questions myself.

descriptor, rpc, wallet: do not echo key material in error messages

Several error messages quote text from the descriptor the caller sent,
and that text can be a private key. Error text is often logged or
shared where the request is not. Even the first characters of a key
give away part of it.

Parsing (src/script/descriptor.cpp, src/util/bip32.cpp):
- "Key '%s' is invalid due to whitespace", "key '%s' is not valid" and
  "Pubkey '%s' is invalid" quoted the key. They quote nothing now.
- "Multi threshold '%s' is not valid" and the multipath message "Key
  path value '%s' specifies multipath ..." quote their text only if it
  is no longer than 11 characters, the length of "4294967295h". No key
  is that short. Longer text is left out.
- "Key path value '%s' is not a valid uint32" follows the same rule.
- "'%s' is not a valid descriptor function" quotes the function name, the
  text up to '(', followed by "(...", as in "'pk(...' is not a valid
  descriptor function". It does so only if the name is 16 characters or
  fewer. Otherwise, and for a bare key with no '(', nothing is quoted,
  and the message reads "Not a valid descriptor function".

Callers (src/rpc/util.cpp, src/wallet/imports.cpp,
src/wallet/rpc/spend.cpp):
- "Cannot derive script without private keys" and "Could not add
  descriptor" quote desc->ToString(), the public form, instead of the
  string sent.
- "Unable to parse descriptor" no longer quotes the string. No parsed
  descriptor exists there, and the parse error says what is wrong.

Tests are updated for the new messages. New tests check that no part of
a key is in the parse errors and the caller errors, including a key with
a typo in its last character. They also check the exact text of the
caller errors.

a3f26e8343

DrahtBot commented at 12:49 PM on October 24, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36648
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36648.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36648#author-ralph


Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK jpk68

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #36402 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36402.svg"> (refactor: wallet_importdescriptors to separate run_test() contents into specific functions by nebula-21)
- #36257 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36257.svg"> (qa: assert_equals -> assert_true/assert_false by hodlinator)
- #36122 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36122.svg"> (BIP460: CISA for Taproot key path spends by fjahr)
- #35918 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35918.svg"> (descriptor: Expose key expression parsing by pseudoramdom)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
jpk68 commented at 8:35 PM on October 27, 2026: contributor

ACK a3f26e834323a129dc2663825ab77053dd2f698f

Type	Reviewers
ACK	jpk68

Contributors

fametrano

DrahtBot

jpk68

descriptor, rpc, wallet: do not echo key material in error messages #36648

External sites

Reviews

Conflicts