Skip to content

fix: redact authorization tokens from trace logs#218

Open
EddieHouston wants to merge 2 commits into
bitcoindevkit:masterfrom
EddieHouston:fix/redact-authorization-trace
Open

fix: redact authorization tokens from trace logs#218
EddieHouston wants to merge 2 commits into
bitcoindevkit:masterfrom
EddieHouston:fix/redact-authorization-trace

Conversation

@EddieHouston

Copy link
Copy Markdown
Contributor

Summary

  • redact the top-level authorization value before logging JSON-RPC requests at TRACE level
  • cover both single and newline-delimited batch request paths without changing wire payloads
  • preserve unauthenticated trace output byte-for-byte and fail closed if redaction cannot parse a request

Testing

  • cargo test --lib --all-features
  • cargo clippy --lib --all-features -- -D warnings

Fixes #215

@EddieHouston

Copy link
Copy Markdown
Contributor Author

note... had to pin zeroize in new commit due to MSRV:
image

@sdmg15 sdmg15 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tAck ab750e3.

I also tested a case in batch requests where one is valid json and another is not and in the logs we have ==> <request redaction failed>

Comment thread src/raw_client.rs
let redacted = redact_authorization(raw);
let request: serde_json::Value = serde_json::from_slice(&redacted).unwrap();

assert_eq!(request["authorization"], "<redacted>");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think we can use the constant value REDACTED_AUTHORIZATION here

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.

Authorization token leaks into TRACE logs (full request is logged)

2 participants