Add closed authorization action catalogue and audit parity#126
Conversation
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR splits AUTH-07 into catalogue and kernel stages, adds a closed typed authorization catalogue, introduces migration 0021 for action-aware audit evidence, updates audit validation and persistence, and records associated planning, review, documentation, and test changes. ChangesAUTH-07 staged authorization rollout
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant AuthorityAuditEventInput
participant AuditService
participant audit_events
Caller->>AuthorityAuditEventInput: submit action and permission identifiers
AuthorityAuditEventInput->>AuthorityAuditEventInput: validate catalogue mapping
AuthorityAuditEventInput->>AuditService: return validated audit input
AuditService->>audit_events: persist action-aware evidence
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/spec_authorization_service.md (1)
353-358: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a hyphen to join "self" and "actions".
Static analysis indicates a missing hyphen. Using a hyphen creates a clearer compound noun for "self-actions".
📝 Proposed fix
-and first active self actions. Reserved action metadata contains only the +and first active self-actions. Reserved action metadata contains only the🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/spec_authorization_service.md` around lines 353 - 358, Update the prose in the registration/completeness section so the compound noun is written as “self-actions,” including the visible “first active self actions” phrase. Preserve the surrounding meaning and wording.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/alembic/versions/0021_authorization_action_evidence.py`:
- Around line 234-238: The authorization evidence validation branch in
backend/alembic/versions/0021_authorization_action_evidence.py must accept only
SensitiveAuthorizationDenied for catalogued planned-action pairs; remove
SensitiveAuthorizationAllowed from that condition. In
backend/tests/test_alembic.py, extend the relevant authorization evidence tests
to assert that an allowed event for a matching planned action raises
IntegrityError.
---
Nitpick comments:
In `@docs/spec_authorization_service.md`:
- Around line 353-358: Update the prose in the registration/completeness section
so the compound noun is written as “self-actions,” including the visible “first
active self actions” phrase. Preserve the surrounding meaning and wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 920d63e5-f8b3-4525-9e61-18b44373ed1d
📒 Files selected for processing (26)
.agent-loop/LOOP_STATE.md.agent-loop/REVIEW_LOG.md.agent-loop/WORK_QUEUE.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DECISIONS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/PLAN.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/RISKS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-07-authorization-kernel.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-07A-closed-permission-action-catalogue.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-07B-deny-default-kernel-self-cutover.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-13-task-assignment-cutover.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-14-submission-checker-cutover.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-07-preimplementation-plan-review.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-07A-internal-review-evidence.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-07A-pr-trust-bundle.md.agent-loop/merge-intents/WS-AUTH-001-07A.jsonbackend/alembic/versions/0021_authorization_action_evidence.pybackend/app/modules/audit/schemas.pybackend/app/modules/authorization/catalogue.pybackend/app/modules/tasks/models.pybackend/tests/test_alembic.pybackend/tests/test_audit.pybackend/tests/test_authorization.pydocs/operations_authorization_service.mddocs/spec_authorization_service.md
| action_id is null or ( | ||
| event_type in ('SensitiveAuthorizationAllowed', 'SensitiveAuthorizationDenied') | ||
| and permission_id is not null | ||
| and (action_id, permission_id) in ({pair_tokens}) | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== migration slice ==\n'
sed -n '200,260p' backend/alembic/versions/0021_authorization_action_evidence.py
printf '\n== test slice ==\n'
sed -n '3680,3825p' backend/tests/test_alembic.py
printf '\n== search related text ==\n'
rg -n "SensitiveAuthorizationAllowed|SensitiveAuthorizationDenied|authorization_action|pair_tokens|planned action|availability" backend/alembic/versions/0021_authorization_action_evidence.py backend/tests/test_alembic.pyRepository: Flow-Research/workstream
Length of output: 13314
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for path, start, end in [
("backend/alembic/versions/0021_authorization_action_evidence.py", 220, 245),
("backend/tests/test_alembic.py", 3725, 3795),
]:
print(f"\n== {path} ==")
lines = Path(path).read_text().splitlines()
for i in range(start-1, min(end, len(lines))):
print(f"{i+1}: {lines[i]}")
PY
printf '\n== wider search ==\n'
rg -n "action_id.*null|event_type in \\('SensitiveAuthorizationAllowed', 'SensitiveAuthorizationDenied'\\)|allowed evidence|planned action|authorization action evidence" backendRepository: Flow-Research/workstream
Length of output: 7338
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== audit schema slice ==\n'
sed -n '340,410p' backend/app/modules/audit/schemas.py
printf '\n== audit test slice ==\n'
sed -n '130,190p' backend/tests/test_audit.py
printf '\n== alembic test around allowed event fixture ==\n'
sed -n '560,620p' backend/tests/test_alembic.py
printf '\n== search for planned action allowed evidence ==\n'
rg -n "planned action cannot produce allowed evidence|SensitiveAuthorizationAllowed|allowed evidence|action_id=null|event_type.*Allowed" backend/app backend/tests backend/alembic/versions/0021_authorization_action_evidence.pyRepository: Flow-Research/workstream
Length of output: 10835
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for path, ranges in {
"backend/app/modules/audit/schemas.py": [(370, 395)],
"backend/tests/test_audit.py": [(150, 175)],
"backend/tests/test_alembic.py": [(570, 610)],
}.items():
print(f"\n== {path} ==")
lines = Path(path).read_text().splitlines()
for start, end in ranges:
for i in range(start-1, min(end, len(lines))):
print(f"{i+1}: {lines[i]}")
PYRepository: Flow-Research/workstream
Length of output: 5016
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== migration definitions ==\n'
sed -n '1,160p' backend/alembic/versions/0021_authorization_action_evidence.py
printf '\n== catalogue planned-action behavior ==\n'
sed -n '330,385p' backend/app/modules/authorization/catalogue.py
printf '\n== audit schema planned-action rule ==\n'
sed -n '372,388p' backend/app/modules/audit/schemas.py
printf '\n== test fixture around allowed event ==\n'
sed -n '560,590p' backend/tests/test_alembic.pyRepository: Flow-Research/workstream
Length of output: 12283
Disallow allowed rows for planned actions. backend/alembic/versions/0021_authorization_action_evidence.py#L234-L238 still accepts SensitiveAuthorizationAllowed when (action_id, permission_id) matches the catalogue, but the audit schema rejects allowed evidence for planned actions. Tighten this branch to SensitiveAuthorizationDenied, and add an allowed-event IntegrityError check in backend/tests/test_alembic.py#L3725-L3790.
📍 Affects 2 files
backend/alembic/versions/0021_authorization_action_evidence.py#L234-L238(this comment)backend/tests/test_alembic.py#L3725-L3790
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/alembic/versions/0021_authorization_action_evidence.py` around lines
234 - 238, The authorization evidence validation branch in
backend/alembic/versions/0021_authorization_action_evidence.py must accept only
SensitiveAuthorizationDenied for catalogued planned-action pairs; remove
SensitiveAuthorizationAllowed from that condition. In
backend/tests/test_alembic.py, extend the relevant authorization evidence tests
to assert that an allowed event for a matching planned action raises
IntegrityError.
Source: Coding guidelines
…to codex/ws-auth-001-07-authorization-kernel # Conflicts: # .agent-loop/WORK_QUEUE.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-07A-external-review-response.md:
- Around line 14-17: Update the review response’s evidence from 30 to 50 planned
actions, including every repeated count and any exhaustive-proof references in
the affected sections. Ensure the wording accurately reflects the contract’s
complete 50-action catalogue without changing unrelated claims.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 340815bc-d52b-42e7-ad13-7a925e317472
📒 Files selected for processing (19)
.agent-loop/LOOP_STATE.md.agent-loop/REVIEW_LOG.md.agent-loop/WORK_QUEUE.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DECISIONS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/PLAN.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-07A-closed-permission-action-catalogue.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-07B-deny-default-kernel-self-cutover.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-07A-external-review-response.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-07A-internal-review-evidence.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-07A-pr-trust-bundle.mdbackend/alembic/versions/0021_authorization_action_evidence.pybackend/app/modules/authorization/catalogue.pybackend/tests/test_alembic.pybackend/tests/test_audit.pybackend/tests/test_authorization.pydocs/decision_0012_workstream_authorization_service.mddocs/operations_authorization_service.mddocs/spec_authorization_service.md
🚧 Files skipped from review as they are similar to previous changes (12)
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-07A-internal-review-evidence.md
- .agent-loop/LOOP_STATE.md
- backend/tests/test_audit.py
- .agent-loop/WORK_QUEUE.md
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-07B-deny-default-kernel-self-cutover.md
- .agent-loop/REVIEW_LOG.md
- backend/alembic/versions/0021_authorization_action_evidence.py
- docs/operations_authorization_service.md
- backend/app/modules/authorization/catalogue.py
- .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md
- backend/tests/test_alembic.py
- docs/spec_authorization_service.md
Chunk And Merge Intent
WS-AUTH-001-07A- Closed Permission And Action Catalogue. Merge intent points toWS-AUTH-001-07Band requires signed merge memory plus a separate explicit human start.Goal
Install one closed typed source for exactly 74 approved PermissionIds and 50 planned ActionIds, then preserve exact typed/PostgreSQL audit parity without making any action executable.
Canonical Review/Revision Amendment
0020permission partitionreview.queue.overrideas a PermissionIdsubmission.createplus 19 review ActionIdssubmission.createaction, permission, and route for initial and revision submissionssubmission.reviseartifact.verification_job.retryPublic Authorization Boundary
Feature modules consume a request-scoped
AuthorizationServicebound once to the currentAuthorizationContextand caller-ownedAsyncSession:The method accepts no raw PermissionId, grant, guard, role, session, or
uowargument and never commits. Feature modules own typed resource-context composition and lifecycle invariants; they do not import AUTH persistence or grant loaders.Scope Control
All 50 actions remain
planned. This PR does not add a decision evaluator, grants, principals, resource loaders, guards, caches, route cutovers, review behavior, artifact operations, or public permission APIs. AUTH-07B remains inactive until this PR merges, signed memory updates, and the user separately starts it.Evidence
Reviewer Results
Senior engineering, architecture/reuse, security/auth, product/ops, docs, QA/test, test-delta, migration integrity, and CI integrity passed the exact implementation and evidence heads with no remaining findings.
CI Integrity
No workflow, dependency, package script, coverage exclusion, skip, or threshold changed. GitHub Backend remains authoritative for the repository-wide 78 percent floor; both materially changed subsystems remain above 90 percent.
Human Review Focus
Review the exact 74/50 catalogue, canonical submission/revision reuse, least-privilege review mappings, single transaction-bound public interface, SQL closure, guarded rollback, and absence of capability activation.
Human Merge Ownership
Only the human may approve and merge this PR. CI, CodeRabbit, and internal review do not authorize merge.
Summary by CodeRabbit
New Features
Documentation