Skip to content

Commit 29a4c90

Browse files
os-zhuangclaude
andauthored
fix(plugin-security): explain posture 证据对齐 enforcement 派生(消除标签漂移) (#2949)
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。 Claude-Session: https://claude.ai/code/session_019QRUvVfpvSycAHMMF2xTxs Co-authored-by: Claude <noreply@anthropic.com>
1 parent ef70521 commit 29a4c90

3 files changed

Lines changed: 173 additions & 18 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
fix(plugin-security): explain posture 证据对齐 enforcement 派生(消除标签漂移)
6+
7+
Security review 低危项。explain-engine 的 `derivePosture(context)` 之前用**松名字匹配**
8+
posture 证据——`permissions.includes(ADMIN_FULL_ACCESS)`(不校验非作用域)+ `positions.includes(
9+
'org_owner'/'org_admin')`(读 better-auth 角色),比 enforcement(`resolve-authz-context.ts` 的
10+
`hasPlatformAdminGrant`:要求**非作用域 admin_full_access user grant**;TENANT_ADMIN 用
11+
`organization_admin` **能力**而非角色)更松,可能让 explain 面板给运维显示**偏高**的 posture 标签
12+
(作用域 org-admin grant 被误标 PLATFORM/TENANT_ADMIN)。
13+
14+
修法——让 explain 的 posture 走 enforcement 已用的同一份证据:
15+
16+
- **优先直接消费 `ctx.posture`**:principal 经完整 `resolveAuthzContext` 时已带 enforcement 派生的
17+
rung,逐字返回 → 结构上不可能漂移。
18+
- **回退(explain 用 `buildContextForUser` 自建 context,不经完整 resolveAuthzContext)**:复制
19+
enforcement 的非作用域 grant 判定——`buildContextForUser` 现按与 `hasPlatformAdminGrant` 逐字节
20+
一致的规则(`admin_full_access``organization_id == null` 的 active user grant)计算并挂出
21+
`hasPlatformAdminGrant``derivePosture` 以此 + 投影出的 `platform_admin` 内建岗位判 PLATFORM_ADMIN,
22+
`organization_admin` **能力**判 TENANT_ADMIN,**不再**`org_owner`/`org_admin` 角色岗位
23+
(ADR-0095 D3:角色只是 provisioning 来源,非裁决输入 — explain 侧同样闭合 #2836 dual-track)。
24+
- 保留 explain 特有的 **guest → EXTERNAL** 底(enforcement floor 是 MEMBER),且置于最前。
25+
26+
只改 explain 的 **posture 标签**证据,不改 explain 的 allow/deny verdict(来自复用的 enforcement
27+
filter),不改 enforcement。#2947 跟踪的「posture 未 plumb 进 enforcement context」更广缺口不在本
28+
任务范围。关联 #2920

packages/plugins/plugin-security/src/explain-engine.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,60 @@ describe('explainAccess — record-grained (C2 / ADR-0095)', () => {
304304
});
305305
});
306306

307+
describe('posture derivation aligns with enforcement (label-drift elimination)', () => {
308+
// posture is surfaced whenever a recordId is supplied; the record-grained deps
309+
// are irrelevant to the posture value, so the base object-level deps suffice.
310+
const postureOf = async (context: any): Promise<string | undefined> => {
311+
const d = await explainAccess(makeDeps({ sets: [ADMIN] }), {
312+
object: 'leave_request',
313+
operation: 'read',
314+
context,
315+
recordId: 'r1',
316+
});
317+
return d.principal.posture;
318+
};
319+
320+
it('reuses ctx.posture verbatim when enforcement already resolved it', async () => {
321+
// A principal resolved through resolveAuthzContext carries ctx.posture — the
322+
// explain panel must echo it, never re-derive a different tier.
323+
expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: [], posture: 'PLATFORM_ADMIN' })).toBe('PLATFORM_ADMIN');
324+
expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: [], posture: 'TENANT_ADMIN' })).toBe('TENANT_ADMIN');
325+
// Explicit posture wins over what the loose fallback would have guessed.
326+
expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: ['admin_full_access'], posture: 'MEMBER' })).toBe('MEMBER');
327+
});
328+
329+
it('a merely-SCOPED admin_full_access grant no longer over-labels as PLATFORM_ADMIN', async () => {
330+
// Name present in `permissions` but NOT held as an unscoped grant and NOT the
331+
// projected platform_admin position → MEMBER, matching enforcement (which
332+
// requires hasPlatformAdminGrant = unscoped admin_full_access user grant).
333+
expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: ['admin_full_access'] })).toBe('MEMBER');
334+
});
335+
336+
it('the unscoped-grant flag (hasPlatformAdminGrant) DOES yield PLATFORM_ADMIN', async () => {
337+
expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: ['admin_full_access'], hasPlatformAdminGrant: true })).toBe('PLATFORM_ADMIN');
338+
});
339+
340+
it('the projected platform_admin built-in position still yields PLATFORM_ADMIN', async () => {
341+
expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['platform_admin', 'everyone'], permissions: [] })).toBe('PLATFORM_ADMIN');
342+
});
343+
344+
it('org_owner / org_admin better-auth role positions no longer confer TENANT_ADMIN (ADR-0095 D3)', async () => {
345+
// The role is a provisioning source, not posture evidence. Absent the
346+
// organization_admin CAPABILITY these resolve to MEMBER, as enforcement does.
347+
expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['org_admin', 'everyone'], permissions: [] })).toBe('MEMBER');
348+
expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['org_owner', 'everyone'], permissions: [] })).toBe('MEMBER');
349+
});
350+
351+
it('the organization_admin capability grant yields TENANT_ADMIN', async () => {
352+
expect(await postureOf({ userId: 'a1', tenantId: 'org1', positions: ['everyone'], permissions: ['organization_admin'] })).toBe('TENANT_ADMIN');
353+
});
354+
355+
it('guest / anonymous → EXTERNAL floor wins even over an attached posture', async () => {
356+
expect(await postureOf({ userId: 'a1', principalKind: 'guest', positions: [], permissions: [], posture: 'PLATFORM_ADMIN' })).toBe('EXTERNAL');
357+
expect(await postureOf({ userId: null, positions: [], permissions: [] })).toBe('EXTERNAL');
358+
});
359+
});
360+
307361
describe('buildContextForUser', () => {
308362
const ql = {
309363
async find(object: string, opts: any) {
@@ -314,6 +368,34 @@ describe('buildContextForUser', () => {
314368
},
315369
};
316370

371+
it('derives hasPlatformAdminGrant from an UNSCOPED admin_full_access user grant (matches resolveAuthzContext)', async () => {
372+
const qlUnscoped = {
373+
async find(object: string, _opts: any) {
374+
if (object === 'sys_user_permission_set') return [{ user_id: 'u2', permission_set_id: 'psAdmin' /* organization_id absent → unscoped */ }];
375+
if (object === 'sys_permission_set') return [{ id: 'psAdmin', name: 'admin_full_access' }];
376+
return [];
377+
},
378+
};
379+
const ctx = await buildContextForUser(qlUnscoped, 'u2');
380+
expect(ctx.hasPlatformAdminGrant).toBe(true);
381+
expect(ctx.permissions).toContain('admin_full_access');
382+
});
383+
384+
it('a SCOPED (org-specific) admin_full_access user grant does NOT set hasPlatformAdminGrant', async () => {
385+
const qlScoped = {
386+
async find(object: string, _opts: any) {
387+
if (object === 'sys_user_permission_set') return [{ user_id: 'u2', permission_set_id: 'psAdmin', organization_id: 'org1' }];
388+
if (object === 'sys_permission_set') return [{ id: 'psAdmin', name: 'admin_full_access' }];
389+
return [];
390+
},
391+
};
392+
const ctx = await buildContextForUser(qlScoped, 'u2');
393+
expect(ctx.hasPlatformAdminGrant).toBe(false);
394+
// The name is still resolved into permissions (it grants object CRUD), but it
395+
// no longer confers platform_admin posture — the drift this closes.
396+
expect(ctx.permissions).toContain('admin_full_access');
397+
});
398+
317399
it('reconstructs positions + direct grants + the everyone anchor', async () => {
318400
const ctx = await buildContextForUser(ql, 'u2');
319401
expect(ctx).toEqual({
@@ -322,6 +404,7 @@ describe('buildContextForUser', () => {
322404
permissions: ['payroll_reader'],
323405
expiredGrants: [],
324406
delegatedPositions: [],
407+
hasPlatformAdminGrant: false,
325408
});
326409
});
327410

packages/plugins/plugin-security/src/explain-engine.ts

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -49,30 +49,56 @@ function kernelTierOf(layer: ExplainLayer['layer']): 'layer_0_tenant' | 'layer_1
4949
return layer === 'tenant_isolation' ? 'layer_0_tenant' : 'layer_1_business';
5050
}
5151

52+
const AUTHZ_POSTURES: ReadonlySet<string> = new Set<AuthzPosture>([
53+
'PLATFORM_ADMIN',
54+
'TENANT_ADMIN',
55+
'MEMBER',
56+
'EXTERNAL',
57+
]);
58+
function isAuthzPosture(v: unknown): v is AuthzPosture {
59+
return typeof v === 'string' && AUTHZ_POSTURES.has(v);
60+
}
61+
5262
/**
53-
* [C2 / ADR-0095 D2/D3] Resolve the principal's posture rung. The
54-
* PLATFORM_ADMIN / TENANT_ADMIN / MEMBER tiers delegate to the single core
55-
* derivation ({@link deriveAdminPosture}, `@objectstack/core`) so the admin-tier
56-
* logic has ONE source of truth (B4/D3 — derived from held capability grants,
57-
* never a better-auth role, closing the #2836 dual-track by construction).
63+
* [C2 / ADR-0095 D2/D3] Resolve the principal's posture rung, using the SAME
64+
* evidence enforcement uses so the explain panel's tier can never sit HIGHER
65+
* than the runtime's (label-drift elimination — security review low-severity).
5866
*
59-
* Explain layers one thing on top the enforcement resolver deliberately does
60-
* NOT: an anonymous / guest principal is represented as EXTERNAL for the
61-
* debugger. The enforcement posture resolver's floor is MEMBER (no external
62-
* principal type exists yet — ADR-0095 D2), so this guest→EXTERNAL mapping lives
63-
* here, in the explanation surface, rather than in `resolveAuthzContext`.
67+
* Order of preference:
68+
*
69+
* 1. **guest / anonymous → EXTERNAL.** Explain layers one thing on top the
70+
* enforcement resolver deliberately does NOT: a guest principal is EXTERNAL
71+
* for the debugger. The enforcement floor is MEMBER (no external principal
72+
* type exists yet — ADR-0095 D2), so this mapping lives here, and it wins
73+
* first.
74+
* 2. **Reuse `ctx.posture` verbatim when present.** A principal resolved through
75+
* the full `resolveAuthzContext` already carries the enforcement-derived
76+
* rung; consuming it directly makes drift structurally impossible.
77+
* 3. **Fallback — re-derive from capability-grant evidence.** The explain API's
78+
* `buildContextForUser` builds a context WITHOUT running the full
79+
* `resolveAuthzContext`, so no `posture` is attached. We then derive from the
80+
* SAME evidence `resolveAuthzContext` uses — NOT the previous loose
81+
* permission-set-NAME match:
82+
* - `PLATFORM_ADMIN` ← the **unscoped `admin_full_access` USER grant**
83+
* (`hasPlatformAdminGrant`, computed by `buildContextForUser` byte-for-
84+
* byte as `resolveAuthzContext` computes it), OR the `platform_admin`
85+
* built-in position (which is itself only ever PROJECTED from that same
86+
* grant — ADR-0068 D2). A merely-SCOPED `admin_full_access` grant (name
87+
* present in `permissions`, not held unscoped) no longer over-labels.
88+
* - `TENANT_ADMIN` ← the `organization_admin` **capability** grant, exactly
89+
* like enforcement (ADR-0095 D3). The better-auth `org_owner`/`org_admin`
90+
* role positions are a provisioning source only and are no longer read
91+
* as posture evidence — closing the same #2836 dual-track explain-side.
6492
*/
6593
function derivePosture(context: any): AuthzPosture {
94+
if (!context?.userId || context?.principalKind === 'guest') return 'EXTERNAL';
95+
if (isAuthzPosture(context?.posture)) return context.posture;
6696
const positions: string[] = Array.isArray(context?.positions) ? context.positions : [];
6797
const permissions: string[] = Array.isArray(context?.permissions) ? context.permissions : [];
68-
if (!context?.userId || context?.principalKind === 'guest') return 'EXTERNAL';
6998
return deriveAdminPosture({
7099
isPlatformAdmin:
71-
positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN) || permissions.includes(ADMIN_FULL_ACCESS),
72-
isTenantAdmin:
73-
positions.includes('org_owner') ||
74-
positions.includes('org_admin') ||
75-
permissions.includes(ORGANIZATION_ADMIN),
100+
context?.hasPlatformAdminGrant === true || positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN),
101+
isTenantAdmin: permissions.includes(ORGANIZATION_ADMIN),
76102
});
77103
}
78104

@@ -206,11 +232,25 @@ export async function buildContextForUser(ql: any, userId: string, nowMs: number
206232
}
207233
}
208234
} catch { /* table unavailable → positions stay empty */ }
235+
// [ADR-0095 D3 / ADR-0068 D2] platform_admin posture is DERIVED from an
236+
// UNSCOPED (`organization_id == null`) `admin_full_access` USER grant — the
237+
// single source of truth enforcement (`resolveAuthzContext.hasPlatformAdminGrant`)
238+
// trusts. We compute it here with the IDENTICAL rule so the explain panel's
239+
// posture cannot sit higher than enforcement's: a merely org-SCOPED
240+
// admin_full_access grant must NOT confer platform_admin.
241+
let hasPlatformAdminGrant = false;
209242
try {
210243
const grants = await ql.find('sys_user_permission_set', { where: { user_id: userId }, limit: 500, context: SYSTEM_CTX });
211244
const grantRows = (Array.isArray(grants) ? grants : []) as any[];
212245
const activeRows = grantRows.filter((g) => isGrantActive(g, nowMs));
213246
const expiredRows = grantRows.filter((g) => !isGrantActive(g, nowMs) && isGrantExpired(g, nowMs));
247+
// permission-set-ids held via an UNSCOPED active user grant (org == null).
248+
const unscopedActiveIds = new Set<string>(
249+
activeRows
250+
.filter((g) => ((g?.organization_id ?? g?.organizationId) ?? null) === null)
251+
.map((g: any) => String(g?.permission_set_id ?? g?.permissionSetId ?? ''))
252+
.filter(Boolean),
253+
);
214254
const ids = [...activeRows, ...expiredRows].map((g: any) => g?.permission_set_id).filter(Boolean);
215255
if (ids.length > 0) {
216256
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
219259
if ((s as any)?.id && (s as any)?.name) nameById.set(String((s as any).id), String((s as any).name));
220260
}
221261
for (const g of activeRows) {
222-
const n = nameById.get(String(g?.permission_set_id ?? ''));
262+
const id = String(g?.permission_set_id ?? '');
263+
const n = nameById.get(id);
223264
if (n && !permissions.includes(n)) permissions.push(n);
265+
// Same predicate as resolveAuthzContext: name is admin_full_access AND
266+
// the granting row is an unscoped user grant.
267+
if (n === ADMIN_FULL_ACCESS && unscopedActiveIds.has(id)) hasPlatformAdminGrant = true;
224268
}
225269
for (const g of expiredRows) {
226270
const n = nameById.get(String(g?.permission_set_id ?? ''));
@@ -230,7 +274,7 @@ export async function buildContextForUser(ql: any, userId: string, nowMs: number
230274
} catch { /* ignore */ }
231275
// [ADR-0090 D5] Authenticated principals implicitly hold the everyone anchor.
232276
if (!positions.includes('everyone')) positions.push('everyone');
233-
return { userId, positions, permissions, expiredGrants, delegatedPositions };
277+
return { userId, positions, permissions, expiredGrants, delegatedPositions, hasPlatformAdminGrant };
234278
}
235279

236280
/**

0 commit comments

Comments
 (0)