From f14fd1cde35515e71e756ba46d97bf53981e7c65 Mon Sep 17 00:00:00 2001 From: os-warren Date: Fri, 21 Aug 2026 00:40:57 +0000 Subject: [PATCH] fix(plugin-security): report a metadata-store outage as an outage, not an absent declaration (#10424) --- ...curity-metadata-outage-unresolved-cause.md | 21 + .../metadata-outage-unresolved-cause.test.ts | 368 ++++++++++++++++++ .../plugin-security/src/security-plugin.ts | 101 ++++- .../plugin-security/src/unresolved-posture.ts | 175 +++++++-- 4 files changed, 622 insertions(+), 43 deletions(-) create mode 100644 .changeset/security-metadata-outage-unresolved-cause.md create mode 100644 packages/plugins/plugin-security/src/metadata-outage-unresolved-cause.test.ts diff --git a/.changeset/security-metadata-outage-unresolved-cause.md b/.changeset/security-metadata-outage-unresolved-cause.md new file mode 100644 index 0000000000..5244dce750 --- /dev/null +++ b/.changeset/security-metadata-outage-unresolved-cause.md @@ -0,0 +1,21 @@ +--- +"@objectstack/plugin-security": patch +--- + +Report a metadata-store OUTAGE as an outage, not as an absent declaration +(#10424). When an object's security posture cannot be resolved, the refusal +now consumes the `degraded` verdict `IMetadataService.getDiagnosed` was already +computing and discarding (#5840), so a store that could not answer no longer +wears the sentence written for an object that was never declared — "Check that +the object is declared and published on this runtime" sent operators to +re-check a healthy declaration in the middle of an incident. The refusal now +names the store, says the declaration may well be fine, and the operator log +line carries a grep-able `DEGRADED` / `metadata-store OUTAGE`. + +Explanation and logging only. The deny is unchanged in every case — same +`PermissionDeniedError`, same `PERMISSION_DENIED`, same 403, still fail-closed +per #3545 — and the set of requests that are accepted or rejected does not +move: the resolving read is untouched and `getDiagnosed` is consulted as a +separate best-effort probe on the path that is already refusing. A metadata +service that does not implement the optional `getDiagnosed` reports `unknown` +and keeps the previous wording; it is never reported as an outage. diff --git a/packages/plugins/plugin-security/src/metadata-outage-unresolved-cause.test.ts b/packages/plugins/plugin-security/src/metadata-outage-unresolved-cause.test.ts new file mode 100644 index 0000000000..ef0ec747f9 --- /dev/null +++ b/packages/plugins/plugin-security/src/metadata-outage-unresolved-cause.test.ts @@ -0,0 +1,368 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10424] A metadata-store OUTAGE must not be reported as an absent + * declaration. + * + * ## The premise these tests were written against, measured before the fix + * + * On `origin/main` (f094214b3, with #10401 landed) the two inputs below — + * an object that is genuinely not declared, and a metadata store that cannot + * answer — produced BYTE-IDENTICAL output: + * + * message: "[Security] Access denied: the security posture of object 'task' + * could not be resolved for operation 'find' — neither the live + * schema nor the metadata service returned a declaration for it, + * so access fails closed. Check that the object is declared and + * published on this runtime. …" + * log: "[security] object security posture unresolvable for operation + * 'find' on object 'task' (user u1) — denying request + * (fail-closed, #3545)" + * + * …even with a metadata service that DID implement `getDiagnosed` and DID + * report `degraded: true`. The verdict was computed and discarded (#5840). + * "Check that the object is declared" is right for the first input and sends + * an operator to re-check a perfectly good declaration during the second. + * + * ## What is and is NOT under test + * + * The DENY is not moving and these tests pin that it does not: all three causes + * refuse, fail-closed, with the same `PermissionDeniedError` / + * `PERMISSION_DENIED` / 403 envelope (#3545). The accept side is pinned too, + * because the tempting implementation — swapping the resolving `metadata.get` + * for `getDiagnosed` — would make an object's resolvability depend on an + * OPTIONAL member, and that is an externally observable accept/reject change. + * + * The third pin is the one that carries the most weight: a service that does + * NOT implement `getDiagnosed` must report `'unknown'`, never + * `'metadata_unavailable'`. Without it, an implementation that always claims an + * outage passes the first two. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { SecurityPlugin } from './security-plugin.js'; +import { + unresolvedPostureRemedy, + unresolvedPostureDenialMessage, + unresolvedPostureExplainDetail, + unresolvedPostureLogLine, + type UnresolvedPostureCause, +} from './unresolved-posture.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; + +const RESOLVABLE_SCHEMA: any = { + name: 'task', + fields: Object.fromEntries( + ['id', 'organization_id', 'owner_id', 'name'].map((f) => [f, { name: f }]), + ), +}; + +interface HarnessOpts { + /** What the live ObjectQL schema answers. `undefined` ⇒ unresolvable there. */ + schema?: any; + /** The resolving read. Default: answers `undefined` (genuinely absent). */ + metadataGet?: (type: string, name: string) => Promise; + /** + * The OPTIONAL diagnosed read. OMITTED ⇒ the service does not implement the + * capability at all, which is the third pin. + */ + getDiagnosed?: (type: string, name: string) => Promise; + /** What the `sys_metadata` draft probe finds. Default: no draft row. */ + draftRow?: any; +} + +const boot = async (opts: HarnessOpts) => { + let middleware: any; + const ql = { + registerMiddleware: (mw: any) => { if (!middleware) middleware = mw; }, + getSchema: () => opts.schema, + findOne: vi.fn(async () => opts.draftRow ?? null), + }; + const metadata: Record = { + get: opts.metadataGet ?? (async () => undefined), + list: async () => [memberSet], + }; + // Assigned CONDITIONALLY — the `typeof … !== 'function'` probe in + // `probeMetadataOutage` is only exercised when the key is truly absent. + if (opts.getDiagnosed) metadata.getDiagnosed = opts.getDiagnosed; + + const services: Record = { manifest: { register: vi.fn() }, objectql: ql, metadata }; + const logged: string[] = []; + const ctx: any = { + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn((m: string) => { logged.push(String(m)); }), + }, + registerService: vi.fn(), + getService: (n: string) => { + if (!(n in services)) throw new Error(`service not registered: ${n}`); + return services[n]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx); + await plugin.start(ctx); + return { + logged, + run: async () => { + const opCtx: any = { + object: 'task', + operation: 'find', + ast: { where: undefined }, + context: { userId: 'u1', tenantId: 'org-1', positions: [], permissions: [] }, + }; + await middleware(opCtx, async () => {}); + return opCtx; + }, + }; +}; + +/** Drive one request to its refusal and return the whole observable envelope. */ +const refusalOf = async (opts: HarnessOpts) => { + const h = await boot(opts); + let err: any; + try { + await h.run(); + throw new Error('expected the middleware to refuse, but it allowed the operation'); + } catch (e: any) { + err = e; + } + return { + name: err?.name, + code: err?.code, + statusCode: err?.statusCode, + details: err?.details, + message: String(err?.message), + log: h.logged.join('\n'), + }; +}; + +/** The store answers `undefined` and honestly reports the read as healthy. */ +const ABSENT: HarnessOpts = { + schema: undefined, + metadataGet: async () => undefined, + getDiagnosed: async () => ({ data: undefined, degraded: false, errors: [] }), +}; + +/** The store is down: the resolving read throws AND `degraded: true` is reported. */ +const OUTAGE: HarnessOpts = { + schema: undefined, + metadataGet: async () => { throw new Error('metadata store unavailable: ECONNREFUSED'); }, + getDiagnosed: async () => ({ + data: undefined, + degraded: true, + errors: ['loader "db" failed: ECONNREFUSED'], + }), +}; + +/** A service predating #5840: no `getDiagnosed` member at all. */ +const NO_DIAGNOSED: HarnessOpts = { + schema: undefined, + metadataGet: async () => undefined, +}; + +const expectedMessage = (cause: UnresolvedPostureCause) => + unresolvedPostureDenialMessage('task', 'find', cause); +const expectedLog = (cause: UnresolvedPostureCause) => + unresolvedPostureLogLine('task', 'find', 'u1', cause); + +describe('[#10424] the three-way split of an unresolved posture', () => { + it('ABSENT declaration → the pre-existing "check that the object is declared" wording', async () => { + const r = await refusalOf(ABSENT); + expect(r.message).toBe(expectedMessage('unknown')); + expect(r.log).toBe(expectedLog('unknown')); + expect(r.message).toContain('Check that the object is declared and published on this runtime.'); + // It must not have acquired the outage claim. + expect(r.message).not.toContain('DEGRADED'); + expect(r.log).not.toContain('OUTAGE'); + }); + + it('metadata-store OUTAGE → the store wording, NOT the absent-declaration advice', async () => { + const r = await refusalOf(OUTAGE); + expect(r.message).toBe(expectedMessage('metadata_unavailable')); + expect(r.log).toBe(expectedLog('metadata_unavailable')); + expect(r.message).toContain('the metadata service reported its own read as DEGRADED'); + expect(r.message).toContain('Check the metadata store'); + // The wrong remedy — the entire point of the card — is gone. + expect(r.message).not.toContain('Check that the object is declared and published on this runtime.'); + expect(r.message).not.toContain('neither the live schema nor the metadata service returned a declaration'); + // …and the operator-facing line is grep-ably an incident. + expect(r.log).toContain('DEGRADED read'); + expect(r.log).toContain('metadata-store OUTAGE'); + }); + + it('service WITHOUT `getDiagnosed` → `unknown`, never a manufactured outage', async () => { + // The pin that kills an implementation which simply always reports an + // outage: reporting "I don't know" as "the store is down" is new false + // information, in the opposite direction from the defect being fixed. + const r = await refusalOf(NO_DIAGNOSED); + expect(r.message).toBe(expectedMessage('unknown')); + expect(r.log).toBe(expectedLog('unknown')); + expect(r.message).not.toContain('DEGRADED'); + expect(r.message).not.toContain('Check the metadata store'); + expect(r.log).not.toContain('OUTAGE'); + }); + + it('the pre-fix COLLAPSE is gone: absent and outage no longer say the same thing', async () => { + const absent = await refusalOf(ABSENT); + const outage = await refusalOf(OUTAGE); + expect(outage.message).not.toBe(absent.message); + expect(outage.log).not.toBe(absent.log); + }); +}); + +describe('[#10424] fail-safe: the absence of a verdict is never published as a verdict', () => { + it('`getDiagnosed` that THROWS → `unknown` (a failed probe is not an outage report)', async () => { + const r = await refusalOf({ + ...NO_DIAGNOSED, + getDiagnosed: async () => { throw new Error('probe blew up'); }, + }); + expect(r.message).toBe(expectedMessage('unknown')); + }); + + it('`degraded` that is not the boolean `true` → `unknown`', async () => { + for (const degraded of ['true', 1, {}, null, undefined]) { + const r = await refusalOf({ + ...NO_DIAGNOSED, + getDiagnosed: async () => ({ data: undefined, degraded, errors: [] }), + }); + expect(r.message).toBe(expectedMessage('unknown')); + } + }); + + it('`getDiagnosed` that resolves to nothing at all → `unknown`', async () => { + const r = await refusalOf({ ...NO_DIAGNOSED, getDiagnosed: async () => undefined }); + expect(r.message).toBe(expectedMessage('unknown')); + }); +}); + +describe('[#10424] precedence: a degraded read outranks the draft probe', () => { + it('outage + a visible DRAFT row → `metadata_unavailable`, not `unpublished_draft`', async () => { + // "A draft exists but no published one" has a second half the outage made + // unknowable — the store is exactly what could not tell us. Asserting it + // would state an unsupportable fact, the same error the card is about. + const r = await refusalOf({ ...OUTAGE, draftRow: { type: 'object', name: 'task', state: 'draft' } }); + expect(r.message).toBe(expectedMessage('metadata_unavailable')); + expect(r.message).not.toContain('is not published'); + }); + + it('healthy store + a visible DRAFT row → `unpublished_draft` is still reached (#10401 intact)', async () => { + const r = await refusalOf({ ...ABSENT, draftRow: { type: 'object', name: 'task', state: 'draft' } }); + expect(r.message).toBe(expectedMessage('unpublished_draft')); + expect(r.message).toContain('is not published'); + }); +}); + +describe('[#3545] the refusal direction is unchanged — pinned separately from the wording', () => { + const cases: Array<[string, HarnessOpts]> = [ + ['absent declaration', ABSENT], + ['metadata-store outage', OUTAGE], + ['service without getDiagnosed', NO_DIAGNOSED], + ]; + + for (const [label, opts] of cases) { + it(`${label} → denies fail-closed with the unchanged 403 envelope`, async () => { + const r = await refusalOf(opts); + expect(r.name).toBe('PermissionDeniedError'); + expect(r.code).toBe('PERMISSION_DENIED'); + expect(r.statusCode).toBe(403); + expect(r.details).toMatchObject({ object: 'task', operation: 'find' }); + expect(r.message.startsWith('[Security] Access denied:')).toBe(true); + }); + } + + it('a degraded read never resolves to a GRANT, even on a private object', async () => { + const r = await refusalOf({ + ...OUTAGE, + metadataGet: async () => { throw new Error('down'); }, + }); + expect(r.code).toBe('PERMISSION_DENIED'); + }); +}); + +describe('[#10424] the ACCEPT side does not move — the resolving read is untouched', () => { + it('a schema that resolves is still allowed when the service has no `getDiagnosed`', async () => { + const h = await boot({ schema: RESOLVABLE_SCHEMA }); + await expect(h.run()).resolves.toBeTruthy(); + }); + + it('a schema that resolves is still allowed when `getDiagnosed` THROWS', async () => { + // If the implementation had swapped the resolving `metadata.get` for + // `getDiagnosed`, this object would now be REFUSED — an externally + // observable accept/reject change this card must not make. + const h = await boot({ + schema: RESOLVABLE_SCHEMA, + getDiagnosed: async () => { throw new Error('optional member is broken'); }, + }); + await expect(h.run()).resolves.toBeTruthy(); + }); + + it('an object resolvable only via `metadata.get` is still allowed', async () => { + const h = await boot({ + schema: undefined, + metadataGet: async () => RESOLVABLE_SCHEMA, + getDiagnosed: async () => { throw new Error('optional member is broken'); }, + }); + await expect(h.run()).resolves.toBeTruthy(); + }); + + it('a degraded read that STILL returns the declaration resolves normally', async () => { + // `degraded` is about completeness, not about the datum in hand. When the + // posture resolves, the cause path is never reached at all. + const h = await boot({ + schema: undefined, + metadataGet: async () => RESOLVABLE_SCHEMA, + getDiagnosed: async () => ({ data: RESOLVABLE_SCHEMA, degraded: true, errors: ['one loader down'] }), + }); + await expect(h.run()).resolves.toBeTruthy(); + }); +}); + +describe('[#10424] the wording module states three distinct things', () => { + const causes: UnresolvedPostureCause[] = ['unpublished_draft', 'metadata_unavailable', 'unknown']; + + it('every surface differs pairwise across all three causes', () => { + for (const render of [ + (c: UnresolvedPostureCause) => unresolvedPostureRemedy(c), + (c: UnresolvedPostureCause) => unresolvedPostureDenialMessage('task', 'find', c), + (c: UnresolvedPostureCause) => unresolvedPostureExplainDetail('task', c), + (c: UnresolvedPostureCause) => unresolvedPostureLogLine('task', 'find', 'u1', c), + ]) { + const rendered = causes.map(render); + expect(new Set(rendered).size).toBe(causes.length); + } + }); + + it('the outage sentences point at the STORE and disclaim permissions as the lever', () => { + const remedy = unresolvedPostureRemedy('metadata_unavailable'); + expect(remedy).toContain('Check the metadata store'); + expect(remedy).toContain('Do NOT change the declaration'); + expect(remedy).toContain('NOT a permissions problem'); + expect(unresolvedPostureExplainDetail('task', 'metadata_unavailable')).toContain('OUTAGE'); + expect(unresolvedPostureExplainDetail('task', 'metadata_unavailable')).toContain('#3545'); + }); + + it('the outage denial keeps the pinned opening clause verbatim', () => { + // Substring-compatible with the pre-#10401 sentence, so anything matching + // on it keeps matching; only what follows the em dash is new. + const opening = "the security posture of object 'task' could not be resolved for operation 'find'"; + expect(unresolvedPostureDenialMessage('task', 'find', 'metadata_unavailable')).toContain(opening); + expect(unresolvedPostureDenialMessage('task', 'find', 'unknown')).toContain(opening); + }); + + it('every surface reports the deny as fail-closed regardless of cause', () => { + for (const c of causes) { + expect(unresolvedPostureDenialMessage('task', 'find', c)).toContain('[Security] Access denied:'); + expect(unresolvedPostureExplainDetail('task', c)).toContain('fails CLOSED'); + expect(unresolvedPostureLogLine('task', 'find', 'u1', c)).toContain('fail-closed, #3545'); + } + }); +}); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 0740628d3b..4478e1917b 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -230,16 +230,18 @@ interface ObjectSecurityMeta { */ unresolved: boolean; /** - * [#10401] WHICH of the two conditions behind {@link ObjectSecurityMeta.unresolved} - * this is — the object exists only as an unpublished draft, or its declaration + * [#10401 / #10424] WHICH of the conditions behind + * {@link ObjectSecurityMeta.unresolved} this is — the object exists only as an + * unpublished draft, the metadata STORE could not answer, or its declaration * genuinely cannot be read. Meaningful only when `unresolved` is `true`; absent * (and therefore `'unknown'`) on every resolved posture. * - * ⛔ Explanation only. Nothing may branch an ACCESS DECISION on it: both causes - * deny, identically and fail-closed, and the probe that produces it is - * best-effort by design (see `probeUnpublishedDraft`). Its whole job is to stop - * the refusal naming a remedy — "change a sharing rule" — that cannot fix - * either condition. + * ⛔ Explanation only. Nothing may branch an ACCESS DECISION on it: every cause + * denies, identically and fail-closed, and the probes that produce it are + * best-effort by design (see {@link SecurityPlugin.resolveUnresolvedCause}). + * Its whole job is to stop the refusal naming a remedy — "change a sharing + * rule", or "go fix your declaration" mid-outage — that cannot fix the + * condition at hand. */ unresolvedCause?: UnresolvedPostureCause; } @@ -5584,14 +5586,93 @@ export class SecurityPlugin implements Plugin { fieldRequiredPermissions, fieldMaskingRules, unresolved: !obj, - // [#10401] Explanation only, and only on the path that is already - // refusing. Both causes deny identically — see the field's TSDoc. - ...(obj ? {} : { unresolvedCause: await this.probeUnpublishedDraft(object) }), + // [#10401 / #10424] Explanation only, and only on the path that is already + // refusing. All causes deny identically — see the field's TSDoc. + ...(obj ? {} : { unresolvedCause: await this.resolveUnresolvedCause(object) }), }; if (obj) this.objectSecurityMetaCache.set(object, meta); return meta; } + /** + * [#10424] Which condition put this object on the unresolved path — the + * EXPLANATION only. Order matters, and it is the honest order rather than the + * cheap one. + * + * The outage probe runs FIRST and wins outright. When the metadata service + * reports its own read as degraded, "a draft declaration exists but no + * published one" is a claim we cannot support: the draft probe can still see + * a draft row through ObjectQL while the metadata store is exactly the thing + * that could not tell us whether a PUBLISHED one exists. Asserting + * `'unpublished_draft'` there would state as fact the half of the sentence + * the outage made unknowable — the same species of error as the defect this + * fixes, pointed the other way. + * + * Every leg that cannot reach a positive verdict lands on `'unknown'`, whose + * wording covers all cases. + */ + private async resolveUnresolvedCause(object: string): Promise { + if (await this.probeMetadataOutage(object)) return 'metadata_unavailable'; + return await this.probeUnpublishedDraft(object); + } + + /** + * [#10424 / #5840] Best-effort: did the metadata read fail because the STORE + * could not answer, rather than because nothing is declared? + * + * `IMetadataService.get` is ambiguous by construction — its own TSDoc says + * `undefined` means "not found" *and* "every loader that could hold it + * failed", and directs callers to `getDiagnosed` "wherever the difference + * could change a decision". `MetadataManager` defines `get` as + * `(await getDiagnosed(…)).data`, so the verdict is computed and discarded on + * the way here. This asks for it. + * + * ## Why this is a SEPARATE probe and not the read itself + * + * The obvious shape — swap `metadata.get` for `metadata.getDiagnosed` and use + * `.data` — was rejected deliberately. `obj` is what `unresolved` is computed + * from, so it is the input to a #3545 fail-closed DENY: changing which member + * produces it puts an externally observable accept/reject decision at the + * mercy of every third-party `IMetadataService` whose optional `getDiagnosed` + * disagrees with its own `get`, or throws where `get` would have succeeded. + * An object that resolves today would then be refused. This card is an + * explanation change and has to stay one, so the resolving read is untouched + * byte for byte and the verdict is asked for separately, on the path that is + * already refusing. + * + * The cost is one extra read, paid only on a request that is already denied — + * the same bargain `probeUnpublishedDraft` documents, beside which this sits. + * + * ## The fail-safe direction, which is the whole point + * + * `getDiagnosed` is OPTIONAL (`getDiagnosed?`), and the contract is explicit + * that "implementations that predate it simply cannot report the distinction, + * and a consumer that probes for it must keep reading `get` when it is + * absent". So a service without it returns `false` here and the caller + * reports `'unknown'` — NOT `'metadata_unavailable'`. Publishing "I don't + * know" as "the store is down" would manufacture an incident out of a missing + * capability, and the operator would go read healthy dashboards looking for + * an outage that the platform invented. Same for a `getDiagnosed` that throws + * or answers with a non-boolean `degraded`: no verdict is not a verdict. + * `'metadata_unavailable'` is asserted on a positive `degraded === true` and + * on nothing else. + * + * Like the draft probe, this can never turn a resolved posture into a denial + * nor a refusal into a grant — it is read after the deny decision is made. + */ + private async probeMetadataOutage(object: string): Promise { + const service = this.metadata as + | { getDiagnosed?: (type: string, name: string) => Promise<{ degraded?: unknown }> } + | undefined; + if (typeof service?.getDiagnosed !== 'function') return false; + try { + const diagnosed = await service.getDiagnosed('object', object); + return diagnosed?.degraded === true; + } catch { + return false; + } + } + /** * [#10401] Best-effort: is this unresolvable object simply an UNPUBLISHED * draft? diff --git a/packages/plugins/plugin-security/src/unresolved-posture.ts b/packages/plugins/plugin-security/src/unresolved-posture.ts index 4f2fb54e98..c168612a3d 100644 --- a/packages/plugins/plugin-security/src/unresolved-posture.ts +++ b/packages/plugins/plugin-security/src/unresolved-posture.ts @@ -27,6 +27,18 @@ * • the declaration genuinely cannot be read — the remedy is to check that * the object is declared at all, or to look at a metadata-store outage. * + * [#10424] That second bullet was still doing two jobs, and the half it did + * badly is now its own cause. "Check that the object is declared" is correct + * advice for an absent declaration and actively wrong during a metadata-store + * OUTAGE, where the declaration is fine and the store is not. The distinction + * was already being computed and thrown away: `MetadataManager` defines `get` + * as `(await getDiagnosed(…)).data`, so the `degraded` verdict that separates + * a MISS from an OUTAGE existed and was discarded one frame below this module + * (#5840). `'metadata_unavailable'` is what reading it buys — asserted only on + * a positive `degraded: true`, never inferred from a service that cannot + * answer. The DENY is untouched in all three cases: #3545 fail-closed, same + * `PermissionDeniedError`, same `PERMISSION_DENIED`, same 403. + * * …and because it described an internal *security* step, every reader — human * and model alike — read it as a permissions problem and went looking for a * sharing rule to change. Measured downstream (objectstack-ai/cloud#1481): an @@ -53,16 +65,49 @@ */ /** - * Which of the two conditions behind an unresolved posture this is. - * - * `'unknown'` is the honest default and the fail-safe: the draft probe that - * distinguishes the two is best-effort (see `probeUnpublishedDraft` in - * `security-plugin.ts`), so a deployment with no queryable `sys_metadata` — or - * any probe failure at all — reports `'unknown'` and gets the wording that - * covers both cases. The discriminator may never turn a *real* unpublished - * object into a claim the platform cannot support. + * Which of the conditions behind an unresolved posture this is. + * + * `'unknown'` is the honest default and the fail-safe: every probe that + * distinguishes the others is best-effort (see `probeUnpublishedDraft` and + * `probeMetadataOutage` in `security-plugin.ts`), so a deployment with no + * queryable `sys_metadata`, a metadata service that does not implement the + * optional `getDiagnosed` (#5840), or any probe failure at all reports + * `'unknown'` and gets the wording that covers every case. The discriminator + * may never turn a *real* unpublished object — or a *real* outage — into a + * claim the platform cannot support. + * + * [#10424] `'metadata_unavailable'` is the third member, and it is the one the + * fail-safe rule bites hardest on. `IMetadataService.get` is ambiguous by + * construction — its own TSDoc says `undefined` means "not found" *and* "every + * loader that could hold it failed" — so an outage and an absent declaration + * arrived here as the same `unresolved: true` wearing the same sentence, and + * that sentence told the reader to go check the declaration. `getDiagnosed` + * already computes the `degraded` verdict that separates them; this member is + * what consuming it buys. It is asserted ONLY when the service positively + * reports `degraded: true`. A service that cannot report the distinction is + * NOT an outage — "I don't know" must never be published as "the store is + * down", which would be manufacturing false information in the opposite + * direction from the defect this fixes. + */ +export type UnresolvedPostureCause = + | 'unpublished_draft' + | 'metadata_unavailable' + | 'unknown'; + +/** + * Compile-time exhaustiveness for the switches below, with a RUNTIME fallback + * to the `'unknown'` wording rather than a throw. + * + * Both halves are deliberate. Adding a fourth cause without wording it is a + * type error at build time (the assignment to `never` fails). But these + * functions run on the middleware's already-refusing path, where an + * unhandled throw would turn a correct `403 PERMISSION_DENIED` into a `500` — + * so the runtime half degrades to the sentence that covers every case, which + * is exactly this module's documented fail-safe. */ -export type UnresolvedPostureCause = 'unpublished_draft' | 'unknown'; +function assertCauseExhausted(cause: never): void { + void cause; +} /** * The remedy half — the sentence that tells the reader what to actually do, and @@ -70,12 +115,29 @@ export type UnresolvedPostureCause = 'unpublished_draft' | 'unknown'; * cannot diverge from the diagnosis. */ export function unresolvedPostureRemedy(cause: UnresolvedPostureCause): string { - return cause === 'unpublished_draft' - ? 'Publish the object to make it queryable. This is NOT a permissions problem — no sharing rule, ' - + 'visibility setting or permission-set change grants access to an unpublished object.' - : 'Check that the object is declared and published on this runtime. This is NOT a permissions ' - + 'problem — no sharing rule, visibility setting or permission-set change grants access to an ' - + 'object whose declaration cannot be read.'; + const unknown = + 'Check that the object is declared and published on this runtime. This is NOT a permissions ' + + 'problem — no sharing rule, visibility setting or permission-set change grants access to an ' + + 'object whose declaration cannot be read.'; + switch (cause) { + case 'unpublished_draft': + return 'Publish the object to make it queryable. This is NOT a permissions problem — no sharing rule, ' + + 'visibility setting or permission-set change grants access to an unpublished object.'; + // [#10424] Points at the STORE, not at the declaration. The old sentence + // sent an operator mid-outage to re-check a declaration that was fine, and + // that is the whole defect: a refusal naming the wrong remedy is worse than + // a terse one, because the next reader's diagnosis is built on it. + case 'metadata_unavailable': + return 'Check the metadata store — the object may well be declared and published, and simply ' + + 'unreadable right now. Do NOT change the declaration on the strength of this message. This is ' + + 'NOT a permissions problem either — no sharing rule, visibility setting or permission-set ' + + 'change restores a read the metadata store could not serve.'; + case 'unknown': + return unknown; + default: + assertCauseExhausted(cause); + return unknown; + } } /** @@ -91,12 +153,29 @@ export function unresolvedPostureDenialMessage( cause: UnresolvedPostureCause, ): string { const remedy = unresolvedPostureRemedy(cause); - return cause === 'unpublished_draft' - ? `[Security] Access denied: object '${object}' is not published — a draft declaration exists but ` - + `no published one, so there is no security posture to authorize '${operation}' against. ${remedy}` - : `[Security] Access denied: the security posture of object '${object}' could not be resolved for ` - + `operation '${operation}' — neither the live schema nor the metadata service returned a ` - + `declaration for it, so access fails closed. ${remedy}`; + const unknown = + `[Security] Access denied: the security posture of object '${object}' could not be resolved for ` + + `operation '${operation}' — neither the live schema nor the metadata service returned a ` + + `declaration for it, so access fails closed. ${remedy}`; + switch (cause) { + case 'unpublished_draft': + return `[Security] Access denied: object '${object}' is not published — a draft declaration exists but ` + + `no published one, so there is no security posture to authorize '${operation}' against. ${remedy}`; + // [#10424] Keeps the SAME opening clause as the `'unknown'` branch, verbatim + // through "could not be resolved for operation '…'", so anything pinning + // that substring keeps matching; only what follows the em dash differs. The + // divergence is the factual half: we are not claiming a declaration is + // absent, because a degraded read cannot support that claim. + case 'metadata_unavailable': + return `[Security] Access denied: the security posture of object '${object}' could not be resolved for ` + + `operation '${operation}' — the metadata service reported its own read as DEGRADED, so whether a ` + + `declaration exists is UNKNOWN rather than answered, and access fails closed. ${remedy}`; + case 'unknown': + return unknown; + default: + assertCauseExhausted(cause); + return unknown; + } } /** @@ -112,13 +191,26 @@ export function unresolvedPostureExplainDetail( cause: UnresolvedPostureCause, ): string { const remedy = unresolvedPostureRemedy(cause); - return cause === 'unpublished_draft' - ? `'${object}' is not published — a draft declaration exists but no published one, so its 'private' ` - + `flag and required-capability contract are unknown and access fails CLOSED rather than ` - + `defaulting to public/uncontracted (#3545). ${remedy}` - : `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). ${remedy}`; + const unknown = + `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). ${remedy}`; + switch (cause) { + case 'unpublished_draft': + return `'${object}' is not published — a draft declaration exists but no published one, so its 'private' ` + + `flag and required-capability contract are unknown and access fails CLOSED rather than ` + + `defaulting to public/uncontracted (#3545). ${remedy}`; + case 'metadata_unavailable': + return `The security posture of '${object}' could not be resolved: the metadata service reported its ` + + `own read as DEGRADED, so this is a metadata-store OUTAGE and not necessarily an absent ` + + `declaration. Its 'private' flag and required-capability contract are unknown, so access fails ` + + `CLOSED rather than defaulting to public/uncontracted (#3545). ${remedy}`; + case 'unknown': + return unknown; + default: + assertCauseExhausted(cause); + return unknown; + } } /** @@ -132,9 +224,26 @@ export function unresolvedPostureLogLine( userId: string, cause: UnresolvedPostureCause, ): string { - return cause === 'unpublished_draft' - ? `[security] object '${object}' has a DRAFT declaration and no published one — denying operation ` - + `'${operation}' (user ${userId}) with the unpublished-object refusal (fail-closed, #3545/#10401)` - : `[security] object security posture unresolvable for operation '${operation}' on ` - + `object '${object}' (user ${userId}) — denying request (fail-closed, #3545)`; + const unknown = + `[security] object security posture unresolvable for operation '${operation}' on ` + + `object '${object}' (user ${userId}) — denying request (fail-closed, #3545)`; + switch (cause) { + case 'unpublished_draft': + return `[security] object '${object}' has a DRAFT declaration and no published one — denying operation ` + + `'${operation}' (user ${userId}) with the unpublished-object refusal (fail-closed, #3545/#10401)`; + // [#10424] The operator-facing half of the split, and the reason the log + // line is worth changing at all: a metadata-store outage is an INCIDENT and + // a query against a missing object is routine, and they were the same line. + // `DEGRADED` and `OUTAGE` are here to be grep-able. + case 'metadata_unavailable': + return `[security] object security posture unresolvable for operation '${operation}' on ` + + `object '${object}' (user ${userId}) — the metadata service reported a DEGRADED read, i.e. a ` + + `metadata-store OUTAGE rather than an absent declaration — denying request ` + + `(fail-closed, #3545/#10424)`; + case 'unknown': + return unknown; + default: + assertCauseExhausted(cause); + return unknown; + } }