Add bootstrap and administrative role grants#131
Conversation
📝 WalkthroughWalkthroughThis PR implements WS-AUTH-001-08: one-time local Access Administrator bootstrap and administrative role grant management. It adds migration 0022 (AdminRoleGrant/AuthorityControl tables with immutability triggers), extends the authorization kernel/runtime with admin action evaluation, adds admin grant schemas/service/repository/router, a bootstrap CLI script, actor-self profile wiring for admin roles, plus extensive tests, docs, and agent-loop tracking artifacts. ChangesAUTH-08 Bootstrap And Administrative Role Grants
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant Router
participant AdminRoleGrantService
participant AuthorizationService
participant AdminAuthorizationRepository
participant AuditService
Router->>AdminRoleGrantService: reserve(idempotency_key)
Router->>AuthorizationService: require(issue resource context)
AuthorizationService->>AdminAuthorizationRepository: lock control/grant/resource
AdminAuthorizationRepository-->>AuthorizationService: guard results
AuthorizationService->>AuditService: stage decision + evidence
AuditService-->>Router: AuthorizationDecision
Router->>AdminRoleGrantService: complete_issue(claim, decision)
AdminRoleGrantService->>AdminAuthorizationRepository: add_grant
AdminRoleGrantService-->>Router: AuthorityMutationResponse
sequenceDiagram
participant Operator
participant BootstrapScript
participant AdminRoleGrantService
participant Database
Operator->>BootstrapScript: run --execute --actor-profile-id
BootstrapScript->>AdminRoleGrantService: bootstrap(actor_profile_id)
AdminRoleGrantService->>Database: lock AuthorityControl
AdminRoleGrantService->>Database: verify eligibility, create grant
AdminRoleGrantService->>Database: mark bootstrap completed + audit event
AdminRoleGrantService-->>BootstrapScript: grant_id
BootstrapScript-->>Operator: JSON result + exit code
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/tests/test_alembic.py (1)
413-425: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winPin this guard test below migration 0022.
The test upgrades to
headbut still expects a failed downgrade to restore revision0021_auth_action_evidence. With head now at 0022, the transactional failure restores 0022 instead. Upgrade explicitly to 0021 so this remains a focused canonical-actor test.Proposed fix
- command.upgrade(config, "head") + command.upgrade(config, "0021_auth_action_evidence")🤖 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/tests/test_alembic.py` around lines 413 - 425, Update the migration setup in the canonical-actor downgrade guard test to upgrade explicitly to revision 0021_authority_idempotency instead of head, while preserving the existing failed-downgrade assertions and expected revision 0021_auth_action_evidence.
🧹 Nitpick comments (1)
backend/app/modules/authorization/models.py (1)
193-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep ORM server defaults aligned with migration 0022.
The migration defines server defaults for status/version/bootstrap fields and uses
clock_timestamp(), while these models use client defaults orfunc.now(). Mirror the migration defaults to prevent schema metadata drift and misleading future Alembic revisions.As per coding guidelines, ORM and migrations must use SQLAlchemy 2.x and Alembic.
Also applies to: 225-233
🤖 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/app/modules/authorization/models.py` around lines 193 - 203, Align the mapped columns in the authorization model, including status, version, bootstrap/grant fields, and granted_at, with migration 0022 by replacing client-side default or func.now() usage with matching SQLAlchemy server_default expressions. Use the migration’s exact default values and clock_timestamp() expression so ORM metadata and Alembic schema definitions remain consistent.Source: Coding guidelines
🤖 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/0022_bootstrap_admin_grants.py`:
- Around line 473-479: Add a deferred constraint trigger in the bootstrap grant
migration that validates the cross-table invariant at transaction commit: allow
either no bootstrap grant with incomplete authority control, or exactly one
bootstrap grant referenced by completed control. Apply it to changes in both
admin_role_grants and authority_control, using the existing bootstrap grant and
control identifiers, while preserving the current row-level guards.
In `@backend/tests/test_alembic.py`:
- Around line 572-595: The migration 0022 upgrade adoption guard must recognize
the same AUTH-08 evidence as its downgrade guard. Update the upgrade predicate
in the 0022 migration to include AdminRoleGrantIssueDenied and
LastAccessAdministratorOperationDenied, then extend
test_bootstrap_admin_grant_schema_is_immutable_and_guarded to insert and assert
rejection of both denial events before cleanup.
In `@backend/tests/test_authorization.py`:
- Around line 1919-1920: Update the admin grant/revoke test cases around
admin_revoke_target and the referenced ranges so provenance records the distinct
grant ID that authorized the decision, not the newly issued grant or revoked
target. Pass the authorizer grant ID separately for both admin operations,
preserving matched_grant_id as the authorization source and avoiding
self-referential provenance.
In `@docs/operations_authorization_service.md`:
- Around line 542-544: Synchronize the action-catalogue counts throughout
docs/operations_authorization_service.md with the canonical contract in
docs/spec_authorization_service.md: update the earlier staging text from 50
total and two active to 57 total, 9 active, and 48 planned, while preserving the
existing description of the two self actions and seven AUTH-08 actions.
---
Outside diff comments:
In `@backend/tests/test_alembic.py`:
- Around line 413-425: Update the migration setup in the canonical-actor
downgrade guard test to upgrade explicitly to revision
0021_authority_idempotency instead of head, while preserving the existing
failed-downgrade assertions and expected revision 0021_auth_action_evidence.
---
Nitpick comments:
In `@backend/app/modules/authorization/models.py`:
- Around line 193-203: Align the mapped columns in the authorization model,
including status, version, bootstrap/grant fields, and granted_at, with
migration 0022 by replacing client-side default or func.now() usage with
matching SQLAlchemy server_default expressions. Use the migration’s exact
default values and clock_timestamp() expression so ORM metadata and Alembic
schema definitions remain consistent.
🪄 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: 51650bbb-780b-452d-bbc4-1f37ae78c1ef
📒 Files selected for processing (43)
.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/STATUS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-08-bootstrap-admin-grants.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-08-internal-review-evidence.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-08-pr-trust-bundle.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-08-preimplementation-plan-review.md.agent-loop/merge-intents/WS-AUTH-001-08.jsonbackend/alembic/versions/0022_bootstrap_admin_grants.pybackend/app/api/deps/authorization.pybackend/app/api/router.pybackend/app/api/routes/auth.pybackend/app/db/models.pybackend/app/modules/actors/repository.pybackend/app/modules/actors/service.pybackend/app/modules/audit/schemas.pybackend/app/modules/authorization/admin_schemas.pybackend/app/modules/authorization/admin_service.pybackend/app/modules/authorization/catalogue.pybackend/app/modules/authorization/kernel.pybackend/app/modules/authorization/models.pybackend/app/modules/authorization/policy.pybackend/app/modules/authorization/repository.pybackend/app/modules/authorization/router.pybackend/app/modules/authorization/runtime.pybackend/app/modules/authorization/schemas.pybackend/app/modules/authorization/service.pybackend/app/modules/projects/repository.pybackend/scripts/bootstrap_access_administrator.pybackend/tests/conftest.pybackend/tests/test_actors.pybackend/tests/test_alembic.pybackend/tests/test_api_controls.pybackend/tests/test_api_rate_controls.pybackend/tests/test_audit.pybackend/tests/test_auth.pybackend/tests/test_authorization.pydocs/operations_authorization_service.mddocs/operations_roles_permissions.mddocs/spec_authorization_service.md
Workstream PR Trust Bundle
Chunk
WS-AUTH-001-08- Bootstrap And Administrative Role GrantsMerge intent:
.agent-loop/merge-intents/WS-AUTH-001-08.jsonGoal
Establish one-time local Access Administrator bootstrap and immutable administrative grants, then activate only the seven administrative actions owned by AUTH-08 through the central authorization kernel.
Human-Approved Intent
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/INTENT.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-08-bootstrap-admin-grants.mdWhat Changed
0022with immutableAdminRoleGrantandAuthorityControltruth.Why It Changed
AUTH-07B installed deny-by-default authorization but deliberately granted no administrative authority. AUTH-08 provides the first local grant truth needed for later service-actor, project-grant, and product cutover chunks without treating token roles, email, or default Contributor state as authority.
Design Chosen
Identity remains global and external-issuer verified. Administrative authority comes only from active Workstream grants with exact system or project scope. Bootstrap is a serialized operational ceremony, while all later issue/revoke operations require an active system-scoped Access Administrator grant. Deferred database triggers require the bootstrap grant and control row to reach one valid final state in the same transaction.
Alternatives Rejected
Scope Control
Allowed Files Changed
Files Outside Contract
Product Behavior
Evidence
Commands Run
Result Summary
GitHub Backend remains authoritative for the repository-wide 78% floor. No workflow, threshold, exclusion, dependency, or skip changed.
Acceptance Criteria Proof
Test Delta
Tests Added
Tests Modified
Tests Removed Or Skipped
Internal Reviewer Results
Reviewed code SHA:
a284a718e68f08e072cb965cc5834bcc7ab45ee9Reviewed at:
2026-07-16T07:43:50ZReviewer run IDs:
auth08_final_senior,auth08_final_qa,auth08_final_securityExternal Review
External review response file:
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-08-external-review-response.mda284a71; all four threads are resolved. The enforced docstring gate passes at 92.5%.29481047118passed in 19m14s; Agent Gates passed.CI And Gate Integrity
Remaining Risks
Follow-Up Work
WS-AUTH-001-09is named only as the inactive same-initiative successor. It requires AUTH-08 merge, automated merge memory, and a separate explicit user start.Human Review Focus
Please inspect:
Human Merge Ownership