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 993a51b1..a1a4f17a 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 @@ -726,7 +726,7 @@ AUTH-10B becomes a planning-only parent and splits into two sequential same-initiative children. AUTH-10B1 extends the existing durable PostgreSQL API-rate-control system with the closed `authorization_read` scope, dedicated limit/window configuration, and an unattached FastAPI dependency. It owns -forward migration `0032_authorization_read_rate_control`; it adds or activates +forward migration `0033_authorization_read_rate`; it adds or activates no authorization read route. Historical migration `0017` remains immutable, and existing first-access and administrative-mutation behavior is unchanged. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/INTENT.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/INTENT.md index 9fcd97ac..25f06ff3 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/INTENT.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/INTENT.md @@ -167,7 +167,8 @@ Resolved: privacy-safe reads second, and PREP-bound mutations third. D33 further splits 10B into 10B1 durable authorization-read rate control and 10B2 privacy-safe disclosure after review proved no reusable read scope existed. Migration - `0031` belongs to 10A, `0032` belongs to 10B1, and actor authorization-context + `0031` belongs to 10A, ART owns `0032_artifact_recovery`, and + `0033_authorization_read_rate` belongs to 10B1. Actor authorization-context disclosure remains deferred to AUTH-11. External deployment details such as issuer URL, JWKS URL, approved algorithms, diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/PLAN.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/PLAN.md index c0d642a3..de719434 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/PLAN.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/PLAN.md @@ -195,8 +195,10 @@ proving the same issuer role metadata alone no longer authorizes. separately started children. 10A establishes migration `0031`, immutable snapshots/grants, the typed/PostgreSQL three-role clean cut, and all five planned action rows with exact 10B/10C custody but no active surface. The - planning-only 10B parent splits into 10B1 migration `0032` and a dedicated - durable authorization-read rate-control scope, followed by 10B2 activation + planning-only 10B parent splits into 10B1 migration + `0033_authorization_read_rate`, directly after ART-owned + `0032_artifact_recovery`, and a dedicated durable authorization-read + rate-control scope, followed by 10B2 activation of candidate/list/detail reads with audited concealment and signed cursors. 10C activates PREP-bound issue/revoke mutations and concurrency proof. 13. Cut project identity, guide, source, and visibility queries over to local diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-10B1-authorization-read-rate-control.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-10B1-authorization-read-rate-control.md index 0a75df9d..2c39ef1c 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-10B1-authorization-read-rate-control.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-10B1-authorization-read-rate-control.md @@ -33,7 +33,7 @@ P1 ## Allowed files ```text -backend/alembic/versions/0032_authorization_read_rate_control.py +backend/alembic/versions/0033_authorization_read_rate_control.py backend/app/modules/api_controls/** backend/app/api/deps/api_controls.py backend/app/core/config.py @@ -41,6 +41,7 @@ backend/tests/test_alembic.py backend/tests/test_api_rate_controls.py backend/tests/test_config.py .github/workflows/backend.yml +scripts/test_agent_gates.py docs/operations_authorization_service.md docs/spec_authorization_service.md .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/** @@ -62,7 +63,8 @@ lowering, replacing, or broadening exclusions in any coverage threshold ## Acceptance criteria -- Migration `0032` changes only the current counter scope constraint from exact +- Migration `0033_authorization_read_rate`, directly after + `0032_artifact_recovery`, changes only the current counter scope constraint from exact `first_access|admin_mutation` to exact `first_access|admin_mutation|authorization_read`. Historical migrations stay immutable; upgrade preserves rows; downgrade refuses while new-scope rows @@ -88,7 +90,7 @@ lowering, replacing, or broadening exclusions in any coverage threshold `app/modules/api_controls/*` and `app/api/deps/api_controls.py`; existing reports and thresholds remain byte-for-byte unchanged. Zero action/OpenAPI delta is asserted. -- Operations/spec docs cover `0032` upgrade and downgrade preflight, refusal +- Operations/spec docs cover `0033_authorization_read_rate` upgrade and downgrade preflight, refusal with live read counters, forward recovery, limit/window settings, shared privacy HMAC key and secret separation, 429/Retry-After, retryable 503, and the intentionally unattached/no-action rollout state. @@ -96,7 +98,7 @@ lowering, replacing, or broadening exclusions in any coverage threshold ## Verification commands ```bash -(cd backend && .venv/bin/python -m ruff check app/modules/api_controls app/api/deps/api_controls.py app/core/config.py tests/test_api_rate_controls.py tests/test_config.py tests/test_alembic.py alembic/versions/0032_authorization_read_rate_control.py) +(cd backend && .venv/bin/python -m ruff check app/modules/api_controls app/api/deps/api_controls.py app/core/config.py tests/test_api_rate_controls.py tests/test_config.py tests/test_alembic.py alembic/versions/0033_authorization_read_rate_control.py) (cd backend && WORKSTREAM_TEST_ADMIN_DATABASE_URL= .venv/bin/python scripts/run_isolated_tests.py --metadata-json --timeout-seconds 300 -- .venv/bin/python -m pytest -q tests/test_api_rate_controls.py tests/test_config.py tests/test_alembic.py -k 'authorization_read or api_rate_control') python3 scripts/check_stale_authorization_docs.py python3 scripts/check_markdown_links.py diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B-internal-review-evidence.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B-internal-review-evidence.md index 02421b25..838eaa4c 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B-internal-review-evidence.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B-internal-review-evidence.md @@ -1,15 +1,16 @@ # WS-AUTH-001-10B Internal Review Evidence -Reviewed code SHA: `dd8afb790ddbe0e04581ee717e5fc03952e6b23e` +Reviewed code SHA: `746e577adca41d81cc0fbc9ee12dfbab12aac464` Reviewed planning SHA: `25b6ae134e3e3db4350fbcbb5c7cfeaa9e261044` -Reviewed against trusted main: `f2aa57a45f9088a91e8f7adcf79ec7e05a2b5734` +Reviewed against trusted main: `92b8a7aa813c5914d8191547b62eb3823a37a140` -Reviewed at: `2026-07-21T16:26:06Z` +Reviewed at: `2026-07-22T00:30:00Z` -Reviewer run IDs: `auth10_plan_core`, `auth10_plan_security_qa`, -`auth10_plan_ops_ci_docs` +Reviewer run IDs: original `auth10_plan_core`, `auth10_plan_security_qa`, and +`auth10_plan_ops_ci_docs`; integration `auth10b1_final_core`, +`auth10b1_final_security_qa`, and `auth10b1_final_ops_docs_ci` Reviewer tracks: senior engineering, QA/test, security/auth, product/ops, architecture, CI integrity, docs, reuse/dedup, and test delta @@ -50,8 +51,10 @@ public documentation, route, action availability, or authored live status. Initial review failed because no reusable read-rate scope existed and current 403/404 translation could not conceal sensitive resources without preserving -denial evidence. The repaired plan adds 10B1 migration `0032` for one durable -`authorization_read` scope and makes 10B2 depend on it. Further review froze +denial evidence. The repaired plan originally allocated migration `0032` to +10B1 for one durable `authorization_read` scope and made 10B2 depend on it. +After ART merged its own `0032_artifact_recovery`, the unchanged AUTH migration +was rebased linearly to `0033_authorization_read_rate`. Further review froze downgrade locking, exact capacity bounds, hosted 90 percent API-controls coverage, action-aware concealment, nonhuman prelookup behavior, unique candidate SQL, repository ownership, strict response fields, exact keyset diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B-pr-trust-bundle.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B-pr-trust-bundle.md index fce73542..f620f94d 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B-pr-trust-bundle.md @@ -49,7 +49,8 @@ CodeRabbit review, and human merge approval. ## Human review focus and merge ownership -Review the 10B1/10B2 boundary, migration `0032` ownership, action-aware +Review the 10B1/10B2 boundary, ART-owned `0032_artifact_recovery` followed by +AUTH-owned `0033_authorization_read_rate`, action-aware concealment, cursor threat model, hosted coverage requirements, and exact successor order. The user retains final approval authority for this specific PR and merge. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B1-external-review-response.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B1-external-review-response.md new file mode 100644 index 00000000..b75abda1 --- /dev/null +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B1-external-review-response.md @@ -0,0 +1,71 @@ +# WS-AUTH-001-10B1 External Review Response + +## GitHub Actions run 29872935281 + +Agent Gates, preflight, API E2E, and shards 1, 2, and 4 passed. Shard 3 failed +after `test_project_role_migration_constraints_and_immutable_history` expected +the former `0031_project_role_grants` head after a successful upgrade. That +assertion stopped before test-owned cleanup, so later migration tests correctly +refused to downgrade the leaked immutable project-role evidence. + +Repair `a8a0daef60c1374f103e26c092b59600f5465480` updated exactly three +current-head expectations to then-current AUTH head `0032`: project-role +schema, outbox schema, and the outbox downgrade transaction that rolls back to +its pre-attempt head. + +## GitHub Actions run 29875491247 + +The next refusal matrix proved that the requested `0032` to `0030` migration +is one transactional Alembic downgrade: refusal in `0031` rolls back the +preceding steps and retains the current authorization-read head. +Repair `8ceb4e16d8e152572c94ad3032d8a2edc2cea55e` changes only those two +multi-step refusal-state expectations. The separate successful direct +downgrade-to-`0031` expectation remains unchanged. + +## Post-main integration + +Trusted main `92b8a7aa813c5914d8191547b62eb3823a37a140` merged ART-owned +`0032_artifact_recovery`. Integration merge +`3b90fbd7cf3c80c3dcfc199953317492e4ddcd2e` rebased the unchanged AUTH +migration to direct successor `0033_authorization_read_rate`, leaving exactly +one Alembic head. Final implementation/docs SHA +`2d6d347e1e3f16821218d257ccb29e5e458d4a45` passed all nine internal tracks; +the combined ART/AUTH migration proof passed 3/3 on a fresh isolated database. +A hosted rerun remains required on the integrated PR head. + +## Repair evidence + +- The affected project-role schema, refusal-matrix, and outbox migration tests + pass together, 3/3, in a fresh isolated PostgreSQL database. +- Ruff and diff integrity pass. +- Senior engineering, architecture, reuse/dedup, security/auth, QA/test, + test-delta, product/ops, docs, and CI-integrity tracks re-reviewed the exact + repair SHA. No actionable finding remains after the metadata update. +- All nine tracks re-reviewed exact repair SHA `8ceb4e16` and passed. +- GitHub full shards and aggregate coverage must pass on the pushed repair. + +CodeRabbit reported no comments on the initial run; its status was pass with a +rate-limit note. + +## CodeRabbit final review + +CodeRabbit asked for explicit PostgreSQL-version validation because the +migration intentionally compares PostgreSQL-rendered `pg_get_expr` text. The +operations runbook now requires PostgreSQL major version 16, matching CI, and +provides an executable `server_version_num` preflight. Operators must stop and +use a reviewed forward migration change for another major version; they must +not bypass the fail-closed drift check. + +Its generated docstring warning reported 33.33 percent, but GitHub preflight's +repository-owned Docstring Coverage gate passed on the exact PR head. No +unrelated docstrings were added to satisfy a contradictory advisory metric. + +## Post-integration CodeRabbit repair + +CodeRabbit correctly observed that the migration round-trip seeded only +`first_access`. Repair `746e577a` now seeds both legacy scopes, +`first_access` and `admin_mutation`, and proves both survive the upgrade, the +refused downgrade, and the successful downgrade after only +`authorization_read` rows are cleared. The focused test passes against a +freshly provisioned PostgreSQL 16 database. All nine internal reviewer tracks +re-reviewed this one-file test strengthening and passed with no findings. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B1-internal-review-evidence.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B1-internal-review-evidence.md new file mode 100644 index 00000000..f719fb9d --- /dev/null +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B1-internal-review-evidence.md @@ -0,0 +1,97 @@ +# WS-AUTH-001-10B1 Internal Review Evidence + +Reviewed code SHA: `746e577adca41d81cc0fbc9ee12dfbab12aac464` + +Reviewed against trusted main: `92b8a7aa813c5914d8191547b62eb3823a37a140` + +Reviewed at: `2026-07-22T00:30:00Z` + +Reviewer run IDs: `auth10b1_final_core`, +`auth10b1_final_security_qa`, and `auth10b1_final_ops_docs_ci` + +Reviewer tracks: senior engineering, QA/test, security/auth, product/ops, +architecture, CI integrity, docs, reuse/dedup, and test delta + +Main-integration merge SHA: `3b90fbd7cf3c80c3dcfc199953317492e4ddcd2e`. +The reviewed tree also contains the CodeRabbit-requested PostgreSQL-major +prerequisite and the post-ART migration-lineage reconciliation. + +## Scope + +AUTH-10B1 adds one closed `authorization_read` scope to the existing durable +PostgreSQL API control, bounded configuration, and an unattached dependency. It +adds no route, action activation, cursor, disclosure, or mutation behavior. + +## Deterministic evidence + +- Focused isolated selector: PASS, 27 tests; 174 deselected. +- Migration/new-scope database proofs: PASS, 4 tests. +- Final old/new-scope concurrent consumption proof: PASS, 2 tests. +- Post-CI migration-head repair proof: PASS, 3 tests together in a fresh + isolated PostgreSQL database. +- Post-main combined ART/AUTH lineage proof: PASS, 3 tests in a fresh isolated + PostgreSQL database. +- Alembic heads: PASS, exactly `0033_authorization_read_rate`. +- Dependency and digest proofs: PASS, 6 tests. +- Ruff on all contract-owned Python paths: PASS. +- Agent Gates: PASS, 89 tests. +- Stale authorization documentation: PASS. +- Markdown links: PASS for all changed Markdown files. +- `git diff --check`: PASS. +- Full backend shards, aggregate 78 percent coverage, additive API-controls 90 + percent coverage, migration proof, and API E2E remain GitHub-owned evidence. + +## Reviewer results + +| Reviewer | Result | Blocking findings | +|---|---|---| +| senior engineering | PASS AFTER FIXES | none | +| QA/test | PASS AFTER FIXES | none | +| security/auth | PASS AFTER FIXES | none | +| product/ops | PASS AFTER FIXES | none | +| architecture | PASS AFTER FIXES | none | +| CI integrity | PASS AFTER FIXES | none | +| docs | PASS AFTER FIXES | none | +| reuse/dedup | PASS AFTER FIXES | none | +| test delta | PASS AFTER FIXES | none | + +## Findings resolved + +The repair loop added exact fail-closed migration constraint validation and +drift/no-mutation tests, reused the existing scoped writer helper, directly +proved durable new-scope limit/reset/separation/concurrency behavior, moved the +new scope's own 429 and private 503 behavior under test, regression-protected +the additive coverage command, and documented safe constraint-drift recovery. +Old first-access and admin-mutation proofs remain present alongside the new +scope; no test was removed, skipped, or weakened. + +GitHub shard 3 initially failed because three pre-existing migration tests +still treated `0031_project_role_grants` as current `head`. The first stale +assertion aborted before test-owned cleanup and caused the remaining downgrade +failures. Repair `a8a0daef` changed those expectations to then-current AUTH +head `0032`. Run `29875491247` exposed two multi-step refusal-state +expectations; repair `8ceb4e16` correctly proved a refusal retains the starting +head. After ART merged `0032_artifact_recovery`, integration merge `3b90fbd7` +rebased AUTH linearly to `0033_authorization_read_rate`. Current-head and +refusal expectations now retain `0033`, while successful AUTH downgrade stops +at direct predecessor ART `0032`. All nine tracks passed on final integrated +implementation/docs SHA `2d6d347e` against main `92b8a7aa`. + +## Post-integration CodeRabbit repair + +CodeRabbit's preservation finding was repaired in test SHA `746e577a`. The +round-trip now seeds and asserts both legacy scopes through upgrade, refused +downgrade, and successful downgrade. The focused isolated PostgreSQL 16 proof +passed 1/1, and all nine reviewer tracks re-reviewed the one-file delta: senior +engineering, architecture, reuse/dedup, security/auth, QA/test, test delta, +product/ops, docs, and CI integrity all passed with no findings. + +Valid findings addressed: yes + +Open sub-agent sessions: none after evidence publication + +## Remaining gate + +GitHub Actions, CodeRabbit, and explicit human review remain. After merge, +signed loop memory must stop at declared successor `WS-AUTH-001-10B2`; it must +not start without a separate explicit event on exact trusted `main`. diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B1-pr-trust-bundle.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B1-pr-trust-bundle.md new file mode 100644 index 00000000..63cf2f80 --- /dev/null +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-10B1-pr-trust-bundle.md @@ -0,0 +1,65 @@ +# WS-AUTH-001-10B1 PR Trust Bundle + +## Chunk + +`WS-AUTH-001-10B1` — Authorization Read Rate Control + +Merge intent: `.agent-loop/merge-intents/WS-AUTH-001-10B1.json` + +## Goal and approved boundary + +Provide durable, cross-replica abuse control before privacy-sensitive project +role reads are exposed. This chunk only prepares the shared control; 10B2 owns +route attachment, disclosure, concealment, cursors, and action activation. + +## What changed + +- Migration `0033_authorization_read_rate` safely adds one closed counter scope. +- Existing privacy-safe HMAC framing, PostgreSQL time, repository, and committed + independent session remain the only rate-control implementation. +- Dedicated bounded limit/window settings and one unattached FastAPI dependency + are added. +- Operations/spec documentation covers rollout, rollback, drift refusal, + recovery, 429/Retry-After, private 503, and secret separation. +- GitHub receives one additive 90 percent API-controls coverage report while + the repository-wide 78 percent and every existing subsystem gate remain. + +## Evidence and review + +Exact integrated implementation/docs tree +`2d6d347e1e3f16821218d257ccb29e5e458d4a45`, including integration merge +`3b90fbd7cf3c80c3dcfc199953317492e4ddcd2e`, passed all nine required internal +tracks against trusted main `92b8a7aa813c5914d8191547b62eb3823a37a140`. +Focused PostgreSQL, dependency, migration, concurrency, Ruff, Agent Gates, +stale-doc, Markdown-link, and diff-integrity checks pass. Full sharded tests and +coverage run in GitHub Actions because the local full suite takes hours. + +Before the ART merge, the first GitHub run exposed three stale tests that still +named `0031` as current head, and the first repair updated those pre-rebase +expectations to AUTH-owned `0032`. Run `29875491247` then proved a multi-step +refusal rolls back the full migration transaction and retains its starting +head. After ART claimed `0032_artifact_recovery`, AUTH rebased linearly to +`0033_authorization_read_rate`; current-head and refusal-state assertions now +retain `0033`, while successful AUTH downgrade stops at direct predecessor +`0032_artifact_recovery`. The combined lineage/migration suite passes 3/3 on a +fresh isolated database. A new hosted run remains required for the merged tree. +Alembic reports exactly one head: `0033_authorization_read_rate`. + +Post-integration CodeRabbit repair `746e577a` strengthens the migration +round-trip to seed and preserve both legacy scopes. Its focused isolated +PostgreSQL 16 test passes, and all nine internal tracks passed the exact +one-file delta. The hosted checks must pass again on the final evidence head. + +## Risks and controls + +Migration drift fails closed before DDL and leaves revision, constraint, and +rows unchanged. Downgrade locks before refusing any live or expired new-scope +row. The dependency is not attached to production routes, and stored keys never +contain raw issuer, subject, token, actor, grant, or network data. + +## Human review focus and merge ownership + +Review the exact constraint transition, locked downgrade refusal, old/new scope +isolation, absence of route/action changes, and additive CI coverage gate. The +user retains approval authority for this PR and merge. After merge, automation +records 10B2 as stopped/next; it does not begin automatically. diff --git a/.agent-loop/merge-intents/WS-AUTH-001-10B1.json b/.agent-loop/merge-intents/WS-AUTH-001-10B1.json new file mode 100644 index 00000000..a8f3a76e --- /dev/null +++ b/.agent-loop/merge-intents/WS-AUTH-001-10B1.json @@ -0,0 +1,9 @@ +{ + "chunk_id": "WS-AUTH-001-10B1", + "chunk_title": "Durable Authorization Read Rate Control", + "initiative_id": "WS-AUTH-001", + "next_chunk_id": "WS-AUTH-001-10B2", + "next_chunk_title": "Privacy-Safe Project Role Grant Reads", + "next_requires_explicit_start": true, + "schema_version": 2 +} diff --git a/.github/workflows/backend.yml b/.github/workflows/backend.yml index 3b5c50af..41c37726 100644 --- a/.github/workflows/backend.yml +++ b/.github/workflows/backend.yml @@ -366,6 +366,14 @@ jobs: working-directory: backend run: coverage report --include='app/modules/authorization/*' --precision=2 --fail-under=90 + - name: API controls coverage + working-directory: backend + run: >- + coverage report + --include='app/modules/api_controls/*,app/api/deps/api_controls.py' + --precision=2 + --fail-under=90 + - name: Task subsystem coverage working-directory: backend run: coverage report --include='app/modules/tasks/*' --precision=2 --fail-under=90 diff --git a/backend/alembic/versions/0033_authorization_read_rate_control.py b/backend/alembic/versions/0033_authorization_read_rate_control.py new file mode 100644 index 00000000..02643669 --- /dev/null +++ b/backend/alembic/versions/0033_authorization_read_rate_control.py @@ -0,0 +1,75 @@ +"""add durable authorization-read rate-control scope + +Revision ID: 0033_authorization_read_rate +Revises: 0032_artifact_recovery +Create Date: 2026-07-21 +""" + +from __future__ import annotations + +from alembic import op +import sqlalchemy as sa + + +revision = "0033_authorization_read_rate" +down_revision = "0032_artifact_recovery" +branch_labels = depends_on = None + +_TABLE = "api_rate_control_counters" +_CONSTRAINT = "ck_api_rate_control_counters_scope_token" +_OLD_SCOPE_SQL = "control_scope in ('first_access', 'admin_mutation')" +_NEW_SCOPE_SQL = ( + "control_scope in ('first_access', 'admin_mutation', 'authorization_read')" +) +_OLD_SCOPE_EXPRESSION = ( + "((control_scope)::text = ANY ((ARRAY['first_access'::character varying, " + "'admin_mutation'::character varying])::text[]))" +) +_NEW_SCOPE_EXPRESSION = ( + "((control_scope)::text = ANY ((ARRAY['first_access'::character varying, " + "'admin_mutation'::character varying, 'authorization_read'::character varying])::text[]))" +) + + +def _require_scope_constraint(expected: str) -> None: + definition = op.get_bind().execute( + sa.text( + "select pg_get_expr(conbin,conrelid) from pg_constraint " + "where conrelid=cast(:table as regclass) and conname=:constraint" + ), + {"table": _TABLE, "constraint": _CONSTRAINT}, + ).scalar_one_or_none() + if definition != expected: + raise RuntimeError("unexpected API rate-control scope constraint") + + +def _replace_scope_constraint(definition: str) -> None: + op.execute(sa.text(f"alter table {_TABLE} drop constraint {_CONSTRAINT}")) + op.execute( + sa.text( + f"alter table {_TABLE} add constraint {_CONSTRAINT} check ({definition})" + ) + ) + + +def upgrade() -> None: + """Add one closed scope while preserving all existing counters.""" + op.execute(sa.text(f"lock table {_TABLE} in access exclusive mode")) + _require_scope_constraint(_OLD_SCOPE_EXPRESSION) + _replace_scope_constraint(_NEW_SCOPE_SQL) + + +def downgrade() -> None: + """Restore the prior scope only when no authorization-read counters exist.""" + bind = op.get_bind() + bind.execute(sa.text(f"lock table {_TABLE} in access exclusive mode")) + _require_scope_constraint(_NEW_SCOPE_EXPRESSION) + has_rows = bind.execute( + sa.text( + f"select exists(select 1 from {_TABLE} " + "where control_scope='authorization_read')" + ) + ).scalar_one() + if has_rows: + raise RuntimeError("cannot downgrade live authorization-read rate controls") + _replace_scope_constraint(_OLD_SCOPE_SQL) diff --git a/backend/app/api/deps/api_controls.py b/backend/app/api/deps/api_controls.py index c4037ef6..a2b36af8 100644 --- a/backend/app/api/deps/api_controls.py +++ b/backend/app/api/deps/api_controls.py @@ -10,6 +10,7 @@ from app.api.deps.rate_controls import enforce_rate_control, get_rate_control_service from app.modules.api_controls.service import ( ADMIN_MUTATION_SCOPE, + AUTHORIZATION_READ_SCOPE, FIRST_ACCESS_SCOPE, RateControlService, ) @@ -48,3 +49,20 @@ async def enforce_admin_mutation_rate_limit( limit=settings.api_admin_mutation_rate_limit, window_seconds=settings.api_admin_mutation_rate_window_seconds, ) + + +async def enforce_authorization_read_rate_limit( + request: Request, + result: Annotated[AuthVerificationResult, Depends(get_auth_verification_result)], + service: Annotated[RateControlService, Depends(get_rate_control_service)], +) -> None: + """Consume one sensitive authorization-read allowance.""" + settings = request.app.state.settings + await enforce_rate_control( + request=request, + result=result, + service=service, + control_scope=AUTHORIZATION_READ_SCOPE, + limit=settings.api_authorization_read_rate_limit, + window_seconds=settings.api_authorization_read_rate_window_seconds, + ) diff --git a/backend/app/core/config.py b/backend/app/core/config.py index d0435561..cec36099 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -121,6 +121,8 @@ class Settings(BaseSettings): 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) + api_authorization_read_rate_limit: int = Field(default=120, ge=1, le=10_000) + api_authorization_read_rate_window_seconds: int = Field(default=60, ge=1, le=3_600) artifact_store_backend: Literal["disabled", "local", "s3_compatible"] = "disabled" artifact_local_root: Path | None = None artifact_s3_provider_profile: Literal["aws_s3", "minio"] | None = None diff --git a/backend/app/modules/api_controls/models.py b/backend/app/modules/api_controls/models.py index a99dafb2..f7b91852 100644 --- a/backend/app/modules/api_controls/models.py +++ b/backend/app/modules/api_controls/models.py @@ -16,7 +16,7 @@ class ApiRateControlCounter(Base): __tablename__ = "api_rate_control_counters" __table_args__ = ( CheckConstraint( - "control_scope in ('first_access', 'admin_mutation')", + "control_scope in ('first_access', 'admin_mutation', 'authorization_read')", name="scope_token", ), CheckConstraint("octet_length(key_digest) = 32", name="digest_length"), diff --git a/backend/app/modules/api_controls/service.py b/backend/app/modules/api_controls/service.py index 3bd35c1d..70c017f2 100644 --- a/backend/app/modules/api_controls/service.py +++ b/backend/app/modules/api_controls/service.py @@ -20,7 +20,8 @@ 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} +AUTHORIZATION_READ_SCOPE = "authorization_read" +RATE_SCOPES = {FIRST_ACCESS_SCOPE, ADMIN_MUTATION_SCOPE, AUTHORIZATION_READ_SCOPE} MAX_IDENTITY_BYTES = 4_096 PRUNE_BATCH_SIZE = 100 MISSING_DATABASE_ERROR = "WORKSTREAM_DATABASE_URL must be set before database access" diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 6165fd29..3086d109 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -133,7 +133,7 @@ def test_artifact_recovery_schema_and_empty_downgrade( command.downgrade(config, "base") command.upgrade(config, "head") assert asyncio.run(_artifact_recovery_schema(isolated_database_env)) == { - "revision": "0032_artifact_recovery", + "revision": "0033_authorization_read_rate", "constraints": { "artifact_recovery_attempt_custody", "artifact_verification_lineage_custody", @@ -199,7 +199,7 @@ def test_project_role_migration_constraints_and_immutable_history( command.upgrade(config, "head") result = asyncio.run(_exercise_project_role_migration(isolated_database_env)) assert result == { - "revision": "0032_artifact_recovery", + "revision": "0033_authorization_read_rate", "role_count": 3, "invalid_availability": "23514", "duplicate_role": "23505", @@ -407,7 +407,7 @@ def test_project_role_downgrade_refuses_each_reserved_evidence_predicate( ): command.downgrade(config, "0030_artifact_verification") assert asyncio.run(_project_role_refusal_state(isolated_database_env))[:3] == ( - "0032_artifact_recovery", + "0033_authorization_read_rate", True, True, ) @@ -434,7 +434,7 @@ def test_project_role_downgrade_refuses_each_reserved_evidence_predicate( ): command.downgrade(config, "0030_artifact_verification") assert asyncio.run(_project_role_refusal_state(isolated_database_env))[:3] == ( - "0032_artifact_recovery", + "0033_authorization_read_rate", True, True, ) @@ -462,7 +462,7 @@ def test_outbox_migration_schema_and_downgrade_writer_guard( command.upgrade(config, "head") schema = asyncio.run(_outbox_schema(isolated_database_env)) assert schema == { - "revision": "0032_artifact_recovery", + "revision": "0033_authorization_read_rate", "columns": { "aggregate_id", "aggregate_type", @@ -522,7 +522,7 @@ def test_outbox_migration_schema_and_downgrade_writer_guard( ) assert committed == "refused_after_commit" assert asyncio.run(_current_revision(isolated_database_env)) == ( - "0032_artifact_recovery" + "0033_authorization_read_rate" ) asyncio.run(_remove_outbox_migration_row(isolated_database_env, committed_project_id)) command.downgrade(config, "0028_artifact_admission") @@ -2736,6 +2736,211 @@ def guarded_downgrade() -> None: } +def test_authorization_read_rate_scope_upgrade_and_downgrade_refusal( + isolated_database_env: str, + migration_lock, +) -> None: + """Prove 0033 preserves counters and refuses a live new-scope downgrade.""" + project_root = Path(__file__).resolve().parents[1] + config = Config(str(project_root / "alembic.ini")) + config.set_main_option("script_location", str(project_root / "alembic")) + + async def insert(scope: str, marker: int) -> None: + engine = create_async_engine(isolated_database_env) + 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 " + "(:scope,:digest,statement_timestamp()," + "statement_timestamp()+interval '1 minute',1,statement_timestamp())" + ), + {"scope": scope, "digest": bytes([marker]) * 32}, + ) + finally: + await engine.dispose() + + async def scopes() -> list[str]: + engine = create_async_engine(isolated_database_env) + try: + async with engine.begin() as connection: + return list( + ( + await connection.execute( + text( + "select control_scope from api_rate_control_counters " + "order by control_scope" + ) + ) + ).scalars() + ) + finally: + await engine.dispose() + + async def clear_new_scope() -> None: + engine = create_async_engine(isolated_database_env) + try: + async with engine.begin() as connection: + await connection.execute( + text( + "delete from api_rate_control_counters " + "where control_scope='authorization_read'" + ) + ) + finally: + await engine.dispose() + + with migration_lock(): + try: + command.downgrade(config, "base") + command.upgrade(config, "0032_artifact_recovery") + asyncio.run(insert("first_access", 41)) + asyncio.run(insert("admin_mutation", 42)) + command.upgrade(config, "0033_authorization_read_rate") + assert asyncio.run(scopes()) == ["admin_mutation", "first_access"] + asyncio.run(insert("authorization_read", 43)) + with pytest.raises( + RuntimeError, + match="cannot downgrade live authorization-read rate controls", + ): + command.downgrade(config, "0032_artifact_recovery") + assert asyncio.run(scopes()) == [ + "admin_mutation", + "authorization_read", + "first_access", + ] + asyncio.run(clear_new_scope()) + + inserted = threading.Event() + release_insert = threading.Event() + with ThreadPoolExecutor(max_workers=2) as pool: + insert_future = pool.submit( + asyncio.run, + _insert_rate_control_until_released( + isolated_database_env, + bytes([45]) * 32, + inserted, + release_insert, + scope="authorization_read", + ), + ) + assert inserted.wait(timeout=5) + downgrade_future = pool.submit( + command.downgrade, + config, + "0032_artifact_recovery", + ) + asyncio.run( + _wait_for_rate_control_table_lock(isolated_database_env) + ) + release_insert.set() + insert_future.result(timeout=5) + with pytest.raises( + RuntimeError, + match="cannot downgrade live authorization-read rate controls", + ): + downgrade_future.result(timeout=5) + + asyncio.run(clear_new_scope()) + command.downgrade(config, "0032_artifact_recovery") + assert asyncio.run(scopes()) == ["admin_mutation", "first_access"] + with pytest.raises(IntegrityError): + asyncio.run(insert("authorization_read", 43)) + command.upgrade(config, "0033_authorization_read_rate") + asyncio.run(insert("authorization_read", 44)) + finally: + asyncio.run(_clear_api_rate_controls(isolated_database_env)) + command.upgrade(config, "head") + + +def test_authorization_read_rate_scope_migration_refuses_constraint_drift( + isolated_database_env: str, + migration_lock, +) -> None: + """Keep revision and counter state unchanged when the known constraint drifted.""" + config = _alembic_config() + + async def replace_constraint(definition: str) -> None: + engine = create_async_engine(isolated_database_env) + try: + async with engine.begin() as connection: + await connection.execute( + text( + "alter table api_rate_control_counters drop constraint " + "ck_api_rate_control_counters_scope_token" + ) + ) + await connection.execute( + text( + "alter table api_rate_control_counters add constraint " + "ck_api_rate_control_counters_scope_token check " + f"({definition})" + ) + ) + finally: + await engine.dispose() + + async def state() -> tuple[str, str, int]: + engine = create_async_engine(isolated_database_env) + try: + async with engine.connect() as connection: + revision = str( + await connection.scalar(text("select version_num from alembic_version")) + ) + definition = str( + await connection.scalar( + text( + "select pg_get_expr(conbin,conrelid) from pg_constraint " + "where conrelid='api_rate_control_counters'::regclass " + "and conname='ck_api_rate_control_counters_scope_token'" + ) + ) + ) + rows = int( + await connection.scalar( + text("select count(*) from api_rate_control_counters") + ) + ) + return revision, definition, rows + finally: + await engine.dispose() + + old_definition = "control_scope in ('first_access', 'admin_mutation')" + new_definition = ( + "control_scope in " + "('first_access', 'admin_mutation', 'authorization_read')" + ) + drifted_definition = "control_scope in ('first_access')" + + with migration_lock(): + try: + command.downgrade(config, "base") + command.upgrade(config, "0032_artifact_recovery") + asyncio.run(replace_constraint(drifted_definition)) + before_upgrade = asyncio.run(state()) + with pytest.raises( + RuntimeError, match="unexpected API rate-control scope constraint" + ): + command.upgrade(config, "0033_authorization_read_rate") + assert asyncio.run(state()) == before_upgrade + + asyncio.run(replace_constraint(old_definition)) + command.upgrade(config, "0033_authorization_read_rate") + asyncio.run(replace_constraint(drifted_definition)) + before_downgrade = asyncio.run(state()) + with pytest.raises( + RuntimeError, match="unexpected API rate-control scope constraint" + ): + command.downgrade(config, "0032_artifact_recovery") + assert asyncio.run(state()) == before_downgrade + + asyncio.run(replace_constraint(new_definition)) + finally: + command.upgrade(config, "head") + + def test_authority_audit_schema_preserves_legacy_and_guards_downgrade( isolated_database_env: str, migration_lock, @@ -5102,6 +5307,8 @@ async def _insert_rate_control_until_released( digest: bytes, inserted: threading.Event, release: threading.Event, + *, + scope: str = "first_access", ) -> None: """Hold an uncommitted writer until the downgrade is waiting on its lock.""" engine = create_async_engine(database_url) @@ -5112,10 +5319,10 @@ async def _insert_rate_control_until_released( "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(), " + "(:scope, :digest, statement_timestamp(), " "statement_timestamp() + interval '1 minute', 1, statement_timestamp())" ), - {"digest": digest}, + {"scope": scope, "digest": digest}, ) inserted.set() assert await asyncio.to_thread(release.wait, 5) @@ -5123,6 +5330,27 @@ async def _insert_rate_control_until_released( await engine.dispose() +async def _wait_for_rate_control_table_lock(database_url: str) -> None: + """Wait until downgrade is queued for the table's access-exclusive lock.""" + engine = create_async_engine(database_url) + try: + for _ in range(100): + async with engine.connect() as connection: + waiting = await connection.scalar( + text( + "select exists(select 1 from pg_locks " + "where relation='api_rate_control_counters'::regclass " + "and mode='AccessExclusiveLock' and not granted)" + ) + ) + if waiting: + return + await asyncio.sleep(0) + raise AssertionError("downgrade did not request the table lock") + 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_rate_controls.py b/backend/tests/test_api_rate_controls.py index c341659a..12f65689 100644 --- a/backend/tests/test_api_rate_controls.py +++ b/backend/tests/test_api_rate_controls.py @@ -16,6 +16,7 @@ from app.api.deps.api_controls import ( enforce_admin_mutation_rate_limit, + enforce_authorization_read_rate_limit, enforce_first_access_rate_limit, get_rate_control_service, ) @@ -27,7 +28,9 @@ from app.modules.api_controls.repository import ApiRateControlRepository, ConsumedCounter from app.modules.api_controls.service import ( ADMIN_MUTATION_SCOPE, + AUTHORIZATION_READ_SCOPE, FIRST_ACCESS_SCOPE, + RATE_SCOPES, RateControlDecision, RateControlService, RateControlUnavailableError, @@ -66,6 +69,19 @@ def test_rate_control_orm_model_matches_the_migration_contract() -> None: assert {index.name for index in table.indexes} == { "ix_api_rate_control_counters_window_expires_at" } + assert RATE_SCOPES == { + FIRST_ACCESS_SCOPE, + ADMIN_MUTATION_SCOPE, + AUTHORIZATION_READ_SCOPE, + } + scope_constraint = next( + constraint + for constraint in table.constraints + if constraint.name == "ck_api_rate_control_counters_scope_token" + ) + assert str(scope_constraint.sqltext) == ( + "control_scope in ('first_access', 'admin_mutation', 'authorization_read')" + ) @pytest.fixture @@ -121,6 +137,10 @@ def test_rate_key_digest_matches_literal_vector_and_separates_boundaries() -> No assert rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "subject") != ( rate_key_digest(RATE_SECRET, ADMIN_MUTATION_SCOPE, "issuer", "subject") ) + assert rate_key_digest(RATE_SECRET, AUTHORIZATION_READ_SCOPE, "issuer", "subject") not in { + rate_key_digest(RATE_SECRET, FIRST_ACCESS_SCOPE, "issuer", "subject"), + rate_key_digest(RATE_SECRET, ADMIN_MUTATION_SCOPE, "issuer", "subject"), + } @pytest.mark.parametrize( @@ -257,6 +277,61 @@ async def test_rate_control_commits_fixed_window_denials_and_resets_expiry( ).request_count == 1 +async def test_authorization_read_scope_enforces_and_resets_independently( + rate_control_factory, +) -> None: + service = RateControlService(rate_control_factory) + decisions = [ + await service.consume( + control_scope=AUTHORIZATION_READ_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=2, + window_seconds=60, + secret=RATE_SECRET, + ) + for _ in range(3) + ] + digest = rate_key_digest( + RATE_SECRET, AUTHORIZATION_READ_SCOPE, RATE_ISSUER, RATE_SUBJECT + ) + assert [decision.allowed for decision in decisions] == [True, True, False] + assert [decision.request_count for decision in decisions] == [1, 2, 3] + + for scope in (FIRST_ACCESS_SCOPE, ADMIN_MUTATION_SCOPE): + decision = await service.consume( + control_scope=scope, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=2, + window_seconds=60, + secret=RATE_SECRET, + ) + assert decision.request_count == 1 + + 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": AUTHORIZATION_READ_SCOPE, "digest": digest}, + ) + await session.commit() + + reset = await service.consume( + control_scope=AUTHORIZATION_READ_SCOPE, + issuer=RATE_ISSUER, + subject=RATE_SUBJECT, + limit=2, + window_seconds=60, + secret=RATE_SECRET, + ) + assert reset == RateControlDecision(allowed=True, request_count=1, retry_after=60) + + async def test_repository_persists_the_returned_database_timestamp( rate_control_factory, ) -> None: @@ -276,9 +351,13 @@ async def test_repository_persists_the_returned_database_timestamp( assert updated_at == consumed.db_now +@pytest.mark.parametrize( + "concurrent_scope", [ADMIN_MUTATION_SCOPE, AUTHORIZATION_READ_SCOPE] +) async def test_rate_control_concurrency_has_no_lost_or_rolled_back_consumption( rate_control_factory, monkeypatch: pytest.MonkeyPatch, + concurrent_scope: str, ) -> None: original_consume = ApiRateControlRepository.consume all_started = asyncio.Event() @@ -297,7 +376,7 @@ async def synchronized_consume(repository, *args, **kwargs): async def consume_once(): return await service.consume( - control_scope=ADMIN_MUTATION_SCOPE, + control_scope=concurrent_scope, issuer=RATE_ISSUER, subject="concurrent-subject", limit=7, @@ -307,9 +386,9 @@ async def consume_once(): decisions = await asyncio.gather(*(consume_once() for _ in range(20))) digest = rate_key_digest( - RATE_SECRET, ADMIN_MUTATION_SCOPE, RATE_ISSUER, "concurrent-subject" + RATE_SECRET, concurrent_scope, RATE_ISSUER, "concurrent-subject" ) - persisted = await _stored_rate_row(rate_control_factory, ADMIN_MUTATION_SCOPE, digest) + persisted = await _stored_rate_row(rate_control_factory, concurrent_scope, digest) assert sum(decision.allowed for decision in decisions) == 7 assert sorted(decision.request_count for decision in decisions) == list(range(1, 21)) @@ -674,6 +753,7 @@ async def test_unattached_dependencies_emit_canonical_429_and_use_token_identity [ RateControlDecision(True, 1, 60), RateControlDecision(False, 31, 17), + RateControlDecision(True, 1, 60), ] ) app = create_app( @@ -696,14 +776,23 @@ async def first_access() -> dict[str, bool]: async def admin_mutation() -> dict[str, bool]: return {"allowed": True} + @app.get( + "/_test/authorization-read-rate", + dependencies=[Depends(enforce_authorization_read_rate_limit)], + ) + async def authorization_read() -> 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") + read_allowed = await client.get("/_test/authorization-read-rate") assert allowed.status_code == 200 assert denied.status_code == 429 + assert read_allowed.status_code == 200 assert denied.headers["retry-after"] == "17" assert denied.json() == { "detail": "Rate limit exceeded", @@ -718,11 +807,46 @@ async def admin_mutation() -> dict[str, bool]: assert [call["control_scope"] for call in service.calls] == [ FIRST_ACCESS_SCOPE, ADMIN_MUTATION_SCOPE, + AUTHORIZATION_READ_SCOPE, ] + assert service.calls[2]["limit"] == 120 + assert service.calls[2]["window_seconds"] == 60 assert all(call["issuer"] == RATE_ISSUER for call in service.calls) assert all(call["subject"] == RATE_SUBJECT for call in service.calls) +async def test_authorization_read_dependency_emits_canonical_429() -> None: + service = _DecisionService([RateControlDecision(False, 121, 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.get( + "/_test/authorization-read-rate-denied", + dependencies=[Depends(enforce_authorization_read_rate_limit)], + ) + async def authorization_read() -> dict[str, bool]: + return {"allowed": True} + + async with AsyncClient( + transport=ASGITransport(app=app), base_url="http://testserver" + ) as client: + denied = await client.get("/_test/authorization-read-rate-denied") + + assert denied.status_code == 429 + assert denied.headers["retry-after"] == "17" + assert denied.json()["error"] == { + "code": "rate_limit_exceeded", + "message": "Rate limit exceeded", + "details": {}, + "correlation_id": denied.headers["x-correlation-id"], + "retryable": True, + } + assert service.calls[0]["control_scope"] == AUTHORIZATION_READ_SCOPE + + @pytest.mark.parametrize( ("settings", "subject", "service"), [ @@ -734,10 +858,15 @@ async def admin_mutation() -> dict[str, bool]: ), ], ) +@pytest.mark.parametrize( + "dependency", + [enforce_first_access_rate_limit, enforce_authorization_read_rate_limit], +) async def test_rate_dependency_unavailability_is_private_503( settings: Settings, subject: str, service, + dependency, ) -> None: app = create_app(settings) app.dependency_overrides[get_auth_verification_result] = lambda: _verified_rate_identity( @@ -747,7 +876,7 @@ async def test_rate_dependency_unavailability_is_private_503( @app.post( "/_test/rate-unavailable", - dependencies=[Depends(enforce_first_access_rate_limit)], + dependencies=[Depends(dependency)], ) async def unavailable() -> None: return None @@ -775,6 +904,7 @@ def test_rate_dependencies_are_not_attached_to_production_routes() -> None: forbidden = { enforce_first_access_rate_limit, enforce_admin_mutation_rate_limit, + enforce_authorization_read_rate_limit, } def dependency_calls(dependant) -> set: diff --git a/backend/tests/test_config.py b/backend/tests/test_config.py index 04e65ce9..ce8628ca 100644 --- a/backend/tests/test_config.py +++ b/backend/tests/test_config.py @@ -54,6 +54,8 @@ def test_default_settings_are_fail_closed(monkeypatch: pytest.MonkeyPatch) -> No 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 + assert settings.api_authorization_read_rate_limit == 120 + assert settings.api_authorization_read_rate_window_seconds == 60 def test_rate_limit_secret_is_canonical_and_redacted() -> None: @@ -359,6 +361,10 @@ def test_rate_limit_secret_rejects_invalid_values_without_echo(value: str) -> No ("api_first_access_rate_window_seconds", 3_601), ("api_admin_mutation_rate_window_seconds", 0), ("api_admin_mutation_rate_window_seconds", 3_601), + ("api_authorization_read_rate_limit", 0), + ("api_authorization_read_rate_limit", 10_001), + ("api_authorization_read_rate_window_seconds", 0), + ("api_authorization_read_rate_window_seconds", 3_601), ], ) def test_rate_limit_numeric_bounds_are_enforced(field_name: str, value: int) -> None: diff --git a/docs/operations_authorization_service.md b/docs/operations_authorization_service.md index f6cd5703..eb8ba4f5 100644 --- a/docs/operations_authorization_service.md +++ b/docs/operations_authorization_service.md @@ -304,6 +304,73 @@ Missing secret or database access fails first access closed with retryable HTTP `Retry-After`. Existing exact identity links do not consume first-access capacity. +AUTH-10B1 adds the closed `authorization_read` scope without attaching it to a +route or activating an action. Configure every replica consistently: + +```text +WORKSTREAM_API_AUTHORIZATION_READ_RATE_LIMIT=120 +WORKSTREAM_API_AUTHORIZATION_READ_RATE_WINDOW_SECONDS=60 +``` + +The limit accepts 1 through 10,000 and the window accepts 1 through 3,600 +seconds. This scope uses the existing API rate-control HMAC key; it never uses +an authentication or pagination-cursor key. Missing key or database access +returns the same retryable 503 when a later route attaches the dependency. +Exhaustion returns 429 with `Retry-After`. + +Before upgrading to `0033_authorization_read_rate`, confirm migration +`0032_artifact_recovery` is current and that no unreviewed constraint changes +exist. This migration requires PostgreSQL major version 16, matching the +CI-pinned database used to freeze the exact `pg_get_expr` rendering. Confirm +the target before either direction: + +```sql +SELECT current_setting('server_version_num')::integer / 10000 + AS postgres_major_version; +``` + +Stop if the result is not `16`; validate a different major version through a +reviewed forward migration change rather than bypassing the drift check. +Inspect the exact database-owned expression before either direction: + +```sql +SELECT pg_get_expr(conbin, conrelid) AS scope_constraint +FROM pg_constraint +WHERE conrelid = 'api_rate_control_counters'::regclass + AND conname = 'ck_api_rate_control_counters_scope_token'; +``` + +Before upgrade, the returned scope set must be exactly `first_access` and +`admin_mutation`. Before downgrade, it must be exactly `first_access`, +`admin_mutation`, and `authorization_read`. PostgreSQL may render these as an +`ANY (ARRAY[...])` expression with text casts; compare the complete expression +and values, not a substring. Upgrade takes an access-exclusive lock on +`api_rate_control_counters`, replaces only its closed scope constraint, and +preserves every existing counter. The dependency remains deliberately +unattached after this migration. + +If either direction reports `unexpected API rate-control scope constraint`, it +leaves the Alembic revision, constraint, and counter rows unchanged. Do not +drop, bypass, or force the constraint. Compare the live definition with the +reviewed migrations that actually ran, reconcile it to the canonical expected +definition through a reviewed forward repair, and then retry the migration. +Prefer forward recovery when the provenance of the drift is uncertain. + +Downgrade also takes the table lock before preflight and refuses while any live +or expired `authorization_read` row exists: + +```sql +SELECT count(*) AS authorization_read_rows +FROM api_rate_control_counters +WHERE control_scope = 'authorization_read'; +``` + +Do not delete an unexpired row merely to force rollback. Prefer forward +recovery. If rollback is required before AUTH-10B2 attaches the dependency, +verify the count is zero, quiesce deployments that could run the new scope, +then downgrade. After 10B2, wait for the largest configured window to expire, +quiesce every reader, delete only expired rows using PostgreSQL time, and retry. + Generate the secret outside the repository and store it in the deployment secret manager: diff --git a/docs/spec_authorization_service.md b/docs/spec_authorization_service.md index e56e9144..04b5ebad 100644 --- a/docs/spec_authorization_service.md +++ b/docs/spec_authorization_service.md @@ -853,7 +853,9 @@ The implementation order is fixed by the WS-AUTH-001 chunk map: 16. `WS-AUTH-001-ART-CUSTODY` and `WS-AUTH-001-REV-CUSTODY`: availability-neutral transfer to exact AUTH activation owners; 17. `WS-AUTH-001-PREP`: prepared mutation authorization protocol; -18. `WS-AUTH-001-10`: independent project contributor grants; +18. `WS-AUTH-001-10`: independent project contributor grants, with 10B1 + establishing durable authorization-read rate control before 10B2 exposes + candidate and grant-history reads; 19. `WS-AUTH-001-11` through `WS-AUTH-001-14`: complete resource-family cutovers; 20. `WS-AUTH-001-15`: obsolete authority removal and scanner enforcement; @@ -876,6 +878,11 @@ where existence itself is sensitive. First access and administrative mutations are rate-controlled through Postgres-backed fail-closed controls before their public APIs become available. +Migration `0033_authorization_read_rate` extends that same durable +counter with the closed `authorization_read` scope. Its dependency remains +unattached and activates no action until AUTH-10B2. The dedicated default is +120 requests per 60 seconds per verified issuer/subject digest, independently +configurable within the existing bounded limit and window ranges. ## Conformance Requirements diff --git a/scripts/test_agent_gates.py b/scripts/test_agent_gates.py index 35807a1f..c1568382 100644 --- a/scripts/test_agent_gates.py +++ b/scripts/test_agent_gates.py @@ -150,6 +150,11 @@ "--include='app/interfaces/auth.py,app/core/auth.py,app/adapters/auth/dev.py," "app/adapters/auth/flow.py' --precision=2 --fail-under=90", ) +AUTHORIZATION_READ_COVERAGE_COMMANDS = ( + "coverage report " + "--include='app/modules/api_controls/*,app/api/deps/api_controls.py' " + "--precision=2 --fail-under=90", +) def artifact_contract_phase_for(coverage_phase: str) -> str: @@ -5445,7 +5450,12 @@ def test_backend_coverage_thresholds_are_regression_protected() -> None: if str(step.get("run", "")).strip().startswith("coverage report ") and "--fail-under=90" in str(step.get("run", "")) ) - assert actual_coverage == (*expected_coverage, *AUTH_09B_COVERAGE_COMMANDS) + assert actual_coverage == ( + *expected_coverage, + *AUTH_09B_COVERAGE_COMMANDS[:2], + *AUTHORIZATION_READ_COVERAGE_COMMANDS, + *AUTH_09B_COVERAGE_COMMANDS[2:], + ) for command in expected_coverage: matches = [ step for step in steps if str(step.get("run", "")).strip() == command @@ -5461,8 +5471,10 @@ def test_backend_coverage_thresholds_are_regression_protected() -> None: assert any("app/modules/checkers/*" in command for command in later_commands) assert workflow.count("--fail-under=78") == 1 assert "--cov-fail-under" not in workflow - assert workflow.count("--fail-under=90") == len(expected_coverage) + len( - AUTH_09B_COVERAGE_COMMANDS + assert workflow.count("--fail-under=90") == ( + len(expected_coverage) + + len(AUTH_09B_COVERAGE_COMMANDS) + + len(AUTHORIZATION_READ_COVERAGE_COMMANDS) ) assert "continue-on-error" not in workflow