Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…> holders group Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oint Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… with equivalence gate SP3c-1 Task 4. Rewrites every kind='role' principal in perm.grant_principals, perm.rule_exemptions and perm.engine_grant_principals into a kind='group' ref to that role's managed `role:<name> holders` group, proving the compiled artifact is byte-identical before vs after (role and group expand to the same usernames via members::expand_enumerated_principals) and refusing with a rollback on any divergence. The gate also builds equivalence::derive_probes over each affected profile and runs compare, refusing on any verdict divergence. Provenance (migration 0221, perm.role_principal_migration) records each rewritten row plus a canonical per-row hash; undo is FOR UPDATE on the provenance row, re-hashes every rewritten row, refuses 409 if any diverged, else restores the role refs and recompiles. Only perm.* principal tables are touched — no public.access_grants row is written. principal_refs accepts a `^role:.* holders$` group ref as a valid managed-group principal. Also simplifies two pre-existing explicit_auto_deref clippy nits in role_holder_group.rs so the crate's clippy -D warnings gate is green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…undo restores one row SP3c-1 Task 4 review fixes. Fix 1 (fail-open regression): principal_refs no longer blanket-accepts any `role:<name> holders` group ref. A managed holder group is now treated like any other group — accepted only when its `public.groups` row exists, and flagged fail-open (Unresolved, surfaced by warn_fail_open_principals) when it does not, never silently Empty. The migration is unaffected: it bypasses validation (raw UPDATEs) and syncs the holder group in the same tx before storing any ref to it, so the row always exists by then. Adds a unit test proving an authoring write naming a never-synced `role:ghost holders` group is refused and classified Unresolved, not Empty. Fix 2 (undo hardening): restore_row now asserts it updated exactly one row and returns a Conflict (rolling the tx back) otherwise, so an undo can never delete provenance and report success after restoring nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… write guards Adds POST /perm/migrate/role-principals and .../undo, and refuses NEW role principals and role:<name> holders group refs at the grant, rule and engine-grant write seam (principal_refs write path only; lookup unchanged so migrated refs keep compiling). Two existing tests that authored role principals through the API now seed them directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tion Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The SDD plan (en + vi) for migrating kind='role' principals into managed role:<name> holders groups, with the role-holder sync machinery, the reversible equivalence-gated rewrite, the transitional write guards, and the frontend + docs follow-up tasks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… dialogs The backend now rejects new role principals; the picker no longer offers the role kind or its free-text field. Exempt mode and read/explain rendering of stored role principals are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SP3c-1 final review fixes. Fix 1 (fail-open revoke): after the role-principal migration, a grant that named a role now names its managed `role:<name> holders` group, but the live control-plane role paths (assign/unassign/delete) neither re-synced that group nor recompiled the group-ref profiles, so a revoke left the ex-holder granted until restart. - affected_services_for_role_membership now also matches grant, path/tag rule-exemption and engine-grant principals whose kind='group' ref is the role's holder group, UNIONed with the existing role arms. - with_recompile_in_tx (the shared seam for assign/unassign and every control-plane role path) and delete_platform_role_and_recompile now call sync_role_holder_group_in_tx before recompiling, in the same transaction, guarded on the managed group already existing (so a pre-migration write mints nothing) — a single recompile, mirroring the boot repair. Fix 2 (silent adoption): sync_role_holder_group_in_tx refused to create the managed group over a pre-existing local/oidc group of the same name; it now guards the ON CONFLICT upsert on source='managed' and returns the new CompileError::HolderGroupNameCollision rather than adopting and reconciling an operator- or IdP-owned group outside the equivalence gate. Tests (DB-gated, egret-perm-compile): a live revoke via unassign_platform_role_and_recompile drops the holder from the managed group and recompiles the group-ref grant so the compiled artifact no longer names them (fail-closed); a pre-existing non-managed group of the managed name is refused, not adopted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SP3c-1 follow-up: the scoped re-review found one more live role-holder-change
path that recompiles without syncing the managed `role:<name> holders` group —
egret_workspace::rbac::Authorizer::replace_platform_roles (the admin route
PUT /api/admin/users/{id}/platform-roles). It already widened its affected set
with affected_services_for_role_membership and recompiled, but never re-synced
the holder group, so after the role-principal migration a revoke through this
bulk path re-expanded `role:X holders` from stale user_groups and the ex-holder
kept access until the boot repair. Unlike the single-role path this one was
fail-CLOSED before the migration, so this is a regression.
- Factor the guarded sync into a reusable public helper
role_holder_group::sync_role_holder_group_if_present_in_tx (sync only when the
managed group already exists — no-op pre-migration, mirroring the boot repair),
and use it in with_recompile_in_tx and delete_platform_role_and_recompile in
place of the inlined guard (dropping the now-unused local helper).
- replace_platform_roles now calls it for every changed role (losing + gaining)
after the access_grants write and before its single recompile, in the same
transaction.
Test (DB-gated, egret-workspace): seed a role grant + platform holder carol,
migrate, then revoke via replace_platform_roles — carol is dropped from the
holder group and the recompiled artifact no longer grants her (fail-closed).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Governance SP3c-1 — role principals → managed
role:<name> holdersgroupsFirst of the SP3c packages. Collapses
kind='role'principals into egret's one group-based principal model, so the enforcement input no longer carries role references.What it does
source='managed'groups (migration 0220). A managed grouprole:<name> holdersper referenced role, whoseuser_groupsmembership mirrors the role's holders (role_holder_group.rs).role_principal_migrate.rs, provenance migration 0221): everykind='role'principal in grants / rule exemptions / engine grants becomes akind='group'ref to that holder group. A dual gate (fingerprint over compiled bodies + probe comparison) refuses the migration on any divergence, so the compiled engine output is byte-identical andexplainis unchanged for every affected principal. Admin endpointsPOST /perm/migrate/role-principals(+/undo, hash-guarded).rbac::replace_platform_roles— re-sync the managed group before recompiling, andaffected_services_for_role_membershipnow also matches the holder-group refs. This preserves fail-closed-on-revoke after the migration.kind='role'principals and new refs to arole:* holdersgroup are refused at the write seam (migration targets only). The grant and engine-permission dialogs no longer offer arolekind; role read/explain rendering and rule-exemption authoring are unchanged.Scope (measured)
The demo stack has exactly one referenced role (
data_steward); the code is general but adds nothing for unreferenced roles.Verification
cargo fmt --check,cargo clippy -p egret-perm -p egret-perm-compile -p egret-controlplane -p egret-workspace --all-targets -- -D warningsclean.role_principal_migrate13/13 (byte-identical migrate, undo restore + 409, ghost-ref fails-open, explain byte-identical),platform_role_recompile1/1 (revoke viareplace_platform_rolesdrops the holder and recompiles fail-closed).committed_openapi_json_is_current+ generated-client digest regenerated clean. Frontend vitest green.Reviewed per SDD: per-task reviews + a whole-branch opus review that caught (and this PR fixes) a fail-open-on-revoke across all three live role paths.
Docs: egret-data/docs PR (role → managed group; authoring no longer offers role).
Deferred (pre-existing, boot-repair-caught, not regressions): role-holder changes via user/group hard-delete, grant expiry sweep, and workspace-scope role grants.
🤖 Generated with Claude Code