From 6220017fecc2e94a54e0474d96be225955f6047a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 04:32:42 +0000 Subject: [PATCH] =?UTF-8?q?fix(plugin-security):=20explain=20posture=20?= =?UTF-8?q?=E8=AF=81=E6=8D=AE=E5=AF=B9=E9=BD=90=20enforcement=20=E6=B4=BE?= =?UTF-8?q?=E7=94=9F=EF=BC=88=E6=B6=88=E9=99=A4=E6=A0=87=E7=AD=BE=E6=BC=82?= =?UTF-8?q?=E7=A7=BB=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security review 低危项。explain-engine 的 derivePosture 之前用松名字匹配作证据 （permissions.includes(ADMIN_FULL_ACCESS) 不校验非作用域 + positions org_owner/org_admin 读 better-auth 角色），比 enforcement（hasPlatformAdminGrant：要求非作用域 admin_full_access user grant；TENANT_ADMIN 用 organization_admin 能力）更松，可能让 explain 面板显示偏高的 posture 标签（作用域 org-admin grant 被误标 PLATFORM/TENANT_ADMIN）。 - 优先直接消费 ctx.posture（principal 经完整 resolveAuthzContext 时逐字返回，结构上不可能漂移）。 - 回退（buildContextForUser 自建 context）：复制 enforcement 的非作用域 grant 判定—— buildContextForUser 现按与 hasPlatformAdminGrant 逐字节一致的规则（admin_full_access 且 organization_id==null 的 active user grant）计算并挂出 hasPlatformAdminGrant；derivePosture 以此 + 投影出的 platform_admin 内建岗位判 PLATFORM_ADMIN，以 organization_admin 能力判 TENANT_ADMIN，不再读 org_owner/org_admin 角色岗位（ADR-0095 D3）。 - 保留 explain 特有的 guest→EXTERNAL 底并置于最前。 只改 posture 标签证据，不改 explain verdict、不改 enforcement。#2947 更广缺口不在范围。关联 #2920。 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019QRUvVfpvSycAHMMF2xTxs --- .../authz-secfollowup-explain-posture.md | 28 +++++++ .../src/explain-engine.test.ts | 83 +++++++++++++++++++ .../plugin-security/src/explain-engine.ts | 80 ++++++++++++++---- 3 files changed, 173 insertions(+), 18 deletions(-) create mode 100644 .changeset/authz-secfollowup-explain-posture.md diff --git a/.changeset/authz-secfollowup-explain-posture.md b/.changeset/authz-secfollowup-explain-posture.md new file mode 100644 index 0000000000..29d78029d5 --- /dev/null +++ b/.changeset/authz-secfollowup-explain-posture.md @@ -0,0 +1,28 @@ +--- +'@objectstack/plugin-security': patch +--- + +fix(plugin-security): explain posture 证据对齐 enforcement 派生（消除标签漂移） + +Security review 低危项。explain-engine 的 `derivePosture(context)` 之前用**松名字匹配**作 +posture 证据——`permissions.includes(ADMIN_FULL_ACCESS)`（不校验非作用域）+ `positions.includes( +'org_owner'/'org_admin')`（读 better-auth 角色），比 enforcement（`resolve-authz-context.ts` 的 +`hasPlatformAdminGrant`：要求**非作用域 admin_full_access user grant**；TENANT_ADMIN 用 +`organization_admin` **能力**而非角色）更松，可能让 explain 面板给运维显示**偏高**的 posture 标签 +（作用域 org-admin grant 被误标 PLATFORM/TENANT_ADMIN）。 + +修法——让 explain 的 posture 走 enforcement 已用的同一份证据： + +- **优先直接消费 `ctx.posture`**：principal 经完整 `resolveAuthzContext` 时已带 enforcement 派生的 + rung，逐字返回 → 结构上不可能漂移。 +- **回退（explain 用 `buildContextForUser` 自建 context，不经完整 resolveAuthzContext）**：复制 + enforcement 的非作用域 grant 判定——`buildContextForUser` 现按与 `hasPlatformAdminGrant` 逐字节 + 一致的规则（`admin_full_access` 且 `organization_id == null` 的 active user grant）计算并挂出 + `hasPlatformAdminGrant`；`derivePosture` 以此 + 投影出的 `platform_admin` 内建岗位判 PLATFORM_ADMIN， + 以 `organization_admin` **能力**判 TENANT_ADMIN，**不再**读 `org_owner`/`org_admin` 角色岗位 + （ADR-0095 D3：角色只是 provisioning 来源，非裁决输入 — explain 侧同样闭合 #2836 dual-track）。 +- 保留 explain 特有的 **guest → EXTERNAL** 底（enforcement floor 是 MEMBER），且置于最前。 + +只改 explain 的 **posture 标签**证据，不改 explain 的 allow/deny verdict（来自复用的 enforcement +filter），不改 enforcement。#2947 跟踪的「posture 未 plumb 进 enforcement context」更广缺口不在本 +任务范围。关联 #2920。 diff --git a/packages/plugins/plugin-security/src/explain-engine.test.ts b/packages/plugins/plugin-security/src/explain-engine.test.ts index 4d61a33c9e..1444812a1f 100644 --- a/packages/plugins/plugin-security/src/explain-engine.test.ts +++ b/packages/plugins/plugin-security/src/explain-engine.test.ts @@ -304,6 +304,60 @@ describe('explainAccess — record-grained (C2 / ADR-0095)', () => { }); }); +describe('posture derivation aligns with enforcement (label-drift elimination)', () => { + // posture is surfaced whenever a recordId is supplied; the record-grained deps + // are irrelevant to the posture value, so the base object-level deps suffice. + const postureOf = async (context: any): Promise => { + const d = await explainAccess(makeDeps({ sets: [ADMIN] }), { + object: 'leave_request', + operation: 'read', + context, + recordId: 'r1', + }); + return d.principal.posture; + }; + + it('reuses ctx.posture verbatim when enforcement already resolved it', async () => { + // A principal resolved through resolveAuthzContext carries ctx.posture — the + // explain panel must echo it, never re-derive a different tier. + expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: [], posture: 'PLATFORM_ADMIN' })).toBe('PLATFORM_ADMIN'); + expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: [], posture: 'TENANT_ADMIN' })).toBe('TENANT_ADMIN'); + // Explicit posture wins over what the loose fallback would have guessed. + expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: ['admin_full_access'], posture: 'MEMBER' })).toBe('MEMBER'); + }); + + it('a merely-SCOPED admin_full_access grant no longer over-labels as PLATFORM_ADMIN', async () => { + // Name present in `permissions` but NOT held as an unscoped grant and NOT the + // projected platform_admin position → MEMBER, matching enforcement (which + // requires hasPlatformAdminGrant = unscoped admin_full_access user grant). + expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: ['admin_full_access'] })).toBe('MEMBER'); + }); + + it('the unscoped-grant flag (hasPlatformAdminGrant) DOES yield PLATFORM_ADMIN', async () => { + expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: ['admin_full_access'], hasPlatformAdminGrant: true })).toBe('PLATFORM_ADMIN'); + }); + + it('the projected platform_admin built-in position still yields PLATFORM_ADMIN', async () => { + expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['platform_admin', 'everyone'], permissions: [] })).toBe('PLATFORM_ADMIN'); + }); + + it('org_owner / org_admin better-auth role positions no longer confer TENANT_ADMIN (ADR-0095 D3)', async () => { + // The role is a provisioning source, not posture evidence. Absent the + // organization_admin CAPABILITY these resolve to MEMBER, as enforcement does. + expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['org_admin', 'everyone'], permissions: [] })).toBe('MEMBER'); + expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['org_owner', 'everyone'], permissions: [] })).toBe('MEMBER'); + }); + + it('the organization_admin capability grant yields TENANT_ADMIN', async () => { + expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: ['organization_admin'] })).toBe('TENANT_ADMIN'); + }); + + it('guest / anonymous → EXTERNAL floor wins even over an attached posture', async () => { + expect(await postureOf({ userId: 'a1', principalKind: 'guest', positions: [], permissions: [], posture: 'PLATFORM_ADMIN' })).toBe('EXTERNAL'); + expect(await postureOf({ userId: null, positions: [], permissions: [] })).toBe('EXTERNAL'); + }); +}); + describe('buildContextForUser', () => { const ql = { async find(object: string, opts: any) { @@ -314,6 +368,34 @@ describe('buildContextForUser', () => { }, }; + it('derives hasPlatformAdminGrant from an UNSCOPED admin_full_access user grant (matches resolveAuthzContext)', async () => { + const qlUnscoped = { + async find(object: string, _opts: any) { + if (object === 'sys_user_permission_set') return [{ user_id: 'u2', permission_set_id: 'psAdmin' /* organization_id absent → unscoped */ }]; + if (object === 'sys_permission_set') return [{ id: 'psAdmin', name: 'admin_full_access' }]; + return []; + }, + }; + const ctx = await buildContextForUser(qlUnscoped, 'u2'); + expect(ctx.hasPlatformAdminGrant).toBe(true); + expect(ctx.permissions).toContain('admin_full_access'); + }); + + it('a SCOPED (org-specific) admin_full_access user grant does NOT set hasPlatformAdminGrant', async () => { + const qlScoped = { + async find(object: string, _opts: any) { + if (object === 'sys_user_permission_set') return [{ user_id: 'u2', permission_set_id: 'psAdmin', organization_id: 'org1' }]; + if (object === 'sys_permission_set') return [{ id: 'psAdmin', name: 'admin_full_access' }]; + return []; + }, + }; + const ctx = await buildContextForUser(qlScoped, 'u2'); + expect(ctx.hasPlatformAdminGrant).toBe(false); + // The name is still resolved into permissions (it grants object CRUD), but it + // no longer confers platform_admin posture — the drift this closes. + expect(ctx.permissions).toContain('admin_full_access'); + }); + it('reconstructs positions + direct grants + the everyone anchor', async () => { const ctx = await buildContextForUser(ql, 'u2'); expect(ctx).toEqual({ @@ -322,6 +404,7 @@ describe('buildContextForUser', () => { permissions: ['payroll_reader'], expiredGrants: [], delegatedPositions: [], + hasPlatformAdminGrant: false, }); }); diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index 593a0d7bc9..0b99c36aff 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -49,30 +49,56 @@ function kernelTierOf(layer: ExplainLayer['layer']): 'layer_0_tenant' | 'layer_1 return layer === 'tenant_isolation' ? 'layer_0_tenant' : 'layer_1_business'; } +const AUTHZ_POSTURES: ReadonlySet = new Set([ + 'PLATFORM_ADMIN', + 'TENANT_ADMIN', + 'MEMBER', + 'EXTERNAL', +]); +function isAuthzPosture(v: unknown): v is AuthzPosture { + return typeof v === 'string' && AUTHZ_POSTURES.has(v); +} + /** - * [C2 / ADR-0095 D2/D3] Resolve the principal's posture rung. The - * PLATFORM_ADMIN / TENANT_ADMIN / MEMBER tiers delegate to the single core - * derivation ({@link deriveAdminPosture}, `@objectstack/core`) so the admin-tier - * logic has ONE source of truth (B4/D3 — derived from held capability grants, - * never a better-auth role, closing the #2836 dual-track by construction). + * [C2 / ADR-0095 D2/D3] Resolve the principal's posture rung, using the SAME + * evidence enforcement uses so the explain panel's tier can never sit HIGHER + * than the runtime's (label-drift elimination — security review low-severity). * - * Explain layers one thing on top the enforcement resolver deliberately does - * NOT: an anonymous / guest principal is represented as EXTERNAL for the - * debugger. The enforcement posture resolver's floor is MEMBER (no external - * principal type exists yet — ADR-0095 D2), so this guest→EXTERNAL mapping lives - * here, in the explanation surface, rather than in `resolveAuthzContext`. + * Order of preference: + * + * 1. **guest / anonymous → EXTERNAL.** Explain layers one thing on top the + * enforcement resolver deliberately does NOT: a guest principal is EXTERNAL + * for the debugger. The enforcement floor is MEMBER (no external principal + * type exists yet — ADR-0095 D2), so this mapping lives here, and it wins + * first. + * 2. **Reuse `ctx.posture` verbatim when present.** A principal resolved through + * the full `resolveAuthzContext` already carries the enforcement-derived + * rung; consuming it directly makes drift structurally impossible. + * 3. **Fallback — re-derive from capability-grant evidence.** The explain API's + * `buildContextForUser` builds a context WITHOUT running the full + * `resolveAuthzContext`, so no `posture` is attached. We then derive from the + * SAME evidence `resolveAuthzContext` uses — NOT the previous loose + * permission-set-NAME match: + * - `PLATFORM_ADMIN` ← the **unscoped `admin_full_access` USER grant** + * (`hasPlatformAdminGrant`, computed by `buildContextForUser` byte-for- + * byte as `resolveAuthzContext` computes it), OR the `platform_admin` + * built-in position (which is itself only ever PROJECTED from that same + * grant — ADR-0068 D2). A merely-SCOPED `admin_full_access` grant (name + * present in `permissions`, not held unscoped) no longer over-labels. + * - `TENANT_ADMIN` ← the `organization_admin` **capability** grant, exactly + * like enforcement (ADR-0095 D3). The better-auth `org_owner`/`org_admin` + * role positions are a provisioning source only and are no longer read + * as posture evidence — closing the same #2836 dual-track explain-side. */ function derivePosture(context: any): AuthzPosture { + if (!context?.userId || context?.principalKind === 'guest') return 'EXTERNAL'; + if (isAuthzPosture(context?.posture)) return context.posture; const positions: string[] = Array.isArray(context?.positions) ? context.positions : []; const permissions: string[] = Array.isArray(context?.permissions) ? context.permissions : []; - if (!context?.userId || context?.principalKind === 'guest') return 'EXTERNAL'; return deriveAdminPosture({ isPlatformAdmin: - positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN) || permissions.includes(ADMIN_FULL_ACCESS), - isTenantAdmin: - positions.includes('org_owner') || - positions.includes('org_admin') || - permissions.includes(ORGANIZATION_ADMIN), + context?.hasPlatformAdminGrant === true || positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN), + isTenantAdmin: permissions.includes(ORGANIZATION_ADMIN), }); } @@ -206,11 +232,25 @@ export async function buildContextForUser(ql: any, userId: string, nowMs: number } } } catch { /* table unavailable → positions stay empty */ } + // [ADR-0095 D3 / ADR-0068 D2] platform_admin posture is DERIVED from an + // UNSCOPED (`organization_id == null`) `admin_full_access` USER grant — the + // single source of truth enforcement (`resolveAuthzContext.hasPlatformAdminGrant`) + // trusts. We compute it here with the IDENTICAL rule so the explain panel's + // posture cannot sit higher than enforcement's: a merely org-SCOPED + // admin_full_access grant must NOT confer platform_admin. + let hasPlatformAdminGrant = false; try { const grants = await ql.find('sys_user_permission_set', { where: { user_id: userId }, limit: 500, context: SYSTEM_CTX }); const grantRows = (Array.isArray(grants) ? grants : []) as any[]; const activeRows = grantRows.filter((g) => isGrantActive(g, nowMs)); const expiredRows = grantRows.filter((g) => !isGrantActive(g, nowMs) && isGrantExpired(g, nowMs)); + // permission-set-ids held via an UNSCOPED active user grant (org == null). + const unscopedActiveIds = new Set( + activeRows + .filter((g) => ((g?.organization_id ?? g?.organizationId) ?? null) === null) + .map((g: any) => String(g?.permission_set_id ?? g?.permissionSetId ?? '')) + .filter(Boolean), + ); const ids = [...activeRows, ...expiredRows].map((g: any) => g?.permission_set_id).filter(Boolean); if (ids.length > 0) { const sets = await ql.find('sys_permission_set', { where: { id: { $in: ids } }, limit: ids.length, context: SYSTEM_CTX }); @@ -219,8 +259,12 @@ export async function buildContextForUser(ql: any, userId: string, nowMs: number if ((s as any)?.id && (s as any)?.name) nameById.set(String((s as any).id), String((s as any).name)); } for (const g of activeRows) { - const n = nameById.get(String(g?.permission_set_id ?? '')); + const id = String(g?.permission_set_id ?? ''); + const n = nameById.get(id); if (n && !permissions.includes(n)) permissions.push(n); + // Same predicate as resolveAuthzContext: name is admin_full_access AND + // the granting row is an unscoped user grant. + if (n === ADMIN_FULL_ACCESS && unscopedActiveIds.has(id)) hasPlatformAdminGrant = true; } for (const g of expiredRows) { const n = nameById.get(String(g?.permission_set_id ?? '')); @@ -230,7 +274,7 @@ export async function buildContextForUser(ql: any, userId: string, nowMs: number } catch { /* ignore */ } // [ADR-0090 D5] Authenticated principals implicitly hold the everyone anchor. if (!positions.includes('everyone')) positions.push('everyone'); - return { userId, positions, permissions, expiredGrants, delegatedPositions }; + return { userId, positions, permissions, expiredGrants, delegatedPositions, hasPlatformAdminGrant }; } /**