From 73a11a0a575d4ec06f29eb9442c90035bbebbdc5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 03:55:30 +0000 Subject: [PATCH] =?UTF-8?q?fix(plugin-security):=20=E5=A0=B5=E8=B7=A8?= =?UTF-8?q?=E7=A7=9F=E6=88=B7=20UPDATE=20=E5=86=99=20+=20org=5Fadmin=20pri?= =?UTF-8?q?vate=20=E5=AF=B9=E8=B1=A1=E8=B6=8A=E5=A2=99=EF=BC=88security?= =?UTF-8?q?=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 修复 security review 确认的两个租户墙授权漏洞（写侧热路径， security-plugin.ts / tenant-layer.ts）。 Finding 1 [BLOCKER] — 经 UPDATE 重指 organization_id 的跨租户写。 Layer 0 insert post-image 检查（step 3.7）只管 insert；对称的 update 路径无人管，成员可对自有 org A 记录发 update 带 {organization_id: org B} 把行移动进任意租户（auto-stamp insert-only、FLS、服务端未强制的 readonly、Layer 0 pre-image 只校验旧 org、显式 RLS check 全部漏过）。 修法（Option B，与 insert 对称、最小面）：把 step 3.7 的 Layer 0 post-image 检查扩到 update，复用同一 computeWriteTenantCheckFilter → computeLayeredRlsFilter 的 layer0；显式提供的 organization_id 必须过 Layer 0（== 活动组织）否则 fail-closed 拒 —— 令非平台用户上下文里 organization_id 事实不可变。缺省不碰 org_id 的 update 不受影响；bulk update 跨租户 change-set 也被堵。 Finding 2 [HIGH] — org_admin 在 private 租户对象上越租户墙。 Layer 0 跨租户豁免门此前用「持有 viewAllRecords/modifyAllRecords」判定， organization_admin 经其 '*' 通配持有超级位 → 在 private 租户业务对象上 触发豁免 → 零过滤 → 读写所有租户。修法：把豁免门收窄为真正的平台管理员 判定（hasPlatformAdminPosture：持有平台专属能力 manage_metadata/ manage_platform_settings/studio.access/manage_users）。超级位继续只驱动 Layer 1 业务 RLS 短路。新豁免是旧门严格子集，只收窄不放宽（fail-safe）。 未用 ctx.posture 作豁免门：posture 未被 plumb 进 enforcement 的 ExecutionContext（rest/runtime 都丢弃），直接消费会静默 no-op。 行为收窄（预期）：org admin 不再越 private/platform-global/better-auth 租户对象的墙；真·平台管理员仍豁免；better-auth carve-out 不受影响； 系统上下文（isSystem，含合法跨组织移动）完全不受影响。 验证：authz-matrix-gate.test.ts 更新 private_obj.org_admin 格 + 新增 [Finding 1]（8 格）/[Finding 2]（5 格）；plugin-security 全绿（405）； tsc --noEmit 通过；build 通过；dogfood authz-conformance ledger 绿 （multi-tenant-write-postimage 覆盖 insert+update、新增 multi-tenant-exemption-posture）。关联 objectstack-ai/framework#2920。 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019QRUvVfpvSycAHMMF2xTxs --- .changeset/authz-tenant-write-wall-fix.md | 17 ++ .../dogfood/test/authz-conformance.matrix.ts | 9 +- .../src/authz-matrix-gate.test.ts | 97 ++++++++++- .../plugin-security/src/security-plugin.ts | 156 +++++++++++++----- .../plugin-security/src/tenant-layer.ts | 29 +++- 5 files changed, 257 insertions(+), 51 deletions(-) create mode 100644 .changeset/authz-tenant-write-wall-fix.md diff --git a/.changeset/authz-tenant-write-wall-fix.md b/.changeset/authz-tenant-write-wall-fix.md new file mode 100644 index 0000000000..0ee5bed420 --- /dev/null +++ b/.changeset/authz-tenant-write-wall-fix.md @@ -0,0 +1,17 @@ +--- +'@objectstack/plugin-security': minor +--- + +fix(plugin-security): 堵跨租户 UPDATE 写 + org_admin 越 private 租户对象墙（security） + +**安全修复 + 行为变更（release-notes callout）。** 修复 security review 确认的两个租户墙授权漏洞，两者同在 `security-plugin.ts` / `tenant-layer.ts` 的写侧热路径。多组织模式（`tenancy.mode='multi'` + `@objectstack/organizations`）下生效。 + +**Finding 1 [BLOCKER] — 经 UPDATE 重指 `organization_id` 的跨租户写。** #2937 的 Layer 0 insert post-image 检查（中间件 step 3.7）只管 insert。对称的 update 路径无人管：成员拥有 org A 的记录 R，对 R 发 by-id 或 bulk `update` 带 `{organization_id: 受害者 org B}`，即可把行**移动进任意租户**——auto-stamp（insert-only）、FLS、服务端未强制的 `readonly`、Layer 0 pre-image（只校验旧 org）、显式 RLS check 全部漏过。修法（Option B，最小面 + 与 insert 对称）：把 step 3.7 的 Layer 0 post-image 检查扩到 update，复用**同一套** Layer 0 决策（`computeWriteTenantCheckFilter` → `computeLayeredRlsFilter` 的 `layer0`）。一个**显式提供**的 `organization_id` 必须过 Layer 0（== 调用者活动组织），否则 fail-closed 拒绝——这令非平台用户上下文里 `organization_id` **事实不可变**（只有活动组织值能过，而 pre-image 已把目标锁在活动组织内，故重指到任何**其他**租户被拒）。缺省（不碰 org_id）的 update 不受影响；bulk update 的跨租户 change-set 也被堵。 + +**Finding 2 [HIGH] — org_admin 在 private 租户对象上越租户墙。** Layer 0 跨租户豁免门此前用「持有 `viewAllRecords`/`modifyAllRecords`」判定。`organization_admin`（自动授给每个 org owner/admin）经其 `'*'` 通配持有这两个超级位，于是在 `access.default:'private'` 的**租户业务对象**上触发豁免 → 零过滤 → 读写所有租户的行。修法：把 Layer 0 豁免门从「超级位」收窄为**真正的平台管理员判定**（`hasPlatformAdminPosture`：持有平台专属能力 `manage_metadata`/`manage_platform_settings`/`studio.access`/`manage_users`，即 `admin_full_access` 携带而 `organization_admin` 刻意不给的那组）。超级位继续只驱动 Layer 1 业务 RLS 短路（TENANT_ADMIN 组织内见全行、无所有权收窄）。因新豁免是旧门的严格子集，只会**收窄**、绝不放宽（fail-safe）。 + +**行为收窄（预期的安全收窄，需注意）：** org admin 不再在 private/platform-global/better-auth 的**租户**对象上越租户墙——它现在被 Layer 0 墙到自己的 org。真·平台管理员（`admin_full_access` + 平台 systemPermissions）仍豁免；better-auth 托管身份表 carve-out 不受影响（无 `organization_id` 列，Layer 0 本就 inert）。系统上下文（`isSystem`，含 import/迁移/seed 的合法跨组织移动）在中间件入口即短路，完全不受影响。 + +**为何不用 `ctx.posture` 作豁免门：** B2 已把 `PLATFORM_ADMIN` posture 落进 `resolve-authz-context.ts` 的 `ctx.posture`，但该字段**未被 plumb 进** enforcement 中间件收到的 ExecutionContext（rest-server 与 runtime dispatcher 都丢弃了它），直接消费会静默 no-op。改用平台专属能力探针，读的是 enforcement 已用的同一套 permission sets，覆盖所有入口，且天然 fail-safe。 + +矩阵门：`authz-matrix-gate.test.ts` 更新 `private_obj.org_admin` 格（read `null` → `{organization_id:'org-1'}`）并新增 `[Finding 1 …]`（8 格：成员重指异租户→拒、同租户→通过、不碰 org_id→放行、无活动组织→拒、org_admin 重指→拒、platform-admin private 对象→放行、public 对象→拒、单组织→不检查）与 `[Finding 2 …]`（5 格：org_admin private 对象读/写墙到本租户、真平台管理员仍豁免、org_admin public 对象回归、better-auth carve-out 不受影响）。授权一致性 ledger 更新 `multi-tenant-write-postimage`（覆盖 insert+update）并新增 `multi-tenant-exemption-posture`。关联 objectstack-ai/framework#2920。 diff --git a/packages/dogfood/test/authz-conformance.matrix.ts b/packages/dogfood/test/authz-conformance.matrix.ts index ef6c51fb11..476937a9e2 100644 --- a/packages/dogfood/test/authz-conformance.matrix.ts +++ b/packages/dogfood/test/authz-conformance.matrix.ts @@ -42,9 +42,12 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [ enforcement: 'plugin-security/security-plugin.ts computeControlledByParentFilter + assertControlledByParentWrite', proof: 'controlled-by-parent.dogfood.test.ts' }, { id: 'multi-tenant', summary: 'organization isolation', state: 'enforced', enforcement: '@objectstack/organizations (enterprise) + Layer 0 tenant wall (plugin-security/tenant-layer.ts, AND-composed ahead of business RLS — ADR-0095 D1)', proof: 'rls-multitenant.dogfood.test.ts' }, - { id: 'multi-tenant-insert-postimage', summary: 'Layer 0 tenant post-image check on INSERT (#2937 — forged organization_id cannot cross the tenant wall)', state: 'enforced', - enforcement: 'plugin-security/security-plugin.ts step 3.7 — computeInsertTenantCheckFilter (reuses computeLayeredRlsFilter\'s Layer 0) matched against the insert post-image (fail-closed); enterprise auto-stamp authoritatively overwrites a user-context organization_id (@objectstack/organizations Middleware A)', - note: 'insert has no pre-image, so the AND-composed Layer 0 wall (rls-read/rls-by-id-write) never reached it; a supplied cross-tenant organization_id is now DENIED (platform-admin posture + single-mode exempt, same rule as the read side). Unit-proven in plugin-security/authz-matrix-gate.test.ts ([#2937] Layer 0 insert post-image tenant guard). Multi-org is enterprise-only so it is not in the open-core dogfood boot; see ADR-0095 D1.' }, + { id: 'multi-tenant-write-postimage', summary: 'Layer 0 tenant post-image check on INSERT + UPDATE (#2937 / Finding 1 — a forged OR re-pointed organization_id cannot cross the tenant wall)', state: 'enforced', + enforcement: 'plugin-security/security-plugin.ts step 3.7 — computeWriteTenantCheckFilter (reuses computeLayeredRlsFilter\'s Layer 0) matched against the write post-image (fail-closed) for BOTH insert and update; enterprise auto-stamp authoritatively overwrites a user-context organization_id (@objectstack/organizations Middleware A)', + note: 'INSERT has no pre-image and UPDATE\'s pre-image (step 2.7) validates only the OLD organization_id, so the AND-composed Layer 0 wall never inspected the NEW value: a member could INSERT a forged cross-tenant organization_id (#2937) or UPDATE a row to RE-POINT it into a victim tenant (Finding 1, BLOCKER). A supplied cross-tenant organization_id is now DENIED on both paths — organization_id is effectively immutable in non-platform user contexts (platform-admin posture on a posture-permitting object + single-mode exempt, same rule as the read side). Unit-proven in plugin-security/authz-matrix-gate.test.ts ([#2937] insert + [Finding 1 / #2937] update post-image tenant guard). Multi-org is enterprise-only so it is not in the open-core dogfood boot; see ADR-0095 D1.' }, + { id: 'multi-tenant-exemption-posture', summary: 'Layer 0 cross-tenant exemption requires the PLATFORM_ADMIN posture (Finding 2 — org_admin does not cross the wall)', state: 'enforced', + enforcement: 'plugin-security/security-plugin.ts hasPlatformAdminPosture (platform-exclusive systemPermissions) gates the tenant-layer.ts Layer 0 exemption; the superuser bit (viewAllRecords/modifyAllRecords) governs only the Layer 1 business-RLS short-circuit', + note: 'An organization_admin holds the superuser bit via its `*` wildcard, so it used to also get the Layer 0 exemption and read/write EVERY tenant\'s rows on private tenant objects. The exemption now requires a platform-exclusive capability (manage_metadata/manage_platform_settings/studio.access/manage_users), which org_admin deliberately lacks — a SECURITY NARROWING: org admin is walled to its own org, a true platform admin still crosses, the better-auth carve-out is untouched. Unit-proven in plugin-security/authz-matrix-gate.test.ts ([Finding 2 / #2937] Layer 0 cross-tenant exemption requires the platform posture).' }, { id: 'anonymous-deny', summary: 'secure-by-default anonymous posture (capability)', state: 'enforced', enforcement: 'rest/rest-server.ts enforceAuth (requireAuth)', proof: 'showcase-anonymous-deny.dogfood.test.ts' }, { id: 'default-profile', summary: 'app-declared default profile (isDefault)', state: 'enforced', diff --git a/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts b/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts index 8433a9a6e8..bf9afe0169 100644 --- a/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts +++ b/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts @@ -143,6 +143,27 @@ async function insertOrg(cell: any, roleCtx: any, orgId: unknown): Promise>'; } +/** + * [Finding 1 / #2937] Effective by-id UPDATE outcome for a supplied + * `organization_id`. Drives the REAL update path; the pre-image re-read (step 2.7) + * is stubbed to return a matching row so the test isolates the Layer 0 post-image + * tenant guard (step 3.7). `CRUD_DENY:*` = blocked (pre-image RLS, or the Layer 0 + * post-image re-point guard); otherwise the `organization_id` the row would keep. + */ +async function updateOrg(cell: any, roleCtx: any, orgId: unknown): Promise { + const plugin = new SecurityPlugin(); + const preImage = { id: 'r1', organization_id: roleCtx.tenantId ?? 'org-1', created_by: roleCtx.userId, name: 'old' }; + const h = makeHarness({ ...cell, orgScoping: cell.orgScoping ?? true, findOneImpl: () => preImage }); + await plugin.init(h.ctx); await plugin.start(h.ctx); + const opCtx: any = { + object: cell.objectName, operation: 'update', + data: { id: 'r1', name: 'x', ...(orgId === undefined ? {} : { organization_id: orgId }) }, + options: { where: { id: 'r1' } }, context: roleCtx, + }; + try { await h.run(opCtx); } catch (e: any) { return `CRUD_DENY:${e?.name ?? 'err'}`; } + return 'organization_id' in opCtx.data ? opCtx.data.organization_id : '<>'; +} + // ── Axes ───────────────────────────────────────────────────────────────────── const OBJECTS = { // Ordinary tenant business object: has organization_id, public posture. @@ -190,9 +211,20 @@ const EXPECTED_MATRIX: Record { }); }); + // ── [Finding 1 / #2937 BLOCKER] Layer 0 UPDATE post-image tenant guard ────── + // The insert post-image guard has a symmetric UPDATE twin: a member owning a + // row in org A could `update` it with `{organization_id: victim org B}` and + // MOVE the row into another tenant (auto-stamp is insert-only, FLS/readonly + // don't protect it, the pre-image check validates only the OLD org). The Layer + // 0 post-image check makes `organization_id` immutable in non-platform user + // contexts — the only value that passes is the caller's active org. + describe('[Finding 1 / #2937] Layer 0 update post-image tenant guard (cross-tenant re-point)', () => { + it('member RE-POINTING organization_id to another tenant is DENIED', async () => { + expect(await updateOrg(OBJECTS.task, ROLES.member, 'org-2')).toBe('CRUD_DENY:PermissionDeniedError'); + }); + it('member updating with the SAME (active-org) organization_id is allowed', async () => { + expect(await updateOrg(OBJECTS.task, ROLES.member, 'org-1')).toBe('org-1'); + }); + it('member update that does NOT touch organization_id is unaffected', async () => { + expect(await updateOrg(OBJECTS.task, ROLES.member, undefined)).toBe('<>'); + }); + it('member with NO active org supplying ANY organization_id is fail-closed DENIED', async () => { + expect(await updateOrg(OBJECTS.task, ROLES.no_org_member, 'org-1')).toBe('CRUD_DENY:PermissionDeniedError'); + }); + it('org_admin RE-POINTING organization_id to another tenant is DENIED (not a platform admin)', async () => { + // org_admin holds the superuser bit but not the platform posture, so it is + // Layer-0-walled to its own org on the public business object too. + expect(await updateOrg(OBJECTS.task, ROLES.org_admin, 'org-2')).toBe('CRUD_DENY:PermissionDeniedError'); + }); + it('platform admin may re-point organization_id on a PRIVATE object (posture exemption)', async () => { + expect(await updateOrg(OBJECTS.private_obj, ROLES.platform_admin, 'org-2')).toBe('org-2'); + }); + it('platform admin stays org-scoped on a PUBLIC business object (re-point DENIED)', async () => { + expect(await updateOrg(OBJECTS.task, ROLES.platform_admin, 'org-2')).toBe('CRUD_DENY:PermissionDeniedError'); + }); + it('single-org mode: Layer 0 inert, a supplied organization_id is NOT checked on update', async () => { + const single = { ...OBJECTS.task, orgScoping: false }; + expect(await updateOrg(single, ROLES.member, 'org-2')).toBe('org-2'); + }); + }); + + // ── [Finding 2 / #2937] org_admin does NOT cross the Layer 0 wall ─────────── + // The Layer 0 cross-tenant exemption is gated on the TRUE PLATFORM_ADMIN posture + // (a platform-exclusive capability), not the raw superuser bit an + // `organization_admin` also holds. So an org admin is walled to its own tenant + // on PRIVATE tenant objects, while a real platform admin still crosses it, and + // the better-auth carve-out is untouched. + describe('[Finding 2 / #2937] Layer 0 cross-tenant exemption requires the platform posture', () => { + it('org_admin on a PRIVATE tenant object is Layer-0-walled to its own org (read)', async () => { + expect(await readFilter(OBJECTS.private_obj, ROLES.org_admin)).toEqual({ organization_id: 'org-1' }); + }); + it('org_admin on a PRIVATE tenant object is Layer-0-walled to its own org (write pre-image)', async () => { + expect(await writeFilter(OBJECTS.private_obj, ROLES.org_admin)).toEqual([{ organization_id: 'org-1' }]); + }); + it('TRUE platform admin still crosses the wall on a PRIVATE object (read null)', async () => { + expect(await readFilter(OBJECTS.private_obj, ROLES.platform_admin)).toBeNull(); + }); + it('org_admin stays walled on a PUBLIC tenant object too (regression)', async () => { + expect(await readFilter(OBJECTS.task, ROLES.org_admin)).toEqual({ organization_id: 'org-1' }); + }); + it('better-auth-managed identity table carve-out is unaffected for org_admin (self-only, no org filter)', async () => { + expect(await readFilter(OBJECTS.better_auth, ROLES.org_admin)).toEqual({ $or: [{ id: 'oadmin' }, { id: 'oadmin' }] }); + }); + }); + // ── Single-org mode: Layer 0 is inert; tenant policy stripped (parity today) ─ // With org-scoping absent, collectRLSPolicies strips the wildcard tenant policy // entirely, so a member's read carries NO tenant where and the write keeps only diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 03adc2fbee..757a75e2bb 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -52,6 +52,36 @@ import { securityPluginManifestHeader, } from './manifest.js'; +/** + * [ADR-0095 D3 / Finding 2 / #2937] Platform-admin-EXCLUSIVE capabilities — the + * platform-scoped `systemPermissions` that `admin_full_access` carries and + * `organization_admin` DELIBERATELY withholds (see `default-permission-sets.ts`: + * org admin is granted only `manage_org_users`/`setup.access`/`setup.write`). + * + * Holding ANY of these is the enforcement stand-in for the `PLATFORM_ADMIN` + * posture rung (ADR-0095 D3): it is what distinguishes a true PLATFORM operator + * from a TENANT org admin. Both hold `viewAllRecords`/`modifyAllRecords` via + * their `'*'` wildcard grant, so the superuser-bypass bit ALONE cannot tell them + * apart — which is exactly why an org admin used to cross the Layer 0 tenant wall + * on private/platform-global/better-auth objects (Finding 2). `setup.access`/ + * `setup.write` are EXCLUDED on purpose: org admins hold them (they are the Setup + * app shell + tenant-settings-write caps, not platform powers). + * + * NOTE: the fully-correct signal is the `PLATFORM_ADMIN` posture already derived + * in `resolve-authz-context.ts` (`ctx.posture`), but that field is NOT plumbed + * into the ExecutionContext the enforcement middleware receives (the REST/runtime + * transports drop it), so consuming it here would silently no-op. This capability + * probe reads the SAME resolved permission sets enforcement already uses, works on + * every entry point, and — being a strict subset of the old superuser-bit gate — + * can only NARROW the exemption, never widen it (fail-safe). + */ +const PLATFORM_ADMIN_ONLY_CAPABILITIES: readonly string[] = [ + 'manage_metadata', + 'manage_platform_settings', + 'studio.access', + 'manage_users', +]; + /** * [ADR-0066 D3/⑤] Object `requiredPermissions` normalized into per-CRUD buckets. * `all` holds capabilities required for EVERY operation (the `string[]` form); @@ -1024,28 +1054,50 @@ export class SecurityPlugin implements Plugin { } } - // 3.7. [ADR-0095 D1 / #2937] Layer 0 tenant post-image check for INSERT. + // 3.7. [ADR-0095 D1 / #2937 / Finding 1] Layer 0 tenant post-image check + // for INSERT and UPDATE. // // The tenant wall (Layer 0) is AND-composed onto reads and onto the - // update/delete PRE-image, but an insert has no pre-image and carries no - // AST, so the wall never reached it: a member could `insert` a row bearing - // a FORGED `organization_id` (pointing at another org) and land it in the - // victim tenant — the enterprise auto-stamp only fills a MISSING value, it - // never overwrites a supplied one. Close it here by validating the insert - // post-image against the SAME Layer 0 filter the read side uses (identical - // rule: isolation active, tenant object, platform-admin posture exemption, - // fail-closed on a missing active org). + // update/delete PRE-image, but two write paths escaped it because they + // carry a `organization_id` VALUE that the pre-image/AST scoping never + // inspects: // - // Scope: only a SUPPLIED (non-empty) `organization_id` is validated — an - // ABSENT value is the organizations-plugin auto-stamp's responsibility - // (separation of concerns; plugin-security no longer stamps org ids), and a - // pure plugin-security deployment has no isolation active (Layer 0 → null), - // so this is ordering-independent w.r.t. the auto-stamp middleware. System / - // boot writes carry `isSystem` and short-circuited the whole middleware - // above, so legitimate on-behalf `organization_id` writes (import engine, - // migrations, plugin SYSTEM_CTX) are unaffected. + // • INSERT has no pre-image and builds no AST, so a member could `insert` + // a row bearing a FORGED `organization_id` (another org) and land it in + // the victim tenant (the enterprise auto-stamp only fills a MISSING + // value, never overwrites a supplied one). + // • UPDATE (Finding 1 / BLOCKER): the pre-image check (step 2.7) validates + // only that the caller may touch the EXISTING row (old org == A); it + // never sees the NEW value. `organization_id` is auto-stamp-insert-only, + // FLS doesn't protect it, server-side `readonly` isn't enforced, and the + // RLS `check` fires only for explicit policies — so a member owning a + // row R in org A could `update` R with `{organization_id: victim org B}` + // and MOVE the row into another tenant, where it becomes visible. This + // is a cross-tenant write by any member. + // + // Both close identically here: a SUPPLIED (non-empty) `organization_id` in + // the write payload must satisfy the SAME Layer 0 filter the read side uses + // (isolation active, tenant object, platform-admin posture exemption, + // fail-closed on a missing active org). For UPDATE this makes + // `organization_id` effectively immutable in non-platform user contexts: the + // only value that passes is the caller's active org (which — since the + // pre-image already scoped the target to that org — equals the row's current + // org), so a re-point to any OTHER tenant is denied. A bulk update carrying a + // cross-tenant `organization_id` change-set is caught too (the check inspects + // the change-set value, not a per-row post-image). + // + // Scope: only a SUPPLIED `organization_id` is validated — an ABSENT value on + // insert is the organizations-plugin auto-stamp's responsibility, and an + // update that doesn't touch `organization_id` carries no value here, so + // ordinary writes are unaffected. A pure plugin-security deployment has no + // isolation active (Layer 0 → null), so this is ordering-independent w.r.t. + // the auto-stamp middleware. System / boot writes carry `isSystem` and + // short-circuited the whole middleware above, so legitimate cross-org moves + // (import engine, migrations, plugin SYSTEM_CTX) are unaffected. A true + // platform admin on a posture-permitting object is exempt via Layer 0 (same + // rule as reads/insert). if ( - opCtx.operation === 'insert' && + (opCtx.operation === 'insert' || opCtx.operation === 'update') && opCtx.data && typeof opCtx.data === 'object' && !Array.isArray(opCtx.data) && @@ -1054,16 +1106,17 @@ export class SecurityPlugin implements Plugin { const data = opCtx.data as Record; const suppliedOrg = data.organization_id; if (suppliedOrg != null && suppliedOrg !== '') { - const tenantCheck = await this.computeInsertTenantCheckFilter( + const tenantCheck = await this.computeWriteTenantCheckFilter( permissionSets, opCtx.object, + opCtx.operation, opCtx.context, ); // [ADR-0090 D10] The post-image must also satisfy the delegator's tenant - // wall — an on-behalf-of insert may not land a row in a tenant the + // wall — an on-behalf-of write may not land a row in a tenant the // delegator itself could not reach. const delTenantCheck = delegatorSets - ? await this.computeInsertTenantCheckFilter(delegatorSets, opCtx.object, delegatorContext) + ? await this.computeWriteTenantCheckFilter(delegatorSets, opCtx.object, opCtx.operation, delegatorContext) : null; const tenantParts = [tenantCheck, delTenantCheck].filter(Boolean) as Record[]; if ( @@ -1071,11 +1124,11 @@ export class SecurityPlugin implements Plugin { !tenantParts.every((f) => matchesFilterCondition(data as any, f as any)) ) { this.logger.warn?.( - `[Security] Layer 0 tenant CHECK FAILED on insert '${opCtx.object}' — write denied ` + + `[Security] Layer 0 tenant CHECK FAILED on ${opCtx.operation} '${opCtx.object}' — write denied ` + `(fail-closed); a supplied organization_id does not match the active organization`, ); throw new PermissionDeniedError( - `[Security] Access denied: the insert would place '${opCtx.object}' in another tenant ` + + `[Security] Access denied: the ${opCtx.operation} would place '${opCtx.object}' in another tenant ` + `(organization_id does not match the active organization)`, { operation: opCtx.operation, object: opCtx.object, positions, permissionSets: explicitPermissionSets }, ); @@ -2024,6 +2077,23 @@ export class SecurityPlugin implements Plugin { * wall (Layer 0) and business RLS (Layer 1) to a record SEPARATELY. Single code * path → the record story cannot drift from the effective filter enforcement uses. */ + /** + * [Finding 2 / ADR-0095 D3] Does the caller resolve to the `PLATFORM_ADMIN` + * posture rung? True iff the resolved permission sets grant any + * platform-EXCLUSIVE capability ({@link PLATFORM_ADMIN_ONLY_CAPABILITIES}) — the + * caps `admin_full_access` carries and `organization_admin` deliberately + * withholds. This is what separates a platform operator from a tenant org admin + * (both hold the `viewAllRecords`/`modifyAllRecords` superuser bit), and it is + * the ONLY signal permitted to cross the Layer 0 tenant wall. + */ + private hasPlatformAdminPosture(permissionSets: PermissionSet[]): boolean { + const held = this.permissionEvaluator.getSystemPermissions(permissionSets); + for (const cap of PLATFORM_ADMIN_ONLY_CAPABILITIES) { + if (held.has(cap)) return true; + } + return false; + } + private async computeLayeredRlsFilter( permissionSets: PermissionSet[], object: string, @@ -2039,11 +2109,20 @@ export class SecurityPlugin implements Plugin { // private / platform-global / better-auth-managed objects. Public tenant // business objects do NOT permit it, so a platform admin stays org-scoped. const posturePermits = meta.isPrivate || meta.tenancyDisabled || meta.isBetterAuthManaged; - const platformAdminBypass = posturePermits + // The superuser bit (`viewAllRecords`/`modifyAllRecords`) governs the BUSINESS + // RLS (Layer 1) short-circuit below — a TENANT_ADMIN legitimately sees every + // row WITHIN its org, no ownership/depth/sharing narrowing (ADR-0095 D2). + const superuserBypass = posturePermits ? (isWrite ? this.permissionEvaluator.hasSuperuserWriteBypass(object, permissionSets, { isPrivate: meta.isPrivate }) : this.permissionEvaluator.hasSuperuserReadBypass(object, permissionSets, { isPrivate: meta.isPrivate })) : false; + // [Finding 2 / #2937] The Layer 0 cross-tenant EXEMPTION is stricter: it + // requires a TRUE PLATFORM_ADMIN (the superuser bit AND a platform-exclusive + // capability), never merely the superuser bit an `organization_admin` also + // holds. This is the ONLY place the platform-admin posture gates crossing the + // tenant wall; a tenant org admin stays org-scoped even on private objects. + const isPlatformAdmin = superuserBypass && this.hasPlatformAdminPosture(permissionSets); // Field set drives BOTH the Layer 1 field-existence net and the Layer 0 // "is this a tenant object?" check. @@ -2055,7 +2134,7 @@ export class SecurityPlugin implements Plugin { // this superuser short-circuit now governs BUSINESS RLS only — it can no // longer skip the tenant wall (that is Layer 0's own exemption, below). let layer1: Record | null = null; - if (!(posturePermits && platformAdminBypass)) { + if (!(posturePermits && superuserBypass)) { const allRlsPolicies = this.collectRLSPolicies(permissionSets, object, operation, (context?.positions ?? []) as string[]); if (allRlsPolicies.length > 0) { // Field-existence safety: a wildcard policy targeting a column the object @@ -2092,7 +2171,7 @@ export class SecurityPlugin implements Plugin { objectHasOrgIdField: objectFields ? objectFields.has('organization_id') : undefined, tenancyDisabled, posturePermitsCrossTenant: posturePermits, - platformAdminBypass, + isPlatformAdmin, }); return { layer0, layer1 }; @@ -2128,22 +2207,25 @@ export class SecurityPlugin implements Plugin { } /** - * [ADR-0095 D1 / #2937] Compute the Layer 0 (tenant) filter that an INSERT - * post-image must satisfy — the write-side twin of the read/update Layer 0 - * wall. Reuses {@link computeLayeredRlsFilter} so the tenant decision is - * DERIVED FROM ONE PLACE and can never drift from the read side: same - * isolation probe, same "is this a tenant object?" field/posture test, same - * platform-admin posture exemption, same fail-closed deny sentinel when the - * context has no active organization. Only `layer0` is returned — business RLS - * (`layer1`) is NOT applied to the insert post-image (that path is governed by - * explicit `check` clauses via {@link computeWriteCheckFilter}). + * [ADR-0095 D1 / #2937] Compute the Layer 0 (tenant) filter that a WRITE + * post-image must satisfy — the write-side twin of the read Layer 0 wall, + * applied to both INSERT (Finding: forged `organization_id`) and UPDATE + * (Finding 1: `organization_id` RE-POINTED to another tenant). Reuses + * {@link computeLayeredRlsFilter} so the tenant decision is DERIVED FROM ONE + * PLACE and can never drift from the read side: same isolation probe, same + * "is this a tenant object?" field/posture test, same platform-admin posture + * exemption, same fail-closed deny sentinel when the context has no active + * organization. Only `layer0` is returned — business RLS (`layer1`) is NOT + * applied to the write post-image (that path is governed by explicit `check` + * clauses via {@link computeWriteCheckFilter}). */ - private async computeInsertTenantCheckFilter( + private async computeWriteTenantCheckFilter( permissionSets: PermissionSet[], object: string, + operation: string, context: any, ): Promise | null> { - const { layer0 } = await this.computeLayeredRlsFilter(permissionSets, object, 'insert', context); + const { layer0 } = await this.computeLayeredRlsFilter(permissionSets, object, operation, context); return layer0; } diff --git a/packages/plugins/plugin-security/src/tenant-layer.ts b/packages/plugins/plugin-security/src/tenant-layer.ts index d983da4a3c..5a07a4286c 100644 --- a/packages/plugins/plugin-security/src/tenant-layer.ts +++ b/packages/plugins/plugin-security/src/tenant-layer.ts @@ -53,12 +53,22 @@ export interface TenantLayer0Input { */ posturePermitsCrossTenant: boolean; /** - * Platform-admin evidence for THIS operation side — the same capability the - * superuser bypass already trusts (`viewAllRecords` on reads / - * `modifyAllRecords` on writes). This is the D1 stand-in for the full posture - * enum (B2); it is NOT the better-auth role. + * TRUE-platform-admin evidence (ADR-0095 D3 `PLATFORM_ADMIN` rung) for THIS + * operation side. This is the ONLY gate that lets a caller cross the tenant + * wall, so it MUST distinguish a PLATFORM operator from a TENANT `org_admin`. + * + * It is NOT merely the superuser bit (`viewAllRecords`/`modifyAllRecords`): + * `organization_admin` holds that bit via its `'*'` wildcard grant + * (`default-permission-sets.ts`), so gating the exemption on the superuser bit + * alone let an org admin cross the wall on a `private` (or platform-global / + * better-auth) TENANT object — the D1 "B2 posture stand-in" gap this field's + * former semantics carried. The caller now requires the superuser bit AND a + * platform-EXCLUSIVE capability (`manage_metadata`/`manage_platform_settings`/ + * `studio.access`/`manage_users` — the caps `admin_full_access` carries and + * `organization_admin` deliberately withholds), i.e. the `PLATFORM_ADMIN` + * posture. It is NEVER the better-auth role. */ - platformAdminBypass: boolean; + isPlatformAdmin: boolean; } /** @@ -83,10 +93,11 @@ export function computeTenantLayer0Filter( if (input.tenancyDisabled) return null; if (input.objectHasOrgIdField === false) return null; - // Exemption is a Layer 0 rule (W2 fix): only a PLATFORM_ADMIN-posture caller on - // an object whose posture permits it crosses the wall. Layer 1's bypass no - // longer implies Layer 0's. - if (input.platformAdminBypass && input.posturePermitsCrossTenant) return null; + // Exemption is a Layer 0 rule (W2 fix): only a TRUE PLATFORM_ADMIN caller on an + // object whose posture permits it crosses the wall — NOT a tenant `org_admin` + // that merely holds the superuser bit (Finding 2 / #2937). Layer 1's superuser + // bypass no longer implies Layer 0's. + if (input.isPlatformAdmin && input.posturePermitsCrossTenant) return null; // Enforce the wall. Missing active org in multi mode → fail closed. if (!input.organizationId) return { ...RLS_DENY_FILTER };