Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .specs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ board (`backlog/` · `in-progress/` · `blocked/` · `done/`).
| [plans/2026-07-02-webhook_user_sync_conformance/plan.md](plans/2026-07-02-webhook_user_sync_conformance/plan.md) | Done | [changes/merged/2026-07-01-webhook_user_sync_conformance.md](changes/merged/2026-07-01-webhook_user_sync_conformance.md) |
| [plans/2026-07-02-server_error_handling_and_shutdown/plan.md](plans/2026-07-02-server_error_handling_and_shutdown/plan.md) | Done | [changes/merged/2026-07-01-server_error_handling_and_shutdown.md](changes/merged/2026-07-01-server_error_handling_and_shutdown.md) |
| [plans/2026-07-02-implement_lambda_runtime/plan.md](plans/2026-07-02-implement_lambda_runtime/plan.md) | Done | [changes/merged/2026-07-01-implement_lambda_runtime.md](changes/merged/2026-07-01-implement_lambda_runtime.md) |
| [plans/2026-08-05-eliminate_secret_leakage_in_logs_and_spans/plan.md](plans/2026-08-05-eliminate_secret_leakage_in_logs_and_spans/plan.md) | Review | [changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md](changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md) |

## Conventions

Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Task 01 — Immediate session span redactions

**Status:** Backlog · **Plan:** [plan.md](../plan.md) · **Certificate:** forbidden and intentionally omitted

**Implements:** [source spec §Implementation notes step 1](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#implementation-notes); [`08-persistence.md` Session-only stores](../../../service/specs/08-persistence.md)
**Depends on:** —
**Produces:** LMDB and Valkey session methods explicitly skip sensitive arguments and record only the permitted schema fields before the broad `Secret<T>` migration.
**Pointers:** `crates/adapters/src/lmdb/mod.rs:55,102,140`; `crates/adapters/src/valkey/mod.rs:52,141,211`; sibling patterns in Dynamo/Postgres/SQLite.

## Steps

- [ ] Change LMDB store instrumentation to skip `session` on writes and record only `%session.user_id`; skip `token_hash` on lookup/revoke while retaining an empty `token_hash` schema field.
- [ ] Make the equivalent explicit `skip(...)` changes in Valkey; do not record `Session`, refresh-token hash, IP address, user agent, or device ID.
- [ ] Audit all three methods on both backends for accidental default argument capture.
- [ ] Add focused tracing-capture regression tests with span-close events enabled that place sentinel hash/provenance values in sessions and prove none occurs in output while `user_id`/field schema remains observable.

## Task-specific definition of done

- [ ] LMDB and Valkey write, lookup, and revoke spans cannot render sentinels from the hash or provenance fields.
- [ ] Tests use `FmtSpan::CLOSE` (or equivalent explicit close-event capture), avoiding a vacuous no-span assertion.
- [ ] Existing session behavior and permitted `user_id` observability remain covered.
- [ ] No certificate file is created; test output is the completion evidence.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Task 02 — Bound and validate request IDs

**Status:** Backlog · **Plan:** [plan.md](../plan.md) · **Certificate:** forbidden and intentionally omitted

**Implements:** [source spec §04-http-api — Middleware stack](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#specsservicespecs04-http-apimd--middleware-stack-item-1-modify); [§Implementation notes step 2](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#implementation-notes); [`04-http-api.md` Middleware stack](../../../service/specs/04-http-api.md)
**Depends on:** —
**Produces:** a named 128-byte request-ID limit and predicate that preserves acceptable correlation IDs and silently replaces all other inbound values with UUIDv4 IDs.
**Pointers:** `crates/server/src/middleware/request_id.rs`; its module tests and existing `preserves_existing_request_id` test.

## Steps

- [ ] Add `MAX_REQUEST_ID_LEN: usize = 128` and `is_acceptable_request_id(&str) -> bool` with explicit non-empty, length, and ASCII `[A-Za-z0-9_-]` checks.
- [ ] Use the predicate in the inbound-header path; do not log rejected values and do not turn malformed correlation metadata into a request failure.
- [ ] Keep request span recording and response-header echo behavior unchanged for accepted and generated IDs.
- [ ] Add boundary tests for exactly 128 bytes, 129 bytes, a 64 KiB input, a legal-ASCII but wrongly shaped value, and invalid characters; keep accepted-id reuse and absent/invalid generation tests.

## Task-specific definition of done

- [ ] Only plausible IDs are reused; every rejected input yields a valid generated UUIDv4 in both span/response behavior.
- [ ] The limit is named and tests cover below/at/above boundary behavior plus charset negative space.
- [ ] Rejection remains silent: capture tests show the inbound malformed value is not emitted.
- [ ] No certificate file is created; test output is the completion evidence.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Task 03 — Add `Secret<T>` and migrate core contracts

**Status:** Backlog · **Plan:** [plan.md](../plan.md) · **Certificate:** forbidden and intentionally omitted

**Implements:** [source spec §Type changes](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#type-changes); [§Implementation notes step 6](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#implementation-notes); [`01-domain-model.md` Session](../../../service/specs/01-domain-model.md), [`02-ports-and-adapters.md` SessionRepository](../../../service/specs/02-ports-and-adapters.md), [`06-configuration.md`](../../../service/specs/06-configuration.md)
**Depends on:** —
**Produces:** core-owned serde-transparent `Secret<T>` with deliberate exposure APIs, constant-time `Secret<String>` equality, and compiler-enforced migration of the enumerated credential-derived core/config/session/repository values.
**Pointers:** `crates/core/src/lib.rs`, new `secret.rs`, `error.rs`, `config.rs`, `domain/session.rs`, `domain/token.rs`, `domain/provider.rs`, `ports/repository.rs`, `service/exchange.rs`; every SessionRepository adapter and test fixture.

## Steps

- [ ] Add and re-export `Secret<T>` with `new`, `expose`, and `into_inner`; derive only `Clone`, `Serialize`, and `Deserialize` with `#[serde(transparent)]`, never `Debug`/`Display`/generic `PartialEq`.
- [ ] Add `subtle` to core and implement constant-time `PartialEq` only for `Secret<String>`; cover equal and unequal values without exposing values in assertions/logging.
- [ ] Convert `Session.refresh_token_hash`, refresh-token issuance/`TokenResponse.refresh_token`, `WebhookConfig.secret`, `InternalApiConfig.shared_secret`, and `OidcProviderConfig.client_secret`; preserve serde storage/wire shapes and existing redacting enclosing `Debug` behavior.
- [ ] Replace `Session` derived `Debug` with a manual implementation that redacts only `refresh_token_hash` and preserves the non-sensitive fields.
- [ ] Change `SessionRepository` lookup/revoke signatures to `&Secret<String>` and migrate Dynamo, LMDB, Postgres, SQLite, Valkey, mocks, callers, store-key construction, and tests to use `expose()` only at deliberate storage/constant-time comparison boundaries.
- [ ] Convert adapter OIDC `client_secret` storage to `Option<Secret<String>>`; leave Apple assertion conversion to task 05, which owns its producer and provider call sites.

## Task-specific definition of done

- [ ] Every value in the source table except upstream response bodies and Apple’s generated assertion is `Secret<String>`/`Option<Secret<String>>` at the stated boundary.
- [ ] Session JSON round trips and token response serialization remain string-identical; no schema or migration changes occur.
- [ ] All repository implementations compile under `&Secret<String>` and their instrumentation uses explicit skips.
- [ ] `Session` debug output contains `<redacted>` and excludes a hash sentinel.
- [ ] No certificate file is created; test output and compile checks are the completion evidence.
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Task 04 — Add bounded, redacting upstream-error helper

**Status:** Backlog · **Plan:** [plan.md](../plan.md) · **Certificate:** forbidden and intentionally omitted

**Implements:** [source spec §02-ports-and-adapters — Shared OIDC utilities](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#specsservicespecs02-ports-and-adaptersmd--shared-oidc-utilities-modify); [§Implementation notes step 3](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#implementation-notes); [`02-ports-and-adapters.md` Shared OIDC utilities](../../../service/specs/02-ports-and-adapters.md)
**Depends on:** 03
**Produces:** shared bounded HTTP body read returning `Secret<String>` and a single `upstream::error_detail(status, body)` constructor that produces bounded, percent-decoded redacted diagnostics.
**Pointers:** `crates/adapters/src/shared/http.rs`, new `shared/upstream.rs`, `shared/mod.rs`, `shared/token_endpoint.rs`; adapter test utilities/wiremock fixtures.

## Steps

- [ ] Add `MAX_UPSTREAM_BODY_BYTES: usize = 65_536` and stream a response body to that ceiling rather than calling unbounded `text()`; return `Secret<String>` and define explicit behavior for read/truncation failure.
- [ ] Add `MAX_UPSTREAM_EXCERPT: usize = 256` and `upstream::error_detail(StatusCode, Secret<String>) -> String` as the only shared upstream-body-to-detail path.
- [ ] Prefer RFC 6749 JSON `error` and optional `error_description`; otherwise produce status, original byte length, and only a bounded redacted excerpt.
- [ ] Percent-decode before masking form keys `token`, `refresh_token`, `client_secret`, and `code`, and redact bare compact JWS values; ensure malformed encoding cannot bypass masking or panic.
- [ ] Export the helper and reader from `shared/mod.rs`; add focused unit/wiremock tests for structured OAuth preservation, encoded echoed form values, JWS, malformed input, oversized bodies, and excerpt boundaries.

## Task-specific definition of done

- [ ] No upstream body is fully buffered beyond 64 KiB, and no function returns a format-capable raw body before the reviewed conversion point.
- [ ] Every fallback diagnostic is bounded to 256 characters after redaction and contains no supplied secret sentinels, including percent-encoded forms.
- [ ] Conformant OAuth `error`/`error_description` behavior remains available for task 05’s existing token-endpoint regression.
- [ ] No certificate file is created; test output is the completion evidence.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Task 05 — Route provider boundaries through the safe helper

**Status:** Backlog · **Plan:** [plan.md](../plan.md) · **Certificate:** forbidden and intentionally omitted

**Implements:** [source spec §05-provider-system](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#specsservicespecs05-provider-systemmd--oidcprovider-behaviour-modify); [§Implementation notes step 4](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#implementation-notes); [`05-provider-system.md`](../../../service/specs/05-provider-system.md)
**Depends on:** 03, 04
**Produces:** all three upstream non-2xx paths use `read_bounded` and `upstream::error_detail`; OIDC credentials and Apple-generated assertions remain unprintable through request construction.
**Pointers:** `crates/adapters/src/shared/token_endpoint.rs`; `crates/adapters/src/oidc/mod.rs`; `crates/providers/src/apple.rs`; their existing token-exchange/revocation tests.

## Steps

- [ ] Replace token-endpoint direct `response.text()`/raw-body fallback with the bounded reader and shared error-detail constructor, preserving its conformant OAuth-error test.
- [ ] Convert OIDC revocation’s non-2xx body handling to the same pair; verify an upstream echo of the revoked token cannot appear in the returned internal detail/log path.
- [ ] Change Apple `generate_client_secret` to return `Secret<String>`, expose only to build the outbound form, and route Apple revocation non-2xx handling through the shared pair.
- [ ] Ensure configured OIDC `Option<Secret<String>>` is exposed only when building outbound forms and cannot be captured by traces/errors.
- [ ] Add provider-boundary tests for raw and percent-encoded echoed token/client assertion/client secret/code values, structured OAuth errors, oversize response behavior, and existing success/revocation contracts.

## Task-specific definition of done

- [ ] No `response.text()` remains on these three non-2xx paths, and no raw upstream body reaches `ProviderError.detail`.
- [ ] The existing `exchange_code_surfaces_oauth_error_on_non_2xx` regression remains true for safe structured content.
- [ ] OIDC and Apple revoke failures redact submitted token and credential sentinels, including encoded echo cases.
- [ ] Apple assertion generation and all outbound forms compile with non-formatting types.
- [ ] No certificate file is created; test output is the completion evidence.
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Task 06 — Separate public descriptions from internal diagnostics

**Status:** Backlog · **Plan:** [plan.md](../plan.md) · **Certificate:** forbidden and intentionally omitted

**Implements:** [source spec §04-http-api — Error mapping](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#specsservicespecs04-http-apimd--error-mapping-modify); [§Implementation notes step 5](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#implementation-notes); [`04-http-api.md` Error mapping](../../../service/specs/04-http-api.md)
**Depends on:** 03
**Produces:** `Error::client_description()` supplies stable static public text for every domain variant; server mapping logs full internal `Display` for all mapped classes, under the request span, without publishing diagnostics.
**Pointers:** `crates/core/src/error.rs`; `crates/server/src/error.rs`; `crates/server/tests/routes.rs:148`; existing `invalid_grant_emits_no_server_error_detail_log` test.

## Steps

- [ ] Add exhaustive `Error::client_description(&self) -> &'static str` mapping with a small fixed description set that embeds no caller input, library text, key state, or cache internals.
- [ ] Refactor `map_domain_error_inner` so every domain-error arm emits `client_description()` while retaining the specified status/error code and the static `UnsupportedGrantType` description.
- [ ] Move internal diagnostic logging out of the current `server_error`-only branch: log 5xx at `error!`, 4xx at `warn!`, within the existing request span.
- [ ] Add/debug-assert the mapping invariant that returned text equals `client_description()` for every arm; remove assumptions that public text differs only for server errors.
- [ ] Update leaking expectations and add tests proving unknown `kid` is not echoed and bad signature/expired/wrong-audience grants are indistinguishable in response body while their internal details remain logged with request ID.

## Task-specific definition of done

- [ ] Every mapped domain error returns only a static client description and no internal `reason`/`detail` crosses `/token`.
- [ ] All mapped 4xx and 5xx errors produce the correctly leveled operator event under the request span.
- [ ] Existing tests that codified leaked `code already used`/`contains("code")` behavior are replaced with generic-description expectations.
- [ ] No certificate file is created; test output is the completion evidence.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Task 07 — Prove structural and runtime non-leakage

**Status:** Backlog · **Plan:** [plan.md](../plan.md) · **Certificate:** forbidden and intentionally omitted

**Implements:** [source spec §Implementation notes step 7](../../../changes/2026-08-05-eliminate_secret_leakage_in_logs_and_spans.md#implementation-notes); [`07-telemetry-and-audit.md` Telemetry hygiene](../../../service/specs/07-telemetry-and-audit.md)
**Depends on:** 01, 02, 05, 06
**Produces:** compile-fail proof that `Secret<T>` cannot be formatted and a cross-boundary capture corpus proving sensitive sentinels never reach logs, span fields, or public error bodies.
**Pointers:** core/adapters/providers/server test manifests; session repository implementations; request-ID module tests; provider wiremock tests; server error/routes tests.

## Steps

- [ ] Add `trybuild` as a core dev-dependency and UI cases that fail for `tracing::info!(?secret)`, `%secret`, `format!("{secret}")`, and default `#[instrument]` argument capture; commit expected compiler diagnostics as test fixtures.
- [ ] Add an explicit capturing subscriber with span-close emission and percent-decoded matching helpers; drive store, refresh, revoke, and upstream-error paths using distinct hash/token/config/assertion sentinels.
- [ ] Exercise every `SessionRepository` backend (Dynamo, LMDB, Postgres, SQLite, Valkey, and mocks where applicable) and assert neither event nor close-span output contains secret/hash/provenance sentinels; assert permitted field names remain schema-compatible.
- [ ] Consolidate request-ID boundary/leak assertions and public error-oracle checks from tasks 02 and 06 into end-to-end request paths.
- [ ] Run and record actual Rust quality gates: `cargo fmt --all --check`, `cargo clippy --workspace -- -D warnings`, and `cargo nextest run --workspace`; fix failures before task completion.

## Task-specific definition of done

- [ ] Compile-fail tests prove the type-system control rather than merely redaction convention.
- [ ] Runtime corpus covers all required stores plus refresh/revoke/upstream error paths and detects plain or percent-encoded sentinels with close spans enabled.
- [ ] Request-ID limits and generic error bodies are exercised on an HTTP path, including positive and negative boundary cases.
- [ ] Full Rust gates pass with actual output captured in the PR/task discussion, not a done certificate.
- [ ] No certificate file is created; test output is the completion evidence.
Loading