-
Notifications
You must be signed in to change notification settings - Fork 7
AUTH-12A: register project mutation authorization contracts #226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
abiorh-claw
merged 6 commits into
main
from
codex/ws-auth-001-12a-project-mutation-catalogue
Jul 30, 2026
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
fc3d2be
feat(auth): register project mutation authorization contracts
Abiorh001 3667899
fix(auth): use canonical Alembic constraint name
Abiorh001 62e877e
test(auth): bound migration custody proof
Abiorh001 8cb6eab
test(auth): refresh 0041 schema fingerprint
Abiorh001 232996c
test(auth): preserve historical action boundaries
Abiorh001 76ef371
fix(auth): address project mutation review findings
Abiorh001 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
51 changes: 51 additions & 0 deletions
51
...tream-authorization-service/reviews/WS-AUTH-001-12A-external-review-response.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| # WS-AUTH-001-12A External Review Response | ||
|
|
||
| External review: CodeRabbit review on PR #226 at head `232996c3` | ||
|
|
||
| ## Comments addressed | ||
|
|
||
| - Replaced the migration placeholder in the allowed-file boundary with the | ||
| exact implemented `0041_project_mutation_action_evidence.py` path. The | ||
| revision identifier remains the separately frozen | ||
| `0041_project_mutation_evidence` value. | ||
| - Added `tests/conftest.py` to the chunk's Ruff verification command so every | ||
| changed Python file is covered by the declared lint proof. | ||
| - Replaced the tautological `PermissionId` membership assertion with the exact | ||
| four pre-existing permissions used by the eighteen project-mutation actions. | ||
| - Documented the migration's intentional dependency on PostgreSQL constraint | ||
| rendering and its fail-closed drift guards. | ||
| - Made the project-mutation resource and target-kind maps immutable. | ||
| - Hoisted the static admin action-to-resource map out of the authorization hot | ||
| path and froze it. | ||
| - Made resource-to-PREP scope derivation explicitly static, removing the test's | ||
| `None` receiver workaround. | ||
| - Consolidated repeated setup-service custody checks without changing their | ||
| validation order or error wording. | ||
|
|
||
| ## Comments deferred | ||
|
|
||
| None. All three actionable comments and five collapsed nitpicks were addressed. | ||
|
|
||
| ## Human decisions needed | ||
|
|
||
| None. | ||
|
|
||
| ## Commands rerun | ||
|
|
||
| - `git diff --check` | ||
| - Ruff over `tests/test_alembic.py` and `tests/conftest.py` | ||
| - Focused authorization tests: `2 passed, 366 deselected` | ||
| - The local isolated PostgreSQL runner refused the configured admin database | ||
| with `unsafe_admin_database`; this safety guard was not bypassed, and hosted | ||
| CI owns the isolated migration proof. | ||
| - Stale authorization wording, stale Workstream wording, and Markdown-link | ||
| checks | ||
|
|
||
| Hosted `Backend / test` and `Agent Gates` are pending on the corrective pushed | ||
| head and remain required before merge readiness. | ||
|
|
||
| ## Remaining risks | ||
|
|
||
| The actions remain planned and externally inactive. Runtime activation and | ||
| mutation-service integration remain owned by their separately bounded child | ||
| chunks. |
129 changes: 129 additions & 0 deletions
129
backend/alembic/versions/0041_project_mutation_action_evidence.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| """register planned project-mutation action evidence | ||
|
|
||
| Revision ID: 0041_project_mutation_evidence | ||
| Revises: 0040_guide_materialization | ||
| Create Date: 2026-07-29 | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from alembic import op | ||
| import sqlalchemy as sa | ||
|
|
||
|
|
||
| revision = "0041_project_mutation_evidence" | ||
| down_revision = "0040_guide_materialization" | ||
| branch_labels = depends_on = None | ||
|
|
||
| _ACTIONS = ( | ||
| ("project.create", "project.create"), | ||
| ("project.guide.create", "project.guide.manage"), | ||
| ("project.guide.update", "project.guide.manage"), | ||
| ("project.guide_source_snapshot.create", "project.guide.manage"), | ||
| ("project.review_policy.update", "project.review_policy.manage"), | ||
| ("project.revision_policy.update", "project.review_policy.manage"), | ||
| ("project.guide_sufficiency_report.create", "project.guide.manage"), | ||
| ("project.guide_sufficiency.run", "project.guide.manage"), | ||
| ("project.guide_sufficiency.warnings.acknowledge", "project.guide.manage"), | ||
| ("project.submission_artifact_policy.create", "project.effective_policy.manage"), | ||
| ("project.submission_artifact_policy.derive", "project.effective_policy.manage"), | ||
| ("project.submission_artifact_policy.update", "project.effective_policy.manage"), | ||
| ("project.submission_artifact_policy.approve", "project.effective_policy.manage"), | ||
| ("project.post_submit_checker_policy.approve", "project.effective_policy.manage"), | ||
| ( | ||
| "project.post_submit_checker_policy.correction.request", | ||
| "project.effective_policy.manage", | ||
| ), | ||
| ("project.post_submit_checker_policy.derive", "project.effective_policy.manage"), | ||
| ("project.setup_run.update", "project.guide.manage"), | ||
| ("project.guide.activate", "project.guide.manage"), | ||
| ) | ||
|
|
||
|
|
||
| def _definition() -> str: | ||
| return ( | ||
| op.get_bind() | ||
| .execute( | ||
| sa.text( | ||
| "select pg_get_constraintdef(oid) from pg_constraint " | ||
| "where conrelid='audit_events'::regclass " | ||
| "and conname='ck_audit_events_authorization_action_evidence'" | ||
| ) | ||
| ) | ||
| .scalar_one() | ||
| ) | ||
|
|
||
|
|
||
| def _replace(definition: str) -> None: | ||
| op.drop_constraint( | ||
| "authorization_action_evidence", | ||
| "audit_events", | ||
| type_="check", | ||
| ) | ||
| op.execute( | ||
| "alter table audit_events add constraint " | ||
| f"ck_audit_events_authorization_action_evidence {definition}" | ||
| ) | ||
|
|
||
|
|
||
| def _pair_token(action: str, permission: str) -> str: | ||
| # Keep this byte-for-byte aligned with PostgreSQL's pg_get_constraintdef | ||
| # rendering; _rewrite's marker-count guards fail closed if that format drifts. | ||
| return ( | ||
| f"(((action_id)::text = '{action}'::text) AND " | ||
| f"((permission_id)::text = '{permission}'::text))" | ||
| ) | ||
|
|
||
|
|
||
| def _rewrite(*, add: bool) -> None: | ||
| definition = _definition() | ||
| additions = " OR " + " OR ".join(_pair_token(*pair) for pair in _ACTIONS) | ||
| marker = _pair_token("project.active_guide.read", "project.read") | ||
| if add: | ||
| if definition.count(marker) != 2 or any( | ||
| _pair_token(*pair) in definition for pair in _ACTIONS | ||
| ): | ||
| raise RuntimeError("unexpected project-mutation action registry definition") | ||
| definition = definition.replace(marker, marker + additions) | ||
| else: | ||
| if definition.count(additions) != 2: | ||
| raise RuntimeError("unexpected project-mutation action registry definition") | ||
| definition = definition.replace(additions, "") | ||
| _replace(definition) | ||
|
|
||
|
|
||
| def _lock_evidence() -> None: | ||
| bind = op.get_bind() | ||
| bind.execute(sa.text("lock table authority_idempotency_records in share row exclusive mode")) | ||
| bind.execute(sa.text("lock table audit_events in access exclusive mode")) | ||
|
|
||
|
|
||
| def _has_evidence() -> bool: | ||
| actions = [action for action, _ in _ACTIONS] | ||
| return bool( | ||
| op.get_bind() | ||
| .execute( | ||
| sa.text( | ||
| "select exists(select 1 from audit_events where action_id = any(:actions)) or " | ||
| "exists(select 1 from authority_idempotency_records record " | ||
| "join audit_events event on event.idempotency_reference=record.id " | ||
| "where event.action_id = any(:actions))" | ||
| ), | ||
| {"actions": actions}, | ||
| ) | ||
| .scalar_one() | ||
| ) | ||
|
|
||
|
|
||
| def upgrade() -> None: | ||
| """Register the eighteen project-mutation pairs without activating them.""" | ||
| _lock_evidence() | ||
| _rewrite(add=True) | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| """Remove project-mutation pairs only when no forward evidence exists.""" | ||
| _lock_evidence() | ||
| if _has_evidence(): | ||
| raise RuntimeError("cannot downgrade non-empty project-mutation action evidence") | ||
| _rewrite(add=False) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.