From 4750cdcc295ceb24fe5b96ec5c3ddb19cadfc6d4 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 11:57:51 +0100 Subject: [PATCH 01/12] docs(auth): activate AUTH-04 controls --- .agent-loop/LOOP_STATE.md | 19 +++---- .agent-loop/REVIEW_LOG.md | 12 ++++ .agent-loop/WORK_QUEUE.md | 11 ++-- .../CHUNK_MAP.md | 11 ++-- .../DISCOVERY.md | 57 +++++++++++++++++++ .../STATUS.md | 21 +++---- .../WS-AUTH-001-04-request-api-controls.md | 43 +++++++++++++- 7 files changed, 141 insertions(+), 33 deletions(-) diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index 31f9f768..3494bc00 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -4,18 +4,17 @@ - Active initiative: `WS-AUTH-001` - Workstream Authorization Service - Active planning chunk: none -- Active implementation chunk: none -- Branch: `codex/ws-auth-001-03-post-merge-memory` -- Worktree: `/home/abiorh/flow/workstream-authorization-service` -- Status: AUTH-03 merged through PR #109 as `f06532e` after Backend, Agent - Gates, CodeRabbit, internal review, and explicit human approval passed. +- Active implementation chunk: `WS-AUTH-001-04` - Request, Error, And API + Control Foundation +- Branch: `codex/ws-auth-001-04-request-api-controls` +- Worktree: `/home/abiorh/flow/workstream-auth-001-04` +- Status: AUTH-03 post-merge memory merged through PR #110 as `1864867`; the + user explicitly started AUTH-04 and its L1 preimplementation review is active. - Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836` - Prior `WS-AUTH-001-01` final merged branch head: `b5217e1` -- Latest integrated `main` merge commit: `f06532e` -- Current gate: publish and merge the AUTH-03 post-merge memory update, then - stop. -- Next chunk: none; `WS-AUTH-001-04` remains proposed and inactive pending a - separate explicit user start after this memory update merges. +- Latest integrated `main` merge commit: `1864867` +- Current gate: AUTH-04 discovery and required preimplementation plan review. +- Next chunk: none; do not start `WS-AUTH-001-05` automatically. - Parallel initiative: `WS-QUAL-001-01B1B-R10` merged through PR #108 as `5c47aba`, and this convergence records its post-merge memory. B2 remains inactive pending a separate explicit user start; coverage chunk 02 remains diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 04b761e2..76192dd9 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -1,5 +1,17 @@ # Review Log +## 2026-07-13 - WS-AUTH-001-04 Started + +PR #110 merged AUTH-03 post-merge memory to `main` as `1864867` after Backend, +Agent Gates, CodeRabbit, internal review, and explicit human approval passed. +The user then explicitly started `WS-AUTH-001-04` in the isolated worktree +`/home/abiorh/flow/workstream-auth-001-04` on branch +`codex/ws-auth-001-04-request-api-controls`. + +AUTH-04 is L1/P1 authentication infrastructure. Discovery and required +preimplementation plan review must pass before runtime edits. AUTH-05, +POL-002-04, and all QA implementation chunks remain inactive. + ## 2026-07-13 - WS-AUTH-001-03 Merged PR #109 merged `WS-AUTH-001-03` to `main` as `f06532e` after final branch head diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index 8fca6331..22643f04 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -4,13 +4,12 @@ | Chunk | Title | Risk | Status | |---|---|---:|---| -| - | No active implementation chunk | - | AUTH-03 post-merge memory in progress; no product implementation active | +| `WS-AUTH-001-04` | Request, Error, And API Control Foundation | L1 | Started; discovery and preimplementation review active | ## Planned Next | Chunk | Title | Risk | Status | |---|---|---:|---| -| `WS-AUTH-001-04` | Request, Error, And API Control Foundation | L1 | Proposed; inactive pending AUTH-03 memory merge and explicit user start | | `WS-QUAL-001-01B2` | Baseline Evidence And CI Ratchet | L1 | Inactive pending explicit user start | | `WS-QUAL-001-02` | Project Service Coverage | L1 | Inactive until 01B2 merge/memory plus explicit user start | | `WS-POL-002-04` | Locked Runtime Execution And Routing Hardening | L1 | Inactive pending relevant authorization proof and a separate explicit user start | @@ -56,10 +55,10 @@ ## Proposed Next -`WS-AUTH-001-03` merged through PR #109 as `f06532e` after all required checks -and reviews passed. Its post-merge memory update is active; no AUTH product -implementation is active. Do not start `WS-AUTH-001-04` or `WS-POL-002-04` -automatically. +`WS-AUTH-001-03` post-merge memory merged through PR #110 as `1864867`. The user +explicitly started only `WS-AUTH-001-04`; discovery and preimplementation review +are active in its isolated worktree. Do not start `WS-AUTH-001-05` or +`WS-POL-002-04` automatically. Coverage R10 merged through PR #108. Do not start 01B2, chunk 02, or another coverage implementation chunk from this worktree. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md index f7fe1635..ba1d9b6a 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md @@ -15,7 +15,7 @@ stopped. | `WS-AUTH-001-01` | Adopt Authorization Baseline And Repository Contracts | L1 | Merged through PR #93 as `772af1d` | | `WS-AUTH-001-02` | Verified Issuer Token And JWKS Boundary | L1 | Merged through PR #107 as `060b780` | | `WS-AUTH-001-03` | Legacy Actor Classification Preflight | L1 | Merged through PR #109 as `f06532e` | -| `WS-AUTH-001-04` | Request, Error, And API Control Foundation | L1 | Proposed | +| `WS-AUTH-001-04` | Request, Error, And API Control Foundation | L1 | Started; preimplementation review active | | `WS-AUTH-001-05` | Authority Evidence And Idempotency Foundation | L1 | Proposed | | `WS-AUTH-001-06` | Canonical Actor Profile And Identity Link | L1 | Proposed | | `WS-AUTH-001-07` | Authorization Kernel And Permission Registry | L1 | Proposed | @@ -72,8 +72,7 @@ WS-AUTH-001-PLAN ## Stop condition -`WS-AUTH-001-03` merged through PR #109 as `f06532e` after deterministic proof, -all required internal reviewers, Backend, Agent Gates, CodeRabbit, and explicit -human approval passed. Its post-merge memory update is the current gate. Do not -start `WS-AUTH-001-04` or `WS-POL-002-04`; both retain separate explicit start -signals and prerequisites. +AUTH-03 post-merge memory merged through PR #110 as `1864867`. The user +explicitly started AUTH-04; its discovery and required L1 preimplementation +review are the current gate. Do not start AUTH-05 or POL-002-04; both retain +separate explicit start signals and prerequisites. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DISCOVERY.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DISCOVERY.md index 67894ea9..c2f7a7f1 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DISCOVERY.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DISCOVERY.md @@ -156,3 +156,60 @@ Production deployment inputs remain externally supplied: - Append-only audit facts for lifecycle and authority changes. - Fixed internal system actors for server-owned automation. - One PR-sized chunk at a time with deterministic evidence and internal review. + +## AUTH-04 Delta Discovery (2026-07-13) + +### Current behavior + +- `backend/app/main.py` only customizes FastAPI request-validation errors. It + has no request/correlation context, HTTP/unhandled exception envelope, or + response ID headers. +- Existing HTTP errors primarily expose FastAPI `detail`. Two bounded task + domain errors expose top-level `code/details`; existing tests rely on these + fields, so AUTH-04 must add the canonical nested envelope without deleting + compatibility fields. +- `backend/app/api/deps/auth.py` already distinguishes missing, invalid, and + unavailable authentication conditions internally, but its public responses + do not carry stable machine codes or correlation evidence. +- No rate-control model, query, service, or dependency exists. Redis is Celery + delivery infrastructure and is not an authority or API-control store. +- No first-access or admin-mutation endpoint exists yet. Their control + dependencies must be prepared here and attached only by the later owning + chunks; unrelated legacy reads must not consume those limits. + +### Current migration and ownership boundary + +- Current `main` migration head is `0016_artifact_domain`; the original AUTH-04 + `0016_*` filename allowance is stale after parallel artifact integration. +- AUTH-04 therefore owns `0017_api_controls` with down revision + `0016_artifact_domain`. +- Existing feature modules use `models -> repository -> service`, with + `backend/app/db/models.py` importing feature models for Alembic metadata. + `backend/app/modules/api_controls` must follow that pattern and own SQL and + transaction behavior. + +### Required compatibility and security properties + +- Request/correlation headers require bounded canonical UUID parsing, duplicate + rejection, safe generation, and no reflection of invalid bytes. +- The adopted fallback envelope is + `error.code/message/details/correlation_id/retryable`; raw exceptions, token + material, claims, provider responses, SQL, secrets, and PII are excluded. +- Existing `detail` and coded-domain fields remain additive compatibility + surfaces in this chunk; current intake assertions may not be weakened. +- Privacy-safe rate keys require keyed HMAC rather than persisted raw issuer, + subject, actor, token, network, or email values. +- Cross-replica fixed-window increments require one PostgreSQL atomic upsert + driven by database time. Missing configuration or database failure must fail + closed for the protected dependency. + +### Proof gaps owned by AUTH-04 + +- Header absence/propagation, malformed/duplicate values, success and every + error class, unhandled exception redaction, and legacy-field compatibility. +- Allowed, exact-limit, exceeded, expiry-reset, concurrent, cross-session, + distinct-scope/key, missing-secret, and database-unavailable rate behavior. +- Prior-head upgrade, downgrade, re-upgrade, constraints, unique key, indexes, + and database-time fields for migration `0017`. +- Full backend and API-contract regression proof without modifying product + authorization or attaching future controls to unrelated routes. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md index 50d09df7..0d9185aa 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md @@ -23,7 +23,8 @@ to `main` as `5c47aba`; that main revision is integrated into AUTH, whose prior lifecycle reconciliation was reviewed at `a70b89c`. Backend, Agent Gates, and CodeRabbit passed on final branch head `43ffbfe`, then explicit human approval merged PR #109 to `main` as `f06532e` on 2026-07-13. AUTH-03 post-merge memory -is active; AUTH-04 remains inactive. +merged through PR #110 as `1864867`. The user then explicitly started AUTH-04; +discovery and required L1 preimplementation review are active. ## Active planning chunk @@ -31,12 +32,12 @@ None. ## Active implementation chunk -None. +`WS-AUTH-001-04` - Request, Error, And API Control Foundation. Runtime edits +remain gated on the required preimplementation review. ## Current implementation branch -None. Post-merge memory uses `codex/ws-auth-001-03-post-merge-memory` and makes -no product implementation change. +`codex/ws-auth-001-04-request-api-controls`. ## Chunk status @@ -46,7 +47,7 @@ no product implementation change. | `WS-AUTH-001-01` | Merged | `codex/ws-auth-001-01-adopt-authorization-baseline` | #93 | Authorization baseline, Contributor terminology boundary, scanner, and repository contracts; merged as `772af1d`. | | `WS-AUTH-001-02` | Merged | `codex/ws-auth-001-02-verified-issuer-token` | #107 | Merged as `060b780`; reviewed code SHA `47dd5a7`. | | `WS-AUTH-001-03` | Merged | `codex/ws-auth-001-03-legacy-actor-classification` | #109 | Merged as `f06532e`; reviewed code `8c5334c`; final branch head `43ffbfe`. | -| `WS-AUTH-001-04` | Proposed | - | - | Request, error, and API control foundation. | +| `WS-AUTH-001-04` | Active | `codex/ws-auth-001-04-request-api-controls` | - | Discovery and required preimplementation review active; no runtime edit yet. | | `WS-AUTH-001-05` | Proposed | - | - | Authority evidence and idempotency foundation. | | `WS-AUTH-001-06` | Proposed | - | - | Canonical actor profile and identity link. | | `WS-AUTH-001-07` | Proposed | - | - | Authorization kernel and permissions. | @@ -62,11 +63,11 @@ no product implementation change. ## Blockers -No active implementation blocker because no AUTH product chunk is active. -AUTH-04 remains proposed and may start only after this memory update merges and -the user gives a separate explicit start signal. Non-test operators must later -supply explicit classification evidence rather than inferred kinds before the -owning canonical actor migration. +No current blocker. AUTH-04 must pass preimplementation review before runtime +edits. Discovery corrected its migration from the now-owned `0016` prefix to +`0017` on current main. Non-test operators must later supply explicit +classification evidence rather than inferred kinds before the owning canonical +actor migration. AUTH-03 review evidence and its PR trust bundle are recorded at `reviews/WS-AUTH-001-03-internal-review-evidence.md` and diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04-request-api-controls.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04-request-api-controls.md index f304934b..cac18ee8 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04-request-api-controls.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04-request-api-controls.md @@ -38,7 +38,7 @@ backend/app/core/api_controls.py backend/app/core/config.py backend/app/modules/api_controls/** backend/app/db/models.py -backend/alembic/versions/0016_*.py +backend/alembic/versions/0017_*.py backend/tests/test_app.py backend/tests/test_auth.py backend/tests/test_alembic.py @@ -63,7 +63,16 @@ dependency overrides that bypass the real request path - Every request has validated request/correlation IDs with safe generation and propagation rules. +- `X-Request-ID` and `X-Correlation-ID` accept one canonical UUID value each; + absent request IDs are generated, absent correlation IDs reuse the effective + request ID, and malformed/duplicate values fail as `invalid_request` without + reflecting the supplied bytes. +- Successful and error responses return the effective request/correlation IDs + in headers. The structured error object contains the correlation ID only. - Errors use a stable envelope without raw exception, token, claims, or PII. +- The envelope is `error.code/message/details/correlation_id/retryable`. + Existing `detail`, `code`, and `details` fields remain additively compatible + during the bounded migration so current intake clients and tests do not break. - Postgres-backed rate controls work across replicas, use privacy-safe keys and database time, and are configurable for first access/admin mutations. - `backend/app/modules/api_controls` owns feature models, persistence queries, @@ -73,6 +82,15 @@ dependency overrides that bypass the real request path controls fail closed without bypassing protected operations. - Migration owns counter constraints/indexes/database-time fields and proves prior-head upgrade, downgrade, and re-upgrade. +- The rate-control table has one atomic fixed-window counter per control scope + and HMAC-SHA256 subject key. The repository owns PostgreSQL upsert/database + time; the service owns key derivation and limit decisions. +- Rate-control invocation without its configured secret or with unavailable + persistence returns a retryable safe 503. Exceeded controls return a stable + retryable 429 with a bounded `Retry-After` value. +- AUTH-04 exposes named first-access and admin-mutation control dependencies for + their later owning endpoints but does not attach them to unrelated legacy + reads or implement those future endpoints. - Full backend suite and the intentionally updated API contract drill preserve the existing intake lifecycle and error compatibility. @@ -111,3 +129,26 @@ database-time semantics, and unchanged product authority. Stop if rate controls require a new production dependency without explicit human approval or if existing intake assertions must be weakened. + +## Activation + +AUTH-03 post-merge memory merged through PR #110 as `1864867`. The user +explicitly started AUTH-04 on 2026-07-13. Discovery found that parallel artifact +work already owns migration `0016`, so this chunk uses additive migration +`0017_api_controls` from `0016_artifact_domain`; this is a sequence correction, +not a scope expansion. + +## Implementation Plan + +1. Add pure request-context/error primitives and one ASGI middleware that + validates IDs, normalizes all JSON error responses additively, preserves + response headers, and bounds unhandled failures. +2. Add feature-owned rate-control model, repository, service, and dependencies + with HMAC privacy keys, atomic PostgreSQL fixed windows, database time, + stable exceeded/unavailable errors, and no public endpoint activation. +3. Add migration `0017_api_controls`, model registration, bounded settings, + request/error/rate behavior tests, migration round-trip proof, and the + intentionally updated API contract drill. +4. Run focused and full isolated-PostgreSQL proof, at least 90 percent coverage + for the new subsystem, repository gates, all required internal reviewers, + evidence validation, and one ready PR. Do not start AUTH-05. From f01427a7e0f44663db23d0ba3ef095bfb2c78922 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 12:12:37 +0100 Subject: [PATCH 02/12] docs(auth): split AUTH-04 controls --- .agent-loop/LOOP_STATE.md | 11 +- .agent-loop/REVIEW_LOG.md | 14 ++ .agent-loop/WORK_QUEUE.md | 10 +- .../CHUNK_MAP.md | 18 +- .../DISCOVERY.md | 14 ++ .../STATUS.md | 25 ++- .../WS-AUTH-001-04-request-api-controls.md | 27 ++- .../WS-AUTH-001-04A-request-error-context.md | 175 ++++++++++++++++++ .../WS-AUTH-001-04B-postgres-rate-controls.md | 151 +++++++++++++++ 9 files changed, 407 insertions(+), 38 deletions(-) create mode 100644 .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md create mode 100644 .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04B-postgres-rate-controls.md diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index 3494bc00..90c3563a 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -4,17 +4,20 @@ - Active initiative: `WS-AUTH-001` - Workstream Authorization Service - Active planning chunk: none -- Active implementation chunk: `WS-AUTH-001-04` - Request, Error, And API - Control Foundation +- Active implementation chunk: `WS-AUTH-001-04A` - Request And Error Context - Branch: `codex/ws-auth-001-04-request-api-controls` - Worktree: `/home/abiorh/flow/workstream-auth-001-04` - Status: AUTH-03 post-merge memory merged through PR #110 as `1864867`; the - user explicitly started AUTH-04 and its L1 preimplementation review is active. + user explicitly started AUTH-04. Required plan review split the parent before + implementation; only AUTH-04A contract repair and re-review are active. - Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836` - Prior `WS-AUTH-001-01` final merged branch head: `b5217e1` - Latest integrated `main` merge commit: `1864867` -- Current gate: AUTH-04 discovery and required preimplementation plan review. +- Current gate: AUTH-04A repaired contract and required preimplementation plan + re-review. No runtime edit is permitted until it passes. - Next chunk: none; do not start `WS-AUTH-001-05` automatically. +- `WS-AUTH-001-04B` is inactive until AUTH-04A merges, memory is updated, and + the user gives its separate explicit start signal. - Parallel initiative: `WS-QUAL-001-01B1B-R10` merged through PR #108 as `5c47aba`, and this convergence records its post-merge memory. B2 remains inactive pending a separate explicit user start; coverage chunk 02 remains diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 76192dd9..38c7a3b5 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -1,5 +1,19 @@ # Review Log +## 2026-07-13 - WS-AUTH-001-04 Split Required Before Implementation + +Required L1 plan review rejected the combined request/error/rate-control parent +contract before any runtime edit. The valid findings required exact ASGI header +behavior and OpenAPI compatibility for request/error context, plus an +independent committed PostgreSQL transaction, database-time atomic upsert, +privacy-key framing, and migration proof for rate controls. + +The parent is split into `WS-AUTH-001-04A` Request And Error Context and +`WS-AUTH-001-04B` PostgreSQL Rate Controls. Only 04A contract repair and +preimplementation re-review are active. 04B requires 04A merge/memory and a +separate explicit user start. No runtime implementation was written under the +failed combined plan. + ## 2026-07-13 - WS-AUTH-001-04 Started PR #110 merged AUTH-03 post-merge memory to `main` as `1864867` after Backend, diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index 22643f04..381331bc 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -4,12 +4,13 @@ | Chunk | Title | Risk | Status | |---|---|---:|---| -| `WS-AUTH-001-04` | Request, Error, And API Control Foundation | L1 | Started; discovery and preimplementation review active | +| `WS-AUTH-001-04A` | Request And Error Context | L1 | Active; repaired contract awaiting required preimplementation re-review | ## Planned Next | Chunk | Title | Risk | Status | |---|---|---:|---| +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Inactive until 04A merge/memory and a separate explicit user start | | `WS-QUAL-001-01B2` | Baseline Evidence And CI Ratchet | L1 | Inactive pending explicit user start | | `WS-QUAL-001-02` | Project Service Coverage | L1 | Inactive until 01B2 merge/memory plus explicit user start | | `WS-POL-002-04` | Locked Runtime Execution And Routing Hardening | L1 | Inactive pending relevant authorization proof and a separate explicit user start | @@ -56,9 +57,10 @@ ## Proposed Next `WS-AUTH-001-03` post-merge memory merged through PR #110 as `1864867`. The user -explicitly started only `WS-AUTH-001-04`; discovery and preimplementation review -are active in its isolated worktree. Do not start `WS-AUTH-001-05` or -`WS-POL-002-04` automatically. +explicitly started parent `WS-AUTH-001-04`. Required plan review split it before +runtime implementation. Only child `WS-AUTH-001-04A` contract repair and +re-review are active in its isolated worktree. Do not start 04B, AUTH-05, or +POL-002-04 automatically. Coverage R10 merged through PR #108. Do not start 01B2, chunk 02, or another coverage implementation chunk from this worktree. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md index ba1d9b6a..f5175312 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md @@ -15,7 +15,9 @@ stopped. | `WS-AUTH-001-01` | Adopt Authorization Baseline And Repository Contracts | L1 | Merged through PR #93 as `772af1d` | | `WS-AUTH-001-02` | Verified Issuer Token And JWKS Boundary | L1 | Merged through PR #107 as `060b780` | | `WS-AUTH-001-03` | Legacy Actor Classification Preflight | L1 | Merged through PR #109 as `f06532e` | -| `WS-AUTH-001-04` | Request, Error, And API Control Foundation | L1 | Started; preimplementation review active | +| `WS-AUTH-001-04` | Request, Error, And API Control Foundation | L1 | Split before implementation into 04A and 04B | +| `WS-AUTH-001-04A` | Request And Error Context | L1 | Active; repaired contract awaiting preimplementation re-review | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Inactive pending 04A merge/memory and separate explicit start | | `WS-AUTH-001-05` | Authority Evidence And Idempotency Foundation | L1 | Proposed | | `WS-AUTH-001-06` | Canonical Actor Profile And Identity Link | L1 | Proposed | | `WS-AUTH-001-07` | Authorization Kernel And Permission Registry | L1 | Proposed | @@ -36,7 +38,8 @@ WS-AUTH-001-PLAN -> WS-AUTH-001-01 -> WS-AUTH-001-02 -> WS-AUTH-001-03 --> WS-AUTH-001-04 +-> WS-AUTH-001-04A +-> WS-AUTH-001-04B -> WS-AUTH-001-05 -> WS-AUTH-001-06 -> WS-AUTH-001-07 @@ -55,7 +58,9 @@ WS-AUTH-001-PLAN - Chunk 02 authenticates tokens but grants no product authority. - Chunk 03 provides a supported classification gate before schema migration. -- Chunk 04 establishes request/error/rate controls. +- Parent chunk 04 was split before implementation. Chunk 04A establishes + request/correlation and additive error compatibility; chunk 04B later owns + durable PostgreSQL rate controls and its migration. - Chunk 05 evolves the shared audit path and idempotency before mutations. - Chunk 06 establishes canonical actor resolution while preserving only the enumerated non-authoritative legacy workflow-eligibility consumers required @@ -73,6 +78,7 @@ WS-AUTH-001-PLAN ## Stop condition AUTH-03 post-merge memory merged through PR #110 as `1864867`. The user -explicitly started AUTH-04; its discovery and required L1 preimplementation -review are the current gate. Do not start AUTH-05 or POL-002-04; both retain -separate explicit start signals and prerequisites. +explicitly started parent AUTH-04. Required plan review split it before runtime +implementation. AUTH-04A contract repair and re-review are the current gate. +Do not start AUTH-04B, AUTH-05, or POL-002-04; each retains its separate start +signal and prerequisites. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DISCOVERY.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DISCOVERY.md index c2f7a7f1..b4454949 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DISCOVERY.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DISCOVERY.md @@ -213,3 +213,17 @@ Production deployment inputs remain externally supplied: and database-time fields for migration `0017`. - Full backend and API-contract regression proof without modifying product authorization or attaching future controls to unrelated routes. + +### Required plan-review split + +The first combined AUTH-04 plan failed required preimplementation review before +any runtime edit. Request/error compatibility and durable rate consumption each +need an independently reviewable contract and production-code budget. + +- `WS-AUTH-001-04A` owns pure-ASGI request/correlation context, bounded additive + error envelopes, legacy response/header compatibility, and OpenAPI proof. +- `WS-AUTH-001-04B` later owns HMAC-keyed PostgreSQL counters, a dedicated + committed transaction, atomic database-time windows, `0017_api_controls`, + configuration failure behavior, and migration/concurrency proof. +- 04B is not activated by this split. It requires 04A merge/memory and a + separate explicit user start. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md index 0d9185aa..92bbaaf9 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md @@ -23,8 +23,10 @@ to `main` as `5c47aba`; that main revision is integrated into AUTH, whose prior lifecycle reconciliation was reviewed at `a70b89c`. Backend, Agent Gates, and CodeRabbit passed on final branch head `43ffbfe`, then explicit human approval merged PR #109 to `main` as `f06532e` on 2026-07-13. AUTH-03 post-merge memory -merged through PR #110 as `1864867`. The user then explicitly started AUTH-04; -discovery and required L1 preimplementation review are active. +merged through PR #110 as `1864867`. The user then explicitly started AUTH-04. +Required L1 plan review rejected the combined request/error/rate-control scope +before runtime edits and required a split. Only AUTH-04A contract repair and +preimplementation re-review are now active; AUTH-04B is inactive. ## Active planning chunk @@ -32,8 +34,8 @@ None. ## Active implementation chunk -`WS-AUTH-001-04` - Request, Error, And API Control Foundation. Runtime edits -remain gated on the required preimplementation review. +`WS-AUTH-001-04A` - Request And Error Context. Runtime edits remain gated on +the required repaired-contract preimplementation re-review. ## Current implementation branch @@ -47,7 +49,9 @@ remain gated on the required preimplementation review. | `WS-AUTH-001-01` | Merged | `codex/ws-auth-001-01-adopt-authorization-baseline` | #93 | Authorization baseline, Contributor terminology boundary, scanner, and repository contracts; merged as `772af1d`. | | `WS-AUTH-001-02` | Merged | `codex/ws-auth-001-02-verified-issuer-token` | #107 | Merged as `060b780`; reviewed code SHA `47dd5a7`. | | `WS-AUTH-001-03` | Merged | `codex/ws-auth-001-03-legacy-actor-classification` | #109 | Merged as `f06532e`; reviewed code `8c5334c`; final branch head `43ffbfe`. | -| `WS-AUTH-001-04` | Active | `codex/ws-auth-001-04-request-api-controls` | - | Discovery and required preimplementation review active; no runtime edit yet. | +| `WS-AUTH-001-04` | Split | `codex/ws-auth-001-04-request-api-controls` | - | Parent split before runtime implementation. | +| `WS-AUTH-001-04A` | Active | `codex/ws-auth-001-04-request-api-controls` | - | Repaired contract awaiting required preimplementation re-review; no runtime edit yet. | +| `WS-AUTH-001-04B` | Inactive | - | - | PostgreSQL rate controls; requires 04A merge/memory and separate explicit start. | | `WS-AUTH-001-05` | Proposed | - | - | Authority evidence and idempotency foundation. | | `WS-AUTH-001-06` | Proposed | - | - | Canonical actor profile and identity link. | | `WS-AUTH-001-07` | Proposed | - | - | Authorization kernel and permissions. | @@ -63,11 +67,12 @@ remain gated on the required preimplementation review. ## Blockers -No current blocker. AUTH-04 must pass preimplementation review before runtime -edits. Discovery corrected its migration from the now-owned `0016` prefix to -`0017` on current main. Non-test operators must later supply explicit -classification evidence rather than inferred kinds before the owning canonical -actor migration. +No external blocker. The combined AUTH-04 plan failed its internal gate and was +split before runtime edits. AUTH-04A must pass repaired-contract review before +implementation. AUTH-04B later owns the migration corrected from the now-owned +`0016` prefix to `0017` on current main, but remains inactive. Non-test +operators must later supply explicit classification evidence rather than +inferred kinds before the owning canonical actor migration. AUTH-03 review evidence and its PR trust bundle are recorded at `reviews/WS-AUTH-001-03-internal-review-evidence.md` and diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04-request-api-controls.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04-request-api-controls.md index cac18ee8..cda98b57 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04-request-api-controls.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04-request-api-controls.md @@ -1,5 +1,11 @@ # Chunk Contract: WS-AUTH-001-04 - Request, Error, And API Control Foundation +## Status + +Split before implementation after required plan review. This parent is not an +implementation contract. Its authorized children are `WS-AUTH-001-04A` and +`WS-AUTH-001-04B`; only 04A is currently active. + ## Parent initiative `WS-AUTH-001` - Workstream Authorization Service @@ -138,17 +144,10 @@ work already owns migration `0016`, so this chunk uses additive migration `0017_api_controls` from `0016_artifact_domain`; this is a sequence correction, not a scope expansion. -## Implementation Plan - -1. Add pure request-context/error primitives and one ASGI middleware that - validates IDs, normalizes all JSON error responses additively, preserves - response headers, and bounds unhandled failures. -2. Add feature-owned rate-control model, repository, service, and dependencies - with HMAC privacy keys, atomic PostgreSQL fixed windows, database time, - stable exceeded/unavailable errors, and no public endpoint activation. -3. Add migration `0017_api_controls`, model registration, bounded settings, - request/error/rate behavior tests, migration round-trip proof, and the - intentionally updated API contract drill. -4. Run focused and full isolated-PostgreSQL proof, at least 90 percent coverage - for the new subsystem, repository gates, all required internal reviewers, - evidence validation, and one ready PR. Do not start AUTH-05. +## Split implementation plan + +1. Implement and merge 04A request/error context under its repaired contract. +2. Update post-merge memory and stop. +3. Start 04B only after a separate explicit user signal; implement its durable + rate controls and migration under its own reviewed contract. +4. Do not start AUTH-05 automatically after either child. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md new file mode 100644 index 00000000..318b6513 --- /dev/null +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md @@ -0,0 +1,175 @@ +# Chunk Contract: WS-AUTH-001-04A - Request And Error Context + +## Parent initiative + +`WS-AUTH-001` - Workstream Authorization Service + +## Goal + +Add canonical request/correlation context and a stable privacy-bounded error +contract while preserving existing response fields, headers, routes, and +product authorization behavior. + +## Risk class and SLA + +L1 / P1 + +## Size circuit breaker + +- Stop at 350 changed non-comment production lines for a scope checkpoint. +- Stop and replan above 500 changed non-comment production lines. +- Test, documentation, generated schema, and durable loop evidence lines do not + count toward the production cap. + +## Allowed files + +```text +backend/app/main.py +backend/app/core/api_controls.py +backend/app/api/deps/auth.py +backend/app/modules/tasks/router.py +backend/tests/test_api_controls.py +backend/tests/test_app.py +backend/tests/test_auth.py +backend/tests/test_tasks.py +backend/scripts/api_contract_e2e.py +docs/operations_authorization_service.md +.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/** +.agent-loop/LOOP_STATE.md +.agent-loop/WORK_QUEUE.md +.agent-loop/REVIEW_LOG.md +``` + +## Not allowed + +```text +database models, migrations, rate counters, or rate-control settings +new routes or rate-control dependency attachments +product permission, role, actor-state, or identity behavior +AUTH-04B, AUTH-05, or later chunk implementation +weakened tests, coverage thresholds, or OpenAPI constraints +BaseHTTPMiddleware, response-body buffering, or response JSON rewriting +``` + +## Request-context contract + +- Inspect raw ASGI header pairs so duplicate `X-Request-ID` or + `X-Correlation-ID` fields cannot be collapsed by framework parsing. +- Accept zero or one value for each ID. Reject repeated fields, including + identical duplicates, and reject comma-joined values. +- An accepted ID is exactly 36 ASCII characters and the canonical lowercase, + hyphenated, non-nil RFC 4122 rendering of a UUID with version 1 through 8. +- Generate UUIDv4 for a missing request ID. A missing correlation ID reuses the + effective request ID. +- Invalid ID input produces a bounded `invalid_request` response with a fresh + safe request/correlation pair; supplied bytes are never reflected. +- Store effective IDs only on the request's ASGI scope state. Concurrent + requests must not share context. +- Every response, including malformed-ID, 404, 405, validation, authentication, + unhandled 500, streaming, and background-task responses, returns canonical + `X-Request-ID` and `X-Correlation-ID` headers. Application-supplied ID response + headers are overwritten with the effective pair. +- Use pure ASGI middleware that edits only `http.response.start` headers. It + must preserve status, unrelated duplicate headers, `WWW-Authenticate`, + `Retry-After`, content length, body chunks, streaming, and background work. + +## Error contract + +- The canonical nested object has required fields + `error.code/message/details/correlation_id/retryable`. +- Add the object for explicit API errors, `HTTPException`, request validation, + coded task-domain JSON errors, auth missing/invalid/unavailable conditions, + 404, 405, 409, 422, future 429/503 errors, and unhandled 500 responses. +- Default mappings are: 400/422 `invalid_request`, 401 `invalid_token`, 403 + `permission_not_granted`, 404 `resource_not_found`, 405 + `method_not_allowed`, 409 `conflict`, 429 `rate_limit_exceeded`, 503 + `service_unavailable`, and 500 `internal_error`. Existing auth branches may + emit a more specific already-adopted stable code. +- Existing top-level `detail`, `code`, and `details` fields remain present with + their current types and values. Coded task responses mirror their top-level + `code/details` into the nested object. +- Preserve status codes and compatibility headers, including exact + `WWW-Authenticate` and future `Retry-After` values. +- Validation uses a constant safe message and only the framework's already + redacted, bounded validation-location/type information. It never exposes + submitted input values. +- Unhandled errors use a constant message, empty details, and + `retryable=false` in both debug and non-debug modes. No exception, SQL, + token, claim, provider response, secret, or PII enters a response or log. +- Shared explicit response builders and exception handlers own JSON shapes. + The request-context middleware must not parse or rewrite response bodies. + +## OpenAPI and compatibility contract + +- Document the required canonical error object and only the optional legacy + compatibility fields that routes can actually return. +- Document request/correlation request headers, response ID headers, + `WWW-Authenticate`, and future `Retry-After` behavior where applicable. +- Existing task coded-error schemas retain `additionalProperties: false`, their + required top-level fields, and add required canonical `error` through an + explicit compatible schema or `oneOf` composition. +- No route path, method, response status, or dependency consumer is added, + removed, or attached by this chunk. Tests inventory the route set and relevant + dependency consumers before and after the change. + +## Behavior proof + +- Missing, valid, malformed, non-ASCII, nil, uppercase, non-hyphenated, + unsupported-version, repeated-identical, repeated-different, and comma-joined + ID inputs. +- Request-only, correlation-only, both-ID, and concurrent-request isolation. +- Success, validation, 400, 401, 403, 404, 405, 409, 422, representative coded + task errors, 500, streaming, background task, and duplicate unrelated header + preservation. +- Exact legacy field/status/header preservation and bounded redaction. +- OpenAPI response-schema validation plus unchanged path/method and dependency + consumer inventories. + +## Verification commands + +```bash +(cd backend && WORKSTREAM_DATABASE_URL= .venv/bin/python -m pytest -q tests/test_api_controls.py tests/test_app.py tests/test_auth.py tests/test_tasks.py) +(cd backend && WORKSTREAM_DATABASE_URL= .venv/bin/python -m pytest -q --cov=app.core.api_controls --cov=app.main --cov-report=term-missing --cov-fail-under=90 tests/test_api_controls.py tests/test_app.py tests/test_auth.py tests/test_tasks.py) +(cd backend && .venv/bin/python scripts/run_isolated_tests.py --metadata-json --timeout-seconds 12600 -- .venv/bin/python -m pytest -q --ignore=tests/test_isolated_database_runner.py --cov=app --cov-report=term-missing --cov-fail-under=78) +(cd backend && .venv/bin/python scripts/run_isolated_tests.py --metadata-json --timeout-seconds 12600 -- .venv/bin/python -m pytest -q --ignore=tests/test_isolated_database_runner.py) +(cd backend && WORKSTREAM_DATABASE_URL= .venv/bin/python scripts/api_contract_e2e.py) +(cd backend && .venv/bin/python -m ruff check app tests scripts) +python3 scripts/check_loop_memory_state.py +python3 scripts/check_stale_workstream_wording.py +python3 scripts/check_stale_authorization_docs.py +python3 scripts/check_stale_artifact_contracts.py +python3 scripts/check_markdown_links.py +python3 scripts/check_internal_review_evidence.py +python3 scripts/test_agent_gates.py +git diff --check +``` + +## Required reviewers + +- senior engineering +- QA/test +- security/auth +- product/ops +- architecture +- CI integrity +- docs +- reuse/dedup +- test delta + +## Human review focus + +Review duplicate-ID rejection, ASGI response preservation, error privacy, +legacy compatibility, OpenAPI truth, and unchanged product authority. + +## Stop conditions + +- Stop if a response body must be parsed or buffered in middleware. +- Stop if an existing response field/header/assertion would need weakening. +- Stop if production changes exceed the size circuit breaker. +- Stop after the reviewed 04A PR; do not start 04B or AUTH-05 automatically. + +## Activation + +The user explicitly started parent AUTH-04 on 2026-07-13. Required plan review +split it before implementation. Only this repaired 04A contract is active; +runtime edits remain gated on its required preimplementation re-review. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04B-postgres-rate-controls.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04B-postgres-rate-controls.md new file mode 100644 index 00000000..8b95deb0 --- /dev/null +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04B-postgres-rate-controls.md @@ -0,0 +1,151 @@ +# Chunk Contract: WS-AUTH-001-04B - PostgreSQL Rate Controls + +## Status + +Inactive. This contract requires AUTH-04A merge/memory, a separate explicit +user start, and required preimplementation review before any runtime edit. + +## Parent initiative + +`WS-AUTH-001` - Workstream Authorization Service + +## Goal + +Provide privacy-keyed, cross-replica PostgreSQL fixed-window rate controls for +future first-access and authority-management mutations without attaching them +to routes owned by later chunks. + +## Risk class and SLA + +L1 / P1 + +## Size circuit breaker + +- Stop at 350 changed non-comment production lines for a scope checkpoint. +- Stop and replan above 500 changed non-comment production lines. + +## Allowed files + +```text +backend/app/core/config.py +backend/app/api/deps/api_controls.py +backend/app/modules/api_controls/__init__.py +backend/app/modules/api_controls/models.py +backend/app/modules/api_controls/repository.py +backend/app/modules/api_controls/service.py +backend/app/db/models.py +backend/alembic/versions/0017_api_controls.py +backend/tests/test_api_rate_controls.py +backend/tests/test_config.py +backend/tests/test_alembic.py +backend/scripts/api_contract_e2e.py +docs/operations_authorization_service.md +.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/** +.agent-loop/LOOP_STATE.md +.agent-loop/WORK_QUEUE.md +.agent-loop/REVIEW_LOG.md +``` + +## Not allowed + +```text +request/error-context implementation owned by AUTH-04A +new routes or attachment to unrelated existing routes +first-access, grant, actor-state, or product authorization behavior +Redis/in-memory counters or persisted raw identity/network/token material +AUTH-05 or later chunk implementation +``` + +## Persistence and transaction contract + +- Persist one row per `(control_scope, key_digest)` with bounded scope, fixed + HMAC-SHA256 digest length, `window_started_at`, `window_expires_at`, + `request_count`, and `updated_at` using timezone-aware database timestamps. +- Enforce nonempty bounded scope, exact digest length, positive bounded count, + ordered window timestamps, uniqueness, and the lookup/expiry indexes required + by the executed query. +- One PostgreSQL upsert uses one `statement_timestamp()` value to reset an + expired window or atomically increment an active window and returns database + now, window start/end, and count. Saturate at the database integer maximum. +- Counts `1..limit` allow. Count `limit+1` and later deny and remain durably + consumed. `Retry-After` is the ceiling of remaining database-time seconds, + clamped to `1..window_seconds`. +- Invoke the upsert in a dedicated short-lived session/transaction and commit + before returning either allow or 429. A downstream request rollback cannot + undo rate consumption. + +## Privacy and configuration contract + +- Derive `key_digest` with HMAC-SHA256 using domain + `workstream-api-rate/v1`, the bounded control scope, and exact issuer plus + opaque subject bytes framed by four-byte big-endian lengths and UTF-8 values. +- Persist only scope and digest. Never persist or log raw issuer, subject, + actor ID, email, role, token, claims, or network address. +- Add `api_rate_limit_key_secret` with no default and minimum 32-byte secret + material. Never render or log it. Present malformed values fail settings + construction. +- Defaults are first-access `10/60s` and admin-mutation `30/60s`, with limit + bounds `1..10000` and window bounds `1..3600`. +- Missing secret remains startup-compatible while no route is attached, but + invoking a protected dependency without it returns a safe retryable 503. + Database unavailability also fails closed as safe retryable 503. +- Secret rotation intentionally resets effective counters. The runbook requires + quiescing protected writes for at most the largest configured window before + replacing the secret. + +## Route boundary and behavior proof + +- Export named first-access and admin-mutation dependencies for later owning + chunks, but attach neither dependency here. +- Inventory path/method and dependency consumers to prove no route or consumer + changed. +- Prove allow, exact limit, repeated exceed, expiry reset, distinct key/scope, + concurrent increments, cross-session visibility, downstream rollback + independence, missing/malformed secret, database unavailable, bounded + `Retry-After`, and privacy-safe persistence. + +## Migration proof + +- `0017_api_controls` revises `0016_artifact_domain`. +- Prove `0016 -> 0017 -> 0016 -> 0017`, seeded-row preservation across the + supported direction, unique/check/index enforcement, invalid insert + rejection, and the explicit nonempty-table downgrade policy. + +## Verification commands + +```bash +(cd backend && WORKSTREAM_DATABASE_URL= .venv/bin/python -m pytest -q tests/test_api_rate_controls.py tests/test_config.py tests/test_alembic.py) +(cd backend && WORKSTREAM_DATABASE_URL= .venv/bin/python -m pytest -q --cov=app.modules.api_controls --cov=app.api.deps.api_controls --cov-report=term-missing --cov-fail-under=90 tests/test_api_rate_controls.py tests/test_config.py tests/test_alembic.py) +(cd backend && .venv/bin/python scripts/run_isolated_tests.py --metadata-json --timeout-seconds 12600 -- .venv/bin/python -m pytest -q --ignore=tests/test_isolated_database_runner.py --cov=app --cov-report=term-missing --cov-fail-under=78) +(cd backend && .venv/bin/python scripts/run_isolated_tests.py --metadata-json --timeout-seconds 12600 -- .venv/bin/python -m pytest -q --ignore=tests/test_isolated_database_runner.py) +(cd backend && WORKSTREAM_DATABASE_URL= .venv/bin/python scripts/api_contract_e2e.py) +(cd backend && .venv/bin/python -m ruff check app tests scripts) +python3 scripts/check_loop_memory_state.py +python3 scripts/check_stale_workstream_wording.py +python3 scripts/check_stale_authorization_docs.py +python3 scripts/check_stale_artifact_contracts.py +python3 scripts/check_markdown_links.py +python3 scripts/check_internal_review_evidence.py +python3 scripts/test_agent_gates.py +git diff --check +``` + +## Required reviewers + +- senior engineering +- QA/test +- security/auth +- product/ops +- architecture +- CI integrity +- docs +- reuse/dedup +- test delta + +## Stop conditions + +- Stop if a rate count can be rolled back with downstream work. +- Stop if atomic database-time semantics cannot be proven under concurrency. +- Stop if a raw identity or token-derived value would be persisted or logged. +- Stop if production changes exceed the size circuit breaker. +- Stop after 04B; do not start AUTH-05 automatically. From 56117338c15ad4cf34eac8e751d66ea895bd1ad2 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 12:20:45 +0100 Subject: [PATCH 03/12] docs(auth): lock AUTH-04A behavior proof --- .agent-loop/REVIEW_LOG.md | 6 ++ .../WS-AUTH-001-04A-request-error-context.md | 65 +++++++++++++++---- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 38c7a3b5..8825bd42 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -14,6 +14,12 @@ preimplementation re-review are active. 04B requires 04A merge/memory and a separate explicit user start. No runtime implementation was written under the failed combined plan. +The first split re-review passed senior/architecture/security/product/docs/reuse +at `f01427a` but QA/CI/test-delta required one more 04A contract repair: exact +per-branch errors and invalid-ID behavior, bounded validation shapes, +CI-equivalent isolated-runner/docstring proof, per-file coverage, and additive +test-delta evidence. Runtime implementation remains gated on that re-review. + ## 2026-07-13 - WS-AUTH-001-04 Started PR #110 merged AUTH-03 post-merge memory to `main` as `1864867` after Backend, diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md index 318b6513..05deb4a8 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md @@ -58,11 +58,14 @@ BaseHTTPMiddleware, response-body buffering, or response JSON rewriting - Accept zero or one value for each ID. Reject repeated fields, including identical duplicates, and reject comma-joined values. - An accepted ID is exactly 36 ASCII characters and the canonical lowercase, - hyphenated, non-nil RFC 4122 rendering of a UUID with version 1 through 8. + hyphenated, non-nil RFC 9562 UUID rendering with RFC variant and version 1 + through 8. - Generate UUIDv4 for a missing request ID. A missing correlation ID reuses the effective request ID. -- Invalid ID input produces a bounded `invalid_request` response with a fresh - safe request/correlation pair; supplied bytes are never reflected. +- Invalid ID input short-circuits before any route or dependency runs and + returns HTTP 400 `invalid_request`. Generate one fresh UUIDv4 and use it as + both the safe request ID and safe correlation ID; supplied bytes are never + reflected. - Store effective IDs only on the request's ASGI scope state. Concurrent requests must not share context. - Every response, including malformed-ID, 404, 405, validation, authentication, @@ -80,19 +83,49 @@ BaseHTTPMiddleware, response-body buffering, or response JSON rewriting - Add the object for explicit API errors, `HTTPException`, request validation, coded task-domain JSON errors, auth missing/invalid/unavailable conditions, 404, 405, 409, 422, future 429/503 errors, and unhandled 500 responses. -- Default mappings are: 400/422 `invalid_request`, 401 `invalid_token`, 403 - `permission_not_granted`, 404 `resource_not_found`, 405 - `method_not_allowed`, 409 `conflict`, 429 `rate_limit_exceeded`, 503 - `service_unavailable`, and 500 `internal_error`. Existing auth branches may - emit a more specific already-adopted stable code. +- Use the following exact branch mappings. Every unspecified `details` value is + `{}`. Compatibility `detail`, status, and headers remain as stated below. + +| Condition | HTTP | Code | Canonical message | Details | Retryable | +|---|---:|---|---|---|---| +| invalid request/correlation ID | 400 | `invalid_request` | `Invalid request identifier` | `{}` | false | +| request validation | 422 | `invalid_request` | `Request validation failed` | bounded validation summary | false | +| missing bearer token | 401 | `missing_token` | `Missing bearer token` | `{}` | false | +| invalid bearer token | 401 | `invalid_token` | `Invalid bearer token` | `{}` | false | +| verifier unavailable | 503 | `identity_verification_unavailable` | `Identity verification unavailable` | `{}` | true | +| unsupported verified subject kind | 403 | `unsupported_subject_kind` | `Unsupported subject kind` | `{}` | false | +| actor registry unavailable | 503 | `service_unavailable` | `Service unavailable` | `{}` | true | +| coded pre-submit failure | 422 | `pre_submission_checker_failed` | `Pre-submission checks failed` | exact legacy domain details | false | +| coded locked-context failure | 422 | `task_locked_context_invalid` | `Task locked context is invalid` | exact legacy domain details | false | +| other HTTP 400 | 400 | `invalid_request` | `Invalid request` | `{}` | false | +| other HTTP 401 | 401 | `invalid_token` | `Invalid bearer token` | `{}` | false | +| other HTTP 403 | 403 | `permission_not_granted` | `Permission not granted` | `{}` | false | +| other HTTP 404 | 404 | `resource_not_found` | `Resource not found` | `{}` | false | +| other HTTP 405 | 405 | `method_not_allowed` | `Method not allowed` | `{}` | false | +| other HTTP 409 | 409 | `conflict` | `Request conflict` | `{}` | false | +| other HTTP 422 | 422 | `invalid_request` | `Invalid request` | `{}` | false | +| future HTTP 429 | 429 | `rate_limit_exceeded` | `Rate limit exceeded` | `{}` | true | +| other HTTP 503 | 503 | `service_unavailable` | `Service unavailable` | `{}` | true | +| unhandled exception | 500 | `internal_error` | `Internal server error` | `{}` | false | + +- Actor-registry domain exceptions retain their existing status and top-level + `detail`; their nested code/message/retryable values use the matching generic + status row unless the exact unavailable branch above applies. - Existing top-level `detail`, `code`, and `details` fields remain present with their current types and values. Coded task responses mirror their top-level `code/details` into the nested object. - Preserve status codes and compatibility headers, including exact `WWW-Authenticate` and future `Retry-After` values. -- Validation uses a constant safe message and only the framework's already - redacted, bounded validation-location/type information. It never exposes - submitted input values. +- Validation returns at most 20 errors. Each nested item contains only `type` + and `loc`: `type` is capped at 64 ASCII code characters; `loc` contains at + most eight integer or string components, with strings capped at 64 ASCII + characters and unsupported components replaced by `redacted`. The nested + details shape is `{"errors": [...], "truncated": }`. +- The compatibility `detail` remains a list and preserves the tested `loc` and + `input="redacted"` behavior, but applies the same 20-error/eight-location/ + 64-character bounds, uses constant `msg="Invalid value"`, and emits only + `type`, `loc`, `msg`, and `input`. It never exposes submitted values, exception + text, validation context, URLs, or provider data. - Unhandled errors use a constant message, empty details, and `retryable=false` in both debug and non-debug modes. No exception, SQL, token, claim, provider response, secret, or PII enters a response or log. @@ -124,16 +157,23 @@ BaseHTTPMiddleware, response-body buffering, or response JSON rewriting - Exact legacy field/status/header preservation and bounded redaction. - OpenAPI response-schema validation plus unchanged path/method and dependency consumer inventories. +- Test-delta evidence proves existing test files only gain assertions/fixtures: + no assertion, `pytest.raises`, skip/xfail marker, or `skipTest` line is removed + or relaxed. New behavior runs through real FastAPI routes and ASGI transport, + not direct handler-only tests or dependency overrides that bypass middleware. ## Verification commands ```bash (cd backend && WORKSTREAM_DATABASE_URL= .venv/bin/python -m pytest -q tests/test_api_controls.py tests/test_app.py tests/test_auth.py tests/test_tasks.py) -(cd backend && WORKSTREAM_DATABASE_URL= .venv/bin/python -m pytest -q --cov=app.core.api_controls --cov=app.main --cov-report=term-missing --cov-fail-under=90 tests/test_api_controls.py tests/test_app.py tests/test_auth.py tests/test_tasks.py) +(cd backend && WORKSTREAM_DATABASE_URL= .venv/bin/python -m pytest -q --cov=app --cov-report=term-missing tests/test_api_controls.py tests/test_app.py tests/test_auth.py tests/test_tasks.py) +(cd backend && for file in app/core/api_controls.py app/main.py app/api/deps/auth.py app/modules/tasks/router.py; do .venv/bin/coverage report --include="$file" --precision=2 --fail-under=90; done) +(cd backend && WORKSTREAM_TEST_ADMIN_DATABASE_URL= .venv/bin/python -m pytest -q tests/test_isolated_database_runner.py) (cd backend && .venv/bin/python scripts/run_isolated_tests.py --metadata-json --timeout-seconds 12600 -- .venv/bin/python -m pytest -q --ignore=tests/test_isolated_database_runner.py --cov=app --cov-report=term-missing --cov-fail-under=78) (cd backend && .venv/bin/python scripts/run_isolated_tests.py --metadata-json --timeout-seconds 12600 -- .venv/bin/python -m pytest -q --ignore=tests/test_isolated_database_runner.py) (cd backend && WORKSTREAM_DATABASE_URL= .venv/bin/python scripts/api_contract_e2e.py) (cd backend && .venv/bin/python -m ruff check app tests scripts) +(cd backend && .venv/bin/docstr-coverage --config .docstr.yaml) python3 scripts/check_loop_memory_state.py python3 scripts/check_stale_workstream_wording.py python3 scripts/check_stale_authorization_docs.py @@ -141,6 +181,7 @@ python3 scripts/check_stale_artifact_contracts.py python3 scripts/check_markdown_links.py python3 scripts/check_internal_review_evidence.py python3 scripts/test_agent_gates.py +git diff --unified=0 origin/main...HEAD -- backend/tests | (! rg '^-(.*assert|.*pytest\.raises|.*pytest\.mark\.(skip|xfail)|.*skipTest)') git diff --check ``` From f98bbfcf652360abe7133c73aff003ebd806d388 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 12:23:38 +0100 Subject: [PATCH 04/12] docs(auth): preserve validation compatibility --- .../WS-AUTH-001-04A-request-error-context.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md index 05deb4a8..127fea51 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md @@ -121,11 +121,15 @@ BaseHTTPMiddleware, response-body buffering, or response JSON rewriting most eight integer or string components, with strings capped at 64 ASCII characters and unsupported components replaced by `redacted`. The nested details shape is `{"errors": [...], "truncated": }`. -- The compatibility `detail` remains a list and preserves the tested `loc` and - `input="redacted"` behavior, but applies the same 20-error/eight-location/ - 64-character bounds, uses constant `msg="Invalid value"`, and emits only - `type`, `loc`, `msg`, and `input`. It never exposes submitted values, exception - text, validation context, URLs, or provider data. +- The compatibility `detail` remains the existing sanitized list in original + order for the first 20 errors. Preserve every currently emitted safe + field/value and validator message, including tested `loc`, constant-redacted + `input`, and class-name-only exception context. Only the 20-item list cap is + new; do not truncate locations/messages or remove fields from these retained + compatibility entries. Submitted input, exception values, and non-finite + numbers continue to use the existing redaction/JSON-safety helpers. The strict + eight-location/64-character/type-only privacy shape applies only to nested + `error.details`. - Unhandled errors use a constant message, empty details, and `retryable=false` in both debug and non-debug modes. No exception, SQL, token, claim, provider response, secret, or PII enters a response or log. From 11ca277297bebe0b9b8a448e849525d1560286e5 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 17:02:09 +0100 Subject: [PATCH 05/12] feat(auth): add request and error context --- backend/app/api/deps/auth.py | 25 +- backend/app/core/api_controls.py | 309 ++++++++++++++++++ backend/app/main.py | 127 +++++++- backend/app/modules/tasks/router.py | 41 ++- backend/scripts/api_contract_e2e.py | 17 +- backend/tests/test_api_controls.py | 386 +++++++++++++++++++++++ backend/tests/test_auth.py | 15 + backend/tests/test_tasks.py | 69 ++++ docs/operations_authorization_service.md | 46 +++ 9 files changed, 1007 insertions(+), 28 deletions(-) create mode 100644 backend/app/core/api_controls.py create mode 100644 backend/tests/test_api_controls.py diff --git a/backend/app/api/deps/auth.py b/backend/app/api/deps/auth.py index 82c13f7b..1901b325 100644 --- a/backend/app/api/deps/auth.py +++ b/backend/app/api/deps/auth.py @@ -9,6 +9,7 @@ from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.ext.asyncio import AsyncSession +from app.core.api_controls import StructuredHTTPException from app.core.auth import get_auth_verifier from app.db.session import get_db_session from app.interfaces.auth import ( @@ -29,7 +30,7 @@ def get_application_auth_verifier(request: Request) -> AuthVerifier: return request.app.state.auth_verifier -def unauthorized(detail: str) -> HTTPException: +def unauthorized(detail: str, *, error_code: str) -> HTTPException: """Build a bearer-auth unauthorized response. Args: @@ -38,9 +39,11 @@ def unauthorized(detail: str) -> HTTPException: Returns: HTTP exception with the bearer challenge header. """ - return HTTPException( + return StructuredHTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail=detail, + error_code=error_code, + error_message=detail, headers={"WWW-Authenticate": "Bearer"}, ) @@ -62,17 +65,20 @@ async def get_auth_verification_result( HTTPException: If the bearer token is missing or invalid. """ if credentials is None: - raise unauthorized("Missing bearer token") + raise unauthorized("Missing bearer token", error_code="missing_token") try: return await verifier.verify(credentials.credentials) except AuthVerificationUnavailableError as exc: - raise HTTPException( + raise StructuredHTTPException( status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="Identity verification unavailable", + error_code="identity_verification_unavailable", + error_message="Identity verification unavailable", + retryable=True, ) from exc except AuthVerificationError as exc: - raise unauthorized("Invalid bearer token") from exc + raise unauthorized("Invalid bearer token", error_code="invalid_token") from exc async def get_current_actor( @@ -97,9 +103,11 @@ async def get_registered_actor( this token-derived context, not persisted profile rows. """ if result.token.subject_kind != "human" or result.legacy is None: - raise HTTPException( + raise StructuredHTTPException( status_code=status.HTTP_403_FORBIDDEN, detail="Unsupported subject kind", + error_code="unsupported_subject_kind", + error_message="Unsupported subject kind", ) actor = result.legacy_actor() try: @@ -109,8 +117,11 @@ async def get_registered_actor( raise HTTPException(status_code=exc.status_code, detail=str(exc)) from exc except SQLAlchemyError as exc: await session.rollback() - raise HTTPException( + raise StructuredHTTPException( status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="Actor registry unavailable", + error_code="service_unavailable", + error_message="Service unavailable", + retryable=True, ) from exc return actor diff --git a/backend/app/core/api_controls.py b/backend/app/core/api_controls.py new file mode 100644 index 00000000..c004aa96 --- /dev/null +++ b/backend/app/core/api_controls.py @@ -0,0 +1,309 @@ +"""Request context and stable API error primitives.""" + +from __future__ import annotations + +from collections.abc import Mapping, Sequence +import logging +from typing import Any +from uuid import RFC_4122, UUID, uuid4 + +from fastapi import FastAPI, HTTPException, Request +from fastapi.responses import JSONResponse +from pydantic import BaseModel, ConfigDict +from starlette.types import ASGIApp, Message, Receive, Scope, Send + +REQUEST_ID_HEADER = b"x-request-id" +CORRELATION_ID_HEADER = b"x-correlation-id" +REQUEST_ID_STATE_KEY = "request_id" +CORRELATION_ID_STATE_KEY = "correlation_id" +LOGGER = logging.getLogger(__name__) + + +class ApiError(BaseModel): + """Canonical machine-readable API error.""" + + model_config = ConfigDict(extra="forbid") + + code: str + message: str + details: dict[str, Any] + correlation_id: str + retryable: bool + + +class ApiErrorResponse(BaseModel): + """Canonical error plus additive legacy compatibility fields.""" + + model_config = ConfigDict(extra="forbid") + + error: ApiError + detail: Any | None = None + code: str | None = None + details: dict[str, Any] | None = None + + +class StructuredHTTPException(HTTPException): + """HTTP exception carrying an explicit canonical error classification.""" + + def __init__( + self, + *, + status_code: int, + detail: Any, + error_code: str, + error_message: str, + retryable: bool = False, + headers: Mapping[str, str] | None = None, + ) -> None: + super().__init__(status_code=status_code, detail=detail, headers=headers) + self.error_code = error_code + self.error_message = error_message + self.retryable = retryable + + +def request_ids(request: Request) -> tuple[str, str]: + """Return the request-scoped canonical request and correlation IDs.""" + state = request.scope.setdefault("state", {}) + request_id = state.get(REQUEST_ID_STATE_KEY) + correlation_id = state.get(CORRELATION_ID_STATE_KEY) + if not isinstance(request_id, str) or not isinstance(correlation_id, str): + fallback = str(uuid4()) + state[REQUEST_ID_STATE_KEY] = fallback + state[CORRELATION_ID_STATE_KEY] = fallback + return fallback, fallback + return request_id, correlation_id + + +def error_payload( + request: Request, + *, + code: str, + message: str, + details: dict[str, Any] | None = None, + retryable: bool = False, + compatibility: Mapping[str, Any] | None = None, +) -> dict[str, Any]: + """Build one canonical error body with optional legacy fields.""" + _, correlation_id = request_ids(request) + payload: dict[str, Any] = dict(compatibility or {}) + payload["error"] = { + "code": code, + "message": message, + "details": details or {}, + "correlation_id": correlation_id, + "retryable": retryable, + } + return payload + + +def error_response( + request: Request, + *, + status_code: int, + code: str, + message: str, + details: dict[str, Any] | None = None, + retryable: bool = False, + compatibility: Mapping[str, Any] | None = None, + headers: Mapping[str, str] | None = None, +) -> JSONResponse: + """Return a structured JSON error with canonical response IDs.""" + request_id, correlation_id = request_ids(request) + response_headers = dict(headers or {}) + response_headers["X-Request-ID"] = request_id + response_headers["X-Correlation-ID"] = correlation_id + return JSONResponse( + status_code=status_code, + content=error_payload( + request, + code=code, + message=message, + details=details, + retryable=retryable, + compatibility=compatibility, + ), + headers=response_headers, + ) + + +def _header_value(scope: Scope, name: bytes) -> bytes | None: + values = [value for key, value in scope.get("headers", ()) if key.lower() == name] + if len(values) > 1: + raise ValueError("duplicate request context header") + if not values: + return None + if b"," in values[0]: + raise ValueError("combined request context header") + return values[0] + + +def _canonical_uuid(value: bytes) -> str: + if len(value) != 36: + raise ValueError("invalid request context header") + try: + text = value.decode("ascii") + parsed = UUID(text) + except (UnicodeDecodeError, ValueError) as exc: + raise ValueError("invalid request context header") from exc + if ( + parsed.int == 0 + or parsed.variant != RFC_4122 + or parsed.version not in range(1, 9) + or str(parsed) != text + ): + raise ValueError("invalid request context header") + return text + + +def _resolve_request_ids(scope: Scope) -> tuple[str, str]: + raw_request_id = _header_value(scope, REQUEST_ID_HEADER) + raw_correlation_id = _header_value(scope, CORRELATION_ID_HEADER) + request_id = str(uuid4()) if raw_request_id is None else _canonical_uuid(raw_request_id) + correlation_id = ( + request_id if raw_correlation_id is None else _canonical_uuid(raw_correlation_id) + ) + return request_id, correlation_id + + +def _response_headers( + headers: Sequence[tuple[bytes, bytes]], request_id: str, correlation_id: str +) -> list[tuple[bytes, bytes]]: + retained = [ + (name, value) + for name, value in headers + if name.lower() not in {REQUEST_ID_HEADER, CORRELATION_ID_HEADER} + ] + retained.extend( + [ + (REQUEST_ID_HEADER, request_id.encode("ascii")), + (CORRELATION_ID_HEADER, correlation_id.encode("ascii")), + ] + ) + return retained + + +class RequestContextMiddleware: + """Validate request context and append canonical IDs without buffering.""" + + def __init__(self, app: ASGIApp) -> None: + self.app = app + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] != "http": + await self.app(scope, receive, send) + return + + state = scope.setdefault("state", {}) + try: + request_id, correlation_id = _resolve_request_ids(scope) + except ValueError: + request_id = correlation_id = str(uuid4()) + state[REQUEST_ID_STATE_KEY] = request_id + state[CORRELATION_ID_STATE_KEY] = correlation_id + response = error_response( + Request(scope, receive), + status_code=400, + code="invalid_request", + message="Invalid request identifier", + compatibility={"detail": "Invalid request identifier"}, + ) + await response(scope, receive, send) + return + + state[REQUEST_ID_STATE_KEY] = request_id + state[CORRELATION_ID_STATE_KEY] = correlation_id + response_started = False + + async def send_with_context(message: Message) -> None: + nonlocal response_started + if message["type"] == "http.response.start": + response_started = True + message["headers"] = _response_headers( + message.get("headers", ()), request_id, correlation_id + ) + await send(message) + + try: + await self.app(scope, receive, send_with_context) + except Exception: + if response_started: + LOGGER.error( + "request_failed_after_response_start", + extra={"correlation_id": correlation_id}, + ) + return + response = error_response( + Request(scope, receive), + status_code=500, + code="internal_error", + message="Internal server error", + compatibility={"detail": "Internal server error"}, + ) + await response(scope, receive, send) + + +def install_api_control_openapi(app: FastAPI) -> None: + """Document request context headers across the existing route inventory.""" + original_openapi = app.openapi + + def openapi() -> dict[str, Any]: + if app.openapi_schema is not None: + return app.openapi_schema + schema = original_openapi() + schemas = schema.setdefault("components", {}).setdefault("schemas", {}) + schemas["HTTPValidationError"] = { + "type": "object", + "required": ["detail", "error"], + "properties": { + "detail": {"type": "array", "items": {"type": "object"}}, + "error": {"$ref": "#/components/schemas/ApiError"}, + }, + "additionalProperties": False, + } + request_parameters = tuple( + { + "name": header, + "in": "header", + "required": False, + "schema": { + "type": "string", + "format": "uuid", + "minLength": 36, + "maxLength": 36, + }, + } + for header in ("X-Request-ID", "X-Correlation-ID") + ) + response_headers = { + "X-Request-ID": { + "required": True, + "schema": {"type": "string", "format": "uuid"}, + }, + "X-Correlation-ID": { + "required": True, + "schema": {"type": "string", "format": "uuid"}, + }, + } + methods = {"get", "put", "post", "delete", "options", "head", "patch", "trace"} + for path_item in schema.get("paths", {}).values(): + for method, operation in path_item.items(): + if method not in methods: + continue + parameters = operation.setdefault("parameters", []) + present = { + (parameter.get("in"), parameter.get("name", "").lower()) + for parameter in parameters + if isinstance(parameter, dict) + } + for parameter in request_parameters: + identity = (parameter["in"], parameter["name"].lower()) + if identity not in present: + parameters.append(parameter) + for response in operation.get("responses", {}).values(): + headers = response.setdefault("headers", {}) + for name, header_schema in response_headers.items(): + headers.setdefault(name, header_schema) + app.openapi_schema = schema + return schema + + app.openapi = openapi diff --git a/backend/app/main.py b/backend/app/main.py index 15adb0dc..01b1f7df 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -11,12 +11,52 @@ from fastapi.encoders import jsonable_encoder from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse +from starlette.exceptions import HTTPException as StarletteHTTPException from app.api.router import api_router +from app.core.api_controls import ( + ApiErrorResponse, + RequestContextMiddleware, + StructuredHTTPException, + error_response, + install_api_control_openapi, +) from app.core.auth import build_auth_verifier, cache_auth_verifier, prepare_auth_verifier from app.core.config import Settings, get_settings PRODUCTION_LIKE_ENVIRONMENTS = {"staging", "preview", "prod", "production"} +MAX_VALIDATION_ERRORS = 20 +MAX_VALIDATION_LOCATION_PARTS = 8 +MAX_VALIDATION_CODE_LENGTH = 64 +ERROR_RESPONSE_HEADERS = { + "X-Request-ID": {"schema": {"type": "string", "format": "uuid"}}, + "X-Correlation-ID": {"schema": {"type": "string", "format": "uuid"}}, +} +DEFAULT_ERROR_RESPONSES = { + status_code: { + "model": ApiErrorResponse, + "description": description, + "headers": dict(ERROR_RESPONSE_HEADERS), + } + for status_code, description in { + 400: "Invalid request.", + 401: "Authentication failed.", + 403: "Permission denied.", + 404: "Resource not found.", + 405: "Method not allowed.", + 409: "Request conflict.", + 422: "Request validation failed.", + 429: "Rate limit exceeded.", + 500: "Internal server error.", + 503: "Service unavailable.", + }.items() +} +DEFAULT_ERROR_RESPONSES[401]["headers"]["WWW-Authenticate"] = { + "schema": {"type": "string"} +} +DEFAULT_ERROR_RESPONSES[429]["headers"]["Retry-After"] = { + "schema": {"type": "integer", "minimum": 1} +} @asynccontextmanager @@ -73,18 +113,86 @@ def _json_safe_validation_value(value: Any) -> Any: return value +def _validation_summary(error: dict[str, Any]) -> dict[str, Any]: + """Return bounded type/location evidence for the canonical error object.""" + raw_type = error.get("type") + error_type = raw_type if isinstance(raw_type, str) and raw_type.isascii() else "validation_error" + error_type = error_type[:MAX_VALIDATION_CODE_LENGTH] + location: list[int | str] = [] + raw_location = error.get("loc") + if isinstance(raw_location, (list, tuple)): + for part in raw_location[:MAX_VALIDATION_LOCATION_PARTS]: + if isinstance(part, int): + location.append(part) + elif isinstance(part, str) and part.isascii(): + location.append(part[:MAX_VALIDATION_CODE_LENGTH]) + else: + location.append("redacted") + return {"type": error_type, "loc": location} + + async def request_validation_exception_handler( - _request: Request, + request: Request, exc: RequestValidationError, ) -> JSONResponse: """Return validation errors without echoing non-finite JSON values.""" - return JSONResponse( + errors = exc.errors() + compatibility_detail = [ + _validation_error_detail(error) for error in errors[:MAX_VALIDATION_ERRORS] + ] + return error_response( + request, status_code=422, - content=jsonable_encoder( - { - "detail": [_validation_error_detail(error) for error in exc.errors()], - } - ), + code="invalid_request", + message="Request validation failed", + details={ + "errors": [_validation_summary(error) for error in errors[:MAX_VALIDATION_ERRORS]], + "truncated": len(errors) > MAX_VALIDATION_ERRORS, + }, + compatibility={"detail": jsonable_encoder(compatibility_detail)}, + ) + + +def _generic_http_error(status_code: int) -> tuple[str, str, bool]: + """Return the canonical fallback classification for an HTTP status.""" + return { + 400: ("invalid_request", "Invalid request", False), + 401: ("invalid_token", "Invalid bearer token", False), + 403: ("permission_not_granted", "Permission not granted", False), + 404: ("resource_not_found", "Resource not found", False), + 405: ("method_not_allowed", "Method not allowed", False), + 409: ("conflict", "Request conflict", False), + 422: ("invalid_request", "Invalid request", False), + 429: ("rate_limit_exceeded", "Rate limit exceeded", True), + 503: ("service_unavailable", "Service unavailable", True), + }.get(status_code, ("http_error", "Request failed", False)) + + +async def http_exception_handler(request: Request, exc: StarletteHTTPException) -> JSONResponse: + """Add the canonical envelope while retaining FastAPI compatibility fields.""" + if isinstance(exc, StructuredHTTPException): + code, message, retryable = exc.error_code, exc.error_message, exc.retryable + else: + code, message, retryable = _generic_http_error(exc.status_code) + return error_response( + request, + status_code=exc.status_code, + code=code, + message=message, + retryable=retryable, + compatibility={"detail": exc.detail}, + headers=exc.headers, + ) + + +async def unhandled_exception_handler(request: Request, _exc: Exception) -> JSONResponse: + """Return a constant private response for an unhandled application error.""" + return error_response( + request, + status_code=500, + code="internal_error", + message="Internal server error", + compatibility={"detail": "Internal server error"}, ) @@ -103,6 +211,7 @@ def create_app(settings: Settings | None = None) -> FastAPI: version=settings.app_version, debug=settings.debug, lifespan=_application_lifespan, + responses=DEFAULT_ERROR_RESPONSES, ) app.state.settings = settings app.state.auth_verifier, app.state.auth_configuration_valid = prepare_auth_verifier(settings) @@ -111,7 +220,11 @@ def create_app(settings: Settings | None = None) -> FastAPI: RequestValidationError, request_validation_exception_handler, ) + app.add_exception_handler(StarletteHTTPException, http_exception_handler) + app.add_exception_handler(Exception, unhandled_exception_handler) + app.add_middleware(RequestContextMiddleware) app.include_router(api_router) + install_api_control_openapi(app) return app diff --git a/backend/app/modules/tasks/router.py b/backend/app/modules/tasks/router.py index 03adbab1..f157c4c1 100644 --- a/backend/app/modules/tasks/router.py +++ b/backend/app/modules/tasks/router.py @@ -4,11 +4,12 @@ from typing import Annotated -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Request from fastapi.responses import JSONResponse from sqlalchemy.ext.asyncio import AsyncSession from app.api.deps.auth import get_registered_actor +from app.core.api_controls import error_response from app.core.permissions import PermissionDenied from app.db.session import get_db_session from app.modules.actors.schemas import ActorProfileActivationRequest, ActorProfileResponse @@ -30,17 +31,19 @@ router = APIRouter(tags=["tasks"]) +CANONICAL_ERROR_OBJECT_SCHEMA = {"$ref": "#/components/schemas/ApiError"} PRE_SUBMIT_DOMAIN_ERROR_RESPONSE_SCHEMA = { "oneOf": [ { "type": "object", - "required": ["code", "details"], + "required": ["code", "details", "error"], "properties": { "code": { "type": "string", "enum": ["pre_submission_checker_failed"], }, "details": {"type": "object"}, + "error": CANONICAL_ERROR_OBJECT_SCHEMA, }, "additionalProperties": False, }, @@ -51,13 +54,14 @@ "oneOf": [ { "type": "object", - "required": ["code", "details"], + "required": ["code", "details", "error"], "properties": { "code": { "type": "string", "enum": ["task_locked_context_invalid"], }, "details": {"type": "object"}, + "error": CANONICAL_ERROR_OBJECT_SCHEMA, }, "additionalProperties": False, }, @@ -78,14 +82,21 @@ def task_http_error(exc: TaskServiceError) -> HTTPException: return HTTPException(status_code=exc.status_code, detail=str(exc)) -def task_domain_error_response(exc: TaskServiceError) -> JSONResponse: +def task_domain_error_response(request: Request, exc: TaskServiceError) -> JSONResponse: """Convert a coded domain error into the public API error body.""" - return JSONResponse( + code = getattr(exc, "code") + details = getattr(exc, "details", None) or {} + message = { + "pre_submission_checker_failed": "Pre-submission checks failed", + "task_locked_context_invalid": "Task locked context is invalid", + }[code] + return error_response( + request, status_code=exc.status_code, - content={ - "code": getattr(exc, "code"), - "details": getattr(exc, "details", None) or {}, - }, + code=code, + message=message, + details=details, + compatibility={"code": code, "details": details}, ) @@ -180,6 +191,7 @@ async def get_task( }, ) async def get_task_work_context( + request: Request, task_id: str, actor: Annotated[ActorContext, Depends(get_registered_actor)], session: Annotated[AsyncSession, Depends(get_db_session)], @@ -191,7 +203,7 @@ async def get_task_work_context( raise permission_http_error(exc) from exc except TaskServiceError as exc: if getattr(exc, "code", None) is not None: - return task_domain_error_response(exc) + return task_domain_error_response(request, exc) raise task_http_error(exc) from exc @@ -211,6 +223,7 @@ async def get_task_work_context( }, ) async def get_task_submission_requirements( + request: Request, task_id: str, actor: Annotated[ActorContext, Depends(get_registered_actor)], session: Annotated[AsyncSession, Depends(get_db_session)], @@ -222,7 +235,7 @@ async def get_task_submission_requirements( raise permission_http_error(exc) from exc except TaskServiceError as exc: if getattr(exc, "code", None) is not None: - return task_domain_error_response(exc) + return task_domain_error_response(request, exc) raise task_http_error(exc) from exc @@ -242,6 +255,7 @@ async def get_task_submission_requirements( }, ) async def get_task_locked_context( + request: Request, task_id: str, actor: Annotated[ActorContext, Depends(get_registered_actor)], session: Annotated[AsyncSession, Depends(get_db_session)], @@ -253,7 +267,7 @@ async def get_task_locked_context( raise permission_http_error(exc) from exc except TaskServiceError as exc: if getattr(exc, "code", None) is not None: - return task_domain_error_response(exc) + return task_domain_error_response(request, exc) raise task_http_error(exc) from exc @@ -370,6 +384,7 @@ async def start_task( }, ) async def create_submission( + request: Request, task_id: str, payload: SubmissionCreate, actor: Annotated[ActorContext, Depends(get_registered_actor)], @@ -382,7 +397,7 @@ async def create_submission( raise permission_http_error(exc) from exc except TaskServiceError as exc: if getattr(exc, "code", None) is not None: - return task_domain_error_response(exc) + return task_domain_error_response(request, exc) raise task_http_error(exc) from exc diff --git a/backend/scripts/api_contract_e2e.py b/backend/scripts/api_contract_e2e.py index 5e95ea40..87919274 100644 --- a/backend/scripts/api_contract_e2e.py +++ b/backend/scripts/api_contract_e2e.py @@ -300,10 +300,16 @@ async def request_json( Raises: AssertionError: If the response status does not match. """ + request_id = str(uuid4()) + correlation_id = str(uuid4()) + headers = {} if token is None else auth_headers(token) + headers.update( + {"X-Request-ID": request_id, "X-Correlation-ID": correlation_id} + ) response = await client.request( method, path, - headers={} if token is None else auth_headers(token), + headers=headers, json=payload, ) if response.status_code != expected_status: @@ -322,6 +328,15 @@ async def request_json( ) from exc if not isinstance(body, dict | list): raise AssertionError(f"{method} {path} returned non-JSON payload: {body!r}") + if response.headers.get("x-request-id") != request_id: + raise AssertionError(f"{method} {path} did not preserve the request ID") + if response.headers.get("x-correlation-id") != correlation_id: + raise AssertionError(f"{method} {path} did not preserve the correlation ID") + if expected_status >= 400: + if not isinstance(body, dict) or body.get("error", {}).get( + "correlation_id" + ) != correlation_id: + raise AssertionError(f"{method} {path} returned invalid error context") print(f"PASS {method} {path} -> {response.status_code}") return body diff --git a/backend/tests/test_api_controls.py b/backend/tests/test_api_controls.py new file mode 100644 index 00000000..0f3dbb45 --- /dev/null +++ b/backend/tests/test_api_controls.py @@ -0,0 +1,386 @@ +from __future__ import annotations + +import asyncio +from typing import Annotated +from uuid import UUID, uuid4 + +from fastapi import BackgroundTasks, HTTPException +from fastapi.exceptions import RequestValidationError +from fastapi.responses import Response, StreamingResponse +from httpx import ASGITransport, AsyncClient +from pydantic import BaseModel, Field, field_validator +import pytest +from starlette.requests import Request + +from app.core.api_controls import error_response +from app.core.config import Settings +from app.main import create_app + + +def _assert_uuid(value: str) -> None: + parsed = UUID(value) + assert str(parsed) == value + assert parsed.int != 0 + assert parsed.version in range(1, 9) + + +def test_error_response_fallback_ids_remain_consistent() -> None: + request = Request({"type": "http", "method": "GET", "path": "/", "headers": []}) + + response = error_response( + request, + status_code=500, + code="internal_error", + message="Internal server error", + ) + + correlation_id = response.headers["x-correlation-id"] + assert response.headers["x-request-id"] == correlation_id + assert f'"correlation_id":"{correlation_id}"'.encode() in response.body + + +async def _get(app, path: str = "/health", **kwargs): + async with AsyncClient( + transport=ASGITransport(app=app, raise_app_exceptions=False), + base_url="http://testserver", + ) as client: + return await client.get(path, **kwargs) + + +async def test_missing_and_supplied_request_context_propagates() -> None: + app = create_app() + generated = await _get(app) + request_id = str(uuid4()) + correlation_id = str(uuid4()) + supplied = await _get( + app, + headers={"X-Request-ID": request_id, "X-Correlation-ID": correlation_id}, + ) + request_only = await _get(app, headers={"X-Request-ID": request_id}) + correlation_only = await _get(app, headers={"X-Correlation-ID": correlation_id}) + + _assert_uuid(generated.headers["x-request-id"]) + assert generated.headers["x-correlation-id"] == generated.headers["x-request-id"] + assert supplied.headers["x-request-id"] == request_id + assert supplied.headers["x-correlation-id"] == correlation_id + assert request_only.headers["x-request-id"] == request_id + assert request_only.headers["x-correlation-id"] == request_id + _assert_uuid(correlation_only.headers["x-request-id"]) + assert correlation_only.headers["x-correlation-id"] == correlation_id + + +@pytest.mark.parametrize( + "invalid_id", + [ + str(uuid4()).upper(), + str(uuid4()).replace("-", ""), + "00000000-0000-0000-0000-000000000000", + "00000000-0000-9000-8000-000000000001", + f"{uuid4()}, {uuid4()}", + ], +) +async def test_invalid_request_context_short_circuits_without_reflection( + invalid_id: str, +) -> None: + app = create_app() + called = False + + @app.get("/_test/not-called") + async def not_called() -> dict[str, bool]: + nonlocal called + called = True + return {"called": True} + + response = await _get(app, "/_test/not-called", headers={"X-Request-ID": invalid_id}) + + assert response.status_code == 400 + assert called is False + assert response.headers["x-request-id"] == response.headers["x-correlation-id"] + _assert_uuid(response.headers["x-request-id"]) + assert invalid_id not in response.text + assert response.json() == { + "detail": "Invalid request identifier", + "error": { + "code": "invalid_request", + "message": "Invalid request identifier", + "details": {}, + "correlation_id": response.headers["x-correlation-id"], + "retryable": False, + }, + } + + +@pytest.mark.parametrize( + "headers", + [ + [(b"x-request-id", str(uuid4()).encode())] * 2, + [ + (b"x-request-id", str(uuid4()).encode()), + (b"X-Request-ID", str(uuid4()).encode()), + ], + [(b"x-correlation-id", str(uuid4()).encode())] * 2, + [(b"x-request-id", b"\xff" * 36)], + ], +) +async def test_duplicate_and_non_ascii_request_context_is_rejected( + headers: list[tuple[bytes, bytes]], +) -> None: + response = await _get(create_app(), headers=headers) + + assert response.status_code == 400 + assert response.headers["x-request-id"] == response.headers["x-correlation-id"] + assert response.json()["error"]["code"] == "invalid_request" + + +async def test_concurrent_request_context_remains_isolated() -> None: + app = create_app() + identifiers = [(str(uuid4()), str(uuid4())) for _ in range(12)] + + async def call(request_id: str, correlation_id: str): + return await _get( + app, + headers={"X-Request-ID": request_id, "X-Correlation-ID": correlation_id}, + ) + + responses = await asyncio.gather(*(call(*pair) for pair in identifiers)) + + assert [ + (response.headers["x-request-id"], response.headers["x-correlation-id"]) + for response in responses + ] == identifiers + + +async def test_response_context_overwrites_ids_and_preserves_headers() -> None: + app = create_app() + request_id = str(uuid4()) + correlation_id = str(uuid4()) + + @app.get("/_test/headers") + async def headers() -> Response: + response = Response( + "ok", + headers={ + "X-Request-ID": str(uuid4()), + "X-Correlation-ID": str(uuid4()), + "Retry-After": "7", + }, + ) + response.raw_headers.extend( + [(b"set-cookie", b"first=1"), (b"set-cookie", b"second=2")] + ) + return response + + response = await _get( + app, + "/_test/headers", + headers={"X-Request-ID": request_id, "X-Correlation-ID": correlation_id}, + ) + + assert response.status_code == 200 + assert response.content == b"ok" + assert response.headers["content-length"] == "2" + assert response.headers["retry-after"] == "7" + assert response.headers.get_list("set-cookie") == ["first=1", "second=2"] + assert response.headers["x-request-id"] == request_id + assert response.headers["x-correlation-id"] == correlation_id + + +async def test_streaming_and_background_responses_keep_context() -> None: + app = create_app() + completed: list[str] = [] + + @app.get("/_test/stream") + async def stream(background_tasks: BackgroundTasks) -> StreamingResponse: + async def content(): + yield b"first" + yield b"second" + + background_tasks.add_task(completed.append, "done") + return StreamingResponse(content(), background=background_tasks) + + response = await _get(app, "/_test/stream") + + assert response.status_code == 200 + assert response.content == b"firstsecond" + _assert_uuid(response.headers["x-request-id"]) + assert response.headers["x-correlation-id"] == response.headers["x-request-id"] + assert completed == ["done"] + + +async def test_background_failure_is_bounded_after_response_start( + caplog: pytest.LogCaptureFixture, +) -> None: + app = create_app() + + @app.get("/_test/background-failure") + async def background_failure(background_tasks: BackgroundTasks) -> dict[str, bool]: + def fail() -> None: + raise RuntimeError("token=secret-background-value") + + background_tasks.add_task(fail) + return {"accepted": True} + + response = await _get(app, "/_test/background-failure") + + assert response.status_code == 200 + assert response.json() == {"accepted": True} + assert response.headers["x-request-id"] + assert "request_failed_after_response_start" in caplog.text + assert "secret-background-value" not in caplog.text + + +@pytest.mark.parametrize( + ("status_code", "code", "message", "retryable"), + [ + (400, "invalid_request", "Invalid request", False), + (403, "permission_not_granted", "Permission not granted", False), + (409, "conflict", "Request conflict", False), + (429, "rate_limit_exceeded", "Rate limit exceeded", True), + (503, "service_unavailable", "Service unavailable", True), + ], +) +async def test_http_errors_use_stable_mapping_and_preserve_legacy_detail( + status_code: int, code: str, message: str, retryable: bool +) -> None: + app = create_app() + + async def fail() -> None: + headers = {"Retry-After": "9"} if status_code == 429 else None + raise HTTPException(status_code=status_code, detail="legacy detail", headers=headers) + + app.add_api_route(f"/_test/error/{status_code}", fail, methods=["GET"]) + response = await _get(app, f"/_test/error/{status_code}") + + assert response.status_code == status_code + assert response.json()["detail"] == "legacy detail" + assert response.json()["error"] == { + "code": code, + "message": message, + "details": {}, + "correlation_id": response.headers["x-correlation-id"], + "retryable": retryable, + } + if status_code == 429: + assert response.headers["retry-after"] == "9" + + +@pytest.mark.parametrize("debug", [False, True]) +async def test_not_found_method_not_allowed_and_unhandled_errors_are_private( + debug: bool, +) -> None: + app = create_app(Settings(debug=debug)) + + @app.get("/_test/boom") + async def boom() -> None: + raise RuntimeError("token=secret database failure") + + not_found = await _get(app, "/_test/missing") + async with AsyncClient( + transport=ASGITransport(app=app, raise_app_exceptions=False), + base_url="http://testserver", + ) as client: + method_not_allowed = await client.post("/health") + failed = await _get(app, "/_test/boom") + + assert (not_found.status_code, not_found.json()["error"]["code"]) == ( + 404, + "resource_not_found", + ) + assert (method_not_allowed.status_code, method_not_allowed.json()["error"]["code"]) == ( + 405, + "method_not_allowed", + ) + assert failed.status_code == 500 + assert failed.json()["detail"] == "Internal server error" + assert failed.json()["error"]["code"] == "internal_error" + assert failed.json()["error"]["retryable"] is False + assert "secret" not in failed.text + + +class ManyInvalidValues(BaseModel): + """Request fixture that can produce more errors than the response cap.""" + + values: list[Annotated[int, Field(gt=0)]] + + +class SafeValidatorMessage(BaseModel): + """Request fixture for legacy sanitized validator compatibility.""" + + value: str + + @field_validator("value") + @classmethod + def reject_value(cls, _value: str) -> str: + raise ValueError("safe compatibility message") + + +async def test_validation_summary_is_bounded_and_legacy_messages_remain() -> None: + app = create_app() + + @app.post("/_test/many-values") + async def many_values(_payload: ManyInvalidValues) -> None: + return None + + @app.post("/_test/safe-message") + async def safe_message(_payload: SafeValidatorMessage) -> None: + return None + + async with AsyncClient( + transport=ASGITransport(app=app), base_url="http://testserver" + ) as client: + many = await client.post("/_test/many-values", json={"values": [-1] * 25}) + safe = await client.post("/_test/safe-message", json={"value": "rejected"}) + + assert many.status_code == 422 + assert len(many.json()["detail"]) == 20 + assert len(many.json()["error"]["details"]["errors"]) == 20 + assert many.json()["error"]["details"]["truncated"] is True + assert set(many.json()["error"]["details"]["errors"][0]) == {"type", "loc"} + assert safe.status_code == 422 + assert "safe compatibility message" in safe.text + assert "safe compatibility message" not in str(safe.json()["error"]["details"]) + assert safe.json()["detail"][0]["input"] == "redacted" + + +async def test_validation_summary_redacts_unsupported_location_parts() -> None: + app = create_app() + + @app.get("/_test/unsupported-location") + async def unsupported_location() -> None: + raise RequestValidationError( + [{"type": "value_error", "loc": ("query", None), "input": "secret"}] + ) + + response = await _get(app, "/_test/unsupported-location") + + assert response.status_code == 422 + assert response.json()["detail"][0]["loc"] == ["query", None] + assert response.json()["error"]["details"]["errors"][0]["loc"] == [ + "query", + "redacted", + ] + + +def test_openapi_documents_request_error_and_response_context() -> None: + schema = create_app().openapi() + error_schema = schema["components"]["schemas"]["ApiError"] + validation_schema = schema["components"]["schemas"]["HTTPValidationError"] + + assert set(error_schema["required"]) == { + "code", + "message", + "details", + "correlation_id", + "retryable", + } + assert "error" in validation_schema["required"] + for path_item in schema["paths"].values(): + for method, operation in path_item.items(): + if method not in {"get", "put", "post", "delete", "options", "head", "patch"}: + continue + request_headers = { + parameter["name"] for parameter in operation["parameters"] + } + assert {"X-Request-ID", "X-Correlation-ID"} <= request_headers + for response in operation["responses"].values(): + assert {"X-Request-ID", "X-Correlation-ID"} <= set(response["headers"]) diff --git a/backend/tests/test_auth.py b/backend/tests/test_auth.py index 0ccd3c0e..035d54c4 100644 --- a/backend/tests/test_auth.py +++ b/backend/tests/test_auth.py @@ -729,6 +729,7 @@ async def test_service_and_agent_tokens_receive_no_legacy_authority( with pytest.raises(HTTPException) as exc_info: await get_registered_actor(agent, None) # type: ignore[arg-type] assert exc_info.value.status_code == 403 + assert exc_info.value.error_code == "unsupported_subject_kind" with pytest.raises(AuthVerificationError, match="service scope"): await verifier.verify( @@ -1205,6 +1206,13 @@ async def test_missing_bearer_token_is_rejected() -> None: assert response.status_code == 401 assert response.json()["detail"] == "Missing bearer token" assert response.headers["www-authenticate"] == "Bearer" + assert response.json()["error"] == { + "code": "missing_token", + "message": "Missing bearer token", + "details": {}, + "correlation_id": response.headers["x-correlation-id"], + "retryable": False, + } async def test_invalid_bearer_token_is_rejected(monkeypatch: pytest.MonkeyPatch) -> None: @@ -1226,6 +1234,8 @@ async def test_invalid_bearer_token_is_rejected(monkeypatch: pytest.MonkeyPatch) assert response.status_code == 401 assert response.json()["detail"] == "Invalid bearer token" + assert response.json()["error"]["code"] == "invalid_token" + assert response.json()["error"]["retryable"] is False async def test_invalid_production_verifier_configuration_is_service_unavailable() -> None: @@ -1242,6 +1252,8 @@ async def test_invalid_production_verifier_configuration_is_service_unavailable( assert response.status_code == 503 assert response.json()["detail"] == "Identity verification unavailable" + assert response.json()["error"]["code"] == "identity_verification_unavailable" + assert response.json()["error"]["retryable"] is True async def test_valid_dev_token_resolves_actor_context( @@ -1314,6 +1326,9 @@ async def fail_register_actor(self, actor): assert response.status_code == 503 assert response.json()["detail"] == "Actor registry unavailable" + assert response.json()["error"]["code"] == "service_unavailable" + assert response.json()["error"]["message"] == "Service unavailable" + assert response.json()["error"]["retryable"] is True async def test_actor_id_uses_subject_and_issuer_not_email() -> None: diff --git a/backend/tests/test_tasks.py b/backend/tests/test_tasks.py index 7872ce2f..1fd77d92 100644 --- a/backend/tests/test_tasks.py +++ b/backend/tests/test_tasks.py @@ -21,6 +21,7 @@ from app.adapters.auth.dev import actor_id_from_external_identity from app.core.config import get_settings from app.core.hashing import canonical_json_hash +from app.core.permissions import PermissionDenied from app.db import models as db_models from app.db import session as db_session from app.db.base import Base @@ -52,6 +53,7 @@ WorkstreamTask, ) from app.modules.tasks.repository import TaskRepository +from app.modules.tasks.service import TaskService, TaskServiceError from app.schemas.auth import ActorContext @@ -808,6 +810,7 @@ def test_submission_create_openapi_documents_domain_error() -> None: domain_schema = next(option for option in response_422["oneOf"] if "properties" in option) assert domain_schema["properties"]["code"]["enum"] == ["pre_submission_checker_failed"] assert "details" in domain_schema["properties"] + assert "error" in domain_schema["required"] def test_task_context_openapi_documents_locked_context_domain_error() -> None: @@ -821,6 +824,7 @@ def test_task_context_openapi_documents_locked_context_domain_error() -> None: domain_schema = next(option for option in response_422["oneOf"] if "properties" in option) assert domain_schema["properties"]["code"]["enum"] == ["task_locked_context_invalid"] assert "details" in domain_schema["properties"] + assert "error" in domain_schema["required"] def test_task_locked_context_constraints_bind_task_submission_and_hashes() -> None: @@ -911,6 +915,67 @@ def test_task_locked_context_constraints_bind_task_submission_and_hashes() -> No } +async def test_task_router_service_errors_use_canonical_request_context( + task_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + async def fail_with_service_error(*_args, **_kwargs): + raise TaskServiceError("bounded task failure") + + cases = [ + ("create_task", "POST", "/api/v1/projects/project-id/tasks", complete_task_payload()), + ("get_task", "GET", "/api/v1/tasks/task-id", None), + ("get_task_work_context", "GET", "/api/v1/tasks/task-id/work-context", None), + ( + "get_task_submission_requirements", + "GET", + "/api/v1/tasks/task-id/submission-requirements", + None, + ), + ("get_task_locked_context", "GET", "/api/v1/tasks/task-id/locked-context", None), + ("move_to_screening", "POST", "/api/v1/tasks/task-id/screen", None), + ("release_to_ready", "POST", "/api/v1/tasks/task-id/release", None), + ("claim_task", "POST", "/api/v1/tasks/task-id/claim", None), + ("start_task", "POST", "/api/v1/tasks/task-id/start", None), + ( + "create_submission", + "POST", + "/api/v1/tasks/task-id/submissions", + complete_submission_payload(), + ), + ("list_task_submissions", "GET", "/api/v1/tasks/task-id/submissions", None), + ("get_submission", "GET", "/api/v1/submissions/submission-id", None), + ("finalize_submission", "POST", "/api/v1/submissions/submission-id/finalize", None), + ("list_task_audit_events", "GET", "/api/v1/tasks/task-id/audit-events", None), + ] + + for service_method, method, path, payload in cases: + monkeypatch.setattr(TaskService, service_method, fail_with_service_error) + response = await task_client.request( + method, + path, + headers=auth_headers(), + json=payload, + ) + + assert response.status_code == 400 + assert response.json()["detail"] == "bounded task failure" + assert response.json()["error"]["code"] == "invalid_request" + assert response.json()["error"]["correlation_id"] == response.headers[ + "x-correlation-id" + ] + + async def fail_with_permission_error(*_args, **_kwargs): + raise PermissionDenied("bounded permission failure") + + monkeypatch.setattr(TaskService, "get_task", fail_with_permission_error) + denied = await task_client.get("/api/v1/tasks/task-id", headers=auth_headers()) + + assert denied.status_code == 403 + assert denied.json()["detail"] == "bounded permission failure" + assert denied.json()["error"]["code"] == "permission_not_granted" + + async def test_actor_profile_services_update_existing_actor_rows(task_database_env: str) -> None: async with db_session.get_session_factory()() as session: service = ActorService(session) @@ -1544,6 +1609,8 @@ async def test_task_context_apis_fail_closed_when_locked_context_is_missing( assert response.status_code == 422 assert response.json()["code"] == "task_locked_context_invalid" + assert response.json()["error"]["code"] == "task_locked_context_invalid" + assert response.json()["error"]["details"] == response.json()["details"] assert "locked_guide_version" in response.json()["details"]["missing_fields"] @@ -1831,6 +1898,8 @@ async def test_submission_runtime_uses_locked_project_policy_not_task_required_f assert non_contract_artifact_response.status_code == 422, non_contract_artifact_response.text detail = non_contract_artifact_response.json() assert detail["code"] == "pre_submission_checker_failed" + assert detail["error"]["code"] == "pre_submission_checker_failed" + assert detail["error"]["details"] == detail["details"] required_files = next( result for result in detail["details"]["results"] diff --git a/docs/operations_authorization_service.md b/docs/operations_authorization_service.md index b954650f..fb0636ec 100644 --- a/docs/operations_authorization_service.md +++ b/docs/operations_authorization_service.md @@ -98,6 +98,52 @@ fields remain `null`. Consumers must not treat token identity metadata as a profile source of truth; canonical profile metadata belongs to the later actor profile migration. +## Request And Error Context + +Clients may send one `X-Request-ID` and one `X-Correlation-ID`. Each supplied +value must be a canonical lowercase, hyphenated, non-nil RFC 9562 UUID using the +RFC variant and version 1 through 8. A missing request ID is generated as +UUIDv4; a missing correlation ID reuses the effective request ID. Duplicate, +comma-joined, malformed, non-ASCII, nil, or unsupported-version values stop +before route/dependency execution with HTTP 400 `invalid_request`. That response +uses one newly generated UUIDv4 for both safe headers and never reflects the +rejected bytes. + +Every success and error response returns the effective IDs in +`X-Request-ID` and `X-Correlation-ID`. Application-supplied response values for +those headers are overwritten. Status, content length, duplicate unrelated +headers, `WWW-Authenticate`, `Retry-After`, streaming chunks, and background +work remain unchanged. + +Errors add this canonical object while retaining the existing top-level +`detail` or domain `code/details` compatibility fields: + +```json +{ + "error": { + "code": "invalid_request", + "message": "Request validation failed", + "details": {}, + "correlation_id": "00000000-0000-4000-8000-000000000001", + "retryable": false + } +} +``` + +The authentication boundary uses `missing_token`, `invalid_token`, +`identity_verification_unavailable`, and `unsupported_subject_kind` for their +exact branches. Verification/registry unavailability is retryable; invalid +credentials, unsupported kinds, validation, permission, not-found, conflict, +and internal application errors are not. The canonical nested validation +summary is capped at 20 errors with type/location evidence only. Existing +sanitized validator messages remain solely in the bounded compatibility +`detail` list. + +Use the correlation ID to join operator evidence. Never add bearer tokens, +claims, subjects, emails, SQL, provider bodies, exception text, or secrets to +responses or logs. AUTH-04A does not activate rate controls, grant APIs, or new +product authority; those remain owned by later separately reviewed chunks. + ## JWKS Rotation And Outage Rotation procedure: From 2a129f424d02112d379ed988bc6214fb92bff3a2 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 17:17:10 +0100 Subject: [PATCH 06/12] docs(auth): adopt identity issuer adapter boundary --- .../DECISIONS.md | 29 +++++++++++++++++++ .../STATUS.md | 7 +++++ 2 files changed, 36 insertions(+) diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DECISIONS.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DECISIONS.md index 4193174f..5a0e4141 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DECISIONS.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DECISIONS.md @@ -158,3 +158,32 @@ start does not imply dependency approval. No production dependency or runtime implementation may change until the user explicitly accepts D12 and that acceptance is recorded here. The user explicitly approved D12 by replying "ok apporeved" after the final preimplementation plan review passed. + +## D13: Identity issuer is the stable verification boundary + +Status: accepted by the user on 2026-07-13. + +Workstream depends on one provider-neutral `IdentityIssuerVerifier` capability +port. It extends the repository-wide `ExternalServiceAdapter` convention and +is constructed through the typed +`ExternalServiceAdapterFactory[IdentityIssuerVerifier]` foundation being +prepared by `WS-ART-001-01C`. AUTH must adopt that merged shared foundation; it +must not create a second generic adapter registry or factory. + +The `/auth` dependency and product authorization layers must not change when an +additional external identity issuer is configured. Issuer-specific discovery, +JWKS, signature, claim, and introspection behavior remains behind explicit +adapter registration, construction, and configuration; it does not become a +second internal auth system or grant Workstream product authority. + +The current `AuthVerifier` port, `FlowAuthVerifier` adapter, and `flow_auth_*` +configuration names are migration inputs rather than the target vocabulary. +Their rename must be one bounded, atomic specification and implementation +chunk covering the port, adapter, factory, configuration compatibility, +tests, operator documentation, and stale-name proof. AUTH-04A does not absorb +that cross-cutting rename because its reviewed request/error scope is already +at the production size circuit breaker. The owning follow-up chunk is blocked +until the shared convention merges, and must then remove the old AUTH factory +entry point and migrate all callers in one clean cut. It must be planned and +reviewed before implementation; no compatibility factory alias, duplicate +verifier, service locator, or public login/session API may be introduced. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md index 92bbaaf9..c4dd3bcf 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md @@ -83,3 +83,10 @@ AUTH-03 review evidence and its PR trust bundle are recorded at Production issuer configuration and legacy non-test actor classification are future implementation/live-proof inputs and are tracked explicitly in `DISCOVERY.md` and chunk stop conditions. + +The user accepted D13 on 2026-07-13: the target provider-neutral boundary is +`IdentityIssuerVerifier`, extending the shared `ExternalServiceAdapter` +convention and using its typed factory. The current verifier/configuration +names remain functional migration inputs. The atomic AUTH adoption requires +its own reviewed chunk after `WS-ART-001-01C` installs the shared foundation; +it is not part of the size-capped AUTH-04A request/error implementation. From 8987dcef83f59cfe71e236ed11ec8bf0a70b156f Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 18:54:43 +0100 Subject: [PATCH 07/12] fix(auth): tighten request context contracts --- .agent-loop/LOOP_STATE.md | 17 ++-- .agent-loop/REVIEW_LOG.md | 20 +++++ .agent-loop/WORK_QUEUE.md | 4 +- .../CHUNK_MAP.md | 5 +- .../STATUS.md | 20 +++-- backend/app/core/api_controls.py | 34 +++++++- backend/app/main.py | 24 ++---- backend/tests/test_api_controls.py | 77 ++++++++++++++++--- backend/tests/test_auth.py | 36 ++++++++- backend/tests/test_tasks.py | 10 ++- 10 files changed, 188 insertions(+), 59 deletions(-) diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index 90c3563a..6844193c 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -7,21 +7,20 @@ - Active implementation chunk: `WS-AUTH-001-04A` - Request And Error Context - Branch: `codex/ws-auth-001-04-request-api-controls` - Worktree: `/home/abiorh/flow/workstream-auth-001-04` -- Status: AUTH-03 post-merge memory merged through PR #110 as `1864867`; the - user explicitly started AUTH-04. Required plan review split the parent before - implementation; only AUTH-04A contract repair and re-review are active. +- Status: AUTH-04A repaired-contract preimplementation review passed at + `f98bbfc`; implementation candidate `2a129f4` entered internal review and is + undergoing bounded reviewer repair. - Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836` - Prior `WS-AUTH-001-01` final merged branch head: `b5217e1` - Latest integrated `main` merge commit: `1864867` -- Current gate: AUTH-04A repaired contract and required preimplementation plan - re-review. No runtime edit is permitted until it passes. +- Current gate: repair valid internal-review findings, rerun deterministic + evidence, and obtain exact-head internal review before PR publication. - Next chunk: none; do not start `WS-AUTH-001-05` automatically. - `WS-AUTH-001-04B` is inactive until AUTH-04A merges, memory is updated, and the user gives its separate explicit start signal. -- Parallel initiative: `WS-QUAL-001-01B1B-R10` merged through PR #108 as - `5c47aba`, and this convergence records its post-merge memory. B2 remains - inactive pending a separate explicit user start; coverage chunk 02 remains - inactive pending B2 merge/memory and its separate explicit start. +- Parallel initiative: `WS-QUAL-001-01B2` is paused at the user's direction so + AUTH receives the laptop's test capacity. Its last official whole-app result + remains `6466/8159` statements (`79.249908%`); no replacement evidence exists. - Parallel initiative: `WS-ART-001-01` merged through PR #101 as `050eb15`. `WS-ART-001-02` remains proposed and inactive pending a separate explicit user start. diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 8825bd42..adbafc0e 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -1,5 +1,25 @@ # Review Log +## 2026-07-13 - WS-AUTH-001-04A Implementation Review Repair + +The final repaired 04A contract passed both required preimplementation review +tracks at exact commit `f98bbfc` before runtime implementation began. The +implementation crossed the 350 non-comment production-line checkpoint; scope +was inspected and frozen to the approved request/error boundary, with no rate +control, schema migration, grant, role, or product-authority work. The durable +checkpoint record was omitted at that time and is repaired here. The candidate +remains below the 500-line hard stop; 04B and later chunks are inactive. + +Required implementation review of exact head `2a129f4` passed security, +product authority, test-delta, CI-integrity, ASGI architecture, and shared +adapter-boundary checks, but rejected publication pending truthful per-route +OpenAPI, required response-header metadata, bounded pre-response failure +logging, complete real-ASGI error/schema/inventory proof, and synchronized loop +state. Those valid findings are in the first bounded repair cycle. A full-suite +run against `2a129f4` was stopped after findings made that exact-head evidence +obsolete; its partial output contained one unidentified failure and is not +completion evidence. + ## 2026-07-13 - WS-AUTH-001-04 Split Required Before Implementation Required L1 plan review rejected the combined request/error/rate-control parent diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index 381331bc..79679616 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -4,14 +4,14 @@ | Chunk | Title | Risk | Status | |---|---|---:|---| -| `WS-AUTH-001-04A` | Request And Error Context | L1 | Active; repaired contract awaiting required preimplementation re-review | +| `WS-AUTH-001-04A` | Request And Error Context | L1 | Implementation repair under required internal review | ## Planned Next | Chunk | Title | Risk | Status | |---|---|---:|---| | `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Inactive until 04A merge/memory and a separate explicit user start | -| `WS-QUAL-001-01B2` | Baseline Evidence And CI Ratchet | L1 | Inactive pending explicit user start | +| `WS-QUAL-001-01B2` | Baseline Evidence And CI Ratchet | L1 | Paused for AUTH priority; no valid replacement baseline yet | | `WS-QUAL-001-02` | Project Service Coverage | L1 | Inactive until 01B2 merge/memory plus explicit user start | | `WS-POL-002-04` | Locked Runtime Execution And Routing Hardening | L1 | Inactive pending relevant authorization proof and a separate explicit user start | | `WS-ART-001-02` | Flow Node Adapter And Reconciliation | L1 | Proposed; inactive pending separate explicit user start | diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md index f5175312..53edc9ad 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md @@ -16,7 +16,7 @@ stopped. | `WS-AUTH-001-02` | Verified Issuer Token And JWKS Boundary | L1 | Merged through PR #107 as `060b780` | | `WS-AUTH-001-03` | Legacy Actor Classification Preflight | L1 | Merged through PR #109 as `f06532e` | | `WS-AUTH-001-04` | Request, Error, And API Control Foundation | L1 | Split before implementation into 04A and 04B | -| `WS-AUTH-001-04A` | Request And Error Context | L1 | Active; repaired contract awaiting preimplementation re-review | +| `WS-AUTH-001-04A` | Request And Error Context | L1 | Implementation repair under internal review | | `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Inactive pending 04A merge/memory and separate explicit start | | `WS-AUTH-001-05` | Authority Evidence And Idempotency Foundation | L1 | Proposed | | `WS-AUTH-001-06` | Canonical Actor Profile And Identity Link | L1 | Proposed | @@ -79,6 +79,7 @@ WS-AUTH-001-PLAN AUTH-03 post-merge memory merged through PR #110 as `1864867`. The user explicitly started parent AUTH-04. Required plan review split it before runtime -implementation. AUTH-04A contract repair and re-review are the current gate. +implementation. AUTH-04A's repaired-contract review passed at `f98bbfc`; +exact-head implementation repair and review are the current gate. Do not start AUTH-04B, AUTH-05, or POL-002-04; each retains its separate start signal and prerequisites. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md index c4dd3bcf..8921887a 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md @@ -25,8 +25,11 @@ CodeRabbit passed on final branch head `43ffbfe`, then explicit human approval merged PR #109 to `main` as `f06532e` on 2026-07-13. AUTH-03 post-merge memory merged through PR #110 as `1864867`. The user then explicitly started AUTH-04. Required L1 plan review rejected the combined request/error/rate-control scope -before runtime edits and required a split. Only AUTH-04A contract repair and -preimplementation re-review are now active; AUTH-04B is inactive. +before runtime edits and required a split. AUTH-04A's final repaired contract +passed both required preimplementation reviews at `f98bbfc`. Implementation +candidate `2a129f4` then entered required internal review; valid OpenAPI, +logging, behavior-proof, and lifecycle-evidence findings are under repair. +AUTH-04B remains inactive. ## Active planning chunk @@ -34,8 +37,8 @@ None. ## Active implementation chunk -`WS-AUTH-001-04A` - Request And Error Context. Runtime edits remain gated on -the required repaired-contract preimplementation re-review. +`WS-AUTH-001-04A` - Request And Error Context. Implementation repair is active +under the reviewed contract; no later chunk is active. ## Current implementation branch @@ -50,7 +53,7 @@ the required repaired-contract preimplementation re-review. | `WS-AUTH-001-02` | Merged | `codex/ws-auth-001-02-verified-issuer-token` | #107 | Merged as `060b780`; reviewed code SHA `47dd5a7`. | | `WS-AUTH-001-03` | Merged | `codex/ws-auth-001-03-legacy-actor-classification` | #109 | Merged as `f06532e`; reviewed code `8c5334c`; final branch head `43ffbfe`. | | `WS-AUTH-001-04` | Split | `codex/ws-auth-001-04-request-api-controls` | - | Parent split before runtime implementation. | -| `WS-AUTH-001-04A` | Active | `codex/ws-auth-001-04-request-api-controls` | - | Repaired contract awaiting required preimplementation re-review; no runtime edit yet. | +| `WS-AUTH-001-04A` | In review | `codex/ws-auth-001-04-request-api-controls` | - | Repaired contract passed at `f98bbfc`; implementation repair active after review of `2a129f4`. | | `WS-AUTH-001-04B` | Inactive | - | - | PostgreSQL rate controls; requires 04A merge/memory and separate explicit start. | | `WS-AUTH-001-05` | Proposed | - | - | Authority evidence and idempotency foundation. | | `WS-AUTH-001-06` | Proposed | - | - | Canonical actor profile and identity link. | @@ -68,9 +71,10 @@ the required repaired-contract preimplementation re-review. ## Blockers No external blocker. The combined AUTH-04 plan failed its internal gate and was -split before runtime edits. AUTH-04A must pass repaired-contract review before -implementation. AUTH-04B later owns the migration corrected from the now-owned -`0016` prefix to `0017` on current main, but remains inactive. Non-test +split before runtime edits. AUTH-04A's repaired contract passed at `f98bbfc`; +valid implementation-review findings must pass repair review before PR. +AUTH-04B later owns the migration corrected from the now-owned `0016` prefix to +`0017` on current main, but remains inactive. Non-test operators must later supply explicit classification evidence rather than inferred kinds before the owning canonical actor migration. diff --git a/backend/app/core/api_controls.py b/backend/app/core/api_controls.py index c004aa96..576e6ca9 100644 --- a/backend/app/core/api_controls.py +++ b/backend/app/core/api_controls.py @@ -38,8 +38,6 @@ class ApiErrorResponse(BaseModel): error: ApiError detail: Any | None = None - code: str | None = None - details: dict[str, Any] | None = None class StructuredHTTPException(HTTPException): @@ -232,6 +230,10 @@ async def send_with_context(message: Message) -> None: extra={"correlation_id": correlation_id}, ) return + LOGGER.error( + "request_failed_before_response_start", + extra={"correlation_id": correlation_id}, + ) response = error_response( Request(scope, receive), status_code=500, @@ -285,7 +287,7 @@ def openapi() -> dict[str, Any]: }, } methods = {"get", "put", "post", "delete", "options", "head", "patch", "trace"} - for path_item in schema.get("paths", {}).values(): + for path, path_item in schema.get("paths", {}).items(): for method, operation in path_item.items(): if method not in methods: continue @@ -299,7 +301,31 @@ def openapi() -> dict[str, Any]: identity = (parameter["in"], parameter["name"].lower()) if identity not in present: parameters.append(parameter) - for response in operation.get("responses", {}).values(): + responses = operation.get("responses", {}) + if path not in {"/health", "/api/v1/health"}: + for status, description in { + "401": "Authentication failed.", + "403": "Permission denied.", + "503": "Identity verification unavailable.", + }.items(): + headers = dict(response_headers) + if status == "401": + headers["WWW-Authenticate"] = {"schema": {"type": "string"}} + responses.setdefault( + status, + { + "description": description, + "headers": headers, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiErrorResponse" + } + } + }, + }, + ) + for response in responses.values(): headers = response.setdefault("headers", {}) for name, header_schema in response_headers.items(): headers.setdefault(name, header_schema) diff --git a/backend/app/main.py b/backend/app/main.py index 01b1f7df..a95c4f82 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -29,8 +29,14 @@ MAX_VALIDATION_LOCATION_PARTS = 8 MAX_VALIDATION_CODE_LENGTH = 64 ERROR_RESPONSE_HEADERS = { - "X-Request-ID": {"schema": {"type": "string", "format": "uuid"}}, - "X-Correlation-ID": {"schema": {"type": "string", "format": "uuid"}}, + "X-Request-ID": { + "required": True, + "schema": {"type": "string", "format": "uuid"}, + }, + "X-Correlation-ID": { + "required": True, + "schema": {"type": "string", "format": "uuid"}, + }, } DEFAULT_ERROR_RESPONSES = { status_code: { @@ -40,23 +46,9 @@ } for status_code, description in { 400: "Invalid request.", - 401: "Authentication failed.", - 403: "Permission denied.", - 404: "Resource not found.", - 405: "Method not allowed.", - 409: "Request conflict.", - 422: "Request validation failed.", - 429: "Rate limit exceeded.", 500: "Internal server error.", - 503: "Service unavailable.", }.items() } -DEFAULT_ERROR_RESPONSES[401]["headers"]["WWW-Authenticate"] = { - "schema": {"type": "string"} -} -DEFAULT_ERROR_RESPONSES[429]["headers"]["Retry-After"] = { - "schema": {"type": "integer", "minimum": 1} -} @asynccontextmanager diff --git a/backend/tests/test_api_controls.py b/backend/tests/test_api_controls.py index 0f3dbb45..d76d40d8 100644 --- a/backend/tests/test_api_controls.py +++ b/backend/tests/test_api_controls.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +from hashlib import sha256 from typing import Annotated from uuid import UUID, uuid4 @@ -12,7 +13,7 @@ import pytest from starlette.requests import Request -from app.core.api_controls import error_response +from app.core.api_controls import ApiErrorResponse, error_response from app.core.config import Settings from app.main import create_app @@ -233,8 +234,10 @@ def fail() -> None: ("status_code", "code", "message", "retryable"), [ (400, "invalid_request", "Invalid request", False), + (401, "invalid_token", "Invalid bearer token", False), (403, "permission_not_granted", "Permission not granted", False), (409, "conflict", "Request conflict", False), + (422, "invalid_request", "Invalid request", False), (429, "rate_limit_exceeded", "Rate limit exceeded", True), (503, "service_unavailable", "Service unavailable", True), ], @@ -260,6 +263,7 @@ async def fail() -> None: "correlation_id": response.headers["x-correlation-id"], "retryable": retryable, } + ApiErrorResponse.model_validate(response.json()) if status_code == 429: assert response.headers["retry-after"] == "9" @@ -267,6 +271,7 @@ async def fail() -> None: @pytest.mark.parametrize("debug", [False, True]) async def test_not_found_method_not_allowed_and_unhandled_errors_are_private( debug: bool, + caplog: pytest.LogCaptureFixture, ) -> None: app = create_app(Settings(debug=debug)) @@ -282,19 +287,36 @@ async def boom() -> None: method_not_allowed = await client.post("/health") failed = await _get(app, "/_test/boom") - assert (not_found.status_code, not_found.json()["error"]["code"]) == ( - 404, - "resource_not_found", - ) - assert (method_not_allowed.status_code, method_not_allowed.json()["error"]["code"]) == ( - 405, - "method_not_allowed", + for response, detail, code, message in [ + (not_found, "Not Found", "resource_not_found", "Resource not found"), + ( + method_not_allowed, + "Method Not Allowed", + "method_not_allowed", + "Method not allowed", + ), + (failed, "Internal server error", "internal_error", "Internal server error"), + ]: + assert response.json() == { + "detail": detail, + "error": { + "code": code, + "message": message, + "details": {}, + "correlation_id": response.headers["x-correlation-id"], + "retryable": False, + }, + } + ApiErrorResponse.model_validate(response.json()) + + failure_record = next( + record + for record in caplog.records + if record.message == "request_failed_before_response_start" ) - assert failed.status_code == 500 - assert failed.json()["detail"] == "Internal server error" - assert failed.json()["error"]["code"] == "internal_error" - assert failed.json()["error"]["retryable"] is False + assert failure_record.correlation_id == failed.headers["x-correlation-id"] assert "secret" not in failed.text + assert "secret" not in caplog.text class ManyInvalidValues(BaseModel): @@ -374,6 +396,35 @@ def test_openapi_documents_request_error_and_response_context() -> None: "retryable", } assert "error" in validation_schema["required"] + response_schema = schema["components"]["schemas"]["ApiErrorResponse"] + assert response_schema["required"] == ["error"] + assert set(response_schema["properties"]) == {"error", "detail"} + assert response_schema["additionalProperties"] is False + methods = {"get", "put", "post", "delete", "options", "head", "patch", "trace"} + route_inventory = sorted( + f"{method.upper()} {path}" + for path, path_item in schema["paths"].items() + for method in path_item + if method in methods + ) + protected_inventory = sorted( + f"{method.upper()} {path}" + for path, path_item in schema["paths"].items() + for method, operation in path_item.items() + if method in methods and operation.get("security") + ) + assert len(route_inventory) == 46 + assert sha256("\n".join(route_inventory).encode()).hexdigest() == ( + "991f50d0dd6009e96c1cd8d0a8b6f403d6f48d81bb247075c103a9d74341425b" + ) + assert len(protected_inventory) == 44 + assert sha256("\n".join(protected_inventory).encode()).hexdigest() == ( + "ae15e39df8b1710e16b9c20cfa588a8ee8f96a98505f34e1cbcc5fe4c96a17d4" + ) + assert set(schema["paths"]["/health"]["get"]["responses"]) == {"200", "400", "500"} + assert {"401", "403", "503"} <= set( + schema["paths"]["/api/v1/auth/me"]["get"]["responses"] + ) for path_item in schema["paths"].values(): for method, operation in path_item.items(): if method not in {"get", "put", "post", "delete", "options", "head", "patch"}: @@ -384,3 +435,5 @@ def test_openapi_documents_request_error_and_response_context() -> None: assert {"X-Request-ID", "X-Correlation-ID"} <= request_headers for response in operation["responses"].values(): assert {"X-Request-ID", "X-Correlation-ID"} <= set(response["headers"]) + assert response["headers"]["X-Request-ID"]["required"] is True + assert response["headers"]["X-Correlation-ID"]["required"] is True diff --git a/backend/tests/test_auth.py b/backend/tests/test_auth.py index 035d54c4..fbd28867 100644 --- a/backend/tests/test_auth.py +++ b/backend/tests/test_auth.py @@ -28,6 +28,7 @@ from app.core.config import Settings, get_settings from app.core.permissions import PermissionDenied, require_any_role from app.db import session as db_session +from app.db.session import get_db_session from app.interfaces.auth import AuthVerificationError, AuthVerificationUnavailableError from app.main import create_app from app.modules.actors.service import ActorService @@ -716,9 +717,12 @@ async def test_service_and_agent_tokens_receive_no_legacy_authority( scope="workstream:service", ) ) - agent = await verifier.verify( - issue_asymmetric_token(private_key, subject_kind="agent", scope="agent:identity") + agent_token = issue_asymmetric_token( + private_key, + subject_kind="agent", + scope="agent:identity", ) + agent = await verifier.verify(agent_token) space = await verifier.verify( issue_asymmetric_token(private_key, subject_kind="space", scope="space:identity") ) @@ -731,6 +735,34 @@ async def test_service_and_agent_tokens_receive_no_legacy_authority( assert exc_info.value.status_code == 403 assert exc_info.value.error_code == "unsupported_subject_kind" + app = create_app(production_verifier_settings()) + app.state.auth_verifier = verifier + + async def no_database_session(): + yield None + + app.dependency_overrides[get_db_session] = no_database_session + async with AsyncClient( + transport=ASGITransport(app=app), + base_url="http://testserver", + ) as client: + response = await client.get( + "/api/v1/auth/me", + headers={"Authorization": f"Bearer {agent_token}"}, + ) + + assert response.status_code == 403 + assert response.json() == { + "detail": "Unsupported subject kind", + "error": { + "code": "unsupported_subject_kind", + "message": "Unsupported subject kind", + "details": {}, + "correlation_id": response.headers["x-correlation-id"], + "retryable": False, + }, + } + with pytest.raises(AuthVerificationError, match="service scope"): await verifier.verify( issue_asymmetric_token( diff --git a/backend/tests/test_tasks.py b/backend/tests/test_tasks.py index 1fd77d92..5742fc92 100644 --- a/backend/tests/test_tasks.py +++ b/backend/tests/test_tasks.py @@ -809,8 +809,8 @@ def test_submission_create_openapi_documents_domain_error() -> None: assert {"$ref": "#/components/schemas/HTTPValidationError"} in response_422["oneOf"] domain_schema = next(option for option in response_422["oneOf"] if "properties" in option) assert domain_schema["properties"]["code"]["enum"] == ["pre_submission_checker_failed"] - assert "details" in domain_schema["properties"] - assert "error" in domain_schema["required"] + assert set(domain_schema["required"]) == {"code", "details", "error"} + assert domain_schema["additionalProperties"] is False def test_task_context_openapi_documents_locked_context_domain_error() -> None: @@ -823,8 +823,8 @@ def test_task_context_openapi_documents_locked_context_domain_error() -> None: assert {"$ref": "#/components/schemas/HTTPValidationError"} in response_422["oneOf"] domain_schema = next(option for option in response_422["oneOf"] if "properties" in option) assert domain_schema["properties"]["code"]["enum"] == ["task_locked_context_invalid"] - assert "details" in domain_schema["properties"] - assert "error" in domain_schema["required"] + assert set(domain_schema["required"]) == {"code", "details", "error"} + assert domain_schema["additionalProperties"] is False def test_task_locked_context_constraints_bind_task_submission_and_hashes() -> None: @@ -1608,6 +1608,7 @@ async def test_task_context_apis_fail_closed_when_locked_context_is_missing( ) assert response.status_code == 422 + assert set(response.json()) == {"code", "details", "error"} assert response.json()["code"] == "task_locked_context_invalid" assert response.json()["error"]["code"] == "task_locked_context_invalid" assert response.json()["error"]["details"] == response.json()["details"] @@ -1897,6 +1898,7 @@ async def test_submission_runtime_uses_locked_project_policy_not_task_required_f assert non_contract_artifact_response.status_code == 422, non_contract_artifact_response.text detail = non_contract_artifact_response.json() + assert set(detail) == {"code", "details", "error"} assert detail["code"] == "pre_submission_checker_failed" assert detail["error"]["code"] == "pre_submission_checker_failed" assert detail["error"]["details"] == detail["details"] From cdcaf77f4a73d091afd54232f378f9a2831376c5 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 19:10:17 +0100 Subject: [PATCH 08/12] fix(auth): document applicable domain errors --- backend/app/core/api_controls.py | 9 +++++++-- backend/tests/test_api_controls.py | 25 ++++++++++++++++++++++--- backend/tests/test_tasks.py | 2 ++ 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/backend/app/core/api_controls.py b/backend/app/core/api_controls.py index 576e6ca9..35880c0b 100644 --- a/backend/app/core/api_controls.py +++ b/backend/app/core/api_controls.py @@ -303,11 +303,16 @@ def openapi() -> dict[str, Any]: parameters.append(parameter) responses = operation.get("responses", {}) if path not in {"/health", "/api/v1/health"}: - for status, description in { + applicable_errors = { "401": "Authentication failed.", "403": "Permission denied.", "503": "Identity verification unavailable.", - }.items(): + } + if "{" in path: + applicable_errors["404"] = "Resource not found." + if "{" in path and method in {"post", "put", "patch", "delete"}: + applicable_errors["409"] = "Request conflict." + for status, description in applicable_errors.items(): headers = dict(response_headers) if status == "401": headers["WWW-Authenticate"] = {"schema": {"type": "string"}} diff --git a/backend/tests/test_api_controls.py b/backend/tests/test_api_controls.py index d76d40d8..b63e9c84 100644 --- a/backend/tests/test_api_controls.py +++ b/backend/tests/test_api_controls.py @@ -287,16 +287,24 @@ async def boom() -> None: method_not_allowed = await client.post("/health") failed = await _get(app, "/_test/boom") - for response, detail, code, message in [ - (not_found, "Not Found", "resource_not_found", "Resource not found"), + for response, status_code, detail, code, message in [ + (not_found, 404, "Not Found", "resource_not_found", "Resource not found"), ( method_not_allowed, + 405, "Method Not Allowed", "method_not_allowed", "Method not allowed", ), - (failed, "Internal server error", "internal_error", "Internal server error"), + ( + failed, + 500, + "Internal server error", + "internal_error", + "Internal server error", + ), ]: + assert response.status_code == status_code assert response.json() == { "detail": detail, "error": { @@ -425,6 +433,17 @@ def test_openapi_documents_request_error_and_response_context() -> None: assert {"401", "403", "503"} <= set( schema["paths"]["/api/v1/auth/me"]["get"]["responses"] ) + assert {"404"} <= set( + schema["paths"]["/api/v1/tasks/{task_id}"]["get"]["responses"] + ) + assert {"404", "409"} <= set( + schema["paths"]["/api/v1/tasks/{task_id}/claim"]["post"]["responses"] + ) + assert {"404", "409"} <= set( + schema["paths"]["/api/v1/projects/{project_id}/guides/{guide_id}/activate"][ + "post" + ]["responses"] + ) for path_item in schema["paths"].values(): for method, operation in path_item.items(): if method not in {"get", "put", "post", "delete", "options", "head", "patch"}: diff --git a/backend/tests/test_tasks.py b/backend/tests/test_tasks.py index 5742fc92..4701faef 100644 --- a/backend/tests/test_tasks.py +++ b/backend/tests/test_tasks.py @@ -809,6 +809,7 @@ def test_submission_create_openapi_documents_domain_error() -> None: assert {"$ref": "#/components/schemas/HTTPValidationError"} in response_422["oneOf"] domain_schema = next(option for option in response_422["oneOf"] if "properties" in option) assert domain_schema["properties"]["code"]["enum"] == ["pre_submission_checker_failed"] + assert "details" in domain_schema["properties"] assert set(domain_schema["required"]) == {"code", "details", "error"} assert domain_schema["additionalProperties"] is False @@ -823,6 +824,7 @@ def test_task_context_openapi_documents_locked_context_domain_error() -> None: assert {"$ref": "#/components/schemas/HTTPValidationError"} in response_422["oneOf"] domain_schema = next(option for option in response_422["oneOf"] if "properties" in option) assert domain_schema["properties"]["code"]["enum"] == ["task_locked_context_invalid"] + assert "details" in domain_schema["properties"] assert set(domain_schema["required"]) == {"code", "details", "error"} assert domain_schema["additionalProperties"] is False From 47241cf8eb1c444910d4f4ba911cbc1e5447ef03 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 20:10:17 +0100 Subject: [PATCH 09/12] test(auth): cover scalar validation context --- backend/tests/test_api_controls.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/backend/tests/test_api_controls.py b/backend/tests/test_api_controls.py index b63e9c84..168277c8 100644 --- a/backend/tests/test_api_controls.py +++ b/backend/tests/test_api_controls.py @@ -378,13 +378,21 @@ async def test_validation_summary_redacts_unsupported_location_parts() -> None: @app.get("/_test/unsupported-location") async def unsupported_location() -> None: raise RequestValidationError( - [{"type": "value_error", "loc": ("query", None), "input": "secret"}] + [ + { + "type": "value_error", + "loc": ("query", None), + "input": "secret", + "ctx": "bounded context", + } + ] ) response = await _get(app, "/_test/unsupported-location") assert response.status_code == 422 assert response.json()["detail"][0]["loc"] == ["query", None] + assert response.json()["detail"][0]["ctx"] == "bounded context" assert response.json()["error"]["details"]["errors"][0]["loc"] == [ "query", "redacted", From 4196b1594b0395c97174337fca35a8cc2c88cbe2 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 20:26:31 +0100 Subject: [PATCH 10/12] fix(auth): stabilize request logging proof --- .agent-loop/LOOP_STATE.md | 10 +- .agent-loop/REVIEW_LOG.md | 22 +++ .agent-loop/WORK_QUEUE.md | 2 +- .../CHUNK_MAP.md | 6 +- .../STATUS.md | 17 ++- ...S-AUTH-001-04A-internal-review-evidence.md | 130 +++++++++++++++++ .../WS-AUTH-001-04A-pr-trust-bundle.md | 131 ++++++++++++++++++ backend/tests/test_api_controls.py | 10 ++ 8 files changed, 314 insertions(+), 14 deletions(-) create mode 100644 .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-internal-review-evidence.md create mode 100644 .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-pr-trust-bundle.md diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index 6844193c..b980a016 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -7,14 +7,14 @@ - Active implementation chunk: `WS-AUTH-001-04A` - Request And Error Context - Branch: `codex/ws-auth-001-04-request-api-controls` - Worktree: `/home/abiorh/flow/workstream-auth-001-04` -- Status: AUTH-04A repaired-contract preimplementation review passed at - `f98bbfc`; implementation candidate `2a129f4` entered internal review and is - undergoing bounded reviewer repair. +- Status: AUTH-04A implementation and required internal repair review pass on + production SHA `cdcaf77`; final test-only head `47241cf` also passed exact-head + test-delta confirmation. - Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836` - Prior `WS-AUTH-001-01` final merged branch head: `b5217e1` - Latest integrated `main` merge commit: `1864867` -- Current gate: repair valid internal-review findings, rerun deterministic - evidence, and obtain exact-head internal review before PR publication. +- Current gate: publish the ready PR, then require GitHub Backend, Agent Gates, + CodeRabbit, and explicit human review before merge. - Next chunk: none; do not start `WS-AUTH-001-05` automatically. - `WS-AUTH-001-04B` is inactive until AUTH-04A merges, memory is updated, and the user gives its separate explicit start signal. diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index adbafc0e..5cd72bfc 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -1,5 +1,27 @@ # Review Log +## 2026-07-13 - WS-AUTH-001-04A Internal Review Passed + +All required implementation tracks pass on production SHA `cdcaf77`. Final +test-only head `47241cf` adds one genuine real-ASGI scalar validation-context +branch assertion; exact-head test-delta review confirmed identical production +blobs and retained every earlier result. Valid OpenAPI, response-header, +pre-response logging, real-ASGI behavior, inventory, compatibility, and memory +findings were repaired without rate controls, schema, grants, roles, routes, or +product-authority changes. + +Focused evidence is 235 passing tests. Changed-file statement coverage is +98.08 percent for API controls, 90.82 percent for the application factory, +90.70 percent for auth dependencies, and 92.36 percent for the task router. The +API drill, isolated-runner lifecycle suite, Ruff, docstring threshold, stale +scans, Markdown links, Agent Gates, additive test-delta scan, and diff hygiene +pass. A complete isolated backend regression run exposed an Alembic logging +state leak after 114 passing tests; the affected real-ASGI test now establishes +and restores its logging capture precondition, and the exact failing order +passes. GitHub Backend owns the complete-suite rerun before merge. The official +separate whole-app coverage baseline remains 79.249908 percent; no replacement +is claimed. + ## 2026-07-13 - WS-AUTH-001-04A Implementation Review Repair The final repaired 04A contract passed both required preimplementation review diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index 79679616..e815bcb2 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -4,7 +4,7 @@ | Chunk | Title | Risk | Status | |---|---|---:|---| -| `WS-AUTH-001-04A` | Request And Error Context | L1 | Implementation repair under required internal review | +| `WS-AUTH-001-04A` | Request And Error Context | L1 | Internal review passed; ready PR publication active, complete GitHub checks required before merge | ## Planned Next diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md index 53edc9ad..f308ce68 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md @@ -16,7 +16,7 @@ stopped. | `WS-AUTH-001-02` | Verified Issuer Token And JWKS Boundary | L1 | Merged through PR #107 as `060b780` | | `WS-AUTH-001-03` | Legacy Actor Classification Preflight | L1 | Merged through PR #109 as `f06532e` | | `WS-AUTH-001-04` | Request, Error, And API Control Foundation | L1 | Split before implementation into 04A and 04B | -| `WS-AUTH-001-04A` | Request And Error Context | L1 | Implementation repair under internal review | +| `WS-AUTH-001-04A` | Request And Error Context | L1 | Internally reviewed; ready PR publication active and complete GitHub checks required | | `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Inactive pending 04A merge/memory and separate explicit start | | `WS-AUTH-001-05` | Authority Evidence And Idempotency Foundation | L1 | Proposed | | `WS-AUTH-001-06` | Canonical Actor Profile And Identity Link | L1 | Proposed | @@ -80,6 +80,8 @@ WS-AUTH-001-PLAN AUTH-03 post-merge memory merged through PR #110 as `1864867`. The user explicitly started parent AUTH-04. Required plan review split it before runtime implementation. AUTH-04A's repaired-contract review passed at `f98bbfc`; -exact-head implementation repair and review are the current gate. +production review passed at `cdcaf77`, and final test-only head `47241cf` passed +exact-head confirmation. Ready PR publication and complete GitHub checks are +the current gate. Do not start AUTH-04B, AUTH-05, or POL-002-04; each retains its separate start signal and prerequisites. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md index 8921887a..aa35654c 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md @@ -27,8 +27,10 @@ merged through PR #110 as `1864867`. The user then explicitly started AUTH-04. Required L1 plan review rejected the combined request/error/rate-control scope before runtime edits and required a split. AUTH-04A's final repaired contract passed both required preimplementation reviews at `f98bbfc`. Implementation -candidate `2a129f4` then entered required internal review; valid OpenAPI, -logging, behavior-proof, and lifecycle-evidence findings are under repair. +candidate `2a129f4` then entered required internal review. Valid OpenAPI, +logging, behavior-proof, and lifecycle-evidence findings were repaired; all +required tracks pass on production SHA `cdcaf77`, and final test-only head +`47241cf` passed exact-head confirmation. AUTH-04B remains inactive. ## Active planning chunk @@ -37,8 +39,9 @@ None. ## Active implementation chunk -`WS-AUTH-001-04A` - Request And Error Context. Implementation repair is active -under the reviewed contract; no later chunk is active. +`WS-AUTH-001-04A` - Request And Error Context. Internal review passed and ready +PR publication is active. Complete GitHub checks remain required before merge. +No later chunk is active. ## Current implementation branch @@ -53,7 +56,7 @@ under the reviewed contract; no later chunk is active. | `WS-AUTH-001-02` | Merged | `codex/ws-auth-001-02-verified-issuer-token` | #107 | Merged as `060b780`; reviewed code SHA `47dd5a7`. | | `WS-AUTH-001-03` | Merged | `codex/ws-auth-001-03-legacy-actor-classification` | #109 | Merged as `f06532e`; reviewed code `8c5334c`; final branch head `43ffbfe`. | | `WS-AUTH-001-04` | Split | `codex/ws-auth-001-04-request-api-controls` | - | Parent split before runtime implementation. | -| `WS-AUTH-001-04A` | In review | `codex/ws-auth-001-04-request-api-controls` | - | Repaired contract passed at `f98bbfc`; implementation repair active after review of `2a129f4`. | +| `WS-AUTH-001-04A` | Internally reviewed | `codex/ws-auth-001-04-request-api-controls` | - | Production review passed at `cdcaf77`; final test-only head `47241cf` confirmed. | | `WS-AUTH-001-04B` | Inactive | - | - | PostgreSQL rate controls; requires 04A merge/memory and separate explicit start. | | `WS-AUTH-001-05` | Proposed | - | - | Authority evidence and idempotency foundation. | | `WS-AUTH-001-06` | Proposed | - | - | Canonical actor profile and identity link. | @@ -72,7 +75,9 @@ under the reviewed contract; no later chunk is active. No external blocker. The combined AUTH-04 plan failed its internal gate and was split before runtime edits. AUTH-04A's repaired contract passed at `f98bbfc`; -valid implementation-review findings must pass repair review before PR. +valid implementation-review findings passed repair review. A local complete +run exposed and reproduced an Alembic logging-state test interaction; its exact +order now passes, and GitHub Backend owns complete-suite proof before merge. AUTH-04B later owns the migration corrected from the now-owned `0016` prefix to `0017` on current main, but remains inactive. Non-test operators must later supply explicit classification evidence rather than diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-internal-review-evidence.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-internal-review-evidence.md new file mode 100644 index 00000000..4ae7fc42 --- /dev/null +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-internal-review-evidence.md @@ -0,0 +1,130 @@ +# Internal Review Evidence: WS-AUTH-001-04A + +## Chunk + +`WS-AUTH-001-04A` - Request And Error Context + +open sub-agent sessions: none + +valid findings addressed: yes + +## Reviewed Revision + +Reviewed code SHA: `47241cf8eb1c444910d4f4ba911cbc1e5447ef03` + +Reviewed production SHA: `cdcaf77f4a73d091afd54232f378f9a2831376c5` + +Reviewed at: 2026-07-13T19:15:16Z + +Reviewer run IDs: engineering-architecture=AUTH-04A-FINAL-ENG-ARCH-20260713; +qa-ci-test-delta=AUTH-04A-FINAL-QA-CI-20260713; +security-product-docs-reuse=AUTH-04A-FINAL-SEC-PROD-20260713; +test-only-confirmation=AUTH-04A-FINAL-TEST-DELTA-20260713 + +The final revision differs from the reviewed production SHA only by one +additive real-ASGI validation test that exercises scalar validation context. +The final test-delta review confirmed identical production blobs and retained +the complete prior review results. + +## Reviewed Change + +- Added pure ASGI request context that validates or generates canonical request + and correlation UUIDs and overwrites only the two response ID headers. +- Added stable privacy-bounded nested error objects while retaining existing + top-level compatibility fields, status codes, and response headers. +- Added truthful per-route OpenAPI error and response-header contracts without + changing the route or dependency-consumer inventories. +- Added bounded correlation-only request failure logging before and after + response start. +- Added real-ASGI behavior, compatibility, privacy, concurrency, OpenAPI, + inventory, streaming, background-task, and API-drill proof. + +## Reviewer Results + +| Reviewer | Result | Blocking findings | Notes | +|---|---:|---|---| +| senior engineering | PASS | None | The bounded ASGI and error helpers are maintainable and preserve compatibility. | +| QA/test | PASS | None | The behavior matrix covers success, failure, privacy, headers, schemas, and inventory through real ASGI paths. | +| security/auth | PASS | None | Invalid identifiers, validation context, unhandled failures, and logs remain privacy bounded and fail closed. | +| product/ops | PASS | None | No role, grant, lifecycle, or product-authority behavior changed. | +| architecture | PASS | None | Pure ASGI middleware changes response-start headers only; rate controls and adapter migration remain separate. | +| CI integrity | PASS | None | No workflow, dependency, threshold, exclusion, or bypass changed. | +| docs | PASS | None | The authorization runbook and durable loop state describe the implemented boundary and inactive follow-up. | +| reuse/dedup | PASS | None | Shared builders centralize error and OpenAPI behavior without duplicating provider or factory boundaries. | +| test delta | PASS | None | No assertion, raises block, skip, xfail, or skipTest line was removed or weakened. | + +## Valid Findings Addressed + +- Replaced broad global OpenAPI error claims with per-route applicable statuses + and explicit response ID headers. +- Added bounded pre-response failure logging and proved post-response-start + failure behavior without reflecting exceptions or identity data. +- Completed real-ASGI proof for malformed headers, every stable error branch, + concurrent isolation, streaming, background tasks, and compatibility headers. +- Added exact route and protected-consumer inventory digests and additive schema + assertions for task domain errors. +- Reconciled lifecycle memory and recorded the 350-line scope checkpoint. +- Added the final scalar validation-context branch test; production code did not + change after the final production review. + +## Commands Run + +```bash +(cd backend && isolated-runner pytest -q tests/test_api_controls.py tests/test_app.py tests/test_auth.py tests/test_tasks.py) +(cd backend && isolated-runner pytest -q --cov=app tests/test_api_controls.py tests/test_app.py tests/test_auth.py tests/test_tasks.py) +(cd backend && for file in app/core/api_controls.py app/main.py app/api/deps/auth.py app/modules/tasks/router.py; do coverage report --include="$file" --precision=2 --fail-under=90; done) +(cd backend && pytest -q tests/test_isolated_database_runner.py) +(cd backend && isolated-runner pytest -q -x --ignore=tests/test_isolated_database_runner.py) +(cd backend && WORKSTREAM_DATABASE_URL= python scripts/api_contract_e2e.py) +(cd backend && ruff check app tests scripts) +(cd backend && docstr-coverage --config .docstr.yaml) +python3 scripts/check_loop_memory_state.py +python3 scripts/check_stale_workstream_wording.py +python3 scripts/check_stale_authorization_docs.py +python3 scripts/check_stale_artifact_contracts.py +python3 scripts/check_markdown_links.py +python3 scripts/test_agent_gates.py +git diff --unified=0 origin/main...HEAD -- backend/tests | (! rg '^-(.*assert|.*pytest\.raises|.*pytest\.mark\.(skip|xfail)|.*skipTest)') +git diff --check +``` + +Results: + +- focused real-ASGI behavior matrix: 235 passed; +- changed-file statement coverage: API controls 98.08 percent, application + factory 90.82 percent, auth dependencies 90.70 percent, and task router 92.36 + percent; +- isolated-runner lifecycle suite: 16 passed; +- API contract drill: passed; +- Ruff, docstring threshold, stale wording, authorization docs, artifact + contracts, Markdown links, loop memory, Agent Gates, additive test-delta, and + diff hygiene: passed; +- complete isolated backend regression run: exposed one order-dependent logging + capture failure after 114 passes; the failing Alembic-before-ASGI order and + both logging branches pass after the test-only repair. + +## Evidence Gate + +Evidence gate: PASS WITH EXTERNAL COMPLETE-SUITE PROOF PENDING + +The reviewed implementation stays inside the approved request/error contract +and below the 500 non-comment production-line hard stop. It adds no migration, +rate counter, grant, role, identity state, product permission, route, dependency +consumer, or later AUTH behavior. GitHub Backend must supply complete-suite +proof before merge; the exact locally failing order now passes. + +## Remaining Risks + +- GitHub Backend must complete the full regression suite, and Agent Gates must + repeat repository checks on the published branch before human merge. +- The repository-wide coverage improvement initiative remains paused at its + last official 79.249908 percent result; this AUTH chunk proves each changed + production file at or above 90 percent and does not claim a new whole-app + baseline. +- PostgreSQL rate controls and provider-neutral `IdentityIssuerVerifier` + adoption remain separate reviewed chunks with unmet prerequisites. + +## Stop Condition + +After ready PR publication, stop for complete GitHub checks and explicit human +review. Do not merge or start AUTH-04B or AUTH-05 automatically. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-pr-trust-bundle.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-pr-trust-bundle.md new file mode 100644 index 00000000..f848e341 --- /dev/null +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-pr-trust-bundle.md @@ -0,0 +1,131 @@ +# PR Trust Bundle: WS-AUTH-001-04A + +## Chunk + +`WS-AUTH-001-04A` - Request And Error Context + +## Goal + +Give every API response canonical request and correlation identifiers and a +stable privacy-bounded error object without changing product authority or +breaking existing clients. + +## Human-Approved Intent + +The user explicitly started AUTH-04 after AUTH-03 memory merged through PR +#110. Required L1 review split request/error context from PostgreSQL rate +controls before implementation. This PR contains only the reviewed 04A child; +04B and later authorization chunks remain inactive. + +## What Changed + +- Added strict canonical request/correlation UUID handling in pure ASGI + middleware. +- Added stable nested error objects while preserving legacy response fields, + status codes, and headers. +- Added per-route OpenAPI error and response-header documentation. +- Added privacy-bounded correlation-only failure logs. +- Added real-ASGI behavior, inventory, schema, and API drill coverage. + +## Why It Changed + +Workstream needs traceable API failures and a dependable error contract before +authority enforcement and rate controls expand, but existing consumers cannot +be forced through a breaking response migration. + +## Design Chosen + +Pure ASGI middleware reads raw header pairs, stores effective IDs on request +scope state, and edits only `http.response.start` headers. Shared response +builders and exception handlers own JSON shapes. Existing compatibility fields +remain additive beside the canonical nested object. + +## Alternatives Rejected + +- `BaseHTTPMiddleware`, response buffering, or response-body rewriting. +- Collapsed framework header parsing that cannot detect duplicate fields. +- Replacing existing top-level error fields in one breaking migration. +- Global OpenAPI statuses that routes cannot actually emit. +- Adding PostgreSQL rate controls or provider-adapter migration to this chunk. + +## Scope Control + +The production change remains below the contract's 500 non-comment line hard +stop. No route, dependency consumer, database model, migration, setting, +permission, role, grant, actor state, rate counter, or product lifecycle changed. + +## Product Behavior + +Clients receive canonical `X-Request-ID` and `X-Correlation-ID` headers on +success and failure. Errors gain a required nested object with stable code, +message, bounded details, correlation ID, and retryability while retaining +existing compatible fields and headers. Product authorization is unchanged. + +## Acceptance Criteria Proof + +Real-ASGI tests cover missing, valid, malformed, duplicate, non-ASCII, nil, +uppercase, non-hyphenated, unsupported-version, and comma-joined identifiers; +concurrency; success and every required error class; validation privacy; +streaming; background tasks; duplicate unrelated headers; task compatibility; +OpenAPI schemas; and exact route/dependency inventories. + +## Tests And Checks + +- 235 focused behavior tests passed. +- Every changed production file exceeds 90 percent statement coverage: 98.08, + 90.82, 90.70, and 92.36 percent respectively. +- The isolated-runner lifecycle suite passed 16/16 and the live API contract + drill passed. +- Ruff, docstring threshold, stale scans, Markdown links, loop memory, Agent + Gates, additive test delta, and diff hygiene passed. +- A complete local run found one Alembic logging-state test interaction after + 114 passes; its exact order passes after repair. GitHub Backend must provide + the complete-suite rerun before merge. + +## Test Delta + +No assertion, `pytest.raises`, skip/xfail marker, or `skipTest` line was removed +or weakened. New proof exercises actual FastAPI routes through ASGI transport. + +## CI Integrity + +No workflow, package, dependency, threshold, exclusion, or bypass changed. +GitHub Backend and Agent Gates remain mandatory before merge. The separate +whole-app coverage initiative remains paused at its official 79.249908 percent +baseline; this chunk's changed-file threshold is at least 90 percent. + +## Reviewer Results + +Senior engineering, QA/test, security/auth, product/ops, architecture, CI +integrity, docs, reuse/dedup, and test-delta review pass. Production was reviewed +at `cdcaf77`; final test-only head `47241cf` received exact-head confirmation +with identical production blobs and no weakened tests. + +## External Review + +GitHub Backend, Agent Gates, CodeRabbit, and explicit human review begin after +the ready PR is published. None substitutes for the completed internal review. + +## Remaining Risks + +- Rate controls are intentionally absent and remain owned by inactive 04B. +- Provider-neutral `IdentityIssuerVerifier` adoption depends on the shared + external-service adapter foundation and a separate reviewed AUTH chunk. +- The canonical nested error is additive; compatibility fields remain until a + separately reviewed removal is safe. + +## Follow-Up Work + +After this PR merges and memory is updated, AUTH-04B still requires a separate +explicit user start. AUTH-05 and later cutover chunks remain proposed. + +## Human Review Focus + +Inspect duplicate-ID rejection, response-start-only header mutation, error and +log privacy, exact legacy compatibility, per-route OpenAPI truth, and unchanged +route/dependency inventories. + +## Human Merge Ownership + +Only the user may approve and merge this PR. Publication is not merge approval, +and no later AUTH chunk starts automatically. diff --git a/backend/tests/test_api_controls.py b/backend/tests/test_api_controls.py index 168277c8..f80697dd 100644 --- a/backend/tests/test_api_controls.py +++ b/backend/tests/test_api_controls.py @@ -2,6 +2,7 @@ import asyncio from hashlib import sha256 +import logging from typing import Annotated from uuid import UUID, uuid4 @@ -210,7 +211,12 @@ async def content(): async def test_background_failure_is_bounded_after_response_start( caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, ) -> None: + logger = logging.getLogger("app.core.api_controls") + # In-process Alembic tests disable existing loggers through fileConfig. + monkeypatch.setattr(logger, "disabled", False) + caplog.set_level(logging.ERROR, logger=logger.name) app = create_app() @app.get("/_test/background-failure") @@ -272,7 +278,11 @@ async def fail() -> None: async def test_not_found_method_not_allowed_and_unhandled_errors_are_private( debug: bool, caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, ) -> None: + logger = logging.getLogger("app.core.api_controls") + monkeypatch.setattr(logger, "disabled", False) + caplog.set_level(logging.ERROR, logger=logger.name) app = create_app(Settings(debug=debug)) @app.get("/_test/boom") From 4fd6db9ebe9911d30ec85657903b71707fc3bbfb Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 20:31:07 +0100 Subject: [PATCH 11/12] docs(auth): clarify final review provenance --- .agent-loop/REVIEW_LOG.md | 11 +++++++---- .../CHUNK_MAP.md | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 5cd72bfc..44ead3d1 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -2,10 +2,13 @@ ## 2026-07-13 - WS-AUTH-001-04A Internal Review Passed -All required implementation tracks pass on production SHA `cdcaf77`. Final -test-only head `47241cf` adds one genuine real-ASGI scalar validation-context -branch assertion; exact-head test-delta review confirmed identical production -blobs and retained every earlier result. Valid OpenAPI, response-header, +All required implementation tracks pass on production SHA `cdcaf77`. +Test-validation head `47241cf` added one genuine real-ASGI scalar +validation-context branch assertion. A later test-only repair establishes and +restores logging capture after in-process Alembic configuration disables an +existing logger. Exact-head test-delta review confirmed identical production +blobs and retained every earlier result; the final reviewed test revision is +bound in the internal review evidence. Valid OpenAPI, response-header, pre-response logging, real-ASGI behavior, inventory, compatibility, and memory findings were repaired without rate controls, schema, grants, roles, routes, or product-authority changes. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md index f308ce68..d57a57ba 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md @@ -80,8 +80,8 @@ WS-AUTH-001-PLAN AUTH-03 post-merge memory merged through PR #110 as `1864867`. The user explicitly started parent AUTH-04. Required plan review split it before runtime implementation. AUTH-04A's repaired-contract review passed at `f98bbfc`; -production review passed at `cdcaf77`, and final test-only head `47241cf` passed -exact-head confirmation. Ready PR publication and complete GitHub checks are -the current gate. +production review passed at `cdcaf77`, and the final test-only revision is +bound in internal review evidence after exact-head confirmation. Ready PR +publication and complete GitHub checks are the current gate. Do not start AUTH-04B, AUTH-05, or POL-002-04; each retains its separate start signal and prerequisites. From 36c4aa5b940152114ac57384c867077f3ff9094e Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 20:34:55 +0100 Subject: [PATCH 12/12] docs(auth): bind final internal review --- .agent-loop/LOOP_STATE.md | 4 ++-- .../STATUS.md | 5 +++-- ...S-AUTH-001-04A-internal-review-evidence.md | 20 +++++++++---------- .../WS-AUTH-001-04A-pr-trust-bundle.md | 5 +++-- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index b980a016..4b35c46c 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -8,8 +8,8 @@ - Branch: `codex/ws-auth-001-04-request-api-controls` - Worktree: `/home/abiorh/flow/workstream-auth-001-04` - Status: AUTH-04A implementation and required internal repair review pass on - production SHA `cdcaf77`; final test-only head `47241cf` also passed exact-head - test-delta confirmation. + production SHA `cdcaf77`; final reviewed candidate `4fd6db9` includes only + additive scalar-context and logging-state behavior-test repairs plus evidence. - Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836` - Prior `WS-AUTH-001-01` final merged branch head: `b5217e1` - Latest integrated `main` merge commit: `1864867` diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md index aa35654c..50d2b7cd 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md @@ -30,7 +30,8 @@ passed both required preimplementation reviews at `f98bbfc`. Implementation candidate `2a129f4` then entered required internal review. Valid OpenAPI, logging, behavior-proof, and lifecycle-evidence findings were repaired; all required tracks pass on production SHA `cdcaf77`, and final test-only head -`47241cf` passed exact-head confirmation. +`4fd6db9` passed exact-head confirmation for the additive scalar-context and +logging-state behavior-test repairs plus lifecycle evidence. AUTH-04B remains inactive. ## Active planning chunk @@ -56,7 +57,7 @@ No later chunk is active. | `WS-AUTH-001-02` | Merged | `codex/ws-auth-001-02-verified-issuer-token` | #107 | Merged as `060b780`; reviewed code SHA `47dd5a7`. | | `WS-AUTH-001-03` | Merged | `codex/ws-auth-001-03-legacy-actor-classification` | #109 | Merged as `f06532e`; reviewed code `8c5334c`; final branch head `43ffbfe`. | | `WS-AUTH-001-04` | Split | `codex/ws-auth-001-04-request-api-controls` | - | Parent split before runtime implementation. | -| `WS-AUTH-001-04A` | Internally reviewed | `codex/ws-auth-001-04-request-api-controls` | - | Production review passed at `cdcaf77`; final test-only head `47241cf` confirmed. | +| `WS-AUTH-001-04A` | Internally reviewed | `codex/ws-auth-001-04-request-api-controls` | - | Production review passed at `cdcaf77`; final reviewed candidate `4fd6db9` confirmed. | | `WS-AUTH-001-04B` | Inactive | - | - | PostgreSQL rate controls; requires 04A merge/memory and separate explicit start. | | `WS-AUTH-001-05` | Proposed | - | - | Authority evidence and idempotency foundation. | | `WS-AUTH-001-06` | Proposed | - | - | Canonical actor profile and identity link. | diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-internal-review-evidence.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-internal-review-evidence.md index 4ae7fc42..cc38c9b4 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-internal-review-evidence.md @@ -10,21 +10,21 @@ valid findings addressed: yes ## Reviewed Revision -Reviewed code SHA: `47241cf8eb1c444910d4f4ba911cbc1e5447ef03` +Reviewed code SHA: `4fd6db9ebe9911d30ec85657903b71707fc3bbfb` Reviewed production SHA: `cdcaf77f4a73d091afd54232f378f9a2831376c5` -Reviewed at: 2026-07-13T19:15:16Z +Reviewed at: 2026-07-13T19:34:02Z -Reviewer run IDs: engineering-architecture=AUTH-04A-FINAL-ENG-ARCH-20260713; -qa-ci-test-delta=AUTH-04A-FINAL-QA-CI-20260713; -security-product-docs-reuse=AUTH-04A-FINAL-SEC-PROD-20260713; -test-only-confirmation=AUTH-04A-FINAL-TEST-DELTA-20260713 +Reviewer run IDs: engineering-architecture=AUTH-04A-4FD6DB9-ENG-ARCH-20260713; +qa-ci-test-delta=AUTH-04A-4FD6DB9-QA-CI-20260713; +security-product-docs-reuse=AUTH-04A-4FD6DB9-SEC-PROD-20260713 -The final revision differs from the reviewed production SHA only by one -additive real-ASGI validation test that exercises scalar validation context. -The final test-delta review confirmed identical production blobs and retained -the complete prior review results. +The final revision differs from the reviewed production SHA only by additive +real-ASGI test changes: scalar validation context is exercised, and logging +tests establish and restore capture after in-process Alembic configuration +disables an existing logger. Final test-delta review confirmed identical +production blobs and retained the complete prior review results. ## Reviewed Change diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-pr-trust-bundle.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-pr-trust-bundle.md index f848e341..2f7f771a 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-pr-trust-bundle.md @@ -98,8 +98,9 @@ baseline; this chunk's changed-file threshold is at least 90 percent. Senior engineering, QA/test, security/auth, product/ops, architecture, CI integrity, docs, reuse/dedup, and test-delta review pass. Production was reviewed -at `cdcaf77`; final test-only head `47241cf` received exact-head confirmation -with identical production blobs and no weakened tests. +at `cdcaf77`; final reviewed candidate `4fd6db9` received exact-head confirmation +for both additive test repairs with identical production blobs and no weakened +tests. ## External Review