From 2c969d986db6042a7a4fd6ee454b31cf2cced2af Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 03:06:32 +0000 Subject: [PATCH] =?UTF-8?q?fix(security):=20govern=20`sys=5Fmember`=20writ?= =?UTF-8?q?es=20=E2=80=94=20membership=20is=20not=20a=20delegable=20capabi?= =?UTF-8?q?lity=20(#3697=20follow-up)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_015s4qPrCKBvVwmFeSsFHgNp --- .changeset/govern-sys-member-writes.md | 34 +++++++++ .../permissions/delegated-administration.mdx | 3 +- .../src/delegated-admin-gate.test.ts | 70 +++++++++++++++++++ .../src/delegated-admin-gate.ts | 44 ++++++++++++ 4 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 .changeset/govern-sys-member-writes.md diff --git a/.changeset/govern-sys-member-writes.md b/.changeset/govern-sys-member-writes.md new file mode 100644 index 0000000000..87cef72704 --- /dev/null +++ b/.changeset/govern-sys-member-writes.md @@ -0,0 +1,34 @@ +--- +"@objectstack/plugin-security": patch +--- + +fix(security): govern `sys_member` writes — organization membership is not a delegable 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. + +That matters because 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. 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; 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 (AGENTS.md Prime Directive #10). + +The rule is tenant-admin-only rather than scope-delegable, deliberately: no axis +of `AdminScope` expresses "organization membership" (its vocabulary is BU +subtree, action flags and an 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 to an organization already has a delegable +path: the **invitation**, whose placement is authorized against the issuer's +`adminScope` and whose role is capped at the issuer's own grade. The refusal +message says so. diff --git a/content/docs/permissions/delegated-administration.mdx b/content/docs/permissions/delegated-administration.mdx index ab3470acfe..6e9d1364ae 100644 --- a/content/docs/permissions/delegated-administration.mdx +++ b/content/docs/permissions/delegated-administration.mdx @@ -52,7 +52,7 @@ misconfigured name resolves to an empty subtree and **approves nothing** Writes to the governed tables (`sys_user_position`, `sys_position_permission_set`, `sys_user_permission_set`, -`sys_permission_set`) pass through the `DelegatedAdminGate`: +`sys_permission_set`, `sys_member`) pass through the `DelegatedAdminGate`: | Rule | Effect | |---|---| @@ -64,6 +64,7 @@ Writes to the governed tables (`sys_user_position`, | Single-row writes | Delegates write single rows by id only — a broad filter-write cannot be boundary-checked | | Audit stamp | Every assignment a delegate creates is `granted_by`-stamped automatically | | Tenant-level assets stay tenant-level | The `everyone`/`guest` audience anchors and security-domain publishes are untouchable from any delegated scope | +| Membership is not delegable | `sys_member` writes are tenant-admin-only: no `adminScope` axis expresses org membership, and an admin-grade membership row is auto-elevated to `organization_admin` — writing one would mint a tenant admin. Adding people *is* delegable, through an [invitation](#letting-a-delegate-invite-delegated_admin) | | No scope, no admin | Holders of plain CRUD on the RBAC tables with **no** scope are refused: administration is a scoped capability now, not a side effect of table access | Every rule above is exercised end-to-end by the showcase permission zoo diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts b/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts index e11bffd762..16ca4edb9e 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts @@ -146,6 +146,76 @@ describe('DelegatedAdminGate — tenant admins and outsiders', () => { }); }); +// ── [#3697 follow-up] Organization membership is governed, and NOT delegable ── +// +// A `sys_member` row whose `role` is admin-grade is auto-elevated to +// `organization_admin` (`auto-org-admin-grant.ts`), whose wildcard +// `modifyAllRecords` IS `isTenantAdmin()`. Writing one therefore mints a tenant +// admin — the same escalation the invitation role cap closes on the issuance +// path, one layer down at the table. +// +// Unreachable today: every writer is a better-auth path running under +// `isSystem`, which short-circuits the whole security middleware before this +// gate. These cases exist so the chain cannot silently reopen the day a direct +// write surface is added — the defence has to live at the CALL SITE, not in a +// comment (AGENTS.md Prime Directive #10). +describe('DelegatedAdminGate — organization membership (sys_member)', () => { + const writeMember = (ctx: any, row: any, operation = 'insert') => + h.gate.assert({ object: 'sys_member', operation, data: row, context: ctx }); + + it('a tenant admin still passes — delegation constrains delegates, not HQ', async () => { + await expect( + writeMember(h.ctxOf('tenant_admin'), { user_id: 'u_x', organization_id: 'org_1', role: 'admin' }), + ).resolves.toBeUndefined(); + }); + + it('a DELEGATE holding a real adminScope is refused — membership is not a delegable axis', async () => { + // The escalation this closes: `role: 'admin'` here resolves to + // organization_admin, i.e. authority strictly greater than the delegate's. + await expect( + writeMember(h.ctxOf('delegate'), { user_id: 'u_east_1', organization_id: 'org_1', role: 'admin' }), + ).rejects.toThrow(/tenant-level only/); + }); + + it('a delegate is refused even for a plain member row — no adminScope axis approves any of it', async () => { + await expect( + writeMember(h.ctxOf('delegate'), { user_id: 'u_east_1', organization_id: 'org_1', role: 'member' }), + ).rejects.toThrow(/not a delegable capability/); + }); + + it('the refusal points at the delegable path that DOES exist', async () => { + await expect( + writeMember(h.ctxOf('delegate'), { user_id: 'u_east_1', organization_id: 'org_1', role: 'member' }), + ).rejects.toThrow(/invitation/); + }); + + it('plain CRUD on the table buys nothing', async () => { + await expect( + writeMember(h.ctxOf('crud_only'), { user_id: 'u_east_1', organization_id: 'org_1', role: 'admin' }), + ).rejects.toThrow(/tenant-level only/); + }); + + it('a principal-less non-system write fails closed', async () => { + await expect( + writeMember({}, { user_id: 'u_east_1', organization_id: 'org_1', role: 'admin' }), + ).rejects.toThrow(/authenticated administrator/); + }); + + it('every governed mutation verb is covered, not just insert', async () => { + for (const op of ['update', 'delete', 'transfer', 'restore', 'purge']) { + await expect( + writeMember(h.ctxOf('delegate'), { id: 'mem_1', role: 'admin' }, op), + ).rejects.toThrow(/tenant-level only/); + } + }); + + it('reads are untouched — this governs writes only', async () => { + await expect( + h.gate.assert({ object: 'sys_member', operation: 'find', context: h.ctxOf('delegate') }), + ).resolves.toBeUndefined(); + }); +}); + describe('DelegatedAdminGate — assignments (sys_user_position)', () => { it('delegate assigns an allowlisted position inside the subtree; granted_by is stamped', async () => { const row: any = { user_id: 'u_east_1', position: 'sales_rep', business_unit_id: 'bu_es' }; diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate.ts b/packages/plugins/plugin-security/src/delegated-admin-gate.ts index 49d093844b..cccf135e80 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-gate.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-gate.ts @@ -25,6 +25,13 @@ * to an anchor are rejected for every caller (anchors are implicit — a * stored assignment row is a modeling error). * + * `sys_member` (organization membership) is likewise tenant-level only: no + * `adminScope` axis expresses membership, and an admin-grade membership row is + * auto-elevated to `organization_admin` — writing one mints a tenant admin + * (#3697 follow-up). The delegable way to add people is an INVITATION, whose + * placement this gate authorizes and whose role the framework caps at the + * issuer's own grade. + * * System/boot writes carry `isSystem` and short-circuit the security * middleware before this gate (seeders, publish materializer, better-auth * reconciliation are unaffected). @@ -57,6 +64,19 @@ const GOVERNED_OBJECTS = new Set([ 'sys_position_permission_set', 'sys_user_permission_set', 'sys_permission_set', + // [#3697 follow-up] Organization MEMBERSHIP is an authority surface too: a + // row whose `role` is admin-grade is auto-elevated to `organization_admin` + // by `auto-org-admin-grant.ts`, whose wildcard `modifyAllRecords` IS + // `isTenantAdmin()`. So writing one mints a tenant admin — the same + // escalation the invitation role cap closes on the issuance path. + // + // Unreachable today (every writer is a better-auth path running under + // `isSystem`, short-circuited before this gate; the ADR-0092 D2 identity + // write guard refuses user-context writes to better-auth tables upstream). + // It is here so the chain does not silently reopen the day someone adds a + // direct-write surface — a `case` label is not enforcement, and the call + // site is what matters (AGENTS.md Prime Directive #10). + 'sys_member', ]); const GOVERNED_OPERATIONS = new Set(['insert', 'update', 'delete', 'transfer', 'restore', 'purge']); const ANCHOR_POSITIONS = new Set(['everyone', 'guest']); @@ -186,6 +206,30 @@ export class DelegatedAdminGate { if (isTenantAdmin(sets)) return; // status quo — downstream CRUD/RLS decide + // ── [#3697 follow-up] Organization membership is NOT delegable ──────── + // Every other governed object has an adminScope axis that can approve part + // of it (a subtree, an action flag, an allowlist). `sys_member` has none: + // no axis of `AdminScope` expresses "org membership", and the row's own + // `role` column is an authority dial — admin-grade membership is + // auto-elevated to `organization_admin`, i.e. to `isTenantAdmin()`. A + // delegate who could write one would mint authority strictly greater than + // their own, which is exactly what ADR-0090 D12 exists to prevent. + // + // So it is tenant-admin-only, full stop — reached only by a hypothetical + // future direct-write surface (better-auth's own writes are `isSystem` and + // never arrive here). Adding people to an organization already has a + // delegable path: the INVITATION, whose placement is authorized against + // the issuer's adminScope and whose role is held to the issuer's own grade. + if (opCtx.object === 'sys_member') { + throw new PermissionDeniedError( + `[Security] Access denied: '${opCtx.operation}' on 'sys_member' is tenant-level only — ` + + `organization membership is not a delegable capability (ADR-0090 D12), and an ` + + `admin-grade membership row resolves to tenant administration. Add people through an ` + + `invitation instead: its placement is authorized against your adminScope.`, + { operation: opCtx.operation, object: opCtx.object, userId: ctx.userId }, + ); + } + // ── [ADR-0091 D3] Self-service delegation of duty (职务代理) ────────── // A write that STAMPS `delegated_from` is a delegation: the holder passes // their OWN hat, judged by delegation rules — not by an adminScope. This