fix(codegen/rust): stop recording request bodies in instrumented client spans#173
Merged
Merged
Conversation
|
📖 Documentation Preview: https://reflectapi-docs-preview-pr-173.partly.workers.dev Updated automatically from commit 9cf7e9a |
…nt spans Instrumented Rust clients emitted #[tracing::instrument(name = ..., skip(self, headers))], which records the input argument as a span field via its Debug impl. Request bodies can carry credentials, so passwords and tokens reached logs in cleartext. Switch the generated attribute to skip_all: no function arguments are recorded, while the span keeps the endpoint path as its name. This protects every field, including secrets typed as plain String, independently of future first-class secrecy::SecretString support. Part of #169 (the secrecy::SecretString type mapping remains as follow-up).
fba10cb to
b4045ab
Compare
Collaborator
|
I thought about doing this but then concluded the better more complete fix would be to tell via reflectapi attribute per struct or per field what to include into the instrumentation. But that is bigger change so I parked if for now. |
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.
Part of #169 (just the mitigation); first-class
secrecy::SecretStringsupport (type mapping across all backends) remains as a follow-up.Problem
Instrumented Rust clients emit:
inputis not skipped, so tracing records the entire request body as a span field via itsDebugimpl. Since codegen maps string-like types to plainString, credentials print to logs in cleartext.Fix
The generated attribute now uses
skip_all. No function arguments are recorded; the span keeps the endpoint path as its name. This protects every request field — including secrets typed as plainStringon the server — which first-classSecretStringsupport alone would not.Trade-off: request bodies no longer appear as span fields at all. Given the payload is arbitrary user data with no redaction story, recording it by default was a liability rather than a feature.
Tests & validation
instrumented_rust_client_does_not_record_request_bodiesgenerates the demo schema with.instrument(true)and asserts every emitted#[tracing::instrument]attribute usesskip_all(and that attributes are emitted at all).skip_all) and compiles as a workspace member.tracing-subscriber:INFO /auth.login{input=LoginRequest { username: "brian", password: "hunter2-s3cret" }}: sending request— password in cleartextINFO /auth.login: sending request— nothing recorded, span name retained