fix(security): govern sys_member writes — membership is not a delegable capability (#3697 follow-up) - #3767
Merged
Merged
Conversation
…able capability (#3697 follow-up) `DelegatedAdminGate`'s `GOVERNED_OBJECTS` covered the four RBAC link tables but not `sys_member`, so the table that decides WHO IS AN ORG ADMIN was the one authority surface the delegated-administration gate never saw. A membership row is an authority dial: `role` containing `owner`/`admin` is auto-elevated to `organization_admin` by `auto-org-admin-grant.ts`, and that set's wildcard `modifyAllRecords` is exactly what `isTenantAdmin()` tests. So writing one mints a tenant admin — the same escalation the invitation role cap closes on the issuance path, one layer down at the table. Not exploitable today, and this changes no working behaviour: every `sys_member` writer is a better-auth path running under `isSystem`, which short-circuits the whole security middleware before this gate, and the ADR-0092 D2 identity write guard refuses user-context writes to better-auth-managed tables upstream of it. The gate is added so the chain cannot silently reopen the day a direct-write surface is introduced — a `case` label is not enforcement, and the call site is what decides (Prime Directive #10). Tenant-admin-only rather than scope-delegable, deliberately: no axis of `AdminScope` expresses "organization membership" (its vocabulary is BU subtree, action flags, assignable-set allowlist), so there is nothing for a delegated scope to approve part of — and a delegate who could write one would mint authority strictly greater than their own, which is what ADR-0090 D12 exists to prevent. Adding people already has a delegable path: the INVITATION, whose placement this gate authorizes and whose role the framework caps at the issuer's own grade. The refusal message points there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015s4qPrCKBvVwmFeSsFHgNp
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 11 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 28, 2026 03:44
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.
Closes the one open non-goal from #3697 / #3722.
The gap
DelegatedAdminGate.GOVERNED_OBJECTScovered the four RBAC link tables but notsys_member— so the table that decides who is an org admin was the one authority surface the delegated-administration gate never saw.A membership row is an authority dial:
rolecontainingowner/adminis auto-elevated toorganization_adminbyauto-org-admin-grant.ts, and that set's wildcardmodifyAllRecordsis exactly whatisTenantAdmin()tests. Writing one mints a tenant admin — the same escalation the invitation role cap (#3722) closes on the issuance path, one layer down at the table.Not exploitable today — and this changes no working behaviour
Every
sys_memberwriter is a better-auth path running underisSystem, which short-circuits the whole security middleware before this gate; the ADR-0092 D2 identity write guard refuses user-context writes to better-auth-managed tables upstream of it. So nothing legitimate reaches the new refusal.It is added so the chain cannot silently reopen the day a direct-write surface is introduced. That is precisely the "declared ≠ enforced, and the call site is what matters" lesson from Prime Directive #10 — the previous state left the invariant true only by accident of routing.
Why tenant-admin-only rather than scope-delegable
Every other governed object has an
adminScopeaxis that can approve part of it — a subtree, an action flag, an allowlist.sys_memberhas none: the scope vocabulary is BU subtree /manageAssignments/manageBindings/authorEnvironmentSets/assignablePermissionSets, and not one of them expresses "organization membership". There is nothing for a delegated scope to approve part of, and a delegate who could write one would mint authority strictly greater than their own — the thing ADR-0090 D12 exists to prevent.Adding people to an organization already has a delegable path: the invitation, whose placement this same gate authorizes against the issuer's
adminScopeand whose role #3722 caps at the issuer's own grade. The refusal message points there rather than dead-ending:Changes
packages/plugins/plugin-security/src/delegated-admin-gate.tssys_memberjoinsGOVERNED_OBJECTS; refused for everyone below tenant admin, placed right after the tenant-admin short-circuit so the message is specific rather than falling through to the generic RBAC-table onepackages/plugins/plugin-security/src/delegated-admin-gate.test.tsadminScopeholder is refused (including for a plainmemberrow — no axis approves any of it); plain CRUD buys nothing; principal-less fails closed; every governed mutation verb covered, not justinsert; reads untouchedcontent/docs/permissions/delegated-administration.mdxVerification
pnpm --filter @objectstack/plugin-security test— 32 files, 673 tests passing. Fullturbo run test— 132/132 tasks green.Generated by Claude Code