fix: stop writing data to stdout - #132
Open
coreyphillips wants to merge 1 commit into
Open
Conversation
The crate ships inside a wallet, and println!/eprintln! bypass the log facade the platforms route to their own sinks, so anything printed can be captured by a debugger, a redirected stdio stream or a sysdiagnose bundle. Removes all 21 print macros from non-test library code: 15 deleted, 6 routed through log::*.
Collaborator
Author
|
Will make a new build once approved. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the logging findings from the key generation security audit (prior ISSUES/01, reported again as F-02 and its adjacent "println hygiene" finding).
Why
bitkit-core is a library that ships inside a wallet.
println!andeprintln!bypass thelogfacade that the platforms route to their own sinks, so anything printed can be captured by a debugger, a redirected stdio stream, or a sysdiagnose bundle.Two of these prints carried data that must never leave the process:
generate_mnemonicprinted the full BIP39 mnemonic, the root secret of the wallet, at the moment of generation. The audit rated this High rather than Critical only because no production caller exists today, but the UniFFI export is live and nothing marks it test-only.create_and_store_orderprinted the entire Blocktank order response, including invoice, addresses, and amounts, on every order creation.The rest were debug leftovers, several of which printed the Bitcoin address under validation.
What changed
All 21 print macros in non-test library code are gone: 15 deleted, 6 routed through
log::error!/log::warn!. The converted sites log the error value only, never the input that produced it.There are no behavior changes: every affected function already returned a
Result, so the prints were purely additive tracing.To prevent regression, the library crate root now carries:
#![deny(clippy::print_stdout, clippy::print_stderr, clippy::dbg_macro)]This is enforced by the existing
cargo clippystep rather than a new CI grep. It applies to the lib target only, soexample/main.rskeeps its prints, and the three test modules that print carry a scoped#![allow(clippy::print_stdout)].Testing
cargo clippy --all-targetsis clean of the new lints and of errors.cargo fmt --checkpasses.cargo test modules::onchainpasses 66/66.