Skip to content

fix: stop writing data to stdout - #132

Open
coreyphillips wants to merge 1 commit into
masterfrom
fix/remove-stdout-logging
Open

fix: stop writing data to stdout#132
coreyphillips wants to merge 1 commit into
masterfrom
fix/remove-stdout-logging

Conversation

@coreyphillips

Copy link
Copy Markdown
Collaborator

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! and eprintln! bypass the log facade 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_mnemonic printed 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_order printed 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 clippy step rather than a new CI grep. It applies to the lib target only, so example/main.rs keeps its prints, and the three test modules that print carry a scoped #![allow(clippy::print_stdout)].

Testing

  • cargo clippy --all-targets is clean of the new lints and of errors.
  • cargo fmt --check passes.
  • cargo test modules::onchain passes 66/66.
  • Full suite: 474 passed / 11 failed. The 11 failures are the Blocktank staging API tests, confirmed failing identically on clean master, so they are pre-existing and network-dependent rather than regressions.

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::*.
@coreyphillips
coreyphillips requested a review from ovitrif August 7, 2026 12:52
@coreyphillips coreyphillips self-assigned this Aug 7, 2026
@coreyphillips

Copy link
Copy Markdown
Collaborator Author

Will make a new build once approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant