[RBAC PR 1 follow-up] Fail closed for grant delegation#2339
Open
philipfweiss wants to merge 1 commit into
Open
[RBAC PR 1 follow-up] Fail closed for grant delegation#2339philipfweiss wants to merge 1 commit into
philipfweiss wants to merge 1 commit into
Conversation
✅ Deploy Preview for thriving-cassata-78ae72 canceled.
|
philipfweiss
force-pushed
the
rbac-control-plane-readiness
branch
2 times, most recently
from
July 18, 2026 00:45
d66b710 to
241c040
Compare
philipfweiss
marked this pull request as ready for review
July 18, 2026 01:16
Require explicit MANAGE grants containing every delegated scope so permissive fallback cannot authorize role or group administration. Align RBAC enum serialization and history labels with migrated schemas.
philipfweiss
force-pushed
the
rbac-control-plane-readiness
branch
from
July 18, 2026 01:42
241c040 to
a24b834
Compare
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.
Tracking: #2234 (step 1 follow-up in the RBAC enablement sequence).
DJ's default RBAC policy is permissive, and the endpoints that hand out permissions (create a role, add a scope, assign a role, manage a group) rely on that same fallback. So on a default deployment, any logged-in user can
POST /roles/grantingnode:*(write on every node) and assign it to themselves. The permission system cannot be enforced if any user can grant themselves any permission.Why this was missed initially: #2230 added the
MANAGEchecks and was verified with the policy flipped to restrictive, and its endpoint tests mockedMANAGEdenial directly. Neither path ran the real permission logic under the normal permissive default, which is exactly where the hole is. The test database is also built from ORM metadata, so it never hit the Alembic enum mismatch that this PR also fixes.Background: what "grant delegation" means here
The RBAC admin endpoints are how one principal hands permissions to another. Handing out access is itself a privileged action. The rule this PR enforces: you can only give away access that you already administer. Concretely, if you hold
MANAGEon thefinance.*namespace, you may create or assign roles scoped insidefinance.*, but not roles that touchgrowth.*or everything.What this PR changes
These rules now apply to the role and group admin endpoints only. Ordinary metadata calls (creating nodes, etc.) are unchanged.
No permissive fallback for granting permissions. Creating, updating, assigning, or revoking a role, and creating or editing a group, now require the caller to actually hold a matching
MANAGEgrant. The permissive default no longer counts as authorization for these endpoints. This is the core fix: it is what stops a normal user from self-grantingnode:*.You can only delegate what you manage (containment). Every scope in a role you create or assign must fit inside one of your own
MANAGEscopes. With a caller who holdsMANAGEonnamespace:finance.*:readonfinance.revenue.*(insidefinance.*)readongrowth.*(a namespace you do not manage)writeonnode:*ornamespace:*(everything)Over-broad and malformed patterns are rejected. Only three scope shapes are accepted for containment: a single namespace/node (
finance.revenue), a subtree (finance.*), or global (*). Ambiguous values likefinance*,.*,**, or mid-string wildcards can never satisfy the containment check, so they cannot be used to sneak past as an accidental match-all.Admins still bypass. The existing
is_adminbreak-glass path still authorizes these endpoints, so a fresh deployment can bootstrap its first grants.Migrated-database fix (found during manual verification). On a real Alembic-migrated database, the RBAC scope enums are stored as lowercase and the
historyentity enum was missing the RBAC labels, so creating a role returned a 500. This PR serializes the enums with the values Alembic actually created and adds a migration for the missing history labels, so the endpoints work on deployed schemas, not only on test databases.Behavior change: on the default permissive policy, the role and group admin endpoints now return 403 instead of silently allowing a self-grant. A caller needs an explicit
MANAGEgrant (or admin). Everything outside these endpoints keeps its current behavior, and the passthrough provider still approves everything for local dev and tests.Verification:
djuser, and confirmed the server was healthy.MANAGE namespace:finance.*) assigned todj, then droppeddjback to a non-admin user.finance.*is allowed while anything outside it is denied.