Skip to content

Commit 95c6b5f

Browse files
committed
fix(security): fail closed when an object's security posture can't be resolved (#3545)
#3545 assessed the API-exposure gate's fail-open on unresolvable metadata and accepted it, resting on one load-bearing premise: the gate is a SURFACE-AREA control, while the real authorization boundary — auth + the ObjectQL security middleware (CRUD/FLS/RLS) — enforces unconditionally on the data call whatever the gate answers. Verifying that premise instead of assuming it shows it did not hold. The middleware does run unconditionally, but two of its INPUTS were read from the same object metadata and defaulted permissively when it could not be resolved, so the very trigger this issue is about reached one layer PAST the gate, into the boundary itself: * `access.default: 'private'` -> `isPrivate` defaulted to false. ADR-0066 D2 deliberately excludes a private object from a plain (non-superuser) `'*'` wildcard; read as public it IS covered — a grant nobody authored. * `requiredPermissions` -> defaulted to `[]`, which skips the ADR-0066 D3 capability AND-gate entirely (`if (required.length > 0)`). `getObjectSecurityMeta` now flags `unresolved`, and the three consumers that turn posture into an access decision fail closed on it: the middleware denies (with an error log, so a persistent metadata outage is observable rather than a silent blanket-allow), `canExport` denies, `getReadableFields` exposes no columns — the same stance already taken for a permission-resolution failure and a dangling delegator. `computeLayeredRlsFilter` keeps consuming the defaults on purpose: there the permissive value WITHHOLDS the cross-tenant exemption, so it is already the closed direction. Blast radius is bounded to the risky case. System/boot writes (`isSystem`) and principal-less/anonymous contexts short-circuit earlier in the middleware, so reaching the new check means an authenticated principal with resolved grants asking for an object whose declaration is missing. The cold-start window is served by those short-circuits, not by the permissive default — which is why the tiered decision recorded for the exposure gate (transient unavailability -> fail open) stands unchanged now that the boundary underneath it actually holds. The explain engine reports the denial on its existing `object_crud` layer naming the real cause, so the "why am I denied?" surface cannot drift from enforcement. Regression-pinned in metadata-unresolvable-posture.test.ts: both directions of each axis (resolvable -> denied per ADR-0066; unresolvable -> still denied), the unaffected anonymous / system / public-object paths, the error log, and explain parity. Verified green: plugin-security (645), runtime, rest, objectql, and the dogfood suite that boots the real showcase app (369). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019288JyDfHtCiYVgAJLxmsg
1 parent b098b0e commit 95c6b5f

4 files changed

Lines changed: 324 additions & 16 deletions

File tree

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+
});

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

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,19 @@ interface ObjectSecurityMeta {
149149
isBetterAuthManaged: boolean;
150150
requiredPermissions: NormalizedRequiredPermissions;
151151
fieldRequiredPermissions: Record<string, string[]>;
152+
/**
153+
* [#3545] The object's posture could NOT be resolved — neither the live
154+
* ObjectQL schema nor the metadata service returned it. Every other field is
155+
* then a DEFAULT, not the author's declaration, and each default happens to be
156+
* the permissive end of its axis (`isPrivate: false` is covered by a plain
157+
* `'*'` wildcard; empty `requiredPermissions` skips the capability AND-gate).
158+
* Consumers that turn this into an ACCESS DECISION must fail closed on it —
159+
* see the call sites in the middleware, {@link canExport} and
160+
* {@link getReadableFields}. Consumers that only widen scoping from it (the
161+
* RLS posture exemption in {@link computeLayeredRlsFilter}) are already safe:
162+
* the permissive default withholds the exemption.
163+
*/
164+
unresolved: boolean;
152165
}
153166

154167
const EMPTY_REQUIRED_PERMISSIONS: NormalizedRequiredPermissions = Object.freeze({
@@ -909,7 +922,41 @@ export class SecurityPlugin implements Plugin {
909922
const secMeta =
910923
permissionSets.length > 0
911924
? await this.getObjectSecurityMeta(opCtx.object)
912-
: { isPrivate: false, tenancyDisabled: false, isBetterAuthManaged: false, requiredPermissions: EMPTY_REQUIRED_PERMISSIONS, fieldRequiredPermissions: {} as Record<string, string[]> };
925+
: { isPrivate: false, tenancyDisabled: false, isBetterAuthManaged: false, requiredPermissions: EMPTY_REQUIRED_PERMISSIONS, fieldRequiredPermissions: {} as Record<string, string[]>, unresolved: false };
926+
927+
// [#3545] Fail CLOSED when the object's own posture could not be resolved.
928+
// #3545 accepted the API-exposure gate's fail-open on unresolvable metadata
929+
// because that gate is a SURFACE-AREA control while THIS middleware is the
930+
// authorization boundary and enforces regardless. That holds only if the
931+
// boundary's own inputs are trustworthy — and two of them are read from the
932+
// same metadata and default permissively: an unresolved `access.default`
933+
// reads as PUBLIC (so a plain `'*'` wildcard covers an object ADR-0066 D2
934+
// says it must not) and an unresolved `requiredPermissions` reads as NO
935+
// CONTRACT (so the D3 capability AND-gate below is skipped entirely). Both
936+
// are access-NARROWING declarations, so failing to read them must never
937+
// resolve to a grant (ADR-0049) — the same stance the permission-resolution
938+
// failure above and the dangling-delegator checks already take.
939+
//
940+
// Blast radius is bounded to exactly the risky case: system/boot writes
941+
// (`isSystem`) and principal-less/anonymous contexts short-circuited above,
942+
// so reaching here means an AUTHENTICATED principal with resolved grants
943+
// asking for an object whose declaration is missing. Cold start therefore
944+
// does NOT trip this — that window is served by the earlier short-circuits,
945+
// not by the permissive default — which is why the tiered decision recorded
946+
// for the exposure gate (transient unavailability → fail open) can stay
947+
// fail-open there while the boundary itself fails closed here.
948+
if (secMeta.unresolved) {
949+
ctx.logger.error(
950+
`[security] object security posture unresolvable for operation '${opCtx.operation}' on ` +
951+
`object '${opCtx.object}' (user ${opCtx.context?.userId ?? 'unknown'}) — ` +
952+
`denying request (fail-closed, #3545)`,
953+
);
954+
throw new PermissionDeniedError(
955+
`[Security] Access denied: the security posture of object '${opCtx.object}' ` +
956+
`could not be resolved for operation '${opCtx.operation}'`,
957+
{ operation: opCtx.operation, object: opCtx.object },
958+
);
959+
}
913960

914961
// [#2850] $expand sub-read gate relaxation. The engine's expand path
915962
// re-enters `find` for a referenced object carrying `__expandRead` (a
@@ -2233,6 +2280,11 @@ export class SecurityPlugin implements Plugin {
22332280
if (permissionSets.length === 0) return allFields;
22342281

22352282
const secMeta = await this.getObjectSecurityMeta(objectName);
2283+
// [#3545] Posture unresolvable → expose no columns, the same fail-closed
2284+
// stance this method already takes on a dangling delegator below. The
2285+
// per-field capability contract (`fieldRequiredPermissions`) would otherwise
2286+
// default to empty and silently unmask every capability-gated column.
2287+
if (secMeta.unresolved) return [];
22362288
let fieldPerms = this.permissionEvaluator.getFieldPermissions(objectName, permissionSets);
22372289
fieldPerms = this.foldFieldRequiredPermissions(fieldPerms, secMeta.fieldRequiredPermissions, permissionSets);
22382290

@@ -2287,7 +2339,11 @@ export class SecurityPlugin implements Plugin {
22872339
// (`if (permissionSets.length > 0)` guards its whole CRUD gate).
22882340
if (permissionSets.length === 0) return true;
22892341

2290-
const { isPrivate } = await this.getObjectSecurityMeta(objectName);
2342+
const { isPrivate, unresolved } = await this.getObjectSecurityMeta(objectName);
2343+
// [#3545] Posture unresolvable → deny. `isPrivate` would default to `false`,
2344+
// which is precisely what lets a plain wildcard reach the object; a bulk
2345+
// egress decision must not rest on a default we could not read.
2346+
if (unresolved) return false;
22912347
if (!this.permissionEvaluator.checkObjectPermission('export', objectName, permissionSets, { isPrivate })) {
22922348
return false;
22932349
}
@@ -3159,8 +3215,13 @@ export class SecurityPlugin implements Plugin {
31593215
* is `private` (access.default), platform-global (tenancy disabled), and its
31603216
* `requiredPermissions` capability contract. Prefers the live ObjectQL schema
31613217
* (reflects registry-time augmentation) and falls back to the metadata service.
3162-
* Returns the permissive default when the schema can't be resolved yet (boot) —
3163-
* the CRUD/RLS checks then behave as pre-0066 and the miss is retried next call.
3218+
*
3219+
* [#3545] When NEITHER source resolves the object, the returned values are
3220+
* defaults rather than declarations, and it is flagged `unresolved: true`. The
3221+
* defaults are NOT safe to make an access decision from — each is the
3222+
* permissive end of its axis — so callers that gate access must fail closed on
3223+
* the flag instead of consuming the defaults. Only positive resolutions are
3224+
* cached, so a transient boot miss is retried on the next call.
31643225
*/
31653226
private async getObjectSecurityMeta(
31663227
object: string,
@@ -3203,6 +3264,7 @@ export class SecurityPlugin implements Plugin {
32033264
isBetterAuthManaged: (obj as any)?.managedBy === 'better-auth',
32043265
requiredPermissions: normalizeRequiredPermissions((obj as any)?.requiredPermissions),
32053266
fieldRequiredPermissions,
3267+
unresolved: !obj,
32063268
};
32073269
if (obj) this.objectSecurityMetaCache.set(object, meta);
32083270
return meta;

0 commit comments

Comments
 (0)