Skip to content

docs(spec): eliminate secret leakage in logs, spans, and error responses - #31

Draft
antstanley wants to merge 1 commit into
spec/runtime-parity-across-interfacesfrom
spec/eliminate-secret-leakage-logs-spans
Draft

docs(spec): eliminate secret leakage in logs, spans, and error responses#31
antstanley wants to merge 1 commit into
spec/runtime-parity-across-interfacesfrom
spec/eliminate-secret-leakage-logs-spans

Conversation

@antstanley

@antstanley antstanley commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Note

Stacked PR 13 of 16 — part of stack #35, which targets main.
Base: #27 (spec/runtime-parity-across-interfaces) · Followed by: #30

This PR's diff shows only its own change. Merge the stack bottom-up; do not merge this before its base.

Why it sits at position 13: Introduces Secret<T> and the read_bounded helper that #33 relies on for the JWKS success path; cross-references #24 for the fixed-classification audit reason.

Scope of this PR

This PR currently carries the change spec. It will also carry the implementation plan and the build work for that spec — plan and code land as later commits on this same branch, so the spec, its plan, and its implementation review and merge as one unit.

Review order within the PR: spec first, then plan, then implementation.


Change spec proposing that secrets and internal detail stop reaching logs, spans, and error responses.

Problem. Seven confirmed findings with one root cause: credential-bearing values are ordinary Strings, so keeping them out of telemetry is a matter of reviewer discipline rather than something the compiler enforces. Three provider sites interpolate an upstream non-2xx body — which can carry provider tokens and client secrets — into ProviderError.detail; the LMDB and Valkey adapters put a refresh-token hash and session data into #[instrument] spans; the /token error path distinguishes failure causes to the caller; and X-Request-Id is client-chosen, unbounded, and propagated into every downstream span.

Proposed delta.

  • Secret<T> in crates/core — a newtype implementing neither Debug nor Display, so a credential reaching a tracing macro, a format!, or #[instrument]'s default argument capture becomes a compile error rather than a review miss. The spec enumerates the nine values it wraps, from Session.refresh_token_hash through the three configured secrets to every upstream response body read at a provider boundary. #[serde(transparent)], so no schema change.
  • One audited constructorshared::upstream::error_detail, fed by a 64 KiB-bounded read_bounded, replaces the three sites that interpolate an upstream body, redacting after percent-decoding.
  • A split error surface — a stable client_description() for the caller, with the full Display logged under the request span for every error class rather than only server_error, closing the /token validation oracle.
  • A bounded request id — 128 bytes of [A-Za-z0-9_-], minting a fresh UUIDv4 silently on rejection rather than failing the request.
  • The LMDB and Valkey #[instrument] redactions are specified to ship immediately, ahead of the type — one-line changes with no design dependency.

What reading the code changed.

  • The redacting-Debug precedent is six sites, not two. Beyond WebhookConfig.secret and InternalApiConfig.shared_secret, the code hand-implements redacting Debug for OidcProviderConfig, TokenResponse, ProviderTokens, and AppleProvider. Six correct applications and one miss — Session plainly derives Debug — is precisely the discipline-lapse pattern that argues for a type instead of a convention, so the Motivation is written to that fact.
  • OidcProvider has no Debug impl at all, derived or manual. Its client_secret: Option<String> is protected by the absence of Debug, not by a redacting one — no finding mentions this, and adding #[derive(Debug)] later would silently open a leak. Covered by wrapping the field.
  • LocalKeyManager is not an eighth leak. It derives Debug and holds an ed25519_dalek::SigningKey, whose own Debug uses finish_non_exhaustive() to omit the secret. Recorded as an Assumption — an upstream discipline this repo depends on but does not control — rather than a finding.
  • All scan line numbers verified exact.

Cross-reference note. 2026-08-05-audit_and_throttle_authentication_failures.md (#24) specifies the fixed-classification audit reason for the same root cause seen from the audit side; it is cross-referenced, not restated. That file is not on main yet, so the link resolves once #24 merges.

Drafted with the spec-creator plugin from a deep security scan of the repository at 53cbdec9. Spec only — no source changes.

@antstanley
antstanley force-pushed the spec/eliminate-secret-leakage-logs-spans branch 2 times, most recently from 8031bc4 to 39283a4 Compare August 5, 2026 08:22
@antstanley
antstanley changed the base branch from main to spec/runtime-parity-across-interfaces August 5, 2026 08:22
@antstanley
antstanley marked this pull request as draft August 5, 2026 08:25
@antstanley
antstanley force-pushed the spec/eliminate-secret-leakage-logs-spans branch from 39283a4 to c398ab3 Compare August 5, 2026 09:00
@antstanley
antstanley force-pushed the spec/eliminate-secret-leakage-logs-spans branch from c398ab3 to 7814df6 Compare August 5, 2026 11:29
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@antstanley
antstanley force-pushed the spec/eliminate-secret-leakage-logs-spans branch from 7814df6 to a308cc3 Compare August 5, 2026 12:01
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