Skip to content

Commit b419135

Browse files
authored
fix(plugin-security): report a metadata-store outage as an outage, not an absent declaration (#10424) (#10707)
1 parent e222a53 commit b419135

4 files changed

Lines changed: 622 additions & 43 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
"@objectstack/plugin-security": patch
3+
---
4+
5+
Report a metadata-store OUTAGE as an outage, not as an absent declaration
6+
(#10424). When an object's security posture cannot be resolved, the refusal
7+
now consumes the `degraded` verdict `IMetadataService.getDiagnosed` was already
8+
computing and discarding (#5840), so a store that could not answer no longer
9+
wears the sentence written for an object that was never declared — "Check that
10+
the object is declared and published on this runtime" sent operators to
11+
re-check a healthy declaration in the middle of an incident. The refusal now
12+
names the store, says the declaration may well be fine, and the operator log
13+
line carries a grep-able `DEGRADED` / `metadata-store OUTAGE`.
14+
15+
Explanation and logging only. The deny is unchanged in every case — same
16+
`PermissionDeniedError`, same `PERMISSION_DENIED`, same 403, still fail-closed
17+
per #3545 — and the set of requests that are accepted or rejected does not
18+
move: the resolving read is untouched and `getDiagnosed` is consulted as a
19+
separate best-effort probe on the path that is already refusing. A metadata
20+
service that does not implement the optional `getDiagnosed` reports `unknown`
21+
and keeps the previous wording; it is never reported as an outage.
Lines changed: 368 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,368 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#10424] A metadata-store OUTAGE must not be reported as an absent
5+
* declaration.
6+
*
7+
* ## The premise these tests were written against, measured before the fix
8+
*
9+
* On `origin/main` (f094214b3, with #10401 landed) the two inputs below —
10+
* an object that is genuinely not declared, and a metadata store that cannot
11+
* answer — produced BYTE-IDENTICAL output:
12+
*
13+
* message: "[Security] Access denied: the security posture of object 'task'
14+
* could not be resolved for operation 'find' — neither the live
15+
* schema nor the metadata service returned a declaration for it,
16+
* so access fails closed. Check that the object is declared and
17+
* published on this runtime. …"
18+
* log: "[security] object security posture unresolvable for operation
19+
* 'find' on object 'task' (user u1) — denying request
20+
* (fail-closed, #3545)"
21+
*
22+
* …even with a metadata service that DID implement `getDiagnosed` and DID
23+
* report `degraded: true`. The verdict was computed and discarded (#5840).
24+
* "Check that the object is declared" is right for the first input and sends
25+
* an operator to re-check a perfectly good declaration during the second.
26+
*
27+
* ## What is and is NOT under test
28+
*
29+
* The DENY is not moving and these tests pin that it does not: all three causes
30+
* refuse, fail-closed, with the same `PermissionDeniedError` /
31+
* `PERMISSION_DENIED` / 403 envelope (#3545). The accept side is pinned too,
32+
* because the tempting implementation — swapping the resolving `metadata.get`
33+
* for `getDiagnosed` — would make an object's resolvability depend on an
34+
* OPTIONAL member, and that is an externally observable accept/reject change.
35+
*
36+
* The third pin is the one that carries the most weight: a service that does
37+
* NOT implement `getDiagnosed` must report `'unknown'`, never
38+
* `'metadata_unavailable'`. Without it, an implementation that always claims an
39+
* outage passes the first two.
40+
*/
41+
42+
import { describe, it, expect, vi } from 'vitest';
43+
import { SecurityPlugin } from './security-plugin.js';
44+
import {
45+
unresolvedPostureRemedy,
46+
unresolvedPostureDenialMessage,
47+
unresolvedPostureExplainDetail,
48+
unresolvedPostureLogLine,
49+
type UnresolvedPostureCause,
50+
} from './unresolved-posture.js';
51+
import type { PermissionSet } from '@objectstack/spec/security';
52+
53+
/** Plain member: blanket wildcard grant, no superuser bits, no capabilities. */
54+
const memberSet: PermissionSet = {
55+
name: 'member_default',
56+
label: 'Member',
57+
objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } },
58+
} as any;
59+
60+
const RESOLVABLE_SCHEMA: any = {
61+
name: 'task',
62+
fields: Object.fromEntries(
63+
['id', 'organization_id', 'owner_id', 'name'].map((f) => [f, { name: f }]),
64+
),
65+
};
66+
67+
interface HarnessOpts {
68+
/** What the live ObjectQL schema answers. `undefined` ⇒ unresolvable there. */
69+
schema?: any;
70+
/** The resolving read. Default: answers `undefined` (genuinely absent). */
71+
metadataGet?: (type: string, name: string) => Promise<any>;
72+
/**
73+
* The OPTIONAL diagnosed read. OMITTED ⇒ the service does not implement the
74+
* capability at all, which is the third pin.
75+
*/
76+
getDiagnosed?: (type: string, name: string) => Promise<any>;
77+
/** What the `sys_metadata` draft probe finds. Default: no draft row. */
78+
draftRow?: any;
79+
}
80+
81+
const boot = async (opts: HarnessOpts) => {
82+
let middleware: any;
83+
const ql = {
84+
registerMiddleware: (mw: any) => { if (!middleware) middleware = mw; },
85+
getSchema: () => opts.schema,
86+
findOne: vi.fn(async () => opts.draftRow ?? null),
87+
};
88+
const metadata: Record<string, any> = {
89+
get: opts.metadataGet ?? (async () => undefined),
90+
list: async () => [memberSet],
91+
};
92+
// Assigned CONDITIONALLY — the `typeof … !== 'function'` probe in
93+
// `probeMetadataOutage` is only exercised when the key is truly absent.
94+
if (opts.getDiagnosed) metadata.getDiagnosed = opts.getDiagnosed;
95+
96+
const services: Record<string, any> = { manifest: { register: vi.fn() }, objectql: ql, metadata };
97+
const logged: string[] = [];
98+
const ctx: any = {
99+
logger: {
100+
info: vi.fn(),
101+
warn: vi.fn(),
102+
error: vi.fn((m: string) => { logged.push(String(m)); }),
103+
},
104+
registerService: vi.fn(),
105+
getService: (n: string) => {
106+
if (!(n in services)) throw new Error(`service not registered: ${n}`);
107+
return services[n];
108+
},
109+
};
110+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
111+
await plugin.init(ctx);
112+
await plugin.start(ctx);
113+
return {
114+
logged,
115+
run: async () => {
116+
const opCtx: any = {
117+
object: 'task',
118+
operation: 'find',
119+
ast: { where: undefined },
120+
context: { userId: 'u1', tenantId: 'org-1', positions: [], permissions: [] },
121+
};
122+
await middleware(opCtx, async () => {});
123+
return opCtx;
124+
},
125+
};
126+
};
127+
128+
/** Drive one request to its refusal and return the whole observable envelope. */
129+
const refusalOf = async (opts: HarnessOpts) => {
130+
const h = await boot(opts);
131+
let err: any;
132+
try {
133+
await h.run();
134+
throw new Error('expected the middleware to refuse, but it allowed the operation');
135+
} catch (e: any) {
136+
err = e;
137+
}
138+
return {
139+
name: err?.name,
140+
code: err?.code,
141+
statusCode: err?.statusCode,
142+
details: err?.details,
143+
message: String(err?.message),
144+
log: h.logged.join('\n'),
145+
};
146+
};
147+
148+
/** The store answers `undefined` and honestly reports the read as healthy. */
149+
const ABSENT: HarnessOpts = {
150+
schema: undefined,
151+
metadataGet: async () => undefined,
152+
getDiagnosed: async () => ({ data: undefined, degraded: false, errors: [] }),
153+
};
154+
155+
/** The store is down: the resolving read throws AND `degraded: true` is reported. */
156+
const OUTAGE: HarnessOpts = {
157+
schema: undefined,
158+
metadataGet: async () => { throw new Error('metadata store unavailable: ECONNREFUSED'); },
159+
getDiagnosed: async () => ({
160+
data: undefined,
161+
degraded: true,
162+
errors: ['loader "db" failed: ECONNREFUSED'],
163+
}),
164+
};
165+
166+
/** A service predating #5840: no `getDiagnosed` member at all. */
167+
const NO_DIAGNOSED: HarnessOpts = {
168+
schema: undefined,
169+
metadataGet: async () => undefined,
170+
};
171+
172+
const expectedMessage = (cause: UnresolvedPostureCause) =>
173+
unresolvedPostureDenialMessage('task', 'find', cause);
174+
const expectedLog = (cause: UnresolvedPostureCause) =>
175+
unresolvedPostureLogLine('task', 'find', 'u1', cause);
176+
177+
describe('[#10424] the three-way split of an unresolved posture', () => {
178+
it('ABSENT declaration → the pre-existing "check that the object is declared" wording', async () => {
179+
const r = await refusalOf(ABSENT);
180+
expect(r.message).toBe(expectedMessage('unknown'));
181+
expect(r.log).toBe(expectedLog('unknown'));
182+
expect(r.message).toContain('Check that the object is declared and published on this runtime.');
183+
// It must not have acquired the outage claim.
184+
expect(r.message).not.toContain('DEGRADED');
185+
expect(r.log).not.toContain('OUTAGE');
186+
});
187+
188+
it('metadata-store OUTAGE → the store wording, NOT the absent-declaration advice', async () => {
189+
const r = await refusalOf(OUTAGE);
190+
expect(r.message).toBe(expectedMessage('metadata_unavailable'));
191+
expect(r.log).toBe(expectedLog('metadata_unavailable'));
192+
expect(r.message).toContain('the metadata service reported its own read as DEGRADED');
193+
expect(r.message).toContain('Check the metadata store');
194+
// The wrong remedy — the entire point of the card — is gone.
195+
expect(r.message).not.toContain('Check that the object is declared and published on this runtime.');
196+
expect(r.message).not.toContain('neither the live schema nor the metadata service returned a declaration');
197+
// …and the operator-facing line is grep-ably an incident.
198+
expect(r.log).toContain('DEGRADED read');
199+
expect(r.log).toContain('metadata-store OUTAGE');
200+
});
201+
202+
it('service WITHOUT `getDiagnosed` → `unknown`, never a manufactured outage', async () => {
203+
// The pin that kills an implementation which simply always reports an
204+
// outage: reporting "I don't know" as "the store is down" is new false
205+
// information, in the opposite direction from the defect being fixed.
206+
const r = await refusalOf(NO_DIAGNOSED);
207+
expect(r.message).toBe(expectedMessage('unknown'));
208+
expect(r.log).toBe(expectedLog('unknown'));
209+
expect(r.message).not.toContain('DEGRADED');
210+
expect(r.message).not.toContain('Check the metadata store');
211+
expect(r.log).not.toContain('OUTAGE');
212+
});
213+
214+
it('the pre-fix COLLAPSE is gone: absent and outage no longer say the same thing', async () => {
215+
const absent = await refusalOf(ABSENT);
216+
const outage = await refusalOf(OUTAGE);
217+
expect(outage.message).not.toBe(absent.message);
218+
expect(outage.log).not.toBe(absent.log);
219+
});
220+
});
221+
222+
describe('[#10424] fail-safe: the absence of a verdict is never published as a verdict', () => {
223+
it('`getDiagnosed` that THROWS → `unknown` (a failed probe is not an outage report)', async () => {
224+
const r = await refusalOf({
225+
...NO_DIAGNOSED,
226+
getDiagnosed: async () => { throw new Error('probe blew up'); },
227+
});
228+
expect(r.message).toBe(expectedMessage('unknown'));
229+
});
230+
231+
it('`degraded` that is not the boolean `true` → `unknown`', async () => {
232+
for (const degraded of ['true', 1, {}, null, undefined]) {
233+
const r = await refusalOf({
234+
...NO_DIAGNOSED,
235+
getDiagnosed: async () => ({ data: undefined, degraded, errors: [] }),
236+
});
237+
expect(r.message).toBe(expectedMessage('unknown'));
238+
}
239+
});
240+
241+
it('`getDiagnosed` that resolves to nothing at all → `unknown`', async () => {
242+
const r = await refusalOf({ ...NO_DIAGNOSED, getDiagnosed: async () => undefined });
243+
expect(r.message).toBe(expectedMessage('unknown'));
244+
});
245+
});
246+
247+
describe('[#10424] precedence: a degraded read outranks the draft probe', () => {
248+
it('outage + a visible DRAFT row → `metadata_unavailable`, not `unpublished_draft`', async () => {
249+
// "A draft exists but no published one" has a second half the outage made
250+
// unknowable — the store is exactly what could not tell us. Asserting it
251+
// would state an unsupportable fact, the same error the card is about.
252+
const r = await refusalOf({ ...OUTAGE, draftRow: { type: 'object', name: 'task', state: 'draft' } });
253+
expect(r.message).toBe(expectedMessage('metadata_unavailable'));
254+
expect(r.message).not.toContain('is not published');
255+
});
256+
257+
it('healthy store + a visible DRAFT row → `unpublished_draft` is still reached (#10401 intact)', async () => {
258+
const r = await refusalOf({ ...ABSENT, draftRow: { type: 'object', name: 'task', state: 'draft' } });
259+
expect(r.message).toBe(expectedMessage('unpublished_draft'));
260+
expect(r.message).toContain('is not published');
261+
});
262+
});
263+
264+
describe('[#3545] the refusal direction is unchanged — pinned separately from the wording', () => {
265+
const cases: Array<[string, HarnessOpts]> = [
266+
['absent declaration', ABSENT],
267+
['metadata-store outage', OUTAGE],
268+
['service without getDiagnosed', NO_DIAGNOSED],
269+
];
270+
271+
for (const [label, opts] of cases) {
272+
it(`${label} → denies fail-closed with the unchanged 403 envelope`, async () => {
273+
const r = await refusalOf(opts);
274+
expect(r.name).toBe('PermissionDeniedError');
275+
expect(r.code).toBe('PERMISSION_DENIED');
276+
expect(r.statusCode).toBe(403);
277+
expect(r.details).toMatchObject({ object: 'task', operation: 'find' });
278+
expect(r.message.startsWith('[Security] Access denied:')).toBe(true);
279+
});
280+
}
281+
282+
it('a degraded read never resolves to a GRANT, even on a private object', async () => {
283+
const r = await refusalOf({
284+
...OUTAGE,
285+
metadataGet: async () => { throw new Error('down'); },
286+
});
287+
expect(r.code).toBe('PERMISSION_DENIED');
288+
});
289+
});
290+
291+
describe('[#10424] the ACCEPT side does not move — the resolving read is untouched', () => {
292+
it('a schema that resolves is still allowed when the service has no `getDiagnosed`', async () => {
293+
const h = await boot({ schema: RESOLVABLE_SCHEMA });
294+
await expect(h.run()).resolves.toBeTruthy();
295+
});
296+
297+
it('a schema that resolves is still allowed when `getDiagnosed` THROWS', async () => {
298+
// If the implementation had swapped the resolving `metadata.get` for
299+
// `getDiagnosed`, this object would now be REFUSED — an externally
300+
// observable accept/reject change this card must not make.
301+
const h = await boot({
302+
schema: RESOLVABLE_SCHEMA,
303+
getDiagnosed: async () => { throw new Error('optional member is broken'); },
304+
});
305+
await expect(h.run()).resolves.toBeTruthy();
306+
});
307+
308+
it('an object resolvable only via `metadata.get` is still allowed', async () => {
309+
const h = await boot({
310+
schema: undefined,
311+
metadataGet: async () => RESOLVABLE_SCHEMA,
312+
getDiagnosed: async () => { throw new Error('optional member is broken'); },
313+
});
314+
await expect(h.run()).resolves.toBeTruthy();
315+
});
316+
317+
it('a degraded read that STILL returns the declaration resolves normally', async () => {
318+
// `degraded` is about completeness, not about the datum in hand. When the
319+
// posture resolves, the cause path is never reached at all.
320+
const h = await boot({
321+
schema: undefined,
322+
metadataGet: async () => RESOLVABLE_SCHEMA,
323+
getDiagnosed: async () => ({ data: RESOLVABLE_SCHEMA, degraded: true, errors: ['one loader down'] }),
324+
});
325+
await expect(h.run()).resolves.toBeTruthy();
326+
});
327+
});
328+
329+
describe('[#10424] the wording module states three distinct things', () => {
330+
const causes: UnresolvedPostureCause[] = ['unpublished_draft', 'metadata_unavailable', 'unknown'];
331+
332+
it('every surface differs pairwise across all three causes', () => {
333+
for (const render of [
334+
(c: UnresolvedPostureCause) => unresolvedPostureRemedy(c),
335+
(c: UnresolvedPostureCause) => unresolvedPostureDenialMessage('task', 'find', c),
336+
(c: UnresolvedPostureCause) => unresolvedPostureExplainDetail('task', c),
337+
(c: UnresolvedPostureCause) => unresolvedPostureLogLine('task', 'find', 'u1', c),
338+
]) {
339+
const rendered = causes.map(render);
340+
expect(new Set(rendered).size).toBe(causes.length);
341+
}
342+
});
343+
344+
it('the outage sentences point at the STORE and disclaim permissions as the lever', () => {
345+
const remedy = unresolvedPostureRemedy('metadata_unavailable');
346+
expect(remedy).toContain('Check the metadata store');
347+
expect(remedy).toContain('Do NOT change the declaration');
348+
expect(remedy).toContain('NOT a permissions problem');
349+
expect(unresolvedPostureExplainDetail('task', 'metadata_unavailable')).toContain('OUTAGE');
350+
expect(unresolvedPostureExplainDetail('task', 'metadata_unavailable')).toContain('#3545');
351+
});
352+
353+
it('the outage denial keeps the pinned opening clause verbatim', () => {
354+
// Substring-compatible with the pre-#10401 sentence, so anything matching
355+
// on it keeps matching; only what follows the em dash is new.
356+
const opening = "the security posture of object 'task' could not be resolved for operation 'find'";
357+
expect(unresolvedPostureDenialMessage('task', 'find', 'metadata_unavailable')).toContain(opening);
358+
expect(unresolvedPostureDenialMessage('task', 'find', 'unknown')).toContain(opening);
359+
});
360+
361+
it('every surface reports the deny as fail-closed regardless of cause', () => {
362+
for (const c of causes) {
363+
expect(unresolvedPostureDenialMessage('task', 'find', c)).toContain('[Security] Access denied:');
364+
expect(unresolvedPostureExplainDetail('task', c)).toContain('fails CLOSED');
365+
expect(unresolvedPostureLogLine('task', 'find', 'u1', c)).toContain('fail-closed, #3545');
366+
}
367+
});
368+
});

0 commit comments

Comments
 (0)