diff --git a/.changeset/metadata-unresolvable-posture-fail-closed.md b/.changeset/metadata-unresolvable-posture-fail-closed.md new file mode 100644 index 0000000000..488579677e --- /dev/null +++ b/.changeset/metadata-unresolvable-posture-fail-closed.md @@ -0,0 +1,41 @@ +--- +"@objectstack/plugin-security": patch +"@objectstack/runtime": patch +--- + +fix(security): fail closed when an object's security posture can't be resolved +(#3545) + +#3545 accepted the API-exposure gate's fail-open on unresolvable metadata on one +load-bearing premise: that 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 rather than 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 the issue is about reached one layer PAST the gate, into the +boundary itself: an unresolved `access.default` read as PUBLIC (so a plain `'*'` +wildcard covered an object ADR-0066 D2 excludes from it) and an unresolved +`requiredPermissions` read as NO CONTRACT (so the D3 capability AND-gate was +skipped entirely). + +`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, and `getReadableFields` exposes no columns — +the same stance already taken for a permission-resolution failure and a dangling +delegator. `computeLayeredRlsFilter` keeps consuming the defaults deliberately: +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. The exposure +gate's own tiered decision (transient unavailability → fail open) is therefore +unchanged — it now rests on a boundary that 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. diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index 4ea2efb086..df37e2d805 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -146,6 +146,8 @@ export interface ExplainEngineDeps { isPrivate: boolean; requiredPermissions: any; fieldRequiredPermissions: Record; + /** [#3545] Posture could not be read — the middleware denies (fail-closed). */ + unresolved?: boolean; }>; /** The middleware's requiredPermissions AND-gate resolution for an operation. */ requiredCaps: (meta: any, engineOperation: string) => string[]; @@ -840,22 +842,35 @@ export async function explainAccess(deps: ExplainEngineDeps, input: ExplainInput const delegatorCrud = delegatorSets ? deps.evaluator.checkObjectPermission(engineOp, object, delegatorSets, { isPrivate: secMeta.isPrivate }) : true; - const crudAllowed = agentCrud && delegatorCrud && !delegatorMissing; - const granting = sets - .filter((s) => deps.evaluator.checkObjectPermission(engineOp, object, [s], { isPrivate: secMeta.isPrivate })) - .map((s: any) => String(s.name ?? '?')); + // [#3545] An UNRESOLVED posture is a denial in the middleware, so it must read + // as one here too — explain and enforcement disagreeing on a security surface + // is the same `declared ≠ enforced` gap this engine exists to expose. Reported + // on the existing `object_crud` layer (no new layer kind): the posture is what + // that layer's grant is computed FROM, and reporting the real cause beats a + // misleading "no set grants it" when the sets were never the problem. + const postureUnresolved = secMeta.unresolved === true; + const crudAllowed = agentCrud && delegatorCrud && !delegatorMissing && !postureUnresolved; + const granting = postureUnresolved + ? [] + : sets + .filter((s) => deps.evaluator.checkObjectPermission(engineOp, object, [s], { isPrivate: secMeta.isPrivate })) + .map((s: any) => String(s.name ?? '?')); layers.push({ layer: 'object_crud', verdict: crudAllowed ? 'grants' : 'denies', detail: crudAllowed ? `${operation} on '${object}' is granted by [${granting.join(', ')}]` + (delegatorSets ? ' AND by the delegator (D10 intersection).' : '.') - : delegatorMissing - ? `Delegator no longer exists — D10 fails closed (access denied).` - : agentCrud && !delegatorCrud - ? `The agent grants ${operation} on '${object}' but the DELEGATOR does not — D10 intersection denies (an agent may not exceed the user it acts for).` - : `No resolved permission set grants ${operation} on '${object}'` + - (secMeta.isPrivate ? " (object is 'private' posture — non-superuser '*' wildcards are excluded, ADR-0066 D2)." : '.'), + : postureUnresolved + ? `The security posture of '${object}' could not be resolved (neither the live schema nor the ` + + `metadata service returned it) — its 'private' flag and required-capability contract are ` + + `unknown, so access fails CLOSED rather than defaulting to public/uncontracted (#3545).` + : delegatorMissing + ? `Delegator no longer exists — D10 fails closed (access denied).` + : agentCrud && !delegatorCrud + ? `The agent grants ${operation} on '${object}' but the DELEGATOR does not — D10 intersection denies (an agent may not exceed the user it acts for).` + : `No resolved permission set grants ${operation} on '${object}'` + + (secMeta.isPrivate ? " (object is 'private' posture — non-superuser '*' wildcards are excluded, ADR-0066 D2)." : '.'), contributors: granting.map((n) => ({ kind: 'permission_set' as const, name: n, via: viaOf(n) })), }); diff --git a/packages/plugins/plugin-security/src/metadata-unresolvable-posture.test.ts b/packages/plugins/plugin-security/src/metadata-unresolvable-posture.test.ts new file mode 100644 index 0000000000..e915fac793 --- /dev/null +++ b/packages/plugins/plugin-security/src/metadata-unresolvable-posture.test.ts @@ -0,0 +1,219 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#3545] Object metadata unresolvable → the SECURITY POSTURE must fail CLOSED. + * + * #3545 assessed the fail-open on unresolvable metadata in the API-exposure gate + * (`checkApiExposure` / REST `enforceApiAccess`) and accepted it, on the premise + * that 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 regardless of the gate's answer. + * + * 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 same trigger reached one layer deeper than the assessment + * looked: + * + * • `access.default: 'private'` → `isPrivate` defaulted to `false`. A private + * object is deliberately NOT covered by a plain (non-superuser) `'*'` + * wildcard grant (ADR-0066 D2, `resolveObjectPermission`); read as public it + * IS covered — a grant the author never wrote. + * • `requiredPermissions` → defaulted to `[]`, which skips the ADR-0066 D3 + * capability AND-gate entirely (`if (required.length > 0)`). + * + * These tests pin BOTH directions: the control (metadata resolvable → denied, + * the ADR-0066 behaviour) and the regression (metadata unresolvable → still + * denied, rather than silently promoted to public + uncontracted). + */ + +import { describe, it, expect, vi } from 'vitest'; +import { SecurityPlugin } from './security-plugin.js'; +import { PermissionEvaluator } from './permission-evaluator.js'; +import { explainAccess, type ExplainEngineDeps } from './explain-engine.js'; +import type { PermissionSet } from '@objectstack/spec/security'; + +/** Plain member: blanket wildcard grant, NO superuser bits, NO capabilities. */ +const memberSet: PermissionSet = { + name: 'member_default', + label: 'Member', + objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, +} as any; + +/** + * Middleware harness. `resolvable: false` makes the OBJECT metadata + * unresolvable — `ql.getSchema()` returns undefined and `metadata.get('object', + * …)` throws — while permission-set resolution keeps working. That isolates the + * axis under test: "the object's own posture can't be read", NOT "the permission + * subsystem is down" (which already fails closed, see security-plugin.ts). + */ +const makeHarness = (opts: { schemaExtra?: Record; resolvable: boolean }) => { + const fields: Record = {}; + for (const f of ['id', 'organization_id', 'owner_id', 'name']) fields[f] = { name: f }; + const baseSchema: any = { name: 'task', fields, ...(opts.schemaExtra ?? {}) }; + + let middleware: any; + const ql = { + registerMiddleware: (mw: any) => { + if (!middleware) middleware = mw; + }, + getSchema: () => (opts.resolvable ? baseSchema : undefined), + findOne: vi.fn(async () => null), + }; + const metadata = { + get: async (type: string, name: string) => { + if (!opts.resolvable && type === 'object' && name === 'task') { + throw new Error('metadata store unavailable'); + } + return baseSchema; + }, + // Permission-set resolution stays healthy on BOTH paths. + list: async () => [memberSet], + }; + const services: Record = { + manifest: { register: vi.fn() }, + objectql: ql, + metadata, + }; + const ctx: any = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + return { + ctx, + logger: ctx.logger, + run: async (opCtx: any) => { + await middleware(opCtx, async () => {}); + return opCtx; + }, + }; +}; + +const boot = async (opts: { schemaExtra?: Record; resolvable: boolean }) => { + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + const harness = makeHarness(opts); + await plugin.init(harness.ctx); + await plugin.start(harness.ctx); + return harness; +}; + +/** Authenticated member — resolves to a non-empty permission-set list. */ +const memberRead = (): any => ({ + object: 'task', + operation: 'find', + ast: { where: undefined }, + context: { userId: 'u1', tenantId: 'org-1', positions: [], permissions: [] }, +}); + +describe('[#3545] unresolvable object metadata — security posture fails closed', () => { + describe('private posture (ADR-0066 D2)', () => { + it('control: metadata RESOLVABLE → a plain wildcard does not reach a private object', async () => { + const h = await boot({ schemaExtra: { access: { default: 'private' } }, resolvable: true }); + await expect(h.run(memberRead())).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('metadata UNRESOLVABLE → still denied (posture must not default to public)', async () => { + const h = await boot({ schemaExtra: { access: { default: 'private' } }, resolvable: false }); + await expect(h.run(memberRead())).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + }); + + describe('requiredPermissions capability contract (ADR-0066 D3)', () => { + it('control: metadata RESOLVABLE → a member lacking the capability is denied', async () => { + const h = await boot({ + schemaExtra: { requiredPermissions: ['manage_platform_settings'] }, + resolvable: true, + }); + await expect(h.run(memberRead())).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('metadata UNRESOLVABLE → still denied (the capability AND-gate must not be skipped)', async () => { + const h = await boot({ + schemaExtra: { requiredPermissions: ['manage_platform_settings'] }, + resolvable: false, + }); + await expect(h.run(memberRead())).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + }); + + describe('blast radius', () => { + it('a PUBLIC, uncontracted object is unaffected when its metadata IS resolvable', async () => { + const h = await boot({ resolvable: true }); + await expect(h.run(memberRead())).resolves.toBeDefined(); + }); + + it('an anonymous request is unaffected — it short-circuits before the posture read', async () => { + const h = await boot({ resolvable: false }); + const anon: any = { + object: 'task', + operation: 'find', + ast: { where: undefined }, + context: { positions: [], permissions: [] }, // no userId + }; + await expect(h.run(anon)).resolves.toBeDefined(); + }); + + it('a system/boot operation is unaffected — isSystem short-circuits the middleware', async () => { + const h = await boot({ resolvable: false }); + const sys: any = { + object: 'task', + operation: 'find', + ast: { where: undefined }, + context: { isSystem: true, userId: 'usr_system' }, + }; + await expect(h.run(sys)).resolves.toBeDefined(); + }); + + it('logs the unresolvable posture so a persistent outage is observable', async () => { + const h = await boot({ resolvable: false }); + await expect(h.run(memberRead())).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + expect(h.logger.error).toHaveBeenCalled(); + }); + }); + + // The "why am I denied?" surface must agree with the enforcement path — + // explain reporting `allowed` where the middleware throws is the same + // declared-≠-enforced drift the engine exists to expose. + describe('explain parity', () => { + const explainDeps = (unresolved: boolean): ExplainEngineDeps => + ({ + ql: { getSchema: () => ({ name: 'task' }) }, + resolveSets: async () => [memberSet], + evaluator: new PermissionEvaluator(), + getObjectSecurityMeta: async () => ({ + isPrivate: false, + requiredPermissions: { all: [], read: [], create: [], update: [], delete: [] }, + fieldRequiredPermissions: {}, + unresolved, + }), + requiredCaps: (meta: any, op: string) => { + const bucket = op === 'find' ? 'read' : op === 'insert' ? 'create' : op; + return [...(meta.all ?? []), ...(meta[bucket] ?? [])]; + }, + computeRlsFilter: async () => null, + getFieldMask: () => ({}), + fallbackPermissionSet: 'member_default', + }) as any; + + const ctx = { userId: 'u1', positions: ['everyone'], permissions: [] }; + + it('control: a resolvable posture still explains as granted', async () => { + const d = await explainAccess(explainDeps(false), { object: 'task', operation: 'read', context: ctx }); + expect(d.allowed).toBe(true); + expect(d.layers.find((l) => l.layer === 'object_crud')!.verdict).toBe('grants'); + }); + + it('an unresolvable posture explains as DENIED, naming the real cause', async () => { + const d = await explainAccess(explainDeps(true), { object: 'task', operation: 'read', context: ctx }); + expect(d.allowed).toBe(false); + const crud = d.layers.find((l) => l.layer === 'object_crud')!; + expect(crud.verdict).toBe('denies'); + expect(crud.detail).toContain('could not be resolved'); + // Not misattributed to the permission sets — they were never the problem. + expect(crud.detail).not.toContain('No resolved permission set'); + }); + }); +}); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 6b73efa4f4..f63eeef8c9 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -149,6 +149,19 @@ interface ObjectSecurityMeta { isBetterAuthManaged: boolean; requiredPermissions: NormalizedRequiredPermissions; fieldRequiredPermissions: Record; + /** + * [#3545] The object's posture could NOT be resolved — neither the live + * ObjectQL schema nor the metadata service returned it. Every other field is + * then a DEFAULT, not the author's declaration, and each default happens to be + * the permissive end of its axis (`isPrivate: false` is covered by a plain + * `'*'` wildcard; empty `requiredPermissions` skips the capability AND-gate). + * Consumers that turn this into an ACCESS DECISION must fail closed on it — + * see the call sites in the middleware, {@link canExport} and + * {@link getReadableFields}. Consumers that only widen scoping from it (the + * RLS posture exemption in {@link computeLayeredRlsFilter}) are already safe: + * the permissive default withholds the exemption. + */ + unresolved: boolean; } const EMPTY_REQUIRED_PERMISSIONS: NormalizedRequiredPermissions = Object.freeze({ @@ -909,7 +922,41 @@ export class SecurityPlugin implements Plugin { const secMeta = permissionSets.length > 0 ? await this.getObjectSecurityMeta(opCtx.object) - : { isPrivate: false, tenancyDisabled: false, isBetterAuthManaged: false, requiredPermissions: EMPTY_REQUIRED_PERMISSIONS, fieldRequiredPermissions: {} as Record }; + : { isPrivate: false, tenancyDisabled: false, isBetterAuthManaged: false, requiredPermissions: EMPTY_REQUIRED_PERMISSIONS, fieldRequiredPermissions: {} as Record, unresolved: false }; + + // [#3545] Fail CLOSED when the object's own posture could not be resolved. + // #3545 accepted the API-exposure gate's fail-open on unresolvable metadata + // because that gate is a SURFACE-AREA control while THIS middleware is the + // authorization boundary and enforces regardless. That holds only if the + // boundary's own inputs are trustworthy — and two of them are read from the + // same metadata and default permissively: an unresolved `access.default` + // reads as PUBLIC (so a plain `'*'` wildcard covers an object ADR-0066 D2 + // says it must not) and an unresolved `requiredPermissions` reads as NO + // CONTRACT (so the D3 capability AND-gate below is skipped entirely). Both + // are access-NARROWING declarations, so failing to read them must never + // resolve to a grant (ADR-0049) — the same stance the permission-resolution + // failure above and the dangling-delegator checks already take. + // + // Blast radius is bounded to exactly the risky case: system/boot writes + // (`isSystem`) and principal-less/anonymous contexts short-circuited above, + // so reaching here means an AUTHENTICATED principal with resolved grants + // asking for an object whose declaration is missing. Cold start therefore + // does NOT trip this — that window is served by the earlier short-circuits, + // not by the permissive default — which is why the tiered decision recorded + // for the exposure gate (transient unavailability → fail open) can stay + // fail-open there while the boundary itself fails closed here. + if (secMeta.unresolved) { + ctx.logger.error( + `[security] object security posture unresolvable for operation '${opCtx.operation}' on ` + + `object '${opCtx.object}' (user ${opCtx.context?.userId ?? 'unknown'}) — ` + + `denying request (fail-closed, #3545)`, + ); + throw new PermissionDeniedError( + `[Security] Access denied: the security posture of object '${opCtx.object}' ` + + `could not be resolved for operation '${opCtx.operation}'`, + { operation: opCtx.operation, object: opCtx.object }, + ); + } // [#2850] $expand sub-read gate relaxation. The engine's expand path // re-enters `find` for a referenced object carrying `__expandRead` (a @@ -2233,6 +2280,11 @@ export class SecurityPlugin implements Plugin { if (permissionSets.length === 0) return allFields; const secMeta = await this.getObjectSecurityMeta(objectName); + // [#3545] Posture unresolvable → expose no columns, the same fail-closed + // stance this method already takes on a dangling delegator below. The + // per-field capability contract (`fieldRequiredPermissions`) would otherwise + // default to empty and silently unmask every capability-gated column. + if (secMeta.unresolved) return []; let fieldPerms = this.permissionEvaluator.getFieldPermissions(objectName, permissionSets); fieldPerms = this.foldFieldRequiredPermissions(fieldPerms, secMeta.fieldRequiredPermissions, permissionSets); @@ -2287,7 +2339,11 @@ export class SecurityPlugin implements Plugin { // (`if (permissionSets.length > 0)` guards its whole CRUD gate). if (permissionSets.length === 0) return true; - const { isPrivate } = await this.getObjectSecurityMeta(objectName); + const { isPrivate, unresolved } = await this.getObjectSecurityMeta(objectName); + // [#3545] Posture unresolvable → deny. `isPrivate` would default to `false`, + // which is precisely what lets a plain wildcard reach the object; a bulk + // egress decision must not rest on a default we could not read. + if (unresolved) return false; if (!this.permissionEvaluator.checkObjectPermission('export', objectName, permissionSets, { isPrivate })) { return false; } @@ -3159,8 +3215,13 @@ export class SecurityPlugin implements Plugin { * is `private` (access.default), platform-global (tenancy disabled), and its * `requiredPermissions` capability contract. Prefers the live ObjectQL schema * (reflects registry-time augmentation) and falls back to the metadata service. - * Returns the permissive default when the schema can't be resolved yet (boot) — - * the CRUD/RLS checks then behave as pre-0066 and the miss is retried next call. + * + * [#3545] When NEITHER source resolves the object, the returned values are + * defaults rather than declarations, and it is flagged `unresolved: true`. The + * defaults are NOT safe to make an access decision from — each is the + * permissive end of its axis — so callers that gate access must fail closed on + * the flag instead of consuming the defaults. Only positive resolutions are + * cached, so a transient boot miss is retried on the next call. */ private async getObjectSecurityMeta( object: string, @@ -3203,6 +3264,7 @@ export class SecurityPlugin implements Plugin { isBetterAuthManaged: (obj as any)?.managedBy === 'better-auth', requiredPermissions: normalizeRequiredPermissions((obj as any)?.requiredPermissions), fieldRequiredPermissions, + unresolved: !obj, }; if (obj) this.objectSecurityMetaCache.set(object, meta); return meta; diff --git a/packages/runtime/src/api-exposure.ts b/packages/runtime/src/api-exposure.ts index ba04af7c8e..a388bf32f9 100644 --- a/packages/runtime/src/api-exposure.ts +++ b/packages/runtime/src/api-exposure.ts @@ -35,8 +35,20 @@ * ObjectQL security middleware (CRUD / FLS / RLS) on the data call regardless of * the outcome here. So a fail-open on unresolvable metadata cannot bypass data * authorization; at worst an operation the author meant to HIDE from the API is - * transiently reachable (still fully access-controlled). Given that, the tiered - * decision is: + * transiently reachable (still fully access-controlled). + * + * That premise is load-bearing, so it was VERIFIED rather than assumed — and as + * first stated it was wrong. The middleware does run unconditionally, but two of + * its INPUTS were read from this same object metadata and defaulted permissively + * when it could not be resolved: `access.default` read as PUBLIC (so a plain `'*'` + * wildcard covered an object ADR-0066 D2 excludes from it) and `requiredPermissions` + * read as NO CONTRACT (so the D3 capability AND-gate was skipped). The very trigger + * this issue is about therefore reached one layer PAST the surface-area gate, into + * the boundary itself. Closed in plugin-security's `getObjectSecurityMeta`, which + * now flags an unresolved posture so the middleware, `canExport` and + * `getReadableFields` fail CLOSED on it (pinned by + * `metadata-unresolvable-posture.test.ts`). With the boundary now actually + * unconditional, the tiered decision below stands: * • Metadata service not ready / whole registry unavailable (cold start, * registration race, scoped kernel warming) → KEEP fail-open. Failing closed * would 405 every request during the normal startup window for no security