From 5ed410dd08b6330ab2eb2b5fdc3fec935cc6115c Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 23:07:02 +0100 Subject: [PATCH 01/18] docs(auth): activate AUTH-04B review --- .agent-loop/LOOP_STATE.md | 21 +++++++++---------- .agent-loop/REVIEW_LOG.md | 12 +++++++++++ .agent-loop/WORK_QUEUE.md | 11 +++++----- .../CHUNK_MAP.md | 11 +++++----- .../STATUS.md | 17 ++++++++------- .../WS-AUTH-001-04B-postgres-rate-controls.md | 11 ++++++++-- 6 files changed, 50 insertions(+), 33 deletions(-) diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index e9c8d73..78844af 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -3,21 +3,20 @@ ## Current State - Active initiative: `WS-AUTH-001` - Workstream Authorization Service -- Active planning chunk: none +- Active planning chunk: `WS-AUTH-001-04B` - PostgreSQL Rate Controls - Active implementation chunk: none -- Branch: `codex/ws-auth-001-04a-post-merge-memory` -- Worktree: `/home/abiorh/flow/workstream-authorization-service` -- Status: AUTH-04A merged through PR #111 as `90c9a28` after Backend, Agent - Gates, CodeRabbit, required internal review, and explicit human approval - passed. +- Branch: `codex/ws-auth-001-04b-postgres-rate-controls` +- Worktree: `/home/abiorh/flow/workstream-auth-001-04b` +- Status: AUTH-04A post-merge memory merged through PR #112 as `7749f54`; the + user explicitly started AUTH-04B, whose 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: `90c9a28` -- Current gate: publish and merge the AUTH-04A post-merge memory update, then - stop. +- Latest integrated `main` merge commit: `7749f54` +- Current gate: required AUTH-04B preimplementation review; no runtime edit is + permitted until the contract passes. - Next chunk: none; do not start `WS-AUTH-001-05` automatically. -- `WS-AUTH-001-04B` is inactive until this memory update merges and the user - gives its separate explicit start signal. +- `WS-AUTH-001-04B` is active for planning/review only. AUTH-05 and later chunks + remain inactive. - 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. diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 4337239..fe10c0e 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-04B Started + +PR #112 merged AUTH-04A post-merge memory to `main` as `7749f54` after Backend, +Agent Gates, CodeRabbit, required internal review, and explicit human approval +passed. The user then explicitly started `WS-AUTH-001-04B` in the isolated +worktree `/home/abiorh/flow/workstream-auth-001-04b` on branch +`codex/ws-auth-001-04b-postgres-rate-controls`. + +AUTH-04B is L1/P1 authorization infrastructure. Its existing bounded contract +must pass required preimplementation review before runtime edits. AUTH-05, +POL-002-04, and QA implementation remain inactive. + ## 2026-07-13 - WS-AUTH-001-04A Merged PR #111 merged `WS-AUTH-001-04A` to `main` as `90c9a28` after final branch head diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index a3a1c11..795c9db 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-04A post-merge memory in progress; no product implementation active | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Explicitly started; required preimplementation review active, runtime gated | ## 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 | 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 | @@ -57,10 +56,10 @@ ## Proposed Next -`WS-AUTH-001-04A` merged through PR #111 as `90c9a28` after all required checks -and reviews passed. Its post-merge memory update is active; no AUTH product -implementation is active. Do not start 04B, AUTH-05, or POL-002-04 -automatically. +`WS-AUTH-001-04A` post-merge memory merged through PR #112 as `7749f54`. The +user explicitly started `WS-AUTH-001-04B`; only its required preimplementation +review is active, and runtime edits remain gated. Do not start 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 aa89b88..e8704a0 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 @@ -17,7 +17,7 @@ stopped. | `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 | Merged through PR #111 as `90c9a28` | -| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Inactive pending 04A memory merge and separate explicit start | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Explicitly started; required 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 | @@ -79,8 +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 merged through PR #111 as `90c9a28` after required -internal review, Backend, Agent Gates, CodeRabbit, and explicit human approval -passed. Its post-merge memory update is the current gate. -Do not start AUTH-04B, AUTH-05, or POL-002-04; each retains its separate start -signal and prerequisites. +implementation. AUTH-04A merged through PR #111 as `90c9a28`, and its +post-merge memory merged through PR #112 as `7749f54`. The user explicitly +started AUTH-04B; required preimplementation review is the current gate and no +runtime edit is permitted before it passes. Do not start AUTH-05 or POL-002-04. 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 287de74..b8b5aaa 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 @@ -34,11 +34,14 @@ required tracks pass on production SHA `cdcaf77`, and final test-only head logging-state behavior-test repairs plus lifecycle evidence. Backend, Agent Gates, and CodeRabbit passed on final branch head `36c4aa5`, then explicit human approval merged PR #111 to `main` as `90c9a28` on 2026-07-13. -AUTH-04A post-merge memory is active; AUTH-04B remains inactive. +AUTH-04A post-merge memory merged through PR #112 as `7749f54`. The user then +explicitly started AUTH-04B; its required L1 preimplementation review is active +and runtime edits remain gated. ## Active planning chunk -None. +`WS-AUTH-001-04B` - PostgreSQL Rate Controls. Planning/review only; runtime +implementation has not started. ## Active implementation chunk @@ -46,8 +49,7 @@ None. ## Current implementation branch -None. Post-merge memory uses `codex/ws-auth-001-04a-post-merge-memory` and makes -no product implementation change. +`codex/ws-auth-001-04b-postgres-rate-controls`. ## Chunk status @@ -59,7 +61,7 @@ no product implementation change. | `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` | Merged | `codex/ws-auth-001-04-request-api-controls` | #111 | Merged as `90c9a28`; production review `cdcaf77`; final branch head `36c4aa5`. | -| `WS-AUTH-001-04B` | Inactive | - | - | PostgreSQL rate controls; requires 04A merge/memory and separate explicit start. | +| `WS-AUTH-001-04B` | Preimplementation review | `codex/ws-auth-001-04b-postgres-rate-controls` | - | Explicitly started after 04A memory merged; runtime gated on required review. | | `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. | @@ -75,9 +77,8 @@ no product implementation change. ## Blockers -No active implementation blocker because no AUTH product chunk is active. -AUTH-04B may start only after this memory update merges and the user gives a -separate explicit start signal. It later owns migration `0017`, following the +No external blocker. AUTH-04B runtime implementation is internally gated until +its L1 preimplementation review passes. It owns migration `0017`, following the now-owned `0016` prefix on current main. Non-test operators must later supply explicit classification evidence rather than inferred kinds before the owning canonical actor migration. 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 index 8b95deb..e6d35ad 100644 --- 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 @@ -2,8 +2,9 @@ ## Status -Inactive. This contract requires AUTH-04A merge/memory, a separate explicit -user start, and required preimplementation review before any runtime edit. +Active for required preimplementation review after AUTH-04A merge/memory and +the user's separate explicit start. No runtime edit is permitted until review +passes. ## Parent initiative @@ -149,3 +150,9 @@ git diff --check - 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. + +## Activation + +AUTH-04A post-merge memory merged through PR #112 as `7749f54`. The user +explicitly started AUTH-04B on 2026-07-13. Required L1 preimplementation review +must pass before runtime edits. From 78e2170b99db25111811b34e46e345bebbd59794 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 23:20:13 +0100 Subject: [PATCH 02/18] docs(auth): lock AUTH-04B rate semantics --- .agent-loop/LOOP_STATE.md | 4 +- .agent-loop/REVIEW_LOG.md | 15 ++ .agent-loop/WORK_QUEUE.md | 2 +- .../RISKS.md | 1 + .../STATUS.md | 11 +- .../WS-AUTH-001-04B-postgres-rate-controls.md | 136 +++++++++++++----- 6 files changed, 128 insertions(+), 41 deletions(-) diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index 78844af..cf01988 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -12,8 +12,8 @@ - Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836` - Prior `WS-AUTH-001-01` final merged branch head: `b5217e1` - Latest integrated `main` merge commit: `7749f54` -- Current gate: required AUTH-04B preimplementation review; no runtime edit is - permitted until the contract passes. +- Current gate: repair the rejected AUTH-04B contract and pass exact-head + preimplementation re-review; no runtime edit is permitted before that pass. - Next chunk: none; do not start `WS-AUTH-001-05` automatically. - `WS-AUTH-001-04B` is active for planning/review only. AUTH-05 and later chunks remain inactive. diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index fe10c0e..1ad23e0 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -1,5 +1,20 @@ # Review Log +## 2026-07-13 - WS-AUTH-001-04B Contract Repair Required + +Required L1 preimplementation review rejected activation head `5ed410d` before +any runtime edit. The contract did not bind exact HMAC/secret bytes, overflow- +safe SQL, database-time boundaries, dedicated commit ownership, dependency +errors, real concurrency counts, transactional downgrade refusal, or expired +pseudonymous-row retention. + +The repaired contract specifies canonical Base64 secret material, exact framed +HMAC input and bounds, `BYTEA(32)` persistence, one clock-bound saturating +upsert, dedicated committed sessions, canonical 429/503 behavior, bounded +pruning plus operator cleanup, exact migration custody, real-ASGI/concurrency +proof, per-file 90 percent coverage, and additive test-delta evidence. Runtime +remains gated on exact-head re-review. + ## 2026-07-13 - WS-AUTH-001-04B Started PR #112 merged AUTH-04A post-merge memory to `main` as `7749f54` after Backend, diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index 795c9db..0e61d35 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -4,7 +4,7 @@ | Chunk | Title | Risk | Status | |---|---|---:|---| -| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Explicitly started; required preimplementation review active, runtime gated | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Initial preimplementation review rejected ambiguity; repaired-contract re-review active, runtime gated | ## Planned Next diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/RISKS.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/RISKS.md index 767f5d1..7e6fe72 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/RISKS.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/RISKS.md @@ -33,6 +33,7 @@ | A17 | Canonical actor migration deletes typed-profile workflow eligibility before task/submission cutover | An intermediate merged release cannot claim, start, or submit work | Bounded non-authoritative workflow-eligibility adapter in chunk 06; remove task consumers in 13 and final consumer plus adapter in 14 | Full suite/API drill after chunks 06, 13, 14, and scanner proof in 15 | | A18 | Authority evidence is mutable or denial events arrive late | Security decisions cannot be reconstructed | Insert/read-only repository API, database append-only enforcement, per-mutation allowed/denied event proof, operational retention controls | Update/delete rejection plus atomic allowed/denied event tests in owning chunks | | A19 | Authority invalidation releases a needs-revision task as ordinary ready work | Prior findings, context, supersession, or replay obligations are bypassed | Keep the task in needs_revision with a durable unassigned obligation and controlled replacement assignment | Replacement-contributor revision-context, supersession, and high/medium replay tests | +| A20 | Rate-limit replicas split counters, leak identity keys, lose increments, or retain pseudonymous rows indefinitely | Abuse controls bypassed or privacy/availability incident | Canonical Base64 HMAC secret, exact framed digest, one committed PostgreSQL statement, coordinated quiesced rotation, bounded opportunistic pruning, and operator idle cleanup | Known-answer/privacy tests, synchronized independent-session concurrency, commit-failure proof, rotation runbook, and retention tests | ## Required reviewers 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 b8b5aaa..d3cdaad 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 @@ -36,12 +36,13 @@ Backend, Agent Gates, and CodeRabbit passed on final branch head `36c4aa5`, then explicit human approval merged PR #111 to `main` as `90c9a28` on 2026-07-13. AUTH-04A post-merge memory merged through PR #112 as `7749f54`. The user then explicitly started AUTH-04B; its required L1 preimplementation review is active -and runtime edits remain gated. +and rejected the first activated contract before runtime edits. Repaired- +contract re-review is active; runtime remains gated. ## Active planning chunk -`WS-AUTH-001-04B` - PostgreSQL Rate Controls. Planning/review only; runtime -implementation has not started. +`WS-AUTH-001-04B` - PostgreSQL Rate Controls. Repaired-contract review only; +runtime implementation has not started. ## Active implementation chunk @@ -61,7 +62,7 @@ None. | `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` | Merged | `codex/ws-auth-001-04-request-api-controls` | #111 | Merged as `90c9a28`; production review `cdcaf77`; final branch head `36c4aa5`. | -| `WS-AUTH-001-04B` | Preimplementation review | `codex/ws-auth-001-04b-postgres-rate-controls` | - | Explicitly started after 04A memory merged; runtime gated on required review. | +| `WS-AUTH-001-04B` | Contract re-review | `codex/ws-auth-001-04b-postgres-rate-controls` | - | Initial review rejected ambiguity; repaired contract is under exact-head review and runtime remains gated. | | `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. | @@ -78,7 +79,7 @@ None. ## Blockers No external blocker. AUTH-04B runtime implementation is internally gated until -its L1 preimplementation review passes. It owns migration `0017`, following the +its repaired L1 contract passes re-review. It owns migration `0017`, following the now-owned `0016` prefix on current main. Non-test operators must later supply explicit classification evidence rather than inferred kinds before the owning canonical actor migration. 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 index e6d35ad..a494a12 100644 --- 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 @@ -2,9 +2,9 @@ ## Status -Active for required preimplementation review after AUTH-04A merge/memory and -the user's separate explicit start. No runtime edit is permitted until review -passes. +Active for contract repair after required preimplementation review rejected the +first activated revision. No runtime edit is permitted until repaired-contract +re-review passes. ## Parent initiative @@ -24,6 +24,8 @@ L1 / P1 - Stop at 350 changed non-comment production lines for a scope checkpoint. - Stop and replan above 500 changed non-comment production lines. +- Production count includes `backend/app/**` and the Alembic migration. Tests, + docs, and `.agent-loop` evidence do not count. ## Allowed files @@ -41,6 +43,7 @@ backend/tests/test_config.py backend/tests/test_alembic.py backend/scripts/api_contract_e2e.py docs/operations_authorization_service.md +docs/architecture_data_model.md .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/** .agent-loop/LOOP_STATE.md .agent-loop/WORK_QUEUE.md @@ -59,68 +62,134 @@ 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. +- Create only `api_rate_control_counters` with `control_scope VARCHAR(32)`, + `key_digest BYTEA`, `window_started_at TIMESTAMPTZ`, + `window_expires_at TIMESTAMPTZ`, `request_count BIGINT`, and + `updated_at TIMESTAMPTZ`, all non-null. The composite primary key is + `(control_scope, key_digest)`; there is no actor or surrogate identifier. +- Name and enforce the composite primary key, exact server scope-token check, + `octet_length(key_digest) = 32`, `request_count between 1 and + 9223372036854775807`, and `window_started_at < window_expires_at`. Create + `ix_api_rate_control_counters_window_expires_at`, which is consumed by the + pruning query. +- Windows are first-consumption-anchored fixed windows. One PostgreSQL upsert + binds a `clock` CTE to one `statement_timestamp()` value. It inserts count 1; + on conflict it resets start/end/count when existing `window_expires_at <=` + database now, otherwise it increments. Active-window saturation must use + `CASE WHEN request_count = 9223372036854775807 THEN request_count ELSE + request_count + 1 END`; never evaluate max plus one. New expiry is database + now plus parameterized `make_interval(secs => :window_seconds)`. +- The one statement returns the CTE database now, persisted window start/end, + and persisted count. All ORM timestamps are timezone aware. - 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. + clamped to integer `1..window_seconds`; application time is never consulted. +- The service accepts an injectable async session factory whose production + default is the existing `get_session_factory()`. Every consume opens an + independent short-lived session, performs bounded pruning plus the upsert, + and commits before returning a decision. The dependency may construct/raise + 429 only after the committed decision returns. Downstream rollback cannot + undo consumption. +- Session-open, prune, execute, and commit `SQLAlchemyError` failures map to one + local unavailable error and then a constant retryable structured 503. A + failed transaction is rolled back without overriding the stable error. + Cancellation and other `BaseException` values propagate. ## 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. +- The exact server-owned scope tokens are ASCII `first_access` and + `admin_mutation`; callers cannot supply another scope. +- Derive `key_digest` with HMAC-SHA256. The key setting is a Pydantic + `SecretStr` containing canonical padded RFC 4648 Base64. Decode strictly with + no whitespace or alternate encoding; require 32..64 decoded bytes and the + canonical re-encoding to equal the supplied ASCII string. Pydantic repr and + validation errors must never expose the value. +- The HMAC preimage is exactly + `len(domain)||domain||len(scope)||scope||len(issuer)||issuer||len(subject)||subject`, + in that order. Every length is an unsigned four-byte big-endian byte length. + Domain is exact ASCII `workstream-api-rate/v1`; scope is its exact ASCII + token; issuer and subject are exact UTF-8. Do not trim, case-fold, or Unicode + normalize. Enforce issuer and subject independently at 1..4096 UTF-8 bytes + before framing. Commit literal known-answer vectors and collision-separation + tests that do not calculate expected values through the production helper. - 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. +- Add `api_rate_limit_key_secret: SecretStr | None` with no default. Present + malformed, non-ASCII, noncanonical, whitespace-bearing, too-short, or + too-long 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. + invoking a protected dependency without it returns structured HTTP 503 with + code `service_unavailable`, message/detail `Service unavailable`, empty + details, and `retryable=true`. Database unavailability uses the same response. - Secret rotation intentionally resets effective counters. The runbook requires - quiescing protected writes for at most the largest configured window before - replacing the secret. + quiescing every protected write for at least the largest configured window, + rotating every replica while writes remain quiesced, pruning expired rows, + and only then resuming. Mixed-secret replicas must never serve protected + writes. + +## Retention contract + +- The service owns bounded opportunistic pruning in the same dedicated + transaction before its upsert: delete at most 100 rows ordered by expiry whose + `window_expires_at <= statement_timestamp()`, using row locking with + `SKIP LOCKED`, while excluding the exact key being consumed so its expiry-reset + branch remains atomic and observable. +- Expired rows are no longer rate authority. The runbook owns an operator + cleanup command for idle systems and secret rotation; it deletes only expired + rows by database time. Tests prove pruning is bounded, concurrent-safe, and + never persists or emits identity material. ## 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. +- Each dependency consumes only `AuthVerificationResult.token.issuer` and + `.subject` from `get_auth_verification_result`. It must not consume + `ActorContext`, `get_registered_actor`, actor ID, roles, email, or an + issuer-specific adapter. Scope, limit, and window are server constants plus + validated settings. +- A denied dependency raises `StructuredHTTPException` 429 with code + `rate_limit_exceeded`, message/detail `Rate limit exceeded`, empty details, + `retryable=true`, and canonical integer `Retry-After`. Reuse 04A handlers and + database/session infrastructure; introduce no parallel error or DB layer. +- Inventory path/method and the recursive dependency graph to prove no + production route or consumer changed. Exercise dependencies only through + test-only FastAPI routes using real ASGI transport and 04A handlers. - 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. + `Retry-After`, subsecond and application-clock-skew behavior, and privacy-safe + persistence/logging. +- Synchronized real-PostgreSQL concurrency uses independently created sessions + with `N > limit` and proves exactly `limit` allows, `N-limit` denials, final + durable count `N`, repeated-denial consumption, no lost increments, and + saturation at BIGINT max without overflow. ## 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. +- Seed representative existing `0016` domain rows and prove they survive the + upgrade unchanged. Prove every named primary-key/check/index contract and + reject invalid direct inserts. +- Downgrade must query the rate table and raise a bounded `RuntimeError` when it + is nonempty. PostgreSQL/Alembic transaction rollback must leave its rows, + table, and revision at `0017`. After explicit test-only cleanup, prove empty + `0017 -> 0016 -> 0017` succeeds. Never claim a rate row survives a successful + downgrade that drops its table. ## 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 && WORKSTREAM_DATABASE_URL= .venv/bin/python -m pytest -q --cov=app.modules.api_controls --cov=app.api.deps.api_controls --cov=app.core.config --cov-report=term-missing tests/test_api_rate_controls.py tests/test_config.py tests/test_alembic.py) +(cd backend && for file in app/core/config.py app/api/deps/api_controls.py app/modules/api_controls/models.py app/modules/api_controls/repository.py app/modules/api_controls/service.py; do .venv/bin/coverage report --include="$file" --precision=2 --fail-under=90; done) (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 @@ -128,6 +197,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 b5dceb18786fae8fd4c34459271a9838570abb31 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 23:25:28 +0100 Subject: [PATCH 03/18] docs(auth): close AUTH-04B contract gaps --- .agent-loop/REVIEW_LOG.md | 9 +++- .../WS-AUTH-001-04B-postgres-rate-controls.md | 54 ++++++++++++------- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 1ad23e0..12e82b8 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -9,12 +9,19 @@ errors, real concurrency counts, transactional downgrade refusal, or expired pseudonymous-row retention. The repaired contract specifies canonical Base64 secret material, exact framed -HMAC input and bounds, `BYTEA(32)` persistence, one clock-bound saturating +HMAC input and bounds, `BYTEA` persistence guarded to 32 bytes, one clock-bound saturating upsert, dedicated committed sessions, canonical 429/503 behavior, bounded pruning plus operator cleanup, exact migration custody, real-ASGI/concurrency proof, per-file 90 percent coverage, and additive test-delta evidence. Runtime remains gated on exact-head re-review. +First repaired head `78e2170` resolved the original security/data ambiguities +but failed re-review on optional-secret syntax, missing-database mapping, +prune-before-upsert lock ordering, exact setting/constraint names, same-clock +`updated_at`, and local oversized-identity handling. This second repair is the +final in-place contract cycle for those classes; another failure stops and +replans before runtime code. + ## 2026-07-13 - WS-AUTH-001-04B Started PR #112 merged AUTH-04A post-merge memory to `main` as `7749f54` after Backend, 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 index a494a12..78b8485 100644 --- 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 @@ -67,9 +67,13 @@ AUTH-05 or later chunk implementation `window_expires_at TIMESTAMPTZ`, `request_count BIGINT`, and `updated_at TIMESTAMPTZ`, all non-null. The composite primary key is `(control_scope, key_digest)`; there is no actor or surrogate identifier. -- Name and enforce the composite primary key, exact server scope-token check, - `octet_length(key_digest) = 32`, `request_count between 1 and - 9223372036854775807`, and `window_started_at < window_expires_at`. Create +- Name the constraints `pk_api_rate_control_counters`, + `ck_api_rate_control_counters_scope_token`, + `ck_api_rate_control_counters_digest_length`, + `ck_api_rate_control_counters_request_count`, and + `ck_api_rate_control_counters_window_order`. They enforce the composite key, + exact scope tokens, `octet_length(key_digest) = 32`, `request_count between 1 + and 9223372036854775807`, and `window_started_at < window_expires_at`. Create `ix_api_rate_control_counters_window_expires_at`, which is consumed by the pruning query. - Windows are first-consumption-anchored fixed windows. One PostgreSQL upsert @@ -80,7 +84,9 @@ AUTH-05 or later chunk implementation request_count + 1 END`; never evaluate max plus one. New expiry is database now plus parameterized `make_interval(secs => :window_seconds)`. - The one statement returns the CTE database now, persisted window start/end, - and persisted count. All ORM timestamps are timezone aware. + and persisted count. `updated_at` is set to that same CTE database timestamp + on insert, expiry reset, ordinary increment, and saturated denial. All ORM + timestamps are timezone aware. - 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 integer `1..window_seconds`; application time is never consulted. @@ -91,9 +97,12 @@ AUTH-05 or later chunk implementation 429 only after the committed decision returns. Downstream rollback cannot undo consumption. - Session-open, prune, execute, and commit `SQLAlchemyError` failures map to one - local unavailable error and then a constant retryable structured 503. A - failed transaction is rolled back without overriding the stable error. - Cancellation and other `BaseException` values propagate. + local unavailable error and then a constant retryable structured 503. The + existing `get_session_factory()` missing-database `RuntimeError` maps narrowly + by its exact stable configuration failure; every other `RuntimeError` is + re-raised. A failed transaction is rolled back without overriding the stable + error. Cancellation and other `BaseException` values propagate. Tests cover + absent database configuration separately from open/execute/commit failures. ## Privacy and configuration contract @@ -114,28 +123,34 @@ AUTH-05 or later chunk implementation tests that do not calculate expected values through the production helper. - 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: SecretStr | None` with no default. Present +- Add `api_rate_limit_key_secret: SecretStr | None = None`; there is no non-null + fallback and no generated secret. Present malformed, non-ASCII, noncanonical, whitespace-bearing, too-short, or too-long 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`. +- Exact numeric settings are `api_first_access_rate_limit=10`, + `api_first_access_rate_window_seconds=60`, + `api_admin_mutation_rate_limit=30`, and + `api_admin_mutation_rate_window_seconds=60`, 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 structured HTTP 503 with code `service_unavailable`, message/detail `Service unavailable`, empty details, and `retryable=true`. Database unavailability uses the same response. - Secret rotation intentionally resets effective counters. The runbook requires - quiescing every protected write for at least the largest configured window, - rotating every replica while writes remain quiesced, pruning expired rows, - and only then resuming. Mixed-secret replicas must never serve protected - writes. + quiescing every protected write for at least the largest effective + pre-rotation window configured on any replica, rotating every replica while + writes remain quiesced, pruning expired rows, and only then resuming. + Mixed-secret replicas must never serve protected writes. ## Retention contract - The service owns bounded opportunistic pruning in the same dedicated - transaction before its upsert: delete at most 100 rows ordered by expiry whose - `window_expires_at <= statement_timestamp()`, using row locking with - `SKIP LOCKED`, while excluding the exact key being consumed so its expiry-reset - branch remains atomic and observable. + transaction after its upsert: delete at most 100 other rows ordered by expiry + whose `window_expires_at <= statement_timestamp()`, using row locking with + `SKIP LOCKED`. Upsert-first lock ordering ensures a concurrently consumed key + is already active/locked and skipped rather than creating a two-key lock + cycle. A synchronized two-expired-key PostgreSQL test proves no deadlock or + 503. - Expired rows are no longer rate authority. The runbook owns an operator cleanup command for idle systems and secret rotation; it deletes only expired rows by database time. Tests prove pruning is bounded, concurrent-safe, and @@ -150,6 +165,9 @@ AUTH-05 or later chunk implementation `ActorContext`, `get_registered_actor`, actor ID, roles, email, or an issuer-specific adapter. Scope, limit, and window are server constants plus validated settings. +- A verified issuer or subject outside the local 1..4096 UTF-8 byte boundary is + treated as the same non-echoing local unavailable condition and structured + retryable `service_unavailable` 503, never an unhandled 500. - A denied dependency raises `StructuredHTTPException` 429 with code `rate_limit_exceeded`, message/detail `Rate limit exceeded`, empty details, `retryable=true`, and canonical integer `Retry-After`. Reuse 04A handlers and From df50f62385ae29ab7ecb9e7de13a28cfd074a501 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 13 Jul 2026 23:29:33 +0100 Subject: [PATCH 04/18] docs(auth): authorize AUTH-04B implementation --- .agent-loop/LOOP_STATE.md | 8 ++++---- .agent-loop/REVIEW_LOG.md | 12 ++++++++++++ .agent-loop/WORK_QUEUE.md | 2 +- .../CHUNK_MAP.md | 6 ++++-- .../STATUS.md | 15 ++++++++------- .../WS-AUTH-001-04B-postgres-rate-controls.md | 8 ++++---- 6 files changed, 33 insertions(+), 18 deletions(-) diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index cf01988..65f894e 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -3,8 +3,8 @@ ## Current State - Active initiative: `WS-AUTH-001` - Workstream Authorization Service -- Active planning chunk: `WS-AUTH-001-04B` - PostgreSQL Rate Controls -- Active implementation chunk: none +- Active planning chunk: none +- Active implementation chunk: `WS-AUTH-001-04B` - PostgreSQL Rate Controls - Branch: `codex/ws-auth-001-04b-postgres-rate-controls` - Worktree: `/home/abiorh/flow/workstream-auth-001-04b` - Status: AUTH-04A post-merge memory merged through PR #112 as `7749f54`; the @@ -12,8 +12,8 @@ - Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836` - Prior `WS-AUTH-001-01` final merged branch head: `b5217e1` - Latest integrated `main` merge commit: `7749f54` -- Current gate: repair the rejected AUTH-04B contract and pass exact-head - preimplementation re-review; no runtime edit is permitted before that pass. +- Current gate: bounded AUTH-04B implementation and deterministic evidence under + the reviewed contract at `b5dceb1`. - Next chunk: none; do not start `WS-AUTH-001-05` automatically. - `WS-AUTH-001-04B` is active for planning/review only. AUTH-05 and later chunks remain inactive. diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 12e82b8..077f277 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-04B Repaired Contract Passed + +Required senior engineering, architecture, security/data, QA/test, CI-integrity, +test-delta, product/ops, docs, and reuse review passed exact repaired-contract +head `b5dceb1`. The second repair closed optional-secret, missing-database, +lock-order, oversized-identity, exact-setting/constraint, same-clock timestamp, +and cross-replica rotation gaps without runtime edits. + +Bounded AUTH-04B implementation is authorized under the 350-line checkpoint and +500-line hard stop. Named dependencies remain unattached; AUTH-05 and all +product authority changes remain inactive. + ## 2026-07-13 - WS-AUTH-001-04B Contract Repair Required Required L1 preimplementation review rejected activation head `5ed410d` before diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index 0e61d35..04770c0 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -4,7 +4,7 @@ | Chunk | Title | Risk | Status | |---|---|---:|---| -| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Initial preimplementation review rejected ambiguity; repaired-contract re-review active, runtime gated | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Repaired contract passed at `b5dceb1`; bounded implementation active | ## 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 e8704a0..ef1a058 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 @@ -17,7 +17,7 @@ stopped. | `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 | Merged through PR #111 as `90c9a28` | -| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Explicitly started; required preimplementation review active | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Repaired contract passed at `b5dceb1`; bounded implementation 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 | @@ -82,4 +82,6 @@ explicitly started parent AUTH-04. Required plan review split it before runtime implementation. AUTH-04A merged through PR #111 as `90c9a28`, and its post-merge memory merged through PR #112 as `7749f54`. The user explicitly started AUTH-04B; required preimplementation review is the current gate and no -runtime edit is permitted before it passes. Do not start AUTH-05 or POL-002-04. +runtime edit was permitted before it passed. The second repaired contract passed +at `b5dceb1`; bounded AUTH-04B implementation is active. Do not start AUTH-05 or +POL-002-04. 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 d3cdaad..76a1af4 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 @@ -36,13 +36,14 @@ Backend, Agent Gates, and CodeRabbit passed on final branch head `36c4aa5`, then explicit human approval merged PR #111 to `main` as `90c9a28` on 2026-07-13. AUTH-04A post-merge memory merged through PR #112 as `7749f54`. The user then explicitly started AUTH-04B; its required L1 preimplementation review is active -and rejected the first activated contract before runtime edits. Repaired- -contract re-review is active; runtime remains gated. +and rejected the first activated contract before runtime edits. The second +repaired contract passed all required tracks at `b5dceb1`; bounded runtime +implementation is active. ## Active planning chunk -`WS-AUTH-001-04B` - PostgreSQL Rate Controls. Repaired-contract review only; -runtime implementation has not started. +`WS-AUTH-001-04B` - PostgreSQL Rate Controls. Bounded implementation under the +reviewed contract at `b5dceb1`. ## Active implementation chunk @@ -62,7 +63,7 @@ None. | `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` | Merged | `codex/ws-auth-001-04-request-api-controls` | #111 | Merged as `90c9a28`; production review `cdcaf77`; final branch head `36c4aa5`. | -| `WS-AUTH-001-04B` | Contract re-review | `codex/ws-auth-001-04b-postgres-rate-controls` | - | Initial review rejected ambiguity; repaired contract is under exact-head review and runtime remains gated. | +| `WS-AUTH-001-04B` | Implementing | `codex/ws-auth-001-04b-postgres-rate-controls` | - | Second repaired contract passed at `b5dceb1`; 350/500-line circuit breaker applies. | | `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. | @@ -78,8 +79,8 @@ None. ## Blockers -No external blocker. AUTH-04B runtime implementation is internally gated until -its repaired L1 contract passes re-review. It owns migration `0017`, following the +No external blocker. AUTH-04B runtime implementation is active under its passed +repaired L1 contract. It owns migration `0017`, following the now-owned `0016` prefix on current main. Non-test operators must later supply explicit classification evidence rather than inferred kinds before the owning canonical actor migration. 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 index 78b8485..5dbf110 100644 --- 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 @@ -2,9 +2,9 @@ ## Status -Active for contract repair after required preimplementation review rejected the -first activated revision. No runtime edit is permitted until repaired-contract -re-review passes. +Active for bounded implementation. Required preimplementation review passed the +second repaired contract at `b5dceb1`; the 350-line checkpoint and 500-line hard +stop apply. ## Parent initiative @@ -243,4 +243,4 @@ git diff --check AUTH-04A post-merge memory merged through PR #112 as `7749f54`. The user explicitly started AUTH-04B on 2026-07-13. Required L1 preimplementation review -must pass before runtime edits. +passed the second repaired contract at `b5dceb1` before runtime edits. From 62dd18e800f2d011e2b1024538062c8150dd97f6 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 00:05:16 +0100 Subject: [PATCH 05/18] feat(auth): add durable API rate controls --- .agent-loop/LOOP_STATE.md | 14 +- .agent-loop/REVIEW_LOG.md | 18 + .agent-loop/WORK_QUEUE.md | 7 +- .../STATUS.md | 13 +- .../WS-AUTH-001-04B-postgres-rate-controls.md | 7 + backend/alembic/versions/0017_api_controls.py | 69 +++ backend/app/api/deps/api_controls.py | 97 ++++ backend/app/core/config.py | 33 +- backend/app/db/models.py | 1 + backend/app/modules/api_controls/__init__.py | 1 + backend/app/modules/api_controls/models.py | 42 ++ .../app/modules/api_controls/repository.py | 101 ++++ backend/app/modules/api_controls/service.py | 133 +++++ backend/scripts/api_contract_e2e.py | 60 ++ backend/tests/test_alembic.py | 193 +++++++ backend/tests/test_api_controls.py | 541 +++++++++++++++++- backend/tests/test_auth.py | 3 +- backend/tests/test_config.py | 55 ++ docs/architecture_data_model.md | 16 + docs/operations_authorization_service.md | 47 ++ 20 files changed, 1432 insertions(+), 19 deletions(-) create mode 100644 backend/alembic/versions/0017_api_controls.py create mode 100644 backend/app/api/deps/api_controls.py create mode 100644 backend/app/modules/api_controls/__init__.py create mode 100644 backend/app/modules/api_controls/models.py create mode 100644 backend/app/modules/api_controls/repository.py create mode 100644 backend/app/modules/api_controls/service.py diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index 65f894e..e86bcbd 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -8,15 +8,19 @@ - Branch: `codex/ws-auth-001-04b-postgres-rate-controls` - Worktree: `/home/abiorh/flow/workstream-auth-001-04b` - Status: AUTH-04A post-merge memory merged through PR #112 as `7749f54`; the - user explicitly started AUTH-04B, whose L1 preimplementation review is active. + user explicitly started AUTH-04B; its implementation candidate is entering + required internal review after deterministic evidence passed. - Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836` - Prior `WS-AUTH-001-01` final merged branch head: `b5217e1` - Latest integrated `main` merge commit: `7749f54` -- Current gate: bounded AUTH-04B implementation and deterministic evidence under - the reviewed contract at `b5dceb1`. +- Current gate: required AUTH-04B implementation review against an immutable + candidate SHA; no PR until all valid findings are resolved. +- Size checkpoint: implementation is 409 changed non-comment production lines; the + required 350-line inspection passed and scope is frozen below the 500 stop. - Next chunk: none; do not start `WS-AUTH-001-05` automatically. -- `WS-AUTH-001-04B` is active for planning/review only. AUTH-05 and later chunks - remain inactive. +- Focused evidence: 93 owned tests pass at 99 percent aggregate changed-runtime + coverage; the isolated PostgreSQL concurrency/migration proofs and real API + E2E pass. AUTH-05 and later chunks remain inactive. - 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. diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 077f277..8d536ae 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -12,6 +12,24 @@ Bounded AUTH-04B implementation is authorized under the 350-line checkpoint and 500-line hard stop. Named dependencies remain unattached; AUTH-05 and all product authority changes remain inactive. +`backend/tests/test_auth.py` was added as a test-only scope amendment after its +canonical-verification consumer allowlist correctly detected the new unattached +rate dependency. Only that expected inventory may change; auth runtime, routes, +compatibility behavior, and authority remain out of scope. + +The implementation candidate passed 93 owned tests with 99 percent aggregate +coverage across config, dependency, model, repository, and service modules. +Real PostgreSQL proofs passed for atomic concurrency, durable denial, expiry, +saturation, pruning, exact schema, and guarded downgrade. The real API E2E and +all stale-wording, authorization-doc, artifact-contract, link, loop-state, +ruff, and diff checks pass. Required implementation review is the current gate. + +The first production pass reached 408 changed non-comment lines. The mandatory +350-line checkpoint inspected every production path and froze scope to the +approved config, unattached dependencies, model, repository, service, model +registration, and `0017` migration. Ninety-two lines remain before the hard +stop; tests, docs, and evidence do not count. + ## 2026-07-13 - WS-AUTH-001-04B Contract Repair Required Required L1 preimplementation review rejected activation head `5ed410d` before diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index 04770c0..456b470 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -4,7 +4,7 @@ | Chunk | Title | Risk | Status | |---|---|---:|---| -| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Repaired contract passed at `b5dceb1`; bounded implementation active | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Implementation candidate evidence passed; required internal review pending | ## Planned Next @@ -57,9 +57,8 @@ ## Proposed Next `WS-AUTH-001-04A` post-merge memory merged through PR #112 as `7749f54`. The -user explicitly started `WS-AUTH-001-04B`; only its required preimplementation -review is active, and runtime edits remain gated. Do not start AUTH-05 or -POL-002-04 automatically. +user explicitly started `WS-AUTH-001-04B`; its bounded implementation candidate +is in required internal review. Do not start 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/STATUS.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md index 76a1af4..2a37e86 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 @@ -38,16 +38,17 @@ AUTH-04A post-merge memory merged through PR #112 as `7749f54`. The user then explicitly started AUTH-04B; its required L1 preimplementation review is active and rejected the first activated contract before runtime edits. The second repaired contract passed all required tracks at `b5dceb1`; bounded runtime -implementation is active. +implementation and deterministic evidence are complete, and the candidate is +entering required internal review. ## Active planning chunk -`WS-AUTH-001-04B` - PostgreSQL Rate Controls. Bounded implementation under the -reviewed contract at `b5dceb1`. +None. ## Active implementation chunk -None. +`WS-AUTH-001-04B` - PostgreSQL Rate Controls. Implementation candidate under +required internal review; AUTH-05 remains inactive. ## Current implementation branch @@ -63,7 +64,7 @@ None. | `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` | Merged | `codex/ws-auth-001-04-request-api-controls` | #111 | Merged as `90c9a28`; production review `cdcaf77`; final branch head `36c4aa5`. | -| `WS-AUTH-001-04B` | Implementing | `codex/ws-auth-001-04b-postgres-rate-controls` | - | Second repaired contract passed at `b5dceb1`; 350/500-line circuit breaker applies. | +| `WS-AUTH-001-04B` | Internal review | `codex/ws-auth-001-04b-postgres-rate-controls` | - | 409 production lines; 93 owned tests and real API E2E pass; required reviewers pending. | | `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. | @@ -79,7 +80,7 @@ None. ## Blockers -No external blocker. AUTH-04B runtime implementation is active under its passed +No external blocker. AUTH-04B implementation review is active under its passed repaired L1 contract. It owns migration `0017`, following the now-owned `0016` prefix on current main. Non-test operators must later supply explicit classification evidence rather than 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 index 5dbf110..4d2fd2d 100644 --- 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 @@ -39,6 +39,7 @@ 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_auth.py backend/tests/test_config.py backend/tests/test_alembic.py backend/scripts/api_contract_e2e.py @@ -219,6 +220,12 @@ git diff --unified=0 origin/main...HEAD -- backend/tests | (! rg '^-(.*assert|.* git diff --check ``` +`backend/tests/test_auth.py` was added as a test-only scope amendment after the +implemented canonical-verification consumer correctly tripped its static +consumer allowlist. The amendment may update only that allowlist for the new +unattached `api/deps/api_controls.py` consumer; it does not permit auth runtime, +route, compatibility, or authority changes. + ## Required reviewers - senior engineering diff --git a/backend/alembic/versions/0017_api_controls.py b/backend/alembic/versions/0017_api_controls.py new file mode 100644 index 0000000..35d5c00 --- /dev/null +++ b/backend/alembic/versions/0017_api_controls.py @@ -0,0 +1,69 @@ +"""add durable API rate controls + +Revision ID: 0017_api_controls +Revises: 0016_artifact_domain +Create Date: 2026-07-13 +""" + +from __future__ import annotations + +from alembic import op +import sqlalchemy as sa + +revision = "0017_api_controls" +down_revision = "0016_artifact_domain" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + """Create the privacy-keyed cross-replica counter table.""" + op.create_table( + "api_rate_control_counters", + sa.Column("control_scope", sa.String(32), nullable=False), + sa.Column("key_digest", sa.LargeBinary(), nullable=False), + sa.Column("window_started_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("window_expires_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("request_count", sa.BigInteger(), nullable=False), + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), + sa.PrimaryKeyConstraint( + "control_scope", + "key_digest", + name="pk_api_rate_control_counters", + ), + sa.CheckConstraint( + "control_scope in ('first_access', 'admin_mutation')", + name=op.f("ck_api_rate_control_counters_scope_token"), + ), + sa.CheckConstraint( + "octet_length(key_digest) = 32", + name=op.f("ck_api_rate_control_counters_digest_length"), + ), + sa.CheckConstraint( + "request_count between 1 and 9223372036854775807", + name=op.f("ck_api_rate_control_counters_request_count"), + ), + sa.CheckConstraint( + "window_started_at < window_expires_at", + name=op.f("ck_api_rate_control_counters_window_order"), + ), + ) + op.create_index( + "ix_api_rate_control_counters_window_expires_at", + "api_rate_control_counters", + ["window_expires_at"], + ) + + +def downgrade() -> None: + """Drop only an empty rate-control table.""" + has_rows = op.get_bind().execute( + sa.text("select exists(select 1 from api_rate_control_counters)") + ).scalar_one() + if has_rows: + raise RuntimeError("cannot downgrade non-empty API rate controls") + op.drop_index( + "ix_api_rate_control_counters_window_expires_at", + table_name="api_rate_control_counters", + ) + op.drop_table("api_rate_control_counters") diff --git a/backend/app/api/deps/api_controls.py b/backend/app/api/deps/api_controls.py new file mode 100644 index 0000000..62af6f7 --- /dev/null +++ b/backend/app/api/deps/api_controls.py @@ -0,0 +1,97 @@ +"""Unattached FastAPI dependencies for future protected mutations.""" + +from __future__ import annotations + +from typing import Annotated + +from fastapi import Depends, Request, status + +from app.api.deps.auth import get_auth_verification_result +from app.core.api_controls import StructuredHTTPException +from app.modules.api_controls.service import ( + ADMIN_MUTATION_SCOPE, + FIRST_ACCESS_SCOPE, + RateControlService, + RateControlUnavailableError, +) +from app.schemas.auth import AuthVerificationResult + + +def get_rate_control_service() -> RateControlService: + """Return a limiter whose consumption owns a dedicated database session.""" + return RateControlService() + + +def _unavailable() -> StructuredHTTPException: + return StructuredHTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Service unavailable", + error_code="service_unavailable", + error_message="Service unavailable", + retryable=True, + ) + + +async def _enforce( + *, + request: Request, + result: AuthVerificationResult, + service: RateControlService, + control_scope: str, + limit: int, + window_seconds: int, +) -> None: + try: + decision = await service.consume( + control_scope=control_scope, + issuer=result.token.issuer, + subject=result.token.subject, + limit=limit, + window_seconds=window_seconds, + secret=request.app.state.settings.api_rate_limit_key_secret, + ) + except RateControlUnavailableError as exc: + raise _unavailable() from exc + if not decision.allowed: + raise StructuredHTTPException( + status_code=status.HTTP_429_TOO_MANY_REQUESTS, + detail="Rate limit exceeded", + error_code="rate_limit_exceeded", + error_message="Rate limit exceeded", + retryable=True, + headers={"Retry-After": str(decision.retry_after)}, + ) + + +async def enforce_first_access_rate_limit( + request: Request, + result: Annotated[AuthVerificationResult, Depends(get_auth_verification_result)], + service: Annotated[RateControlService, Depends(get_rate_control_service)], +) -> None: + """Consume the future first-access mutation allowance.""" + settings = request.app.state.settings + await _enforce( + request=request, + result=result, + service=service, + control_scope=FIRST_ACCESS_SCOPE, + limit=settings.api_first_access_rate_limit, + window_seconds=settings.api_first_access_rate_window_seconds, + ) + + +async def enforce_admin_mutation_rate_limit( + request: Request, + result: Annotated[AuthVerificationResult, Depends(get_auth_verification_result)], + service: Annotated[RateControlService, Depends(get_rate_control_service)], +) -> None: + """Consume the future authority-management mutation allowance.""" + settings = request.app.state.settings + await _enforce( + request=request, + result=result, + service=service, + control_scope=ADMIN_MUTATION_SCOPE, + limit=settings.api_admin_mutation_rate_limit, + window_seconds=settings.api_admin_mutation_rate_window_seconds, + ) diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 85de4fc..2ee19be 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -2,11 +2,13 @@ from __future__ import annotations +import base64 +import binascii from functools import lru_cache from pathlib import Path from typing import Literal -from pydantic import Field, model_validator +from pydantic import Field, SecretStr, field_validator, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict @@ -76,6 +78,11 @@ class Settings(BaseSettings): celery_result_backend_url: str | None = None celery_task_always_eager: bool = False actor_registry_refresh_interval_seconds: int = Field(default=300, ge=0, le=86_400) + api_rate_limit_key_secret: SecretStr | None = None + api_first_access_rate_limit: int = Field(default=10, ge=1, le=10_000) + api_first_access_rate_window_seconds: int = Field(default=60, ge=1, le=3_600) + api_admin_mutation_rate_limit: int = Field(default=30, ge=1, le=10_000) + api_admin_mutation_rate_window_seconds: int = Field(default=60, ge=1, le=3_600) artifact_store_backend: Literal["disabled", "local", "flow_node"] = "disabled" artifact_local_root: Path | None = None artifact_retention_policy_version: str | None = None @@ -86,8 +93,19 @@ class Settings(BaseSettings): env_prefix="WORKSTREAM_", env_file=".env", extra="ignore", + hide_input_in_errors=True, ) + @field_validator("api_rate_limit_key_secret") + @classmethod + def validate_api_rate_limit_key_secret( + cls, value: SecretStr | None + ) -> SecretStr | None: + """Require canonical bounded Base64 while retaining redacted storage.""" + if value is not None: + decode_api_rate_limit_key_secret(value) + return value + @model_validator(mode="after") def validate_artifact_storage(self) -> Settings: """Reject unsafe or incomplete artifact-storage combinations. @@ -120,3 +138,16 @@ def get_settings() -> Settings: Settings resolved from environment variables and optional ``.env``. """ return Settings() + + +def decode_api_rate_limit_key_secret(value: SecretStr) -> bytes: + """Decode one canonical padded Base64 rate-control key.""" + secret = value.get_secret_value() + try: + encoded = secret.encode("ascii") + decoded = base64.b64decode(encoded, validate=True) + except (UnicodeEncodeError, binascii.Error, ValueError) as exc: + raise ValueError("invalid API rate limit key secret") from exc + if not 32 <= len(decoded) <= 64 or base64.b64encode(decoded) != encoded: + raise ValueError("invalid API rate limit key secret") + return decoded diff --git a/backend/app/db/models.py b/backend/app/db/models.py index 947d31d..67e2d1d 100644 --- a/backend/app/db/models.py +++ b/backend/app/db/models.py @@ -1,6 +1,7 @@ """Import domain models so Alembic can discover metadata.""" from app.modules.actors.models import ActorIdentity, ActorProfile # noqa: F401 +from app.modules.api_controls.models import ApiRateControlCounter # noqa: F401 from app.modules.artifacts.models import ( # noqa: F401 ArtifactBinding, ArtifactContent, diff --git a/backend/app/modules/api_controls/__init__.py b/backend/app/modules/api_controls/__init__.py new file mode 100644 index 0000000..1a5b9bd --- /dev/null +++ b/backend/app/modules/api_controls/__init__.py @@ -0,0 +1 @@ +"""Durable API control primitives.""" diff --git a/backend/app/modules/api_controls/models.py b/backend/app/modules/api_controls/models.py new file mode 100644 index 0000000..a99dafb --- /dev/null +++ b/backend/app/modules/api_controls/models.py @@ -0,0 +1,42 @@ +"""SQLAlchemy models for durable API controls.""" + +from __future__ import annotations + +from datetime import datetime + +from sqlalchemy import CheckConstraint, DateTime, Index, LargeBinary, BigInteger, String +from sqlalchemy.orm import Mapped, mapped_column + +from app.db.base import Base + + +class ApiRateControlCounter(Base): + """Privacy-keyed fixed-window counter shared by every API replica.""" + + __tablename__ = "api_rate_control_counters" + __table_args__ = ( + CheckConstraint( + "control_scope in ('first_access', 'admin_mutation')", + name="scope_token", + ), + CheckConstraint("octet_length(key_digest) = 32", name="digest_length"), + CheckConstraint( + "request_count between 1 and 9223372036854775807", + name="request_count", + ), + CheckConstraint( + "window_started_at < window_expires_at", + name="window_order", + ), + Index( + "ix_api_rate_control_counters_window_expires_at", + "window_expires_at", + ), + ) + + control_scope: Mapped[str] = mapped_column(String(32), primary_key=True) + key_digest: Mapped[bytes] = mapped_column(LargeBinary, primary_key=True) + window_started_at: Mapped[datetime] = mapped_column(DateTime(timezone=True)) + window_expires_at: Mapped[datetime] = mapped_column(DateTime(timezone=True)) + request_count: Mapped[int] = mapped_column(BigInteger) + updated_at: Mapped[datetime] = mapped_column(DateTime(timezone=True)) diff --git a/backend/app/modules/api_controls/repository.py b/backend/app/modules/api_controls/repository.py new file mode 100644 index 0000000..9072015 --- /dev/null +++ b/backend/app/modules/api_controls/repository.py @@ -0,0 +1,101 @@ +"""PostgreSQL statements for durable API controls.""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime + +from sqlalchemy import text +from sqlalchemy.ext.asyncio import AsyncSession + +UPSERT_COUNTER = text( + """ + with clock as (select statement_timestamp() as db_now) + insert into api_rate_control_counters as counters ( + control_scope, key_digest, window_started_at, window_expires_at, + request_count, updated_at + ) + select :control_scope, :key_digest, db_now, + db_now + make_interval(secs => :window_seconds), 1, db_now + from clock + on conflict (control_scope, key_digest) do update set + window_started_at = case + when counters.window_expires_at <= (select db_now from clock) + then (select db_now from clock) else counters.window_started_at end, + window_expires_at = case + when counters.window_expires_at <= (select db_now from clock) + then (select db_now from clock) + make_interval(secs => :window_seconds) + else counters.window_expires_at end, + request_count = case + when counters.window_expires_at <= (select db_now from clock) then 1 + when counters.request_count = 9223372036854775807 + then counters.request_count else counters.request_count + 1 end, + updated_at = (select db_now from clock) + returning (select db_now from clock) as db_now, window_started_at, + window_expires_at, request_count + """ +) + +PRUNE_EXPIRED = text( + """ + with expired as ( + select control_scope, key_digest + from api_rate_control_counters + where window_expires_at <= statement_timestamp() + and not (control_scope = :control_scope and key_digest = :key_digest) + order by window_expires_at + limit :batch_size + for update skip locked + ) + delete from api_rate_control_counters as counters + using expired + where counters.control_scope = expired.control_scope + and counters.key_digest = expired.key_digest + """ +) + + +@dataclass(frozen=True) +class ConsumedCounter: + """Database-time result of one atomic counter consumption.""" + + db_now: datetime + window_started_at: datetime + window_expires_at: datetime + request_count: int + + +class ApiRateControlRepository: + """Consume and prune counters inside a caller-owned transaction.""" + + def __init__(self, session: AsyncSession) -> None: + self._session = session + + async def consume( + self, control_scope: str, key_digest: bytes, window_seconds: int + ) -> ConsumedCounter: + """Atomically insert, reset, increment, or saturate one counter.""" + row = ( + await self._session.execute( + UPSERT_COUNTER, + { + "control_scope": control_scope, + "key_digest": key_digest, + "window_seconds": window_seconds, + }, + ) + ).one() + return ConsumedCounter(*row) + + async def prune_expired( + self, control_scope: str, key_digest: bytes, *, batch_size: int + ) -> None: + """Delete a bounded batch of other expired pseudonymous rows.""" + await self._session.execute( + PRUNE_EXPIRED, + { + "control_scope": control_scope, + "key_digest": key_digest, + "batch_size": batch_size, + }, + ) diff --git a/backend/app/modules/api_controls/service.py b/backend/app/modules/api_controls/service.py new file mode 100644 index 0000000..621d754 --- /dev/null +++ b/backend/app/modules/api_controls/service.py @@ -0,0 +1,133 @@ +"""Privacy-keyed rate-control service with independent commit ownership.""" + +from __future__ import annotations + +from contextlib import suppress +from dataclasses import dataclass +import hashlib +import hmac +import math +from typing import TypeAlias + +from pydantic import SecretStr +from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from app.core.config import decode_api_rate_limit_key_secret +from app.db.session import get_session_factory +from app.modules.api_controls.repository import ApiRateControlRepository + +RATE_KEY_DOMAIN = b"workstream-api-rate/v1" +FIRST_ACCESS_SCOPE = "first_access" +ADMIN_MUTATION_SCOPE = "admin_mutation" +RATE_SCOPES = {FIRST_ACCESS_SCOPE, ADMIN_MUTATION_SCOPE} +MAX_IDENTITY_BYTES = 4_096 +PRUNE_BATCH_SIZE = 100 +MISSING_DATABASE_ERROR = "WORKSTREAM_DATABASE_URL must be set before database access" +SessionFactory: TypeAlias = async_sessionmaker[AsyncSession] + + +class RateControlUnavailableError(RuntimeError): + """Raised when a rate decision cannot be made safely.""" + + +@dataclass(frozen=True) +class RateControlDecision: + """Committed fixed-window decision derived from database time.""" + + allowed: bool + request_count: int + retry_after: int + + +def _frame(value: bytes) -> bytes: + return len(value).to_bytes(4, "big") + value + + +def rate_key_digest( + secret: SecretStr, control_scope: str, issuer: str, subject: str +) -> bytes: + """Derive the exact privacy key for one verified external identity.""" + if control_scope not in RATE_SCOPES: + raise ValueError("unsupported rate control scope") + issuer_bytes = issuer.encode("utf-8") + subject_bytes = subject.encode("utf-8") + if not 1 <= len(issuer_bytes) <= MAX_IDENTITY_BYTES or not 1 <= len( + subject_bytes + ) <= MAX_IDENTITY_BYTES: + raise RateControlUnavailableError("rate control unavailable") + preimage = b"".join( + _frame(value) + for value in ( + RATE_KEY_DOMAIN, + control_scope.encode("ascii"), + issuer_bytes, + subject_bytes, + ) + ) + return hmac.new( + decode_api_rate_limit_key_secret(secret), preimage, hashlib.sha256 + ).digest() + + +class RateControlService: + """Consume durable counters in a transaction independent of request work.""" + + def __init__(self, session_factory: SessionFactory | None = None) -> None: + self._session_factory = session_factory + + def _factory(self) -> SessionFactory: + if self._session_factory is not None: + return self._session_factory + try: + return get_session_factory() + except RuntimeError as exc: + if str(exc) != MISSING_DATABASE_ERROR: + raise + raise RateControlUnavailableError("rate control unavailable") from exc + + async def consume( + self, + *, + control_scope: str, + issuer: str, + subject: str, + limit: int, + window_seconds: int, + secret: SecretStr | None, + ) -> RateControlDecision: + """Commit one counter use before returning its allow/deny decision.""" + if secret is None: + raise RateControlUnavailableError("rate control unavailable") + digest = rate_key_digest(secret, control_scope, issuer, subject) + session: AsyncSession | None = None + try: + async with self._factory()() as session: + repository = ApiRateControlRepository(session) + consumed = await repository.consume( + control_scope, digest, window_seconds + ) + await repository.prune_expired( + control_scope, digest, batch_size=PRUNE_BATCH_SIZE + ) + await session.commit() + except SQLAlchemyError as exc: + if session is not None: + with suppress(SQLAlchemyError): + await session.rollback() + raise RateControlUnavailableError("rate control unavailable") from exc + + retry_after = max( + 1, + min( + window_seconds, + math.ceil( + (consumed.window_expires_at - consumed.db_now).total_seconds() + ), + ), + ) + return RateControlDecision( + allowed=consumed.request_count <= limit, + request_count=consumed.request_count, + retry_after=retry_after, + ) diff --git a/backend/scripts/api_contract_e2e.py b/backend/scripts/api_contract_e2e.py index 8791927..f8f9f38 100644 --- a/backend/scripts/api_contract_e2e.py +++ b/backend/scripts/api_contract_e2e.py @@ -20,8 +20,15 @@ import httpx from alembic import command from alembic.config import Config +from pydantic import SecretStr +from sqlalchemy import text from app.db import session as db_session +from app.modules.api_controls.service import ( + FIRST_ACCESS_SCOPE, + RateControlService, + rate_key_digest, +) from app.modules.projects.models import PostSubmitCheckerPolicy, ProjectSetupRun from app.modules.projects.post_submit_policy import ( build_project_post_submit_checker_spec, @@ -210,10 +217,62 @@ def api_environment() -> dict[str, str]: env["WORKSTREAM_CELERY_TASK_ALWAYS_EAGER"] = "true" env["WORKSTREAM_CELERY_BROKER_URL"] = "memory://" env["WORKSTREAM_CELERY_RESULT_BACKEND_URL"] = "cache+memory://" + env.setdefault( + "WORKSTREAM_API_RATE_LIMIT_KEY_SECRET", + base64.b64encode(os.urandom(32)).decode("ascii"), + ) env["PYTHONPATH"] = str(project_root()) return env +async def exercise_rate_control_contract(env: dict[str, str]) -> None: + """Prove one durable denial without exposing the pseudonymous key.""" + subject = f"api-contract-rate-{uuid4()}" + secret = SecretStr(env["WORKSTREAM_API_RATE_LIMIT_KEY_SECRET"]) + service = RateControlService() + first = await service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=DEFAULT_FLOW_ISSUER, + subject=subject, + limit=1, + window_seconds=60, + secret=secret, + ) + denied = await service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=DEFAULT_FLOW_ISSUER, + subject=subject, + limit=1, + window_seconds=60, + secret=secret, + ) + assert first.allowed is True and first.request_count == 1 + assert denied.allowed is False and denied.request_count == 2 + assert 1 <= denied.retry_after <= 60 + + digest = rate_key_digest(secret, FIRST_ACCESS_SCOPE, DEFAULT_FLOW_ISSUER, subject) + async with db_session.get_session_factory()() as session: + row = ( + await session.execute( + text( + "select control_scope, key_digest, request_count " + "from api_rate_control_counters " + "where control_scope = :scope and key_digest = :digest" + ), + {"scope": FIRST_ACCESS_SCOPE, "digest": digest}, + ) + ).one() + assert tuple(row) == (FIRST_ACCESS_SCOPE, digest, 2) + await session.execute( + text( + "delete from api_rate_control_counters " + "where control_scope = :scope and key_digest = :digest" + ), + {"scope": FIRST_ACCESS_SCOPE, "digest": digest}, + ) + await session.commit() + + def start_api_server(port: int, env: dict[str, str]) -> tuple[subprocess.Popen, Path]: """Start a real uvicorn server process. @@ -1320,6 +1379,7 @@ async def main(env: dict[str, str]) -> None: env: Environment variables for the API server. """ await db_session.dispose_engine() + await exercise_rate_control_contract(env) port = find_free_port() base_url = f"http://127.0.0.1:{port}" diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 8f0a8d0..b98b557 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -292,6 +292,82 @@ def test_artifact_foundation_enforces_immutable_facts_and_guarded_downgrade( command.downgrade(config, "base") +def test_api_rate_control_schema_preserves_domain_and_guards_downgrade( + isolated_database_env: str, + migration_lock, +) -> None: + """Prove 0017 schema guards, preservation, and transactional downgrade refusal.""" + project_root = Path(__file__).resolve().parents[1] + config = Config(str(project_root / "alembic.ini")) + config.set_main_option("script_location", str(project_root / "alembic")) + project_id = str(uuid4()) + digest = bytes(range(32)) + + with migration_lock(): + try: + command.downgrade(config, "base") + command.upgrade(config, "0015_post_submit_correction") + asyncio.run(_seed_artifact_prior_head(isolated_database_env, project_id)) + command.upgrade(config, "0016_artifact_domain") + before = asyncio.run( + _artifact_prior_head_project(isolated_database_env, project_id) + ) + + command.upgrade(config, "0017_api_controls") + after = asyncio.run( + _artifact_prior_head_project(isolated_database_env, project_id) + ) + schema = asyncio.run(_api_rate_control_schema(isolated_database_env)) + asyncio.run(_assert_api_rate_control_guards(isolated_database_env, digest)) + + with pytest.raises(RuntimeError, match="non-empty API rate controls"): + command.downgrade(config, "0016_artifact_domain") + refused_state = asyncio.run(_api_rate_control_state(isolated_database_env)) + + asyncio.run(_clear_api_rate_controls(isolated_database_env)) + command.downgrade(config, "0016_artifact_domain") + downgraded_state = asyncio.run( + _api_rate_control_state(isolated_database_env) + ) + command.upgrade(config, "0017_api_controls") + finally: + asyncio.run(_clear_api_rate_controls(isolated_database_env)) + command.downgrade(config, "base") + + assert after == before + assert schema == { + "columns": { + "control_scope:character varying:NO", + "key_digest:bytea:NO", + "window_started_at:timestamp with time zone:NO", + "window_expires_at:timestamp with time zone:NO", + "request_count:bigint:NO", + "updated_at:timestamp with time zone:NO", + }, + "constraints": { + "pk_api_rate_control_counters", + "ck_api_rate_control_counters_scope_token", + "ck_api_rate_control_counters_digest_length", + "ck_api_rate_control_counters_request_count", + "ck_api_rate_control_counters_window_order", + }, + "indexes": { + "pk_api_rate_control_counters", + "ix_api_rate_control_counters_window_expires_at", + }, + } + assert refused_state == { + "revision": "0017_api_controls", + "table_exists": True, + "row_count": 1, + } + assert downgraded_state == { + "revision": "0016_artifact_domain", + "table_exists": False, + "row_count": None, + } + + async def _fetch_columns(database_url: str) -> set[str]: """Return current public table columns as table.column names.""" engine = create_async_engine(database_url) @@ -1704,3 +1780,120 @@ async def _truncate_artifact_foundation(database_url: str) -> None: ) finally: await engine.dispose() + + +async def _api_rate_control_schema(database_url: str) -> dict[str, set[str]]: + """Return the exact public schema contract for the rate table.""" + engine = create_async_engine(database_url) + try: + async with engine.begin() as connection: + columns = ( + await connection.execute( + text( + "select column_name, data_type, is_nullable " + "from information_schema.columns " + "where table_schema = 'public' " + "and table_name = 'api_rate_control_counters'" + ) + ) + ).all() + constraints = ( + await connection.execute( + text( + "select conname from pg_constraint " + "where conrelid = 'api_rate_control_counters'::regclass" + ) + ) + ).scalars() + indexes = ( + await connection.execute( + text( + "select indexname from pg_indexes " + "where schemaname = 'public' " + "and tablename = 'api_rate_control_counters'" + ) + ) + ).scalars() + return { + "columns": { + f"{row.column_name}:{row.data_type}:{row.is_nullable}" + for row in columns + }, + "constraints": set(constraints), + "indexes": set(indexes), + } + finally: + await engine.dispose() + + +async def _assert_api_rate_control_guards(database_url: str, digest: bytes) -> None: + """Insert one valid counter and reject every malformed direct variant.""" + engine = create_async_engine(database_url) + insert_sql = text( + "insert into api_rate_control_counters " + "(control_scope, key_digest, window_started_at, window_expires_at, " + "request_count, updated_at) values " + "(:scope, :digest, statement_timestamp(), " + "statement_timestamp() + make_interval(secs => :seconds), " + ":count, statement_timestamp())" + ) + valid = { + "scope": "first_access", + "digest": digest, + "seconds": 60, + "count": 1, + } + try: + async with engine.begin() as connection: + await connection.execute(insert_sql, valid) + + invalid = [ + {**valid, "scope": "caller_supplied", "digest": bytes([1]) * 32}, + {**valid, "digest": bytes(31)}, + {**valid, "digest": bytes([2]) * 32, "count": 0}, + {**valid, "digest": bytes([3]) * 32, "seconds": 0}, + valid, + ] + for values in invalid: + with pytest.raises(IntegrityError): + async with engine.begin() as connection: + await connection.execute(insert_sql, values) + finally: + await engine.dispose() + + +async def _api_rate_control_state(database_url: str) -> dict[str, object]: + """Return revision, table existence, and row count after migration actions.""" + engine = create_async_engine(database_url) + try: + async with engine.begin() as connection: + revision = await connection.scalar(text("select version_num from alembic_version")) + exists = await connection.scalar( + text("select to_regclass('public.api_rate_control_counters') is not null") + ) + count = None + if exists: + count = await connection.scalar( + text("select count(*) from api_rate_control_counters") + ) + return { + "revision": revision, + "table_exists": exists, + "row_count": count, + } + finally: + await engine.dispose() + + +async def _clear_api_rate_controls(database_url: str) -> None: + """Clear rate rows only when the migration table exists.""" + engine = create_async_engine(database_url) + try: + async with engine.begin() as connection: + exists = await connection.scalar( + text("select to_regclass('public.api_rate_control_counters') is not null") + ) + if exists: + await connection.execute(text("delete from api_rate_control_counters")) + finally: + await engine.dispose() diff --git a/backend/tests/test_api_controls.py b/backend/tests/test_api_controls.py index f80697d..46dd5e8 100644 --- a/backend/tests/test_api_controls.py +++ b/backend/tests/test_api_controls.py @@ -1,22 +1,43 @@ from __future__ import annotations import asyncio +from datetime import UTC, datetime, timedelta from hashlib import sha256 import logging from typing import Annotated from uuid import UUID, uuid4 -from fastapi import BackgroundTasks, HTTPException +from fastapi import BackgroundTasks, Depends, HTTPException from fastapi.exceptions import RequestValidationError from fastapi.responses import Response, StreamingResponse from httpx import ASGITransport, AsyncClient -from pydantic import BaseModel, Field, field_validator +from pydantic import BaseModel, Field, SecretStr, field_validator import pytest +from sqlalchemy import text +from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from starlette.requests import Request +from app.api.deps.api_controls import ( + enforce_admin_mutation_rate_limit, + enforce_first_access_rate_limit, + get_rate_control_service, +) +from app.api.deps.auth import get_auth_verification_result from app.core.api_controls import ApiErrorResponse, error_response from app.core.config import Settings from app.main import create_app +from app.modules.api_controls import service as rate_service_module +from app.modules.api_controls.repository import ApiRateControlRepository, ConsumedCounter +from app.modules.api_controls.service import ( + ADMIN_MUTATION_SCOPE, + FIRST_ACCESS_SCOPE, + RateControlDecision, + RateControlService, + RateControlUnavailableError, + rate_key_digest, +) +from app.schemas.auth import AuthVerificationResult, VerifiedIssuerToken def _assert_uuid(value: str) -> None: @@ -474,3 +495,519 @@ def test_openapi_documents_request_error_and_response_context() -> None: 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 + + +RATE_SECRET_TEXT = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" +RATE_SECRET = SecretStr(RATE_SECRET_TEXT) +RATE_ISSUER = "https://issuer.example.test" +RATE_SUBJECT = "opaque-subject" + + +@pytest.fixture +async def rate_control_factory(postgres_database_url: str): + """Provide independent sessions over a clean, migrated rate-control table.""" + engine = create_async_engine(postgres_database_url) + factory = async_sessionmaker(engine, expire_on_commit=False) + async with engine.begin() as connection: + await connection.execute(text("delete from api_rate_control_counters")) + try: + yield factory + finally: + async with engine.begin() as connection: + await connection.execute(text("delete from api_rate_control_counters")) + await engine.dispose() + + +def test_rate_key_digest_matches_literal_vector_and_separates_boundaries() -> None: + expected = bytes.fromhex( + "35b4fb3e6a647a52596a5240b0b3ad5c2976d91771f95e2497be247081e53b31" + ) + + assert rate_key_digest( + RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, RATE_SUBJECT + ) == expected + assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "ab", "c") != ( + rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "a", "bc") + ) + assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "Issuer", "subject") != ( + rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "subject") + ) + assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "e\u0301") != ( + rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "\u00e9") + ) + assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "subject") != ( + rate_key_digest(RATE_SECRET, ADMIN_MUTATION_SCOPE, "issuer", "subject") + ) + + +@pytest.mark.parametrize("identity", ["", "x" * 4_097, "\u00e9" * 2_049]) +def test_rate_key_digest_rejects_unbounded_identity_without_echo(identity: str) -> None: + with pytest.raises(RateControlUnavailableError) as caught: + rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, identity, "subject") + assert str(caught.value) == "rate control unavailable" + + +async def _stored_rate_row(factory, scope: str, digest: bytes): + async with factory() as session: + return ( + await session.execute( + text( + "select window_started_at, window_expires_at, request_count " + "from api_rate_control_counters " + "where control_scope = :scope and key_digest = :digest" + ), + {"scope": scope, "digest": digest}, + ) + ).one() + + +async def test_rate_control_commits_fixed_window_denials_and_resets_expiry( + rate_control_factory, +) -> None: + service = RateControlService(rate_control_factory) + decisions = [ + await service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=2, + window_seconds=60, + secret=RATE_SECRET, + ) + for _ in range(4) + ] + digest = rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, RATE_SUBJECT) + original = await _stored_rate_row(rate_control_factory, FIRST_ACCESS_SCOPE, digest) + + assert [decision.allowed for decision in decisions] == [True, True, False, False] + assert [decision.request_count for decision in decisions] == [1, 2, 3, 4] + assert all(1 <= decision.retry_after <= 60 for decision in decisions) + assert original.request_count == 4 + + async with rate_control_factory() as session: + await session.execute( + text( + "update api_rate_control_counters set " + "window_started_at = statement_timestamp() - interval '2 seconds', " + "window_expires_at = statement_timestamp() - interval '1 second' " + "where control_scope = :scope and key_digest = :digest" + ), + {"scope": FIRST_ACCESS_SCOPE, "digest": digest}, + ) + await session.commit() + + reset = await service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=2, + window_seconds=60, + secret=RATE_SECRET, + ) + persisted = await _stored_rate_row(rate_control_factory, FIRST_ACCESS_SCOPE, digest) + assert reset == RateControlDecision(allowed=True, request_count=1, retry_after=60) + assert persisted.request_count == 1 + assert persisted.window_started_at > original.window_started_at + + +async def test_rate_control_concurrency_has_no_lost_or_rolled_back_consumption( + rate_control_factory, +) -> None: + service = RateControlService(rate_control_factory) + + async def consume_once(): + return await service.consume( + control_scope=ADMIN_MUTATION_SCOPE, + issuer=RATE_ISSUER, + subject="concurrent-subject", + limit=7, + window_seconds=30, + secret=RATE_SECRET, + ) + + decisions = await asyncio.gather(*(consume_once() for _ in range(20))) + digest = rate_key_digest( + RATE_SECRET, ADMIN_MUTATION_SCOPE, RATE_ISSUER, "concurrent-subject" + ) + persisted = await _stored_rate_row(rate_control_factory, ADMIN_MUTATION_SCOPE, digest) + + assert sum(decision.allowed for decision in decisions) == 7 + assert sorted(decision.request_count for decision in decisions) == list(range(1, 21)) + assert persisted.request_count == 20 + + async with rate_control_factory() as downstream: + await downstream.execute( + text( + "update api_rate_control_counters set request_count = 21 " + "where control_scope = :scope and key_digest = :digest" + ), + {"scope": ADMIN_MUTATION_SCOPE, "digest": digest}, + ) + await downstream.rollback() + assert ( + await _stored_rate_row(rate_control_factory, ADMIN_MUTATION_SCOPE, digest) + ).request_count == 20 + + +async def test_rate_control_saturates_bigint_and_prunes_only_bounded_other_rows( + rate_control_factory, +) -> None: + service = RateControlService(rate_control_factory) + digest = rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, "saturated") + async with rate_control_factory() as session: + await session.execute( + text( + "insert into api_rate_control_counters " + "(control_scope, key_digest, window_started_at, window_expires_at, " + "request_count, updated_at) values " + "(:scope, :digest, statement_timestamp(), " + "statement_timestamp() + interval '1 minute', 9223372036854775807, " + "statement_timestamp())" + ), + {"scope": FIRST_ACCESS_SCOPE, "digest": digest}, + ) + for value in range(125): + await session.execute( + text( + "insert into api_rate_control_counters " + "(control_scope, key_digest, window_started_at, window_expires_at, " + "request_count, updated_at) values " + "(:scope, :digest, statement_timestamp() - interval '2 minutes', " + "statement_timestamp() - interval '1 minute', 1, " + "statement_timestamp() - interval '1 minute')" + ), + { + "scope": ADMIN_MUTATION_SCOPE, + "digest": value.to_bytes(32, "big"), + }, + ) + await session.commit() + + decision = await service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject="saturated", + limit=10_000, + window_seconds=60, + secret=RATE_SECRET, + ) + async with rate_control_factory() as session: + expired = await session.scalar( + text( + "select count(*) from api_rate_control_counters " + "where window_expires_at <= statement_timestamp()" + ) + ) + assert decision.request_count == 9_223_372_036_854_775_807 + assert decision.allowed is False + assert expired == 25 + + +async def test_concurrent_expired_keys_do_not_deadlock_during_pruning( + rate_control_factory, +) -> None: + subjects = ("expired-a", "expired-b") + async with rate_control_factory() as session: + for subject in subjects: + await session.execute( + text( + "insert into api_rate_control_counters " + "(control_scope, key_digest, window_started_at, window_expires_at, " + "request_count, updated_at) values " + "(:scope, :digest, statement_timestamp() - interval '2 seconds', " + "statement_timestamp() - interval '1 second', 5, " + "statement_timestamp() - interval '1 second')" + ), + { + "scope": FIRST_ACCESS_SCOPE, + "digest": rate_key_digest( + RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, subject + ), + }, + ) + await session.commit() + + service = RateControlService(rate_control_factory) + decisions = await asyncio.wait_for( + asyncio.gather( + *( + service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=subject, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + for subject in subjects + ) + ), + timeout=5, + ) + assert decisions == [ + RateControlDecision(allowed=True, request_count=1, retry_after=60), + RateControlDecision(allowed=True, request_count=1, retry_after=60), + ] + + +class _FakeSession: + """Minimal async-session context for deterministic failure tests.""" + + def __init__(self, *, commit_error: bool = False) -> None: + self.commit_error = commit_error + self.rolled_back = False + + async def __aenter__(self): + return self + + async def __aexit__(self, *_args) -> None: + return None + + async def commit(self) -> None: + if self.commit_error: + raise SQLAlchemyError("private commit detail") + + async def rollback(self) -> None: + self.rolled_back = True + + +async def test_rate_control_maps_only_database_failures_and_rolls_back( + monkeypatch: pytest.MonkeyPatch, +) -> None: + now = datetime.now(UTC) + session = _FakeSession() + + async def fail_execute(*_args, **_kwargs): + raise SQLAlchemyError("private execute detail") + + monkeypatch.setattr(ApiRateControlRepository, "consume", fail_execute) + with pytest.raises(RateControlUnavailableError, match="^rate control unavailable$"): + await RateControlService(lambda: session).consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + assert session.rolled_back is True + + async def successful_consume(*_args, **_kwargs): + return ConsumedCounter(now, now, now + timedelta(seconds=60), 1) + + async def successful_prune(*_args, **_kwargs): + return None + + commit_session = _FakeSession(commit_error=True) + monkeypatch.setattr(ApiRateControlRepository, "consume", successful_consume) + monkeypatch.setattr(ApiRateControlRepository, "prune_expired", successful_prune) + with pytest.raises(RateControlUnavailableError, match="^rate control unavailable$"): + await RateControlService(lambda: commit_session).consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + assert commit_session.rolled_back is True + + +async def test_rate_control_narrowly_maps_missing_database_and_propagates_cancel( + monkeypatch: pytest.MonkeyPatch, +) -> None: + def missing_database(): + raise RuntimeError("WORKSTREAM_DATABASE_URL must be set before database access") + + monkeypatch.setattr(rate_service_module, "get_session_factory", missing_database) + with pytest.raises(RateControlUnavailableError, match="^rate control unavailable$"): + await RateControlService().consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + + def unrelated_failure(): + raise RuntimeError("unrelated runtime failure") + + monkeypatch.setattr(rate_service_module, "get_session_factory", unrelated_failure) + with pytest.raises(RuntimeError, match="unrelated runtime failure"): + await RateControlService().consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + + async def cancel(*_args, **_kwargs): + raise asyncio.CancelledError + + monkeypatch.setattr(ApiRateControlRepository, "consume", cancel) + with pytest.raises(asyncio.CancelledError): + await RateControlService(lambda: _FakeSession()).consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + + +def _verified_rate_identity(subject: str = RATE_SUBJECT) -> AuthVerificationResult: + return AuthVerificationResult( + token=VerifiedIssuerToken( + issuer=RATE_ISSUER, + subject=subject, + audience=("workstream",), + expires_at=2_000_000_000, + issued_at=1_999_999_000, + token_id="rate-token", + subject_kind="human", + scopes=frozenset({"workstream:access"}), + ) + ) + + +class _DecisionService: + def __init__(self, decisions: list[RateControlDecision] | None = None) -> None: + self.decisions = decisions or [] + self.calls: list[dict] = [] + + async def consume(self, **kwargs) -> RateControlDecision: + self.calls.append(kwargs) + if not self.decisions: + raise RateControlUnavailableError("private database detail") + return self.decisions.pop(0) + + +async def test_unattached_dependencies_emit_canonical_429_and_use_token_identity() -> None: + service = _DecisionService( + [ + RateControlDecision(True, 1, 60), + RateControlDecision(False, 31, 17), + ] + ) + app = create_app( + Settings(environment="test", api_rate_limit_key_secret=RATE_SECRET_TEXT) + ) + app.dependency_overrides[get_auth_verification_result] = _verified_rate_identity + app.dependency_overrides[get_rate_control_service] = lambda: service + + @app.post( + "/_test/first-access-rate", + dependencies=[Depends(enforce_first_access_rate_limit)], + ) + async def first_access() -> dict[str, bool]: + return {"allowed": True} + + @app.post( + "/_test/admin-mutation-rate", + dependencies=[Depends(enforce_admin_mutation_rate_limit)], + ) + async def admin_mutation() -> dict[str, bool]: + return {"allowed": True} + + async with AsyncClient( + transport=ASGITransport(app=app), base_url="http://testserver" + ) as client: + allowed = await client.post("/_test/first-access-rate") + denied = await client.post("/_test/admin-mutation-rate") + + assert allowed.status_code == 200 + assert denied.status_code == 429 + assert denied.headers["retry-after"] == "17" + assert denied.json() == { + "detail": "Rate limit exceeded", + "error": { + "code": "rate_limit_exceeded", + "message": "Rate limit exceeded", + "details": {}, + "correlation_id": denied.headers["x-correlation-id"], + "retryable": True, + }, + } + assert [call["control_scope"] for call in service.calls] == [ + FIRST_ACCESS_SCOPE, + ADMIN_MUTATION_SCOPE, + ] + assert all(call["issuer"] == RATE_ISSUER for call in service.calls) + assert all(call["subject"] == RATE_SUBJECT for call in service.calls) + + +@pytest.mark.parametrize( + ("settings", "subject", "service"), + [ + (Settings(environment="test"), RATE_SUBJECT, RateControlService()), + ( + Settings(environment="test", api_rate_limit_key_secret=RATE_SECRET_TEXT), + "x" * 4_097, + RateControlService(), + ), + ( + Settings(environment="test", api_rate_limit_key_secret=RATE_SECRET_TEXT), + RATE_SUBJECT, + _DecisionService(), + ), + ], +) +async def test_rate_dependency_unavailability_is_private_503( + settings: Settings, + subject: str, + service, +) -> None: + app = create_app(settings) + app.dependency_overrides[get_auth_verification_result] = lambda: _verified_rate_identity( + subject + ) + app.dependency_overrides[get_rate_control_service] = lambda: service + + @app.post( + "/_test/rate-unavailable", + dependencies=[Depends(enforce_first_access_rate_limit)], + ) + async def unavailable() -> None: + return None + + async with AsyncClient( + transport=ASGITransport(app=app), base_url="http://testserver" + ) as client: + response = await client.post("/_test/rate-unavailable") + + assert response.status_code == 503 + assert response.json() == { + "detail": "Service unavailable", + "error": { + "code": "service_unavailable", + "message": "Service unavailable", + "details": {}, + "correlation_id": response.headers["x-correlation-id"], + "retryable": True, + }, + } + assert subject not in response.text + + +def test_rate_dependencies_are_not_attached_to_production_routes() -> None: + forbidden = { + enforce_first_access_rate_limit, + enforce_admin_mutation_rate_limit, + } + + def dependency_calls(dependant) -> set: + calls = {dependant.call} + for dependency in dependant.dependencies: + calls.update(dependency_calls(dependency)) + return calls + + app = create_app() + assert all( + forbidden.isdisjoint(dependency_calls(route.dependant)) + for route in app.routes + if hasattr(route, "dependant") + ) diff --git a/backend/tests/test_auth.py b/backend/tests/test_auth.py index fbd2886..b5a38e3 100644 --- a/backend/tests/test_auth.py +++ b/backend/tests/test_auth.py @@ -1203,10 +1203,11 @@ def test_legacy_compatibility_dependency_has_fixed_consumer_allowlist() -> None: } assert { path for path, source in sources.items() if "get_auth_verification_result" in source - } == {"api/deps/auth.py"} + } == {"api/deps/api_controls.py", "api/deps/auth.py"} assert {path for path, source in sources.items() if "AuthVerificationResult" in source} == { "adapters/auth/dev.py", "adapters/auth/flow.py", + "api/deps/api_controls.py", "api/deps/auth.py", "core/auth.py", "interfaces/auth.py", diff --git a/backend/tests/test_config.py b/backend/tests/test_config.py index b267798..1262733 100644 --- a/backend/tests/test_config.py +++ b/backend/tests/test_config.py @@ -1,5 +1,7 @@ from __future__ import annotations +import base64 + import pytest from pydantic import ValidationError @@ -23,6 +25,59 @@ def test_default_settings_are_fail_closed(monkeypatch: pytest.MonkeyPatch) -> No assert settings.auth_provider == "flow" assert settings.database_url is None assert settings.dev_auth_token is None + assert settings.api_rate_limit_key_secret is None + assert settings.api_first_access_rate_limit == 10 + assert settings.api_first_access_rate_window_seconds == 60 + assert settings.api_admin_mutation_rate_limit == 30 + assert settings.api_admin_mutation_rate_window_seconds == 60 + + +def test_rate_limit_secret_is_canonical_and_redacted() -> None: + encoded = base64.b64encode(bytes(range(32))).decode("ascii") + settings = Settings(api_rate_limit_key_secret=encoded) + + assert settings.api_rate_limit_key_secret is not None + assert settings.api_rate_limit_key_secret.get_secret_value() == encoded + assert encoded not in repr(settings) + + +@pytest.mark.parametrize( + "value", + [ + "", + " ", + "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=\n", + "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8", + base64.b64encode(bytes(31)).decode("ascii"), + base64.b64encode(bytes(65)).decode("ascii"), + "\u00e9" * 44, + ], +) +def test_rate_limit_secret_rejects_invalid_values_without_echo(value: str) -> None: + with pytest.raises(ValidationError) as caught: + Settings(api_rate_limit_key_secret=value) + rendered = str(caught.value) + assert "input_value" not in rendered + if value.strip(): + assert value not in rendered + + +@pytest.mark.parametrize( + ("field_name", "value"), + [ + ("api_first_access_rate_limit", 0), + ("api_first_access_rate_limit", 10_001), + ("api_admin_mutation_rate_limit", 0), + ("api_admin_mutation_rate_limit", 10_001), + ("api_first_access_rate_window_seconds", 0), + ("api_first_access_rate_window_seconds", 3_601), + ("api_admin_mutation_rate_window_seconds", 0), + ("api_admin_mutation_rate_window_seconds", 3_601), + ], +) +def test_rate_limit_numeric_bounds_are_enforced(field_name: str, value: int) -> None: + with pytest.raises(ValidationError): + Settings(**{field_name: value}) def test_settings_reject_unknown_environment() -> None: diff --git a/docs/architecture_data_model.md b/docs/architecture_data_model.md index 6996e0e..07c1182 100644 --- a/docs/architecture_data_model.md +++ b/docs/architecture_data_model.md @@ -111,6 +111,22 @@ Canonical idempotency records bind operation, actor, request hash, committed result, and replay status. Authority invalidation events are written atomically with grant/profile/link changes and drive bounded reconciliation. +### API Rate Control Counter + +`api_rate_control_counters` is a cross-replica fixed-window control for future +first-access and authority-management mutations. Its composite key is the +server-owned `control_scope` plus a 32-byte HMAC-SHA256 `key_digest` derived +from the exact verified issuer and opaque subject. It stores database-time +window start/expiry, a saturating `BIGINT` request count, and database-time +update evidence. It has no actor/profile foreign key or surrogate identifier +and stores no raw issuer, subject, email, role, token, claim, or network data. + +One PostgreSQL upsert atomically inserts, increments, saturates, or resets an +expired counter using a single statement timestamp. Consumption and bounded +expired-row pruning commit in a short independent session before a route may +continue. These counters are abuse controls, not identity, grants, product +authority, audit events, or a replacement for exact authorization checks. + ### Legacy Migration Existing external `ActorIdentity.actor_id` UUIDs may become canonical profile diff --git a/docs/operations_authorization_service.md b/docs/operations_authorization_service.md index fb0636e..060ab8c 100644 --- a/docs/operations_authorization_service.md +++ b/docs/operations_authorization_service.md @@ -60,6 +60,11 @@ preview fail closed when a required value is absent or outside its bound. | `WORKSTREAM_TOKEN_INTROSPECTION_WRITE_TIMEOUT_SECONDS` | Float `0.1..10` | `3` | | `WORKSTREAM_TOKEN_INTROSPECTION_POOL_TIMEOUT_SECONDS` | Float `0.1..10` | `1` | | `WORKSTREAM_TOKEN_INTROSPECTION_TOTAL_TIMEOUT_SECONDS` | Float `0.5..15` | `5` | +| `WORKSTREAM_API_RATE_LIMIT_KEY_SECRET` | Canonical padded RFC 4648 Base64 decoding to `32..64` bytes | Optional until a protected route is attached; then required | +| `WORKSTREAM_API_FIRST_ACCESS_RATE_LIMIT` | Integer `1..10000` | `10` | +| `WORKSTREAM_API_FIRST_ACCESS_RATE_WINDOW_SECONDS` | Integer `1..3600` | `60` | +| `WORKSTREAM_API_ADMIN_MUTATION_RATE_LIMIT` | Integer `1..10000` | `30` | +| `WORKSTREAM_API_ADMIN_MUTATION_RATE_WINDOW_SECONDS` | Integer `1..3600` | `60` | Secrets, private keys, bearer tokens, full claims, and raw JWKS documents must not appear in committed configuration or evidence. @@ -144,6 +149,48 @@ 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. +## PostgreSQL Rate Controls + +AUTH-04B provides unattached dependencies for future first-access and +authority-management mutations. No production route is rate limited by this +chunk. When an owning route attaches one, every replica must use the same +secret and settings. Missing secret or database access fails the dependency +closed with retryable HTTP 503; an exhausted window returns retryable HTTP 429 +with an integer `Retry-After`. + +Generate the secret outside the repository and store it in the deployment +secret manager: + +```bash +python3 -c 'import base64,secrets; print(base64.b64encode(secrets.token_bytes(32)).decode())' +``` + +Counters store only the server-owned scope and an HMAC-SHA256 digest of the +verified issuer/subject. They do not store raw identity, actor, token, claim, +role, email, or network values. Consumption uses database time and commits in +an independent session before the protected mutation may continue or return +429, so downstream rollback does not restore allowance. + +Secret rotation intentionally creates a new digest space. Before rotating: + +1. Quiesce every protected write for at least the largest effective + pre-rotation window configured on any replica. +2. Rotate every replica while writes remain quiesced; mixed-secret replicas + must not serve protected writes. +3. Delete only rows expired by PostgreSQL time: + +```sql +DELETE FROM api_rate_control_counters +WHERE window_expires_at <= statement_timestamp(); +``` + +4. Confirm all replicas use the new secret, then resume protected writes. + +Runtime consumption opportunistically deletes at most 100 expired other rows. +On idle systems, operators may run the same expired-only SQL cleanup. Never +delete active rows to recover capacity, and never downgrade migration `0017` +while the table is nonempty; the guarded downgrade refuses and rolls back. + ## JWKS Rotation And Outage Rotation procedure: From 2d70581fcba2cbd4d27465c6e027e8b4bae7f716 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 00:23:52 +0100 Subject: [PATCH 06/18] fix(auth): harden rate-control custody --- .agent-loop/LOOP_STATE.md | 8 +- .agent-loop/REVIEW_LOG.md | 9 + .agent-loop/WORK_QUEUE.md | 2 +- .../STATUS.md | 7 +- backend/alembic/versions/0017_api_controls.py | 4 +- backend/app/core/config.py | 16 +- backend/app/modules/api_controls/service.py | 7 +- backend/tests/test_alembic.py | 110 ++- backend/tests/test_api_controls.py | 541 +------------ backend/tests/test_api_rate_controls.py | 737 ++++++++++++++++++ backend/tests/test_config.py | 8 +- docs/operations_authorization_service.md | 4 +- 12 files changed, 888 insertions(+), 565 deletions(-) create mode 100644 backend/tests/test_api_rate_controls.py diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index e86bcbd..3ad3d33 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -8,13 +8,13 @@ - Branch: `codex/ws-auth-001-04b-postgres-rate-controls` - Worktree: `/home/abiorh/flow/workstream-auth-001-04b` - Status: AUTH-04A post-merge memory merged through PR #112 as `7749f54`; the - user explicitly started AUTH-04B; its implementation candidate is entering - required internal review after deterministic evidence passed. + user explicitly started AUTH-04B. Exact-head review rejected candidate + `62dd18e`; one bounded repair cycle 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: `7749f54` -- Current gate: required AUTH-04B implementation review against an immutable - candidate SHA; no PR until all valid findings are resolved. +- Current gate: deterministic repair evidence, then exact-head re-review; no PR + until every valid finding is resolved. - Size checkpoint: implementation is 409 changed non-comment production lines; the required 350-line inspection passed and scope is frozen below the 500 stop. - Next chunk: none; do not start `WS-AUTH-001-05` automatically. diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 8d536ae..f2a9147 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -2,6 +2,15 @@ ## 2026-07-13 - WS-AUTH-001-04B Repaired Contract Passed +Implementation candidate `62dd18e` failed required exact-head review. Valid +findings were structured Pydantic secret retention, unpaired-surrogate identity +500s, a concurrent downgrade custody race, unsynchronized database tests, +incomplete downstream-rollback/session-open/prune/database-clock evidence, no +representative 0016 artifact row, and tests mistakenly placed in the prior +AUTH-04A file instead of the contract-owned rate-control file. One bounded +repair cycle moves the tests, closes each runtime/migration issue, adds exact +proof, and requires full-suite coverage plus exact-head re-review. + Required senior engineering, architecture, security/data, QA/test, CI-integrity, test-delta, product/ops, docs, and reuse review passed exact repaired-contract head `b5dceb1`. The second repair closed optional-secret, missing-database, diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index 456b470..9c40aac 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -4,7 +4,7 @@ | Chunk | Title | Risk | Status | |---|---|---:|---| -| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Implementation candidate evidence passed; required internal review pending | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Candidate `62dd18e` failed internal review; bounded repair/evidence active | ## Planned Next 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 2a37e86..15daae9 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 @@ -39,7 +39,8 @@ explicitly started AUTH-04B; its required L1 preimplementation review is active and rejected the first activated contract before runtime edits. The second repaired contract passed all required tracks at `b5dceb1`; bounded runtime implementation and deterministic evidence are complete, and the candidate is -entering required internal review. +in one bounded repair cycle after exact-head internal review rejected +`62dd18e`. ## Active planning chunk @@ -64,7 +65,7 @@ required internal review; AUTH-05 remains inactive. | `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` | Merged | `codex/ws-auth-001-04-request-api-controls` | #111 | Merged as `90c9a28`; production review `cdcaf77`; final branch head `36c4aa5`. | -| `WS-AUTH-001-04B` | Internal review | `codex/ws-auth-001-04b-postgres-rate-controls` | - | 409 production lines; 93 owned tests and real API E2E pass; required reviewers pending. | +| `WS-AUTH-001-04B` | Repair | `codex/ws-auth-001-04b-postgres-rate-controls` | - | `62dd18e` failed required review; secret, Unicode, downgrade-lock, synchronization, and missing-proof repairs active. | | `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. | @@ -80,7 +81,7 @@ required internal review; AUTH-05 remains inactive. ## Blockers -No external blocker. AUTH-04B implementation review is active under its passed +No external blocker. AUTH-04B internal repair is active under its passed repaired L1 contract. It owns migration `0017`, following the now-owned `0016` prefix on current main. Non-test operators must later supply explicit classification evidence rather than diff --git a/backend/alembic/versions/0017_api_controls.py b/backend/alembic/versions/0017_api_controls.py index 35d5c00..a8f5c41 100644 --- a/backend/alembic/versions/0017_api_controls.py +++ b/backend/alembic/versions/0017_api_controls.py @@ -57,7 +57,9 @@ def upgrade() -> None: def downgrade() -> None: """Drop only an empty rate-control table.""" - has_rows = op.get_bind().execute( + bind = op.get_bind() + bind.execute(sa.text("lock table api_rate_control_counters in access exclusive mode")) + has_rows = bind.execute( sa.text("select exists(select 1 from api_rate_control_counters)") ).scalar_one() if has_rows: diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 2ee19be..8bfbaac 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -8,7 +8,7 @@ from pathlib import Path from typing import Literal -from pydantic import Field, SecretStr, field_validator, model_validator +from pydantic import Field, SecretStr, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict @@ -96,15 +96,11 @@ class Settings(BaseSettings): hide_input_in_errors=True, ) - @field_validator("api_rate_limit_key_secret") - @classmethod - def validate_api_rate_limit_key_secret( - cls, value: SecretStr | None - ) -> SecretStr | None: - """Require canonical bounded Base64 while retaining redacted storage.""" - if value is not None: - decode_api_rate_limit_key_secret(value) - return value + def __init__(self, **values: object) -> None: + """Validate secret material after Pydantic has discarded raw input.""" + super().__init__(**values) + if self.api_rate_limit_key_secret is not None: + decode_api_rate_limit_key_secret(self.api_rate_limit_key_secret) @model_validator(mode="after") def validate_artifact_storage(self) -> Settings: diff --git a/backend/app/modules/api_controls/service.py b/backend/app/modules/api_controls/service.py index 621d754..3bd35c1 100644 --- a/backend/app/modules/api_controls/service.py +++ b/backend/app/modules/api_controls/service.py @@ -50,8 +50,11 @@ def rate_key_digest( """Derive the exact privacy key for one verified external identity.""" if control_scope not in RATE_SCOPES: raise ValueError("unsupported rate control scope") - issuer_bytes = issuer.encode("utf-8") - subject_bytes = subject.encode("utf-8") + try: + issuer_bytes = issuer.encode("utf-8") + subject_bytes = subject.encode("utf-8") + except UnicodeEncodeError: + raise RateControlUnavailableError("rate control unavailable") from None if not 1 <= len(issuer_bytes) <= MAX_IDENTITY_BYTES or not 1 <= len( subject_bytes ) <= MAX_IDENTITY_BYTES: diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index b98b557..84091dc 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -1,8 +1,11 @@ from __future__ import annotations import asyncio +from concurrent.futures import ThreadPoolExecutor import json from pathlib import Path +import threading +import time from uuid import uuid4 import pytest @@ -301,6 +304,7 @@ def test_api_rate_control_schema_preserves_domain_and_guards_downgrade( config = Config(str(project_root / "alembic.ini")) config.set_main_option("script_location", str(project_root / "alembic")) project_id = str(uuid4()) + artifact_id = str(uuid4()) digest = bytes(range(32)) with migration_lock(): @@ -312,18 +316,53 @@ def test_api_rate_control_schema_preserves_domain_and_guards_downgrade( before = asyncio.run( _artifact_prior_head_project(isolated_database_env, project_id) ) + artifact_before = asyncio.run( + _seed_and_fetch_0016_artifact(isolated_database_env, artifact_id) + ) command.upgrade(config, "0017_api_controls") after = asyncio.run( _artifact_prior_head_project(isolated_database_env, project_id) ) + artifact_after = asyncio.run( + _fetch_0016_artifact(isolated_database_env, artifact_id) + ) schema = asyncio.run(_api_rate_control_schema(isolated_database_env)) asyncio.run(_assert_api_rate_control_guards(isolated_database_env, digest)) with pytest.raises(RuntimeError, match="non-empty API rate controls"): command.downgrade(config, "0016_artifact_domain") - refused_state = asyncio.run(_api_rate_control_state(isolated_database_env)) + asyncio.run(_clear_api_rate_controls(isolated_database_env)) + inserted = threading.Event() + release_insert = threading.Event() + downgrade_started = threading.Event() + + def guarded_downgrade() -> None: + downgrade_started.set() + command.downgrade(config, "0016_artifact_domain") + + with ThreadPoolExecutor(max_workers=2) as pool: + insert_future = pool.submit( + asyncio.run, + _insert_rate_control_until_released( + isolated_database_env, + digest, + inserted, + release_insert, + ), + ) + assert inserted.wait(timeout=5) + downgrade_future = pool.submit(guarded_downgrade) + assert downgrade_started.wait(timeout=5) + time.sleep(0.2) + assert downgrade_future.done() is False + release_insert.set() + insert_future.result(timeout=5) + with pytest.raises(RuntimeError, match="non-empty API rate controls"): + downgrade_future.result(timeout=5) + + refused_state = asyncio.run(_api_rate_control_state(isolated_database_env)) asyncio.run(_clear_api_rate_controls(isolated_database_env)) command.downgrade(config, "0016_artifact_domain") downgraded_state = asyncio.run( @@ -332,9 +371,11 @@ def test_api_rate_control_schema_preserves_domain_and_guards_downgrade( command.upgrade(config, "0017_api_controls") finally: asyncio.run(_clear_api_rate_controls(isolated_database_env)) + asyncio.run(_truncate_artifact_foundation(isolated_database_env)) command.downgrade(config, "base") assert after == before + assert artifact_after == artifact_before assert schema == { "columns": { "control_scope:character varying:NO", @@ -1826,6 +1867,73 @@ async def _api_rate_control_schema(database_url: str) -> dict[str, set[str]]: await engine.dispose() +async def _seed_and_fetch_0016_artifact( + database_url: str, artifact_id: str +) -> dict[str, object]: + """Seed one representative 0016 row and return its exact persisted value.""" + engine = create_async_engine(database_url) + try: + async with engine.begin() as connection: + await connection.execute( + text( + "insert into artifact_contents " + "(id, sha256, byte_count, media_type, normalized_display_name) " + "values (:id, :sha256, 17, 'text/plain', 'rate-migration.txt')" + ), + {"id": artifact_id, "sha256": "sha256:" + "7" * 64}, + ) + finally: + await engine.dispose() + return await _fetch_0016_artifact(database_url, artifact_id) + + +async def _fetch_0016_artifact( + database_url: str, artifact_id: str +) -> dict[str, object]: + """Fetch the representative 0016 artifact-domain row.""" + engine = create_async_engine(database_url) + try: + async with engine.begin() as connection: + row = ( + await connection.execute( + text( + "select id, sha256, byte_count, media_type, " + "normalized_display_name from artifact_contents where id = :id" + ), + {"id": artifact_id}, + ) + ).mappings().one() + return dict(row) + finally: + await engine.dispose() + + +async def _insert_rate_control_until_released( + database_url: str, + digest: bytes, + inserted: threading.Event, + release: threading.Event, +) -> None: + """Hold an uncommitted writer until the downgrade is waiting on its lock.""" + engine = create_async_engine(database_url) + try: + async with engine.begin() as connection: + await connection.execute( + text( + "insert into api_rate_control_counters " + "(control_scope, key_digest, window_started_at, window_expires_at, " + "request_count, updated_at) values " + "('first_access', :digest, statement_timestamp(), " + "statement_timestamp() + interval '1 minute', 1, statement_timestamp())" + ), + {"digest": digest}, + ) + inserted.set() + assert await asyncio.to_thread(release.wait, 5) + finally: + await engine.dispose() + + async def _assert_api_rate_control_guards(database_url: str, digest: bytes) -> None: """Insert one valid counter and reject every malformed direct variant.""" engine = create_async_engine(database_url) diff --git a/backend/tests/test_api_controls.py b/backend/tests/test_api_controls.py index 46dd5e8..f80697d 100644 --- a/backend/tests/test_api_controls.py +++ b/backend/tests/test_api_controls.py @@ -1,43 +1,22 @@ from __future__ import annotations import asyncio -from datetime import UTC, datetime, timedelta from hashlib import sha256 import logging from typing import Annotated from uuid import UUID, uuid4 -from fastapi import BackgroundTasks, Depends, HTTPException +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, SecretStr, field_validator +from pydantic import BaseModel, Field, field_validator import pytest -from sqlalchemy import text -from sqlalchemy.exc import SQLAlchemyError -from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from starlette.requests import Request -from app.api.deps.api_controls import ( - enforce_admin_mutation_rate_limit, - enforce_first_access_rate_limit, - get_rate_control_service, -) -from app.api.deps.auth import get_auth_verification_result from app.core.api_controls import ApiErrorResponse, error_response from app.core.config import Settings from app.main import create_app -from app.modules.api_controls import service as rate_service_module -from app.modules.api_controls.repository import ApiRateControlRepository, ConsumedCounter -from app.modules.api_controls.service import ( - ADMIN_MUTATION_SCOPE, - FIRST_ACCESS_SCOPE, - RateControlDecision, - RateControlService, - RateControlUnavailableError, - rate_key_digest, -) -from app.schemas.auth import AuthVerificationResult, VerifiedIssuerToken def _assert_uuid(value: str) -> None: @@ -495,519 +474,3 @@ def test_openapi_documents_request_error_and_response_context() -> None: 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 - - -RATE_SECRET_TEXT = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" -RATE_SECRET = SecretStr(RATE_SECRET_TEXT) -RATE_ISSUER = "https://issuer.example.test" -RATE_SUBJECT = "opaque-subject" - - -@pytest.fixture -async def rate_control_factory(postgres_database_url: str): - """Provide independent sessions over a clean, migrated rate-control table.""" - engine = create_async_engine(postgres_database_url) - factory = async_sessionmaker(engine, expire_on_commit=False) - async with engine.begin() as connection: - await connection.execute(text("delete from api_rate_control_counters")) - try: - yield factory - finally: - async with engine.begin() as connection: - await connection.execute(text("delete from api_rate_control_counters")) - await engine.dispose() - - -def test_rate_key_digest_matches_literal_vector_and_separates_boundaries() -> None: - expected = bytes.fromhex( - "35b4fb3e6a647a52596a5240b0b3ad5c2976d91771f95e2497be247081e53b31" - ) - - assert rate_key_digest( - RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, RATE_SUBJECT - ) == expected - assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "ab", "c") != ( - rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "a", "bc") - ) - assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "Issuer", "subject") != ( - rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "subject") - ) - assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "e\u0301") != ( - rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "\u00e9") - ) - assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "subject") != ( - rate_key_digest(RATE_SECRET, ADMIN_MUTATION_SCOPE, "issuer", "subject") - ) - - -@pytest.mark.parametrize("identity", ["", "x" * 4_097, "\u00e9" * 2_049]) -def test_rate_key_digest_rejects_unbounded_identity_without_echo(identity: str) -> None: - with pytest.raises(RateControlUnavailableError) as caught: - rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, identity, "subject") - assert str(caught.value) == "rate control unavailable" - - -async def _stored_rate_row(factory, scope: str, digest: bytes): - async with factory() as session: - return ( - await session.execute( - text( - "select window_started_at, window_expires_at, request_count " - "from api_rate_control_counters " - "where control_scope = :scope and key_digest = :digest" - ), - {"scope": scope, "digest": digest}, - ) - ).one() - - -async def test_rate_control_commits_fixed_window_denials_and_resets_expiry( - rate_control_factory, -) -> None: - service = RateControlService(rate_control_factory) - decisions = [ - await service.consume( - control_scope=FIRST_ACCESS_SCOPE, - issuer=RATE_ISSUER, - subject=RATE_SUBJECT, - limit=2, - window_seconds=60, - secret=RATE_SECRET, - ) - for _ in range(4) - ] - digest = rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, RATE_SUBJECT) - original = await _stored_rate_row(rate_control_factory, FIRST_ACCESS_SCOPE, digest) - - assert [decision.allowed for decision in decisions] == [True, True, False, False] - assert [decision.request_count for decision in decisions] == [1, 2, 3, 4] - assert all(1 <= decision.retry_after <= 60 for decision in decisions) - assert original.request_count == 4 - - async with rate_control_factory() as session: - await session.execute( - text( - "update api_rate_control_counters set " - "window_started_at = statement_timestamp() - interval '2 seconds', " - "window_expires_at = statement_timestamp() - interval '1 second' " - "where control_scope = :scope and key_digest = :digest" - ), - {"scope": FIRST_ACCESS_SCOPE, "digest": digest}, - ) - await session.commit() - - reset = await service.consume( - control_scope=FIRST_ACCESS_SCOPE, - issuer=RATE_ISSUER, - subject=RATE_SUBJECT, - limit=2, - window_seconds=60, - secret=RATE_SECRET, - ) - persisted = await _stored_rate_row(rate_control_factory, FIRST_ACCESS_SCOPE, digest) - assert reset == RateControlDecision(allowed=True, request_count=1, retry_after=60) - assert persisted.request_count == 1 - assert persisted.window_started_at > original.window_started_at - - -async def test_rate_control_concurrency_has_no_lost_or_rolled_back_consumption( - rate_control_factory, -) -> None: - service = RateControlService(rate_control_factory) - - async def consume_once(): - return await service.consume( - control_scope=ADMIN_MUTATION_SCOPE, - issuer=RATE_ISSUER, - subject="concurrent-subject", - limit=7, - window_seconds=30, - secret=RATE_SECRET, - ) - - decisions = await asyncio.gather(*(consume_once() for _ in range(20))) - digest = rate_key_digest( - RATE_SECRET, ADMIN_MUTATION_SCOPE, RATE_ISSUER, "concurrent-subject" - ) - persisted = await _stored_rate_row(rate_control_factory, ADMIN_MUTATION_SCOPE, digest) - - assert sum(decision.allowed for decision in decisions) == 7 - assert sorted(decision.request_count for decision in decisions) == list(range(1, 21)) - assert persisted.request_count == 20 - - async with rate_control_factory() as downstream: - await downstream.execute( - text( - "update api_rate_control_counters set request_count = 21 " - "where control_scope = :scope and key_digest = :digest" - ), - {"scope": ADMIN_MUTATION_SCOPE, "digest": digest}, - ) - await downstream.rollback() - assert ( - await _stored_rate_row(rate_control_factory, ADMIN_MUTATION_SCOPE, digest) - ).request_count == 20 - - -async def test_rate_control_saturates_bigint_and_prunes_only_bounded_other_rows( - rate_control_factory, -) -> None: - service = RateControlService(rate_control_factory) - digest = rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, "saturated") - async with rate_control_factory() as session: - await session.execute( - text( - "insert into api_rate_control_counters " - "(control_scope, key_digest, window_started_at, window_expires_at, " - "request_count, updated_at) values " - "(:scope, :digest, statement_timestamp(), " - "statement_timestamp() + interval '1 minute', 9223372036854775807, " - "statement_timestamp())" - ), - {"scope": FIRST_ACCESS_SCOPE, "digest": digest}, - ) - for value in range(125): - await session.execute( - text( - "insert into api_rate_control_counters " - "(control_scope, key_digest, window_started_at, window_expires_at, " - "request_count, updated_at) values " - "(:scope, :digest, statement_timestamp() - interval '2 minutes', " - "statement_timestamp() - interval '1 minute', 1, " - "statement_timestamp() - interval '1 minute')" - ), - { - "scope": ADMIN_MUTATION_SCOPE, - "digest": value.to_bytes(32, "big"), - }, - ) - await session.commit() - - decision = await service.consume( - control_scope=FIRST_ACCESS_SCOPE, - issuer=RATE_ISSUER, - subject="saturated", - limit=10_000, - window_seconds=60, - secret=RATE_SECRET, - ) - async with rate_control_factory() as session: - expired = await session.scalar( - text( - "select count(*) from api_rate_control_counters " - "where window_expires_at <= statement_timestamp()" - ) - ) - assert decision.request_count == 9_223_372_036_854_775_807 - assert decision.allowed is False - assert expired == 25 - - -async def test_concurrent_expired_keys_do_not_deadlock_during_pruning( - rate_control_factory, -) -> None: - subjects = ("expired-a", "expired-b") - async with rate_control_factory() as session: - for subject in subjects: - await session.execute( - text( - "insert into api_rate_control_counters " - "(control_scope, key_digest, window_started_at, window_expires_at, " - "request_count, updated_at) values " - "(:scope, :digest, statement_timestamp() - interval '2 seconds', " - "statement_timestamp() - interval '1 second', 5, " - "statement_timestamp() - interval '1 second')" - ), - { - "scope": FIRST_ACCESS_SCOPE, - "digest": rate_key_digest( - RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, subject - ), - }, - ) - await session.commit() - - service = RateControlService(rate_control_factory) - decisions = await asyncio.wait_for( - asyncio.gather( - *( - service.consume( - control_scope=FIRST_ACCESS_SCOPE, - issuer=RATE_ISSUER, - subject=subject, - limit=1, - window_seconds=60, - secret=RATE_SECRET, - ) - for subject in subjects - ) - ), - timeout=5, - ) - assert decisions == [ - RateControlDecision(allowed=True, request_count=1, retry_after=60), - RateControlDecision(allowed=True, request_count=1, retry_after=60), - ] - - -class _FakeSession: - """Minimal async-session context for deterministic failure tests.""" - - def __init__(self, *, commit_error: bool = False) -> None: - self.commit_error = commit_error - self.rolled_back = False - - async def __aenter__(self): - return self - - async def __aexit__(self, *_args) -> None: - return None - - async def commit(self) -> None: - if self.commit_error: - raise SQLAlchemyError("private commit detail") - - async def rollback(self) -> None: - self.rolled_back = True - - -async def test_rate_control_maps_only_database_failures_and_rolls_back( - monkeypatch: pytest.MonkeyPatch, -) -> None: - now = datetime.now(UTC) - session = _FakeSession() - - async def fail_execute(*_args, **_kwargs): - raise SQLAlchemyError("private execute detail") - - monkeypatch.setattr(ApiRateControlRepository, "consume", fail_execute) - with pytest.raises(RateControlUnavailableError, match="^rate control unavailable$"): - await RateControlService(lambda: session).consume( - control_scope=FIRST_ACCESS_SCOPE, - issuer=RATE_ISSUER, - subject=RATE_SUBJECT, - limit=1, - window_seconds=60, - secret=RATE_SECRET, - ) - assert session.rolled_back is True - - async def successful_consume(*_args, **_kwargs): - return ConsumedCounter(now, now, now + timedelta(seconds=60), 1) - - async def successful_prune(*_args, **_kwargs): - return None - - commit_session = _FakeSession(commit_error=True) - monkeypatch.setattr(ApiRateControlRepository, "consume", successful_consume) - monkeypatch.setattr(ApiRateControlRepository, "prune_expired", successful_prune) - with pytest.raises(RateControlUnavailableError, match="^rate control unavailable$"): - await RateControlService(lambda: commit_session).consume( - control_scope=FIRST_ACCESS_SCOPE, - issuer=RATE_ISSUER, - subject=RATE_SUBJECT, - limit=1, - window_seconds=60, - secret=RATE_SECRET, - ) - assert commit_session.rolled_back is True - - -async def test_rate_control_narrowly_maps_missing_database_and_propagates_cancel( - monkeypatch: pytest.MonkeyPatch, -) -> None: - def missing_database(): - raise RuntimeError("WORKSTREAM_DATABASE_URL must be set before database access") - - monkeypatch.setattr(rate_service_module, "get_session_factory", missing_database) - with pytest.raises(RateControlUnavailableError, match="^rate control unavailable$"): - await RateControlService().consume( - control_scope=FIRST_ACCESS_SCOPE, - issuer=RATE_ISSUER, - subject=RATE_SUBJECT, - limit=1, - window_seconds=60, - secret=RATE_SECRET, - ) - - def unrelated_failure(): - raise RuntimeError("unrelated runtime failure") - - monkeypatch.setattr(rate_service_module, "get_session_factory", unrelated_failure) - with pytest.raises(RuntimeError, match="unrelated runtime failure"): - await RateControlService().consume( - control_scope=FIRST_ACCESS_SCOPE, - issuer=RATE_ISSUER, - subject=RATE_SUBJECT, - limit=1, - window_seconds=60, - secret=RATE_SECRET, - ) - - async def cancel(*_args, **_kwargs): - raise asyncio.CancelledError - - monkeypatch.setattr(ApiRateControlRepository, "consume", cancel) - with pytest.raises(asyncio.CancelledError): - await RateControlService(lambda: _FakeSession()).consume( - control_scope=FIRST_ACCESS_SCOPE, - issuer=RATE_ISSUER, - subject=RATE_SUBJECT, - limit=1, - window_seconds=60, - secret=RATE_SECRET, - ) - - -def _verified_rate_identity(subject: str = RATE_SUBJECT) -> AuthVerificationResult: - return AuthVerificationResult( - token=VerifiedIssuerToken( - issuer=RATE_ISSUER, - subject=subject, - audience=("workstream",), - expires_at=2_000_000_000, - issued_at=1_999_999_000, - token_id="rate-token", - subject_kind="human", - scopes=frozenset({"workstream:access"}), - ) - ) - - -class _DecisionService: - def __init__(self, decisions: list[RateControlDecision] | None = None) -> None: - self.decisions = decisions or [] - self.calls: list[dict] = [] - - async def consume(self, **kwargs) -> RateControlDecision: - self.calls.append(kwargs) - if not self.decisions: - raise RateControlUnavailableError("private database detail") - return self.decisions.pop(0) - - -async def test_unattached_dependencies_emit_canonical_429_and_use_token_identity() -> None: - service = _DecisionService( - [ - RateControlDecision(True, 1, 60), - RateControlDecision(False, 31, 17), - ] - ) - app = create_app( - Settings(environment="test", api_rate_limit_key_secret=RATE_SECRET_TEXT) - ) - app.dependency_overrides[get_auth_verification_result] = _verified_rate_identity - app.dependency_overrides[get_rate_control_service] = lambda: service - - @app.post( - "/_test/first-access-rate", - dependencies=[Depends(enforce_first_access_rate_limit)], - ) - async def first_access() -> dict[str, bool]: - return {"allowed": True} - - @app.post( - "/_test/admin-mutation-rate", - dependencies=[Depends(enforce_admin_mutation_rate_limit)], - ) - async def admin_mutation() -> dict[str, bool]: - return {"allowed": True} - - async with AsyncClient( - transport=ASGITransport(app=app), base_url="http://testserver" - ) as client: - allowed = await client.post("/_test/first-access-rate") - denied = await client.post("/_test/admin-mutation-rate") - - assert allowed.status_code == 200 - assert denied.status_code == 429 - assert denied.headers["retry-after"] == "17" - assert denied.json() == { - "detail": "Rate limit exceeded", - "error": { - "code": "rate_limit_exceeded", - "message": "Rate limit exceeded", - "details": {}, - "correlation_id": denied.headers["x-correlation-id"], - "retryable": True, - }, - } - assert [call["control_scope"] for call in service.calls] == [ - FIRST_ACCESS_SCOPE, - ADMIN_MUTATION_SCOPE, - ] - assert all(call["issuer"] == RATE_ISSUER for call in service.calls) - assert all(call["subject"] == RATE_SUBJECT for call in service.calls) - - -@pytest.mark.parametrize( - ("settings", "subject", "service"), - [ - (Settings(environment="test"), RATE_SUBJECT, RateControlService()), - ( - Settings(environment="test", api_rate_limit_key_secret=RATE_SECRET_TEXT), - "x" * 4_097, - RateControlService(), - ), - ( - Settings(environment="test", api_rate_limit_key_secret=RATE_SECRET_TEXT), - RATE_SUBJECT, - _DecisionService(), - ), - ], -) -async def test_rate_dependency_unavailability_is_private_503( - settings: Settings, - subject: str, - service, -) -> None: - app = create_app(settings) - app.dependency_overrides[get_auth_verification_result] = lambda: _verified_rate_identity( - subject - ) - app.dependency_overrides[get_rate_control_service] = lambda: service - - @app.post( - "/_test/rate-unavailable", - dependencies=[Depends(enforce_first_access_rate_limit)], - ) - async def unavailable() -> None: - return None - - async with AsyncClient( - transport=ASGITransport(app=app), base_url="http://testserver" - ) as client: - response = await client.post("/_test/rate-unavailable") - - assert response.status_code == 503 - assert response.json() == { - "detail": "Service unavailable", - "error": { - "code": "service_unavailable", - "message": "Service unavailable", - "details": {}, - "correlation_id": response.headers["x-correlation-id"], - "retryable": True, - }, - } - assert subject not in response.text - - -def test_rate_dependencies_are_not_attached_to_production_routes() -> None: - forbidden = { - enforce_first_access_rate_limit, - enforce_admin_mutation_rate_limit, - } - - def dependency_calls(dependant) -> set: - calls = {dependant.call} - for dependency in dependant.dependencies: - calls.update(dependency_calls(dependency)) - return calls - - app = create_app() - assert all( - forbidden.isdisjoint(dependency_calls(route.dependant)) - for route in app.routes - if hasattr(route, "dependant") - ) diff --git a/backend/tests/test_api_rate_controls.py b/backend/tests/test_api_rate_controls.py new file mode 100644 index 0000000..d8f7655 --- /dev/null +++ b/backend/tests/test_api_rate_controls.py @@ -0,0 +1,737 @@ +from __future__ import annotations + +import asyncio +from datetime import UTC, datetime, timedelta + +from fastapi import Depends +from httpx import ASGITransport, AsyncClient +from pydantic import SecretStr +import pytest +from sqlalchemy import text +from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine + +from app.api.deps.api_controls import ( + enforce_admin_mutation_rate_limit, + enforce_first_access_rate_limit, + get_rate_control_service, +) +from app.api.deps.auth import get_auth_verification_result +from app.core.config import Settings +from app.main import create_app +from app.modules.api_controls import service as rate_service_module +from app.modules.api_controls.repository import ApiRateControlRepository, ConsumedCounter +from app.modules.api_controls.service import ( + ADMIN_MUTATION_SCOPE, + FIRST_ACCESS_SCOPE, + RateControlDecision, + RateControlService, + RateControlUnavailableError, + rate_key_digest, +) +from app.schemas.auth import AuthVerificationResult, VerifiedIssuerToken + +RATE_SECRET_TEXT = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" +RATE_SECRET = SecretStr(RATE_SECRET_TEXT) +RATE_ISSUER = "https://issuer.example.test" +RATE_SUBJECT = "opaque-subject" + + +@pytest.fixture +async def rate_control_factory(postgres_database_url: str): + """Provide independent sessions over a clean, migrated rate-control table.""" + engine = create_async_engine(postgres_database_url) + factory = async_sessionmaker(engine, expire_on_commit=False) + async with engine.begin() as connection: + await connection.execute(text("delete from api_rate_control_counters")) + try: + yield factory + finally: + async with engine.begin() as connection: + await connection.execute(text("delete from api_rate_control_counters")) + await engine.dispose() + + +def test_rate_key_digest_matches_literal_vector_and_separates_boundaries() -> None: + expected = bytes.fromhex( + "35b4fb3e6a647a52596a5240b0b3ad5c2976d91771f95e2497be247081e53b31" + ) + + assert rate_key_digest( + RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, RATE_SUBJECT + ) == expected + assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "ab", "c") != ( + rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "a", "bc") + ) + assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "Issuer", "subject") != ( + rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "subject") + ) + assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "e\u0301") != ( + rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "\u00e9") + ) + assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "subject") != ( + rate_key_digest(RATE_SECRET, ADMIN_MUTATION_SCOPE, "issuer", "subject") + ) + + +@pytest.mark.parametrize( + ("issuer", "subject"), + [ + ("", "subject"), + ("x" * 4_097, "subject"), + ("\u00e9" * 2_049, "subject"), + ("\ud800", "subject"), + ("issuer", "\ud800"), + ], +) +def test_rate_key_digest_rejects_unbounded_identity_without_echo( + issuer: str, subject: str +) -> None: + with pytest.raises(RateControlUnavailableError) as caught: + rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, issuer, subject) + assert str(caught.value) == "rate control unavailable" + + +async def _stored_rate_row(factory, scope: str, digest: bytes): + async with factory() as session: + return ( + await session.execute( + text( + "select window_started_at, window_expires_at, request_count " + "from api_rate_control_counters " + "where control_scope = :scope and key_digest = :digest" + ), + {"scope": scope, "digest": digest}, + ) + ).one() + + +async def test_rate_control_commits_fixed_window_denials_and_resets_expiry( + rate_control_factory, +) -> None: + service = RateControlService(rate_control_factory) + decisions = [ + await service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=2, + window_seconds=60, + secret=RATE_SECRET, + ) + for _ in range(4) + ] + digest = rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, RATE_SUBJECT) + original = await _stored_rate_row(rate_control_factory, FIRST_ACCESS_SCOPE, digest) + + assert [decision.allowed for decision in decisions] == [True, True, False, False] + assert [decision.request_count for decision in decisions] == [1, 2, 3, 4] + assert all(1 <= decision.retry_after <= 60 for decision in decisions) + assert original.request_count == 4 + + async with rate_control_factory() as session: + await session.execute( + text( + "update api_rate_control_counters set " + "window_started_at = statement_timestamp() - interval '2 seconds', " + "window_expires_at = statement_timestamp() - interval '1 second' " + "where control_scope = :scope and key_digest = :digest" + ), + {"scope": FIRST_ACCESS_SCOPE, "digest": digest}, + ) + await session.commit() + + reset = await service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=2, + window_seconds=60, + secret=RATE_SECRET, + ) + persisted = await _stored_rate_row(rate_control_factory, FIRST_ACCESS_SCOPE, digest) + assert reset == RateControlDecision(allowed=True, request_count=1, retry_after=60) + assert persisted.request_count == 1 + assert persisted.window_started_at > original.window_started_at + + other_scope = await service.consume( + control_scope=ADMIN_MUTATION_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=2, + window_seconds=60, + secret=RATE_SECRET, + ) + other_subject = await service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject="distinct-subject", + limit=2, + window_seconds=60, + secret=RATE_SECRET, + ) + assert other_scope.request_count == other_subject.request_count == 1 + assert ( + await _stored_rate_row(rate_control_factory, FIRST_ACCESS_SCOPE, digest) + ).request_count == 1 + + +async def test_repository_persists_the_returned_database_timestamp( + rate_control_factory, +) -> None: + digest = bytes([17]) * 32 + async with rate_control_factory() as session: + consumed = await ApiRateControlRepository(session).consume( + FIRST_ACCESS_SCOPE, digest, 60 + ) + updated_at = await session.scalar( + text( + "select updated_at from api_rate_control_counters " + "where control_scope = :scope and key_digest = :digest" + ), + {"scope": FIRST_ACCESS_SCOPE, "digest": digest}, + ) + await session.rollback() + assert updated_at == consumed.db_now + + +async def test_rate_control_concurrency_has_no_lost_or_rolled_back_consumption( + rate_control_factory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + original_consume = ApiRateControlRepository.consume + all_started = asyncio.Event() + started = 0 + + async def synchronized_consume(repository, *args, **kwargs): + nonlocal started + started += 1 + if started == 20: + all_started.set() + await asyncio.wait_for(all_started.wait(), timeout=5) + return await original_consume(repository, *args, **kwargs) + + monkeypatch.setattr(ApiRateControlRepository, "consume", synchronized_consume) + service = RateControlService(rate_control_factory) + + async def consume_once(): + return await service.consume( + control_scope=ADMIN_MUTATION_SCOPE, + issuer=RATE_ISSUER, + subject="concurrent-subject", + limit=7, + window_seconds=30, + secret=RATE_SECRET, + ) + + decisions = await asyncio.gather(*(consume_once() for _ in range(20))) + digest = rate_key_digest( + RATE_SECRET, ADMIN_MUTATION_SCOPE, RATE_ISSUER, "concurrent-subject" + ) + persisted = await _stored_rate_row(rate_control_factory, ADMIN_MUTATION_SCOPE, digest) + + assert sum(decision.allowed for decision in decisions) == 7 + assert sorted(decision.request_count for decision in decisions) == list(range(1, 21)) + assert persisted.request_count == 20 + + probe_digest = bytes([255]) * 32 + independent_subject = "rollback-independent" + independent_digest = rate_key_digest( + RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, independent_subject + ) + async with rate_control_factory() as outer: + await outer.execute( + text( + "insert into api_rate_control_counters " + "(control_scope, key_digest, window_started_at, window_expires_at, " + "request_count, updated_at) values " + "(:scope, :digest, statement_timestamp(), " + "statement_timestamp() + interval '1 minute', 1, statement_timestamp())" + ), + {"scope": FIRST_ACCESS_SCOPE, "digest": probe_digest}, + ) + independent = await service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=independent_subject, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + await outer.rollback() + async with rate_control_factory() as session: + probe_count = await session.scalar( + text( + "select count(*) from api_rate_control_counters " + "where control_scope = :scope and key_digest = :digest" + ), + {"scope": FIRST_ACCESS_SCOPE, "digest": probe_digest}, + ) + assert independent.allowed is True + assert probe_count == 0 + assert ( + await _stored_rate_row(rate_control_factory, FIRST_ACCESS_SCOPE, independent_digest) + ).request_count == 1 + + +async def test_rate_control_saturates_bigint_and_prunes_only_bounded_other_rows( + rate_control_factory, +) -> None: + service = RateControlService(rate_control_factory) + digest = rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, "saturated") + async with rate_control_factory() as session: + await session.execute( + text( + "insert into api_rate_control_counters " + "(control_scope, key_digest, window_started_at, window_expires_at, " + "request_count, updated_at) values " + "(:scope, :digest, statement_timestamp(), " + "statement_timestamp() + interval '1 minute', 9223372036854775807, " + "statement_timestamp())" + ), + {"scope": FIRST_ACCESS_SCOPE, "digest": digest}, + ) + for value in range(125): + await session.execute( + text( + "insert into api_rate_control_counters " + "(control_scope, key_digest, window_started_at, window_expires_at, " + "request_count, updated_at) values " + "(:scope, :digest, statement_timestamp() - interval '2 minutes', " + "statement_timestamp() - interval '1 minute', 1, " + "statement_timestamp() - interval '1 minute')" + ), + { + "scope": ADMIN_MUTATION_SCOPE, + "digest": value.to_bytes(32, "big"), + }, + ) + await session.commit() + + decision = await service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject="saturated", + limit=10_000, + window_seconds=60, + secret=RATE_SECRET, + ) + async with rate_control_factory() as session: + expired = await session.scalar( + text( + "select count(*) from api_rate_control_counters " + "where window_expires_at <= statement_timestamp()" + ) + ) + assert decision.request_count == 9_223_372_036_854_775_807 + assert decision.allowed is False + assert expired == 25 + + +async def test_concurrent_expired_keys_do_not_deadlock_during_pruning( + rate_control_factory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + subjects = ("expired-a", "expired-b") + async with rate_control_factory() as session: + for subject in subjects: + await session.execute( + text( + "insert into api_rate_control_counters " + "(control_scope, key_digest, window_started_at, window_expires_at, " + "request_count, updated_at) values " + "(:scope, :digest, statement_timestamp() - interval '2 seconds', " + "statement_timestamp() - interval '1 second', 5, " + "statement_timestamp() - interval '1 second')" + ), + { + "scope": FIRST_ACCESS_SCOPE, + "digest": rate_key_digest( + RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, subject + ), + }, + ) + await session.commit() + + original_prune = ApiRateControlRepository.prune_expired + both_upserted = asyncio.Event() + upserted = 0 + + async def synchronized_prune(repository, *args, **kwargs): + nonlocal upserted + upserted += 1 + if upserted == 2: + both_upserted.set() + await asyncio.wait_for(both_upserted.wait(), timeout=5) + return await original_prune(repository, *args, **kwargs) + + monkeypatch.setattr(ApiRateControlRepository, "prune_expired", synchronized_prune) + service = RateControlService(rate_control_factory) + decisions = await asyncio.wait_for( + asyncio.gather( + *( + service.consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=subject, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + for subject in subjects + ) + ), + timeout=5, + ) + assert decisions == [ + RateControlDecision(allowed=True, request_count=1, retry_after=60), + RateControlDecision(allowed=True, request_count=1, retry_after=60), + ] + + +class _FakeSession: + """Minimal async-session context for deterministic failure tests.""" + + def __init__( + self, *, commit_error: bool = False, enter_error: bool = False + ) -> None: + self.commit_error = commit_error + self.enter_error = enter_error + self.rolled_back = False + + async def __aenter__(self): + if self.enter_error: + raise SQLAlchemyError("private session-open detail") + return self + + async def __aexit__(self, *_args) -> None: + return None + + async def commit(self) -> None: + if self.commit_error: + raise SQLAlchemyError("private commit detail") + + async def rollback(self) -> None: + self.rolled_back = True + + +async def test_rate_control_maps_only_database_failures_and_rolls_back( + monkeypatch: pytest.MonkeyPatch, +) -> None: + now = datetime.now(UTC) + with pytest.raises(RateControlUnavailableError, match="^rate control unavailable$"): + await RateControlService(lambda: _FakeSession(enter_error=True)).consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + + session = _FakeSession() + + async def fail_execute(*_args, **_kwargs): + raise SQLAlchemyError("private execute detail") + + monkeypatch.setattr(ApiRateControlRepository, "consume", fail_execute) + with pytest.raises(RateControlUnavailableError, match="^rate control unavailable$"): + await RateControlService(lambda: session).consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + assert session.rolled_back is True + + async def successful_consume(*_args, **_kwargs): + return ConsumedCounter(now, now, now + timedelta(seconds=60), 1) + + async def successful_prune(*_args, **_kwargs): + return None + + async def fail_prune(*_args, **_kwargs): + raise SQLAlchemyError("private prune detail") + + prune_session = _FakeSession() + monkeypatch.setattr(ApiRateControlRepository, "consume", successful_consume) + monkeypatch.setattr(ApiRateControlRepository, "prune_expired", fail_prune) + with pytest.raises(RateControlUnavailableError, match="^rate control unavailable$"): + await RateControlService(lambda: prune_session).consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + assert prune_session.rolled_back is True + + commit_session = _FakeSession(commit_error=True) + monkeypatch.setattr(ApiRateControlRepository, "consume", successful_consume) + monkeypatch.setattr(ApiRateControlRepository, "prune_expired", successful_prune) + with pytest.raises(RateControlUnavailableError, match="^rate control unavailable$"): + await RateControlService(lambda: commit_session).consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + assert commit_session.rolled_back is True + + +async def test_retry_after_uses_only_returned_database_clock( + monkeypatch: pytest.MonkeyPatch, +) -> None: + database_now = datetime(2000, 1, 1, tzinfo=UTC) + + async def subsecond(*_args, **_kwargs): + return ConsumedCounter( + database_now, + database_now, + database_now + timedelta(milliseconds=200), + 2, + ) + + async def no_prune(*_args, **_kwargs): + return None + + monkeypatch.setattr(ApiRateControlRepository, "consume", subsecond) + monkeypatch.setattr(ApiRateControlRepository, "prune_expired", no_prune) + decision = await RateControlService(lambda: _FakeSession()).consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + assert decision.retry_after == 1 + + async def beyond_window(*_args, **_kwargs): + return ConsumedCounter( + database_now, + database_now, + database_now + timedelta(seconds=120), + 2, + ) + + monkeypatch.setattr(ApiRateControlRepository, "consume", beyond_window) + decision = await RateControlService(lambda: _FakeSession()).consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + assert decision.retry_after == 60 + + +async def test_rate_control_narrowly_maps_missing_database_and_propagates_cancel( + monkeypatch: pytest.MonkeyPatch, +) -> None: + def missing_database(): + raise RuntimeError("WORKSTREAM_DATABASE_URL must be set before database access") + + monkeypatch.setattr(rate_service_module, "get_session_factory", missing_database) + with pytest.raises(RateControlUnavailableError, match="^rate control unavailable$"): + await RateControlService().consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + + def unrelated_failure(): + raise RuntimeError("unrelated runtime failure") + + monkeypatch.setattr(rate_service_module, "get_session_factory", unrelated_failure) + with pytest.raises(RuntimeError, match="unrelated runtime failure"): + await RateControlService().consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + + async def cancel(*_args, **_kwargs): + raise asyncio.CancelledError + + monkeypatch.setattr(ApiRateControlRepository, "consume", cancel) + with pytest.raises(asyncio.CancelledError): + await RateControlService(lambda: _FakeSession()).consume( + control_scope=FIRST_ACCESS_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=1, + window_seconds=60, + secret=RATE_SECRET, + ) + + +def _verified_rate_identity(subject: str = RATE_SUBJECT) -> AuthVerificationResult: + return AuthVerificationResult( + token=VerifiedIssuerToken( + issuer=RATE_ISSUER, + subject=subject, + audience=("workstream",), + expires_at=2_000_000_000, + issued_at=1_999_999_000, + token_id="rate-token", + subject_kind="human", + scopes=frozenset({"workstream:access"}), + ) + ) + + +class _DecisionService: + def __init__(self, decisions: list[RateControlDecision] | None = None) -> None: + self.decisions = decisions or [] + self.calls: list[dict] = [] + + async def consume(self, **kwargs) -> RateControlDecision: + self.calls.append(kwargs) + if not self.decisions: + raise RateControlUnavailableError("private database detail") + return self.decisions.pop(0) + + +async def test_unattached_dependencies_emit_canonical_429_and_use_token_identity() -> None: + service = _DecisionService( + [ + RateControlDecision(True, 1, 60), + RateControlDecision(False, 31, 17), + ] + ) + app = create_app( + Settings(environment="test", api_rate_limit_key_secret=RATE_SECRET_TEXT) + ) + app.dependency_overrides[get_auth_verification_result] = _verified_rate_identity + app.dependency_overrides[get_rate_control_service] = lambda: service + + @app.post( + "/_test/first-access-rate", + dependencies=[Depends(enforce_first_access_rate_limit)], + ) + async def first_access() -> dict[str, bool]: + return {"allowed": True} + + @app.post( + "/_test/admin-mutation-rate", + dependencies=[Depends(enforce_admin_mutation_rate_limit)], + ) + async def admin_mutation() -> dict[str, bool]: + return {"allowed": True} + + async with AsyncClient( + transport=ASGITransport(app=app), base_url="http://testserver" + ) as client: + allowed = await client.post("/_test/first-access-rate") + denied = await client.post("/_test/admin-mutation-rate") + + assert allowed.status_code == 200 + assert denied.status_code == 429 + assert denied.headers["retry-after"] == "17" + assert denied.json() == { + "detail": "Rate limit exceeded", + "error": { + "code": "rate_limit_exceeded", + "message": "Rate limit exceeded", + "details": {}, + "correlation_id": denied.headers["x-correlation-id"], + "retryable": True, + }, + } + assert [call["control_scope"] for call in service.calls] == [ + FIRST_ACCESS_SCOPE, + ADMIN_MUTATION_SCOPE, + ] + assert all(call["issuer"] == RATE_ISSUER for call in service.calls) + assert all(call["subject"] == RATE_SUBJECT for call in service.calls) + + +@pytest.mark.parametrize( + ("settings", "subject", "service"), + [ + (Settings(environment="test"), RATE_SUBJECT, RateControlService()), + ( + Settings(environment="test", api_rate_limit_key_secret=RATE_SECRET_TEXT), + "x" * 4_097, + RateControlService(), + ), + ( + Settings(environment="test", api_rate_limit_key_secret=RATE_SECRET_TEXT), + "\ud800", + RateControlService(), + ), + ( + Settings(environment="test", api_rate_limit_key_secret=RATE_SECRET_TEXT), + RATE_SUBJECT, + _DecisionService(), + ), + ], +) +async def test_rate_dependency_unavailability_is_private_503( + settings: Settings, + subject: str, + service, +) -> None: + app = create_app(settings) + app.dependency_overrides[get_auth_verification_result] = lambda: _verified_rate_identity( + subject + ) + app.dependency_overrides[get_rate_control_service] = lambda: service + + @app.post( + "/_test/rate-unavailable", + dependencies=[Depends(enforce_first_access_rate_limit)], + ) + async def unavailable() -> None: + return None + + async with AsyncClient( + transport=ASGITransport(app=app), base_url="http://testserver" + ) as client: + response = await client.post("/_test/rate-unavailable") + + assert response.status_code == 503 + assert response.json() == { + "detail": "Service unavailable", + "error": { + "code": "service_unavailable", + "message": "Service unavailable", + "details": {}, + "correlation_id": response.headers["x-correlation-id"], + "retryable": True, + }, + } + assert subject not in response.text + + +def test_rate_dependencies_are_not_attached_to_production_routes() -> None: + forbidden = { + enforce_first_access_rate_limit, + enforce_admin_mutation_rate_limit, + } + + def dependency_calls(dependant) -> set: + calls = {dependant.call} + for dependency in dependant.dependencies: + calls.update(dependency_calls(dependency)) + return calls + + app = create_app() + assert all( + forbidden.isdisjoint(dependency_calls(route.dependant)) + for route in app.routes + if hasattr(route, "dependant") + ) diff --git a/backend/tests/test_config.py b/backend/tests/test_config.py index 1262733..d52f09c 100644 --- a/backend/tests/test_config.py +++ b/backend/tests/test_config.py @@ -54,10 +54,12 @@ def test_rate_limit_secret_is_canonical_and_redacted() -> None: ], ) def test_rate_limit_secret_rejects_invalid_values_without_echo(value: str) -> None: - with pytest.raises(ValidationError) as caught: + with pytest.raises(ValueError, match="^invalid API rate limit key secret$") as caught: Settings(api_rate_limit_key_secret=value) - rendered = str(caught.value) - assert "input_value" not in rendered + assert not isinstance(caught.value, ValidationError) + rendered = f"{caught.value!s} {caught.value!r}" + assert not hasattr(caught.value, "errors") + assert not hasattr(caught.value, "json") if value.strip(): assert value not in rendered diff --git a/docs/operations_authorization_service.md b/docs/operations_authorization_service.md index 060ab8c..3b9915c 100644 --- a/docs/operations_authorization_service.md +++ b/docs/operations_authorization_service.md @@ -189,7 +189,9 @@ WHERE window_expires_at <= statement_timestamp(); Runtime consumption opportunistically deletes at most 100 expired other rows. On idle systems, operators may run the same expired-only SQL cleanup. Never delete active rows to recover capacity, and never downgrade migration `0017` -while the table is nonempty; the guarded downgrade refuses and rolls back. +while the table is nonempty; the guarded downgrade takes an exclusive table +lock, refuses, and rolls back. Quiesce every protected write before attempting +the downgrade so waiting writers cannot resume against a removed table. ## JWKS Rotation And Outage From 8f54fd97f809ab5621409d990f224d8e723ba82f Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 02:14:07 +0100 Subject: [PATCH 07/18] docs(auth): bind artifact authorization ownership --- .agent-loop/LOOP_STATE.md | 7 +++-- .agent-loop/REVIEW_LOG.md | 7 +++++ .agent-loop/WORK_QUEUE.md | 2 +- .../CHUNK_MAP.md | 5 ++++ .../DECISIONS.md | 25 +++++++++++++++++ .../STATUS.md | 5 ++-- backend/tests/test_api_rate_controls.py | 28 +++++++++++++++++++ 7 files changed, 72 insertions(+), 7 deletions(-) diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index 3ad3d33..75f736a 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -9,12 +9,13 @@ - Worktree: `/home/abiorh/flow/workstream-auth-001-04b` - Status: AUTH-04A post-merge memory merged through PR #112 as `7749f54`; the user explicitly started AUTH-04B. Exact-head review rejected candidate - `62dd18e`; one bounded repair cycle is active. + `62dd18e`; repair head `2d70581` plus artifact-authorization intent is being + bound to final evidence before exact-head re-review. - Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836` - Prior `WS-AUTH-001-01` final merged branch head: `b5217e1` - Latest integrated `main` merge commit: `7749f54` -- Current gate: deterministic repair evidence, then exact-head re-review; no PR - until every valid finding is resolved. +- Current gate: final focused coverage and exact-head re-review; no PR until + every valid finding is resolved. - Size checkpoint: implementation is 409 changed non-comment production lines; the required 350-line inspection passed and scope is frozen below the 500 stop. - Next chunk: none; do not start `WS-AUTH-001-05` automatically. diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index f2a9147..c6340de 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -11,6 +11,13 @@ AUTH-04A file instead of the contract-owned rate-control file. One bounded repair cycle moves the tests, closes each runtime/migration issue, adds exact proof, and requires full-suite coverage plus exact-head re-review. +The repository-wide coverage run for repair head `2d70581` was interrupted by +the host shutdown on 2026-07-14 and produced no valid result. Under the current +repository rule and the chunk's laptop-capacity clause, local evidence must +prove the materially changed AUTH-04B subsystem remains at least 90 percent; +GitHub CI owns the unchanged repository-wide 78 percent gate. No interrupted +result is treated as evidence. + Required senior engineering, architecture, security/data, QA/test, CI-integrity, test-delta, product/ops, docs, and reuse review passed exact repaired-contract head `b5dceb1`. The second repair closed optional-secret, missing-database, diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index 9c40aac..ac7d093 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -4,7 +4,7 @@ | Chunk | Title | Risk | Status | |---|---|---:|---| -| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Candidate `62dd18e` failed internal review; bounded repair/evidence active | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Bounded repair behavior passed; final coverage and exact-head re-review pending | ## 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 ef1a058..0346b73 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 @@ -68,6 +68,11 @@ WS-AUTH-001-PLAN - Chunk 07 provides the single authorization engine before grant APIs. - Chunks 08-10 establish local grant truth before product cutover. - Chunks 11-15 migrate bounded complete product/system surfaces. +- Artifact upload, read, retention, release/delete, replication, integrity, and + reconciliation remain mechanically owned by the artifact subsystem but must + receive centralized AUTH decisions. Chunk 07 owns the permission-registry + boundary, chunk 09 owns artifact service principals, the applicable 11-15 + resource cutovers attach enforcement, and chunk 16 proves no bypass remains. - Chunk 16 proves the complete initiative; it does not backfill missing audit or idempotency evidence. - `WS-POL-002-03` merged separately through PR #90 as `a7aa474`. This initiative 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 5a0e414..9decb31 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 @@ -187,3 +187,28 @@ 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. + +## D14: Artifact operations use centralized authorization + +Status: accepted by the user on 2026-07-14. + +The artifact subsystem owns upload/storage, retention, replication, integrity, +and reconciliation mechanics behind the shared object-storage adapter +foundation. It does not own or infer product authority. Every human-initiated +or internal-service artifact operation must be authorized by Workstream's one +central authorization kernel using an operation-specific permission and the +exact project/resource scope before an adapter performs external I/O. + +Human callers resolve through canonical `ActorProfile`, identity-link status, +and the applicable administrative or exact-project contributor grant. Artifact +storage, retention, and reconciliation services authenticate as explicitly +provisioned service principals with registered system permissions; they are not +Contributors and never receive fabricated human roles. Upload, read, retain, +release/delete, replicate, verify, and reconcile authority are distinct and do +not imply one another. + +The authorization decision and operation receipt must share bounded request/ +correlation evidence, resource identity, operation, and service principal. +Receipts prove that storage work occurred; they do not create authority. Exact +permission tokens and route ownership are locked in the owning AUTH cutover +chunk contracts. AUTH-04B adds no artifact permission or route attachment. 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 15daae9..72801c2 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 @@ -39,8 +39,7 @@ explicitly started AUTH-04B; its required L1 preimplementation review is active and rejected the first activated contract before runtime edits. The second repaired contract passed all required tracks at `b5dceb1`; bounded runtime implementation and deterministic evidence are complete, and the candidate is -in one bounded repair cycle after exact-head internal review rejected -`62dd18e`. +in final repair evidence after exact-head internal review rejected `62dd18e`. ## Active planning chunk @@ -65,7 +64,7 @@ required internal review; AUTH-05 remains inactive. | `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` | Merged | `codex/ws-auth-001-04-request-api-controls` | #111 | Merged as `90c9a28`; production review `cdcaf77`; final branch head `36c4aa5`. | -| `WS-AUTH-001-04B` | Repair | `codex/ws-auth-001-04b-postgres-rate-controls` | - | `62dd18e` failed required review; secret, Unicode, downgrade-lock, synchronization, and missing-proof repairs active. | +| `WS-AUTH-001-04B` | Re-review pending | `codex/ws-auth-001-04b-postgres-rate-controls` | - | Repair behavior and migration proofs pass; final subsystem coverage and exact-head reviewers pending. | | `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. | diff --git a/backend/tests/test_api_rate_controls.py b/backend/tests/test_api_rate_controls.py index d8f7655..ff64c5c 100644 --- a/backend/tests/test_api_rate_controls.py +++ b/backend/tests/test_api_rate_controls.py @@ -20,6 +20,7 @@ from app.core.config import Settings from app.main import create_app from app.modules.api_controls import service as rate_service_module +from app.modules.api_controls.models import ApiRateControlCounter from app.modules.api_controls.repository import ApiRateControlRepository, ConsumedCounter from app.modules.api_controls.service import ( ADMIN_MUTATION_SCOPE, @@ -37,6 +38,33 @@ RATE_SUBJECT = "opaque-subject" +def test_rate_control_orm_model_matches_the_migration_contract() -> None: + table = ApiRateControlCounter.__table__ + + assert list(table.columns) == [ + table.c.control_scope, + table.c.key_digest, + table.c.window_started_at, + table.c.window_expires_at, + table.c.request_count, + table.c.updated_at, + ] + assert {column.name for column in table.primary_key.columns} == { + "control_scope", + "key_digest", + } + assert {constraint.name for constraint in table.constraints} == { + "pk_api_rate_control_counters", + "ck_api_rate_control_counters_scope_token", + "ck_api_rate_control_counters_digest_length", + "ck_api_rate_control_counters_request_count", + "ck_api_rate_control_counters_window_order", + } + assert {index.name for index in table.indexes} == { + "ix_api_rate_control_counters_window_expires_at" + } + + @pytest.fixture async def rate_control_factory(postgres_database_url: str): """Provide independent sessions over a clean, migrated rate-control table.""" From 0b9d0fe2b71bbf55bdae1f634320f07b22ebe808 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 02:59:18 +0100 Subject: [PATCH 08/18] fix(auth): prevent rate secret retention --- .agent-loop/REVIEW_LOG.md | 26 ++++++ .../PLAN.md | 7 +- ...001-02-flow-node-adapter-reconciliation.md | 19 +++- .../WS-AUTH-001-04B-postgres-rate-controls.md | 12 ++- backend/app/core/config.py | 76 +++++++++++++-- backend/tests/test_config.py | 93 +++++++++++++++++++ 6 files changed, 217 insertions(+), 16 deletions(-) diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index c6340de..ddf4c01 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -1,5 +1,31 @@ # Review Log +## 2026-07-14 - WS-AUTH-001-04B Exact-Head Review Repair + +Required implementation review at `8f54fd9` passed QA, CI integrity, and test +delta with 72 fresh isolated PostgreSQL tests. Senior engineering and security +rejected PR publication because unrelated Pydantic validation errors could +retain the raw rate-control secret through structured error APIs. Security also +found that the canonical inactive artifact plan did not yet enforce D14's +central authorization prerequisite for adapter I/O. + +The bounded repair removes the secret from Pydantic's public field graph before +all structured validation while preserving constructor, environment, and dotenv +loading. Regression tests inspect `errors()` and `json()` and cover +`model_validate`. The user-directed artifact ownership clarification amends only +the canonical ART plan and inactive chunk contract: mechanics may be developed +independently, but production dispatch requires AUTH-07 permissions, AUTH-09 +service principals, exact resource authorization evidence, and never derives +authority from a credential or receipt. No artifact runtime or AUTH-05 behavior +is activated. + +Repair evidence is 78 migration-inclusive isolated PostgreSQL behavior tests, +69 focused rate-control/config tests at 98 percent subsystem coverage, and the +real API contract E2E. Ruff, docstring coverage, stale authorization/artifact/ +Workstream wording, Markdown links, loop memory, diff integrity, and all 36 +agent-gate tests pass. The repaired production delta is 437 non-comment lines, +below the 500-line hard stop. Exact-SHA internal re-review is the current gate. + ## 2026-07-13 - WS-AUTH-001-04B Repaired Contract Passed Implementation candidate `62dd18e` failed required exact-head review. Valid diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/PLAN.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/PLAN.md index 56dd8fa..4091961 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/PLAN.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/PLAN.md @@ -261,7 +261,12 @@ approval, locking, and deterministic checker compilation. ## WS-AUTH Dependencies -- Internal artifact foundations and Flow Node work may proceed independently. +- Internal artifact schemas and adapter mechanics may proceed independently, + but production dispatch and every external adapter I/O remain disabled until + `WS-AUTH-001-07` defines the exact operation permission and + `WS-AUTH-001-09` provisions the calling service principal. Each dispatch must + carry a central authorization decision for the exact operation and resource; + an adapter credential or provider receipt never creates authority. - Guide source API cutover waits for WS-AUTH project mutation cutover (`WS-AUTH-001-12`) or its approved replacement. - Upload session and submission cutovers wait for task/submission/checker diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-02-flow-node-adapter-reconciliation.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-02-flow-node-adapter-reconciliation.md index ffb0026..52f7187 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-02-flow-node-adapter-reconciliation.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-02-flow-node-adapter-reconciliation.md @@ -5,7 +5,9 @@ Parent: `WS-ART-001` | Repository: Workstream | Risk: L1 | SLA: P1 ## Goal Implement `FlowNodeAdapter`, metadata-only operation outbox/reconciliation, -pinned provider delivery, and local integration. +pinned provider delivery, and local integration. Production dispatch remains +disabled until the central AUTH permission and service-principal prerequisites +are merged. ## Allowed Files @@ -20,7 +22,8 @@ pinned provider delivery, and local integration. ## Not Allowed No byte payload in DB/Redis/outbox, product cutover, provider fallback, human -token forwarding, or provider-triggered lifecycle transition. +token forwarding, provider-triggered lifecycle transition, or adapter I/O that +bypasses a central authorization decision for the exact operation and resource. ## Acceptance Criteria @@ -38,6 +41,11 @@ token forwarding, or provider-triggered lifecycle transition. visibility. - Reconciliation, quarantine, retain, and release product audit events use the shared `AuditRepository`; provider receipts remain operation evidence only. +- Production upload, read, retain, release/delete, replicate, verify, and + reconcile dispatch requires the exact permission from `WS-AUTH-001-07` and an + explicitly provisioned service principal from `WS-AUTH-001-09`. Mechanical + adapter work may be tested before those chunks merge, but production dispatch + stays disabled and no receipt or adapter credential creates authority. - Concurrent dispatch/reconcile has one monotonic provider effect and one canonical receipt outcome; row counts are asserted. - Same v1 vectors pass LocalStorageAdapter and a Flow Node image pinned by full @@ -48,9 +56,10 @@ token forwarding, or provider-triggered lifecycle transition. populated-state preservation, empty downgrade, and re-upgrade are owned and proved in `test_alembic.py`; otherwise the chunk records that no migration was created. -- Compose health proves adapter-level authenticated ingest/retrieve/status from - Workstream's service principal directly through `FlowNodeAdapter`; it does not - claim a public Workstream artifact API before the product cutover. +- Compose health proves test-only adapter mechanics for ingest/retrieve/status. + Production service-principal dispatch additionally requires central AUTH + decision evidence and remains disabled before the owning AUTH cutovers; this + chunk does not claim a public Workstream artifact API. ## Verification 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 index 4d2fd2d..148d32e 100644 --- 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 @@ -45,6 +45,8 @@ backend/tests/test_alembic.py backend/scripts/api_contract_e2e.py docs/operations_authorization_service.md docs/architecture_data_model.md +.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/PLAN.md +.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-02-flow-node-adapter-reconciliation.md .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/** .agent-loop/LOOP_STATE.md .agent-loop/WORK_QUEUE.md @@ -124,10 +126,14 @@ AUTH-05 or later chunk implementation tests that do not calculate expected values through the production helper. - 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: SecretStr | None = None`; there is no non-null - fallback and no generated secret. Present +- Expose `api_rate_limit_key_secret` as `SecretStr | None`, backed by private + settings storage after constructor, environment, or dotenv input is removed + from Pydantic's structured validation graph. There is no non-null fallback + and no generated secret. Present malformed, non-ASCII, noncanonical, whitespace-bearing, too-short, or - too-long values fail settings construction. + too-long values fail settings construction. Unrelated Pydantic failures and + the alternate `model_validate` entry point must not retain the raw key in + structured error APIs. - Exact numeric settings are `api_first_access_rate_limit=10`, `api_first_access_rate_window_seconds=60`, `api_admin_mutation_rate_limit=30`, and diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 8bfbaac..0d05c51 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -4,12 +4,20 @@ import base64 import binascii +import os +from collections.abc import Mapping from functools import lru_cache from pathlib import Path -from typing import Literal +from typing import Literal, Self -from pydantic import Field, SecretStr, model_validator -from pydantic_settings import BaseSettings, SettingsConfigDict +from dotenv import dotenv_values +from pydantic import Field, PrivateAttr, SecretStr, model_validator +from pydantic_settings import ( + BaseSettings, + DotEnvSettingsSource, + PydanticBaseSettingsSource, + SettingsConfigDict, +) class Settings(BaseSettings): @@ -78,7 +86,6 @@ class Settings(BaseSettings): celery_result_backend_url: str | None = None celery_task_always_eager: bool = False actor_registry_refresh_interval_seconds: int = Field(default=300, ge=0, le=86_400) - api_rate_limit_key_secret: SecretStr | None = None api_first_access_rate_limit: int = Field(default=10, ge=1, le=10_000) api_first_access_rate_window_seconds: int = Field(default=60, ge=1, le=3_600) api_admin_mutation_rate_limit: int = Field(default=30, ge=1, le=10_000) @@ -96,11 +103,42 @@ class Settings(BaseSettings): hide_input_in_errors=True, ) + _api_rate_limit_key_secret: SecretStr | None = PrivateAttr(default=None) + def __init__(self, **values: object) -> None: - """Validate secret material after Pydantic has discarded raw input.""" + """Remove rate-control secret material before structured validation.""" + secret = _extract_api_rate_limit_key_secret(values) super().__init__(**values) - if self.api_rate_limit_key_secret is not None: - decode_api_rate_limit_key_secret(self.api_rate_limit_key_secret) + self._api_rate_limit_key_secret = secret + + @classmethod + def model_validate(cls, obj: object, **kwargs: object) -> Self: + """Sanitize secret-bearing mappings before Pydantic retains input.""" + if isinstance(obj, Mapping) and "api_rate_limit_key_secret" in obj: + sanitized = dict(obj) + secret = _extract_api_rate_limit_key_secret(sanitized) + sanitized["api_rate_limit_key_secret"] = secret + return super().model_validate(sanitized, **kwargs) + return super().model_validate(obj, **kwargs) + + @classmethod + def settings_customise_sources( + cls, + settings_cls: type[BaseSettings], + init_settings: PydanticBaseSettingsSource, + env_settings: PydanticBaseSettingsSource, + dotenv_settings: PydanticBaseSettingsSource, + file_secret_settings: PydanticBaseSettingsSource, + ) -> tuple[PydanticBaseSettingsSource, ...]: + """Keep the manually resolved rate key out of dotenv validation input.""" + if isinstance(dotenv_settings, DotEnvSettingsSource): + dotenv_settings.env_vars.pop("workstream_api_rate_limit_key_secret", None) + return init_settings, env_settings, dotenv_settings, file_secret_settings + + @property + def api_rate_limit_key_secret(self) -> SecretStr | None: + """Return the validated, redacted rate-control key setting.""" + return self._api_rate_limit_key_secret @model_validator(mode="after") def validate_artifact_storage(self) -> Settings: @@ -136,6 +174,30 @@ def get_settings() -> Settings: return Settings() +def _extract_api_rate_limit_key_secret(values: dict[str, object]) -> SecretStr | None: + """Resolve and validate the key without entering Pydantic's input graph.""" + missing = object() + raw_value = values.pop("api_rate_limit_key_secret", missing) + if raw_value is missing: + raw_value = os.environ.get("WORKSTREAM_API_RATE_LIMIT_KEY_SECRET", missing) + if raw_value is missing: + env_file = values.get("_env_file", ".env") + if env_file is not None: + env_encoding = values.get("_env_file_encoding", "utf-8") + dotenv = dotenv_values(env_file, encoding=env_encoding) + raw_value = dotenv.get("WORKSTREAM_API_RATE_LIMIT_KEY_SECRET", missing) + if raw_value is missing or raw_value is None: + return None + if isinstance(raw_value, SecretStr): + secret = raw_value + elif isinstance(raw_value, str): + secret = SecretStr(raw_value) + else: + raise ValueError("invalid API rate limit key secret") + decode_api_rate_limit_key_secret(secret) + return secret + + def decode_api_rate_limit_key_secret(value: SecretStr) -> bytes: """Decode one canonical padded Base64 rate-control key.""" secret = value.get_secret_value() diff --git a/backend/tests/test_config.py b/backend/tests/test_config.py index d52f09c..c24372d 100644 --- a/backend/tests/test_config.py +++ b/backend/tests/test_config.py @@ -1,6 +1,7 @@ from __future__ import annotations import base64 +from pathlib import Path import pytest from pydantic import ValidationError @@ -17,6 +18,7 @@ def test_default_settings_are_fail_closed(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("WORKSTREAM_DATABASE_URL", raising=False) monkeypatch.delenv("WORKSTREAM_DEV_AUTH_TOKEN", raising=False) + monkeypatch.delenv("WORKSTREAM_API_RATE_LIMIT_KEY_SECRET", raising=False) settings = Settings() @@ -41,6 +43,97 @@ def test_rate_limit_secret_is_canonical_and_redacted() -> None: assert encoded not in repr(settings) +def test_rate_limit_secret_loads_from_environment_and_dotenv( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + env_encoded = base64.b64encode(bytes(range(32))).decode("ascii") + dotenv_encoded = base64.b64encode(bytes(range(1, 33))).decode("ascii") + (tmp_path / ".env").write_text( + f"WORKSTREAM_API_RATE_LIMIT_KEY_SECRET={dotenv_encoded}\n", + encoding="utf-8", + ) + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("WORKSTREAM_API_RATE_LIMIT_KEY_SECRET", env_encoded) + + env_settings = Settings() + monkeypatch.delenv("WORKSTREAM_API_RATE_LIMIT_KEY_SECRET") + dotenv_settings = Settings() + + assert env_settings.api_rate_limit_key_secret is not None + assert env_settings.api_rate_limit_key_secret.get_secret_value() == env_encoded + assert dotenv_settings.api_rate_limit_key_secret is not None + assert dotenv_settings.api_rate_limit_key_secret.get_secret_value() == dotenv_encoded + + +def test_rate_limit_secret_is_absent_from_unrelated_structured_errors() -> None: + encoded = base64.b64encode(bytes(range(32))).decode("ascii") + + with pytest.raises(ValidationError) as caught: + Settings( + api_rate_limit_key_secret=encoded, + artifact_store_backend="flow_node", + ) + + assert encoded not in repr(caught.value.errors()) + assert encoded not in caught.value.json() + + +def test_environment_rate_limit_secret_is_absent_from_unrelated_structured_errors( + monkeypatch: pytest.MonkeyPatch, +) -> None: + encoded = base64.b64encode(bytes(range(32))).decode("ascii") + monkeypatch.setenv("WORKSTREAM_API_RATE_LIMIT_KEY_SECRET", encoded) + + with pytest.raises(ValidationError) as caught: + Settings(artifact_store_backend="flow_node") + + assert encoded not in repr(caught.value.errors()) + assert encoded not in caught.value.json() + + +def test_dotenv_rate_limit_secret_is_absent_from_unrelated_structured_errors( + tmp_path: Path, +) -> None: + encoded = base64.b64encode(bytes(range(32))).decode("ascii") + env_file = tmp_path / ".env" + env_file.write_text( + f"WORKSTREAM_API_RATE_LIMIT_KEY_SECRET={encoded}\n", + encoding="utf-8", + ) + + with pytest.raises(ValidationError) as caught: + Settings(_env_file=env_file, artifact_store_backend="flow_node") + + assert encoded not in repr(caught.value.errors()) + assert encoded not in caught.value.json() + + +def test_model_validate_rejects_rate_limit_secret_without_structured_echo() -> None: + invalid = "not-a-canonical-secret" + + with pytest.raises(ValueError, match="^invalid API rate limit key secret$") as caught: + Settings.model_validate({"api_rate_limit_key_secret": invalid}) + + assert not isinstance(caught.value, ValidationError) + assert invalid not in f"{caught.value!s} {caught.value!r}" + + +def test_model_validate_rate_limit_secret_is_absent_from_unrelated_errors() -> None: + encoded = base64.b64encode(bytes(range(32))).decode("ascii") + + with pytest.raises(ValidationError) as caught: + Settings.model_validate( + { + "api_rate_limit_key_secret": encoded, + "artifact_store_backend": "flow_node", + } + ) + + assert encoded not in repr(caught.value.errors()) + assert encoded not in caught.value.json() + + @pytest.mark.parametrize( "value", [ From 629d10c49a3e026252ab15cb716d8ac986fe6b55 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 03:12:13 +0100 Subject: [PATCH 09/18] fix(auth): sanitize alternate settings validation --- .agent-loop/REVIEW_LOG.md | 19 ++++- .../WS-AUTH-001-04B-postgres-rate-controls.md | 7 +- backend/app/core/config.py | 36 ++++++++- backend/tests/test_config.py | 77 +++++++++++++++++++ 4 files changed, 133 insertions(+), 6 deletions(-) diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index ddf4c01..3c3be73 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -19,13 +19,28 @@ service principals, exact resource authorization evidence, and never derives authority from a credential or receipt. No artifact runtime or AUTH-05 behavior is activated. -Repair evidence is 78 migration-inclusive isolated PostgreSQL behavior tests, +Initial repair evidence is 78 migration-inclusive isolated PostgreSQL behavior tests, 69 focused rate-control/config tests at 98 percent subsystem coverage, and the real API contract E2E. Ruff, docstring coverage, stale authorization/artifact/ Workstream wording, Markdown links, loop memory, diff integrity, and all 36 -agent-gate tests pass. The repaired production delta is 437 non-comment lines, +agent-gate tests pass. The repaired production delta is 465 non-comment lines, below the 500-line hard stop. Exact-SHA internal re-review is the current gate. +Exact-head review of `0b9d0fe` passed QA, CI integrity, and test delta, but +senior engineering and security found the same retention class in +`model_validate_json` and `model_validate_strings`; malformed JSON could also +retain its complete document. Senior engineering additionally found that the +manual secret source did not preserve BaseSettings' supported layered-dotenv +precedence. The final bounded repair covers those public entry points, rejects +malformed JSON with a constant non-retaining error, and resolves layered dotenv +files in order. Exact-SHA re-review remains mandatory before publication. + +Final repair evidence adds 77 focused behavior tests at 98 percent subsystem +coverage, 56 direct configuration tests, and the exact AUTH-04B migration test +passing against a fresh isolated database. The production delta is 465 +non-comment lines and remains below the 500-line hard stop. All static gates +continue to pass. + ## 2026-07-13 - WS-AUTH-001-04B Repaired Contract Passed Implementation candidate `62dd18e` failed required exact-head review. Valid 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 index 148d32e..5add7f2 100644 --- 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 @@ -132,8 +132,11 @@ AUTH-05 or later chunk implementation and no generated secret. Present malformed, non-ASCII, noncanonical, whitespace-bearing, too-short, or too-long values fail settings construction. Unrelated Pydantic failures and - the alternate `model_validate` entry point must not retain the raw key in - structured error APIs. + alternate `model_validate`, `model_validate_json`, and + `model_validate_strings` entry points must not retain the raw key in + structured error APIs. Malformed settings JSON fails with a constant error + that does not retain the supplied document. Layered dotenv files preserve + BaseSettings' later-file precedence. - Exact numeric settings are `api_first_access_rate_limit=10`, `api_first_access_rate_window_seconds=60`, `api_admin_mutation_rate_limit=30`, and diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 0d05c51..24cedc2 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -4,6 +4,7 @@ import base64 import binascii +import json import os from collections.abc import Mapping from functools import lru_cache @@ -121,6 +122,31 @@ def model_validate(cls, obj: object, **kwargs: object) -> Self: return super().model_validate(sanitized, **kwargs) return super().model_validate(obj, **kwargs) + @classmethod + def model_validate_json( + cls, + json_data: str | bytes | bytearray, + **kwargs: object, + ) -> Self: + """Sanitize JSON secret input before Pydantic retains the document.""" + try: + parsed = json.loads(json_data) + except (json.JSONDecodeError, UnicodeDecodeError): + raise ValueError("invalid settings JSON") from None + if isinstance(parsed, Mapping) and "api_rate_limit_key_secret" in parsed: + return cls.model_validate(parsed, **kwargs) + return super().model_validate_json(json_data, **kwargs) + + @classmethod + def model_validate_strings(cls, obj: object, **kwargs: object) -> Self: + """Sanitize string-mapping secret input before structured validation.""" + if isinstance(obj, Mapping) and "api_rate_limit_key_secret" in obj: + sanitized = dict(obj) + secret = _extract_api_rate_limit_key_secret(sanitized) + sanitized["api_rate_limit_key_secret"] = secret + return super().model_validate_strings(sanitized, **kwargs) + return super().model_validate_strings(obj, **kwargs) + @classmethod def settings_customise_sources( cls, @@ -184,8 +210,14 @@ def _extract_api_rate_limit_key_secret(values: dict[str, object]) -> SecretStr | env_file = values.get("_env_file", ".env") if env_file is not None: env_encoding = values.get("_env_file_encoding", "utf-8") - dotenv = dotenv_values(env_file, encoding=env_encoding) - raw_value = dotenv.get("WORKSTREAM_API_RATE_LIMIT_KEY_SECRET", missing) + env_files = ( + (env_file,) + if isinstance(env_file, (str, os.PathLike)) + else tuple(env_file) + ) + for path in env_files: + dotenv = dotenv_values(path, encoding=env_encoding) + raw_value = dotenv.get("WORKSTREAM_API_RATE_LIMIT_KEY_SECRET", raw_value) if raw_value is missing or raw_value is None: return None if isinstance(raw_value, SecretStr): diff --git a/backend/tests/test_config.py b/backend/tests/test_config.py index c24372d..5d1a1f1 100644 --- a/backend/tests/test_config.py +++ b/backend/tests/test_config.py @@ -1,6 +1,7 @@ from __future__ import annotations import base64 +import json from pathlib import Path import pytest @@ -66,6 +67,26 @@ def test_rate_limit_secret_loads_from_environment_and_dotenv( assert dotenv_settings.api_rate_limit_key_secret.get_secret_value() == dotenv_encoded +def test_rate_limit_secret_loads_from_layered_dotenv_files(tmp_path: Path) -> None: + first_encoded = base64.b64encode(bytes(range(32))).decode("ascii") + second_encoded = base64.b64encode(bytes(range(1, 33))).decode("ascii") + first = tmp_path / "first.env" + second = tmp_path / "second.env" + first.write_text( + f"WORKSTREAM_API_RATE_LIMIT_KEY_SECRET={first_encoded}\n", + encoding="utf-8", + ) + second.write_text( + f"WORKSTREAM_API_RATE_LIMIT_KEY_SECRET={second_encoded}\n", + encoding="utf-8", + ) + + settings = Settings(_env_file=(first, second)) + + assert settings.api_rate_limit_key_secret is not None + assert settings.api_rate_limit_key_secret.get_secret_value() == second_encoded + + def test_rate_limit_secret_is_absent_from_unrelated_structured_errors() -> None: encoded = base64.b64encode(bytes(range(32))).decode("ascii") @@ -134,6 +155,62 @@ def test_model_validate_rate_limit_secret_is_absent_from_unrelated_errors() -> N assert encoded not in caught.value.json() +@pytest.mark.parametrize("method_name", ["model_validate_json", "model_validate_strings"]) +def test_alternate_validation_loads_rate_limit_secret(method_name: str) -> None: + encoded = base64.b64encode(bytes(range(32))).decode("ascii") + payload = {"api_rate_limit_key_secret": encoded} + method = getattr(Settings, method_name) + + settings = method(json.dumps(payload) if method_name.endswith("json") else payload) + + assert settings.api_rate_limit_key_secret is not None + assert settings.api_rate_limit_key_secret.get_secret_value() == encoded + + +@pytest.mark.parametrize("method_name", ["model_validate_json", "model_validate_strings"]) +def test_alternate_validation_rejects_rate_limit_secret_without_echo( + method_name: str, +) -> None: + invalid = "not-a-canonical-secret" + payload = {"api_rate_limit_key_secret": invalid} + method = getattr(Settings, method_name) + + with pytest.raises(ValueError, match="^invalid API rate limit key secret$") as caught: + method(json.dumps(payload) if method_name.endswith("json") else payload) + + assert not isinstance(caught.value, ValidationError) + assert invalid not in f"{caught.value!s} {caught.value!r}" + + +@pytest.mark.parametrize("method_name", ["model_validate_json", "model_validate_strings"]) +def test_alternate_validation_secret_is_absent_from_unrelated_errors( + method_name: str, +) -> None: + encoded = base64.b64encode(bytes(range(32))).decode("ascii") + payload = { + "api_rate_limit_key_secret": encoded, + "artifact_store_backend": "flow_node", + } + method = getattr(Settings, method_name) + + with pytest.raises(ValidationError) as caught: + method(json.dumps(payload) if method_name.endswith("json") else payload) + + assert encoded not in repr(caught.value.errors()) + assert encoded not in caught.value.json() + + +def test_model_validate_json_rejects_malformed_document_without_echo() -> None: + invalid = "not-a-canonical-secret" + payload = f'{{"api_rate_limit_key_secret":"{invalid}"' + + with pytest.raises(ValueError, match="^invalid settings JSON$") as caught: + Settings.model_validate_json(payload) + + assert not isinstance(caught.value, ValidationError) + assert invalid not in f"{caught.value!s} {caught.value!r}" + + @pytest.mark.parametrize( "value", [ From 67484b508637b7fc893e441e7afb5f5b34dd7715 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 03:23:48 +0100 Subject: [PATCH 10/18] fix(auth): eliminate secret exception retention --- .agent-loop/REVIEW_LOG.md | 16 +++++++- .../WS-AUTH-001-04B-postgres-rate-controls.md | 7 ++-- backend/app/core/config.py | 29 ++++++++++---- backend/tests/test_config.py | 40 ++++++++++++++++++- 4 files changed, 80 insertions(+), 12 deletions(-) diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 3c3be73..b1bc3e7 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -37,10 +37,24 @@ files in order. Exact-SHA re-review remains mandatory before publication. Final repair evidence adds 77 focused behavior tests at 98 percent subsystem coverage, 56 direct configuration tests, and the exact AUTH-04B migration test -passing against a fresh isolated database. The production delta is 465 +passing against a fresh isolated database. The production delta is 480 non-comment lines and remains below the 500-line hard stop. All static gates continue to pass. +Exact-head review of `629d10c` found that rendered redaction was still weaker +than object-graph non-retention: Pydantic-version-dependent error input could +retain a recoverable `SecretStr`, malformed JSON remained reachable through +`JSONDecodeError.__context__`, and non-ASCII rejection chained a +`UnicodeEncodeError` containing the original key. The final privacy repair +passes only `None` into Pydantic, assigns the private validated key after +successful validation, and raises constant parse/decode failures outside catch +scopes. Tests traverse structured errors and complete exception cause/context +graphs rather than checking rendered output alone. + +The object-graph repair passes 77 focused behavior tests at 97 percent +subsystem coverage. The final production delta is 480 non-comment lines, below +the 500-line hard stop. + ## 2026-07-13 - WS-AUTH-001-04B Repaired Contract Passed Implementation candidate `62dd18e` failed required exact-head review. Valid 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 index 5add7f2..ae26dc9 100644 --- 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 @@ -134,9 +134,10 @@ AUTH-05 or later chunk implementation too-long values fail settings construction. Unrelated Pydantic failures and alternate `model_validate`, `model_validate_json`, and `model_validate_strings` entry points must not retain the raw key in - structured error APIs. Malformed settings JSON fails with a constant error - that does not retain the supplied document. Layered dotenv files preserve - BaseSettings' later-file precedence. + structured error APIs, recoverable `SecretStr` objects, or exception cause/ + context graphs. Malformed settings JSON fails with a constant error that does + not retain the supplied document anywhere in its exception graph. Layered + dotenv files preserve BaseSettings' later-file precedence. - Exact numeric settings are `api_first_access_rate_limit=10`, `api_first_access_rate_window_seconds=60`, `api_admin_mutation_rate_limit=30`, and diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 24cedc2..da44220 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -118,8 +118,10 @@ def model_validate(cls, obj: object, **kwargs: object) -> Self: if isinstance(obj, Mapping) and "api_rate_limit_key_secret" in obj: sanitized = dict(obj) secret = _extract_api_rate_limit_key_secret(sanitized) - sanitized["api_rate_limit_key_secret"] = secret - return super().model_validate(sanitized, **kwargs) + sanitized["api_rate_limit_key_secret"] = None + settings = super().model_validate(sanitized, **kwargs) + settings._api_rate_limit_key_secret = secret + return settings return super().model_validate(obj, **kwargs) @classmethod @@ -129,10 +131,14 @@ def model_validate_json( **kwargs: object, ) -> Self: """Sanitize JSON secret input before Pydantic retains the document.""" + malformed = False try: parsed = json.loads(json_data) except (json.JSONDecodeError, UnicodeDecodeError): - raise ValueError("invalid settings JSON") from None + malformed = True + parsed = None + if malformed: + raise ValueError("invalid settings JSON") if isinstance(parsed, Mapping) and "api_rate_limit_key_secret" in parsed: return cls.model_validate(parsed, **kwargs) return super().model_validate_json(json_data, **kwargs) @@ -143,8 +149,10 @@ def model_validate_strings(cls, obj: object, **kwargs: object) -> Self: if isinstance(obj, Mapping) and "api_rate_limit_key_secret" in obj: sanitized = dict(obj) secret = _extract_api_rate_limit_key_secret(sanitized) - sanitized["api_rate_limit_key_secret"] = secret - return super().model_validate_strings(sanitized, **kwargs) + sanitized["api_rate_limit_key_secret"] = None + settings = super().model_validate_strings(sanitized, **kwargs) + settings._api_rate_limit_key_secret = secret + return settings return super().model_validate_strings(obj, **kwargs) @classmethod @@ -233,11 +241,18 @@ def _extract_api_rate_limit_key_secret(values: dict[str, object]) -> SecretStr | def decode_api_rate_limit_key_secret(value: SecretStr) -> bytes: """Decode one canonical padded Base64 rate-control key.""" secret = value.get_secret_value() + if not secret.isascii(): + raise ValueError("invalid API rate limit key secret") + invalid = False try: encoded = secret.encode("ascii") decoded = base64.b64decode(encoded, validate=True) - except (UnicodeEncodeError, binascii.Error, ValueError) as exc: - raise ValueError("invalid API rate limit key secret") from exc + except (binascii.Error, ValueError): + invalid = True + encoded = b"" + decoded = b"" + if invalid: + raise ValueError("invalid API rate limit key secret") if not 32 <= len(decoded) <= 64 or base64.b64encode(decoded) != encoded: raise ValueError("invalid API rate limit key secret") return decoded diff --git a/backend/tests/test_config.py b/backend/tests/test_config.py index 5d1a1f1..ef7cb97 100644 --- a/backend/tests/test_config.py +++ b/backend/tests/test_config.py @@ -2,10 +2,11 @@ import base64 import json +from collections.abc import Mapping from pathlib import Path import pytest -from pydantic import ValidationError +from pydantic import SecretStr, ValidationError from app.adapters.artifacts import resolve_artifact_store from app.adapters.auth.flow import FlowAuthVerifier @@ -16,6 +17,31 @@ from app.main import create_app +def _assert_secret_not_retained(value: object, secret: str, seen: set[int] | None = None) -> None: + """Assert a secret is unreachable through an error's public object graph.""" + if seen is None: + seen = set() + if id(value) in seen: + return + seen.add(id(value)) + if isinstance(value, str): + assert secret not in value + elif isinstance(value, SecretStr): + assert value.get_secret_value() != secret + elif isinstance(value, BaseException): + _assert_secret_not_retained(value.args, secret, seen) + _assert_secret_not_retained(vars(value), secret, seen) + _assert_secret_not_retained(value.__cause__, secret, seen) + _assert_secret_not_retained(value.__context__, secret, seen) + elif isinstance(value, Mapping): + for key, item in value.items(): + _assert_secret_not_retained(key, secret, seen) + _assert_secret_not_retained(item, secret, seen) + elif isinstance(value, (list, tuple, set)): + for item in value: + _assert_secret_not_retained(item, secret, seen) + + def test_default_settings_are_fail_closed(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("WORKSTREAM_DATABASE_URL", raising=False) monkeypatch.delenv("WORKSTREAM_DEV_AUTH_TOKEN", raising=False) @@ -42,6 +68,7 @@ def test_rate_limit_secret_is_canonical_and_redacted() -> None: assert settings.api_rate_limit_key_secret is not None assert settings.api_rate_limit_key_secret.get_secret_value() == encoded assert encoded not in repr(settings) + assert "api_rate_limit_key_secret" not in settings.model_dump() def test_rate_limit_secret_loads_from_environment_and_dotenv( @@ -98,6 +125,7 @@ def test_rate_limit_secret_is_absent_from_unrelated_structured_errors() -> None: assert encoded not in repr(caught.value.errors()) assert encoded not in caught.value.json() + _assert_secret_not_retained(caught.value, encoded) def test_environment_rate_limit_secret_is_absent_from_unrelated_structured_errors( @@ -111,6 +139,7 @@ def test_environment_rate_limit_secret_is_absent_from_unrelated_structured_error assert encoded not in repr(caught.value.errors()) assert encoded not in caught.value.json() + _assert_secret_not_retained(caught.value, encoded) def test_dotenv_rate_limit_secret_is_absent_from_unrelated_structured_errors( @@ -128,6 +157,7 @@ def test_dotenv_rate_limit_secret_is_absent_from_unrelated_structured_errors( assert encoded not in repr(caught.value.errors()) assert encoded not in caught.value.json() + _assert_secret_not_retained(caught.value, encoded) def test_model_validate_rejects_rate_limit_secret_without_structured_echo() -> None: @@ -138,6 +168,7 @@ def test_model_validate_rejects_rate_limit_secret_without_structured_echo() -> N assert not isinstance(caught.value, ValidationError) assert invalid not in f"{caught.value!s} {caught.value!r}" + _assert_secret_not_retained(caught.value, invalid) def test_model_validate_rate_limit_secret_is_absent_from_unrelated_errors() -> None: @@ -153,6 +184,7 @@ def test_model_validate_rate_limit_secret_is_absent_from_unrelated_errors() -> N assert encoded not in repr(caught.value.errors()) assert encoded not in caught.value.json() + _assert_secret_not_retained(caught.value, encoded) @pytest.mark.parametrize("method_name", ["model_validate_json", "model_validate_strings"]) @@ -180,6 +212,7 @@ def test_alternate_validation_rejects_rate_limit_secret_without_echo( assert not isinstance(caught.value, ValidationError) assert invalid not in f"{caught.value!s} {caught.value!r}" + _assert_secret_not_retained(caught.value, invalid) @pytest.mark.parametrize("method_name", ["model_validate_json", "model_validate_strings"]) @@ -198,6 +231,7 @@ def test_alternate_validation_secret_is_absent_from_unrelated_errors( assert encoded not in repr(caught.value.errors()) assert encoded not in caught.value.json() + _assert_secret_not_retained(caught.value, encoded) def test_model_validate_json_rejects_malformed_document_without_echo() -> None: @@ -209,6 +243,9 @@ def test_model_validate_json_rejects_malformed_document_without_echo() -> None: assert not isinstance(caught.value, ValidationError) assert invalid not in f"{caught.value!s} {caught.value!r}" + assert caught.value.__cause__ is None + assert caught.value.__context__ is None + _assert_secret_not_retained(caught.value, invalid) @pytest.mark.parametrize( @@ -232,6 +269,7 @@ def test_rate_limit_secret_rejects_invalid_values_without_echo(value: str) -> No assert not hasattr(caught.value, "json") if value.strip(): assert value not in rendered + _assert_secret_not_retained(caught.value, value) @pytest.mark.parametrize( From 922778b8f48c0ccd74299797dc684f904c10601d Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 03:27:35 +0100 Subject: [PATCH 11/18] test(auth): prove Pydantic secret boundary --- .agent-loop/REVIEW_LOG.md | 8 +++++++ backend/tests/test_config.py | 42 ++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index b1bc3e7..331c6f2 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -55,6 +55,14 @@ The object-graph repair passes 77 focused behavior tests at 97 percent subsystem coverage. The final production delta is 480 non-comment lines, below the 500-line hard stop. +QA passed runtime and CI integrity at `67484b5` but rejected the test proof: +the recursive helper did not traverse `ValidationError.errors()`, so rendered +redaction could hide a recoverable `SecretStr` on another Pydantic version. The +test-only repair now traverses the actual structured error objects and +instruments the `BaseSettings` boundary for mapping, JSON, and string-mapping +validation, asserting that only `None` crosses into Pydantic. This +deterministically fails the prior implementation that forwarded `SecretStr`. + ## 2026-07-13 - WS-AUTH-001-04B Repaired Contract Passed Implementation candidate `62dd18e` failed required exact-head review. Valid diff --git a/backend/tests/test_config.py b/backend/tests/test_config.py index ef7cb97..7c96972 100644 --- a/backend/tests/test_config.py +++ b/backend/tests/test_config.py @@ -7,6 +7,7 @@ import pytest from pydantic import SecretStr, ValidationError +from pydantic_settings import BaseSettings from app.adapters.artifacts import resolve_artifact_store from app.adapters.auth.flow import FlowAuthVerifier @@ -29,6 +30,8 @@ def _assert_secret_not_retained(value: object, secret: str, seen: set[int] | Non elif isinstance(value, SecretStr): assert value.get_secret_value() != secret elif isinstance(value, BaseException): + if isinstance(value, ValidationError): + _assert_secret_not_retained(value.errors(), secret, seen) _assert_secret_not_retained(value.args, secret, seen) _assert_secret_not_retained(vars(value), secret, seen) _assert_secret_not_retained(value.__cause__, secret, seen) @@ -187,6 +190,45 @@ def test_model_validate_rate_limit_secret_is_absent_from_unrelated_errors() -> N _assert_secret_not_retained(caught.value, encoded) +@pytest.mark.parametrize( + ("method_name", "base_method_name"), + [ + ("model_validate", "model_validate"), + ("model_validate_json", "model_validate"), + ("model_validate_strings", "model_validate_strings"), + ], +) +def test_alternate_validation_never_passes_secret_into_pydantic( + monkeypatch: pytest.MonkeyPatch, + method_name: str, + base_method_name: str, +) -> None: + encoded = base64.b64encode(bytes(range(32))).decode("ascii") + observed: dict[str, object] = {} + + class PydanticBoundaryReached(Exception): + pass + + def capture_input( + cls: type[BaseSettings], + obj: object, + **kwargs: object, + ) -> BaseSettings: + observed["input"] = obj + raise PydanticBoundaryReached + + monkeypatch.setattr(BaseSettings, base_method_name, classmethod(capture_input)) + payload = {"api_rate_limit_key_secret": encoded} + method = getattr(Settings, method_name) + + with pytest.raises(PydanticBoundaryReached): + method(json.dumps(payload) if method_name.endswith("json") else payload) + + assert isinstance(observed["input"], Mapping) + assert observed["input"]["api_rate_limit_key_secret"] is None + _assert_secret_not_retained(observed, encoded) + + @pytest.mark.parametrize("method_name", ["model_validate_json", "model_validate_strings"]) def test_alternate_validation_loads_rate_limit_secret(method_name: str) -> None: encoded = base64.b64encode(bytes(range(32))).decode("ascii") From bde3985b42ad8bc3df1f5c09e4e25feeec876610 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 03:32:48 +0100 Subject: [PATCH 12/18] docs(auth): record AUTH-04B review evidence --- .agent-loop/LOOP_STATE.md | 21 ++-- .agent-loop/REVIEW_LOG.md | 12 +++ .agent-loop/WORK_QUEUE.md | 2 +- .../CHUNK_MAP.md | 9 +- .../STATUS.md | 17 ++-- ...S-AUTH-001-04B-internal-review-evidence.md | 97 +++++++++++++++++++ .../WS-AUTH-001-04B-pr-trust-bundle.md | 89 +++++++++++++++++ 7 files changed, 224 insertions(+), 23 deletions(-) create mode 100644 .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md create mode 100644 .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index 75f736a..1dca0e2 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -7,21 +7,20 @@ - Active implementation chunk: `WS-AUTH-001-04B` - PostgreSQL Rate Controls - Branch: `codex/ws-auth-001-04b-postgres-rate-controls` - Worktree: `/home/abiorh/flow/workstream-auth-001-04b` -- Status: AUTH-04A post-merge memory merged through PR #112 as `7749f54`; the - user explicitly started AUTH-04B. Exact-head review rejected candidate - `62dd18e`; repair head `2d70581` plus artifact-authorization intent is being - bound to final evidence before exact-head re-review. +- Status: AUTH-04B implementation and all required internal review tracks pass + final SHA `922778b`; reviewed production SHA is `67484b5`. PR publication is + ready. - Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836` - Prior `WS-AUTH-001-01` final merged branch head: `b5217e1` - Latest integrated `main` merge commit: `7749f54` -- Current gate: final focused coverage and exact-head re-review; no PR until - every valid finding is resolved. -- Size checkpoint: implementation is 409 changed non-comment production lines; the - required 350-line inspection passed and scope is frozen below the 500 stop. +- Current gate: ready PR publication, GitHub checks, CodeRabbit, and explicit + human review. +- Size checkpoint: implementation is 480 changed non-comment production lines; + the required 350-line inspection passed and the 500-line hard stop holds. - Next chunk: none; do not start `WS-AUTH-001-05` automatically. -- Focused evidence: 93 owned tests pass at 99 percent aggregate changed-runtime - coverage; the isolated PostgreSQL concurrency/migration proofs and real API - E2E pass. AUTH-05 and later chunks remain inactive. +- Focused evidence: 77 isolated rate/config tests pass at 97 percent subsystem + coverage, 59 final config/object-graph tests pass, the exact migration proof + and real API E2E pass. AUTH-05 and later chunks remain inactive. - 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. diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 331c6f2..0b5e3e9 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -1,5 +1,17 @@ # Review Log +## 2026-07-14 - WS-AUTH-001-04B Internal Review Passed + +All required tracks pass final SHA `922778b`; reviewed production SHA is +`67484b5`. Senior engineering, architecture, docs, reuse, QA/test, CI integrity, +test delta, security/auth, privacy/data, and product/ops found no remaining +blocker. The final test-only proof instruments the BaseSettings boundary and +would fail the prior recoverable-`SecretStr` implementation. Evidence and the +PR trust bundle are recorded under the AUTH initiative reviews directory. + +Ready PR publication and external GitHub checks are the current gate. AUTH-05, +artifact runtime, and all route attachments remain inactive. + ## 2026-07-14 - WS-AUTH-001-04B Exact-Head Review Repair Required implementation review at `8f54fd9` passed QA, CI integrity, and test diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index ac7d093..73d282e 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -4,7 +4,7 @@ | Chunk | Title | Risk | Status | |---|---|---:|---| -| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Bounded repair behavior passed; final coverage and exact-head re-review pending | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Internal review passed at `922778b`; ready PR publication | ## 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 0346b73..8c17f66 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 @@ -17,7 +17,7 @@ stopped. | `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 | Merged through PR #111 as `90c9a28` | -| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Repaired contract passed at `b5dceb1`; bounded implementation active | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Internal review passed at `922778b`; ready PR publication | | `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 | @@ -86,7 +86,6 @@ 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 merged through PR #111 as `90c9a28`, and its post-merge memory merged through PR #112 as `7749f54`. The user explicitly -started AUTH-04B; required preimplementation review is the current gate and no -runtime edit was permitted before it passed. The second repaired contract passed -at `b5dceb1`; bounded AUTH-04B implementation is active. Do not start AUTH-05 or -POL-002-04. +started AUTH-04B. Its repaired contract passed at `b5dceb1`; bounded +implementation and all required internal review tracks pass final SHA +`922778b`. Publish the ready PR, then stop. Do not start AUTH-05 or POL-002-04. 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 72801c2..b2fdf3c 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 @@ -39,7 +39,8 @@ explicitly started AUTH-04B; its required L1 preimplementation review is active and rejected the first activated contract before runtime edits. The second repaired contract passed all required tracks at `b5dceb1`; bounded runtime implementation and deterministic evidence are complete, and the candidate is -in final repair evidence after exact-head internal review rejected `62dd18e`. +internally approved at final SHA `922778b`; reviewed production SHA is +`67484b5`. Ready PR publication is the current gate. ## Active planning chunk @@ -47,8 +48,8 @@ None. ## Active implementation chunk -`WS-AUTH-001-04B` - PostgreSQL Rate Controls. Implementation candidate under -required internal review; AUTH-05 remains inactive. +`WS-AUTH-001-04B` - PostgreSQL Rate Controls. Internal review passed; AUTH-05 +remains inactive. ## Current implementation branch @@ -64,7 +65,7 @@ required internal review; AUTH-05 remains inactive. | `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` | Merged | `codex/ws-auth-001-04-request-api-controls` | #111 | Merged as `90c9a28`; production review `cdcaf77`; final branch head `36c4aa5`. | -| `WS-AUTH-001-04B` | Re-review pending | `codex/ws-auth-001-04b-postgres-rate-controls` | - | Repair behavior and migration proofs pass; final subsystem coverage and exact-head reviewers pending. | +| `WS-AUTH-001-04B` | Ready PR | `codex/ws-auth-001-04b-postgres-rate-controls` | - | All required tracks pass final `922778b`; production review `67484b5`. | | `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. | @@ -80,12 +81,16 @@ required internal review; AUTH-05 remains inactive. ## Blockers -No external blocker. AUTH-04B internal repair is active under its passed -repaired L1 contract. It owns migration `0017`, following the +No external blocker. AUTH-04B internal review passed under its repaired L1 +contract. It owns migration `0017`, following the now-owned `0016` prefix on current main. Non-test operators must later supply explicit classification evidence rather than inferred kinds before the owning canonical actor migration. +AUTH-04B review evidence and its PR trust bundle are recorded at +`reviews/WS-AUTH-001-04B-internal-review-evidence.md` and +`reviews/WS-AUTH-001-04B-pr-trust-bundle.md`. + AUTH-04A review evidence and its PR trust bundle are recorded at `reviews/WS-AUTH-001-04A-internal-review-evidence.md` and `reviews/WS-AUTH-001-04A-pr-trust-bundle.md`. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md new file mode 100644 index 0000000..8b5d1e2 --- /dev/null +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md @@ -0,0 +1,97 @@ +# Internal Review Evidence: WS-AUTH-001-04B + +## Chunk + +`WS-AUTH-001-04B` - PostgreSQL Rate Controls + +open sub-agent sessions: none + +valid findings addressed: yes + +## Reviewed Revision + +Reviewed code SHA: `922778b8f48c0ccd74299797dc684f904c10601d` + +Reviewed production SHA: `67484b508637b7fc893e441e7afb5f5b34dd7715` + +Reviewed at: 2026-07-14T02:29:56Z + +Reviewer run IDs: engineering-architecture-docs-reuse=AUTH-04B-922778B-ENG-20260714; +qa-ci-test-delta=AUTH-04B-922778B-QA-20260714; +security-privacy-product=AUTH-04B-922778B-SEC-20260714 + +The final SHA differs from the reviewed production SHA only by additive tests +and review memory. Exact-head review confirmed identical production blobs and +that the boundary test fails the prior recoverable-`SecretStr` behavior. + +## Reviewed Change + +- Added PostgreSQL-owned fixed-window counters with database-clock boundaries, + atomic saturating upsert, bounded lock-safe pruning, and durable denial. +- Added privacy-keyed HMAC-SHA256 derivation and unattached first-access and + administrative-mutation dependencies with canonical 429/503 behavior. +- Added guarded migration `0017_api_controls` and exact preservation/downgrade + custody proof. +- Kept the rate key outside Pydantic's structured input/error graphs across + constructor, environment, layered dotenv, and all public model validators. +- Bound future artifact adapter I/O to central AUTH-07 permissions, AUTH-09 + service principals, and exact operation/resource authorization evidence. + +## Reviewer Results + +| Reviewer | Result | Blocking findings | Notes | +|---|---:|---|---| +| senior engineering | PASS | None | Database ownership, configuration privacy, and the 480-line bounded implementation are maintainable. | +| QA/test | PASS | None | Behavior, concurrency, migration, failure, and secret object-graph tests are genuine and mutation-sensitive. | +| security/auth | PASS | None | No raw or recoverable rate key remains in persistence, structured errors, or exception graphs. | +| product/ops | PASS | None | Dependencies remain unattached; artifact authority stays central and later chunks remain inactive. | +| architecture | PASS | None | Reuses shared sessions/errors and introduces no parallel authorization or storage authority path. | +| CI integrity | PASS | None | No workflow, threshold, exclusion, dependency, or bypass was weakened. | +| docs | PASS | None | AUTH operations and inactive ART contracts match the implemented boundaries. | +| reuse/dedup | PASS | None | One repository/service/dependency path owns rate controls. | +| test delta | PASS | None | Additive tests; no assertion, raises, skip, xfail, or skipTest weakening. | + +## Valid Findings Addressed + +- Made session-open, prune, execute, commit, rollback, cancellation, and + missing-database behavior explicit and privacy bounded. +- Proved concurrent increments, downstream rollback independence, database + time, same-clock `updated_at`, saturation, and deadlock-safe pruning. +- Locked downgrade before checking emptiness and preserved representative + `0016` artifact rows. +- Removed the secret from Pydantic's public field graph and closed constructor, + environment, layered-dotenv, mapping, JSON, string-mapping, malformed JSON, + non-ASCII, and exception-chain retention paths. +- Added deterministic BaseSettings boundary instrumentation proving that only + `None`, never a raw value or `SecretStr`, enters Pydantic validation. +- Reconciled ART planning so credentials and receipts never create authority. + +## Verification Results + +- focused isolated PostgreSQL rate/config matrix: 77 passed; +- changed subsystem statement coverage: 97 percent aggregate; dependency 96, + config 97, model 100, repository 100, and service 99 percent; +- final direct configuration/object-graph suite: 59 passed; +- exact AUTH-04B migration test: 1 passed, 8 deselected; +- earlier migration-inclusive repaired matrix: 78 passed; +- real API contract E2E: passed; +- Ruff, docstring threshold, stale wording, authorization docs, artifact + contracts, Markdown links, loop memory, 36 Agent Gates, additive test delta, + and diff hygiene: passed. + +The repository-wide local coverage run was interrupted by the host shutdown and +is not claimed. GitHub Backend retains the unchanged 78 percent global floor; +the materially changed subsystem exceeds the required 90 percent threshold. + +## Evidence Gate + +Evidence gate: PASS WITH GITHUB GLOBAL SUITE PENDING + +The reviewed implementation is 480 changed non-comment production lines, +below the 500-line hard stop. It adds no protected-route consumer, actor role, +grant, product permission, artifact runtime, or AUTH-05 behavior. + +## Stop Condition + +Publish a ready PR, then stop for GitHub Backend, Agent Gates, CodeRabbit, and +explicit human review. Do not merge or start AUTH-05 automatically. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md new file mode 100644 index 0000000..71e0e1c --- /dev/null +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md @@ -0,0 +1,89 @@ +# PR Trust Bundle: WS-AUTH-001-04B + +## Chunk + +`WS-AUTH-001-04B` - PostgreSQL Rate Controls + +## Goal + +Provide privacy-keyed, cross-replica PostgreSQL rate controls for future first +access and authority mutations without attaching them to production routes. + +## What Changed + +- Added atomic database-time fixed-window counters with durable denial, + saturation safety, and bounded lock-safe pruning. +- Added HMAC-keyed server dependencies for `first_access` and + `admin_mutation`; both remain unattached. +- Added migration `0017_api_controls` with guarded downgrade custody. +- Added non-retaining secret configuration across constructor, environment, + layered dotenv, and Pydantic's public model-validation methods. +- Clarified that artifact production I/O requires central AUTH permissions, + service principals, and exact resource authorization before adapter work. + +## Design + +Each consume owns a short independent committed session. One PostgreSQL upsert +uses one `statement_timestamp()` CTE to insert, reset, increment, saturate, and +return authoritative window state. HMAC framing stores only scope and a 32-byte +digest. The server key is private `SecretStr` state assigned only after +successful Pydantic validation; Pydantic receives no recoverable key object. + +## Scope Control + +The production delta is 480 non-comment lines, below the 500-line hard stop. +No route consumer, actor/grant behavior, product authority, artifact runtime, +AUTH-05 implementation, Redis counter, or in-memory cross-replica state was +added. + +## Behavior Proof + +Tests cover exact limits, repeated denial, expiry, database-clock retry values, +concurrency, cross-session durability, downstream rollback, saturation, +pruning, unavailable database phases, cancellation, privacy-safe persistence, +schema constraints, migration races, and representative artifact preservation. +Secret tests traverse structured error objects and complete exception graphs +and instrument the BaseSettings boundary directly. + +## Tests And Checks + +- 77 focused isolated PostgreSQL tests passed at 97 percent changed-subsystem + statement coverage. +- 59 final configuration/object-graph tests passed. +- Exact AUTH-04B migration and real API E2E passed. +- Ruff, docstring threshold, stale scans, links, loop memory, 36 Agent Gates, + additive test delta, and diff hygiene passed. +- GitHub Backend must provide the unchanged repository-wide 78 percent floor; + the interrupted local global run is not evidence. + +## Reviewer Results + +Senior engineering, QA/test, security/auth, privacy/data, product/ops, +architecture, CI integrity, docs, reuse/dedup, and test-delta tracks pass final +SHA `922778b`. Reviewed production SHA is `67484b5`; the final delta is additive +tests and review memory only. + +## External Review + +GitHub Backend, Agent Gates, CodeRabbit, and explicit human review begin after +ready PR publication. None replaces the completed internal review. + +## Remaining Risks + +- The dependencies are intentionally unattached; later owning chunks must add + route-specific permissions and consumer tests. +- Secret rotation resets effective counters and requires the documented + cross-replica quiescence procedure. +- The global suite and 78 percent repository floor remain GitHub-owned because + the local run was interrupted by host shutdown. + +## Human Review Focus + +Inspect transaction custody, HMAC framing/privacy, database-time math, pruning +lock order, migration downgrade refusal, secret object-graph non-retention, and +the fact that no production route consumes either dependency. + +## Human Merge Ownership + +Only the user may approve and merge this PR. Publication is not merge approval, +and AUTH-05 does not start automatically. From 81d6e685c6a24cd8df9d2f69c56c4a04074810a9 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 03:36:07 +0100 Subject: [PATCH 13/18] docs(auth): reconcile AUTH-04B publication state --- .agent-loop/WORK_QUEUE.md | 5 +++-- .../WS-AUTH-001-workstream-authorization-service/STATUS.md | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index 73d282e..2e1216c 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -57,8 +57,9 @@ ## Proposed Next `WS-AUTH-001-04A` post-merge memory merged through PR #112 as `7749f54`. The -user explicitly started `WS-AUTH-001-04B`; its bounded implementation candidate -is in required internal review. Do not start AUTH-05 or POL-002-04 automatically. +user explicitly started `WS-AUTH-001-04B`; all required internal review tracks +pass and ready PR publication is the current gate. Do not start 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/STATUS.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md index b2fdf3c..2523058 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 @@ -35,8 +35,8 @@ logging-state behavior-test repairs plus lifecycle evidence. Backend, Agent Gates, and CodeRabbit passed on final branch head `36c4aa5`, then explicit human approval merged PR #111 to `main` as `90c9a28` on 2026-07-13. AUTH-04A post-merge memory merged through PR #112 as `7749f54`. The user then -explicitly started AUTH-04B; its required L1 preimplementation review is active -and rejected the first activated contract before runtime edits. The second +explicitly started AUTH-04B; its required L1 preimplementation review rejected +the first activated contract before runtime edits. The second repaired contract passed all required tracks at `b5dceb1`; bounded runtime implementation and deterministic evidence are complete, and the candidate is internally approved at final SHA `922778b`; reviewed production SHA is From 5a186ad86658cfcbbc5061b74cc7418c6b6a2de3 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 03:38:09 +0100 Subject: [PATCH 14/18] docs(auth): bind AUTH-04B reviewed lifecycle head --- .../WS-AUTH-001-04B-internal-review-evidence.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md index 8b5d1e2..14a8cb5 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md @@ -10,15 +10,15 @@ valid findings addressed: yes ## Reviewed Revision -Reviewed code SHA: `922778b8f48c0ccd74299797dc684f904c10601d` +Reviewed code SHA: `81d6e685c6a24cd8df9d2f69c56c4a04074810a9` Reviewed production SHA: `67484b508637b7fc893e441e7afb5f5b34dd7715` -Reviewed at: 2026-07-14T02:29:56Z +Reviewed at: 2026-07-14T02:37:45Z -Reviewer run IDs: engineering-architecture-docs-reuse=AUTH-04B-922778B-ENG-20260714; -qa-ci-test-delta=AUTH-04B-922778B-QA-20260714; -security-privacy-product=AUTH-04B-922778B-SEC-20260714 +Reviewer run IDs: engineering-architecture-docs-reuse=AUTH-04B-81D6E68-ENG-20260714; +qa-ci-test-delta=AUTH-04B-81D6E68-QA-20260714; +security-privacy-product=AUTH-04B-81D6E68-SEC-20260714 The final SHA differs from the reviewed production SHA only by additive tests and review memory. Exact-head review confirmed identical production blobs and From f08aa9e82cfc504461239161ab4b04e598533452 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 03:40:25 +0100 Subject: [PATCH 15/18] docs(auth): record AUTH-04B PR publication --- .agent-loop/LOOP_STATE.md | 7 +++---- .agent-loop/REVIEW_LOG.md | 7 +++++++ .agent-loop/WORK_QUEUE.md | 6 +++--- .../CHUNK_MAP.md | 5 +++-- .../WS-AUTH-001-workstream-authorization-service/STATUS.md | 5 +++-- .../reviews/WS-AUTH-001-04B-pr-trust-bundle.md | 4 ++-- 6 files changed, 21 insertions(+), 13 deletions(-) diff --git a/.agent-loop/LOOP_STATE.md b/.agent-loop/LOOP_STATE.md index 1dca0e2..44018d7 100644 --- a/.agent-loop/LOOP_STATE.md +++ b/.agent-loop/LOOP_STATE.md @@ -8,13 +8,12 @@ - Branch: `codex/ws-auth-001-04b-postgres-rate-controls` - Worktree: `/home/abiorh/flow/workstream-auth-001-04b` - Status: AUTH-04B implementation and all required internal review tracks pass - final SHA `922778b`; reviewed production SHA is `67484b5`. PR publication is - ready. + final implementation SHA `922778b`; reviewed production SHA is `67484b5`. + Ready PR #113 is open. - Prior `WS-AUTH-001-01` reviewed implementation SHA: `be0b836` - Prior `WS-AUTH-001-01` final merged branch head: `b5217e1` - Latest integrated `main` merge commit: `7749f54` -- Current gate: ready PR publication, GitHub checks, CodeRabbit, and explicit - human review. +- Current gate: GitHub checks, CodeRabbit, and explicit human review on PR #113. - Size checkpoint: implementation is 480 changed non-comment production lines; the required 350-line inspection passed and the 500-line hard stop holds. - Next chunk: none; do not start `WS-AUTH-001-05` automatically. diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 0b5e3e9..972b899 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -1,5 +1,12 @@ # Review Log +## 2026-07-14 - WS-AUTH-001-04B Ready PR Published + +Ready PR #113 is open at +`https://github.com/Flow-Research/workstream/pull/113`. GitHub Backend, Agent +Gates, CodeRabbit, and explicit human review are the current gate. Publication +is not merge approval; AUTH-05 and artifact runtime remain inactive. + ## 2026-07-14 - WS-AUTH-001-04B Internal Review Passed All required tracks pass final SHA `922778b`; reviewed production SHA is diff --git a/.agent-loop/WORK_QUEUE.md b/.agent-loop/WORK_QUEUE.md index 2e1216c..f47c0d8 100644 --- a/.agent-loop/WORK_QUEUE.md +++ b/.agent-loop/WORK_QUEUE.md @@ -4,7 +4,7 @@ | Chunk | Title | Risk | Status | |---|---|---:|---| -| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Internal review passed at `922778b`; ready PR publication | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Internal review passed; PR #113 in external review | ## Planned Next @@ -58,8 +58,8 @@ `WS-AUTH-001-04A` post-merge memory merged through PR #112 as `7749f54`. The user explicitly started `WS-AUTH-001-04B`; all required internal review tracks -pass and ready PR publication is the current gate. Do not start AUTH-05 or -POL-002-04 automatically. +pass and ready PR #113 is in external review. Do not start 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 8c17f66..7d595b5 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 @@ -17,7 +17,7 @@ stopped. | `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 | Merged through PR #111 as `90c9a28` | -| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Internal review passed at `922778b`; ready PR publication | +| `WS-AUTH-001-04B` | PostgreSQL Rate Controls | L1 | Internal review passed; PR #113 in external review | | `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 | @@ -88,4 +88,5 @@ implementation. AUTH-04A merged through PR #111 as `90c9a28`, and its post-merge memory merged through PR #112 as `7749f54`. The user explicitly started AUTH-04B. Its repaired contract passed at `b5dceb1`; bounded implementation and all required internal review tracks pass final SHA -`922778b`. Publish the ready PR, then stop. Do not start AUTH-05 or POL-002-04. +`922778b`. Ready PR #113 is in external review. Stop; do not start AUTH-05 or +POL-002-04. 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 2523058..cb75b71 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 @@ -40,7 +40,8 @@ the first activated contract before runtime edits. The second repaired contract passed all required tracks at `b5dceb1`; bounded runtime implementation and deterministic evidence are complete, and the candidate is internally approved at final SHA `922778b`; reviewed production SHA is -`67484b5`. Ready PR publication is the current gate. +`67484b5`. Ready PR #113 is open; GitHub checks, CodeRabbit, and explicit human +review are the current gate. ## Active planning chunk @@ -65,7 +66,7 @@ remains inactive. | `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` | Merged | `codex/ws-auth-001-04-request-api-controls` | #111 | Merged as `90c9a28`; production review `cdcaf77`; final branch head `36c4aa5`. | -| `WS-AUTH-001-04B` | Ready PR | `codex/ws-auth-001-04b-postgres-rate-controls` | - | All required tracks pass final `922778b`; production review `67484b5`. | +| `WS-AUTH-001-04B` | External review | `codex/ws-auth-001-04b-postgres-rate-controls` | #113 | All required tracks pass final `922778b`; production review `67484b5`. | | `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. | diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md index 71e0e1c..a3dd6c4 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md @@ -65,8 +65,8 @@ tests and review memory only. ## External Review -GitHub Backend, Agent Gates, CodeRabbit, and explicit human review begin after -ready PR publication. None replaces the completed internal review. +Ready PR #113 is open. GitHub Backend, Agent Gates, CodeRabbit, and explicit +human review are pending. None replaces the completed internal review. ## Remaining Risks From c5ca3402ca9fa938183923b4c30577d509acb9ad Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 03:42:58 +0100 Subject: [PATCH 16/18] docs(auth): bind AUTH-04B publication review --- .../WS-AUTH-001-04B-internal-review-evidence.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md index 14a8cb5..4631675 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md @@ -10,15 +10,15 @@ valid findings addressed: yes ## Reviewed Revision -Reviewed code SHA: `81d6e685c6a24cd8df9d2f69c56c4a04074810a9` +Reviewed code SHA: `f08aa9e82cfc504461239161ab4b04e598533452` Reviewed production SHA: `67484b508637b7fc893e441e7afb5f5b34dd7715` -Reviewed at: 2026-07-14T02:37:45Z +Reviewed at: 2026-07-14T02:42:37Z -Reviewer run IDs: engineering-architecture-docs-reuse=AUTH-04B-81D6E68-ENG-20260714; -qa-ci-test-delta=AUTH-04B-81D6E68-QA-20260714; -security-privacy-product=AUTH-04B-81D6E68-SEC-20260714 +Reviewer run IDs: engineering-architecture-docs-reuse=AUTH-04B-F08AA9E-ENG-20260714; +qa-ci-test-delta=AUTH-04B-F08AA9E-QA-20260714; +security-privacy-product=AUTH-04B-F08AA9E-SEC-20260714 The final SHA differs from the reviewed production SHA only by additive tests and review memory. Exact-head review confirmed identical production blobs and From 4fb846c76029dc0bebb11ef90982207f162b54c9 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 05:34:20 +0100 Subject: [PATCH 17/18] test(auth): repair CI schema isolation --- .agent-loop/REVIEW_LOG.md | 14 +++ ...S-AUTH-001-04B-external-review-response.md | 51 ++++++++ ...S-AUTH-001-04B-internal-review-evidence.md | 4 + .../WS-AUTH-001-04B-pr-trust-bundle.md | 53 +++++++-- backend/tests/test_alembic.py | 1 + backend/tests/test_api_rate_controls.py | 109 ++++++++++-------- 6 files changed, 178 insertions(+), 54 deletions(-) create mode 100644 .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-external-review-response.md diff --git a/.agent-loop/REVIEW_LOG.md b/.agent-loop/REVIEW_LOG.md index 972b899..bc924d8 100644 --- a/.agent-loop/REVIEW_LOG.md +++ b/.agent-loop/REVIEW_LOG.md @@ -1,5 +1,19 @@ # Review Log +## 2026-07-14 - WS-AUTH-001-04B Backend CI Repair + +PR #113 Backend reached 82.12 percent repository coverage but failed because +the final AUTH-04B Alembic test left the shared isolated database at `base`. +Pytest's CI discovery order then entered `test_api_rate_controls.py` without +table `api_rate_control_counters`. The owning migration test now restores +Alembic `head` after its destructive cleanup. The exact failing +`test_alembic.py -> test_api_rate_controls.py` order is required repair proof; +no runtime, migration, coverage threshold, or workflow changes are permitted. +CodeRabbit posted one valid test-deduplication nit, now addressed through a +transaction-preserving shared insert helper. Its PR-description template +warning is addressed in the trust bundle; its docstring heuristic is superseded +by the passing repository-owned 95.8 percent docstring gate. + ## 2026-07-14 - WS-AUTH-001-04B Ready PR Published Ready PR #113 is open at diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-external-review-response.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-external-review-response.md new file mode 100644 index 0000000..e13dbe1 --- /dev/null +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-external-review-response.md @@ -0,0 +1,51 @@ +# External Review Response: WS-AUTH-001-04B + +## Pull Request + +PR #113 - Add durable PostgreSQL authorization rate controls + +## Backend CI Failure + +The first Backend run reached 82.12 percent repository coverage but failed in +test setup. The final AUTH-04B Alembic test downgraded the shared isolated +database to `base`; CI discovery then entered the rate-control suite without +table `api_rate_control_counters`. The owning migration test now restores +Alembic `head` after destructive cleanup. The exact failing order passes 22/22. + +## CodeRabbit Findings + +One valid nitpick is addressed: repeated raw counter inserts in concurrency, +saturation, and pruning tests now use one `_insert_counter_row` helper while +preserving transaction ownership and exact timestamp/count inputs. + +The PR-description warning is addressed by adding the missing human intent, +acceptance proof, evidence, reviewer table, and CI/gate-integrity sections to +the trust bundle and published PR body. + +The CodeRabbit docstring warning is not a repository failure. The authoritative +`docstr-coverage --config .docstr.yaml` gate passes at 95.8 percent, and the +Backend docstring step passed. Adding narration-only docstrings to private test +and implementation helpers would not improve behavior or ownership clarity. + +CodeRabbit's later incremental run was rate-limited and produced no additional +actionable thread. + +## Commands Rerun + +```bash +(cd backend && isolated-runner pytest -q tests/test_alembic.py::test_api_rate_control_schema_preserves_domain_and_guards_downgrade tests/test_api_rate_controls.py) +(cd backend && ruff check tests/test_alembic.py tests/test_api_rate_controls.py) +python3 scripts/check_loop_memory_state.py +python3 scripts/check_markdown_links.py +git diff --unified=0 origin/main -- backend/tests | (! rg '^-(.*assert|.*pytest\.raises|.*pytest\.mark\.(skip|xfail)|.*skipTest)') +git diff --check +``` + +- Exact CI failure order: 22 passed. +- No assertion, raises, skip, xfail, threshold, workflow, runtime, or migration + implementation was weakened. + +## Remaining Gate + +Push the reviewed repair and require Backend, Agent Gates, CodeRabbit status, +and explicit human review on the new PR head. Publication is not merge approval. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md index 4631675..d741903 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md @@ -73,6 +73,8 @@ that the boundary test fails the prior recoverable-`SecretStr` behavior. config 97, model 100, repository 100, and service 99 percent; - final direct configuration/object-graph suite: 59 passed; - exact AUTH-04B migration test: 1 passed, 8 deselected; +- Backend CI repair order (AUTH-04B migration test followed by rate suite): + 22 passed; the migration test restores Alembic head after cleanup; - earlier migration-inclusive repaired matrix: 78 passed; - real API contract E2E: passed; - Ruff, docstring threshold, stale wording, authorization docs, artifact @@ -82,6 +84,8 @@ that the boundary test fails the prior recoverable-`SecretStr` behavior. The repository-wide local coverage run was interrupted by the host shutdown and is not claimed. GitHub Backend retains the unchanged 78 percent global floor; the materially changed subsystem exceeds the required 90 percent threshold. +The first PR Backend run reached 82.12 percent but failed on test-owned schema +cleanup; the bounded test isolation repair requires a final Backend rerun. ## Evidence Gate diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md index a3dd6c4..2fb9d22 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.md @@ -9,6 +9,13 @@ Provide privacy-keyed, cross-replica PostgreSQL rate controls for future first access and authority mutations without attaching them to production routes. +## Human-Approved Intent + +The user explicitly started AUTH-04B after AUTH-04A and its post-merge memory +merged. The approved boundary is a cross-replica PostgreSQL rate-control +foundation only: dependencies remain unattached, product authority remains in +later AUTH chunks, and only the user may approve PR merge. + ## What Changed - Added atomic database-time fixed-window counters with durable denial, @@ -45,12 +52,26 @@ schema constraints, migration races, and representative artifact preservation. Secret tests traverse structured error objects and complete exception graphs and instrument the BaseSettings boundary directly. -## Tests And Checks +## Acceptance Criteria Proof + +- PostgreSQL owns one clock-bound atomic upsert, durable denial, saturation, + and bounded lock-safe pruning. +- Persistence contains only exact scope and a 32-byte HMAC digest. +- Constructor, environment, layered dotenv, and all public validation methods + retain no raw or recoverable secret in structured errors or exception graphs. +- Migration proof covers populated `0016` preservation, exact schema guards, + refusal on nonempty downgrade, writer races, empty downgrade, and re-upgrade. +- Route/dependency inventory proves neither new dependency is attached. +- Artifact adapter I/O remains inactive behind central AUTH-07/AUTH-09 gates. + +## Evidence Commands And Results - 77 focused isolated PostgreSQL tests passed at 97 percent changed-subsystem statement coverage. - 59 final configuration/object-graph tests passed. - Exact AUTH-04B migration and real API E2E passed. +- The exact CI failure order now passes 22/22 after the migration test restores + Alembic head following destructive cleanup. - Ruff, docstring threshold, stale scans, links, loop memory, 36 Agent Gates, additive test delta, and diff hygiene passed. - GitHub Backend must provide the unchanged repository-wide 78 percent floor; @@ -58,15 +79,33 @@ and instrument the BaseSettings boundary directly. ## Reviewer Results -Senior engineering, QA/test, security/auth, privacy/data, product/ops, -architecture, CI integrity, docs, reuse/dedup, and test-delta tracks pass final -SHA `922778b`. Reviewed production SHA is `67484b5`; the final delta is additive -tests and review memory only. +| Reviewer | Result | Blocking findings | +|---|---:|---| +| Senior engineering / architecture / docs / reuse | PASS | None | +| QA / test delta / CI integrity | PASS | None | +| Security / privacy / product ops | PASS | None | + +Reviewed implementation SHA is `922778b`; reviewed production SHA is +`67484b5`. Later reviewed heads contain only tests, evidence, and lifecycle +memory unless explicitly recorded in the external-review response. + +## CI And Gate Integrity + +- [x] No workflow, threshold, exclusion, skip, or coverage bypass changed. +- [x] Changed AUTH-04B subsystem remains above 90 percent coverage. +- [x] Repository docstring gate passes at 95.8 percent. +- [x] Internal evidence, loop memory, stale scans, links, and Agent Gates pass. +- [ ] Backend must pass the repaired exact PR head at the 78 percent global floor. +- [ ] Explicit human review and merge approval remain pending. ## External Review -Ready PR #113 is open. GitHub Backend, Agent Gates, CodeRabbit, and explicit -human review are pending. None replaces the completed internal review. +Ready PR #113 is open. Agent Gates passed on the prior published head. +CodeRabbit completed its available review with one valid test-deduplication +nit addressed; its later incremental review was rate-limited. The first Backend +run reached 82.12 percent coverage but exposed test-owned schema cleanup. All +final-head external checks and explicit human review remain pending. None +replaces the completed internal review. ## Remaining Risks diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 84091dc..646b35f 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -373,6 +373,7 @@ def guarded_downgrade() -> None: asyncio.run(_clear_api_rate_controls(isolated_database_env)) asyncio.run(_truncate_artifact_foundation(isolated_database_env)) command.downgrade(config, "base") + command.upgrade(config, "head") assert after == before assert artifact_after == artifact_before diff --git a/backend/tests/test_api_rate_controls.py b/backend/tests/test_api_rate_controls.py index ff64c5c..d2773fa 100644 --- a/backend/tests/test_api_rate_controls.py +++ b/backend/tests/test_api_rate_controls.py @@ -134,6 +134,38 @@ async def _stored_rate_row(factory, scope: str, digest: bytes): ).one() +async def _insert_counter_row( + session, + scope: str, + digest: bytes, + *, + started_offset: int, + expires_offset: int, + request_count: int, + updated_offset: int = 0, +) -> None: + await session.execute( + text( + "insert into api_rate_control_counters " + "(control_scope, key_digest, window_started_at, window_expires_at, " + "request_count, updated_at) values " + "(:scope, :digest, " + "statement_timestamp() + make_interval(secs => :started_offset), " + "statement_timestamp() + make_interval(secs => :expires_offset), " + ":request_count, " + "statement_timestamp() + make_interval(secs => :updated_offset))" + ), + { + "scope": scope, + "digest": digest, + "started_offset": started_offset, + "expires_offset": expires_offset, + "request_count": request_count, + "updated_offset": updated_offset, + }, + ) + + async def test_rate_control_commits_fixed_window_denials_and_resets_expiry( rate_control_factory, ) -> None: @@ -268,15 +300,13 @@ async def consume_once(): RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, independent_subject ) async with rate_control_factory() as outer: - await outer.execute( - text( - "insert into api_rate_control_counters " - "(control_scope, key_digest, window_started_at, window_expires_at, " - "request_count, updated_at) values " - "(:scope, :digest, statement_timestamp(), " - "statement_timestamp() + interval '1 minute', 1, statement_timestamp())" - ), - {"scope": FIRST_ACCESS_SCOPE, "digest": probe_digest}, + await _insert_counter_row( + outer, + FIRST_ACCESS_SCOPE, + probe_digest, + started_offset=0, + expires_offset=60, + request_count=1, ) independent = await service.consume( control_scope=FIRST_ACCESS_SCOPE, @@ -308,31 +338,23 @@ async def test_rate_control_saturates_bigint_and_prunes_only_bounded_other_rows( service = RateControlService(rate_control_factory) digest = rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, "saturated") async with rate_control_factory() as session: - await session.execute( - text( - "insert into api_rate_control_counters " - "(control_scope, key_digest, window_started_at, window_expires_at, " - "request_count, updated_at) values " - "(:scope, :digest, statement_timestamp(), " - "statement_timestamp() + interval '1 minute', 9223372036854775807, " - "statement_timestamp())" - ), - {"scope": FIRST_ACCESS_SCOPE, "digest": digest}, + await _insert_counter_row( + session, + FIRST_ACCESS_SCOPE, + digest, + started_offset=0, + expires_offset=60, + request_count=9_223_372_036_854_775_807, ) for value in range(125): - await session.execute( - text( - "insert into api_rate_control_counters " - "(control_scope, key_digest, window_started_at, window_expires_at, " - "request_count, updated_at) values " - "(:scope, :digest, statement_timestamp() - interval '2 minutes', " - "statement_timestamp() - interval '1 minute', 1, " - "statement_timestamp() - interval '1 minute')" - ), - { - "scope": ADMIN_MUTATION_SCOPE, - "digest": value.to_bytes(32, "big"), - }, + await _insert_counter_row( + session, + ADMIN_MUTATION_SCOPE, + value.to_bytes(32, "big"), + started_offset=-120, + expires_offset=-60, + request_count=1, + updated_offset=-60, ) await session.commit() @@ -363,21 +385,14 @@ async def test_concurrent_expired_keys_do_not_deadlock_during_pruning( subjects = ("expired-a", "expired-b") async with rate_control_factory() as session: for subject in subjects: - await session.execute( - text( - "insert into api_rate_control_counters " - "(control_scope, key_digest, window_started_at, window_expires_at, " - "request_count, updated_at) values " - "(:scope, :digest, statement_timestamp() - interval '2 seconds', " - "statement_timestamp() - interval '1 second', 5, " - "statement_timestamp() - interval '1 second')" - ), - { - "scope": FIRST_ACCESS_SCOPE, - "digest": rate_key_digest( - RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, subject - ), - }, + await _insert_counter_row( + session, + FIRST_ACCESS_SCOPE, + rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, RATE_ISSUER, subject), + started_offset=-2, + expires_offset=-1, + request_count=5, + updated_offset=-1, ) await session.commit() From 94fb2fee9e8668ec6671379830bc187ae0b0fa21 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 14 Jul 2026 05:40:21 +0100 Subject: [PATCH 18/18] docs(auth): bind AUTH-04B CI repair review --- .../WS-AUTH-001-04B-internal-review-evidence.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md index d741903..96684cd 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md @@ -10,15 +10,15 @@ valid findings addressed: yes ## Reviewed Revision -Reviewed code SHA: `f08aa9e82cfc504461239161ab4b04e598533452` +Reviewed code SHA: `4fb846c76029dc0bebb11ef90982207f162b54c9` Reviewed production SHA: `67484b508637b7fc893e441e7afb5f5b34dd7715` -Reviewed at: 2026-07-14T02:42:37Z +Reviewed at: 2026-07-14T04:39:18Z -Reviewer run IDs: engineering-architecture-docs-reuse=AUTH-04B-F08AA9E-ENG-20260714; -qa-ci-test-delta=AUTH-04B-F08AA9E-QA-20260714; -security-privacy-product=AUTH-04B-F08AA9E-SEC-20260714 +Reviewer run IDs: engineering-architecture-docs-reuse=AUTH-04B-4FB846C-ENG-20260714; +qa-ci-test-delta=AUTH-04B-4FB846C-QA-20260714; +security-privacy-product=AUTH-04B-4FB846C-SEC-20260714 The final SHA differs from the reviewed production SHA only by additive tests and review memory. Exact-head review confirmed identical production blobs and