From f19540c12a8f160060925634639b60534744205d Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sun, 2 Aug 2026 18:37:29 +0100 Subject: [PATCH 1/9] feat(auth): activate guide policy mutations --- ...H-001-12D2-guide-bound-policy-mutations.md | 6 +- .../ACTION_CUSTODY.md | 4 +- .../DISCOVERY.md | 54 ++ .../REVIEW_LOG.md | 10 + .../WS-XINT-003-rev-auth-end-to-end/STATUS.md | 12 +- ...XINT-003-02B-policy-mutation-activation.md | 175 ++++- .../WS-XINT-003-02B-internal-review.md | 46 ++ .../WS-XINT-003-02B-pr-trust-bundle.md | 82 +++ .../0048_review_revision_policy_authority.py | 402 +++++++++++ backend/app/api/router.py | 2 + .../app/modules/authorization/catalogue.py | 12 +- backend/app/modules/authorization/kernel.py | 38 +- backend/app/modules/authorization/prepared.py | 142 +++- backend/app/modules/authorization/runtime.py | 74 ++ backend/app/modules/projects/models.py | 127 +++- .../policy_mutation_replay_repository.py | 111 +++ .../projects/policy_mutation_router.py | 145 ++++ .../projects/policy_mutation_service.py | 392 +++++++++++ backend/app/modules/projects/repository.py | 24 + backend/app/modules/projects/schemas.py | 6 +- backend/scripts/api_contract_e2e.py | 320 ++++----- backend/tests/test_alembic.py | 148 +++- backend/tests/test_artifact_admission.py | 132 ++-- backend/tests/test_authorization.py | 183 ++++- backend/tests/test_policy_identity_lineage.py | 6 +- .../tests/test_project_policy_mutations.py | 654 ++++++++++++++++++ backend/tests/test_projects.py | 209 +++--- backend/tests/test_tasks.py | 93 +-- docs/operations_authorization_service.md | 23 + docs/operations_project_operating_manual.md | 16 +- docs/operations_roles_permissions.md | 7 + docs/spec_authorization_service.md | 16 +- docs/spec_review_lifecycle.md | 10 +- 33 files changed, 3180 insertions(+), 501 deletions(-) create mode 100644 .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-internal-review.md create mode 100644 .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md create mode 100644 backend/alembic/versions/0048_review_revision_policy_authority.py create mode 100644 backend/app/modules/projects/policy_mutation_replay_repository.py create mode 100644 backend/app/modules/projects/policy_mutation_router.py create mode 100644 backend/app/modules/projects/policy_mutation_service.py create mode 100644 backend/tests/test_project_policy_mutations.py diff --git a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md index dcf32bbf3..fe909e140 100644 --- a/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md +++ b/.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md @@ -22,8 +22,10 @@ alternate writer. ## One writer path -- Surviving API: separate `PUT /projects/{project_id}/review-policy` and - `PUT /projects/{project_id}/revision-policy` routes in the dedicated +- Surviving API: separate + `PUT /api/v1/projects/{project_id}/guides/{guide_id}/review-policy` and + `PUT /api/v1/projects/{project_id}/guides/{guide_id}/revision-policy` routes + in the dedicated `backend/app/modules/projects/policy_mutation_router.py`, registered once by `backend/app/api/router.py`, each declaring its exact primary ActionId. This follows the project-create and guide-mutation router boundary on current main. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md index 153ec03b7..b26ea48f6 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md @@ -6,8 +6,8 @@ This table is the planning source of truth for the v0.1 review and human-revisio | ActionId | PermissionId | Principal and scope | Resource family | Surface owner | State | Activation wave | |---|---|---|---|---|---|---| -| `project.review_policy.update` | `project.review_policy.manage` | Project Manager grant for exact project | draft guide + ReviewPolicy version | project/REV semantics; AUTH mutation | registered planned | `WS-XINT-003-02B` after 02A lineage | -| `project.revision_policy.update` | `project.review_policy.manage` | Project Manager grant for exact project | draft guide + RevisionPolicy version | project/REV semantics; AUTH mutation | registered planned | `WS-XINT-003-02B` after 02A lineage | +| `project.review_policy.update` | `project.review_policy.manage` | Project Manager grant for exact project | draft guide + ReviewPolicy version | project/REV semantics; AUTH mutation | active | `WS-XINT-003-02B` after 02A lineage | +| `project.revision_policy.update` | `project.review_policy.manage` | Project Manager grant for exact project | draft guide + RevisionPolicy version | project/REV semantics; AUTH mutation | active | `WS-XINT-003-02B` after 02A lineage | | `review.queue.read` | `review.queue.read` | reviewer grant; exact project; self-review denied | concealed current-work view | REV | registered planned | `WS-XINT-003-03A` | | `review.claim` | `review.claim` | reviewer grant; exact project; self-review denied | queue entry + global reviewer lease state | REV | registered planned | `WS-XINT-003-03A` | | `review.release` | `review.release` | owning reviewer and active lease | ReviewLease | REV | registered planned | `WS-XINT-003-03A` | diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/DISCOVERY.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/DISCOVERY.md index 9a7d2f4d1..35a99b43b 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/DISCOVERY.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/DISCOVERY.md @@ -1,5 +1,59 @@ # Discovery: WS-XINT-003 REV-AUTH End-to-End Contract +## WS-XINT-003-02B current-main refresh — 2026-08-02 + +### Observed merged baseline + +- PR #242 merged 02A as `9618b938`; Alembic `0047` makes `ReviewPolicy` and + `RevisionPolicy` append-only identities with generation, digest, predecessor, + and complete/legacy semantics. +- `ProjectGuide` selects exact policy identities, and Task, Submission, and + CheckerRun retain the immutable lineage. Existing reads lock the selected row + through `ProjectRepository.lock_review_policy()` and + `lock_revision_policy()`. +- AUTH already registers strict + `ProjectReviewPolicyMutationResourceContext` and + `ProjectRevisionPolicyMutationResourceContext`; both actions remain planned. +- No `ProjectPolicyMutationService`, policy mutation router, replay repository, + idempotency ledger, provenance/evidence columns, append-only repository + writers, or public mutation routes exist on merged main. + +### Existing conventions to preserve + +- Project-create and guide-mutation routes use dedicated router/service modules, + caller-owned root transactions, opaque PREP handles, exact locked facts, and + atomic decision evidence. +- Committed idempotent replay is checked before fresh authority; pending or + changed reuse conflicts. A replay-only repository cannot write product rows. +- `policy_lineage.py` owns strict semantic validation and domain-separated + policy digests. `0047` owns immutability and downstream lineage and must not be + weakened or duplicated. + +### Gaps owned by 02B + +- Add one dedicated two-route API and one orchestration service. +- Add the two internal append-only repository primitives and a replay-only + ledger repository. +- Add migration `0048` for the ledger and nullable historical provenance plus + evidence references. +- Extend the existing PREP evaluator only for the two typed policy resources and + activate exactly their catalogue rows. +- Add PostgreSQL tests for replay, revocation, stale/crossed replacement, + active-guide freeze, immutability, atomic rollback, and migration round trip. + +### Risks and resolved assumptions + +- A combined persistence/activation chunk was rejected earlier; 02A has now + removed that blocker. 02B does not modify policy meanings or downstream + lineage. +- The API needs an explicit optimistic precondition. The implementation uses an + `If-Match` opaque selector binding the current policy ID, generation, and + canonical digest plus an explicit no-current sentinel for first creation; + wildcard/omitted preconditions deny. Digest-only matching was rejected because + equal policy semantics must not hide predecessor advancement. +- No external credentials are required. PostgreSQL proof runs in hosted CI when + local isolated database variables are absent. + ## Canonical references - [Review lifecycle specification](../../../docs/spec_review_lifecycle.md) diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md index e08ebeff3..e5a5cec17 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md @@ -81,3 +81,13 @@ test semantics mapping. Its post-main-merge review also found that the guide sufficiency migration test did not protect cleanup when the initial downgrade failed. All findings were valid and corrected; the exact response is in `reviews/WS-XINT-003-02A-external-review-response.md`. + +## WS-XINT-003-02B guide-bound policy mutation activation + +Architecture, security/auth, product/operations, QA/test, senior engineering, +reuse/dedup, docs, test-delta, and CI integrity reviewed the completed runtime +chunk. Valid findings corrected full PREP and denial binding, replay-before-PREP +ordering, same-actor replay, exact opaque selectors, route rollback, database +successor/predecessor custody, live fixture bypasses, and operator docs. All +tracks passed after correction; no finding remains open. Final evidence is in +`reviews/WS-XINT-003-02B-internal-review.md`. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md index 8b681c932..fd4036168 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/STATUS.md @@ -2,9 +2,10 @@ ## Current status -WS-XINT-003-01 is merged. WS-XINT-003-02A immutable policy identity and -downstream lineage is implemented on its bounded branch and has passed final -internal review. No policy mutation action or public surface is activated. +WS-XINT-003-01 and WS-XINT-003-02A are merged. WS-XINT-003-02B policy mutation +activation is the active bounded implementation chunk from `origin/main` at +`2c24c91d`. No policy mutation action or public surface was active at its +starting baseline. ## Baseline @@ -44,5 +45,6 @@ REV-owned semantics with AUTH-owned mutation authorization. ## Next step -Open the 02A PR, run exact-head hosted CI and CodeRabbit, obtain human merge, -and stop. WS-XINT-003-02B requires a new explicit user start after 02A merges. +WS-XINT-003-02B implementation and internal review are complete. Open the PR, +run exact-head hosted review, resolve every valid external finding, obtain human +merge, and stop before review lifecycle activation. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md index 6d114da6b..c29791af6 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md @@ -2,7 +2,9 @@ ## Status -Planning skeleton. Refresh only after 02A merges; do not start automatically. +In progress after PR #242 merged 02A and the user explicitly started 02B on +2026-08-02. Refreshed from `origin/main` at merge `6babf81b` before application +code changes and rebased onto `2c24c91d` before external review. ## Goal @@ -18,10 +20,18 @@ existing opaque, transaction-bound PREP protocol. - A replay-only repository may touch only the idempotency ledger. - Exact committed replay returns the recorded response without new PREP, policy write, or allowed evidence, including after later grant/link - revocation. Changed or pending replay conflicts without product state. + revocation, but only after current authentication proves the same active + ActorProfile and the replay row matches the actor, action, project, guide, + idempotency key, and canonical request digest. An idempotency key alone never + discloses a response. Changed or pending replay conflicts without product + state. - Final PREP consumption follows locks on the exact project, draft guide, selected current policy, reserved replacement identity, actor/link/grant, operation, request digest, session, and transaction. +- The two typed policy PREP contexts bind the operation identity, canonical + request digest, reserved successor identity and generation, predecessor + identity and digest (or the explicit no-current sentinel), draft-guide + status, exact actor/link/grant/project scope, session, and root transaction. - New versions persist actor, identity link, matched grant, project scope, ActionId, decision-event reference, predecessor identity/digest, generation, and canonical policy digest atomically with selection advancement. @@ -30,8 +40,163 @@ existing opaque, transaction-bound PREP protocol. replacement races fail with no partial policy or audit state. - No review lifecycle ActionId or behavior is activated. +## Allowed files + +```text +backend/app/api/router.py +backend/app/modules/projects/models.py +backend/app/modules/projects/repository.py +backend/app/modules/projects/schemas.py +backend/app/modules/projects/policy_lineage.py +backend/app/modules/projects/policy_mutation_replay_repository.py +backend/app/modules/projects/policy_mutation_service.py +backend/app/modules/projects/policy_mutation_router.py +backend/app/modules/authorization/catalogue.py +backend/app/modules/authorization/kernel.py +backend/app/modules/authorization/prepared.py +backend/app/modules/authorization/runtime.py +backend/alembic/versions/0048_review_revision_policy_authority.py +backend/tests/test_authorization.py +backend/tests/test_project_policy_mutations.py +backend/tests/test_policy_identity_lineage.py +backend/tests/test_projects.py +backend/tests/test_tasks.py +backend/tests/test_artifact_admission.py +backend/tests/test_alembic.py +backend/tests/test_artifact_architecture.py +backend/scripts/api_contract_e2e.py +docs/spec_authorization_service.md +docs/spec_review_lifecycle.md +docs/operations_authorization_service.md +docs/operations_roles_permissions.md +docs/operations_project_operating_manual.md +.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-12D2-guide-bound-policy-mutations.md +.agent-loop/initiatives/WS-REV-001-review-revision-lifecycle/chunks/WS-REV-001-03P-review-revision-policy-persistence.md +.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/** +``` + +## Not allowed changes + +- Queue, lease, review, finding, decision, or revision-execution behavior. +- Artifact, contribution, compensation, reputation, frontend, or token-verifier + behavior. +- Compatibility routes, embedded guide policy fields, duplicate policy models, + alternate writers, generic authorization protocols, or fallback authority. +- Mutation of historical policy versions or rewriting 02A lineage. + +## Migration and schema ownership + +- `0048_review_revision_policy_authority` follows merged `0047` and adds only + the replay ledger plus nullable historical provenance/evidence columns needed + by newly appended complete policy versions. +- Database constraints require each newly appended complete 02B row to carry a + coherent non-null actor profile, identity link, matched grant, project scope, + ActionId, decision-event reference, predecessor identity/digest (or the + explicit first-version marker), generation, and canonical digest. Existing + legacy rows remain explicitly grandfathered and nullable. +- Existing 02A immutable guards, lineage keys, Task/Submission/CheckerRun locks, + and policy semantic fields remain authoritative. +- `0048` corrects the 02A draft-setup selector shape so the review selector + triple and revision selector triple are each independently all-null or + all-present. This is required because the two policies have separate routes + and may be attached in either order. The active/superseded-guide constraint + continues to require both complete selector triples, and partial triples are + always rejected. +- The replay ledger has one UUID operation identity, action, actor, exact + project/guide, request digest, state, reserved replacement policy identity, + and committed response. It cannot be a policy writer. + +## API and transaction contract + +- `PUT /api/v1/projects/{project_id}/guides/{guide_id}/review-policy` declares + only `project.review_policy.update`. +- `PUT /api/v1/projects/{project_id}/guides/{guide_id}/revision-policy` declares + only `project.revision_policy.update`. +- Both require an `Idempotency-Key` UUID and an `If-Match` opaque selector that + binds the current policy ID, generation, and canonical digest; + creation requires the exact quoted sentinel `If-Match: "no-current-policy"`, + never an omitted or wildcard precondition. +- The canonical request digest includes the HTTP method, exact ActionId, + project ID, guide ID, policy kind, exact `If-Match` selector or no-current sentinel, + normalized policy semantics, and idempotency operation identity. +- The service owns one root transaction. It checks committed replay before PREP, + prepares exact Project Manager authority, locks project/guide/current policy + and replay reservation, validates server-computed semantics/digest, consumes + PREP, appends one policy, advances the guide selector, stages allowed evidence, + records the response, and commits once. +- Denial and conflict paths expose bounded errors and create no policy row, + selector advancement, committed replay, or allowed evidence. +- Denial/conflict audit evidence, when recorded, is limited to action, + project/guide, reason code, and request digest; it never contains the raw + request body, policy semantics, or internal error details. + +## Acceptance criteria + +- Exactly the two policy actions become active; no other action availability or + permission mapping changes. +- Exact covered-project Project Manager authority is required and revalidated + against the locked actor, identity link, grant, project, draft guide, current + selected policy, reserved successor, generation, predecessor digest, request, + session, and transaction. +- Complete request semantics are normalized through `policy_lineage.py`; the + server owns the canonical policy digest. +- A successful replacement appends one immutable policy row, records complete + provenance/evidence, and advances only the matching guide selector atomically. +- A draft guide with neither policy may attach review then revision or revision + then review; the intermediate state has one complete selector triple. Guide + activation remains impossible until both complete policies are selected. +- Exact committed replay returns the recorded response without PREP or a new + allowed event, even after grant/link revocation, only for the same currently + authenticated active ActorProfile and exact recorded actor/action/project/ + guide/idempotency/request tuple. Pending, changed, crossed, stale, or + differently authenticated replay conflicts without product mutation. +- Active guide, stale `If-Match`, stale selected policy, cross-project/guide, + wrong action, copied/replayed/wrong-session/wrong-transaction handle, revoked + link/grant, and concurrent replacement all fail closed. +- Direct update/delete/truncate remains refused by 02A guards; old mutators and + embedded guide-policy inputs do not reappear. +- OpenAPI and architecture tests prove one router/service/writer path and exact + primary actions. + +## Verification commands + +```bash +(cd backend && .venv/bin/ruff check app tests scripts) +(cd backend && .venv/bin/pytest -q tests/test_artifact_architecture.py) +(cd backend && WORKSTREAM_TEST_ADMIN_DATABASE_URL="$WORKSTREAM_TEST_ADMIN_DATABASE_URL" \ + .venv/bin/python scripts/run_isolated_tests.py \ + --metadata-json .ci/xint-003-02b-focused.json --lane xint_003_02b \ + -- .venv/bin/pytest -q tests/test_authorization.py \ + tests/test_project_policy_mutations.py tests/test_alembic.py) +(cd backend && WORKSTREAM_TEST_DATABASE_URL="$WORKSTREAM_TEST_DATABASE_URL" \ + .venv/bin/pytest -q tests/test_project_policy_mutations.py \ + --cov=app.modules.projects.policy_mutation_replay_repository \ + --cov=app.modules.projects.policy_mutation_service \ + --cov=app.modules.projects.policy_mutation_router \ + --cov-report=term-missing --cov-fail-under=90) +(cd backend && .venv/bin/python scripts/api_contract_e2e.py) +python3 scripts/check_stale_authorization_docs.py +python3 scripts/check_stale_artifact_contracts.py +python3 scripts/check_stale_workstream_wording.py +python3 scripts/check_markdown_links.py +git diff --check +``` + +GitHub `Backend / test`, `Agent Gates / agent-gates`, and CodeRabbit must pass +on the exact final head. No local full-suite run is required. + +## Required reviewers + +Architecture, security/auth, product/operations, QA/test, senior engineering, +reuse/dedup, docs, test-delta, and CI integrity. + +## Human review focus + +The sole writer path, exact Project Manager/project/guide/policy binding, +append-only semantics, replay-before-PREP behavior, atomic provenance/evidence, +active-guide freeze, and absence of review-lifecycle activation. + ## Risk, review, and stop -L1. Exact files and commands must be refreshed from post-02A main. Require all -L1 reviewers, hosted full coverage, CodeRabbit, and human merge. Merge and stop -before 03A. +L1. Require all named reviewers, hosted full coverage, CodeRabbit, and human +merge. Merge and stop before 03A. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-internal-review.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-internal-review.md new file mode 100644 index 000000000..f9225bf91 --- /dev/null +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-internal-review.md @@ -0,0 +1,46 @@ +# Internal Review: WS-XINT-003-02B + +## Scope + +Final working-tree review of the sole authorized, guide-bound ReviewPolicy and +RevisionPolicy append-and-select path. + +## Results + +- Architecture: PASS after prepare-denial evidence became request-digest bound + and replay classification moved before PREP. +- Security/auth: PASS after database custody began proving the exact selected + successor and actual predecessor generation/digest. +- Product/operations: PASS; only covered Project Managers may configure draft + policy, and no review/revision lifecycle behavior is activated. +- QA/test: PASS WITH LOW RISKS after full PREP lineage, same-actor replay, and + real API/database custody proof were added. +- Senior engineering: PASS WITH LOW RISKS after rebase, replay ordering, route + rollback, and database integration proof. +- Reuse/dedup: PASS WITH LOW RISKS; the existing PREP and policy-digest + abstractions remain the only paths. +- Docs: PASS after roles, active routes, and migration rollback were documented. +- Test delta: PASS WITH LOW RISKS; no test was removed, skipped, or weakened. +- CI integrity: PASS; no CI file, threshold, or failure behavior changed. + +No blocking finding remains. All reviewer sessions completed. + +## Deterministic evidence + +- Ruff passed across the changed backend surface. +- Focused policy/PREP tests passed: 11 tests. +- Policy mutation service/router/replay coverage passed: 10 tests, 90.58 percent. +- Artifact architecture passed: 20 tests. +- Migration `0047:0048` offline PostgreSQL SQL generation passed. +- Authorization, artifact, wording, Markdown-link, and whitespace checks passed. + +The PostgreSQL-isolated migration/API cases and repository-wide 78-percent +coverage suite remain assigned to hosted GitHub Actions on the exact PR head. + +## Residual low risks + +- Clients currently compose the documented opaque replacement selector from + returned policy ID, generation, and digest; a named response ETag may be + considered later without changing this authorization boundary. +- Shared key-gated mutation-router wiring should be extracted only if a third + occurrence makes the duplication material. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md new file mode 100644 index 000000000..8b98e4fb6 --- /dev/null +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md @@ -0,0 +1,82 @@ +# PR Trust Bundle: WS-XINT-003-02B + +## Chunk + +`WS-XINT-003-02B` — Guide-bound policy mutation activation. + +## Goal and human-approved intent + +Activate exactly `project.review_policy.update` and +`project.revision_policy.update` so a covered Project Manager can append and +select immutable policy versions for one exact draft guide. Do not activate the +review or revision lifecycle. + +## What changed and why + +- Added the two guide-bound `PUT` routes and one policy mutation service. +- Added append-only ReviewPolicy/RevisionPolicy provenance and one replay ledger. +- Extended the existing opaque PREP protocol with exact policy lineage facts. +- Added deferred PostgreSQL custody joining the selected successor, real + predecessor, replay record, authority evidence, actor/link/grant, and digest. +- Replaced remaining live fixture bypasses with the public policy routes and + retained explicitly historical incomplete fixtures only where required. + +This removes direct or embedded policy writes and makes the authorized path the +sole live configuration boundary. + +## Design and alternatives rejected + +The design uses opaque exact `If-Match` selectors, UUID idempotency keys, +replay classification before PREP, locked selector revalidation, single-use +transaction-bound PREP consumption, append-only rows, and atomic evidence. +Raw AuthorizationContext authority, role-only fallback, mutable policy rows, +digest-only selectors, and a second authorization protocol were rejected. + +## Scope and product behavior + +Only draft-guide policy configuration changes. Reviewer queues, leases, +findings, decisions, contributor revisions, artifacts, payments, contribution +records, and reputation remain unavailable or unchanged. + +## Acceptance proof and tests + +- Focused policy/PREP tests: 11 passed. +- New-subsystem coverage: 10 passed, 90.58 percent. +- Artifact architecture: 20 passed. +- Ruff, migration SQL generation, stale authorization/artifact/wording scans, + Markdown links, and `git diff --check`: passed. +- Full PostgreSQL-isolated and repository coverage gates are delegated to + GitHub Actions as required; no local full-suite run was performed. + +## Test delta and CI integrity + +No tests were removed, skipped, or weakened. Live project/task/E2E fixtures now +use the real routes. Historical artifact fixtures remain explicitly +`legacy_incomplete`. No workflow, lane, threshold, or failure behavior changed. + +## Reviewer results + +Architecture, security, product/operations, docs, and CI integrity passed. QA, +senior engineering, reuse/dedup, and test-delta passed with low non-blocking +risks. Every blocking first-round finding was fixed and re-reviewed. + +## External review + +GitHub `Backend / test`, `Agent Gates / agent-gates`, and CodeRabbit must pass +on the exact final head. Valid findings must be corrected before human merge. + +## Remaining risks and follow-up + +The API may later expose the opaque replacement selector as a response ETag. +The next REV/AUTH lifecycle chunk remains separate and requires a new explicit +start after this PR is human-merged. + +## Human review focus + +Confirm replay-before-PREP ordering, exact successor/predecessor custody, +Project Manager scope, append-only behavior, denial side-effect ordering, and +the absence of review/revision lifecycle activation. + +## Human merge ownership + +Only the human may merge this PR. diff --git a/backend/alembic/versions/0048_review_revision_policy_authority.py b/backend/alembic/versions/0048_review_revision_policy_authority.py new file mode 100644 index 000000000..1528e8655 --- /dev/null +++ b/backend/alembic/versions/0048_review_revision_policy_authority.py @@ -0,0 +1,402 @@ +"""activate authorized review and revision policy mutation + +Revision ID: 0048_policy_authority +Revises: 0047_policy_identity_lineage +Create Date: 2026-08-02 +""" + +from __future__ import annotations + +from alembic import op +import sqlalchemy as sa + + +revision = "0048_policy_authority" +down_revision = "0047_policy_identity_lineage" +branch_labels = depends_on = None + + +_PROVENANCE_COLUMNS = ( + ("predecessor_policy_hash", sa.String(71)), + ("created_by_actor_profile_id", sa.String(36)), + ("created_via_identity_link_id", sa.String(36)), + ("created_by_admin_role_grant_id", sa.Uuid()), + ("creation_scope_type", sa.String(16)), + ("creation_scope_project_id", sa.String(36)), + ("creation_action_id", sa.String(160)), + ("authorization_decision_event_id", sa.String(36)), +) + + +def _add_policy_authority(table: str, kind: str) -> None: + for name, type_ in _PROVENANCE_COLUMNS: + op.add_column(table, sa.Column(name, type_)) + op.create_foreign_key( + f"fk_{table}_actor_profile", + table, + "actor_profiles", + ["created_by_actor_profile_id"], + ["id"], + ) + op.create_foreign_key( + f"fk_{table}_identity_link", + table, + "actor_identity_links", + ["created_via_identity_link_id"], + ["id"], + ) + op.create_foreign_key( + f"fk_{table}_admin_grant", + table, + "admin_role_grants", + ["created_by_admin_role_grant_id"], + ["id"], + ) + op.create_foreign_key( + f"fk_{table}_decision_event", + table, + "audit_events", + ["authorization_decision_event_id"], + ["id"], + ) + op.create_check_constraint( + f"{kind}_policy_predecessor_shape", + table, + "(supersedes_policy_id is null and predecessor_policy_hash is null and " + "policy_generation = 1) or (supersedes_policy_id is not null and " + "predecessor_policy_hash ~ '^sha256:[0-9a-f]{64}$' and policy_generation > 1) " + "or semantics_status='legacy_incomplete'", + ) + op.create_check_constraint( + f"{kind}_policy_authority_shape", + table, + "semantics_status='legacy_incomplete' or " + "(created_by_actor_profile_id is not null and " + "created_via_identity_link_id is not null and " + "created_by_admin_role_grant_id is not null and " + "creation_scope_type in ('system','project') and " + f"creation_action_id='project.{kind}_policy.update' and " + "authorization_decision_event_id is not null)", + ) + + +def upgrade() -> None: + """Install one replay ledger and complete policy mutation provenance.""" + op.drop_constraint("policy_selection_shape", "project_guides", type_="check") + op.create_check_constraint( + "policy_selection_shape", + "project_guides", + "((selected_review_policy_id is null and " + "selected_review_policy_generation is null and selected_review_policy_hash is null) or " + "(selected_review_policy_id is not null and " + "selected_review_policy_generation is not null and " + "selected_review_policy_hash is not null)) and " + "((selected_revision_policy_id is null and " + "selected_revision_policy_generation is null and " + "selected_revision_policy_hash is null) or " + "(selected_revision_policy_id is not null and " + "selected_revision_policy_generation is not null and " + "selected_revision_policy_hash is not null))", + ) + _add_policy_authority("review_policies", "review") + _add_policy_authority("revision_policies", "revision") + op.create_table( + "policy_mutation_idempotency_records", + sa.Column("id", sa.Uuid(), primary_key=True), + sa.Column( + "actor_profile_id", sa.String(36), sa.ForeignKey("actor_profiles.id"), nullable=False + ), + sa.Column( + "identity_link_id", + sa.String(36), + sa.ForeignKey("actor_identity_links.id"), + nullable=False, + ), + sa.Column("action_id", sa.String(160), nullable=False), + sa.Column("idempotency_key", sa.Uuid(), nullable=False), + sa.Column("request_digest", sa.String(71), nullable=False), + sa.Column("policy_hash", sa.String(71), nullable=False), + sa.Column("resource_context_digest", sa.String(71), nullable=False), + sa.Column("operation_id", sa.Uuid(), nullable=False), + sa.Column("project_id", sa.String(36), sa.ForeignKey("projects.id"), nullable=False), + sa.Column("guide_id", sa.String(36), sa.ForeignKey("project_guides.id"), nullable=False), + sa.Column("policy_id", sa.String(36), nullable=False), + sa.Column("policy_generation", sa.Integer(), nullable=False), + sa.Column("status", sa.String(16), nullable=False), + sa.Column("response_json", sa.JSON()), + sa.Column( + "created_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False + ), + sa.Column("committed_at", sa.DateTime(timezone=True)), + sa.UniqueConstraint( + "actor_profile_id", + "action_id", + "idempotency_key", + name="uq_policy_mutation_replay_namespace", + ), + sa.UniqueConstraint("operation_id", name="uq_policy_mutation_operation_identity"), + sa.CheckConstraint( + "action_id in ('project.review_policy.update','project.revision_policy.update')", + name="ck_policy_mutation_action", + ), + sa.CheckConstraint( + "request_digest ~ '^sha256:[0-9a-f]{64}$' and " + "policy_hash ~ '^sha256:[0-9a-f]{64}$' and " + "resource_context_digest ~ '^sha256:[0-9a-f]{64}$'", + name="ck_policy_mutation_digests", + ), + sa.CheckConstraint("policy_generation > 0", name="ck_policy_mutation_generation"), + sa.CheckConstraint("status in ('pending','committed')", name="ck_policy_mutation_status"), + sa.CheckConstraint( + "(status='pending' and response_json is null and committed_at is null) or " + "(status='committed' and response_json is not null and committed_at is not null)", + name="ck_policy_mutation_state_shape", + ), + ) + op.execute( + """ + create function guard_policy_mutation_replay() returns trigger language plpgsql as $$ + begin + if tg_op='INSERT' then + if new.status<>'pending' then + raise exception 'policy mutation must begin pending' using errcode='23514'; + end if; + return new; + elsif tg_op='DELETE' then + raise exception 'policy mutation replay is immutable' using errcode='55000'; + elsif new is not distinct from old then + return new; + elsif old.status='pending' and new.status='committed' + and (new.id,new.actor_profile_id,new.identity_link_id,new.action_id, + new.idempotency_key,new.request_digest,new.policy_hash, + new.resource_context_digest, + new.operation_id,new.project_id,new.guide_id,new.policy_id, + new.policy_generation) + is not distinct from + (old.id,old.actor_profile_id,old.identity_link_id,old.action_id, + old.idempotency_key,old.request_digest,old.policy_hash, + old.resource_context_digest, + old.operation_id,old.project_id,old.guide_id,old.policy_id, + old.policy_generation) then + return new; + end if; + raise exception 'policy mutation replay is immutable' using errcode='23514'; + end $$ + """ + ) + op.execute( + "create trigger policy_mutation_replay_immutable before insert or update or delete " + "on policy_mutation_idempotency_records for each row " + "execute function guard_policy_mutation_replay()" + ) + op.execute( + """ + create function reject_policy_mutation_replay_truncate() returns trigger + language plpgsql as $$ begin + raise exception 'policy mutation replay is immutable' using errcode='55000'; + end $$ + """ + ) + op.execute( + "create trigger policy_mutation_replay_reject_truncate before truncate " + "on policy_mutation_idempotency_records " + "execute function reject_policy_mutation_replay_truncate()" + ) + op.execute( + """ + create function validate_policy_mutation_custody() returns trigger language plpgsql as $$ + declare reservation policy_mutation_idempotency_records%rowtype; + evidence audit_events%rowtype; + actor_id text; link_id text; grant_id uuid; action_value text; + scope_type text; scope_project text; decision_id text; + product_project text; product_guide text; product_id text; + product_generation integer; + product_hash text; predecessor_id text; predecessor_hash text; + selector_id text; selector_generation integer; selector_hash text; + predecessor_valid boolean; + begin + if tg_table_name='policy_mutation_idempotency_records' then + select * into reservation from policy_mutation_idempotency_records where id=new.id; + if reservation.status<>'committed' then + raise exception 'pending policy mutation custody cannot commit' using errcode='23514'; + end if; + if reservation.action_id='project.review_policy.update' then + select created_by_actor_profile_id,created_via_identity_link_id, + created_by_admin_role_grant_id,creation_action_id, + creation_scope_type,creation_scope_project_id, + authorization_decision_event_id,p.project_id,g.id,p.id, + p.policy_generation,p.policy_hash,p.supersedes_policy_id, + p.predecessor_policy_hash,g.selected_review_policy_id, + g.selected_review_policy_generation,g.selected_review_policy_hash + into actor_id,link_id,grant_id,action_value,scope_type,scope_project, + decision_id,product_project,product_guide,product_id,product_generation, + product_hash,predecessor_id,predecessor_hash,selector_id, + selector_generation,selector_hash + from review_policies p join project_guides g + on g.project_id=p.project_id and g.version=p.guide_version + where p.id=reservation.policy_id and g.id=reservation.guide_id; + else + select created_by_actor_profile_id,created_via_identity_link_id, + created_by_admin_role_grant_id,creation_action_id, + creation_scope_type,creation_scope_project_id, + authorization_decision_event_id,p.project_id,g.id,p.id, + p.policy_generation,p.policy_hash,p.supersedes_policy_id, + p.predecessor_policy_hash,g.selected_revision_policy_id, + g.selected_revision_policy_generation,g.selected_revision_policy_hash + into actor_id,link_id,grant_id,action_value,scope_type,scope_project, + decision_id,product_project,product_guide,product_id,product_generation, + product_hash,predecessor_id,predecessor_hash,selector_id, + selector_generation,selector_hash + from revision_policies p join project_guides g + on g.project_id=p.project_id and g.version=p.guide_version + where p.id=reservation.policy_id and g.id=reservation.guide_id; + end if; + else + actor_id:=new.created_by_actor_profile_id; + link_id:=new.created_via_identity_link_id; + grant_id:=new.created_by_admin_role_grant_id; + action_value:=new.creation_action_id; + scope_type:=new.creation_scope_type; + scope_project:=new.creation_scope_project_id; + decision_id:=new.authorization_decision_event_id; + product_project:=new.project_id; product_id:=new.id; + product_generation:=new.policy_generation; product_hash:=new.policy_hash; + predecessor_id:=new.supersedes_policy_id; + predecessor_hash:=new.predecessor_policy_hash; + if tg_table_name='review_policies' then + select g.id,g.selected_review_policy_id,g.selected_review_policy_generation, + g.selected_review_policy_hash + into product_guide,selector_id,selector_generation,selector_hash + from project_guides g + where g.project_id=new.project_id and g.version=new.guide_version; + else + select g.id,g.selected_revision_policy_id,g.selected_revision_policy_generation, + g.selected_revision_policy_hash + into product_guide,selector_id,selector_generation,selector_hash + from project_guides g + where g.project_id=new.project_id and g.version=new.guide_version; + end if; + select r.* into reservation from policy_mutation_idempotency_records r + where r.policy_id=new.id and r.action_id=new.creation_action_id + and r.policy_generation=new.policy_generation and r.status='committed'; + end if; + if reservation.id is null or product_id is null + or reservation.actor_profile_id is distinct from actor_id + or reservation.identity_link_id is distinct from link_id + or reservation.action_id is distinct from action_value + or reservation.project_id is distinct from product_project + or reservation.guide_id is distinct from product_guide + or reservation.policy_id is distinct from product_id + or reservation.policy_generation is distinct from product_generation + or reservation.policy_hash is distinct from product_hash + or selector_id is distinct from product_id + or selector_generation is distinct from product_generation + or selector_hash is distinct from product_hash + or scope_type not in ('system','project') + or (scope_type='project' and scope_project is distinct from product_project) + or (scope_type='system' and scope_project is not null) then + raise exception 'policy mutation custody mismatch' using errcode='23514'; + end if; + if product_generation=1 then + predecessor_valid:=predecessor_id is null and predecessor_hash is null; + elsif reservation.action_id='project.review_policy.update' then + select exists(select 1 from review_policies prior + where prior.id=predecessor_id and prior.project_id=product_project + and prior.guide_version=(select version from project_guides where id=product_guide) + and prior.policy_generation=product_generation-1 + and prior.policy_hash=predecessor_hash) into predecessor_valid; + else + select exists(select 1 from revision_policies prior + where prior.id=predecessor_id and prior.project_id=product_project + and prior.guide_version=(select version from project_guides where id=product_guide) + and prior.policy_generation=product_generation-1 + and prior.policy_hash=predecessor_hash) into predecessor_valid; + end if; + if predecessor_valid is not true then + raise exception 'policy mutation lineage mismatch' using errcode='23514'; + end if; + select * into evidence from audit_events where id=decision_id; + if evidence.id is null or evidence.event_domain is distinct from 'authority' + or evidence.event_type is distinct from 'SensitiveAuthorizationAllowed' + or evidence.denial_code is not null + or evidence.actor_ref_kind is distinct from 'actor_profile' + or evidence.actor_id is distinct from actor_id + or evidence.matched_grant_id is distinct from grant_id::text + or evidence.permission_id is distinct from 'project.review_policy.manage' + or evidence.action_id is distinct from action_value + or evidence.resource_type is distinct from 'project' + or evidence.resource_id is distinct from product_project + or evidence.target_ref_kind is distinct from 'project' + or evidence.target_ref_id is distinct from product_project + or evidence.after_facts->>'allowed' is distinct from 'true' + or evidence.after_facts->>'resource_context_digest' + is distinct from reservation.resource_context_digest then + raise exception 'policy mutation evidence mismatch' using errcode='23514'; + end if; + return null; + end $$ + """ + ) + for name, table in ( + ("review_policy_mutation_custody", "review_policies"), + ("revision_policy_mutation_custody", "revision_policies"), + ("policy_mutation_replay_custody", "policy_mutation_idempotency_records"), + ): + op.execute( + f"create constraint trigger {name} after insert or update on {table} " + "deferrable initially deferred for each row " + "execute function validate_policy_mutation_custody()" + ) + + +def downgrade() -> None: + """Remove 02B authority state while preserving 02A policy lineage.""" + bind = op.get_bind() + has_custody = bool( + bind.scalar( + sa.text( + "select exists(select 1 from policy_mutation_idempotency_records) or " + "exists(select 1 from review_policies where creation_action_id=" + "'project.review_policy.update') or exists(select 1 from revision_policies " + "where creation_action_id='project.revision_policy.update')" + ) + ) + ) + if has_custody: + raise RuntimeError("cannot downgrade populated policy mutation authority") + for name, table in ( + ("review_policy_mutation_custody", "review_policies"), + ("revision_policy_mutation_custody", "revision_policies"), + ("policy_mutation_replay_custody", "policy_mutation_idempotency_records"), + ): + op.execute(f"drop trigger {name} on {table}") + op.execute("drop function validate_policy_mutation_custody()") + op.execute( + "drop trigger policy_mutation_replay_reject_truncate on policy_mutation_idempotency_records" + ) + op.execute("drop function reject_policy_mutation_replay_truncate()") + op.execute( + "drop trigger policy_mutation_replay_immutable on policy_mutation_idempotency_records" + ) + op.execute("drop function guard_policy_mutation_replay()") + op.drop_table("policy_mutation_idempotency_records") + for table, kind in (("revision_policies", "revision"), ("review_policies", "review")): + op.drop_constraint(f"{kind}_policy_authority_shape", table, type_="check") + op.drop_constraint(f"{kind}_policy_predecessor_shape", table, type_="check") + for suffix in ("decision_event", "admin_grant", "identity_link", "actor_profile"): + op.drop_constraint(f"fk_{table}_{suffix}", table, type_="foreignkey") + for name, _type in reversed(_PROVENANCE_COLUMNS): + op.drop_column(table, name) + op.drop_constraint("policy_selection_shape", "project_guides", type_="check") + op.create_check_constraint( + "policy_selection_shape", + "project_guides", + "(selected_review_policy_id is null and selected_review_policy_generation is null " + "and selected_review_policy_hash is null and selected_revision_policy_id is null " + "and selected_revision_policy_generation is null and " + "selected_revision_policy_hash is null) or (selected_review_policy_id is not null " + "and selected_review_policy_generation is not null and " + "selected_review_policy_hash is not null and selected_revision_policy_id is not null " + "and selected_revision_policy_generation is not null and " + "selected_revision_policy_hash is not null)", + ) diff --git a/backend/app/api/router.py b/backend/app/api/router.py index 0a891d92d..92a61445c 100644 --- a/backend/app/api/router.py +++ b/backend/app/api/router.py @@ -12,6 +12,7 @@ from app.modules.projects.router import router as projects_router from app.modules.projects.create_router import router as project_create_router from app.modules.projects.guide_mutation_router import router as guide_mutation_router +from app.modules.projects.policy_mutation_router import router as policy_mutation_router from app.modules.tasks.router import router as tasks_router api_router = APIRouter() @@ -22,6 +23,7 @@ api_router.include_router(authorization_router, prefix="/api/v1") api_router.include_router(project_create_router, prefix="/api/v1") api_router.include_router(guide_mutation_router, prefix="/api/v1") +api_router.include_router(policy_mutation_router, prefix="/api/v1") api_router.include_router(projects_router, prefix="/api/v1") api_router.include_router(tasks_router, prefix="/api/v1") api_router.include_router(checkers_router, prefix="/api/v1") diff --git a/backend/app/modules/authorization/catalogue.py b/backend/app/modules/authorization/catalogue.py index c637edb62..1b5114d08 100644 --- a/backend/app/modules/authorization/catalogue.py +++ b/backend/app/modules/authorization/catalogue.py @@ -214,7 +214,7 @@ class ActionOwner(StrEnum): AUTH_12B2 = "WS-AUTH-001-12B2" AUTH_12C = "WS-AUTH-001-12C" AUTH_12D = "WS-AUTH-001-12D" - AUTH_12D2 = "WS-AUTH-001-12D2" + XINT_003_02B = "WS-XINT-003-02B" AUTH_12E = "WS-AUTH-001-12E" AUTH_12F = "WS-AUTH-001-12F" AUTH_12G = "WS-AUTH-001-12G" @@ -450,15 +450,15 @@ def _active( PermissionId.PROJECT_GUIDE_MANAGE, ActionOwner.AUTH_12D, ), - _planned( + _active( ActionId.PROJECT_REVIEW_POLICY_UPDATE, PermissionId.PROJECT_REVIEW_POLICY_MANAGE, - ActionOwner.AUTH_12D2, + ActionOwner.XINT_003_02B, ), - _planned( + _active( ActionId.PROJECT_REVISION_POLICY_UPDATE, PermissionId.PROJECT_REVIEW_POLICY_MANAGE, - ActionOwner.AUTH_12D2, + ActionOwner.XINT_003_02B, ), _planned( ActionId.PROJECT_GUIDE_SUFFICIENCY_REPORT_CREATE, @@ -800,6 +800,8 @@ def _index_actions( ActionId.PROJECT_GUIDE_CREATE, ActionId.PROJECT_GUIDE_UPDATE, ActionId.PROJECT_GUIDE_SOURCE_SNAPSHOT_CREATE, + ActionId.PROJECT_REVIEW_POLICY_UPDATE, + ActionId.PROJECT_REVISION_POLICY_UPDATE, ActionId.PROJECT_READ, ActionId.ACTOR_AUTHORIZATION_CONTEXT_READ, ActionId.PROJECT_SETUP_RUN_READ, diff --git a/backend/app/modules/authorization/kernel.py b/backend/app/modules/authorization/kernel.py index 106a91b65..3bb49e0a5 100644 --- a/backend/app/modules/authorization/kernel.py +++ b/backend/app/modules/authorization/kernel.py @@ -69,6 +69,7 @@ ProjectReadResourceContext, ProjectDiagnosticReadResourceContext, ProjectPolicyReadResourceContext, + ProjectPolicyMutationPrepareDenialResourceContext, ProjectActiveGuideReadResourceContext, ProjectRoleGrantCollectionResourceContext, ProjectRoleGrantIssueResourceContext, @@ -92,11 +93,13 @@ Awaitable[HumanAuthorizationContext], ] -_GUIDE_METADATA_MUTATIONS = frozenset( +_GUIDE_BOUND_PROJECT_MANAGER_MUTATIONS = frozenset( { ActionId.PROJECT_GUIDE_CREATE, ActionId.PROJECT_GUIDE_UPDATE, ActionId.PROJECT_GUIDE_SOURCE_SNAPSHOT_CREATE, + ActionId.PROJECT_REVIEW_POLICY_UPDATE, + ActionId.PROJECT_REVISION_POLICY_UPDATE, } ) @@ -293,9 +296,7 @@ def __init__( object.__setattr__(self, "action_id", action_id) object.__setattr__(self, "scope_project_id", scope_project_id) object.__setattr__(self, "matched_grant_id", matched_grant_id) - object.__setattr__( - self, "matched_grant_scope_project_id", matched_grant_scope_project_id - ) + object.__setattr__(self, "matched_grant_scope_project_id", matched_grant_scope_project_id) object.__setattr__(self, "matched_grant_status", matched_grant_status) object.__setattr__(self, "permission_id", permission_id) object.__setattr__(self, "artifact_resource_type", artifact_resource_type) @@ -481,7 +482,7 @@ async def _prepare_prelocked( raise PreparedAuthorizationUnsupported( AuthorizationDenialCode.PERMISSION_NOT_GRANTED ) - elif action_id in _GUIDE_METADATA_MUTATIONS: + elif action_id in _GUIDE_BOUND_PROJECT_MANAGER_MUTATIONS: if ( not isinstance(context, HumanAuthorizationContext) or scope.kind is not PreparedAuthorityScopeKind.PROJECT @@ -636,11 +637,22 @@ async def _complete_prepared_denial( self._validate_prepared_consumer(consumer_token) action = ACTION_BY_ID.get(action_id) supported = ( - action_id is ActionId.PROJECT_CREATE - and isinstance(resource_context, ProjectCreateResourceContext) - ) or ( - action_id in _GUIDE_METADATA_MUTATIONS - and isinstance(resource_context, ProjectGuideMutationPrepareDenialResourceContext) + ( + action_id is ActionId.PROJECT_CREATE + and isinstance(resource_context, ProjectCreateResourceContext) + ) + or ( + action_id in _GUIDE_BOUND_PROJECT_MANAGER_MUTATIONS + and isinstance(resource_context, ProjectGuideMutationPrepareDenialResourceContext) + ) + or ( + action_id + in { + ActionId.PROJECT_REVIEW_POLICY_UPDATE, + ActionId.PROJECT_REVISION_POLICY_UPDATE, + } + and isinstance(resource_context, ProjectPolicyMutationPrepareDenialResourceContext) + ) ) if not supported: raise TypeError("unsupported prepared denial") @@ -962,7 +974,7 @@ async def _require_prelocked( matched_kind = MatchedAuthorityKind.ADMIN_ROLE_GRANT matched_grant_id = authority.matched_grant_id matched_project_id = authority.scope_project_id - elif action_id in _GUIDE_METADATA_MUTATIONS: + elif action_id in _GUIDE_BOUND_PROJECT_MANAGER_MUTATIONS: denial = self._lifecycle_denial(context) expected = PROJECT_MUTATION_RESOURCE_BY_ACTION.get(action_id) if denial is None and action.availability is not ActionAvailability.ACTIVE: @@ -1426,7 +1438,7 @@ async def _stage_decision( audit_resource_id = str(resource_context.resource_id) target_ref_kind = "project" target_ref_id = str(resource_context.requested_project_id) - elif decision.action_id in _GUIDE_METADATA_MUTATIONS: + elif decision.action_id in _GUIDE_BOUND_PROJECT_MANAGER_MUTATIONS: if resource_context is not None: project_id = self._resource_project_id(resource_context) if project_id is not None: @@ -1448,7 +1460,7 @@ async def _stage_decision( } or decision.action_id in { ActionId.ARTIFACT_GUIDE_SOURCE_INGEST, ActionId.PROJECT_CREATE, - *_GUIDE_METADATA_MUTATIONS, + *_GUIDE_BOUND_PROJECT_MANAGER_MUTATIONS, }: after_facts["resource_context_digest"] = decision.resource_context_digest try: diff --git a/backend/app/modules/authorization/prepared.py b/backend/app/modules/authorization/prepared.py index 23d7f8383..abdbdb2e3 100644 --- a/backend/app/modules/authorization/prepared.py +++ b/backend/app/modules/authorization/prepared.py @@ -39,6 +39,9 @@ ProjectGuideMutationResourceContext, ProjectGuideMutationPrepareDenialResourceContext, ProjectGuideSourceSnapshotMutationResourceContext, + ProjectPolicyMutationPrepareDenialResourceContext, + ProjectReviewPolicyMutationResourceContext, + ProjectRevisionPolicyMutationResourceContext, ) @@ -94,6 +97,17 @@ class _PreparedAuthorizationBinding: guide_mutation_guide_id: UUID | None = None guide_mutation_target_resource_id: UUID | None = None guide_mutation_operation_id: UUID | None = None + policy_mutation_project_id: UUID | None = None + policy_mutation_guide_id: UUID | None = None + policy_mutation_policy_id: UUID | None = None + policy_mutation_operation_id: UUID | None = None + policy_mutation_request_digest: str | None = None + policy_mutation_policy_digest: str | None = None + policy_mutation_generation: int | None = None + policy_mutation_predecessor_id: UUID | None = None + policy_mutation_predecessor_generation: int | None = None + policy_mutation_predecessor_digest: str | None = None + policy_mutation_guide_status: str | None = None @dataclass(slots=True) @@ -135,6 +149,45 @@ def _guide_mutation_binding_matches( ) +def _policy_mutation_binding_matches( + binding: _PreparedAuthorizationBinding, + resource: ProjectReviewPolicyMutationResourceContext + | ProjectRevisionPolicyMutationResourceContext, +) -> bool: + """Return whether final policy lineage matches prepared route facts.""" + return ( + binding.policy_mutation_project_id == resource.scope_project_id + and binding.policy_mutation_guide_id == resource.guide_id + and binding.policy_mutation_policy_id == resource.resource_id + and binding.policy_mutation_operation_id == resource.operation_id + and binding.policy_mutation_request_digest == resource.request_digest + and binding.policy_mutation_policy_digest == resource.policy_digest + and binding.policy_mutation_generation == resource.policy_generation + and binding.policy_mutation_predecessor_id == resource.predecessor_policy_id + and binding.policy_mutation_predecessor_generation == resource.predecessor_policy_generation + and binding.policy_mutation_predecessor_digest == resource.current_policy_digest + and binding.policy_mutation_guide_status == resource.guide_status + ) + + +def _policy_mutation_denial_binding_matches( + binding: _PreparedAuthorizationBinding, + resource: ProjectPolicyMutationPrepareDenialResourceContext, +) -> bool: + """Return whether bounded denial facts match the prepared policy request.""" + expected_action = ( + ActionId.PROJECT_REVIEW_POLICY_UPDATE + if resource.requested_policy_kind == "review" + else ActionId.PROJECT_REVISION_POLICY_UPDATE + ) + return ( + binding.action_id is expected_action + and binding.policy_mutation_project_id == resource.scope_project_id + and binding.policy_mutation_guide_id == resource.requested_guide_id + and binding.policy_mutation_request_digest == resource.request_digest + ) + + _CONSUMED = _Consumed() @@ -211,9 +264,20 @@ async def consume( raise PreparedAuthorizationHandleInvalid("invalid prepared authorization handle") if isinstance( final_resource_context, - (ProjectGuideMutationResourceContext, ProjectGuideSourceSnapshotMutationResourceContext), + ( + ProjectGuideMutationResourceContext, + ProjectGuideSourceSnapshotMutationResourceContext, + ), ) and not _guide_mutation_binding_matches(issuance.binding, final_resource_context): raise PreparedAuthorizationHandleInvalid("invalid prepared authorization handle") + if isinstance( + final_resource_context, + ( + ProjectReviewPolicyMutationResourceContext, + ProjectRevisionPolicyMutationResourceContext, + ), + ) and not _policy_mutation_binding_matches(issuance.binding, final_resource_context): + raise PreparedAuthorizationHandleInvalid("invalid prepared authorization handle") self._issued[handle] = _CONSUMED return await self._authorization._require_prelocked( self._consumer_token, @@ -240,6 +304,10 @@ async def deny_unsupported( _project_create_binding_matches(binding, final_resource_context) ): raise PreparedAuthorizationHandleInvalid("invalid prepared authorization handle") + if isinstance( + final_resource_context, ProjectPolicyMutationPrepareDenialResourceContext + ) and not _policy_mutation_denial_binding_matches(binding, final_resource_context): + raise PreparedAuthorizationHandleInvalid("invalid prepared authorization handle") await self._authorization._complete_prepared_denial( self._consumer_token, action_id, @@ -285,6 +353,13 @@ def _binding( operation_generation = None guide_mutation_project_id = guide_mutation_guide_id = None guide_mutation_target_resource_id = guide_mutation_operation_id = None + policy_mutation_project_id = policy_mutation_guide_id = None + policy_mutation_policy_id = policy_mutation_operation_id = None + policy_mutation_request_digest = None + policy_mutation_policy_digest = policy_mutation_predecessor_digest = None + policy_mutation_generation = policy_mutation_predecessor_generation = None + policy_mutation_predecessor_id = None + policy_mutation_guide_status = None if action_id is ActionId.PROJECT_CREATE: try: operation_id = UUID(str(caller_input.request_value["operation_id"])) @@ -306,9 +381,7 @@ def _binding( ActionId.PROJECT_GUIDE_SOURCE_SNAPSHOT_CREATE, }: try: - guide_mutation_project_id = UUID( - str(caller_input.request_value["project_id"]) - ) + guide_mutation_project_id = UUID(str(caller_input.request_value["project_id"])) raw_guide_id = caller_input.request_value.get("guide_id") guide_mutation_guide_id = ( UUID(str(raw_guide_id)) if raw_guide_id is not None else None @@ -316,9 +389,7 @@ def _binding( guide_mutation_target_resource_id = UUID( str(caller_input.request_value["target_resource_id"]) ) - guide_mutation_operation_id = UUID( - str(caller_input.request_value["operation_id"]) - ) + guide_mutation_operation_id = UUID(str(caller_input.request_value["operation_id"])) except (KeyError, TypeError, ValueError) as exc: raise PreparedAuthorizationHandleInvalid( "invalid prepared authorization handle" @@ -336,9 +407,40 @@ def _binding( and guide_mutation_target_resource_id != guide_mutation_guide_id ) ): + raise PreparedAuthorizationHandleInvalid("invalid prepared authorization handle") + if action_id in { + ActionId.PROJECT_REVIEW_POLICY_UPDATE, + ActionId.PROJECT_REVISION_POLICY_UPDATE, + }: + try: + policy_mutation_project_id = UUID(str(caller_input.request_value["project_id"])) + policy_mutation_guide_id = UUID(str(caller_input.request_value["guide_id"])) + policy_mutation_policy_id = UUID(str(caller_input.request_value["policy_id"])) + policy_mutation_operation_id = UUID(str(caller_input.request_value["operation_id"])) + policy_mutation_request_digest = str(caller_input.request_value["request_digest"]) + policy_mutation_policy_digest = str(caller_input.request_value["policy_digest"]) + policy_mutation_generation = int(caller_input.request_value["policy_generation"]) + raw_predecessor_id = caller_input.request_value["predecessor_policy_id"] + policy_mutation_predecessor_id = ( + UUID(str(raw_predecessor_id)) if raw_predecessor_id is not None else None + ) + raw_predecessor_generation = caller_input.request_value[ + "predecessor_policy_generation" + ] + policy_mutation_predecessor_generation = ( + int(raw_predecessor_generation) + if raw_predecessor_generation is not None + else None + ) + raw_predecessor_digest = caller_input.request_value["predecessor_policy_digest"] + policy_mutation_predecessor_digest = ( + str(raw_predecessor_digest) if raw_predecessor_digest is not None else None + ) + policy_mutation_guide_status = str(caller_input.request_value["guide_status"]) + except (KeyError, TypeError, ValueError) as exc: raise PreparedAuthorizationHandleInvalid( "invalid prepared authorization handle" - ) + ) from exc return _PreparedAuthorizationBinding( action_id=action_id, actor_ref_kind=ActorReferenceKind.ACTOR_PROFILE, @@ -358,6 +460,17 @@ def _binding( guide_mutation_guide_id=guide_mutation_guide_id, guide_mutation_target_resource_id=guide_mutation_target_resource_id, guide_mutation_operation_id=guide_mutation_operation_id, + policy_mutation_project_id=policy_mutation_project_id, + policy_mutation_guide_id=policy_mutation_guide_id, + policy_mutation_policy_id=policy_mutation_policy_id, + policy_mutation_operation_id=policy_mutation_operation_id, + policy_mutation_request_digest=policy_mutation_request_digest, + policy_mutation_policy_digest=policy_mutation_policy_digest, + policy_mutation_generation=policy_mutation_generation, + policy_mutation_predecessor_id=policy_mutation_predecessor_id, + policy_mutation_predecessor_generation=(policy_mutation_predecessor_generation), + policy_mutation_predecessor_digest=policy_mutation_predecessor_digest, + policy_mutation_guide_status=policy_mutation_guide_status, ) @staticmethod @@ -366,10 +479,7 @@ def _scope_from_resource( resource: AuthorizationResourceContext, ) -> PreparedAuthorityScope: guide_resource = _GUIDE_RESOURCE_BY_ACTION.get(action_id) - if ( - guide_resource is not None - and isinstance(resource, guide_resource[1]) - ): + if guide_resource is not None and isinstance(resource, guide_resource[1]): return PreparedAuthorityScope( kind=PreparedAuthorityScopeKind.ARTIFACT_INTERNAL, artifact_resource_type=guide_resource[0], @@ -415,6 +525,14 @@ def _scope_from_resource( kind=PreparedAuthorityScopeKind.PROJECT, project_id=resource.scope_project_id, ) + if action_id in { + ActionId.PROJECT_REVIEW_POLICY_UPDATE, + ActionId.PROJECT_REVISION_POLICY_UPDATE, + } and isinstance(resource, ProjectPolicyMutationPrepareDenialResourceContext): + return PreparedAuthorityScope( + kind=PreparedAuthorityScopeKind.PROJECT, + project_id=resource.scope_project_id, + ) if expected_project_mutation is not None and isinstance( resource, expected_project_mutation ): diff --git a/backend/app/modules/authorization/runtime.py b/backend/app/modules/authorization/runtime.py index f1c5cf2f4..d3c7d58f4 100644 --- a/backend/app/modules/authorization/runtime.py +++ b/backend/app/modules/authorization/runtime.py @@ -563,11 +563,17 @@ class ProjectReviewPolicyMutationResourceContext(BaseModel): resource_type: Literal["project_review_policy_mutation"] resource_id: UUID + operation_id: UUID + request_digest: str = Field(pattern=r"^sha256:[0-9a-f]{64}$") scope_project_id: UUID guide_id: UUID guide_version: str + guide_status: Literal["draft"] review_policy_id: UUID policy_generation: int = Field(ge=1) + policy_digest: str = Field(pattern=r"^sha256:[0-9a-f]{64}$") + predecessor_policy_id: UUID | None = None + predecessor_policy_generation: int | None = Field(default=None, ge=1) current_policy_digest: str | None = Field(default=None, pattern=r"^sha256:[0-9a-f]{64}$") @model_validator(mode="after") @@ -575,6 +581,26 @@ def require_review_policy_identity(self): """Bind the resource selector to the review policy only.""" if self.resource_id != self.review_policy_id: raise ValueError("review policy resource must match policy") + if ( + len( + { + self.predecessor_policy_id is None, + self.predecessor_policy_generation is None, + self.current_policy_digest is None, + } + ) + != 1 + ): + raise ValueError("review policy predecessor facts must be bound together") + if self.policy_generation == 1 and self.predecessor_policy_id is not None: + raise ValueError("first review policy cannot have a predecessor") + if self.policy_generation > 1 and self.predecessor_policy_id is None: + raise ValueError("replacement review policy requires a predecessor") + if ( + self.predecessor_policy_generation is not None + and self.policy_generation != self.predecessor_policy_generation + 1 + ): + raise ValueError("review policy successor generation must be exact") return self @@ -585,11 +611,17 @@ class ProjectRevisionPolicyMutationResourceContext(BaseModel): resource_type: Literal["project_revision_policy_mutation"] resource_id: UUID + operation_id: UUID + request_digest: str = Field(pattern=r"^sha256:[0-9a-f]{64}$") scope_project_id: UUID guide_id: UUID guide_version: str + guide_status: Literal["draft"] revision_policy_id: UUID policy_generation: int = Field(ge=1) + policy_digest: str = Field(pattern=r"^sha256:[0-9a-f]{64}$") + predecessor_policy_id: UUID | None = None + predecessor_policy_generation: int | None = Field(default=None, ge=1) current_policy_digest: str | None = Field(default=None, pattern=r"^sha256:[0-9a-f]{64}$") @model_validator(mode="after") @@ -597,6 +629,45 @@ def require_revision_policy_identity(self): """Bind the resource selector to the revision policy only.""" if self.resource_id != self.revision_policy_id: raise ValueError("revision policy resource must match policy") + if ( + len( + { + self.predecessor_policy_id is None, + self.predecessor_policy_generation is None, + self.current_policy_digest is None, + } + ) + != 1 + ): + raise ValueError("revision policy predecessor facts must be bound together") + if self.policy_generation == 1 and self.predecessor_policy_id is not None: + raise ValueError("first revision policy cannot have a predecessor") + if self.policy_generation > 1 and self.predecessor_policy_id is None: + raise ValueError("replacement revision policy requires a predecessor") + if ( + self.predecessor_policy_generation is not None + and self.policy_generation != self.predecessor_policy_generation + 1 + ): + raise ValueError("revision policy successor generation must be exact") + return self + + +class ProjectPolicyMutationPrepareDenialResourceContext(BaseModel): + """Privacy-bounded requested selectors for policy PREP denial evidence.""" + + model_config = _STRICT_FROZEN + + resource_type: Literal["project_policy_mutation_request"] + resource_id: UUID + scope_project_id: UUID + requested_guide_id: UUID + requested_policy_kind: Literal["review", "revision"] + request_digest: str = Field(pattern=r"^sha256:[0-9a-f]{64}$") + + @model_validator(mode="after") + def require_requested_guide(self): + if self.resource_id != self.requested_guide_id: + raise ValueError("policy mutation denial must identify the requested guide") return self @@ -1385,6 +1456,9 @@ class AuthorizationDecision(BaseModel): "project_guide_mutation", "project_guide_source_snapshot_mutation", "project_guide_mutation_request", + "project_review_policy_mutation", + "project_revision_policy_mutation", + "project_policy_mutation_request", "actor_identity_link", "system", "permission_catalogue", diff --git a/backend/app/modules/projects/models.py b/backend/app/modules/projects/models.py index 71c00ad8d..49ed6d102 100644 --- a/backend/app/modules/projects/models.py +++ b/backend/app/modules/projects/models.py @@ -169,6 +169,56 @@ class GuideMutationIdempotencyRecord(Base): committed_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) +class PolicyMutationIdempotencyRecord(Base): + """Replay custody for one guide-bound policy replacement.""" + + __tablename__ = "policy_mutation_idempotency_records" + __table_args__ = ( + UniqueConstraint( + "actor_profile_id", + "action_id", + "idempotency_key", + name="uq_policy_mutation_replay_namespace", + ), + UniqueConstraint("operation_id", name="uq_policy_mutation_operation_identity"), + CheckConstraint( + "action_id in ('project.review_policy.update','project.revision_policy.update')", + name="ck_policy_mutation_action", + ), + CheckConstraint( + "request_digest ~ '^sha256:[0-9a-f]{64}$' and " + "policy_hash ~ '^sha256:[0-9a-f]{64}$' and " + "resource_context_digest ~ '^sha256:[0-9a-f]{64}$'", + name="ck_policy_mutation_digests", + ), + CheckConstraint("policy_generation > 0", name="ck_policy_mutation_generation"), + CheckConstraint("status in ('pending','committed')", name="ck_policy_mutation_status"), + CheckConstraint( + "(status='pending' and response_json is null and committed_at is null) or " + "(status='committed' and response_json is not null and committed_at is not null)", + name="ck_policy_mutation_state_shape", + ), + ) + + id: Mapped[UUID] = mapped_column(Uuid(), primary_key=True) + actor_profile_id: Mapped[str] = mapped_column(ForeignKey("actor_profiles.id")) + identity_link_id: Mapped[str] = mapped_column(ForeignKey("actor_identity_links.id")) + action_id: Mapped[str] = mapped_column(String(160), nullable=False) + idempotency_key: Mapped[UUID] = mapped_column(Uuid(), nullable=False) + request_digest: Mapped[str] = mapped_column(String(71), nullable=False) + policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) + resource_context_digest: Mapped[str] = mapped_column(String(71), nullable=False) + operation_id: Mapped[UUID] = mapped_column(Uuid(), nullable=False) + project_id: Mapped[str] = mapped_column(ForeignKey("projects.id"), nullable=False) + guide_id: Mapped[str] = mapped_column(ForeignKey("project_guides.id"), nullable=False) + policy_id: Mapped[str] = mapped_column(String(36), nullable=False) + policy_generation: Mapped[int] = mapped_column(Integer, nullable=False) + status: Mapped[str] = mapped_column(String(16), nullable=False, default="pending") + response_json: Mapped[dict | None] = mapped_column(JSON) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) + committed_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + + class ProjectGuide(Base): """Versioned human-facing project guide material.""" @@ -176,17 +226,17 @@ class ProjectGuide(Base): __table_args__ = ( UniqueConstraint("project_id", "version", name="uq_project_guides_project_version"), CheckConstraint( - "(selected_review_policy_id is null and " + "((selected_review_policy_id is null and " "selected_review_policy_generation is null and selected_review_policy_hash is null " - "and selected_revision_policy_id is null and " + ") or (selected_review_policy_id is not null and " + "selected_review_policy_generation is not null and " + "selected_review_policy_hash is not null)) and " + "((selected_revision_policy_id is null and " "selected_revision_policy_generation is null and " "selected_revision_policy_hash is null) or " - "(selected_review_policy_id is not null and " - "selected_review_policy_generation is not null and " - "selected_review_policy_hash is not null and " - "selected_revision_policy_id is not null and " + "(selected_revision_policy_id is not null and " "selected_revision_policy_generation is not null and " - "selected_revision_policy_hash is not null)", + "selected_revision_policy_hash is not null))", name="policy_selection_shape", ), CheckConstraint( @@ -440,6 +490,22 @@ class ReviewPolicy(Base): "semantics_status in ('complete','legacy_incomplete')", name="review_policy_identity_shape", ), + CheckConstraint( + "semantics_status='legacy_incomplete' or " + "(created_by_actor_profile_id is not null and " + "created_via_identity_link_id is not null and " + "created_by_admin_role_grant_id is not null and " + "creation_scope_type in ('system','project') and creation_action_id = " + "'project.review_policy.update' and authorization_decision_event_id is not null)", + name="review_policy_authority_shape", + ), + CheckConstraint( + "semantics_status='legacy_incomplete' or " + "((supersedes_policy_id is null and predecessor_policy_hash is null and " + "policy_generation=1) or (supersedes_policy_id is not null and " + "predecessor_policy_hash ~ '^sha256:[0-9a-f]{64}$' and policy_generation>1))", + name="review_policy_predecessor_shape", + ), ) id: Mapped[str] = mapped_column(String(36), primary_key=True) @@ -449,6 +515,20 @@ class ReviewPolicy(Base): policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) semantics_status: Mapped[str] = mapped_column(String(24), nullable=False) supersedes_policy_id: Mapped[str | None] = mapped_column(ForeignKey("review_policies.id")) + predecessor_policy_hash: Mapped[str | None] = mapped_column(String(71)) + created_by_actor_profile_id: Mapped[str | None] = mapped_column(ForeignKey("actor_profiles.id")) + created_via_identity_link_id: Mapped[str | None] = mapped_column( + ForeignKey("actor_identity_links.id") + ) + created_by_admin_role_grant_id: Mapped[UUID | None] = mapped_column( + Uuid(), ForeignKey("admin_role_grants.id") + ) + creation_scope_type: Mapped[str | None] = mapped_column(String(16)) + creation_scope_project_id: Mapped[str | None] = mapped_column(String(36)) + creation_action_id: Mapped[str | None] = mapped_column(String(160)) + authorization_decision_event_id: Mapped[str | None] = mapped_column( + ForeignKey("audit_events.id") + ) review_preference_window_seconds: Mapped[int | None] = mapped_column(Integer) review_lease_duration_seconds: Mapped[int | None] = mapped_column(Integer) max_active_review_leases_per_reviewer: Mapped[int | None] = mapped_column(Integer) @@ -493,6 +573,22 @@ class RevisionPolicy(Base): "semantics_status in ('complete','legacy_incomplete')", name="revision_policy_identity_shape", ), + CheckConstraint( + "semantics_status='legacy_incomplete' or " + "(created_by_actor_profile_id is not null and " + "created_via_identity_link_id is not null and " + "created_by_admin_role_grant_id is not null and " + "creation_scope_type in ('system','project') and creation_action_id = " + "'project.revision_policy.update' and authorization_decision_event_id is not null)", + name="revision_policy_authority_shape", + ), + CheckConstraint( + "semantics_status='legacy_incomplete' or " + "((supersedes_policy_id is null and predecessor_policy_hash is null and " + "policy_generation=1) or (supersedes_policy_id is not null and " + "predecessor_policy_hash ~ '^sha256:[0-9a-f]{64}$' and policy_generation>1))", + name="revision_policy_predecessor_shape", + ), ) id: Mapped[str] = mapped_column(String(36), primary_key=True) @@ -502,6 +598,20 @@ class RevisionPolicy(Base): policy_hash: Mapped[str] = mapped_column(String(71), nullable=False) semantics_status: Mapped[str] = mapped_column(String(24), nullable=False) supersedes_policy_id: Mapped[str | None] = mapped_column(ForeignKey("revision_policies.id")) + predecessor_policy_hash: Mapped[str | None] = mapped_column(String(71)) + created_by_actor_profile_id: Mapped[str | None] = mapped_column(ForeignKey("actor_profiles.id")) + created_via_identity_link_id: Mapped[str | None] = mapped_column( + ForeignKey("actor_identity_links.id") + ) + created_by_admin_role_grant_id: Mapped[UUID | None] = mapped_column( + Uuid(), ForeignKey("admin_role_grants.id") + ) + creation_scope_type: Mapped[str | None] = mapped_column(String(16)) + creation_scope_project_id: Mapped[str | None] = mapped_column(String(36)) + creation_action_id: Mapped[str | None] = mapped_column(String(160)) + authorization_decision_event_id: Mapped[str | None] = mapped_column( + ForeignKey("audit_events.id") + ) max_revision_rounds: Mapped[int] = mapped_column(Integer, nullable=False) revision_deadline_hours: Mapped[int] = mapped_column(Integer, nullable=False) allowed_resubmission_states: Mapped[list[str]] = mapped_column( @@ -786,8 +896,7 @@ class GuideSufficiencyReport(Base): name="ck_guide_sufficiency_reports_generation_positive", ), CheckConstraint( - "agent_material_sha256 is null or " - "agent_material_sha256 ~ '^sha256:[0-9a-f]{64}$'", + "agent_material_sha256 is null or agent_material_sha256 ~ '^sha256:[0-9a-f]{64}$'", name="ck_guide_sufficiency_reports_material_sha256", ), CheckConstraint( diff --git a/backend/app/modules/projects/policy_mutation_replay_repository.py b/backend/app/modules/projects/policy_mutation_replay_repository.py new file mode 100644 index 000000000..d23011a6d --- /dev/null +++ b/backend/app/modules/projects/policy_mutation_replay_repository.py @@ -0,0 +1,111 @@ +"""Replay-only persistence for guide-bound policy mutations.""" + +from __future__ import annotations + +from datetime import UTC, datetime +from uuid import UUID, uuid4 + +from sqlalchemy import select, update +from sqlalchemy.dialects.postgresql import insert +from sqlalchemy.ext.asyncio import AsyncSession + +from app.modules.projects.models import PolicyMutationIdempotencyRecord +from app.modules.projects.repository import ProjectRepositoryIntegrityError + + +class PolicyMutationReplayRepository: + """Own only policy-mutation replay records in the caller transaction.""" + + def __init__(self, session: AsyncSession) -> None: + self._session = session + + async def find( + self, actor_profile_id: str, action_id: str, idempotency_key: UUID + ) -> PolicyMutationIdempotencyRecord | None: + return await self._session.scalar( + select(PolicyMutationIdempotencyRecord).where( + PolicyMutationIdempotencyRecord.actor_profile_id == actor_profile_id, + PolicyMutationIdempotencyRecord.action_id == action_id, + PolicyMutationIdempotencyRecord.idempotency_key == idempotency_key, + ) + ) + + async def reserve( + self, + *, + actor_profile_id: str, + identity_link_id: str, + action_id: str, + idempotency_key: UUID, + request_digest: str, + policy_hash: str, + resource_context_digest: str, + operation_id: UUID, + project_id: str, + guide_id: str, + policy_id: str, + policy_generation: int, + ) -> tuple[str, PolicyMutationIdempotencyRecord]: + values = { + "id": uuid4(), + "actor_profile_id": actor_profile_id, + "identity_link_id": identity_link_id, + "action_id": action_id, + "idempotency_key": idempotency_key, + "request_digest": request_digest, + "policy_hash": policy_hash, + "resource_context_digest": resource_context_digest, + "operation_id": operation_id, + "project_id": project_id, + "guide_id": guide_id, + "policy_id": policy_id, + "policy_generation": policy_generation, + "status": "pending", + } + record_id = await self._session.scalar( + insert(PolicyMutationIdempotencyRecord) + .values(**values) + .on_conflict_do_update( + index_elements=[ + PolicyMutationIdempotencyRecord.actor_profile_id, + PolicyMutationIdempotencyRecord.action_id, + PolicyMutationIdempotencyRecord.idempotency_key, + ], + set_={"id": PolicyMutationIdempotencyRecord.id}, + ) + .returning(PolicyMutationIdempotencyRecord.id) + ) + if record_id is None: + raise ProjectRepositoryIntegrityError("policy mutation reservation disappeared") + record = await self._session.get(PolicyMutationIdempotencyRecord, record_id) + if record is None: + raise ProjectRepositoryIntegrityError("policy mutation reservation disappeared") + if record_id == values["id"]: + return "claimed", record + if ( + record.project_id != project_id + or record.guide_id != guide_id + or record.request_digest != request_digest + or record.policy_hash != policy_hash + ): + return "mismatch", record + return ("replayed" if record.status == "committed" else "pending"), record + + async def complete( + self, record: PolicyMutationIdempotencyRecord, *, response_json: dict + ) -> None: + completed = await self._session.scalar( + update(PolicyMutationIdempotencyRecord) + .where( + PolicyMutationIdempotencyRecord.id == record.id, + PolicyMutationIdempotencyRecord.status == "pending", + ) + .values( + status="committed", + response_json=response_json, + committed_at=datetime.now(UTC), + ) + .returning(PolicyMutationIdempotencyRecord.id) + ) + if completed is None: + raise ProjectRepositoryIntegrityError("invalid policy mutation completion") diff --git a/backend/app/modules/projects/policy_mutation_router.py b/backend/app/modules/projects/policy_mutation_router.py new file mode 100644 index 000000000..8cb9b0a48 --- /dev/null +++ b/backend/app/modules/projects/policy_mutation_router.py @@ -0,0 +1,145 @@ +"""FastAPI boundary for authorized guide-bound policy mutations.""" + +from __future__ import annotations + +from typing import Annotated +from uuid import UUID + +from fastapi import APIRouter, Depends, Header, HTTPException, Request +from sqlalchemy.ext.asyncio import AsyncSession + +from app.api.deps.auth import get_auth_verification_result +from app.api.deps.authorization import prepared_authorization_service, resolve_authorization_actor +from app.api.deps.rate_controls import get_rate_control_service +from app.core.api_controls import StructuredHTTPException +from app.db.session import get_db_session +from app.modules.actors.service import ResolvedActor +from app.modules.api_controls.service import RateControlService +from app.modules.authorization.catalogue import ActionId +from app.modules.authorization.prepared import PreparedAuthorizationService +from app.modules.projects.policy_mutation_service import ( + PolicyMutationConflict, + ProjectPolicyMutationService, +) +from app.modules.projects.schemas import ( + ReviewPolicyInput, + ReviewPolicyResponse, + RevisionPolicyInput, + RevisionPolicyResponse, +) +from app.modules.projects.service import ProjectServiceError +from app.schemas.auth import AuthVerificationResult + + +router = APIRouter(prefix="/projects", tags=["projects"]) + + +def require_policy_mutation_key( + idempotency_key: Annotated[str, Header(alias="Idempotency-Key")], +) -> UUID: + try: + return UUID(idempotency_key) + except ValueError as exc: + raise StructuredHTTPException( + status_code=422, + detail="Idempotency-Key must be a UUID", + error_code="validation_error", + error_message="Idempotency-Key must be a UUID", + ) from exc + + +async def policy_authorization_actor( + key: Annotated[UUID, Depends(require_policy_mutation_key)], + request: Request, + result: Annotated[AuthVerificationResult, Depends(get_auth_verification_result)], + session: Annotated[AsyncSession, Depends(get_db_session)], + rate_control: Annotated[RateControlService, Depends(get_rate_control_service)], +) -> ResolvedActor: + del key + return await resolve_authorization_actor(request, result, session, rate_control) + + +async def get_policy_prepared_authorization_service( + request: Request, + resolved: Annotated[ResolvedActor, Depends(policy_authorization_actor)], + session: Annotated[AsyncSession, Depends(get_db_session)], +): + async with prepared_authorization_service(request, resolved, session) as service: + yield service + + +async def policy_authorization( + key: Annotated[UUID, Depends(require_policy_mutation_key)], + resolved: Annotated[ResolvedActor, Depends(policy_authorization_actor)], + prepared: Annotated[ + PreparedAuthorizationService, + Depends(get_policy_prepared_authorization_service), + ], +): + return key, resolved, prepared + + +def _error(exc: ProjectServiceError): + if isinstance(exc, PolicyMutationConflict): + code = str(exc) + return StructuredHTTPException( + status_code=409, + detail=code, + error_code=code, + error_message="Policy mutation conflicts with current state", + retryable=code == "idempotency_pending", + ) + return HTTPException(status_code=exc.status_code, detail=str(exc)) + + +async def _finish(session: AsyncSession, outcome): + await (session.rollback() if outcome.replayed else session.commit()) + return outcome.response + + +@router.put( + "/{project_id}/guides/{guide_id}/review-policy", + response_model=ReviewPolicyResponse, + openapi_extra={"x-workstream-action-id": ActionId.PROJECT_REVIEW_POLICY_UPDATE.value}, +) +async def replace_review_policy( + project_id: UUID, + guide_id: UUID, + payload: ReviewPolicyInput, + if_match: Annotated[str, Header(alias="If-Match")], + authorization: Annotated[tuple, Depends(policy_authorization)], + session: Annotated[AsyncSession, Depends(get_db_session)], +): + key, resolved, prepared = authorization + try: + outcome = await ProjectPolicyMutationService(session).replace_review_policy( + resolved, prepared, key, if_match, project_id, guide_id, payload + ) + return await _finish(session, outcome) + except ProjectServiceError as exc: + await session.rollback() + raise _error(exc) from exc + + +@router.put( + "/{project_id}/guides/{guide_id}/revision-policy", + response_model=RevisionPolicyResponse, + openapi_extra={"x-workstream-action-id": ActionId.PROJECT_REVISION_POLICY_UPDATE.value}, +) +async def replace_revision_policy( + project_id: UUID, + guide_id: UUID, + payload: RevisionPolicyInput, + if_match: Annotated[str, Header(alias="If-Match")], + authorization: Annotated[tuple, Depends(policy_authorization)], + session: Annotated[AsyncSession, Depends(get_db_session)], +): + key, resolved, prepared = authorization + try: + outcome = await ProjectPolicyMutationService(session).replace_revision_policy( + resolved, prepared, key, if_match, project_id, guide_id, payload + ) + return await _finish(session, outcome) + except ProjectServiceError as exc: + await session.rollback() + raise _error(exc) from exc diff --git a/backend/app/modules/projects/policy_mutation_service.py b/backend/app/modules/projects/policy_mutation_service.py new file mode 100644 index 000000000..b8c07b503 --- /dev/null +++ b/backend/app/modules/projects/policy_mutation_service.py @@ -0,0 +1,392 @@ +"""Authorized guide-bound review and revision policy mutation.""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Literal +from uuid import UUID, uuid4 + +from sqlalchemy.ext.asyncio import AsyncSession + +from app.core.hashing import canonical_json_hash +from app.modules.actors.service import ResolvedActor +from app.modules.authorization.catalogue import ActionId +from app.modules.authorization.prepared import PreparedAuthorizationService +from app.modules.authorization.runtime import ( + MatchedAuthorityKind, + PreparedAuthorizationInput, + PreparedAuthorizationHandleInvalid, + PreparedAuthorizationUnsupported, + PreparedAuthorityScope, + PreparedAuthorityScopeKind, + ProjectReviewPolicyMutationResourceContext, + ProjectRevisionPolicyMutationResourceContext, + ProjectPolicyMutationPrepareDenialResourceContext, + authorization_resource_digest, +) +from app.modules.projects.models import ProjectGuide, ReviewPolicy, RevisionPolicy +from app.modules.projects.policy_lineage import ( + ReviewPolicySemantics, + RevisionPolicySemantics, + policy_digest, +) +from app.modules.projects.policy_mutation_replay_repository import ( + PolicyMutationReplayRepository, +) +from app.modules.projects.repository import ProjectRepository +from app.modules.projects.schemas import ( + ReviewPolicyInput, + ReviewPolicyResponse, + RevisionPolicyInput, + RevisionPolicyResponse, +) +from app.modules.projects.service import GuideEditBlocked, GuideNotFound, ProjectServiceError + + +NO_CURRENT_POLICY_ETAG = '"no-current-policy"' + + +def policy_selector_etag(policy_id: str, generation: int, policy_hash: str) -> str: + """Return the exact opaque HTTP entity tag for one selected policy version.""" + return f'"{policy_id}.{generation}.{policy_hash.removeprefix("sha256:")}"' + + +class PolicyMutationConflict(ProjectServiceError): + """The optimistic selector or replay record no longer matches.""" + + status_code = 409 + + +@dataclass(frozen=True, slots=True) +class PolicyMutationOutcome: + """One transaction-owned policy mutation result.""" + + response: ReviewPolicyResponse | RevisionPolicyResponse + replayed: bool + + +class ProjectPolicyMutationService: + """Sole authorized writer for immutable guide policy versions.""" + + def __init__(self, session: AsyncSession) -> None: + self._projects = ProjectRepository(session) + self._replay = PolicyMutationReplayRepository(session) + + @staticmethod + def _if_match_value(if_match: str) -> tuple[UUID, int, str] | None: + if if_match == NO_CURRENT_POLICY_ETAG: + return None + if len(if_match) > 2 and if_match[0] == if_match[-1] == '"': + parts = if_match[1:-1].split(".") + if ( + len(parts) == 3 + and len(parts[2]) == 64 + and all(character in "0123456789abcdef" for character in parts[2]) + ): + try: + generation = int(parts[1]) + if generation < 1 or parts[1] != str(generation): + raise ValueError + return UUID(parts[0]), generation, f"sha256:{parts[2]}" + except ValueError: + pass + raise PolicyMutationConflict("policy_precondition_invalid") + + @staticmethod + def _prove(decision, project_id: UUID) -> None: + if ( + decision.matched_authority_kind is not MatchedAuthorityKind.ADMIN_ROLE_GRANT + or decision.matched_grant_id is None + or decision.matched_scope_project_id not in {None, project_id} + ): + raise RuntimeError("policy mutation lacked Project Manager authority") + + @staticmethod + def _guide_selector( + kind: Literal["review", "revision"], guide: ProjectGuide + ) -> tuple[UUID, int, str] | None: + policy_id = getattr(guide, f"selected_{kind}_policy_id") + generation = getattr(guide, f"selected_{kind}_policy_generation") + digest = getattr(guide, f"selected_{kind}_policy_hash") + if policy_id is None: + return None + return UUID(policy_id), generation, digest + + @staticmethod + def _resource( + *, + kind: Literal["review", "revision"], + policy_id: UUID, + operation_id: UUID, + request_digest: str, + project_id: UUID, + guide_id: UUID, + guide_version: str, + generation: int, + final_digest: str, + predecessor_id: UUID | None, + predecessor_generation: int | None, + predecessor_digest: str | None, + ) -> ProjectReviewPolicyMutationResourceContext | ProjectRevisionPolicyMutationResourceContext: + values = { + "resource_type": f"project_{kind}_policy_mutation", + "resource_id": policy_id, + "operation_id": operation_id, + "request_digest": request_digest, + "scope_project_id": project_id, + "guide_id": guide_id, + "guide_version": guide_version, + "guide_status": "draft", + f"{kind}_policy_id": policy_id, + "policy_generation": generation, + "policy_digest": final_digest, + "predecessor_policy_id": predecessor_id, + "predecessor_policy_generation": predecessor_generation, + "current_policy_digest": predecessor_digest, + } + return ( + ProjectReviewPolicyMutationResourceContext(**values) + if kind == "review" + else ProjectRevisionPolicyMutationResourceContext(**values) + ) + + async def _existing(self, resolved, action, key, digest, project_id, guide_id, response_type): + record = await self._replay.find(resolved.profile.id, action.value, key) + if record is None: + return None + if ( + record.project_id != str(project_id) + or record.guide_id != str(guide_id) + or record.request_digest != digest + ): + raise PolicyMutationConflict("idempotency_mismatch") + if record.status != "committed" or record.response_json is None: + raise PolicyMutationConflict("idempotency_pending") + response = response_type.model_validate(record.response_json) + if response.policy_hash != record.policy_hash: + raise RuntimeError("committed policy replay lost digest custody") + return PolicyMutationOutcome(response, True) + + async def replace_review_policy( + self, + resolved: ResolvedActor, + prepared: PreparedAuthorizationService, + key: UUID, + if_match: str, + project_id: UUID, + guide_id: UUID, + payload: ReviewPolicyInput, + ) -> PolicyMutationOutcome: + semantics = ReviewPolicySemantics.model_validate(payload.model_dump()) + return await self._replace( + "review", + resolved, + prepared, + key, + if_match, + project_id, + guide_id, + semantics, + ReviewPolicyResponse, + ) + + async def replace_revision_policy( + self, + resolved: ResolvedActor, + prepared: PreparedAuthorizationService, + key: UUID, + if_match: str, + project_id: UUID, + guide_id: UUID, + payload: RevisionPolicyInput, + ) -> PolicyMutationOutcome: + semantics = RevisionPolicySemantics.model_validate(payload.model_dump()) + return await self._replace( + "revision", + resolved, + prepared, + key, + if_match, + project_id, + guide_id, + semantics, + RevisionPolicyResponse, + ) + + async def _replace( + self, + kind: Literal["review", "revision"], + resolved: ResolvedActor, + prepared: PreparedAuthorizationService, + key: UUID, + if_match: str, + project_id: UUID, + guide_id: UUID, + semantics: ReviewPolicySemantics | RevisionPolicySemantics, + response_type, + ) -> PolicyMutationOutcome: + expected_selector = self._if_match_value(if_match) + action = ( + ActionId.PROJECT_REVIEW_POLICY_UPDATE + if kind == "review" + else ActionId.PROJECT_REVISION_POLICY_UPDATE + ) + policy_id, operation_id = uuid4(), uuid4() + semantic_values = semantics.model_dump(mode="json") + final_digest = policy_digest(kind, semantics) + predecessor_id = expected_selector[0] if expected_selector is not None else None + predecessor_generation = expected_selector[1] if expected_selector is not None else None + predecessor_digest = expected_selector[2] if expected_selector is not None else None + generation = 1 if expected_selector is None else predecessor_generation + 1 + request_digest = canonical_json_hash( + { + "domain": "workstream.policy_mutation.idempotency.v1", + "method": "PUT", + "action_id": action.value, + "project_id": str(project_id), + "guide_id": str(guide_id), + "policy_kind": kind, + "if_match": if_match, + "semantics": semantic_values, + "idempotency_key": str(key), + } + ) + existing = await self._existing( + resolved, action, key, request_digest, project_id, guide_id, response_type + ) + if existing is not None: + return existing + guide_snapshot = await self._projects.get_guide(str(guide_id)) + if guide_snapshot is None or guide_snapshot.project_id != str(project_id): + raise GuideNotFound("guide not found") + if guide_snapshot.status != "draft": + raise GuideEditBlocked("only draft guides can change policy") + snapshot_selector = self._guide_selector(kind, guide_snapshot) + if snapshot_selector != expected_selector: + raise PolicyMutationConflict("policy_precondition_failed") + resource = self._resource( + kind=kind, + policy_id=policy_id, + operation_id=operation_id, + request_digest=request_digest, + project_id=project_id, + guide_id=guide_id, + guide_version=guide_snapshot.version, + generation=generation, + final_digest=final_digest, + predecessor_id=predecessor_id, + predecessor_generation=predecessor_generation, + predecessor_digest=predecessor_digest, + ) + disposition, replay = await self._replay.reserve( + actor_profile_id=resolved.profile.id, + identity_link_id=resolved.identity_link.id, + action_id=action.value, + idempotency_key=key, + request_digest=request_digest, + policy_hash=final_digest, + resource_context_digest=authorization_resource_digest(resource), + operation_id=operation_id, + project_id=str(project_id), + guide_id=str(guide_id), + policy_id=str(policy_id), + policy_generation=generation, + ) + if disposition != "claimed": + if disposition == "mismatch": + raise PolicyMutationConflict("idempotency_mismatch") + if disposition == "pending" or replay.response_json is None: + raise PolicyMutationConflict("idempotency_pending") + return PolicyMutationOutcome(response_type.model_validate(replay.response_json), True) + caller = PreparedAuthorizationInput( + idempotency_key=key, + request_value={ + "action_id": action.value, + "project_id": str(project_id), + "guide_id": str(guide_id), + "policy_id": str(policy_id), + "operation_id": str(operation_id), + "request_digest": request_digest, + "policy_digest": final_digest, + "policy_generation": generation, + "predecessor_policy_id": ( + str(predecessor_id) if predecessor_id is not None else None + ), + "predecessor_policy_generation": predecessor_generation, + "predecessor_policy_digest": predecessor_digest, + "guide_status": "draft", + }, + ) + try: + handle = await prepared.prepare( + action, + caller, + PreparedAuthorityScope( + kind=PreparedAuthorityScopeKind.PROJECT, + project_id=project_id, + ), + ) + except PreparedAuthorizationUnsupported as exc: + await prepared.deny_unsupported( + action, + caller, + ProjectPolicyMutationPrepareDenialResourceContext( + resource_type="project_policy_mutation_request", + resource_id=guide_id, + scope_project_id=project_id, + requested_guide_id=guide_id, + requested_policy_kind=kind, + request_digest=request_digest, + ), + exc, + ) + guide = await self._projects.lock_project_guide(str(guide_id)) + if guide is None or guide.project_id != str(project_id): + raise GuideNotFound("guide not found") + if guide.status != "draft": + raise GuideEditBlocked("only draft guides can change policy") + current = ( + await self._projects.lock_review_policy(str(project_id), guide.version) + if kind == "review" + else await self._projects.lock_revision_policy(str(project_id), guide.version) + ) + current_selector = self._guide_selector(kind, guide) + if current_selector != expected_selector: + raise PolicyMutationConflict("policy_precondition_failed") + try: + decision = await prepared.consume(handle, action, caller, resource) + except PreparedAuthorizationHandleInvalid as exc: + raise PolicyMutationConflict("authorization_handle_invalid") from exc + self._prove(decision, project_id) + common = { + "id": str(policy_id), + "project_id": str(project_id), + "guide_version": guide.version, + "policy_generation": generation, + "policy_hash": final_digest, + "semantics_status": "complete", + "supersedes_policy_id": current.id if current is not None else None, + "predecessor_policy_hash": predecessor_digest, + "created_by_actor_profile_id": resolved.profile.id, + "created_via_identity_link_id": resolved.identity_link.id, + "created_by_admin_role_grant_id": decision.matched_grant_id, + "creation_scope_type": ( + "system" if decision.matched_scope_project_id is None else "project" + ), + "creation_scope_project_id": ( + str(decision.matched_scope_project_id) + if decision.matched_scope_project_id is not None + else None + ), + "creation_action_id": action.value, + "authorization_decision_event_id": str(decision.decision_id), + **semantic_values, + } + policy = ReviewPolicy(**common) if kind == "review" else RevisionPolicy(**common) + if kind == "review": + await self._projects.add_review_policy_version(policy, guide) + else: + await self._projects.add_revision_policy_version(policy, guide) + response = response_type.model_validate(policy) + await self._replay.complete(replay, response_json=response.model_dump(mode="json")) + return PolicyMutationOutcome(response, False) diff --git a/backend/app/modules/projects/repository.py b/backend/app/modules/projects/repository.py index ba75d4174..d689583e6 100644 --- a/backend/app/modules/projects/repository.py +++ b/backend/app/modules/projects/repository.py @@ -866,6 +866,30 @@ async def lock_revision_policy( .with_for_update(of=RevisionPolicy) ) + async def add_review_policy_version( + self, policy: ReviewPolicy, guide: ProjectGuide + ) -> ReviewPolicy: + """Append one immutable review policy and advance its locked selector.""" + self._session.add(policy) + guide.selected_review_policy_id = policy.id + guide.selected_review_policy_generation = policy.policy_generation + guide.selected_review_policy_hash = policy.policy_hash + await self._session.flush() + await self._session.refresh(policy) + return policy + + async def add_revision_policy_version( + self, policy: RevisionPolicy, guide: ProjectGuide + ) -> RevisionPolicy: + """Append one immutable revision policy and advance its locked selector.""" + self._session.add(policy) + guide.selected_revision_policy_id = policy.id + guide.selected_revision_policy_generation = policy.policy_generation + guide.selected_revision_policy_hash = policy.policy_hash + await self._session.flush() + await self._session.refresh(policy) + return policy + async def get_pre_submit_checker_policy( self, policy_id: str, diff --git a/backend/app/modules/projects/schemas.py b/backend/app/modules/projects/schemas.py index ccb0b3667..9d727e2f1 100644 --- a/backend/app/modules/projects/schemas.py +++ b/backend/app/modules/projects/schemas.py @@ -37,7 +37,11 @@ class RevisionPolicyInput(BaseModel): max_revision_rounds: int = Field(ge=1) revision_deadline_hours: int = Field(ge=1) - allowed_resubmission_states: list[str] = Field(default_factory=lambda: ["needs_revision"]) + allowed_resubmission_states: list[Literal["needs_revision"]] = Field( + default_factory=lambda: ["needs_revision"], + min_length=1, + max_length=1, + ) reviewer_reassignment_rule: str | None = None diff --git a/backend/scripts/api_contract_e2e.py b/backend/scripts/api_contract_e2e.py index ae72163b3..7bddfb1c4 100644 --- a/backend/scripts/api_contract_e2e.py +++ b/backend/scripts/api_contract_e2e.py @@ -34,15 +34,7 @@ PaymentPolicy, PostSubmitCheckerPolicy, PreSubmitCheckerPolicy, - ProjectGuide, ProjectSetupRun, - ReviewPolicy, - RevisionPolicy, -) -from app.modules.projects.policy_lineage import ( - ReviewPolicySemantics, - RevisionPolicySemantics, - policy_digest, ) from app.modules.projects.post_submit_policy import ( build_project_post_submit_checker_spec, @@ -112,6 +104,8 @@ async def seed_active_guide_for_pre_12h_e2e( text("alter table project_guides enable trigger guide_mutation_product_custody") ) await session.commit() + + DEFAULT_FLOW_ISSUER = "https://auth.flow.local/e2e" DEFAULT_FLOW_AUDIENCE = "workstream-api" LOCAL_DATABASE_HOSTS = {"localhost", "127.0.0.1", "::1"} @@ -419,6 +413,7 @@ async def request_json( payload: dict | None = None, expected_status: int = 200, idempotency_key: str | None = None, + if_match: str | None = None, ) -> dict | list: """Call one API endpoint and assert its status. @@ -430,6 +425,7 @@ async def request_json( payload: Optional JSON payload. expected_status: Expected HTTP status code. idempotency_key: Optional UUID replay key for mutation boundaries. + if_match: Optional exact HTTP policy selector precondition. Returns: Parsed JSON response body. @@ -440,11 +436,11 @@ async def request_json( request_id = str(uuid4()) correlation_id = str(uuid4()) headers = {} if token is None else auth_headers(token) - headers.update( - {"X-Request-ID": request_id, "X-Correlation-ID": correlation_id} - ) + headers.update({"X-Request-ID": request_id, "X-Correlation-ID": correlation_id}) if idempotency_key is not None: headers["Idempotency-Key"] = idempotency_key + if if_match is not None: + headers["If-Match"] = if_match response = await client.request( method, path, @@ -472,9 +468,10 @@ async def request_json( if response.headers.get("x-correlation-id") != correlation_id: raise AssertionError(f"{method} {path} did not preserve the correlation ID") if expected_status >= 400: - if not isinstance(body, dict) or body.get("error", {}).get( - "correlation_id" - ) != correlation_id: + if ( + not isinstance(body, dict) + or body.get("error", {}).get("correlation_id") != correlation_id + ): raise AssertionError(f"{method} {path} returned invalid error context") print(f"PASS {method} {path} -> {response.status_code}") return body @@ -574,8 +571,7 @@ def assert_checker_run_result_integrity(checker_run: dict, expected_names: set[s "checker warning count does not match returned results", ) ensure( - checker_run["failed_count"] - == sum(1 for result in results if result["status"] == "failed"), + checker_run["failed_count"] == sum(1 for result in results if result["status"] == "failed"), "checker failed count does not match returned results", ) ensure( @@ -648,92 +644,61 @@ def guide_payload(run_id: str) -> dict: } -async def seed_pending_policy_boundaries(project_id: str, guide_version: str) -> None: - """Seed policies whose clean-cut authorization routes arrive after AUTH-12D. - - The E2E flow must keep proving the downstream lifecycle while guide create no - longer accepts embedded policy writes. These direct fixtures are removed as - each dedicated policy boundary is activated. - """ +async def configure_policy_boundaries( + client: httpx.AsyncClient, + token: str, + project_id: str, + guide_id: str, + guide_version: str, +) -> None: + """Configure both policies through their sole active HTTP boundaries.""" + await request_json( + client, + "PUT", + f"/api/v1/projects/{project_id}/guides/{guide_id}/review-policy", + token, + { + "review_preference_window_seconds": 3600, + "review_lease_duration_seconds": 1800, + "max_active_review_leases_per_reviewer": 1, + "self_review_allowed": False, + "reject_policy": "close_task", + "finding_evidence_requirement": "optional", + "requires_second_review": False, + "allowed_decisions": ["accept", "needs_revision", "reject"], + "minimum_finding_fields": ["issue", "required_fix"], + }, + idempotency_key=str(uuid4()), + if_match='"no-current-policy"', + ) + await request_json( + client, + "PUT", + f"/api/v1/projects/{project_id}/guides/{guide_id}/revision-policy", + token, + { + "max_revision_rounds": 7, + "revision_deadline_hours": 48, + "allowed_resubmission_states": ["needs_revision"], + "reviewer_reassignment_rule": "same reviewer preferred", + }, + idempotency_key=str(uuid4()), + if_match='"no-current-policy"', + ) async with db_session.get_session_factory()() as session: - review_id = str(uuid4()) - revision_id = str(uuid4()) - review_semantics = ReviewPolicySemantics( - review_preference_window_seconds=3600, - review_lease_duration_seconds=1800, - allowed_decisions=("accept", "needs_revision", "reject"), - minimum_finding_fields=("issue", "required_fix"), - ) - revision_semantics = RevisionPolicySemantics( - max_revision_rounds=7, - revision_deadline_hours=48, - allowed_resubmission_states=("needs_revision",), - reviewer_reassignment_rule="same reviewer preferred", - ) - review_hash = policy_digest("review", review_semantics) - revision_hash = policy_digest("revision", revision_semantics) - session.add_all( - [ - ReviewPolicy( - id=review_id, - project_id=project_id, - guide_version=guide_version, - policy_generation=1, - policy_hash=review_hash, - semantics_status="complete", - **review_semantics.model_dump(mode="python"), - ), - RevisionPolicy( - id=revision_id, - project_id=project_id, - guide_version=guide_version, - policy_generation=1, - policy_hash=revision_hash, - semantics_status="complete", - **revision_semantics.model_dump(mode="python"), - ), - PaymentPolicy( - id=str(uuid4()), - project_id=project_id, - guide_version=guide_version, - base_amount="25.00", - currency="USD", - payout_type="fixed", - revision_payment_rule="none", - rejection_payment_rule="none", - accepted_payment_rule="pay base amount", - ), - ] - ) - await session.flush() - guide = await session.scalar( - select(ProjectGuide).where( - ProjectGuide.project_id == project_id, - ProjectGuide.version == guide_version, + session.add( + PaymentPolicy( + id=str(uuid4()), + project_id=project_id, + guide_version=guide_version, + base_amount="25.00", + currency="USD", + payout_type="fixed", + revision_payment_rule="none", + rejection_payment_rule="none", + accepted_payment_rule="pay base amount", ) ) - if guide is None: - raise RuntimeError("policy seed requires its exact project guide") - await session.execute( - text("alter table project_guides disable trigger guide_mutation_product_custody") - ) - await session.execute( - text("alter table project_guides disable trigger guide_lineage_lifecycle_guard") - ) - guide.selected_review_policy_id = review_id - guide.selected_review_policy_generation = 1 - guide.selected_review_policy_hash = review_hash - guide.selected_revision_policy_id = revision_id - guide.selected_revision_policy_generation = 1 - guide.selected_revision_policy_hash = revision_hash - await session.flush() - await session.execute(text("set constraints all immediate")) - await session.execute( - text("alter table project_guides enable trigger guide_lineage_lifecycle_guard") - ) - await session.execute( - text("alter table project_guides enable trigger guide_mutation_product_custody") - ) await session.commit() @@ -1118,7 +1083,8 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: read_actions = { path: item["get"]["x-workstream-action-id"] for path, item in openapi["paths"].items() - if path in { + if path + in { "/api/v1/projects/{project_id}/contributor-candidates", "/api/v1/projects/{project_id}/role-grants", "/api/v1/projects/{project_id}/role-grants/{grant_id}", @@ -1134,13 +1100,9 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: "project.contributor_candidate.list" ), "/api/v1/projects/{project_id}/role-grants": "project_role_grant.list", - "/api/v1/projects/{project_id}/role-grants/{grant_id}": ( - "project_role_grant.read" - ), + "/api/v1/projects/{project_id}/role-grants/{grant_id}": ("project_role_grant.read"), "/api/v1/projects/{project_id}": "project.read", - "/api/v1/actors/me/authorization-context": ( - "actor.authorization_context.read" - ), + "/api/v1/actors/me/authorization-context": ("actor.authorization_context.read"), "/api/v1/projects/{project_id}/active-guide": "project.active_guide.read", "/api/v1/projects/{project_id}/guides/{guide_id}/effective-submission-artifact-policy": ( "project.effective_submission_artifact_policy.read" @@ -1149,19 +1111,30 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: "project.pre_submit_checker_policy.read" ), } - assert openapi["paths"]["/api/v1/projects/{project_id}/role-grants"]["post"][ - "x-workstream-action-id" - ] == "project_role_grant.issue" - assert openapi["paths"]["/api/v1/projects"]["post"][ - "x-workstream-action-id" - ] == "project.create" - assert openapi["paths"][ - "/api/v1/projects/{project_id}/role-grants/{grant_id}/revoke" - ]["post"]["x-workstream-action-id"] == "project_role_grant.revoke" + assert ( + openapi["paths"]["/api/v1/projects/{project_id}/role-grants"]["post"][ + "x-workstream-action-id" + ] + == "project_role_grant.issue" + ) + assert ( + openapi["paths"]["/api/v1/projects"]["post"]["x-workstream-action-id"] + == "project.create" + ) + assert ( + openapi["paths"]["/api/v1/projects/{project_id}/role-grants/{grant_id}/revoke"]["post"][ + "x-workstream-action-id" + ] + == "project_role_grant.revoke" + ) await request_json(client, "GET", "/api/v1/auth/me", expected_status=401) await request_json(client, "GET", "/api/v1/auth/me", invalid_token, expected_status=401) - await request_json(client, "GET", "/api/v1/auth/me", wrong_issuer_token, expected_status=401) - await request_json(client, "GET", "/api/v1/auth/me", wrong_audience_token, expected_status=401) + await request_json( + client, "GET", "/api/v1/auth/me", wrong_issuer_token, expected_status=401 + ) + await request_json( + client, "GET", "/api/v1/auth/me", wrong_audience_token, expected_status=401 + ) await request_json(client, "GET", "/api/v1/auth/me", expired_token, expected_status=401) await request_json(client, "GET", "/api/v1/auth/me", future_nbf_token, expected_status=401) manager = await request_json(client, "GET", "/api/v1/auth/me", manager_token) @@ -1216,10 +1189,7 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: headers=auth_headers(fixed_service_token), ) assert unprovisioned_service.status_code == 403 - assert ( - unprovisioned_service.json()["error"]["code"] - == "service_actor_not_provisioned" - ) + assert unprovisioned_service.json()["error"]["code"] == "service_actor_not_provisioned" service_headers = auth_headers(manager_token) | { "Idempotency-Key": str(uuid4()), "X-Request-ID": str(uuid4()), @@ -1304,18 +1274,14 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: headers=auth_headers(fixed_service_token), ) assert reactivated_service_admission.status_code == 403 - assert ( - reactivated_service_admission.json()["error"]["code"] - == "permission_not_granted" - ) + assert reactivated_service_admission.json()["error"]["code"] == "permission_not_granted" service_link_id = service_admin_link["identity_link_id"] link_lifecycle_key = str(uuid4()) link_lifecycle_reason = "Real HTTP service identity-link lifecycle proof" revoked_service_link = await client.post( f"/api/v1/actor-identity-links/{service_link_id}/revoke", - headers=auth_headers(manager_token) - | {"Idempotency-Key": link_lifecycle_key}, + headers=auth_headers(manager_token) | {"Idempotency-Key": link_lifecycle_key}, json={"reason": link_lifecycle_reason}, ) assert revoked_service_link.status_code == 200, revoked_service_link.text @@ -1331,22 +1297,17 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: headers=auth_headers(fixed_service_token), ) assert revoked_service_admission.status_code == 403 - assert ( - revoked_service_admission.json()["error"]["code"] - == "identity_link_revoked" - ) + assert revoked_service_admission.json()["error"]["code"] == "identity_link_revoked" replayed_service_link = await client.post( f"/api/v1/actor-identity-links/{service_link_id}/revoke", - headers=auth_headers(manager_token) - | {"Idempotency-Key": link_lifecycle_key}, + headers=auth_headers(manager_token) | {"Idempotency-Key": link_lifecycle_key}, json={"reason": link_lifecycle_reason}, ) assert replayed_service_link.status_code == 200, replayed_service_link.text assert replayed_service_link.json() == revoked_service_link.json() mismatched_service_link = await client.post( f"/api/v1/actor-identity-links/{service_link_id}/revoke", - headers=auth_headers(manager_token) - | {"Idempotency-Key": link_lifecycle_key}, + headers=auth_headers(manager_token) | {"Idempotency-Key": link_lifecycle_key}, json={"reason": "Different link lifecycle request"}, ) assert mismatched_service_link.status_code == 409, mismatched_service_link.text @@ -1357,10 +1318,7 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: json={"reason": "Conflicting link lifecycle request"}, ) assert conflicting_service_link.status_code == 409, conflicting_service_link.text - assert ( - conflicting_service_link.json()["error"]["code"] - == "identity_link_already_revoked" - ) + assert conflicting_service_link.json()["error"]["code"] == "identity_link_already_revoked" repaired_service_link = await client.post( f"/api/v1/actor-identity-links/{service_link_id}/reactivate", headers=auth_headers(manager_token) | {"Idempotency-Key": str(uuid4())}, @@ -1398,8 +1356,7 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: project_response = await client.post( "/api/v1/projects", - headers=auth_headers(project_reader_token) - | {"Idempotency-Key": str(uuid4())}, + headers=auth_headers(project_reader_token) | {"Idempotency-Key": str(uuid4())}, json={ "name": f"API Contract Real API {run_id}", "slug": f"api-contract-real-api-{run_id}", @@ -1503,7 +1460,13 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: 201, idempotency_key=str(uuid4()), ) - await seed_pending_policy_boundaries(project["id"], guide["version"]) + await configure_policy_boundaries( + client, + project_reader_token, + project["id"], + guide["id"], + guide["version"], + ) patched_guide = await request_json( client, "PATCH", @@ -1573,8 +1536,7 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: visible_checker_policy = await request_json( client, "GET", - f"/api/v1/projects/{project['id']}/guides/{guide['id']}/" - "pre-submit-checker-policy", + f"/api/v1/projects/{project['id']}/guides/{guide['id']}/pre-submit-checker-policy", project_reader_token, ) ensure( @@ -1745,22 +1707,42 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: project_reader_token, ) assert set(grant) == { - "id", "project_id", "actor_profile_id", "role", "status", "version", - "grant_method", "qualification_snapshot", "granted_by_actor_profile_id", - "granted_by_admin_role_grant_id", "granted_at", "grant_reason", - "revoked_by_actor_profile_id", "revoked_at", "revoked_reason", + "id", + "project_id", + "actor_profile_id", + "role", + "status", + "version", + "grant_method", + "qualification_snapshot", + "granted_by_actor_profile_id", + "granted_by_admin_role_grant_id", + "granted_at", + "grant_reason", + "revoked_by_actor_profile_id", + "revoked_at", + "revoked_reason", } assert set(grant["qualification_snapshot"]) == { - "id", "requested_role", "skills_snapshot", "reputation_snapshot", - "prior_project_work_refs", "external_expertise_refs", - "captured_by_actor_profile_id", "captured_by_admin_role_grant_id", + "id", + "requested_role", + "skills_snapshot", + "reputation_snapshot", + "prior_project_work_refs", + "external_expertise_refs", + "captured_by_actor_profile_id", + "captured_by_admin_role_grant_id", "captured_at", } assert set(grant["qualification_snapshot"]["skills_snapshot"]) == { - "availability", "reference_ids", "unavailable_reason", + "availability", + "reference_ids", + "unavailable_reason", } assert set(grant["qualification_snapshot"]["reputation_snapshot"]) == { - "availability", "reference_ids", "unavailable_reason", + "availability", + "reference_ids", + "unavailable_reason", } assert grant["revoked_by_actor_profile_id"] is None assert grant["revoked_at"] is None @@ -1813,8 +1795,7 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: ) assert issue_after_revoke.status_code == 409, issue_after_revoke.text assert ( - issue_after_revoke.json()["error"]["code"] - == "project_role_grant_replay_state_changed" + issue_after_revoke.json()["error"]["code"] == "project_role_grant_replay_state_changed" ) link_case_body = role_issue_body | { "role": "reviewer", @@ -1833,8 +1814,7 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: ) assert revoked_target_link.status_code == 200, revoked_target_link.text link_case_revoke = await client.post( - f"/api/v1/projects/{project['id']}/role-grants/" - f"{link_case_issue.json()['id']}/revoke", + f"/api/v1/projects/{project['id']}/role-grants/{link_case_issue.json()['id']}/revoke", headers=auth_headers(project_reader_token) | {"Idempotency-Key": str(uuid4())}, json={"reason": "Remove reviewer authority after target link revocation"}, ) @@ -1859,13 +1839,9 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: assert concealed_replay.status_code == 404, concealed_replay.text concealed_replay_error = concealed_replay.json()["error"] assert { - key: value - for key, value in concealed_replay_error.items() - if key != "correlation_id" + key: value for key, value in concealed_replay_error.items() if key != "correlation_id" } == { - key: value - for key, value in missing_grant["error"].items() - if key != "correlation_id" + key: value for key, value in missing_grant["error"].items() if key != "correlation_id" } await request_json(client, "GET", f"/api/v1/tasks/{task['id']}", worker_token) ready_work_context = await request_json( @@ -1967,8 +1943,7 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: {"reason": "real worker claim"}, ) ensure( - claim["assignment"]["contributor_id"] - == canonical_actor["actor_profile_id"], + claim["assignment"]["contributor_id"] == canonical_actor["actor_profile_id"], "task claim did not return canonical contributor attribution", ) await request_json( @@ -2088,7 +2063,9 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: assert locked["locked_revision_policy_generation"] == 1 assert locked["locked_revision_policy_hash"] == screened["locked_revision_policy_hash"] assert locked["locked_payment_policy_version"] == "v1" - assert all(item["finalized_at"] == locked["finalized_at"] for item in locked["evidence_items"]) + assert all( + item["finalized_at"] == locked["finalized_at"] for item in locked["evidence_items"] + ) checker_run = await wait_for_submission_checker_run(client, manager_token, submission["id"]) assert checker_run["routing_recommendation"] == "allow_review" assert checker_run["triggered_by"] == "workstream-system:pre-review-gate" @@ -2125,9 +2102,10 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: assert finalized_event["external_subject"] == worker_subject assert finalized_event["external_issuer"] == flow_issuer assert finalized_event["auth_source"] == "flow" - assert finalized_event["event_payload"]["finalized_at"].replace("+00:00", "Z") == locked[ - "finalized_at" - ] + assert ( + finalized_event["event_payload"]["finalized_at"].replace("+00:00", "Z") + == locked["finalized_at"] + ) requester_actor_id = finalized_event["actor_id"] assert requester_actor_id assert requester_actor_id != "workstream-system:pre-review-gate" @@ -2150,8 +2128,7 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: ) assert all(event["claim_snapshot"] == {} for event in worker_audit_events) assert all( - "artifact_hash_manifest" not in event["event_payload"] - for event in worker_audit_events + "artifact_hash_manifest" not in event["event_payload"] for event in worker_audit_events ) await request_json( client, @@ -2169,6 +2146,7 @@ async def exercise_api_contract(base_url: str, env: dict[str, str]) -> None: print(f"submission_id={submission['id']}") print(f"submission_finalized_at={locked['finalized_at']}") + async def main(env: dict[str, str]) -> None: """Start the API server and exercise the backend API contract. diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 59321af7f..6d1bbdb20 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -93,7 +93,7 @@ ActionOwner.AUTH_12B2, ActionOwner.AUTH_12C, ActionOwner.AUTH_12D, - ActionOwner.AUTH_12D2, + ActionOwner.XINT_003_02B, ActionOwner.AUTH_12E, ActionOwner.AUTH_12F, ActionOwner.AUTH_12G, @@ -11831,9 +11831,7 @@ def test_xint003_02a_policy_lineage_backfill_immutability_and_roundtrip( RuntimeError, match="cannot downgrade populated immutable policy lineage" ): command.downgrade(config, "0045_guide_metadata_authority") - refused_state = asyncio.run( - _xint003_02a_policy_state(isolated_database_env, ids) - ) + refused_state = asyncio.run(_xint003_02a_policy_state(isolated_database_env, ids)) finally: asyncio.run(_remove_xint003_02a_immutable_policies(isolated_database_env, ids)) command.downgrade(config, "0045_guide_metadata_authority") @@ -11864,9 +11862,126 @@ def test_xint003_02a_policy_lineage_backfill_immutability_and_roundtrip( assert refused_state == state -async def _seed_xint003_02a_legacy_policies( - database_url: str, ids: dict[str, str] +def test_xint003_02b_policy_authority_schema_and_roundtrip( + isolated_database_env: str, + migration_lock, ) -> None: + """Prove 0048 installs only the closed policy mutation custody boundary.""" + project_root = Path(__file__).resolve().parents[1] + config = Config(str(project_root / "alembic.ini")) + config.set_main_option("script_location", str(project_root / "alembic")) + + with migration_lock(): + try: + command.downgrade(config, "0047_policy_identity_lineage") + command.upgrade(config, "0048_policy_authority") + shape = asyncio.run(_xint003_02b_authority_shape(isolated_database_env)) + command.downgrade(config, "0047_policy_identity_lineage") + absent = asyncio.run(_xint003_02b_authority_shape(isolated_database_env)) + finally: + command.upgrade(config, "head") + + assert shape == { + "ledger": True, + "review_provenance": 8, + "revision_provenance": 8, + "custody_triggers": 3, + "selector_constraint": True, + "selector_custody": True, + "predecessor_custody": True, + } + assert absent == { + "ledger": False, + "review_provenance": 0, + "revision_provenance": 0, + "custody_triggers": 0, + "selector_constraint": True, + "selector_custody": False, + "predecessor_custody": False, + } + + +async def _xint003_02b_authority_shape(database_url: str) -> dict[str, int | bool]: + engine = create_async_engine(database_url) + provenance = { + "predecessor_policy_hash", + "created_by_actor_profile_id", + "created_via_identity_link_id", + "created_by_admin_role_grant_id", + "creation_scope_type", + "creation_scope_project_id", + "creation_action_id", + "authorization_decision_event_id", + } + try: + async with engine.connect() as connection: + tables = set( + ( + await connection.execute( + text( + "select table_name from information_schema.tables " + "where table_schema='public'" + ) + ) + ).scalars() + ) + columns = {} + for table in ("review_policies", "revision_policies"): + columns[table] = set( + ( + await connection.execute( + text( + "select column_name from information_schema.columns " + "where table_schema='public' and table_name=:table" + ), + {"table": table}, + ) + ).scalars() + ) + triggers = int( + await connection.scalar( + text( + "select count(*) from pg_trigger where not tgisinternal and tgname in " + "('review_policy_mutation_custody'," + "'revision_policy_mutation_custody'," + "'policy_mutation_replay_custody')" + ) + ) + or 0 + ) + selector = bool( + await connection.scalar( + text( + "select exists(select 1 from pg_constraint where " + "conname='policy_selection_shape')" + ) + ) + ) + custody_definition = str( + await connection.scalar( + text( + "select pg_get_functiondef(p.oid) from pg_proc p " + "where p.proname='validate_policy_mutation_custody'" + ) + ) + or "" + ) + return { + "ledger": "policy_mutation_idempotency_records" in tables, + "review_provenance": len(columns["review_policies"] & provenance), + "revision_provenance": len(columns["revision_policies"] & provenance), + "custody_triggers": triggers, + "selector_constraint": selector, + "selector_custody": "selected_review_policy_id" in custody_definition + and "selected_revision_policy_id" in custody_definition, + "predecessor_custody": "prior.policy_generation=product_generation-1" + in custody_definition, + } + finally: + await engine.dispose() + + +async def _seed_xint003_02a_legacy_policies(database_url: str, ids: dict[str, str]) -> None: engine = create_async_engine(database_url) try: async with engine.begin() as connection: @@ -11894,7 +12009,7 @@ async def _seed_xint003_02a_legacy_policies( "insert into review_policies " "(id,project_id,guide_version,requires_second_review,allowed_decisions," "minimum_finding_fields,sla_hours) values " - "(:review,:project,'v1',false,'[\"accept\",\"needs_revision\"," + '(:review,:project,\'v1\',false,\'["accept","needs_revision",' "\"reject\"]'::json,'[]'::json,24)" ), ids, @@ -11912,9 +12027,7 @@ async def _seed_xint003_02a_legacy_policies( await engine.dispose() -async def _xint003_02a_policy_state( - database_url: str, ids: dict[str, str] -) -> dict[str, tuple]: +async def _xint003_02a_policy_state(database_url: str, ids: dict[str, str]) -> dict[str, tuple]: engine = create_async_engine(database_url) try: async with engine.connect() as connection: @@ -11958,9 +12071,7 @@ async def _xint003_02a_policy_state( await engine.dispose() -async def _xint003_02a_policy_immutable_writes( - database_url: str, ids: dict[str, str] -) -> set[str]: +async def _xint003_02a_policy_immutable_writes(database_url: str, ids: dict[str, str]) -> set[str]: engine = create_async_engine(database_url) refused: set[str] = set() try: @@ -11969,8 +12080,7 @@ async def _xint003_02a_policy_immutable_writes( with pytest.raises(IntegrityError): await connection.execute( text( - "update project_guides set selected_review_policy_hash=null " - "where id=:guide" + "update project_guides set selected_review_policy_hash=null where id=:guide" ), ids, ) @@ -12030,9 +12140,7 @@ async def _xint003_02a_policy_immutable_writes( await engine.dispose() -async def _remove_xint003_02a_immutable_policies( - database_url: str, ids: dict[str, str] -) -> None: +async def _remove_xint003_02a_immutable_policies(database_url: str, ids: dict[str, str]) -> None: engine = create_async_engine(database_url) try: async with engine.begin() as connection: @@ -12064,9 +12172,7 @@ async def _remove_xint003_02a_immutable_policies( ids, ) await connection.execute(text("delete from review_policies where id=:review"), ids) - await connection.execute( - text("delete from revision_policies where id=:revision"), ids - ) + await connection.execute(text("delete from revision_policies where id=:revision"), ids) await connection.execute(text("delete from project_guides where id=:guide"), ids) await connection.execute(text("delete from projects where id=:project"), ids) for table in reversed( diff --git a/backend/tests/test_artifact_admission.py b/backend/tests/test_artifact_admission.py index 4a380966a..4dc749e2f 100644 --- a/backend/tests/test_artifact_admission.py +++ b/backend/tests/test_artifact_admission.py @@ -569,66 +569,78 @@ async def _seed_checker_output_relationships(session) -> tuple[str, str, str]: ) ) await session.flush() - session.add_all( - [ - PostSubmitCheckerPolicy( - id=post_submit_policy_id, - project_id=project_id, - guide_id=guide_id, - guide_version=guide_version, - source_snapshot_id=snapshot_id, - source_snapshot_hash=snapshot_hash, - effective_policy_id=effective_policy_id, - effective_policy_hash=effective_policy_hash, - pre_submit_checker_policy_id=pre_submit_policy_id, - pre_submit_checker_bundle_hash=pre_submit_bundle_hash, - required_checkers=[], - warning_checkers=[], - blocking_severities=["error"], - policy_hash=post_submit_policy_hash, - policy_body=post_submit_policy_body, - lifecycle_status="approved", - approved_by_role="admin", - approved_by_actor="setup-actor", - approved_at=now, - created_by="setup-actor", - ), - ReviewPolicy( - id=review_policy_id, - project_id=project_id, - guide_version=guide_version, - policy_generation=1, - policy_hash=review_hash, - semantics_status="complete", - review_preference_window_seconds=3600, - review_lease_duration_seconds=1800, - max_active_review_leases_per_reviewer=1, - self_review_allowed=False, - reject_policy="close_task", - finding_evidence_requirement="optional", - requires_second_review=False, - allowed_decisions=["accept", "needs_revision", "reject"], - minimum_finding_fields=[], - ), - RevisionPolicy( - id=revision_policy_id, - project_id=project_id, - guide_version=guide_version, - policy_generation=1, - policy_hash=revision_hash, - semantics_status="complete", - max_revision_rounds=1, - revision_deadline_hours=24, - allowed_resubmission_states=["needs_revision"], - ), - PaymentPolicy( - id=str(uuid4()), - project_id=project_id, - guide_version=guide_version, - ), - ] - ) - await session.flush() + async with ( + suspend_historical_product_custody( + session, + table="review_policies", + triggers=("review_policy_mutation_custody",), + ), + suspend_historical_product_custody( + session, + table="revision_policies", + triggers=("revision_policy_mutation_custody",), + ), + ): + session.add_all( + [ + PostSubmitCheckerPolicy( + id=post_submit_policy_id, + project_id=project_id, + guide_id=guide_id, + guide_version=guide_version, + source_snapshot_id=snapshot_id, + source_snapshot_hash=snapshot_hash, + effective_policy_id=effective_policy_id, + effective_policy_hash=effective_policy_hash, + pre_submit_checker_policy_id=pre_submit_policy_id, + pre_submit_checker_bundle_hash=pre_submit_bundle_hash, + required_checkers=[], + warning_checkers=[], + blocking_severities=["error"], + policy_hash=post_submit_policy_hash, + policy_body=post_submit_policy_body, + lifecycle_status="approved", + approved_by_role="admin", + approved_by_actor="setup-actor", + approved_at=now, + created_by="setup-actor", + ), + ReviewPolicy( + id=review_policy_id, + project_id=project_id, + guide_version=guide_version, + policy_generation=1, + policy_hash=review_hash, + semantics_status="legacy_incomplete", + review_preference_window_seconds=3600, + review_lease_duration_seconds=1800, + max_active_review_leases_per_reviewer=1, + self_review_allowed=False, + reject_policy="close_task", + finding_evidence_requirement="optional", + requires_second_review=False, + allowed_decisions=["accept", "needs_revision", "reject"], + minimum_finding_fields=[], + ), + RevisionPolicy( + id=revision_policy_id, + project_id=project_id, + guide_version=guide_version, + policy_generation=1, + policy_hash=revision_hash, + semantics_status="legacy_incomplete", + max_revision_rounds=1, + revision_deadline_hours=24, + allowed_resubmission_states=["needs_revision"], + ), + PaymentPolicy( + id=str(uuid4()), + project_id=project_id, + guide_version=guide_version, + ), + ] + ) + await session.flush() async with suspend_historical_product_custody( session, table="project_guides", diff --git a/backend/tests/test_authorization.py b/backend/tests/test_authorization.py index 5983a0385..2b83fa151 100644 --- a/backend/tests/test_authorization.py +++ b/backend/tests/test_authorization.py @@ -1899,11 +1899,11 @@ def test_closed_permission_and_action_catalogue_is_exact_and_non_executable() -> ), "project.review_policy.update": ( "project.review_policy.manage", - "WS-AUTH-001-12D2", + "WS-XINT-003-02B", ), "project.revision_policy.update": ( "project.review_policy.manage", - "WS-AUTH-001-12D2", + "WS-XINT-003-02B", ), "project.guide_sufficiency_report.create": ( "project.guide.manage", @@ -1985,6 +1985,8 @@ def test_closed_permission_and_action_catalogue_is_exact_and_non_executable() -> ActionId.PROJECT_GUIDE_CREATE, ActionId.PROJECT_GUIDE_UPDATE, ActionId.PROJECT_GUIDE_SOURCE_SNAPSHOT_CREATE, + ActionId.PROJECT_REVIEW_POLICY_UPDATE, + ActionId.PROJECT_REVISION_POLICY_UPDATE, ActionId.PROJECT_READ, ActionId.ACTOR_AUTHORIZATION_CONTEXT_READ, ActionId.PROJECT_SETUP_RUN_READ, @@ -2085,14 +2087,14 @@ def test_closed_permission_and_action_catalogue_is_exact_and_non_executable() -> definition.availability is ActionAvailability.ACTIVE for definition in ACTION_DEFINITIONS ) - == 43 + == 45 ) assert ( sum( definition.availability is ActionAvailability.PLANNED for definition in ACTION_DEFINITIONS ) - == 53 + == 51 ) assert resolve_executable_action(ActionId.ACTOR_PROFILE_READ_SELF).permission_id is ( PermissionId.ACTOR_PROFILE_READ_SELF @@ -2172,20 +2174,28 @@ def test_project_mutation_resources_and_prepared_scopes_are_closed() -> None: review_resource = ProjectReviewPolicyMutationResourceContext( resource_type="project_review_policy_mutation", resource_id=review_id, + operation_id=operation_id, + request_digest=DIGEST, scope_project_id=project_id, guide_id=guide_id, guide_version="1", + guide_status="draft", review_policy_id=review_id, policy_generation=1, + policy_digest=DIGEST, ) revision_resource = ProjectRevisionPolicyMutationResourceContext( resource_type="project_revision_policy_mutation", resource_id=revision_id, + operation_id=operation_id, + request_digest=DIGEST, scope_project_id=project_id, guide_id=guide_id, guide_version="1", + guide_status="draft", revision_policy_id=revision_id, policy_generation=1, + policy_digest=DIGEST, ) sufficiency_resources = { ActionId.PROJECT_GUIDE_SUFFICIENCY_REPORT_CREATE: ( @@ -3538,9 +3548,7 @@ async def lock_request_actor(self, identity_link_id, actor_profile_id): actor_profile_id=str(actor_profile_id), status="active", ), - SimpleNamespace( - id=str(actor_profile_id), actor_kind="human", status="active" - ), + SimpleNamespace(id=str(actor_profile_id), actor_kind="human", status="active"), ) async def find_effective_grant( @@ -3561,9 +3569,16 @@ async def find_effective_grant( class _GuideMutationAuthorityFacts: - def __init__(self, context: HumanAuthorizationContext, *, grant=None) -> None: + def __init__( + self, + context: HumanAuthorizationContext, + *, + grant=None, + permission_id: PermissionId = PermissionId.PROJECT_GUIDE_MANAGE, + ) -> None: self.context = context self.grant = grant + self.permission_id = permission_id async def lock_request_actor(self, identity_link_id, actor_profile_id): assert identity_link_id == self.context.identity_link_id @@ -3574,9 +3589,7 @@ async def lock_request_actor(self, identity_link_id, actor_profile_id): actor_profile_id=str(actor_profile_id), status="active", ), - SimpleNamespace( - id=str(actor_profile_id), actor_kind="human", status="active" - ), + SimpleNamespace(id=str(actor_profile_id), actor_kind="human", status="active"), ) async def find_effective_grant( @@ -3589,7 +3602,7 @@ async def find_effective_grant( allowed_roles, ): assert actor_profile_id == self.context.actor_profile_id - assert permission_id is PermissionId.PROJECT_GUIDE_MANAGE + assert permission_id is self.permission_id assert scope_project_id is not None assert for_update is True assert allowed_roles == frozenset({AdminRole.PROJECT_MANAGER}) @@ -3628,11 +3641,12 @@ async def test_guide_source_metadata_authority_uses_exact_single_use_project_han scope_project_id=None if grant_scope == "system" else project_id, ), ) - authorization, evidence = _runtime_service( - context, session=session, admin_repository=facts - ) + authorization, evidence = _runtime_service(context, session=session, admin_repository=facts) prepared = PreparedAuthorizationService( - session, context, authorization, facts # type: ignore[arg-type] + session, + context, + authorization, + facts, # type: ignore[arg-type] ) operation_id = uuid4() target_resource_id = resource_id if resource_type == "snapshot" else guide_id @@ -3683,17 +3697,13 @@ async def test_guide_source_metadata_authority_uses_exact_single_use_project_han ) else: wrong_id = uuid4() - wrong_resource = resource.model_copy( - update={"resource_id": wrong_id, "guide_id": wrong_id} - ) + wrong_resource = resource.model_copy(update={"resource_id": wrong_id, "guide_id": wrong_id}) with pytest.raises(PreparedAuthorizationHandleInvalid): await prepared.consume(handle, action_id, caller, wrong_resource) decision = await prepared.consume(handle, action_id, caller, resource) assert decision.allowed is True assert decision.matched_grant_id == grant_id - assert decision.matched_scope_project_id == ( - None if grant_scope == "system" else project_id - ) + assert decision.matched_scope_project_id == (None if grant_scope == "system" else project_id) assert evidence.events[0].resource_type == "project" assert evidence.events[0].resource_id == str(project_id) assert evidence.events[0].after_facts == { @@ -3704,17 +3714,116 @@ async def test_guide_source_metadata_authority_uses_exact_single_use_project_han await prepared.consume(handle, action_id, caller, resource) +@pytest.mark.asyncio +@pytest.mark.parametrize( + "action_id,resource_type,policy_field", + [ + ( + ActionId.PROJECT_REVIEW_POLICY_UPDATE, + ProjectReviewPolicyMutationResourceContext, + "review_policy_id", + ), + ( + ActionId.PROJECT_REVISION_POLICY_UPDATE, + ProjectRevisionPolicyMutationResourceContext, + "revision_policy_id", + ), + ], +) +async def test_policy_mutation_authority_binds_full_lineage_and_evidence( + action_id, + resource_type, + policy_field, +) -> None: + context = _runtime_context() + assert isinstance(context, HumanAuthorizationContext) + session = _PreparedTestSession() + project_id, guide_id, policy_id, predecessor_id, grant_id = (uuid4() for _ in range(5)) + facts = _GuideMutationAuthorityFacts( + context, + grant=SimpleNamespace(id=grant_id, status="active", scope_project_id=project_id), + permission_id=PermissionId.PROJECT_REVIEW_POLICY_MANAGE, + ) + authorization, evidence = _runtime_service(context, session=session, admin_repository=facts) + prepared = PreparedAuthorizationService( + session, + context, + authorization, + facts, # type: ignore[arg-type] + ) + operation_id = uuid4() + caller = PreparedAuthorizationInput( + idempotency_key=uuid4(), + request_value={ + "project_id": str(project_id), + "guide_id": str(guide_id), + "policy_id": str(policy_id), + "operation_id": str(operation_id), + "request_digest": DIGEST, + "policy_digest": "sha256:" + "b" * 64, + "policy_generation": 2, + "predecessor_policy_id": str(predecessor_id), + "predecessor_policy_generation": 1, + "predecessor_policy_digest": "sha256:" + "c" * 64, + "guide_status": "draft", + }, + ) + handle = await prepared.prepare( + action_id, + caller, + PreparedAuthorityScope( + kind=PreparedAuthorityScopeKind.PROJECT, + project_id=project_id, + ), + ) + resource = resource_type( + resource_type=( + "project_review_policy_mutation" + if action_id is ActionId.PROJECT_REVIEW_POLICY_UPDATE + else "project_revision_policy_mutation" + ), + resource_id=policy_id, + operation_id=operation_id, + request_digest=DIGEST, + scope_project_id=project_id, + guide_id=guide_id, + guide_version="v1", + guide_status="draft", + **{policy_field: policy_id}, + policy_generation=2, + policy_digest="sha256:" + "b" * 64, + predecessor_policy_id=predecessor_id, + predecessor_policy_generation=1, + current_policy_digest="sha256:" + "c" * 64, + ) + with pytest.raises(PreparedAuthorizationHandleInvalid): + await prepared.consume( + handle, + action_id, + caller, + resource.model_copy(update={"policy_generation": 3}), + ) + decision = await prepared.consume(handle, action_id, caller, resource) + assert decision.allowed is True + assert decision.matched_grant_id == grant_id + assert evidence.events[0].resource_type == "project" + assert evidence.events[0].after_facts["resource_context_digest"] == ( + decision.resource_context_digest + ) + + @pytest.mark.asyncio async def test_guide_metadata_preparation_denies_wrong_scope_missing_grant_and_service() -> None: context = _runtime_context() assert isinstance(context, HumanAuthorizationContext) session = _PreparedTestSession() facts = _GuideMutationAuthorityFacts(context) - authorization, evidence = _runtime_service( - context, session=session, admin_repository=facts - ) + authorization, evidence = _runtime_service(context, session=session, admin_repository=facts) prepared = PreparedAuthorizationService( - session, context, authorization, facts # type: ignore[arg-type] + session, + context, + authorization, + facts, # type: ignore[arg-type] ) project_id = uuid4() guide_id = uuid4() @@ -3775,11 +3884,12 @@ async def test_project_create_prepared_authority_is_system_scoped_and_evidenced( facts = _ProjectCreateAuthorityFacts( context, grant=SimpleNamespace(id=grant_id, status="active") ) - authorization, evidence = _runtime_service( - context, session=session, admin_repository=facts - ) + authorization, evidence = _runtime_service(context, session=session, admin_repository=facts) prepared = PreparedAuthorizationService( - session, context, authorization, facts # type: ignore[arg-type] + session, + context, + authorization, + facts, # type: ignore[arg-type] ) operation_id, project_id, key = uuid4(), uuid4(), uuid4() caller_input = PreparedAuthorizationInput( @@ -3845,11 +3955,12 @@ async def test_project_create_preparation_denies_wrong_scope_missing_grant_and_s assert isinstance(context, HumanAuthorizationContext) session = _PreparedTestSession() facts = _ProjectCreateAuthorityFacts(context) - authorization, evidence = _runtime_service( - context, session=session, admin_repository=facts - ) + authorization, evidence = _runtime_service(context, session=session, admin_repository=facts) prepared = PreparedAuthorizationService( - session, context, authorization, facts # type: ignore[arg-type] + session, + context, + authorization, + facts, # type: ignore[arg-type] ) operation_id = uuid4() project_id = uuid4() @@ -3873,9 +3984,7 @@ async def test_project_create_preparation_denies_wrong_scope_missing_grant_and_s await prepared.prepare( ActionId.PROJECT_CREATE, caller_input, - PreparedAuthorityScope( - kind=PreparedAuthorityScopeKind.PROJECT, project_id=uuid4() - ), + PreparedAuthorityScope(kind=PreparedAuthorityScopeKind.PROJECT, project_id=uuid4()), ) assert scoped.value.denial_code is AuthorizationDenialCode.SCOPE_NOT_AUTHORIZED assert evidence.events == [] diff --git a/backend/tests/test_policy_identity_lineage.py b/backend/tests/test_policy_identity_lineage.py index b7b34eb39..ee7204fb6 100644 --- a/backend/tests/test_policy_identity_lineage.py +++ b/backend/tests/test_policy_identity_lineage.py @@ -115,12 +115,12 @@ def test_xint003_02a_fixed_v01_review_guards_cannot_be_weakened() -> None: ) -def test_xint003_02a_policy_actions_remain_unavailable() -> None: +def test_xint003_02b_policy_actions_are_narrowly_active() -> None: assert ( ACTION_BY_ID[ActionId.PROJECT_REVIEW_POLICY_UPDATE].availability - is ActionAvailability.PLANNED + is ActionAvailability.ACTIVE ) assert ( ACTION_BY_ID[ActionId.PROJECT_REVISION_POLICY_UPDATE].availability - is ActionAvailability.PLANNED + is ActionAvailability.ACTIVE ) diff --git a/backend/tests/test_project_policy_mutations.py b/backend/tests/test_project_policy_mutations.py new file mode 100644 index 000000000..33e5b7944 --- /dev/null +++ b/backend/tests/test_project_policy_mutations.py @@ -0,0 +1,654 @@ +"""Focused proof for the sole review/revision policy mutation path.""" + +from __future__ import annotations + +from datetime import UTC, datetime +from dataclasses import replace +from types import SimpleNamespace +from uuid import uuid4 + +import pytest +from fastapi import HTTPException + +from app.core.hashing import canonical_json_hash +from app.main import create_app +from app.modules.projects import policy_mutation_router as router_module +from app.modules.projects.models import PolicyMutationIdempotencyRecord +from app.modules.projects.policy_mutation_replay_repository import ( + PolicyMutationReplayRepository, +) +from app.modules.authorization.catalogue import ActionId +from app.modules.audit.schemas import ActorReferenceKind +from app.modules.authorization.prepared import ( + _PreparedAuthorizationBinding, + _policy_mutation_binding_matches, +) +from app.modules.authorization.runtime import ( + MatchedAuthorityKind, + PreparedAuthorityScope, + PreparedAuthorityScopeKind, + ProjectReviewPolicyMutationResourceContext, +) +from app.modules.projects.policy_mutation_service import ( + NO_CURRENT_POLICY_ETAG, + PolicyMutationConflict, + ProjectPolicyMutationService, + policy_selector_etag, +) +from app.modules.projects.policy_mutation_router import require_policy_mutation_key +from app.modules.projects.schemas import ReviewPolicyInput, RevisionPolicyInput +from app.modules.projects.service import ProjectNotFound + + +def _review_payload() -> ReviewPolicyInput: + return ReviewPolicyInput( + review_preference_window_seconds=3600, + review_lease_duration_seconds=7200, + allowed_decisions=["accept", "needs_revision", "reject"], + ) + + +def _revision_payload() -> RevisionPolicyInput: + return RevisionPolicyInput( + max_revision_rounds=3, + revision_deadline_hours=48, + allowed_resubmission_states=["needs_revision"], + ) + + +class _Repository: + def __init__(self, project_id, guide_id) -> None: + self.guide = SimpleNamespace( + id=str(guide_id), + project_id=str(project_id), + version="v1", + status="draft", + selected_review_policy_id=None, + selected_review_policy_generation=None, + selected_review_policy_hash=None, + selected_revision_policy_id=None, + selected_revision_policy_generation=None, + selected_revision_policy_hash=None, + ) + self.review = None + self.revision = None + + async def lock_project_guide(self, _guide_id): + return self.guide + + async def get_guide(self, _guide_id): + return self.guide + + async def lock_review_policy(self, _project_id, _guide_version): + return self.review + + async def lock_revision_policy(self, _project_id, _guide_version): + return self.revision + + async def add_review_policy_version(self, policy, guide): + policy.created_at = datetime.now(UTC) + self.review = policy + guide.selected_review_policy_id = policy.id + guide.selected_review_policy_generation = policy.policy_generation + guide.selected_review_policy_hash = policy.policy_hash + return policy + + async def add_revision_policy_version(self, policy, guide): + policy.created_at = datetime.now(UTC) + self.revision = policy + guide.selected_revision_policy_id = policy.id + guide.selected_revision_policy_generation = policy.policy_generation + guide.selected_revision_policy_hash = policy.policy_hash + return policy + + +class _Replay: + def __init__(self) -> None: + self.records = {} + self.completed = 0 + + async def find(self, actor_profile_id, action_id, idempotency_key): + return self.records.get((actor_profile_id, action_id, idempotency_key)) + + async def reserve(self, **facts): + record = SimpleNamespace(**facts, status="pending", response_json=None) + self.records[(facts["actor_profile_id"], facts["action_id"], facts["idempotency_key"])] = ( + record + ) + return "claimed", record + + async def complete(self, record, *, response_json): + record.status = "committed" + record.response_json = response_json + self.completed += 1 + + +class _Prepared: + def __init__(self, project_id, grant_id) -> None: + self.project_id = project_id + self.grant_id = grant_id + self.prepared = [] + self.consumed = [] + + async def prepare(self, action, caller, scope): + self.prepared.append((action, caller, scope)) + return object() + + async def consume(self, handle, action, caller, resource): + self.consumed.append((handle, action, caller, resource)) + return SimpleNamespace( + matched_authority_kind=MatchedAuthorityKind.ADMIN_ROLE_GRANT, + matched_grant_id=self.grant_id, + matched_scope_project_id=self.project_id, + resource_context_digest=canonical_json_hash(resource.model_dump(mode="json")), + decision_id=uuid4(), + ) + + +def _subject(): + project_id, guide_id, actor_id, link_id, grant_id = (uuid4() for _ in range(5)) + resolved = SimpleNamespace( + profile=SimpleNamespace(id=str(actor_id)), + identity_link=SimpleNamespace(id=str(link_id)), + ) + repository = _Repository(project_id, guide_id) + replay = _Replay() + prepared = _Prepared(project_id, grant_id) + service = ProjectPolicyMutationService(SimpleNamespace()) + service._projects = repository # type: ignore[assignment] + service._replay = replay # type: ignore[assignment] + return service, resolved, prepared, replay, repository, project_id, guide_id + + +def test_policy_mutation_routes_declare_only_their_exact_primary_actions() -> None: + schema = create_app().openapi() + routes = { + "/api/v1/projects/{project_id}/guides/{guide_id}/review-policy": ( + "project.review_policy.update" + ), + "/api/v1/projects/{project_id}/guides/{guide_id}/revision-policy": ( + "project.revision_policy.update" + ), + } + for path, action in routes.items(): + operation = schema["paths"][path]["put"] + assert operation["x-workstream-action-id"] == action + assert set(schema["paths"][path]) == {"put"} + key = uuid4() + assert require_policy_mutation_key(str(key)) == key + + +def test_policy_prepared_binding_rejects_every_changed_lineage_fact() -> None: + project_id, guide_id, policy_id, operation_id, predecessor_id = (uuid4() for _ in range(5)) + request_digest = "sha256:" + "1" * 64 + policy_digest_value = "sha256:" + "2" * 64 + predecessor_digest = "sha256:" + "3" * 64 + binding = _PreparedAuthorizationBinding( + action_id=ActionId.PROJECT_REVIEW_POLICY_UPDATE, + actor_ref_kind=ActorReferenceKind.ACTOR_PROFILE, + actor_ref=uuid4(), + scope=PreparedAuthorityScope( + kind=PreparedAuthorityScopeKind.PROJECT, + project_id=project_id, + ), + idempotency_key=uuid4(), + request_digest="sha256:" + "4" * 64, + policy_mutation_project_id=project_id, + policy_mutation_guide_id=guide_id, + policy_mutation_policy_id=policy_id, + policy_mutation_operation_id=operation_id, + policy_mutation_request_digest=request_digest, + policy_mutation_policy_digest=policy_digest_value, + policy_mutation_generation=2, + policy_mutation_predecessor_id=predecessor_id, + policy_mutation_predecessor_generation=1, + policy_mutation_predecessor_digest=predecessor_digest, + policy_mutation_guide_status="draft", + ) + resource = ProjectReviewPolicyMutationResourceContext( + resource_type="project_review_policy_mutation", + resource_id=policy_id, + operation_id=operation_id, + request_digest=request_digest, + scope_project_id=project_id, + guide_id=guide_id, + guide_version="v1", + guide_status="draft", + review_policy_id=policy_id, + policy_generation=2, + policy_digest=policy_digest_value, + predecessor_policy_id=predecessor_id, + predecessor_policy_generation=1, + current_policy_digest=predecessor_digest, + ) + assert _policy_mutation_binding_matches(binding, resource) + assert not _policy_mutation_binding_matches( + replace(binding, policy_mutation_generation=3), resource + ) + assert not _policy_mutation_binding_matches( + replace(binding, policy_mutation_predecessor_id=uuid4()), resource + ) + assert not _policy_mutation_binding_matches( + replace(binding, policy_mutation_predecessor_digest="sha256:" + "5" * 64), + resource, + ) + assert not _policy_mutation_binding_matches( + replace(binding, policy_mutation_guide_status="active"), resource + ) + + +@pytest.mark.asyncio +async def test_separate_policy_routes_append_in_either_order_with_exact_authority() -> None: + service, resolved, prepared, replay, repository, project_id, guide_id = _subject() + revision = await service.replace_revision_policy( + resolved, + prepared, + uuid4(), + NO_CURRENT_POLICY_ETAG, + project_id, + guide_id, + _revision_payload(), + ) + review = await service.replace_review_policy( + resolved, + prepared, + uuid4(), + NO_CURRENT_POLICY_ETAG, + project_id, + guide_id, + _review_payload(), + ) + + assert revision.response.policy_generation == review.response.policy_generation == 1 + assert repository.guide.selected_review_policy_id == review.response.id + assert repository.guide.selected_revision_policy_id == revision.response.id + assert [call[0] for call in prepared.prepared] == [ + ActionId.PROJECT_REVISION_POLICY_UPDATE, + ActionId.PROJECT_REVIEW_POLICY_UPDATE, + ] + assert all(call[3].guide_status == "draft" for call in prepared.consumed) + assert replay.completed == 2 + + +@pytest.mark.asyncio +async def test_replay_claim_precedes_prepared_authority() -> None: + service, resolved, prepared, replay, _repository, project_id, guide_id = _subject() + key = uuid4() + original_prepare = prepared.prepare + + async def assert_claimed_before_prepare(*args, **kwargs): + record = replay.records[ + (resolved.profile.id, ActionId.PROJECT_REVIEW_POLICY_UPDATE.value, key) + ] + assert record.status == "pending" + return await original_prepare(*args, **kwargs) + + prepared.prepare = assert_claimed_before_prepare + await service.replace_review_policy( + resolved, + prepared, + key, + NO_CURRENT_POLICY_ETAG, + project_id, + guide_id, + _review_payload(), + ) + + +@pytest.mark.asyncio +async def test_policy_replacement_binds_predecessor_and_exact_etag() -> None: + service, resolved, prepared, _replay, repository, project_id, guide_id = _subject() + first = await service.replace_review_policy( + resolved, + prepared, + uuid4(), + NO_CURRENT_POLICY_ETAG, + project_id, + guide_id, + _review_payload(), + ) + second = await service.replace_review_policy( + resolved, + prepared, + uuid4(), + policy_selector_etag( + first.response.id, + first.response.policy_generation, + first.response.policy_hash, + ), + project_id, + guide_id, + _review_payload().model_copy(update={"review_lease_duration_seconds": 9000}), + ) + + assert second.response.policy_generation == 2 + assert second.response.supersedes_policy_id == first.response.id + resource = prepared.consumed[-1][3] + assert str(resource.predecessor_policy_id) == first.response.id + assert resource.current_policy_digest == first.response.policy_hash + assert repository.review.predecessor_policy_hash == first.response.policy_hash + + +@pytest.mark.asyncio +async def test_stale_or_unquoted_policy_precondition_fails_without_consumption() -> None: + service, resolved, prepared, replay, _repository, project_id, guide_id = _subject() + with pytest.raises(PolicyMutationConflict, match="policy_precondition_invalid"): + await service.replace_review_policy( + resolved, + prepared, + uuid4(), + "sha256:" + "0" * 64, + project_id, + guide_id, + _review_payload(), + ) + for invalid_generation in ("0", "not-an-integer"): + with pytest.raises(PolicyMutationConflict, match="policy_precondition_invalid"): + await service.replace_review_policy( + resolved, + prepared, + uuid4(), + f'"{uuid4()}.{invalid_generation}.{"0" * 64}"', + project_id, + guide_id, + _review_payload(), + ) + assert not prepared.prepared + assert replay.completed == 0 + + with pytest.raises(PolicyMutationConflict, match="policy_precondition_failed"): + await service.replace_review_policy( + resolved, + prepared, + uuid4(), + policy_selector_etag(str(uuid4()), 1, "sha256:" + "0" * 64), + project_id, + guide_id, + _review_payload(), + ) + assert not prepared.prepared + assert not prepared.consumed + assert replay.completed == 0 + + +@pytest.mark.asyncio +async def test_exact_committed_replay_returns_without_new_prep_or_write() -> None: + service, resolved, prepared, replay, repository, project_id, guide_id = _subject() + key = uuid4() + first = await service.replace_revision_policy( + resolved, + prepared, + key, + NO_CURRENT_POLICY_ETAG, + project_id, + guide_id, + _revision_payload(), + ) + prepared.prepared.clear() + prepared.consumed.clear() + resolved.identity_link.id = str(uuid4()) + second = await service.replace_revision_policy( + resolved, + prepared, + key, + NO_CURRENT_POLICY_ETAG, + project_id, + guide_id, + _revision_payload(), + ) + + assert second.replayed is True + assert second.response == first.response + assert not prepared.prepared and not prepared.consumed + assert repository.revision.id == first.response.id + assert replay.completed == 1 + + +@pytest.mark.asyncio +async def test_replay_repository_owns_claim_classification_and_completion() -> None: + actor_id, link_id, project_id, guide_id = (str(uuid4()) for _ in range(4)) + key, operation_id, record_id = uuid4(), uuid4(), uuid4() + digest = "sha256:" + "1" * 64 + resource_digest = "sha256:" + "2" * 64 + + class Session: + scalar_values = [] + record = None + + async def scalar(self, _statement): + return self.scalar_values.pop(0) + + async def get(self, model, selected_id): + assert model is PolicyMutationIdempotencyRecord + assert selected_id == record_id + return self.record + + session = Session() + repository = PolicyMutationReplayRepository(session) # type: ignore[arg-type] + found = SimpleNamespace(id=record_id) + session.scalar_values = [found] + assert ( + await repository.find(actor_id, ActionId.PROJECT_REVIEW_POLICY_UPDATE.value, key) is found + ) + + record = SimpleNamespace( + id=record_id, + identity_link_id=link_id, + project_id=project_id, + guide_id=guide_id, + request_digest=digest, + policy_hash="sha256:" + "3" * 64, + status="pending", + ) + session.record = record + session.scalar_values = [record_id] + disposition, selected = await repository.reserve( + actor_profile_id=actor_id, + identity_link_id=link_id, + action_id=ActionId.PROJECT_REVIEW_POLICY_UPDATE.value, + idempotency_key=key, + request_digest=digest, + policy_hash="sha256:" + "3" * 64, + resource_context_digest=resource_digest, + operation_id=operation_id, + project_id=project_id, + guide_id=guide_id, + policy_id=str(uuid4()), + policy_generation=1, + ) + assert disposition == "pending" and selected is record + + record.status = "committed" + session.scalar_values = [record_id] + disposition, _ = await repository.reserve( + actor_profile_id=actor_id, + identity_link_id=link_id, + action_id=ActionId.PROJECT_REVIEW_POLICY_UPDATE.value, + idempotency_key=key, + request_digest=digest, + policy_hash="sha256:" + "3" * 64, + resource_context_digest=resource_digest, + operation_id=operation_id, + project_id=project_id, + guide_id=guide_id, + policy_id=str(uuid4()), + policy_generation=1, + ) + assert disposition == "replayed" + + session.scalar_values = [record_id] + await repository.complete(record, response_json={"id": "response"}) + + +@pytest.mark.asyncio +async def test_policy_router_dependencies_errors_and_transaction_outcomes(monkeypatch) -> None: + key = uuid4() + with pytest.raises(HTTPException): + require_policy_mutation_key("not-a-uuid") + + resolved = object() + + async def resolve(*_args): + return resolved + + monkeypatch.setattr(router_module, "resolve_authorization_actor", resolve) + assert ( + await router_module.policy_authorization_actor(key, object(), object(), object(), object()) + is resolved + ) + + prepared = object() + + class PreparedContext: + async def __aenter__(self): + return prepared + + async def __aexit__(self, *_args): + return None + + monkeypatch.setattr( + router_module, + "prepared_authorization_service", + lambda *_args: PreparedContext(), + ) + dependency = router_module.get_policy_prepared_authorization_service( + object(), resolved, object() + ) + assert await anext(dependency) is prepared + with pytest.raises(StopAsyncIteration): + await anext(dependency) + assert await router_module.policy_authorization(key, resolved, prepared) == ( + key, + resolved, + prepared, + ) + + pending = router_module._error(PolicyMutationConflict("idempotency_pending")) + assert pending.status_code == 409 and pending.retryable is True + missing = router_module._error(ProjectNotFound("project not found")) + assert missing.status_code == 404 + + class Session: + committed = rolled_back = 0 + + async def commit(self): + self.committed += 1 + + async def rollback(self): + self.rolled_back += 1 + + session = Session() + response = object() + assert ( + await router_module._finish(session, SimpleNamespace(response=response, replayed=False)) + is response + ) + assert ( + await router_module._finish(session, SimpleNamespace(response=response, replayed=True)) + is response + ) + assert (session.committed, session.rolled_back) == (1, 1) + + review_response = object() + revision_response = object() + + class Service: + def __init__(self, selected_session): + assert selected_session is session + + async def replace_review_policy(self, *_args): + return SimpleNamespace(response=review_response, replayed=False) + + async def replace_revision_policy(self, *_args): + return SimpleNamespace(response=revision_response, replayed=False) + + monkeypatch.setattr(router_module, "ProjectPolicyMutationService", Service) + authorization = (key, resolved, prepared) + assert ( + await router_module.replace_review_policy( + uuid4(), + uuid4(), + _review_payload(), + NO_CURRENT_POLICY_ETAG, + authorization, + session, + ) + is review_response + ) + assert ( + await router_module.replace_revision_policy( + uuid4(), + uuid4(), + _revision_payload(), + NO_CURRENT_POLICY_ETAG, + authorization, + session, + ) + is revision_response + ) + + class ErrorService(Service): + async def replace_review_policy(self, *_args): + raise PolicyMutationConflict("policy_precondition_failed") + + async def replace_revision_policy(self, *_args): + raise PolicyMutationConflict("policy_precondition_failed") + + monkeypatch.setattr(router_module, "ProjectPolicyMutationService", ErrorService) + with pytest.raises(HTTPException): + await router_module.replace_review_policy( + uuid4(), + uuid4(), + _review_payload(), + NO_CURRENT_POLICY_ETAG, + authorization, + session, + ) + with pytest.raises(HTTPException): + await router_module.replace_revision_policy( + uuid4(), + uuid4(), + _revision_payload(), + NO_CURRENT_POLICY_ETAG, + authorization, + session, + ) + + +@pytest.mark.asyncio +async def test_policy_service_denies_stale_guide_and_replay_mismatch() -> None: + service, resolved, prepared, replay, repository, project_id, guide_id = _subject() + repository.guide.status = "active" + with pytest.raises(Exception, match="only draft guides"): + await service.replace_revision_policy( + resolved, + prepared, + uuid4(), + NO_CURRENT_POLICY_ETAG, + project_id, + guide_id, + _revision_payload(), + ) + assert not prepared.consumed + + key = uuid4() + replay.records[(resolved.profile.id, ActionId.PROJECT_REVISION_POLICY_UPDATE.value, key)] = ( + SimpleNamespace( + identity_link_id=str(uuid4()), + project_id=str(project_id), + guide_id=str(guide_id), + request_digest="sha256:" + "0" * 64, + status="committed", + response_json={}, + ) + ) + with pytest.raises(PolicyMutationConflict, match="idempotency_mismatch"): + await service.replace_revision_policy( + resolved, + prepared, + key, + NO_CURRENT_POLICY_ETAG, + project_id, + guide_id, + _revision_payload(), + ) diff --git a/backend/tests/test_projects.py b/backend/tests/test_projects.py index 88590d012..5caf2a473 100644 --- a/backend/tests/test_projects.py +++ b/backend/tests/test_projects.py @@ -17,7 +17,7 @@ from httpx import ASGITransport, AsyncClient from sqlalchemy import event, func, select, text, update from sqlalchemy.dialects import postgresql -from sqlalchemy.exc import IntegrityError +from sqlalchemy.exc import DBAPIError, IntegrityError from fastapi import HTTPException from sqlalchemy.schema import CreateIndex @@ -54,6 +54,7 @@ GuideSourceSnapshotItem, GuideSufficiencyReport, PaymentPolicy, + PolicyMutationIdempotencyRecord, PostSubmitCheckerPolicy, PreSubmitCheckerPolicy, Project, @@ -64,11 +65,6 @@ ReviewPolicy, SubmissionArtifactPolicy, ) -from app.modules.projects.policy_lineage import ( - ReviewPolicySemantics, - RevisionPolicySemantics, - policy_digest, -) from app.modules.projects.guide_mutation_repository import GuideMutationRepository from app.modules.tasks.models import AuditEvent from app.modules.authorization.models import ( @@ -164,9 +160,7 @@ async def scalar(self, statement: Any) -> None: await repository.lock_review_policy("project-id", "v1") await repository.lock_revision_policy("project-id", "v1") - rendered = [ - str(statement.compile(dialect=postgresql.dialect())) for statement in statements - ] + rendered = [str(statement.compile(dialect=postgresql.dialect())) for statement in statements] assert "FOR UPDATE OF review_policies" in rendered[0] assert "FOR UPDATE OF project_guides" not in rendered[0] assert "FOR UPDATE OF revision_policies" in rendered[1] @@ -2057,71 +2051,51 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d ) assert response.status_code == 201, response.text guide = response.json() - async with db_session.get_session_factory()() as session: - review_id = revision_id = None - if review_policy is not None: - values = ( - review_policy - if isinstance(review_policy, dict) - else { - "requires_second_review": False, - "allowed_decisions": ["accept", "needs_revision", "reject"], - "minimum_finding_fields": ["issue", "required_fix"], - } - ) - values.pop("sla_hours", None) - values = { - "review_preference_window_seconds": 3600, - "review_lease_duration_seconds": 1800, - "max_active_review_leases_per_reviewer": 1, - "self_review_allowed": False, - "reject_policy": "close_task", - "finding_evidence_requirement": "optional", - **values, + if review_policy is not None: + values = ( + review_policy + if isinstance(review_policy, dict) + else { + "requires_second_review": False, + "allowed_decisions": ["accept", "needs_revision", "reject"], + "minimum_finding_fields": ["issue", "required_fix"], } - review_hash = policy_digest( - "review", - ReviewPolicySemantics.model_validate(values), - ) - review_id = str(uuid4()) - session.add( - ReviewPolicy( - id=review_id, - project_id=project_id, - guide_version=guide["version"], - policy_generation=1, - policy_hash=review_hash, - semantics_status="complete", - **values, - ) - ) - if revision_policy is not None: - values = ( - revision_policy - if isinstance(revision_policy, dict) - else { - "max_revision_rounds": 7, - "revision_deadline_hours": 48, - "allowed_resubmission_states": ["needs_revision"], - "reviewer_reassignment_rule": "same reviewer preferred", - } - ) - values.pop("auto_reject_after_limit", None) - revision_hash = policy_digest( - "revision", RevisionPolicySemantics.model_validate(values) - ) - revision_id = str(uuid4()) - session.add( - RevisionPolicy( - id=revision_id, - project_id=project_id, - guide_version=guide["version"], - policy_generation=1, - policy_hash=revision_hash, - semantics_status="complete", - **values, - ) - ) + ) + values.pop("sla_hours", None) + values = { + "review_preference_window_seconds": 3600, + "review_lease_duration_seconds": 1800, + "max_active_review_leases_per_reviewer": 1, + "self_review_allowed": False, + "reject_policy": "close_task", + "finding_evidence_requirement": "optional", + **values, + } + policy_response = await client.put( + f"/api/v1/projects/{project_id}/guides/{guide['id']}/review-policy", + headers=auth_headers() | {"If-Match": '"no-current-policy"'}, + json=values, + ) + assert policy_response.status_code == 200, policy_response.text + if revision_policy is not None: + values = ( + revision_policy + if isinstance(revision_policy, dict) + else { + "max_revision_rounds": 7, + "revision_deadline_hours": 48, + "allowed_resubmission_states": ["needs_revision"], + "reviewer_reassignment_rule": "same reviewer preferred", + } + ) + values.pop("auto_reject_after_limit", None) + policy_response = await client.put( + f"/api/v1/projects/{project_id}/guides/{guide['id']}/revision-policy", + headers=auth_headers() | {"If-Match": '"no-current-policy"'}, + json=values, + ) + assert policy_response.status_code == 200, policy_response.text + async with db_session.get_session_factory()() as session: if payment_policy is not None: values = ( payment_policy @@ -2143,15 +2117,6 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d **values, ) ) - guide_row = await session.get(ProjectGuide, guide["id"]) - assert guide_row is not None - if review_id is not None and revision_id is not None: - guide_row.selected_review_policy_id = review_id - guide_row.selected_review_policy_generation = 1 - guide_row.selected_review_policy_hash = review_hash - guide_row.selected_revision_policy_id = revision_id - guide_row.selected_revision_policy_generation = 1 - guide_row.selected_revision_policy_hash = revision_hash await session.commit() await add_project_manager_admin_grant(project_id) if source_snapshot is not None: @@ -2159,6 +2124,76 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d return guide +@pytest.mark.asyncio +async def test_policy_mutation_api_commits_exact_custody_and_rejects_direct_append( + project_client: AsyncClient, +) -> None: + project = await create_project(project_client, name="Policy custody") + payload = complete_guide_payload() + payload["review_policy"] = None + payload["revision_policy"] = None + guide = await create_guide(project_client, project["id"], payload) + headers = auth_headers() | {"If-Match": '"no-current-policy"'} + body = { + "review_preference_window_seconds": 3600, + "review_lease_duration_seconds": 1800, + "max_active_review_leases_per_reviewer": 1, + "self_review_allowed": False, + "reject_policy": "close_task", + "finding_evidence_requirement": "optional", + "requires_second_review": False, + "allowed_decisions": ["accept", "needs_revision", "reject"], + "minimum_finding_fields": ["issue", "required_fix"], + } + path = f"/api/v1/projects/{project['id']}/guides/{guide['id']}/review-policy" + created = await project_client.put(path, headers=headers, json=body) + replayed = await project_client.put(path, headers=headers, json=body) + assert created.status_code == replayed.status_code == 200 + assert created.json() == replayed.json() + + async with db_session.get_session_factory()() as session: + policy = await session.get(ReviewPolicy, created.json()["id"]) + assert policy is not None + replay_count = await session.scalar( + select(func.count()) + .select_from(PolicyMutationIdempotencyRecord) + .where(PolicyMutationIdempotencyRecord.policy_id == policy.id) + ) + assert replay_count == 1 + session.add( + ReviewPolicy( + id=str(uuid4()), + project_id=policy.project_id, + guide_version=policy.guide_version, + policy_generation=2, + policy_hash=policy.policy_hash, + semantics_status="complete", + supersedes_policy_id=policy.id, + predecessor_policy_hash=policy.policy_hash, + created_by_actor_profile_id=policy.created_by_actor_profile_id, + created_via_identity_link_id=policy.created_via_identity_link_id, + created_by_admin_role_grant_id=policy.created_by_admin_role_grant_id, + creation_scope_type=policy.creation_scope_type, + creation_scope_project_id=policy.creation_scope_project_id, + creation_action_id=policy.creation_action_id, + authorization_decision_event_id=policy.authorization_decision_event_id, + review_preference_window_seconds=policy.review_preference_window_seconds, + review_lease_duration_seconds=policy.review_lease_duration_seconds, + max_active_review_leases_per_reviewer=( + policy.max_active_review_leases_per_reviewer + ), + self_review_allowed=policy.self_review_allowed, + reject_policy=policy.reject_policy, + finding_evidence_requirement=policy.finding_evidence_requirement, + requires_second_review=policy.requires_second_review, + allowed_decisions=policy.allowed_decisions, + minimum_finding_fields=policy.minimum_finding_fields, + ) + ) + with pytest.raises(DBAPIError, match="policy mutation custody mismatch"): + await session.commit() + + async def test_create_guide_never_enqueues_setup_or_runs_agents( project_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, @@ -7137,9 +7172,13 @@ async def run(_: FakeAgent, prompt: str) -> object: ), ) material = GuideSourceMaterial( - project_id="project-1", guide_id="guide-1", guide_version="v1", - source_snapshot_id="snapshot-1", source_snapshot_hash="sha256:" + "1" * 64, - guide_material={}, verified_artifact_material=True, + project_id="project-1", + guide_id="guide-1", + guide_version="v1", + source_snapshot_id="snapshot-1", + source_snapshot_hash="sha256:" + "1" * 64, + guide_material={}, + verified_artifact_material=True, ) runtime = OpenAIAgentSdkProjectGuideRuntime( Settings(project_agent_openai_agent_sdk_model="gpt-test") diff --git a/backend/tests/test_tasks.py b/backend/tests/test_tasks.py index 0c8545bce..6b176b6ca 100644 --- a/backend/tests/test_tasks.py +++ b/backend/tests/test_tasks.py @@ -54,19 +54,12 @@ PreSubmitCheckerPolicy, ProjectGuide, ProjectSetupRun, - ReviewPolicy, - RevisionPolicy, SubmissionArtifactPolicy, ) from app.modules.projects.post_submit_policy import ( build_project_post_submit_checker_spec, compile_project_post_submit_checker_spec, ) -from app.modules.projects.policy_lineage import ( - ReviewPolicySemantics, - RevisionPolicySemantics, - policy_digest, -) from app.modules.tasks.lifecycle import InvalidTaskTransition, ensure_allowed_transition from app.modules.tasks.models import ( AuditEvent, @@ -959,60 +952,42 @@ async def create_policy_bundle_for_guide( post_submit_warning_checkers: list[str] | None = None, post_submit_blocking_severities: list[str] | None = None, ) -> dict: - async with db_session.get_session_factory()() as session: - guide = await session.get(ProjectGuide, guide_id) - assert guide is not None - review_policy_id = str(uuid4()) - revision_policy_id = str(uuid4()) - review_hash = policy_digest( + for kind, body in ( + ( "review", - ReviewPolicySemantics( - review_preference_window_seconds=3600, - review_lease_duration_seconds=1800, - allowed_decisions=("accept", "needs_revision", "reject"), - minimum_finding_fields=("issue", "required_fix"), - ), - ) - revision_hash = policy_digest( + { + "review_preference_window_seconds": 3600, + "review_lease_duration_seconds": 1800, + "max_active_review_leases_per_reviewer": 1, + "self_review_allowed": False, + "reject_policy": "close_task", + "finding_evidence_requirement": "optional", + "requires_second_review": False, + "allowed_decisions": ["accept", "needs_revision", "reject"], + "minimum_finding_fields": ["issue", "required_fix"], + }, + ), + ( "revision", - RevisionPolicySemantics( - max_revision_rounds=7, - revision_deadline_hours=48, - allowed_resubmission_states=("needs_revision",), - reviewer_reassignment_rule="same reviewer preferred", - ), + { + "max_revision_rounds": 7, + "revision_deadline_hours": 48, + "allowed_resubmission_states": ["needs_revision"], + "reviewer_reassignment_rule": "same reviewer preferred", + }, + ), + ): + response = await client.put( + f"/api/v1/projects/{project_id}/guides/{guide_id}/{kind}-policy", + headers=auth_headers() | {"If-Match": '"no-current-policy"'}, + json=body, ) + assert response.status_code == 200, response.text + async with db_session.get_session_factory()() as session: + guide = await session.get(ProjectGuide, guide_id) + assert guide is not None session.add_all( [ - ReviewPolicy( - id=review_policy_id, - project_id=project_id, - guide_version=guide.version, - policy_generation=1, - policy_hash=review_hash, - semantics_status="complete", - review_preference_window_seconds=3600, - review_lease_duration_seconds=1800, - max_active_review_leases_per_reviewer=1, - self_review_allowed=False, - reject_policy="close_task", - finding_evidence_requirement="optional", - requires_second_review=False, - allowed_decisions=["accept", "needs_revision", "reject"], - minimum_finding_fields=["issue", "required_fix"], - ), - RevisionPolicy( - id=revision_policy_id, - project_id=project_id, - guide_version=guide.version, - policy_generation=1, - policy_hash=revision_hash, - semantics_status="complete", - max_revision_rounds=7, - revision_deadline_hours=48, - allowed_resubmission_states=["needs_revision"], - reviewer_reassignment_rule="same reviewer preferred", - ), PaymentPolicy( id=str(uuid4()), project_id=project_id, @@ -1027,12 +1002,6 @@ async def create_policy_bundle_for_guide( ] ) await session.flush() - guide.selected_review_policy_id = review_policy_id - guide.selected_review_policy_generation = 1 - guide.selected_review_policy_hash = review_hash - guide.selected_revision_policy_id = revision_policy_id - guide.selected_revision_policy_generation = 1 - guide.selected_revision_policy_hash = revision_hash await session.commit() snapshot_response = await client.post( diff --git a/docs/operations_authorization_service.md b/docs/operations_authorization_service.md index 9e368f6ae..820a8405b 100644 --- a/docs/operations_authorization_service.md +++ b/docs/operations_authorization_service.md @@ -1327,3 +1327,26 @@ does not invent or backfill historical custody. Every new covered mutation must commit its complete replay, decision, and row provenance atomically. Once any 12D custody or attributed mutation exists, downgrade is intentionally refused; operators must not delete authority evidence merely to force rollback. + +## Draft review and revision policy authorization + +The guide-bound review-policy and revision-policy `PUT` routes require a UUID +`Idempotency-Key`, an active covered Project Manager grant carrying +`project.review_policy.manage`, and a quoted `If-Match` value. The first version +uses `"no-current-policy"`; replacements use the opaque selector composed from +the current policy ID, generation, and canonical digest. Omitted, wildcard, +unquoted, stale, cross-guide, or cross-project +preconditions fail without advancing a selector. + +Each success appends one immutable policy version, records the exact actor, +identity link, matched grant, action and authorization decision, and advances +only the corresponding draft-guide selector in the same transaction. The two +policies may be attached in either order. Never repair an active guide by +changing these selectors: active and superseded guide selections remain frozen. + +Migration `0048_policy_authority` adds nullable historical provenance columns +and the `policy_mutation_idempotency_records` custody ledger. Historical +`legacy_incomplete` rows remain grandfathered and are not attributed. Downgrade +is refused after any 02B mutation/replay custody exists; do not delete policy or +authorization evidence to force rollback. A populated rollback requires an +explicit reviewed data-retention and migration plan. diff --git a/docs/operations_project_operating_manual.md b/docs/operations_project_operating_manual.md index b288383d5..c8dc403a8 100644 --- a/docs/operations_project_operating_manual.md +++ b/docs/operations_project_operating_manual.md @@ -103,13 +103,25 @@ remain task-scoped through work-context and submission-requirements APIs. The two collection routes return the newest 100 canonical rows. Use the exact single-record route for an older retained report or policy identifier. -The remaining policy and mutation endpoints follow their separately owned -activation chunks: +The active review/revision policy setup endpoints are: + +- `PUT /api/v1/projects/{project_id}/guides/{guide_id}/review-policy` +- `PUT /api/v1/projects/{project_id}/guides/{guide_id}/revision-policy` + +Other policy mutation endpoints follow their separately owned activation +chunks: - `POST /api/v1/projects/{project_id}/guides/{guide_id}/submission-artifact-policies/{policy_id}/approve` - `POST /api/v1/projects/{project_id}/guides/{guide_id}/post-submit-checker-policy/approve` - `POST /api/v1/projects/{project_id}/guides/{guide_id}/post-submit-checker-policy/request-correction` +The two policy `PUT` routes require a UUID `Idempotency-Key` and a quoted +`If-Match` value. Use `"no-current-policy"` for the first version and the quoted +opaque selector returned from the current policy ID, generation, and canonical +digest for a replacement. An authorized Project +Manager may attach review and revision policies in either order while the guide +is draft; activation remains blocked until both are complete. + `ProjectSetupRun` is only a setup ledger. Policy truth remains in the guide source snapshot, sufficiency report, submission artifact policy, effective project policy, compiled project pre-submit checker policy rows, and compiled diff --git a/docs/operations_roles_permissions.md b/docs/operations_roles_permissions.md index 01b2ec2c5..05df7fe7c 100644 --- a/docs/operations_roles_permissions.md +++ b/docs/operations_roles_permissions.md @@ -51,6 +51,13 @@ Operators receive reason-bound operational recovery only. Neither receives a review decision or generic artifact capability without the independent exact grant/action required for that operation. +The active guide-bound configuration actions +`project.review_policy.update` and `project.revision_policy.update` both map to +`project.review_policy.manage`. Only a system or exact-project Project Manager +grant may use them, and only while the selected guide is a draft. Operator, +Audit Authority, Reviewer, Submitter, and other roles do not inherit either +mutation action. + ## Contributor Grants | Grant | Scope | Purpose | diff --git a/docs/spec_authorization_service.md b/docs/spec_authorization_service.md index 5d646daba..ee70469c1 100644 --- a/docs/spec_authorization_service.md +++ b/docs/spec_authorization_service.md @@ -999,14 +999,26 @@ mutation, setup run, or dispatch. Changed, concurrent-pending, cross-project, stale-lineage, revoked, wrong-action, wrong-resource, or wrong-transaction use fails closed with no product write. +Review and revision policy configuration uses two separate guide-bound `PUT` +routes. Each requires an exact covered-project Project Manager grant, a UUID +`Idempotency-Key`, and an HTTP `If-Match` precondition: a quoted opaque selector +binding the current policy ID, generation, and digest for replacement or the +exact `"no-current-policy"` sentinel for initial +attachment. The server normalizes the complete policy semantics, computes the +canonical digest, consumes one transaction-bound PREP handle after locking the +draft guide and predecessor, appends an immutable version with authorization +provenance, and advances only that policy selector. Draft guides may attach the +two policies in either order; activation still requires both. Active guide +policy selection remains frozen. + | ActionId | PermissionId | Activation owner | |---|---|---| | `project.create` (active) | `project.create` | `WS-AUTH-001-12C` | | `project.guide.create` (active) | `project.guide.manage` | `WS-AUTH-001-12D` | | `project.guide.update` (active) | `project.guide.manage` | `WS-AUTH-001-12D` | | `project.guide_source_snapshot.create` (active) | `project.guide.manage` | `WS-AUTH-001-12D` | -| `project.review_policy.update` | `project.review_policy.manage` | `WS-AUTH-001-12D2` | -| `project.revision_policy.update` | `project.review_policy.manage` | `WS-AUTH-001-12D2` | +| `project.review_policy.update` (active) | `project.review_policy.manage` | `WS-XINT-003-02B` | +| `project.revision_policy.update` (active) | `project.review_policy.manage` | `WS-XINT-003-02B` | | `project.guide_sufficiency_report.create` | `project.guide.manage` | `WS-AUTH-001-12E` | | `project.guide_sufficiency.run` | `project.guide.manage` | `WS-AUTH-001-12E` | | `project.guide_sufficiency.warnings.acknowledge` | `project.guide.manage` | `WS-AUTH-001-12E` | diff --git a/docs/spec_review_lifecycle.md b/docs/spec_review_lifecycle.md index a55019a92..51f750543 100644 --- a/docs/spec_review_lifecycle.md +++ b/docs/spec_review_lifecycle.md @@ -19,10 +19,12 @@ The canonical REV-AUTH action custody is `.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.md`. REV owns lifecycle and immutable policy semantics; AUTH owns evaluation, PREP, and decision evidence. ReviewPolicy and RevisionPolicy use immutable, -append-only identities installed by XINT-003-02A; their only future writer is -the PREP-bound mutation surface owned by XINT-003-02B. XINT-002-07A activates -reviewer packet materialization only. ART review-evidence binding remains -planned/unavailable and 07B is reserved pending separate REV-owned intent. +append-only identities installed by XINT-003-02A; their only writer is the +guide-bound PREP mutation surface activated by XINT-003-02B. This configuration +surface does not activate review queues, leases, findings, decisions, or +revision execution. XINT-002-07A activates reviewer packet materialization +only. ART review-evidence binding remains planned/unavailable and 07B is +reserved pending separate REV-owned intent. ## Precedence And Archival Inputs From 73cf762f169df93bd626b4ed1c186a954632eb32 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sun, 2 Aug 2026 18:46:36 +0100 Subject: [PATCH 2/9] docs(auth): satisfy policy mutation docstring gate --- .../REVIEW_LOG.md | 6 ++++ ...S-XINT-003-02B-external-review-response.md | 28 +++++++++++++++++++ .../WS-XINT-003-02B-pr-trust-bundle.md | 5 ++++ backend/app/modules/authorization/runtime.py | 1 + .../policy_mutation_replay_repository.py | 4 +++ .../projects/policy_mutation_router.py | 8 ++++++ .../projects/policy_mutation_service.py | 9 ++++++ 7 files changed, 61 insertions(+) create mode 100644 .agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md index e5a5cec17..3e8f89c45 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md @@ -91,3 +91,9 @@ ordering, same-actor replay, exact opaque selectors, route rollback, database successor/predecessor custody, live fixture bypasses, and operator docs. All tracks passed after correction; no finding remains open. Final evidence is in `reviews/WS-XINT-003-02B-internal-review.md`. + +The first hosted Backend run failed the unchanged docstring gate because 22 new +02B callables reduced coverage to 79.7 percent. The new surface was documented, +and the same local gate passes at 80.5 percent without a threshold or workflow +change. External evidence is in +`reviews/WS-XINT-003-02B-external-review-response.md`. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md new file mode 100644 index 000000000..a87ef0181 --- /dev/null +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md @@ -0,0 +1,28 @@ +# External Review Response: WS-XINT-003-02B + +## GitHub Actions round 1 + +Comments addressed: + +- Backend docstring coverage failed at 79.7 percent because the new policy + mutation surface added 22 undocumented callables. All new router, + replay-repository, service, and validator callables now carry concise + behavioral docstrings. The unchanged gate passes locally at 80.5 percent. + +Comments deferred: + +- None. + +Human decisions needed: + +- None. The gate and threshold were preserved. + +Commands rerun: + +- `cd backend && .venv/bin/ruff check app tests scripts` +- `cd backend && .venv/bin/docstr-coverage --config .docstr.yaml` + +Remaining risks: + +- GitHub Backend, Agent Gates, and CodeRabbit must pass on the replacement + exact head before human merge. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md index 8b98e4fb6..4c9f9f233 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md @@ -65,6 +65,11 @@ risks. Every blocking first-round finding was fixed and re-reviewed. GitHub `Backend / test`, `Agent Gates / agent-gates`, and CodeRabbit must pass on the exact final head. Valid findings must be corrected before human merge. +The first Backend run failed the unchanged 80-percent docstring gate at 79.7 +percent because new 02B callables lacked docstrings. All 22 new callables were +documented; the same gate now passes locally at 80.5 percent. The exact response +is recorded in `WS-XINT-003-02B-external-review-response.md`. + ## Remaining risks and follow-up The API may later expose the opaque replacement selector as a response ETag. diff --git a/backend/app/modules/authorization/runtime.py b/backend/app/modules/authorization/runtime.py index d3c7d58f4..3e64f4ab6 100644 --- a/backend/app/modules/authorization/runtime.py +++ b/backend/app/modules/authorization/runtime.py @@ -666,6 +666,7 @@ class ProjectPolicyMutationPrepareDenialResourceContext(BaseModel): @model_validator(mode="after") def require_requested_guide(self): + """Require denial evidence to identify its exact requested guide.""" if self.resource_id != self.requested_guide_id: raise ValueError("policy mutation denial must identify the requested guide") return self diff --git a/backend/app/modules/projects/policy_mutation_replay_repository.py b/backend/app/modules/projects/policy_mutation_replay_repository.py index d23011a6d..2f0df2ac1 100644 --- a/backend/app/modules/projects/policy_mutation_replay_repository.py +++ b/backend/app/modules/projects/policy_mutation_replay_repository.py @@ -17,11 +17,13 @@ class PolicyMutationReplayRepository: """Own only policy-mutation replay records in the caller transaction.""" def __init__(self, session: AsyncSession) -> None: + """Bind replay persistence to the caller-owned transaction.""" self._session = session async def find( self, actor_profile_id: str, action_id: str, idempotency_key: UUID ) -> PolicyMutationIdempotencyRecord | None: + """Find one actor/action/key replay record.""" return await self._session.scalar( select(PolicyMutationIdempotencyRecord).where( PolicyMutationIdempotencyRecord.actor_profile_id == actor_profile_id, @@ -46,6 +48,7 @@ async def reserve( policy_id: str, policy_generation: int, ) -> tuple[str, PolicyMutationIdempotencyRecord]: + """Claim or classify one exact policy mutation replay tuple.""" values = { "id": uuid4(), "actor_profile_id": actor_profile_id, @@ -94,6 +97,7 @@ async def reserve( async def complete( self, record: PolicyMutationIdempotencyRecord, *, response_json: dict ) -> None: + """Complete one pending replay record with its stable response.""" completed = await self._session.scalar( update(PolicyMutationIdempotencyRecord) .where( diff --git a/backend/app/modules/projects/policy_mutation_router.py b/backend/app/modules/projects/policy_mutation_router.py index 8cb9b0a48..61c8bfa4c 100644 --- a/backend/app/modules/projects/policy_mutation_router.py +++ b/backend/app/modules/projects/policy_mutation_router.py @@ -37,6 +37,7 @@ def require_policy_mutation_key( idempotency_key: Annotated[str, Header(alias="Idempotency-Key")], ) -> UUID: + """Parse the required policy-mutation replay key.""" try: return UUID(idempotency_key) except ValueError as exc: @@ -55,6 +56,7 @@ async def policy_authorization_actor( session: Annotated[AsyncSession, Depends(get_db_session)], rate_control: Annotated[RateControlService, Depends(get_rate_control_service)], ) -> ResolvedActor: + """Resolve the authenticated actor only after key validation.""" del key return await resolve_authorization_actor(request, result, session, rate_control) @@ -64,6 +66,7 @@ async def get_policy_prepared_authorization_service( resolved: Annotated[ResolvedActor, Depends(policy_authorization_actor)], session: Annotated[AsyncSession, Depends(get_db_session)], ): + """Yield one request-local prepared authorization service.""" async with prepared_authorization_service(request, resolved, session) as service: yield service @@ -76,10 +79,12 @@ async def policy_authorization( Depends(get_policy_prepared_authorization_service), ], ): + """Compose the policy mutation authorization dependencies.""" return key, resolved, prepared def _error(exc: ProjectServiceError): + """Translate one policy mutation domain error to HTTP.""" if isinstance(exc, PolicyMutationConflict): code = str(exc) return StructuredHTTPException( @@ -93,6 +98,7 @@ def _error(exc: ProjectServiceError): async def _finish(session: AsyncSession, outcome): + """Commit a new mutation or roll back a replay-only transaction.""" await (session.rollback() if outcome.replayed else session.commit()) return outcome.response @@ -110,6 +116,7 @@ async def replace_review_policy( authorization: Annotated[tuple, Depends(policy_authorization)], session: Annotated[AsyncSession, Depends(get_db_session)], ): + """Append and select one authorized review-policy version.""" key, resolved, prepared = authorization try: outcome = await ProjectPolicyMutationService(session).replace_review_policy( @@ -134,6 +141,7 @@ async def replace_revision_policy( authorization: Annotated[tuple, Depends(policy_authorization)], session: Annotated[AsyncSession, Depends(get_db_session)], ): + """Append and select one authorized revision-policy version.""" key, resolved, prepared = authorization try: outcome = await ProjectPolicyMutationService(session).replace_revision_policy( diff --git a/backend/app/modules/projects/policy_mutation_service.py b/backend/app/modules/projects/policy_mutation_service.py index b8c07b503..7ce4d9a28 100644 --- a/backend/app/modules/projects/policy_mutation_service.py +++ b/backend/app/modules/projects/policy_mutation_service.py @@ -69,11 +69,13 @@ class ProjectPolicyMutationService: """Sole authorized writer for immutable guide policy versions.""" def __init__(self, session: AsyncSession) -> None: + """Bind product and replay repositories to one transaction.""" self._projects = ProjectRepository(session) self._replay = PolicyMutationReplayRepository(session) @staticmethod def _if_match_value(if_match: str) -> tuple[UUID, int, str] | None: + """Parse an exact initial sentinel or canonical policy selector.""" if if_match == NO_CURRENT_POLICY_ETAG: return None if len(if_match) > 2 and if_match[0] == if_match[-1] == '"': @@ -94,6 +96,7 @@ def _if_match_value(if_match: str) -> tuple[UUID, int, str] | None: @staticmethod def _prove(decision, project_id: UUID) -> None: + """Require exact Project Manager authority proof.""" if ( decision.matched_authority_kind is not MatchedAuthorityKind.ADMIN_ROLE_GRANT or decision.matched_grant_id is None @@ -105,6 +108,7 @@ def _prove(decision, project_id: UUID) -> None: def _guide_selector( kind: Literal["review", "revision"], guide: ProjectGuide ) -> tuple[UUID, int, str] | None: + """Read the exact selected policy tuple from one guide.""" policy_id = getattr(guide, f"selected_{kind}_policy_id") generation = getattr(guide, f"selected_{kind}_policy_generation") digest = getattr(guide, f"selected_{kind}_policy_hash") @@ -128,6 +132,7 @@ def _resource( predecessor_generation: int | None, predecessor_digest: str | None, ) -> ProjectReviewPolicyMutationResourceContext | ProjectRevisionPolicyMutationResourceContext: + """Compose the exact final PREP resource facts.""" values = { "resource_type": f"project_{kind}_policy_mutation", "resource_id": policy_id, @@ -151,6 +156,7 @@ def _resource( ) async def _existing(self, resolved, action, key, digest, project_id, guide_id, response_type): + """Return an exact committed replay or reject unsafe reuse.""" record = await self._replay.find(resolved.profile.id, action.value, key) if record is None: return None @@ -177,6 +183,7 @@ async def replace_review_policy( guide_id: UUID, payload: ReviewPolicyInput, ) -> PolicyMutationOutcome: + """Authorize, append, and select one review-policy version.""" semantics = ReviewPolicySemantics.model_validate(payload.model_dump()) return await self._replace( "review", @@ -200,6 +207,7 @@ async def replace_revision_policy( guide_id: UUID, payload: RevisionPolicyInput, ) -> PolicyMutationOutcome: + """Authorize, append, and select one revision-policy version.""" semantics = RevisionPolicySemantics.model_validate(payload.model_dump()) return await self._replace( "revision", @@ -225,6 +233,7 @@ async def _replace( semantics: ReviewPolicySemantics | RevisionPolicySemantics, response_type, ) -> PolicyMutationOutcome: + """Execute the shared exact policy mutation transaction.""" expected_selector = self._if_match_value(if_match) action = ( ActionId.PROJECT_REVIEW_POLICY_UPDATE From 97ca36a62febd29d69bb1af7622266428195dd54 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sun, 2 Aug 2026 18:50:56 +0100 Subject: [PATCH 3/9] test(ci): assign policy mutation lane custody --- ...XINT-003-02B-policy-mutation-activation.md | 1 + ...S-XINT-003-02B-external-review-response.md | 27 +++++++++++++++++++ .../WS-XINT-003-02B-pr-trust-bundle.md | 5 ++++ backend/scripts/run_test_lanes.py | 1 + 4 files changed, 34 insertions(+) diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md index c29791af6..94b9fbf2b 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md @@ -65,6 +65,7 @@ backend/tests/test_artifact_admission.py backend/tests/test_alembic.py backend/tests/test_artifact_architecture.py backend/scripts/api_contract_e2e.py +backend/scripts/run_test_lanes.py docs/spec_authorization_service.md docs/spec_review_lifecycle.md docs/operations_authorization_service.md diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md index a87ef0181..eb7312fec 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md @@ -26,3 +26,30 @@ Remaining risks: - GitHub Backend, Agent Gates, and CodeRabbit must pass on the replacement exact head before human merge. + +## GitHub Actions round 2 + +Comments addressed: + +- The semantic-lane inventory failed closed because the new + `tests/test_project_policy_mutations.py` module had no canonical lane. It is + now assigned to `shared_foundations` beside authorization and immutable policy + lineage tests; no lane, threshold, or execution behavior changed. + +Comments deferred: + +- None. + +Human decisions needed: + +- None. + +Commands rerun: + +- The canonical collect-only runner passed the missing-lane check, then reached + unrelated locally absent Pillow dependencies supplied by hosted CI. +- Focused CI lane-contract tests: 68 passed. + +Remaining risks: + +- Exact-head hosted checks and CodeRabbit remain required. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md index 4c9f9f233..083cd8451 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md @@ -70,6 +70,11 @@ percent because new 02B callables lacked docstrings. All 22 new callables were documented; the same gate now passes locally at 80.5 percent. The exact response is recorded in `WS-XINT-003-02B-external-review-response.md`. +The second Backend run failed closed because the new test module lacked +semantic-lane custody. It is assigned to the existing `shared_foundations` +lane beside authorization and policy-lineage tests; the four-lane design and +all thresholds remain unchanged. + ## Remaining risks and follow-up The API may later expose the opaque replacement selector as a response ETag. diff --git a/backend/scripts/run_test_lanes.py b/backend/scripts/run_test_lanes.py index b7387a54e..6e659dfef 100644 --- a/backend/scripts/run_test_lanes.py +++ b/backend/scripts/run_test_lanes.py @@ -127,6 +127,7 @@ class TestLane: "tests/test_db_session.py", "tests/test_outbox.py", "tests/test_policy_identity_lineage.py", + "tests/test_project_policy_mutations.py", ), ), TestLane( From 4050a340e66411a5079bfa0d59c69ac5863d095a Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sun, 2 Aug 2026 19:03:25 +0100 Subject: [PATCH 4/9] fix(auth): close policy mutation review findings --- .../REVIEW_LOG.md | 6 ++ ...XINT-003-02B-policy-mutation-activation.md | 1 + ...S-XINT-003-02B-external-review-response.md | 39 +++++++++++++ .../WS-XINT-003-02B-internal-review.md | 5 ++ .../WS-XINT-003-02B-pr-trust-bundle.md | 6 ++ .../0048_review_revision_policy_authority.py | 15 ++++- backend/app/modules/projects/models.py | 7 +++ .../policy_mutation_replay_repository.py | 8 ++- .../projects/policy_mutation_router.py | 13 ++++- .../projects/policy_mutation_service.py | 11 +++- backend/scripts/api_contract_e2e.py | 10 +++- backend/tests/project_create_fixtures.py | 5 +- backend/tests/test_alembic.py | 16 ++++++ .../tests/test_project_policy_mutations.py | 55 ++++++++++++++++++- backend/tests/test_projects.py | 4 +- docs/operations_project_operating_manual.md | 5 +- 16 files changed, 189 insertions(+), 17 deletions(-) diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md index 3e8f89c45..7f14d35cf 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md @@ -97,3 +97,9 @@ The first hosted Backend run failed the unchanged docstring gate because 22 new and the same local gate passes at 80.5 percent without a threshold or workflow change. External evidence is in `reviews/WS-XINT-003-02B-external-review-response.md`. + +CodeRabbit's first pass found valid replay timestamp immutability, downgrade +locking, historical trigger allow-list, fixture-copy, and replacement-selector +documentation issues. Its related indexing, typing, exact-exception, +constraint-shape, and reservation-branch notes were also valid. All were fixed; +none was deferred. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md index 94b9fbf2b..c92f8de84 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md @@ -62,6 +62,7 @@ backend/tests/test_policy_identity_lineage.py backend/tests/test_projects.py backend/tests/test_tasks.py backend/tests/test_artifact_admission.py +backend/tests/project_create_fixtures.py backend/tests/test_alembic.py backend/tests/test_artifact_architecture.py backend/scripts/api_contract_e2e.py diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md index eb7312fec..870cb1c8c 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md @@ -53,3 +53,42 @@ Commands rerun: Remaining risks: - Exact-head hosted checks and CodeRabbit remain required. + +## CodeRabbit round 1 + +Comments addressed: + +- Replay immutability now includes `created_at`, and downgrade locks all three + custody tables before its emptiness decision. +- The historical-fixture custody helper admits only the two exact new + review/revision table-trigger pairs. +- Project fixtures copy nested policy input before removing obsolete fields. +- Operator guidance gives the exact replacement selector construction. +- The custody lookup has a matching migration/ORM composite index. +- Replay disposition, route dependency tuples, and `_existing` are typed. +- Reservation mismatch, pending, and replayed service branches now have direct + tests; the migration test distinguishes independent upgraded selectors from + the coupled downgraded constraint. +- The smaller ordering comment, exact exception, repository/E2E docstrings, + and argument documentation were also corrected. + +Comments deferred: + +- None. + +Human decisions needed: + +- None. + +Commands rerun: + +- Ruff: passed. +- Policy mutation and lane-contract tests: 81 passed. +- New-subsystem coverage: 13 passed, 92.86 percent. +- Migration `0047:0048` offline SQL generation: passed. +- Markdown links and whitespace: passed. + +Remaining risks: + +- PostgreSQL migration behavior and the full suite remain assigned to the + replacement exact-head Backend run. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-internal-review.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-internal-review.md index f9225bf91..46d13bb42 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-internal-review.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-internal-review.md @@ -37,6 +37,11 @@ No blocking finding remains. All reviewer sessions completed. The PostgreSQL-isolated migration/API cases and repository-wide 78-percent coverage suite remain assigned to hosted GitHub Actions on the exact PR head. +Security and QA re-reviewed the final CodeRabbit corrective delta. Both passed +after verifying replay timestamp immutability, downgrade locking, custody +index parity, the historical trigger allow-list, independent selector proof, +fixture copying, and all reservation dispositions. + ## Residual low risks - Clients currently compose the documented opaque replacement selector from diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md index 083cd8451..8675d1338 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md @@ -75,6 +75,12 @@ semantic-lane custody. It is assigned to the existing `shared_foundations` lane beside authorization and policy-lineage tests; the four-lane design and all thresholds remain unchanged. +CodeRabbit then identified five valid correctness issues and related quality +notes. Replay timestamp immutability, downgrade locking, historical trigger +allow-listing, fixture copying, exact selector guidance, custody indexing, +typing, migration-shape proof, and reservation-branch tests are corrected. No +comment was deferred; details are in the external response. + ## Remaining risks and follow-up The API may later expose the opaque replacement selector as a response ETag. diff --git a/backend/alembic/versions/0048_review_revision_policy_authority.py b/backend/alembic/versions/0048_review_revision_policy_authority.py index 1528e8655..4192a66ac 100644 --- a/backend/alembic/versions/0048_review_revision_policy_authority.py +++ b/backend/alembic/versions/0048_review_revision_policy_authority.py @@ -153,6 +153,11 @@ def upgrade() -> None: name="ck_policy_mutation_state_shape", ), ) + op.create_index( + "ix_policy_mutation_custody_lookup", + "policy_mutation_idempotency_records", + ["policy_id", "action_id", "policy_generation", "status"], + ) op.execute( """ create function guard_policy_mutation_replay() returns trigger language plpgsql as $$ @@ -171,13 +176,13 @@ def upgrade() -> None: new.idempotency_key,new.request_digest,new.policy_hash, new.resource_context_digest, new.operation_id,new.project_id,new.guide_id,new.policy_id, - new.policy_generation) + new.policy_generation,new.created_at) is not distinct from (old.id,old.actor_profile_id,old.identity_link_id,old.action_id, old.idempotency_key,old.request_digest,old.policy_hash, old.resource_context_digest, old.operation_id,old.project_id,old.guide_id,old.policy_id, - old.policy_generation) then + old.policy_generation,old.created_at) then return new; end if; raise exception 'policy mutation replay is immutable' using errcode='23514'; @@ -352,6 +357,12 @@ def upgrade() -> None: def downgrade() -> None: """Remove 02B authority state while preserving 02A policy lineage.""" bind = op.get_bind() + for table in ( + "policy_mutation_idempotency_records", + "review_policies", + "revision_policies", + ): + bind.execute(sa.text(f"lock table {table} in share row exclusive mode")) has_custody = bool( bind.scalar( sa.text( diff --git a/backend/app/modules/projects/models.py b/backend/app/modules/projects/models.py index 49ed6d102..d8f1139b9 100644 --- a/backend/app/modules/projects/models.py +++ b/backend/app/modules/projects/models.py @@ -181,6 +181,13 @@ class PolicyMutationIdempotencyRecord(Base): name="uq_policy_mutation_replay_namespace", ), UniqueConstraint("operation_id", name="uq_policy_mutation_operation_identity"), + Index( + "ix_policy_mutation_custody_lookup", + "policy_id", + "action_id", + "policy_generation", + "status", + ), CheckConstraint( "action_id in ('project.review_policy.update','project.revision_policy.update')", name="ck_policy_mutation_action", diff --git a/backend/app/modules/projects/policy_mutation_replay_repository.py b/backend/app/modules/projects/policy_mutation_replay_repository.py index 2f0df2ac1..b57f2957a 100644 --- a/backend/app/modules/projects/policy_mutation_replay_repository.py +++ b/backend/app/modules/projects/policy_mutation_replay_repository.py @@ -3,6 +3,7 @@ from __future__ import annotations from datetime import UTC, datetime +from typing import Literal from uuid import UUID, uuid4 from sqlalchemy import select, update @@ -47,8 +48,11 @@ async def reserve( guide_id: str, policy_id: str, policy_generation: int, - ) -> tuple[str, PolicyMutationIdempotencyRecord]: - """Claim or classify one exact policy mutation replay tuple.""" + ) -> tuple[ + Literal["claimed", "mismatch", "pending", "replayed"], + PolicyMutationIdempotencyRecord, + ]: + """Claim or classify an exact tuple as claimed, mismatch, pending, or replayed.""" values = { "id": uuid4(), "actor_profile_id": actor_profile_id, diff --git a/backend/app/modules/projects/policy_mutation_router.py b/backend/app/modules/projects/policy_mutation_router.py index 61c8bfa4c..cf0b6b893 100644 --- a/backend/app/modules/projects/policy_mutation_router.py +++ b/backend/app/modules/projects/policy_mutation_router.py @@ -57,6 +57,7 @@ async def policy_authorization_actor( rate_control: Annotated[RateControlService, Depends(get_rate_control_service)], ) -> ResolvedActor: """Resolve the authenticated actor only after key validation.""" + # Keep this dependency so key validation precedes actor and rate-control work. del key return await resolve_authorization_actor(request, result, session, rate_control) @@ -78,7 +79,7 @@ async def policy_authorization( PreparedAuthorizationService, Depends(get_policy_prepared_authorization_service), ], -): +) -> tuple[UUID, ResolvedActor, PreparedAuthorizationService]: """Compose the policy mutation authorization dependencies.""" return key, resolved, prepared @@ -113,7 +114,10 @@ async def replace_review_policy( guide_id: UUID, payload: ReviewPolicyInput, if_match: Annotated[str, Header(alias="If-Match")], - authorization: Annotated[tuple, Depends(policy_authorization)], + authorization: Annotated[ + tuple[UUID, ResolvedActor, PreparedAuthorizationService], + Depends(policy_authorization), + ], session: Annotated[AsyncSession, Depends(get_db_session)], ): """Append and select one authorized review-policy version.""" @@ -138,7 +142,10 @@ async def replace_revision_policy( guide_id: UUID, payload: RevisionPolicyInput, if_match: Annotated[str, Header(alias="If-Match")], - authorization: Annotated[tuple, Depends(policy_authorization)], + authorization: Annotated[ + tuple[UUID, ResolvedActor, PreparedAuthorizationService], + Depends(policy_authorization), + ], session: Annotated[AsyncSession, Depends(get_db_session)], ): """Append and select one authorized revision-policy version.""" diff --git a/backend/app/modules/projects/policy_mutation_service.py b/backend/app/modules/projects/policy_mutation_service.py index 7ce4d9a28..5534ce1d0 100644 --- a/backend/app/modules/projects/policy_mutation_service.py +++ b/backend/app/modules/projects/policy_mutation_service.py @@ -155,7 +155,16 @@ def _resource( else ProjectRevisionPolicyMutationResourceContext(**values) ) - async def _existing(self, resolved, action, key, digest, project_id, guide_id, response_type): + async def _existing( + self, + resolved: ResolvedActor, + action: ActionId, + key: UUID, + digest: str, + project_id: UUID, + guide_id: UUID, + response_type: type[ReviewPolicyResponse] | type[RevisionPolicyResponse], + ) -> PolicyMutationOutcome | None: """Return an exact committed replay or reject unsafe reuse.""" record = await self._replay.find(resolved.profile.id, action.value, key) if record is None: diff --git a/backend/scripts/api_contract_e2e.py b/backend/scripts/api_contract_e2e.py index 7bddfb1c4..6f1fb1234 100644 --- a/backend/scripts/api_contract_e2e.py +++ b/backend/scripts/api_contract_e2e.py @@ -651,7 +651,15 @@ async def configure_policy_boundaries( guide_id: str, guide_version: str, ) -> None: - """Configure both policies through their sole active HTTP boundaries.""" + """Configure both policies through their sole active HTTP boundaries. + + Args: + client: Real HTTP client. + token: Project Manager Flow bearer token. + project_id: Project whose draft guide is configured. + guide_id: Exact draft guide receiving both policies. + guide_version: Guide version used by the direct PaymentPolicy fixture. + """ await request_json( client, "PUT", diff --git a/backend/tests/project_create_fixtures.py b/backend/tests/project_create_fixtures.py index e17c9e95f..e2025f07d 100644 --- a/backend/tests/project_create_fixtures.py +++ b/backend/tests/project_create_fixtures.py @@ -50,6 +50,8 @@ async def suspend_historical_product_custody( "guide_source_snapshots": {"source_snapshot_product_custody"}, "guide_source_snapshot_items": {"guide_source_snapshot_items_custody"}, "project_setup_runs": {"source_setup_run_custody"}, + "review_policies": {"review_policy_mutation_custody"}, + "revision_policies": {"revision_policy_mutation_custody"}, } if table not in allowed or not triggers or not set(triggers) <= allowed[table]: raise RuntimeError("unsupported historical custody suspension") @@ -257,8 +259,7 @@ async def insert_historical_project( try: await connection.execute( text( - "insert into projects (id, name, slug, status) " - "values (:id, :name, :slug, :status)" + "insert into projects (id, name, slug, status) values (:id, :name, :slug, :status)" ), {"id": project_id, "name": name, "slug": slug, "status": status}, ) diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 6d1bbdb20..20b70965e 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -11887,6 +11887,7 @@ def test_xint003_02b_policy_authority_schema_and_roundtrip( "revision_provenance": 8, "custody_triggers": 3, "selector_constraint": True, + "independent_selector_shape": True, "selector_custody": True, "predecessor_custody": True, } @@ -11896,6 +11897,7 @@ def test_xint003_02b_policy_authority_schema_and_roundtrip( "revision_provenance": 0, "custody_triggers": 0, "selector_constraint": True, + "independent_selector_shape": False, "selector_custody": False, "predecessor_custody": False, } @@ -11957,6 +11959,16 @@ async def _xint003_02b_authority_shape(database_url: str) -> dict[str, int | boo ) ) ) + selector_definition = str( + await connection.scalar( + text( + "select pg_get_constraintdef(oid) from pg_constraint " + "where conname='policy_selection_shape'" + ) + ) + or "" + ) + normalized_selector = " ".join(selector_definition.lower().split()) custody_definition = str( await connection.scalar( text( @@ -11972,6 +11984,10 @@ async def _xint003_02b_authority_shape(database_url: str) -> dict[str, int | boo "revision_provenance": len(columns["revision_policies"] & provenance), "custody_triggers": triggers, "selector_constraint": selector, + "independent_selector_shape": ( + "selected_review_policy_hash is not null)) and " + "((selected_revision_policy_id is null" in normalized_selector + ), "selector_custody": "selected_review_policy_id" in custody_definition and "selected_revision_policy_id" in custody_definition, "predecessor_custody": "prior.policy_generation=product_generation-1" diff --git a/backend/tests/test_project_policy_mutations.py b/backend/tests/test_project_policy_mutations.py index 33e5b7944..b40ce9d1f 100644 --- a/backend/tests/test_project_policy_mutations.py +++ b/backend/tests/test_project_policy_mutations.py @@ -37,7 +37,7 @@ ) from app.modules.projects.policy_mutation_router import require_policy_mutation_key from app.modules.projects.schemas import ReviewPolicyInput, RevisionPolicyInput -from app.modules.projects.service import ProjectNotFound +from app.modules.projects.service import GuideEditBlocked, ProjectNotFound def _review_payload() -> ReviewPolicyInput: @@ -295,6 +295,57 @@ async def assert_claimed_before_prepare(*args, **kwargs): ) +@pytest.mark.asyncio +@pytest.mark.parametrize("disposition", ["mismatch", "pending", "replayed"]) +async def test_service_handles_every_reservation_disposition(disposition: str) -> None: + service, resolved, prepared, replay, _repository, project_id, guide_id = _subject() + first = await service.replace_review_policy( + resolved, + prepared, + uuid4(), + NO_CURRENT_POLICY_ETAG, + project_id, + guide_id, + _review_payload(), + ) + prepared.consumed.clear() + + async def classify(**facts): + response = first.response.model_copy( + update={ + "id": facts["policy_id"], + "policy_generation": facts["policy_generation"], + "policy_hash": facts["policy_hash"], + "supersedes_policy_id": first.response.id, + } + ) + return disposition, SimpleNamespace( + status="committed" if disposition == "replayed" else "pending", + response_json=response.model_dump(mode="json") if disposition == "replayed" else None, + ) + + replay.reserve = classify + invocation = service.replace_review_policy( + resolved, + prepared, + uuid4(), + policy_selector_etag( + first.response.id, + first.response.policy_generation, + first.response.policy_hash, + ), + project_id, + guide_id, + _review_payload().model_copy(update={"review_lease_duration_seconds": 9000}), + ) + if disposition == "replayed": + assert (await invocation).replayed is True + else: + with pytest.raises(PolicyMutationConflict, match=f"idempotency_{disposition}"): + await invocation + assert not prepared.consumed + + @pytest.mark.asyncio async def test_policy_replacement_binds_predecessor_and_exact_etag() -> None: service, resolved, prepared, _replay, repository, project_id, guide_id = _subject() @@ -619,7 +670,7 @@ async def replace_revision_policy(self, *_args): async def test_policy_service_denies_stale_guide_and_replay_mismatch() -> None: service, resolved, prepared, replay, repository, project_id, guide_id = _subject() repository.guide.status = "active" - with pytest.raises(Exception, match="only draft guides"): + with pytest.raises(GuideEditBlocked, match="only draft guides"): await service.replace_revision_policy( resolved, prepared, diff --git a/backend/tests/test_projects.py b/backend/tests/test_projects.py index 5caf2a473..7d4119814 100644 --- a/backend/tests/test_projects.py +++ b/backend/tests/test_projects.py @@ -2053,7 +2053,7 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d guide = response.json() if review_policy is not None: values = ( - review_policy + dict(review_policy) if isinstance(review_policy, dict) else { "requires_second_review": False, @@ -2079,7 +2079,7 @@ async def create_guide(client: AsyncClient, project_id: str, payload: dict) -> d assert policy_response.status_code == 200, policy_response.text if revision_policy is not None: values = ( - revision_policy + dict(revision_policy) if isinstance(revision_policy, dict) else { "max_revision_rounds": 7, diff --git a/docs/operations_project_operating_manual.md b/docs/operations_project_operating_manual.md index c8dc403a8..81b36b03d 100644 --- a/docs/operations_project_operating_manual.md +++ b/docs/operations_project_operating_manual.md @@ -117,8 +117,9 @@ chunks: The two policy `PUT` routes require a UUID `Idempotency-Key` and a quoted `If-Match` value. Use `"no-current-policy"` for the first version and the quoted -opaque selector returned from the current policy ID, generation, and canonical -digest for a replacement. An authorized Project +opaque selector constructed from the returned policy ID, generation, and +canonical digest for a replacement. Remove the digest's `sha256:` prefix and +send `".."`. An authorized Project Manager may attach review and revision policies in either order while the guide is draft; activation remains blocked until both are complete. From 8df336daccde4410df3043d2d1d3b31fbea6101b Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sun, 2 Aug 2026 19:08:45 +0100 Subject: [PATCH 5/9] test(db): register policy replay reset custody --- ...XINT-003-02B-policy-mutation-activation.md | 1 + ...S-XINT-003-02B-external-review-response.md | 27 +++++++++++++++++++ .../WS-XINT-003-02B-pr-trust-bundle.md | 4 +++ backend/tests/conftest.py | 2 ++ 4 files changed, 34 insertions(+) diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md index c92f8de84..32c1eea5b 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/chunks/WS-XINT-003-02B-policy-mutation-activation.md @@ -63,6 +63,7 @@ backend/tests/test_projects.py backend/tests/test_tasks.py backend/tests/test_artifact_admission.py backend/tests/project_create_fixtures.py +backend/tests/conftest.py backend/tests/test_alembic.py backend/tests/test_artifact_architecture.py backend/scripts/api_contract_e2e.py diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md index 870cb1c8c..c28616a55 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md @@ -92,3 +92,30 @@ Remaining risks: - PostgreSQL migration behavior and the full suite remain assigned to the replacement exact-head Backend run. + +## GitHub Actions round 3 + +Comments addressed: + +- All four lanes executed, then evidence validation reported interruption + because database tests rejected the new replay ledger as an unexpected table. + The canonical reset fingerprint now includes + `policy_mutation_idempotency_records`, and its immutable truncate trigger is + included in the exact guarded-table reset list. + +Comments deferred: + +- None. + +Human decisions needed: + +- None. + +Commands rerun: + +- Policy mutation and non-database reset checks passed locally. Database-backed + reset proof remains on hosted CI because this worktree has no test database URL. + +Remaining risks: + +- Exact-head hosted evidence validation and full coverage remain required. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md index 8675d1338..f44ed9d00 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md @@ -81,6 +81,10 @@ allow-listing, fixture copying, exact selector guidance, custody indexing, typing, migration-shape proof, and reservation-branch tests are corrected. No comment was deferred; details are in the external response. +The next hosted run exposed a stale canonical database-reset fingerprint: the +new replay ledger was treated as an unexpected table, interrupting all lanes. +The ledger is now in the exact reset inventory and truncate-guard list. + ## Remaining risks and follow-up The API may later expose the opaque replacement selector as a response ETag. diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 4fefde983..6bf1eefdf 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -71,6 +71,7 @@ "legacy_workflow_eligibility", "outbox_events", "payment_policies", + "policy_mutation_idempotency_records", "pre_submit_checker_policies", "project_create_idempotency_records", "project_guides", @@ -93,6 +94,7 @@ "guide_mutation_idempotency_records", "guide_source_snapshot_items", "outbox_events", + "policy_mutation_idempotency_records", "project_create_idempotency_records", "project_role_grants", "project_role_qualification_snapshots", From 79fa6e0481c5cb93ce565dc0de472a0670f41704 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sun, 2 Aug 2026 19:13:08 +0100 Subject: [PATCH 6/9] test(db): accept reviewed policy authority schema --- ...S-XINT-003-02B-external-review-response.md | 26 +++++++++++++++++++ .../WS-XINT-003-02B-pr-trust-bundle.md | 3 +++ backend/tests/conftest.py | 2 +- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md index c28616a55..543b339c1 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md @@ -119,3 +119,29 @@ Commands rerun: Remaining risks: - Exact-head hosted evidence validation and full coverage remain required. + +## GitHub Actions round 4 + +Comments addressed: + +- The table inventory passed, then the full public-schema object fingerprint + correctly detected migration 0048's new table, index, functions, triggers, + constraints, and columns. The expected fingerprint is updated to the exact + hosted value `0098edcd29ab2c7317fcd598e266742e592aa6bc571d3df0ed025ae150ebd22c`. + +Comments deferred: + +- None. + +Human decisions needed: + +- None. + +Commands rerun: + +- Exact-head hosted Backend and Agent Gates. + +Remaining risks: + +- The replacement Backend run must pass the complete lane, E2E, and coverage + sequence. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md index f44ed9d00..283242148 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md @@ -84,6 +84,9 @@ comment was deferred; details are in the external response. The next hosted run exposed a stale canonical database-reset fingerprint: the new replay ledger was treated as an unexpected table, interrupting all lanes. The ledger is now in the exact reset inventory and truncate-guard list. +The following hosted run reached the independent full-schema fingerprint and +reported the expected 0048 drift; the constant now uses the exact hosted +fingerprint for the complete new schema. ## Remaining risks and follow-up diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 6bf1eefdf..684369e0f 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -21,7 +21,7 @@ from scripts.run_isolated_tests import LOOPBACK, NAME_RE, ROLE_RE DDL_LOCK_DIRECTORY = Path("/tmp") -EXPECTED_PUBLIC_SCHEMA_SHA256 = "94e097066f30f32ace3605de0366b25078d139cdc37bcd7406c1abfb03fc7ffe" +EXPECTED_PUBLIC_SCHEMA_SHA256 = "0098edcd29ab2c7317fcd598e266742e592aa6bc571d3df0ed025ae150ebd22c" PROTECTED_TEST_TABLES = ( "actor_profile_migration_state", "alembic_version", From 02f245f5b76ddfba530f7548e8bb4fde2d54f232 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sun, 2 Aug 2026 19:28:16 +0100 Subject: [PATCH 7/9] fix(projects): order policy selector persistence --- backend/app/modules/projects/repository.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backend/app/modules/projects/repository.py b/backend/app/modules/projects/repository.py index d689583e6..39c2702ae 100644 --- a/backend/app/modules/projects/repository.py +++ b/backend/app/modules/projects/repository.py @@ -871,6 +871,7 @@ async def add_review_policy_version( ) -> ReviewPolicy: """Append one immutable review policy and advance its locked selector.""" self._session.add(policy) + await self._session.flush([policy]) guide.selected_review_policy_id = policy.id guide.selected_review_policy_generation = policy.policy_generation guide.selected_review_policy_hash = policy.policy_hash @@ -883,6 +884,7 @@ async def add_revision_policy_version( ) -> RevisionPolicy: """Append one immutable revision policy and advance its locked selector.""" self._session.add(policy) + await self._session.flush([policy]) guide.selected_revision_policy_id = policy.id guide.selected_revision_policy_generation = policy.policy_generation guide.selected_revision_policy_hash = policy.policy_hash From 1a92f2ad45383ec3bc1b477f41a106b151ce2b60 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Sun, 2 Aug 2026 19:42:50 +0100 Subject: [PATCH 8/9] test(auth): align activation contract inventories --- backend/tests/test_api_controls.py | 14 ++++++++++---- backend/tests/test_audit.py | 2 ++ backend/tests/test_auth.py | 2 ++ backend/tests/test_authorization.py | 2 ++ 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/backend/tests/test_api_controls.py b/backend/tests/test_api_controls.py index 117167796..39524a233 100644 --- a/backend/tests/test_api_controls.py +++ b/backend/tests/test_api_controls.py @@ -439,13 +439,13 @@ def test_openapi_documents_request_error_and_response_context() -> None: for method, operation in path_item.items() if method in methods and operation.get("security") ) - assert len(route_inventory) == 76 + assert len(route_inventory) == 78 assert sha256("\n".join(route_inventory).encode()).hexdigest() == ( - "1df58dc46d473ed04a9c26b2b90899ed471fa9aaedced43b4f1e2dc477cac350" + "eebf8e5d7fc791a4513b80f6da3fa5cb724eb26e4ce46af8306e1836854a9fd2" ) - assert len(protected_inventory) == 74 + assert len(protected_inventory) == 76 assert sha256("\n".join(protected_inventory).encode()).hexdigest() == ( - "8c629cd977bb0eedf99d5ea73f05708f6549f635974998b31abdd9a25636ad76" + "e40f11c7580bfd48b3554add18a41b913104096c480c91b11c8f33ac9d50ee37" ) assert set(schema["paths"]["/health"]["get"]["responses"]) == {"200", "400", "500"} assert {"401", "403", "503"} <= set( @@ -513,6 +513,12 @@ def test_openapi_documents_request_error_and_response_context() -> None: "POST /api/v1/projects": "project.create", "POST /api/v1/projects/{project_id}/guides": "project.guide.create", "PATCH /api/v1/projects/{project_id}/guides/{guide_id}": "project.guide.update", + "PUT /api/v1/projects/{project_id}/guides/{guide_id}/review-policy": ( + "project.review_policy.update" + ), + "PUT /api/v1/projects/{project_id}/guides/{guide_id}/revision-policy": ( + "project.revision_policy.update" + ), "POST /api/v1/projects/{project_id}/guides/{guide_id}/source-snapshots": ( "project.guide_source_snapshot.create" ), diff --git a/backend/tests/test_audit.py b/backend/tests/test_audit.py index 5c8ba0add..6d39f6ae2 100644 --- a/backend/tests/test_audit.py +++ b/backend/tests/test_audit.py @@ -190,6 +190,8 @@ def test_action_aware_audit_input_enforces_mapping_and_action_availability() -> ActionId.PROJECT_GUIDE_UPDATE, ActionId.PROJECT_GUIDE_SOURCE_SNAPSHOT_CREATE, ActionId.PROJECT_READ, + ActionId.PROJECT_REVIEW_POLICY_UPDATE, + ActionId.PROJECT_REVISION_POLICY_UPDATE, ActionId.PROJECT_ROLE_GRANT_LIST, ActionId.PROJECT_ROLE_GRANT_READ, ActionId.PROJECT_ROLE_GRANT_ISSUE, diff --git a/backend/tests/test_auth.py b/backend/tests/test_auth.py index a4c1ba53b..da1853962 100644 --- a/backend/tests/test_auth.py +++ b/backend/tests/test_auth.py @@ -1431,6 +1431,7 @@ def test_legacy_compatibility_dependency_has_fixed_consumer_allowlist() -> None: "api/deps/auth.py", "api/deps/authorization.py", "modules/projects/guide_mutation_router.py", + "modules/projects/policy_mutation_router.py", } assert {path for path, source in sources.items() if "AuthVerificationResult" in source} == { "adapters/auth/dev.py", @@ -1442,6 +1443,7 @@ def test_legacy_compatibility_dependency_has_fixed_consumer_allowlist() -> None: "core/auth.py", "interfaces/auth.py", "modules/projects/guide_mutation_router.py", + "modules/projects/policy_mutation_router.py", "schemas/auth.py", } assert { diff --git a/backend/tests/test_authorization.py b/backend/tests/test_authorization.py index 2b83fa151..0a50faac9 100644 --- a/backend/tests/test_authorization.py +++ b/backend/tests/test_authorization.py @@ -3517,6 +3517,8 @@ def __getattr__(self, name: str): ActionId.PROJECT_GUIDE_CREATE, ActionId.PROJECT_GUIDE_UPDATE, ActionId.PROJECT_GUIDE_SOURCE_SNAPSHOT_CREATE, + ActionId.PROJECT_REVIEW_POLICY_UPDATE, + ActionId.PROJECT_REVISION_POLICY_UPDATE, }: continue scope = PreparedAuthorityScope( From 87c30051f99eeb445aa93693a740b6c5bf3d147c Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 3 Aug 2026 04:30:34 +0100 Subject: [PATCH 9/9] fix(auth): revalidate locked policy context --- .../REVIEW_LOG.md | 7 ++ ...S-XINT-003-02B-external-review-response.md | 31 ++++++++ .../WS-XINT-003-02B-pr-trust-bundle.md | 8 +++ .../projects/policy_mutation_service.py | 2 + backend/tests/test_alembic.py | 72 ++++++++++++++++--- .../tests/test_project_policy_mutations.py | 25 +++++++ 6 files changed, 134 insertions(+), 11 deletions(-) diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md index 7f14d35cf..910ba9394 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/REVIEW_LOG.md @@ -103,3 +103,10 @@ locking, historical trigger allow-list, fixture-copy, and replacement-selector documentation issues. Its related indexing, typing, exact-exception, constraint-shape, and reservation-branch notes were also valid. All were fixed; none was deferred. + +CodeRabbit's second pass found a valid post-lock guide-version revalidation gap; +it now denies before PREP consumption. Hosted migration evidence then exposed a +stale 0047 head constant and an incorrect unprefixed constraint lookup. The +0048 head is now exact, the installed constraint is behaviorally exercised for +independent and partial selector cases, and the focused isolated PostgreSQL +round trip passes. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md index 543b339c1..38cc67cdd 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-external-review-response.md @@ -145,3 +145,34 @@ Remaining risks: - The replacement Backend run must pass the complete lane, E2E, and coverage sequence. + +## CodeRabbit round 2 and GitHub Actions rounds 5-6 + +Comments addressed: + +- The locked guide version is revalidated against the pre-lock snapshot before + PREP consumption, so a concurrent guide-version advance fails without + consuming authority. +- The migration suite now names revision `0048_policy_authority` as the exact + head and executes the naming-convention constraint in a temporary probe to + prove independent upgraded selectors, rejected partial triples, and the + downgraded coupled shape. + +Comments deferred: + +- None. + +Human decisions needed: + +- None. + +Commands rerun: + +- Policy mutation tests: 13 passed. +- Isolated PostgreSQL 0048 round trip: passed. +- Ruff and whitespace checks: passed. + +Remaining risks: + +- The replacement exact-head Backend run must pass the full hosted suite and + coverage sequence. diff --git a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md index 283242148..ceec4edcd 100644 --- a/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/reviews/WS-XINT-003-02B-pr-trust-bundle.md @@ -88,6 +88,14 @@ The following hosted run reached the independent full-schema fingerprint and reported the expected 0048 drift; the constant now uses the exact hosted fingerprint for the complete new schema. +CodeRabbit's second pass found that the pre-lock guide version was not compared +again after locking. The service now rejects a changed version before PREP +consumption. Later hosted migration runs exposed two stale exact-contract test +assumptions: the head revision still named 0047, and the selector lookup ignored +Alembic's constraint naming convention. Both are corrected; the installed +constraint is behaviorally exercised across independent and partial selector +cases, and the isolated 0048 PostgreSQL round trip passes. + ## Remaining risks and follow-up The API may later expose the opaque replacement selector as a response ETag. diff --git a/backend/app/modules/projects/policy_mutation_service.py b/backend/app/modules/projects/policy_mutation_service.py index 5534ce1d0..1554371c3 100644 --- a/backend/app/modules/projects/policy_mutation_service.py +++ b/backend/app/modules/projects/policy_mutation_service.py @@ -363,6 +363,8 @@ async def _replace( raise GuideNotFound("guide not found") if guide.status != "draft": raise GuideEditBlocked("only draft guides can change policy") + if guide.version != guide_snapshot.version: + raise PolicyMutationConflict("policy_precondition_failed") current = ( await self._projects.lock_review_policy(str(project_id), guide.version) if kind == "review" diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 20b70965e..c96bf0980 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -24,7 +24,7 @@ ) from sqlalchemy import select, text from sqlalchemy.exc import DBAPIError, IntegrityError -from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine +from sqlalchemy.ext.asyncio import AsyncConnection, async_sessionmaker, create_async_engine from app.adapters.auth.dev import actor_id_from_external_identity from app.core.hashing import canonical_json_hash @@ -73,7 +73,7 @@ snapshot_existing_service_rows, ) -HEAD_REVISION = "0047_policy_identity_lineage" +HEAD_REVISION = "0048_policy_authority" pytestmark = pytest.mark.postgres_schema_contract @@ -11887,7 +11887,10 @@ def test_xint003_02b_policy_authority_schema_and_roundtrip( "revision_provenance": 8, "custody_triggers": 3, "selector_constraint": True, - "independent_selector_shape": True, + "review_only_selector": True, + "revision_only_selector": True, + "partial_review_selector": False, + "partial_revision_selector": False, "selector_custody": True, "predecessor_custody": True, } @@ -11897,7 +11900,10 @@ def test_xint003_02b_policy_authority_schema_and_roundtrip( "revision_provenance": 0, "custody_triggers": 0, "selector_constraint": True, - "independent_selector_shape": False, + "review_only_selector": False, + "revision_only_selector": False, + "partial_review_selector": False, + "partial_revision_selector": False, "selector_custody": False, "predecessor_custody": False, } @@ -11955,7 +11961,7 @@ async def _xint003_02b_authority_shape(database_url: str) -> dict[str, int | boo await connection.scalar( text( "select exists(select 1 from pg_constraint where " - "conname='policy_selection_shape')" + "conname='ck_project_guides_policy_selection_shape')" ) ) ) @@ -11963,12 +11969,14 @@ async def _xint003_02b_authority_shape(database_url: str) -> dict[str, int | boo await connection.scalar( text( "select pg_get_constraintdef(oid) from pg_constraint " - "where conname='policy_selection_shape'" + "where conname='ck_project_guides_policy_selection_shape'" ) ) or "" ) - normalized_selector = " ".join(selector_definition.lower().split()) + selector_behavior = await _policy_selector_constraint_behavior( + connection, selector_definition + ) custody_definition = str( await connection.scalar( text( @@ -11984,10 +11992,7 @@ async def _xint003_02b_authority_shape(database_url: str) -> dict[str, int | boo "revision_provenance": len(columns["revision_policies"] & provenance), "custody_triggers": triggers, "selector_constraint": selector, - "independent_selector_shape": ( - "selected_review_policy_hash is not null)) and " - "((selected_revision_policy_id is null" in normalized_selector - ), + **selector_behavior, "selector_custody": "selected_review_policy_id" in custody_definition and "selected_revision_policy_id" in custody_definition, "predecessor_custody": "prior.policy_generation=product_generation-1" @@ -11997,6 +12002,51 @@ async def _xint003_02b_authority_shape(database_url: str) -> dict[str, int | boo await engine.dispose() +async def _policy_selector_constraint_behavior( + connection: AsyncConnection, definition: str +) -> dict[str, bool]: + """Exercise the installed selector expression without product trigger noise.""" + await connection.execute( + text( + "create temporary table policy_selector_probe (" + "selected_review_policy_id text, selected_review_policy_generation integer, " + "selected_review_policy_hash text, selected_revision_policy_id text, " + "selected_revision_policy_generation integer, " + "selected_revision_policy_hash text, constraint selector_probe " + f"{definition}) on commit drop" + ) + ) + cases = { + "review_only_selector": ("review", 1, "sha256:" + "1" * 64, None, None, None), + "revision_only_selector": (None, None, None, "revision", 1, "sha256:" + "2" * 64), + "partial_review_selector": ("review", None, None, None, None, None), + "partial_revision_selector": (None, None, None, "revision", None, None), + } + accepted: dict[str, bool] = {} + for name, values in cases.items(): + savepoint = await connection.begin_nested() + try: + await connection.execute( + text( + "insert into policy_selector_probe values " + "(:r_id,:r_generation,:r_hash,:v_id,:v_generation,:v_hash)" + ), + dict( + zip( + ("r_id", "r_generation", "r_hash", "v_id", "v_generation", "v_hash"), + values, + strict=True, + ) + ), + ) + accepted[name] = True + except IntegrityError: + accepted[name] = False + finally: + await savepoint.rollback() + return accepted + + async def _seed_xint003_02a_legacy_policies(database_url: str, ids: dict[str, str]) -> None: engine = create_async_engine(database_url) try: diff --git a/backend/tests/test_project_policy_mutations.py b/backend/tests/test_project_policy_mutations.py index b40ce9d1f..adda4cf62 100644 --- a/backend/tests/test_project_policy_mutations.py +++ b/backend/tests/test_project_policy_mutations.py @@ -682,6 +682,31 @@ async def test_policy_service_denies_stale_guide_and_replay_mismatch() -> None: ) assert not prepared.consumed + service, resolved, prepared, _replay, repository, project_id, guide_id = _subject() + snapshot = SimpleNamespace(**vars(repository.guide)) + + async def stale_snapshot(_guide_id): + return snapshot + + async def advanced_guide(_guide_id): + repository.guide.version = "v2" + return repository.guide + + repository.get_guide = stale_snapshot + repository.lock_project_guide = advanced_guide + with pytest.raises(PolicyMutationConflict, match="policy_precondition_failed"): + await service.replace_revision_policy( + resolved, + prepared, + uuid4(), + NO_CURRENT_POLICY_ETAG, + project_id, + guide_id, + _revision_payload(), + ) + assert not prepared.consumed + + replay = _replay key = uuid4() replay.records[(resolved.profile.id, ActionId.PROJECT_REVISION_POLICY_UPDATE.value, key)] = ( SimpleNamespace(