From 6c5291eead2c471f2c81df0890d70b84b6b1054f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 01:33:42 +0000 Subject: [PATCH] =?UTF-8?q?fix(plugin-security):=20#2937=20=E2=80=94=20Lay?= =?UTF-8?q?er=200=20insert=20post-image=20=E7=A7=9F=E6=88=B7=E6=A3=80?= =?UTF-8?q?=E6=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 多组织模式下用户 insert 可伪造 organization_id 越租户墙 (ADR-0095 D1 读侧 W1 的写侧未竟部分)。Layer 0 的租户墙作用于读 + update/delete 的 pre-image， 但 insert 无 pre-image 且不带 AST，从未被门控。 新增中间件步骤 3.7：对 insert 的 post-image 复用读侧同一套 Layer 0 决策 (computeInsertTenantCheckFilter → computeLayeredRlsFilter 的 layer0)校验 —— 显式提供的 organization_id 必须等于调用者活动组织，否则 fail-closed 拒绝。 缺省值不受影响(由 @objectstack/organizations auto-stamp 补全，故与其注册顺序 无关)；isSystem 上下文中间件入口短路，合法代客设 org_id(import/迁移/seed replay)完全不受影响。matrix gate + 授权一致性 ledger 覆盖。 Closes #2937 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019QRUvVfpvSycAHMMF2xTxs --- .changeset/authz-2937-insert-tenant-guard.md | 29 +++++++ .../dogfood/test/authz-conformance.matrix.ts | 3 + .../src/authz-matrix-gate.test.ts | 60 ++++++++++++++ .../plugin-security/src/security-plugin.ts | 79 +++++++++++++++++++ 4 files changed, 171 insertions(+) create mode 100644 .changeset/authz-2937-insert-tenant-guard.md diff --git a/.changeset/authz-2937-insert-tenant-guard.md b/.changeset/authz-2937-insert-tenant-guard.md new file mode 100644 index 0000000000..a85ea8dccd --- /dev/null +++ b/.changeset/authz-2937-insert-tenant-guard.md @@ -0,0 +1,29 @@ +--- +'@objectstack/plugin-security': minor +--- + +fix(plugin-security): #2937 — Layer 0 insert post-image 租户检查（伪造 organization_id 的用户 insert 现被拒） + +**安全修复 + 行为变更（release-notes callout）。** 多组织模式(`tenancy.mode='multi'` + +`@objectstack/organizations`)下，一个普通用户此前可以 `insert` 一条**伪造 `organization_id`** +(指向别的租户)的业务记录并使其落进受害租户 —— Layer 0 的租户墙 AND-composed 到读 + +update/delete 的 pre-image，但 insert 没有 pre-image、也不带 AST，从未被门控(ADR-0095 D1 +读侧 W1 的写侧未竟部分)。 + +新增 SecurityPlugin 中间件步骤 3.7:对 insert 的 **post-image** 复用读侧同一套 Layer 0 决策 +(`computeInsertTenantCheckFilter` → `computeLayeredRlsFilter` 的 `layer0`)做校验 —— +一个**显式提供**的 `organization_id` 必须等于调用者的活动组织,否则 fail-closed 拒绝。规则与 +读侧完全一致:单组织/隔离未激活、非租户对象(无 `organization_id` 列或 `tenancy.enabled:false`)、 +platform-admin 姿态豁免的对象均不适用;无活动组织的用户提供任意 org_id → 拒(deny sentinel)。 + +**行为 delta(需注意):** 此前能成功的「带跨租户 `organization_id` 的用户级 insert」现被拒绝。 +**缺省(不提供 `organization_id`)的 insert 不受影响** —— 补全仍由 `@objectstack/organizations` +的 auto-stamp 负责(职责分离,因此本检查与 auto-stamp 中间件的注册顺序无关)。系统上下文 +(`isSystem`,含 import 引擎 / 迁移 / 每-org seed replay·clone·orphan-claim 的 `SYSTEM_CTX`) +在中间件入口即短路,合法的「代客设置 org_id」写路径**完全不受影响**。 + +矩阵门:`authz-matrix-gate.test.ts` 新增 `[#2937] Layer 0 insert post-image tenant guard` +八格(伪造异租户→拒、同租户→通过、缺省→放行、无活动组织→拒、platform-admin 私有对象豁免、 +public 业务对象不豁免、tenancy-disabled 对象不适用、单组织模式不检查)。授权一致性 ledger +新增 `multi-tenant-insert-postimage` 行。配套 cloud `@objectstack/organizations` 的 auto-stamp +权威覆盖(纵深防御)。Closes objectstack-ai/framework#2937。 diff --git a/packages/dogfood/test/authz-conformance.matrix.ts b/packages/dogfood/test/authz-conformance.matrix.ts index 2ae661c721..ef6c51fb11 100644 --- a/packages/dogfood/test/authz-conformance.matrix.ts +++ b/packages/dogfood/test/authz-conformance.matrix.ts @@ -42,6 +42,9 @@ 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: '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 45a252f664..9cde1b32c2 100644 --- a/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts +++ b/packages/plugins/plugin-security/src/authz-matrix-gate.test.ts @@ -124,6 +124,25 @@ async function writeFilter(cell: any, roleCtx: any): Promise { return h.findOne.mock.calls[0][1].where.$and.slice(1); } +/** + * [#2937] Effective INSERT outcome for a supplied `organization_id`. Drives the + * REAL insert path through the security CRUD middleware and reports either the + * denial marker or the post-image `organization_id` the row would land with. + * `CRUD_DENY:*` = blocked (CRUD gate or the Layer 0 tenant post-image check). + */ +async function insertOrg(cell: any, roleCtx: any, orgId: unknown): Promise { + const plugin = new SecurityPlugin(); + const h = makeHarness({ ...cell, orgScoping: cell.orgScoping ?? true }); + await plugin.init(h.ctx); await plugin.start(h.ctx); + const opCtx: any = { + object: cell.objectName, operation: 'insert', + data: { name: 'x', ...(orgId === undefined ? {} : { organization_id: orgId }) }, + 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. @@ -270,6 +289,47 @@ describe('authz Layer-0 matrix gate — ADR-0095 D1 (post-extraction)', () => { expect(await readFilter(OBJECTS.task, ROLES.no_org_member)).toEqual({ id: DENY }); }); + // ── #2937: Layer 0 INSERT post-image tenant check ───────────────────────── + // insert has no pre-image, so the tenant wall never reached it: a member could + // `insert` a row with a FORGED organization_id and land it in another tenant. + // The Layer 0 insert post-image check closes this — a SUPPLIED organization_id + // must equal the caller's active org; an absent value is left to the + // enterprise auto-stamp (organizations plugin), and system/platform-admin/ + // single-mode paths are exempt exactly as on the read side. + describe('[#2937] Layer 0 insert post-image tenant guard', () => { + it('member inserting a FORGED cross-tenant organization_id is DENIED', async () => { + expect(await insertOrg(OBJECTS.task, ROLES.member, 'org-2')).toBe('CRUD_DENY:PermissionDeniedError'); + }); + it('member inserting the SAME-tenant organization_id is allowed (row keeps it)', async () => { + expect(await insertOrg(OBJECTS.task, ROLES.member, 'org-1')).toBe('org-1'); + }); + it('member inserting with NO organization_id passes the wall (auto-stamp territory)', async () => { + // plugin-security does not stamp; an absent value is the organizations + // plugin's job. The Layer 0 check must NOT deny it (ordering-independent). + expect(await insertOrg(OBJECTS.task, ROLES.member, undefined)).toBe('<>'); + }); + it('member with NO active org supplying ANY organization_id is fail-closed DENIED', async () => { + expect(await insertOrg(OBJECTS.task, ROLES.no_org_member, 'org-1')).toBe('CRUD_DENY:PermissionDeniedError'); + }); + it('platform admin may insert a cross-org row on a PRIVATE object (posture exemption)', async () => { + // private posture permits the platform-admin superuser bypass → Layer 0 null. + expect(await insertOrg(OBJECTS.private_obj, ROLES.platform_admin, 'org-2')).toBe('org-2'); + }); + it('platform admin stays org-scoped on a PUBLIC business object (no exemption)', async () => { + // public tenant business object → posture gate withholds the bypass → the + // forged cross-org insert is DENIED even for a platform admin. + expect(await insertOrg(OBJECTS.task, ROLES.platform_admin, 'org-2')).toBe('CRUD_DENY:PermissionDeniedError'); + }); + it('platform-global (tenancy-disabled) object: supplied org value is untouched', async () => { + // non-tenant object → Layer 0 contributes nothing → no insert check. + expect(await insertOrg(OBJECTS.platform_global, ROLES.member, 'org-2')).toBe('org-2'); + }); + it('single-org mode: Layer 0 inert, a supplied organization_id is NOT checked', async () => { + const single = { ...OBJECTS.task, orgScoping: false }; + expect(await insertOrg(single, ROLES.member, 'org-2')).toBe('org-2'); + }); + }); + // ── 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 8f0e62991b..974e5ebfbb 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -1024,6 +1024,65 @@ export class SecurityPlugin implements Plugin { } } + // 3.7. [ADR-0095 D1 / #2937] Layer 0 tenant post-image check for INSERT. + // + // 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). + // + // 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. + if ( + opCtx.operation === 'insert' && + opCtx.data && + typeof opCtx.data === 'object' && + !Array.isArray(opCtx.data) && + !!opCtx.context?.userId + ) { + const data = opCtx.data as Record; + const suppliedOrg = data.organization_id; + if (suppliedOrg != null && suppliedOrg !== '') { + const tenantCheck = await this.computeInsertTenantCheckFilter( + permissionSets, + opCtx.object, + 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 + // delegator itself could not reach. + const delTenantCheck = delegatorSets + ? await this.computeInsertTenantCheckFilter(delegatorSets, opCtx.object, delegatorContext) + : null; + const tenantParts = [tenantCheck, delTenantCheck].filter(Boolean) as Record[]; + if ( + tenantParts.length > 0 && + !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 ` + + `(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 ` + + `(organization_id does not match the active organization)`, + { operation: opCtx.operation, object: opCtx.object, positions, permissionSets: explicitPermissionSets }, + ); + } + } + } + // 2.9. Field-level predicate guard (anti filter-oracle, objectui#2251). // FieldMasker (step 4) only strips hidden fields from RESULTS — a // caller could still probe a hidden field's value by filtering / @@ -2068,6 +2127,26 @@ export class SecurityPlugin implements Plugin { return this.rlsCompiler.compileFilter(withCheck, context, 'check'); } + /** + * [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}). + */ + private async computeInsertTenantCheckFilter( + permissionSets: PermissionSet[], + object: string, + context: any, + ): Promise | null> { + const { layer0 } = await this.computeLayeredRlsFilter(permissionSets, object, 'insert', context); + return layer0; + } + /** * Resolve a controlled_by_parent object's master-detail relation (the FK field * key + the master object name), or null. Prefers a required `master_detail`