Skip to content

Commit 7180ed5

Browse files
authored
fix(security): 元数据不可解析时安全姿态 fail-closed (#3545) (#3748)
#3545 上一轮接受曝露 gate 的 fail-open,理由是「中间件是真正的授权边界且无条件执行」。验证该前提后发现不成立:中间件的两个输入(access.default、requiredPermissions)读自同一份对象元数据,读不到时向宽松端默认 —— private 对象被当成 public(被普通 '*' 通配覆盖),能力 AND-gate 被整段跳过。 getObjectSecurityMeta 现标记 unresolved,三个把姿态转成访问决策的消费方 fail-closed:中间件拒绝(+ error 日志)、canExport 拒绝、getReadableFields 不暴露任何列。computeLayeredRlsFilter 有意保持消费默认值 —— 那里宽松默认是扣留跨租户豁免,已是收紧方向。 影响范围限定在「已认证且已解析出授权的主体请求声明缺失的对象」:isSystem 与匿名上下文更早短路,冷启动窗口不受影响,曝露 gate 侧的分级决策维持不变。explain 在既有 object_crud 层报出真实原因,避免解释面与执行面漂移。 验证:plugin-security 645 passed、runtime/rest/objectql 全绿、dogfood 369 passed。
1 parent 08b5a3d commit 7180ed5

5 files changed

Lines changed: 365 additions & 16 deletions

File tree

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/plugin-security": patch
3+
"@objectstack/runtime": patch
4+
---
5+
6+
fix(security): fail closed when an object's security posture can't be resolved
7+
(#3545)
8+
9+
#3545 accepted the API-exposure gate's fail-open on unresolvable metadata on one
10+
load-bearing premise: that gate is a SURFACE-AREA control, while the real
11+
authorization boundary — auth + the ObjectQL security middleware (CRUD/FLS/RLS)
12+
— enforces unconditionally on the data call whatever the gate answers.
13+
14+
Verifying that premise rather than assuming it shows it did not hold. The
15+
middleware does run unconditionally, but two of its INPUTS were read from the
16+
same object metadata and defaulted permissively when it could not be resolved,
17+
so the very trigger the issue is about reached one layer PAST the gate, into the
18+
boundary itself: an unresolved `access.default` read as PUBLIC (so a plain `'*'`
19+
wildcard covered an object ADR-0066 D2 excludes from it) and an unresolved
20+
`requiredPermissions` read as NO CONTRACT (so the D3 capability AND-gate was
21+
skipped entirely).
22+
23+
`getObjectSecurityMeta` now flags `unresolved`, and the three consumers that turn
24+
posture into an access decision fail closed on it: the middleware denies (with an
25+
error log, so a persistent metadata outage is observable rather than a silent
26+
blanket-allow), `canExport` denies, and `getReadableFields` exposes no columns —
27+
the same stance already taken for a permission-resolution failure and a dangling
28+
delegator. `computeLayeredRlsFilter` keeps consuming the defaults deliberately:
29+
there the permissive value WITHHOLDS the cross-tenant exemption, so it is already
30+
the closed direction.
31+
32+
Blast radius is bounded to the risky case. System/boot writes (`isSystem`) and
33+
principal-less/anonymous contexts short-circuit earlier in the middleware, so
34+
reaching the new check means an authenticated principal with resolved grants
35+
asking for an object whose declaration is missing; the cold-start window is
36+
served by those short-circuits, not by the permissive default. The exposure
37+
gate's own tiered decision (transient unavailability → fail open) is therefore
38+
unchanged — it now rests on a boundary that actually holds.
39+
40+
The explain engine reports the denial on its existing `object_crud` layer naming
41+
the real cause, so the "why am I denied?" surface cannot drift from enforcement.

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

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ export interface ExplainEngineDeps {
146146
isPrivate: boolean;
147147
requiredPermissions: any;
148148
fieldRequiredPermissions: Record<string, string[]>;
149+
/** [#3545] Posture could not be read — the middleware denies (fail-closed). */
150+
unresolved?: boolean;
149151
}>;
150152
/** The middleware's requiredPermissions AND-gate resolution for an operation. */
151153
requiredCaps: (meta: any, engineOperation: string) => string[];
@@ -840,22 +842,35 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput
840842
const delegatorCrud = delegatorSets
841843
? deps.evaluator.checkObjectPermission(engineOp, object, delegatorSets, { isPrivate: secMeta.isPrivate })
842844
: true;
843-
const crudAllowed = agentCrud && delegatorCrud && !delegatorMissing;
844-
const granting = sets
845-
.filter((s) => deps.evaluator.checkObjectPermission(engineOp, object, [s], { isPrivate: secMeta.isPrivate }))
846-
.map((s: any) => String(s.name ?? '?'));
845+
// [#3545] An UNRESOLVED posture is a denial in the middleware, so it must read
846+
// as one here too — explain and enforcement disagreeing on a security surface
847+
// is the same `declared ≠ enforced` gap this engine exists to expose. Reported
848+
// on the existing `object_crud` layer (no new layer kind): the posture is what
849+
// that layer's grant is computed FROM, and reporting the real cause beats a
850+
// misleading "no set grants it" when the sets were never the problem.
851+
const postureUnresolved = secMeta.unresolved === true;
852+
const crudAllowed = agentCrud && delegatorCrud && !delegatorMissing && !postureUnresolved;
853+
const granting = postureUnresolved
854+
? []
855+
: sets
856+
.filter((s) => deps.evaluator.checkObjectPermission(engineOp, object, [s], { isPrivate: secMeta.isPrivate }))
857+
.map((s: any) => String(s.name ?? '?'));
847858
layers.push({
848859
layer: 'object_crud',
849860
verdict: crudAllowed ? 'grants' : 'denies',
850861
detail: crudAllowed
851862
? `${operation} on '${object}' is granted by [${granting.join(', ')}]` +
852863
(delegatorSets ? ' AND by the delegator (D10 intersection).' : '.')
853-
: delegatorMissing
854-
? `Delegator no longer exists — D10 fails closed (access denied).`
855-
: agentCrud && !delegatorCrud
856-
? `The agent grants ${operation} on '${object}' but the DELEGATOR does not — D10 intersection denies (an agent may not exceed the user it acts for).`
857-
: `No resolved permission set grants ${operation} on '${object}'` +
858-
(secMeta.isPrivate ? " (object is 'private' posture — non-superuser '*' wildcards are excluded, ADR-0066 D2)." : '.'),
864+
: postureUnresolved
865+
? `The security posture of '${object}' could not be resolved (neither the live schema nor the ` +
866+
`metadata service returned it) — its 'private' flag and required-capability contract are ` +
867+
`unknown, so access fails CLOSED rather than defaulting to public/uncontracted (#3545).`
868+
: delegatorMissing
869+
? `Delegator no longer exists — D10 fails closed (access denied).`
870+
: agentCrud && !delegatorCrud
871+
? `The agent grants ${operation} on '${object}' but the DELEGATOR does not — D10 intersection denies (an agent may not exceed the user it acts for).`
872+
: `No resolved permission set grants ${operation} on '${object}'` +
873+
(secMeta.isPrivate ? " (object is 'private' posture — non-superuser '*' wildcards are excluded, ADR-0066 D2)." : '.'),
859874
contributors: granting.map((n) => ({ kind: 'permission_set' as const, name: n, via: viaOf(n) })),
860875
});
861876

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#3545] Object metadata unresolvable → the SECURITY POSTURE must fail CLOSED.
5+
*
6+
* #3545 assessed the fail-open on unresolvable metadata in the API-exposure gate
7+
* (`checkApiExposure` / REST `enforceApiAccess`) and accepted it, on the premise
8+
* that the gate is a SURFACE-AREA control while the real authorization boundary —
9+
* auth + the ObjectQL security middleware (CRUD / FLS / RLS) — enforces
10+
* unconditionally on the data call regardless of the gate's answer.
11+
*
12+
* The middleware does run unconditionally. But two of its INPUTS were read from
13+
* the same object metadata and defaulted PERMISSIVELY when it could not be
14+
* resolved, so the same trigger reached one layer deeper than the assessment
15+
* looked:
16+
*
17+
* • `access.default: 'private'` → `isPrivate` defaulted to `false`. A private
18+
* object is deliberately NOT covered by a plain (non-superuser) `'*'`
19+
* wildcard grant (ADR-0066 D2, `resolveObjectPermission`); read as public it
20+
* IS covered — a grant the author never wrote.
21+
* • `requiredPermissions` → defaulted to `[]`, which skips the ADR-0066 D3
22+
* capability AND-gate entirely (`if (required.length > 0)`).
23+
*
24+
* These tests pin BOTH directions: the control (metadata resolvable → denied,
25+
* the ADR-0066 behaviour) and the regression (metadata unresolvable → still
26+
* denied, rather than silently promoted to public + uncontracted).
27+
*/
28+
29+
import { describe, it, expect, vi } from 'vitest';
30+
import { SecurityPlugin } from './security-plugin.js';
31+
import { PermissionEvaluator } from './permission-evaluator.js';
32+
import { explainAccess, type ExplainEngineDeps } from './explain-engine.js';
33+
import type { PermissionSet } from '@objectstack/spec/security';
34+
35+
/** Plain member: blanket wildcard grant, NO superuser bits, NO capabilities. */
36+
const memberSet: PermissionSet = {
37+
name: 'member_default',
38+
label: 'Member',
39+
objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } },
40+
} as any;
41+
42+
/**
43+
* Middleware harness. `resolvable: false` makes the OBJECT metadata
44+
* unresolvable — `ql.getSchema()` returns undefined and `metadata.get('object',
45+
* …)` throws — while permission-set resolution keeps working. That isolates the
46+
* axis under test: "the object's own posture can't be read", NOT "the permission
47+
* subsystem is down" (which already fails closed, see security-plugin.ts).
48+
*/
49+
const makeHarness = (opts: { schemaExtra?: Record<string, any>; resolvable: boolean }) => {
50+
const fields: Record<string, any> = {};
51+
for (const f of ['id', 'organization_id', 'owner_id', 'name']) fields[f] = { name: f };
52+
const baseSchema: any = { name: 'task', fields, ...(opts.schemaExtra ?? {}) };
53+
54+
let middleware: any;
55+
const ql = {
56+
registerMiddleware: (mw: any) => {
57+
if (!middleware) middleware = mw;
58+
},
59+
getSchema: () => (opts.resolvable ? baseSchema : undefined),
60+
findOne: vi.fn(async () => null),
61+
};
62+
const metadata = {
63+
get: async (type: string, name: string) => {
64+
if (!opts.resolvable && type === 'object' && name === 'task') {
65+
throw new Error('metadata store unavailable');
66+
}
67+
return baseSchema;
68+
},
69+
// Permission-set resolution stays healthy on BOTH paths.
70+
list: async () => [memberSet],
71+
};
72+
const services: Record<string, any> = {
73+
manifest: { register: vi.fn() },
74+
objectql: ql,
75+
metadata,
76+
};
77+
const ctx: any = {
78+
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
79+
registerService: vi.fn(),
80+
getService: (name: string) => {
81+
if (!(name in services)) throw new Error(`service not registered: ${name}`);
82+
return services[name];
83+
},
84+
};
85+
return {
86+
ctx,
87+
logger: ctx.logger,
88+
run: async (opCtx: any) => {
89+
await middleware(opCtx, async () => {});
90+
return opCtx;
91+
},
92+
};
93+
};
94+
95+
const boot = async (opts: { schemaExtra?: Record<string, any>; resolvable: boolean }) => {
96+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
97+
const harness = makeHarness(opts);
98+
await plugin.init(harness.ctx);
99+
await plugin.start(harness.ctx);
100+
return harness;
101+
};
102+
103+
/** Authenticated member — resolves to a non-empty permission-set list. */
104+
const memberRead = (): any => ({
105+
object: 'task',
106+
operation: 'find',
107+
ast: { where: undefined },
108+
context: { userId: 'u1', tenantId: 'org-1', positions: [], permissions: [] },
109+
});
110+
111+
describe('[#3545] unresolvable object metadata — security posture fails closed', () => {
112+
describe('private posture (ADR-0066 D2)', () => {
113+
it('control: metadata RESOLVABLE → a plain wildcard does not reach a private object', async () => {
114+
const h = await boot({ schemaExtra: { access: { default: 'private' } }, resolvable: true });
115+
await expect(h.run(memberRead())).rejects.toMatchObject({ name: 'PermissionDeniedError' });
116+
});
117+
118+
it('metadata UNRESOLVABLE → still denied (posture must not default to public)', async () => {
119+
const h = await boot({ schemaExtra: { access: { default: 'private' } }, resolvable: false });
120+
await expect(h.run(memberRead())).rejects.toMatchObject({ name: 'PermissionDeniedError' });
121+
});
122+
});
123+
124+
describe('requiredPermissions capability contract (ADR-0066 D3)', () => {
125+
it('control: metadata RESOLVABLE → a member lacking the capability is denied', async () => {
126+
const h = await boot({
127+
schemaExtra: { requiredPermissions: ['manage_platform_settings'] },
128+
resolvable: true,
129+
});
130+
await expect(h.run(memberRead())).rejects.toMatchObject({ name: 'PermissionDeniedError' });
131+
});
132+
133+
it('metadata UNRESOLVABLE → still denied (the capability AND-gate must not be skipped)', async () => {
134+
const h = await boot({
135+
schemaExtra: { requiredPermissions: ['manage_platform_settings'] },
136+
resolvable: false,
137+
});
138+
await expect(h.run(memberRead())).rejects.toMatchObject({ name: 'PermissionDeniedError' });
139+
});
140+
});
141+
142+
describe('blast radius', () => {
143+
it('a PUBLIC, uncontracted object is unaffected when its metadata IS resolvable', async () => {
144+
const h = await boot({ resolvable: true });
145+
await expect(h.run(memberRead())).resolves.toBeDefined();
146+
});
147+
148+
it('an anonymous request is unaffected — it short-circuits before the posture read', async () => {
149+
const h = await boot({ resolvable: false });
150+
const anon: any = {
151+
object: 'task',
152+
operation: 'find',
153+
ast: { where: undefined },
154+
context: { positions: [], permissions: [] }, // no userId
155+
};
156+
await expect(h.run(anon)).resolves.toBeDefined();
157+
});
158+
159+
it('a system/boot operation is unaffected — isSystem short-circuits the middleware', async () => {
160+
const h = await boot({ resolvable: false });
161+
const sys: any = {
162+
object: 'task',
163+
operation: 'find',
164+
ast: { where: undefined },
165+
context: { isSystem: true, userId: 'usr_system' },
166+
};
167+
await expect(h.run(sys)).resolves.toBeDefined();
168+
});
169+
170+
it('logs the unresolvable posture so a persistent outage is observable', async () => {
171+
const h = await boot({ resolvable: false });
172+
await expect(h.run(memberRead())).rejects.toMatchObject({ name: 'PermissionDeniedError' });
173+
expect(h.logger.error).toHaveBeenCalled();
174+
});
175+
});
176+
177+
// The "why am I denied?" surface must agree with the enforcement path —
178+
// explain reporting `allowed` where the middleware throws is the same
179+
// declared-≠-enforced drift the engine exists to expose.
180+
describe('explain parity', () => {
181+
const explainDeps = (unresolved: boolean): ExplainEngineDeps =>
182+
({
183+
ql: { getSchema: () => ({ name: 'task' }) },
184+
resolveSets: async () => [memberSet],
185+
evaluator: new PermissionEvaluator(),
186+
getObjectSecurityMeta: async () => ({
187+
isPrivate: false,
188+
requiredPermissions: { all: [], read: [], create: [], update: [], delete: [] },
189+
fieldRequiredPermissions: {},
190+
unresolved,
191+
}),
192+
requiredCaps: (meta: any, op: string) => {
193+
const bucket = op === 'find' ? 'read' : op === 'insert' ? 'create' : op;
194+
return [...(meta.all ?? []), ...(meta[bucket] ?? [])];
195+
},
196+
computeRlsFilter: async () => null,
197+
getFieldMask: () => ({}),
198+
fallbackPermissionSet: 'member_default',
199+
}) as any;
200+
201+
const ctx = { userId: 'u1', positions: ['everyone'], permissions: [] };
202+
203+
it('control: a resolvable posture still explains as granted', async () => {
204+
const d = await explainAccess(explainDeps(false), { object: 'task', operation: 'read', context: ctx });
205+
expect(d.allowed).toBe(true);
206+
expect(d.layers.find((l) => l.layer === 'object_crud')!.verdict).toBe('grants');
207+
});
208+
209+
it('an unresolvable posture explains as DENIED, naming the real cause', async () => {
210+
const d = await explainAccess(explainDeps(true), { object: 'task', operation: 'read', context: ctx });
211+
expect(d.allowed).toBe(false);
212+
const crud = d.layers.find((l) => l.layer === 'object_crud')!;
213+
expect(crud.verdict).toBe('denies');
214+
expect(crud.detail).toContain('could not be resolved');
215+
// Not misattributed to the permission sets — they were never the problem.
216+
expect(crud.detail).not.toContain('No resolved permission set');
217+
});
218+
});
219+
});

0 commit comments

Comments
 (0)