Skip to content

codegen: honor [debug_redact = true] in generated Debug impls#162

Merged
rpb-ant merged 5 commits into
anthropics:mainfrom
arturenault:renault/debug-redact-debug-impls
Jun 1, 2026
Merged

codegen: honor [debug_redact = true] in generated Debug impls#162
rpb-ant merged 5 commits into
anthropics:mainfrom
arturenault:renault/debug-redact-debug-impls

Conversation

@arturenault
Copy link
Copy Markdown
Contributor

Motivation

debug_redact is protobuf's standard way to mark fields whose values must not appear in debug output — C++/Java honor it in DebugString, and Go has protoc-gen-go-redact. buffa currently prints annotated fields like any other: the generated owned-message Debug impl lists every field's value, and oneof enums / view structs / view-oneof enums #[derive(Debug)]. That makes a tracing::debug!(?req) on a generated request type a credential/PII leak for any consumer that wants to rely on the annotation. We'd like to start annotating sensitive fields (API keys, access/refresh tokens) in our protos and have buffa-generated Rust redact them automatically, matching what our Go codegen already does.

What this does

  • Owned messages (message.rs): the generated Debug impl prints the literal marker [REDACTED] (via ::core::format_args!, so it renders unquoted) in place of an annotated field's value.
  • Oneof enums, view structs, view-oneof enums (oneof.rs, view.rs): when a contained field/variant is annotated, the Debug derive is swapped for a generated impl that redacts those fields/variants; everything else prints as before. FooOwnedView wrappers pick this up transitively via OwnedView's Debug.
  • Unannotated protos generate byte-identical code — the derives and token streams are unchanged when no field carries the option (locked by a codegen test).
  • Scope is Debug formatting only: binary, JSON, and text-format serialization are untouched.
  • Docs: docs/guide.md (new "debug_redact and Debug output" subsection), the type_attribute Pitfalls rustdoc in buffa-build, and a CHANGELOG entry under Unreleased.

Conscious choices, happy to adjust:

  • The marker is the fixed literal [REDACTED] (matches protobuf's own debug-output convention); no config knob in this first cut — one can be added additively later.
  • A view struct containing a redacted field lists proto fields only in its Debug output (matching the owned-message impl); __buffa_unknown_fields is deliberately excluded since unknown fields can carry redacted data from a newer schema version. Noted in the CHANGELOG.

Testing

  • buffa-codegen/src/tests/debug_redact.rs: 4 codegen-output tests (message / oneof / view+view-oneof redaction, plus an unannotated-proto test asserting the derives and absence of the marker).
  • buffa-test/protos/debug_redact.proto + buffa-test/src/tests/debug_redact.rs: 6 runtime tests asserting format!("{:?}") never contains the annotated values for owned message, oneof variant, repeated field, all-redacted scalar oneof, and decoded views — and that unannotated fields/variants still print.
  • Full buffa-codegen (415) and buffa-test suites pass; task gen-wkt-types produces no diff (no vendored proto uses the option); clippy clean on the touched crates.

🤖 Generated with Claude Code

Fields annotated with the standard debug_redact option print [REDACTED]
instead of their value in owned-message, oneof, view, and view-oneof Debug
output. Types without the annotation produce byte-identical code.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 29, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@arturenault arturenault marked this pull request as ready for review May 29, 2026 16:59
@arturenault
Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request May 29, 2026
@arturenault
Copy link
Copy Markdown
Contributor Author

@iainmcgin would appreciate a review when you have a chance — this adds support for the standard [debug_redact = true] field option to the generated Debug impls (details and conscious choices in the description). Checks are green so far (conformance still running as I write this). Happy to adjust the marker or scope per your preference.

iainmcgin added 3 commits May 29, 2026 17:53
Fields whose descriptor carries [debug_redact = true] now print [REDACTED]
in place of their value when a DynamicMessage is Debug-formatted, matching
the redaction performed by generated Debug impls. Covered by an e2e test
with a hand-built descriptor and noted in the guide.
A keyword-named field (e.g. `type`) now prints the label `type` instead
of `r#type`, matching #[derive(Debug)] and the view Debug impl. Includes
the regenerated bootstrap descriptor types and a codegen test.
Assert the redacted bytes/repeated fields stay out of the view Debug output
and exercise the no-lifetime view-oneof Debug impl at runtime. Add changelog
entries for the DynamicMessage redaction and the Debug label fix.
@iainmcgin
Copy link
Copy Markdown
Collaborator

[claude code] Thanks for this contribution — the design is exactly right: the derive swap is keyed on the presence of an annotated field, so unannotated protos generate byte-identical code (verified locally — gen-wkt-types / gen-bootstrap-types produce no diff from your change), and the runtime tests assert that the actual secret strings are absent from the output, which is the right kind of assertion for a redaction feature.

I've pushed three follow-up commits directly to this branch (taking advantage of "Allow edits by maintainers") rather than going through a review round-trip:

4832138DynamicMessage's Debug now honors debug_redact too. The reflective path was the one remaining place where a consumer relying on the annotation for log hygiene could still leak a value: DynamicMessage's hand-written Debug printed every entry of the field map regardless of options. The descriptor (and its FieldOptions) is available at runtime, so the field map now prints [REDACTED] for annotated fields, keeping the redaction contract consistent between generated types and descriptor-driven decoding. Covered by a new e2e test that hand-builds its descriptor (so the checked-in .fds fixtures didn't need regenerating), and mentioned in the guide and changelog.

3d8f6d1 — owned-message Debug labels drop the r# prefix. A pre-existing quirk that your view impl made visible: the view Debug impl trims r# from labels (matching #[derive(Debug)]), while the owned manual impl printed r#type for a keyword-named field. The owned impl now trims as well, so owned and view output agree. This changes the Debug output of the checked-in bootstrap descriptor types for their two type fields (regenerated in the same commit), and gets a codegen test.

24ed306 — two view-side test additions. Assertions that the redacted bytes seed and repeated string scopes stay out of the view's Debug output (previously only asserted on the owned message), and a runtime test for the no-lifetime view-oneof Debug impl (numeric_secret), which was generated and compiled but not exercised at runtime. Plus the changelog entries for the two changes above.

Things we considered and deliberately left as-is:

  • The fixed [REDACTED] literal with no configuration knob — agreed this is the right first cut; the marker is documented, and a knob can come later if anyone needs it.
  • The Debug-output asymmetry between views with a redacted field (proto fields only) and views without (derive output, including internals) — documented in the changelog and acceptable; unifying every view onto the proto-fields-only form would churn all generated view output and is better as a separate change if we decide we want it.
  • The duplicated derive-swap pattern across the four codegen sites — fine for now; if it grows again we'll factor out a shared helper.

With the follow-ups on the branch, task lint (fmt + clippy -D warnings), the full workspace test suite, and the generated-code regeneration checks are all clean locally. Iain will take a final pass for approval once CI confirms.

@rpb-ant rpb-ant merged commit 5baa4be into anthropics:main Jun 1, 2026
7 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants