From bf297e4ea7aa272f091ea85ece2a51013cd0b858 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 14:02:27 +0000 Subject: [PATCH 1/7] fix(core,rest,services): fail loud when a permission-store read fails (#13279) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolveAuthzContext`'s `tryFind` answered a THROWN read the same way it answered an EMPTY one — `[]` — so a permission-store outage resolved as an authenticated principal holding zero capabilities and the package door answered a 403 byte-identical to a genuine capability denial. `tryFind` now distinguishes the two facts and raises `AuthzStoreUnavailableError` (existing ADR-0112 code `SERVICE_UNAVAILABLE`, status 503) when a read is issued and throws. The `!ql` guard is untouched: an absent engine is not a failed read. Making the resolver loud is necessary but not sufficient, which was measured rather than assumed: six of the eight production transports wrap the call in a fail-closed `catch`, and with those untouched the door answered 401 — the outage had merely changed disguises. Each now re-raises the branded error and keeps its prior behaviour for every other fault. Maintainer ruling 2026-08-30, verbatim: 第一批其余同意 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .changeset/authz-read-failure-fails-loud.md | 59 +++++ .../src/marketplace-install-local-plugin.ts | 7 +- .../security/authz-store-unavailable.test.ts | 248 ++++++++++++++++++ .../src/security/authz-store-unavailable.ts | 173 ++++++++++++ packages/core/src/security/index.ts | 12 + .../src/security/resolve-authz-context.ts | 52 +++- .../plugin-sharing/src/sharing-plugin.ts | 8 +- .../rest/src/execctx-consumer-census.test.ts | 15 +- ...ge-door-execctx-fault-reachability.test.ts | 175 +++++++++--- packages/rest/src/package-routes.ts | 9 +- packages/rest/src/rest-server.ts | 47 ++-- .../service-datasource/src/admin-routes.ts | 11 +- .../src/settings-service-plugin.ts | 8 +- .../src/storage-service-plugin.ts | 8 +- 14 files changed, 755 insertions(+), 77 deletions(-) create mode 100644 .changeset/authz-read-failure-fails-loud.md create mode 100644 packages/core/src/security/authz-store-unavailable.test.ts create mode 100644 packages/core/src/security/authz-store-unavailable.ts diff --git a/.changeset/authz-read-failure-fails-loud.md b/.changeset/authz-read-failure-fails-loud.md new file mode 100644 index 0000000000..c539015600 --- /dev/null +++ b/.changeset/authz-read-failure-fails-loud.md @@ -0,0 +1,59 @@ +--- +"@objectstack/core": minor +"@objectstack/rest": patch +"@objectstack/service-datasource": patch +"@objectstack/service-settings": patch +"@objectstack/service-storage": patch +"@objectstack/plugin-sharing": patch +"@objectstack/cloud-connection": patch +--- + +fix(core,rest,services)!: a permission-store read failure now fails LOUD instead of resolving as an authenticated caller holding zero capabilities (#13279) + +**BREAKING** runtime behaviour change on the shared authorization resolver, +shipped as `minor` under the repo's launch-window convention. + +`resolveAuthzContext`'s per-read helper `tryFind` answered a THROWN read exactly +the way it answered an EMPTY one: `[]`. So an outage of the permission store +resolved as a well-formed context for an authenticated principal holding no +capabilities, and the package-management door answered +`403 FORBIDDEN` — "Reading packages requires the `studio.access` or +`setup.access` capability." That answer was measured byte-identical +(`JSON.stringify` equal, against a control that separates two answers which do +differ) to what a caller who genuinely holds nothing receives. An administrator +was told they lack a capability, during an outage of the store that holds the +capability. + +Maintainer ruling 2026-08-30, verbatim 「第一批其余同意」: `tryFind` 区分「无行」 +与「读失败」,读失败 fail-loud —— 权限库不可达时不再解析为「已认证零能力」,而是 +响亮拒绝(与真实能力拒绝的 403 可区分)。 + +**What changed.** A permission-store read that is issued and throws now raises +`AuthzStoreUnavailableError`, which carries the EXISTING ADR-0112 wire code +`SERVICE_UNAVAILABLE` and status `503`. No code is added to the closed wire +vocabulary and no response envelope gains or loses a key — only which declared +code an outage selects. Doors that map thrown errors through +`resolveThrownHttpError` answer 503 with no per-door change. + +**What did NOT change**, and is pinned: + +- A reachable, genuinely EMPTY store still resolves to zero capabilities. +- A genuine capability denial still answers `403 FORBIDDEN` with its message. +- An ABSENT engine (`ql` unwired) still resolves to an empty-but-valid envelope + — "no engine" is not a failed read, and embedders without a data plane are + unaffected. +- Anonymous requests never reach the store, so an outage cannot make them loud. + +**All-transport, not just REST.** Every transport authorizing through +`resolveAuthzContext` inherits this. Six of the eight production transports +wrapped the call in a fail-closed `catch` that would have re-silenced the +outage — measured, not assumed: with the resolver loud but the nets untouched, +the package door answered `401`, i.e. the outage merely changed disguises. Those +`catch` blocks now re-raise via `isAuthzStoreUnavailableError` and keep their +previous behaviour for every other fault. The transport set is rebuilt from +source and audited for set equality on every test run, so a transport added +later cannot inherit the old silence unnoticed. + +Callers that treat any throw from `resolveAuthzContext` as "anonymous" should +re-raise `isAuthzStoreUnavailableError(err)` instead: degrading it restores the +disguise this removes. diff --git a/packages/cloud-connection/src/marketplace-install-local-plugin.ts b/packages/cloud-connection/src/marketplace-install-local-plugin.ts index 78a8d4be33..57388a242e 100644 --- a/packages/cloud-connection/src/marketplace-install-local-plugin.ts +++ b/packages/cloud-connection/src/marketplace-install-local-plugin.ts @@ -46,7 +46,7 @@ */ import type { Plugin, PluginContext } from '@objectstack/core'; -import { resolveAuthzContext } from '@objectstack/core'; +import { resolveAuthzContext, isAuthzStoreUnavailableError } from '@objectstack/core'; import { resolveTenancyPosture, collectGlobalUniques, @@ -1659,7 +1659,10 @@ export class MarketplaceInstallLocalPlugin implements Plugin { userId: String(authz.userId), systemPermissions: Array.isArray(authz.systemPermissions) ? authz.systemPermissions : [], }; - } catch { + } catch (err) { + // [#13279] `null` here means "nobody is authenticated", which is + // not what a permission-store outage established. Re-raised. + if (isAuthzStoreUnavailableError(err)) throw err; return null; } }; diff --git a/packages/core/src/security/authz-store-unavailable.test.ts b/packages/core/src/security/authz-store-unavailable.test.ts new file mode 100644 index 0000000000..38a0ed59e8 --- /dev/null +++ b/packages/core/src/security/authz-store-unavailable.test.ts @@ -0,0 +1,248 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13279] A permission-store OUTAGE must not be answerable as a capability + * denial — at the resolver, and at every transport that authorizes through it. + * + * Maintainer ruling, 2026-08-30, verbatim 「第一批其余同意」: + * + * > `tryFind` 区分「无行」与「读失败」,读失败 fail-loud —— 权限库不可达时不再 + * > 解析为「已认证零能力」,而是响亮拒绝(与真实能力拒绝的 403 可区分)。 + * > ⛔ 全部经 `resolveAuthzContext` 授权的 transport 都继承此变更,派发令必须 + * > 要求全 transport 回归(REST + 其余),不得只测 REST 门。 + * + * ## Reading discipline + * + * Every loud assertion here is paired with its INNOCENT TWIN — the same wiring + * with the store reachable and genuinely empty. Without the twin, "the outage + * throws" is satisfied by a resolver that throws at everything, which would be + * a worse defect than the one being fixed: a deployment where nobody holds a + * capability yet would stop resolving at all. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { join, relative, sep } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { resolveAuthzContext, resolveUserAuthzGrants } from './resolve-authz-context.js'; +import { + AuthzStoreUnavailableError, + isAuthzStoreUnavailableError, + rethrowAuthzStoreUnavailable, + AUTHZ_STORE_UNAVAILABLE_CODE, + AUTHZ_STORE_UNAVAILABLE_STATUS, +} from './authz-store-unavailable.js'; + +const USER = 'u_admin'; +const SESSION = { getSession: async () => ({ user: { id: USER } }) }; + +/** The permission store, unreachable — every read throws, as a driver outage does. */ +const qlDown = () => ({ find: async () => { throw new Error('permission store unreachable'); } }); +/** The INNOCENT TWIN: reachable, and genuinely holding no rows for this user. */ +const qlEmpty = () => ({ find: async () => [] }); +/** A store that actually grants something, so "resolves" is read against a real grant. */ +const qlHealthy = () => ({ + find: async (object: string) => { + if (object === 'sys_user_permission_set') return [{ permission_set_id: 'ps' }]; + if (object === 'sys_permission_set') { + return [{ id: 'ps', name: 'pkg_admin', system_permissions: ['studio.access'] }]; + } + return []; + }, +}); + +const settle = async (p: Promise) => + p.then((v) => ({ ok: true as const, v }), (e) => ({ ok: false as const, e })); + +// --------------------------------------------------------------------------- +// 1. The resolver distinguishes a FAILED read from an EMPTY one. +// --------------------------------------------------------------------------- + +describe('[#13279] resolveAuthzContext — an outage is loud, an empty store is not', () => { + it('a permission-store OUTAGE refuses, with the branded error', async () => { + const r = await settle(resolveAuthzContext({ ql: qlDown(), headers: {}, ...SESSION })); + expect(r.ok).toBe(false); + expect(isAuthzStoreUnavailableError((r as any).e)).toBe(true); + // The wire vocabulary an outage selects — an EXISTING ADR-0112 member. + expect((r as any).e.code).toBe(AUTHZ_STORE_UNAVAILABLE_CODE); + expect((r as any).e.status).toBe(AUTHZ_STORE_UNAVAILABLE_STATUS); + // Names the table, so an operator is told WHAT was unreachable. + expect(typeof (r as any).e.object).toBe('string'); + expect((r as any).e.object.length).toBeGreaterThan(0); + }); + + it('⭐ THE TWIN: a reachable, genuinely EMPTY store still resolves to zero capabilities', async () => { + // The assertion that keeps the fix honest. If this ever throws, the repair + // has stopped distinguishing the two facts and merely moved the lie. + const ctx = await resolveAuthzContext({ ql: qlEmpty(), headers: {}, ...SESSION }); + expect(ctx.userId).toBe(USER); + expect(ctx.systemPermissions).toEqual([]); + }); + + it('⭐ CONTROL: a healthy store still resolves the capability it grants', async () => { + const ctx = await resolveAuthzContext({ ql: qlHealthy(), headers: {}, ...SESSION }); + expect(ctx.userId).toBe(USER); + expect(ctx.systemPermissions).toContain('studio.access'); + }); + + it('an ABSENT engine is not a failed read — it still resolves, as it always did', async () => { + // `tryFind`'s `!ql` guard. An embedder that never wired a data plane must + // keep resolving; only a read that was ISSUED and THREW is loud. + const ctx = await resolveAuthzContext({ ql: undefined, headers: {}, ...SESSION }); + expect(ctx.userId).toBe(USER); + expect(ctx.systemPermissions).toEqual([]); + const noFind = await resolveAuthzContext({ ql: {} as any, headers: {}, ...SESSION }); + expect(noFind.userId).toBe(USER); + }); + + it('an ANONYMOUS request never reaches the store, so an outage cannot make it loud', async () => { + const ctx = await resolveAuthzContext({ ql: qlDown(), headers: {} }); + expect(ctx.userId).toBeUndefined(); + expect(ctx.systemPermissions).toEqual([]); + }); +}); + +describe('[#13279] resolveUserAuthzGrants inherits the distinction', () => { + it('an OUTAGE refuses rather than reporting an empty grant set', async () => { + const r = await settle(resolveUserAuthzGrants(qlDown(), USER)); + expect(r.ok).toBe(false); + expect(isAuthzStoreUnavailableError((r as any).e)).toBe(true); + }); + + it('⭐ THE TWIN: an empty store yields an empty-but-valid envelope', async () => { + const grants = await resolveUserAuthzGrants(qlEmpty(), USER); + expect(grants.systemPermissions).toEqual([]); + expect(grants.org_user_ids).toEqual([USER]); + }); +}); + +// --------------------------------------------------------------------------- +// 2. The brand, and the guard the transports use. +// --------------------------------------------------------------------------- + +describe('[#13279] the brand survives what `instanceof` does not', () => { + it('recognises a genuine instance', () => { + expect(isAuthzStoreUnavailableError(new AuthzStoreUnavailableError('sys_member'))).toBe(true); + }); + + it('⭐ recognises a DUPLICATE class — the monorepo hazard `instanceof` fails', () => { + // A second copy of the module (src under a vitest alias vs dist under the + // published `exports`) produces a structurally identical error from a + // DIFFERENT class object. `instanceof` answers false for it, which would + // silently restore the quiet 403 this card removes. + class DuplicateCopy extends Error { + readonly __objectstackAuthzStoreUnavailable = true as const; + readonly code = AUTHZ_STORE_UNAVAILABLE_CODE; + readonly status = AUTHZ_STORE_UNAVAILABLE_STATUS; + } + const twin = new DuplicateCopy('from another realm'); + expect(twin instanceof AuthzStoreUnavailableError).toBe(false); // the hazard, shown + expect(isAuthzStoreUnavailableError(twin)).toBe(true); // the brand, holding + }); + + it('does NOT claim unrelated failures', () => { + for (const other of [new Error('boom'), null, undefined, 'string', 42, {}, { code: 'FORBIDDEN' }]) { + expect(isAuthzStoreUnavailableError(other)).toBe(false); + } + }); + + it('`rethrowAuthzStoreUnavailable` re-raises the outage and swallows everything else', () => { + expect(() => rethrowAuthzStoreUnavailable(new AuthzStoreUnavailableError('sys_member'))).toThrow(); + expect(rethrowAuthzStoreUnavailable(new Error('unrelated'))).toBeUndefined(); + expect(rethrowAuthzStoreUnavailable(undefined)).toBeUndefined(); + }); +}); + +// --------------------------------------------------------------------------- +// 3. ⭐ ALL-TRANSPORT LEDGER — the ruling's "not just the REST door". +// +// The enumeration is REBUILT FROM SOURCE on every run and audited for SET +// EQUALITY against the ledger, so a transport added later cannot inherit the +// old silence unnoticed: an unlisted call site fails this suite until it is +// classified. A curated list alone would answer "the doors I remembered". +// --------------------------------------------------------------------------- + +const REPO_ROOT = fileURLToPath(new URL('../../../..', import.meta.url)); + +/** + * How each production transport lets the loud failure reach its door. + * + * - `guarded` — the call sits inside a fail-closed `catch` that would have + * re-silenced the outage, so that `catch` now re-raises it explicitly. + * MEASURED, not assumed: with `tryFind` loud but the nets untouched, the + * REST package door answered 401 instead of 403 — the outage had merely + * changed disguises. + * - `propagates` — nothing between the call and the door swallows, so the + * throw reaches the transport's error mapping unaided. + */ +const TRANSPORT_LEDGER: Record = { + 'packages/rest/src/rest-server.ts': 'guarded', + 'packages/runtime/src/security/resolve-execution-context.ts': 'propagates', + 'packages/mcp/src/plugin.ts': 'propagates', + 'packages/services/service-datasource/src/admin-routes.ts': 'guarded', + 'packages/services/service-settings/src/settings-service-plugin.ts': 'guarded', + 'packages/services/service-storage/src/storage-service-plugin.ts': 'guarded', + 'packages/plugins/plugin-sharing/src/sharing-plugin.ts': 'guarded', + 'packages/cloud-connection/src/marketplace-install-local-plugin.ts': 'guarded', +}; + +/** Test scaffolding is not a transport — it drives the resolver, it does not authorize for anyone. */ +const isScaffolding = (rel: string) => + /\.test\.ts$/.test(rel) || /\.testkit\.ts$/.test(rel) + || /\.fixtures?\.ts$/.test(rel) || rel.includes(`${sep}dogfood${sep}`); + +function walk(dir: string, out: string[] = []): string[] { + for (const entry of readdirSync(dir)) { + if (entry === 'node_modules' || entry === 'dist' || entry === '.turbo') continue; + const full = join(dir, entry); + if (statSync(full).isDirectory()) walk(full, out); + else if (full.endsWith('.ts')) out.push(full); + } + return out; +} + +/** Every PRODUCTION file that calls `resolveAuthzContext`, rebuilt from source. */ +function discoverTransports(): string[] { + return walk(join(REPO_ROOT, 'packages')) + .filter((f) => readFileSync(f, 'utf8').includes('resolveAuthzContext({')) + .map((f) => relative(REPO_ROOT, f).split(sep).join('/')) + .filter((rel) => !isScaffolding(rel.split('/').join(sep))) + .sort(); +} + +describe('[#13279] every transport that authorizes through resolveAuthzContext', () => { + it('CONTROL: the scanner finds transports at all, and finds THIS repo', () => { + // Without this, a broken walk would return [] and the set-equality audit + // below would be comparing two empty sets and passing. + const found = discoverTransports(); + expect(found.length).toBeGreaterThanOrEqual(8); + expect(found).toContain('packages/rest/src/rest-server.ts'); + }); + + it('⭐ SET EQUALITY: the ledger names exactly the transports source contains', () => { + // A NEW transport is red here until it is classified — which is the whole + // point: the ruling is about every transport, including the ones written + // after it. + expect(discoverTransports()).toEqual(Object.keys(TRANSPORT_LEDGER).sort()); + }); + + it.each(Object.entries(TRANSPORT_LEDGER))( + '%s (%s) — a fail-closed catch re-raises the outage instead of degrading it', + (rel, disposition) => { + const src = readFileSync(join(REPO_ROOT, rel), 'utf8'); + if (disposition === 'guarded') { + // The guard must be IMPORTED (so it is the shared predicate, not a + // local re-spelling that can drift) and USED. + expect(src).toMatch(/from '@objectstack\/core'/); + expect( + src.includes('isAuthzStoreUnavailableError') || src.includes('rethrowAuthzStoreUnavailable'), + ).toBe(true); + } else { + // `propagates` is a CLAIM about this file, so it is checked rather than + // trusted: a bare `catch {` around the resolver call would silently + // reintroduce the swallow this ledger exists to track. + expect(src).not.toMatch(/resolveAuthzContext\(\{[\s\S]{0,600}?\n\s*\} catch \{\s*\n\s*return (undefined|null|\{\})/); + } + }, + ); +}); diff --git a/packages/core/src/security/authz-store-unavailable.ts b/packages/core/src/security/authz-store-unavailable.ts new file mode 100644 index 0000000000..41b7535a76 --- /dev/null +++ b/packages/core/src/security/authz-store-unavailable.ts @@ -0,0 +1,173 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13279] The LOUD failure an unreachable permission store raises. + * + * ## The defect this exists to end + * + * `resolveAuthzContext`'s per-read helper `tryFind` used to answer a THROWN + * read the same way it answers an EMPTY one: `[]`. So a permission-store + * outage resolved as a well-formed context for an AUTHENTICATED principal + * holding ZERO capabilities, and the package-management door answered + * `403 FORBIDDEN` — "Reading packages requires the `studio.access` or + * `setup.access` capability." That answer was measured BYTE-IDENTICAL + * (`JSON.stringify` equal, against a positive control that separates two + * answers which differ) to the answer a caller who genuinely holds nothing + * gets. An administrator was told they lack a capability, during an outage of + * the store that holds the capability. + * + * The resolver was asserting a fact it did not have. "No rows came back" + * and "the read failed" are different facts, and only one of them licenses + * the sentence "this user holds nothing". + * + * ## Maintainer ruling, 2026-08-30, verbatim 「第一批其余同意」 + * + * > `tryFind` 区分「无行」与「读失败」,读失败 fail-loud —— 权限库不可达时 + * > 不再解析为「已认证零能力」,而是响亮拒绝(与真实能力拒绝的 403 可区分), + * > 让宕机不再伪装成一次逐字节相同的能力否决。 + * + * The ruling fixes the DIRECTION and leaves the spelling to the implementation. + * + * ## Why a THROW, and not a field on the envelope + * + * The alternative was a discriminator field on `ResolvedAuthzContext` — the + * shape `authRefusal` already has. That was rejected on a MEASUREMENT, not a + * preference: `authRefusal` has existed since #8287 and, outside this module + * and its own unit test, has **zero** consumers anywhere in the repo. A + * diagnostic field on this envelope is demonstrably not read by any door. Every + * transport reads `userId` and `systemPermissions`; a new sibling field would + * have to be taught to eight separate call sites before it made a single door + * louder, and would answer the old quiet 403 at every site that was missed. + * + * A field is quiet by default and must be deliberately made loud. A throw is + * loud by default and must be deliberately silenced. On a security surface + * whose whole defect is a silence, the default is the entire decision. + * + * It is also the idiom this platform already uses for unresolvable authority: + * `packages/mcp`'s stdio entry throws and refuses to start rather than run with + * an authority it could not resolve. + * + * ## Why `SERVICE_UNAVAILABLE` / 503, and why that is not a new wire shape + * + * `SERVICE_UNAVAILABLE` is an EXISTING member of the closed ADR-0112 wire + * vocabulary (`StandardErrorCode`, `packages/spec/src/api/errors.zod.ts`), and + * `HttpStatusErrorCodeMap` already maps it to 503 — "service exists but is + * temporarily down". Nothing is added to the vocabulary and no envelope gains + * or loses a key: a door that already renders `{ code, message }` renders this + * one the same way. What changes is WHICH declared code an outage selects — + * from the caller's `FORBIDDEN` to the operator's `SERVICE_UNAVAILABLE`. + * + * That is the ruling's own test, stated on the wire: 503 is not 403, so an + * outage is no longer answerable as a capability denial. + * + * ## Recognise by BRAND, never by `instanceof` + * + * {@link isAuthzStoreUnavailableError} tests a own-property brand rather than + * `instanceof`. This error crosses package boundaries (`@objectstack/core` → + * rest / runtime / mcp / services / plugins) and a monorepo resolves the same + * module through more than one path (`src` under vitest aliases, `dist` under + * the published `exports`). Two copies of this class make `instanceof` answer + * FALSE for a genuine instance — which, here, silently restores the exact + * quiet 403 this module exists to remove. The brand survives duplication. + */ + +/** HTTP status an unreachable authorization store answers with. */ +export const AUTHZ_STORE_UNAVAILABLE_STATUS = 503 as const; + +/** + * Machine code — an EXISTING `StandardErrorCode` member (ADR-0112: SCREAMING). + * Deliberately NOT a new code: the wire vocabulary is closed. + */ +export const AUTHZ_STORE_UNAVAILABLE_CODE = 'SERVICE_UNAVAILABLE' as const; + +/** + * Human-facing message. States the OUTAGE, and says explicitly that no + * capability judgement was reached — so neither the caller nor the operator + * reads it as a permission verdict. + */ +export const AUTHZ_STORE_UNAVAILABLE_MESSAGE = + 'The authorization store could not be read, so this request\'s permissions were never determined. ' + + 'This is a server-side outage, not a permission denial.'; + +/** + * The own-property brand {@link isAuthzStoreUnavailableError} tests for. + * A string-keyed own property (not a `Symbol.for` registry key) so it survives + * `structuredClone`, and so a duplicated copy of this module still brands + * identically. + */ +const AUTHZ_STORE_UNAVAILABLE_BRAND = '__objectstackAuthzStoreUnavailable' as const; + +/** + * Raised when a permission-store read FAILED — never when it legitimately + * returned no rows. + * + * Carries the `object` whose read failed so an operator sees WHICH table was + * unreachable, and the originating error as `cause` so the driver's own + * diagnostic is not lost behind this one. + */ +export class AuthzStoreUnavailableError extends Error { + /** Brand — see the module doc on why this is not `instanceof`. */ + readonly [AUTHZ_STORE_UNAVAILABLE_BRAND] = true as const; + /** ADR-0112 wire code. */ + readonly code = AUTHZ_STORE_UNAVAILABLE_CODE; + /** HTTP status a transport should answer. */ + readonly status = AUTHZ_STORE_UNAVAILABLE_STATUS; + /** The object/table whose read failed (e.g. `sys_user_permission_set`). */ + readonly object: string; + /** The driver's originating failure, kept so its diagnostic is not lost. */ + readonly cause?: unknown; + + constructor(object: string, cause?: unknown) { + // `cause` is assigned manually — the ES2022 `ErrorOptions` constructor + // overload is unavailable at this package's compile target (ES2020). Same + // idiom as `packages/spec`'s `ConnectorUpstreamUnavailableError`. + super(`${AUTHZ_STORE_UNAVAILABLE_MESSAGE} (failed read: \`${object}\`)`); + this.name = 'AuthzStoreUnavailableError'; + this.object = object; + if (cause !== undefined) this.cause = cause; + } +} + +/** + * True when `err` is the loud authorization-store failure above. + * + * The predicate every transport uses to tell "the store was unreachable" apart + * from every other throw, so a fail-closed `catch` can re-raise THIS one + * without loosening its handling of anything else. + */ +export function isAuthzStoreUnavailableError(err: unknown): err is AuthzStoreUnavailableError { + return ( + typeof err === 'object' + && err !== null + && (err as Record)[AUTHZ_STORE_UNAVAILABLE_BRAND] === true + ); +} + +/** + * The `.catch` argument every fail-closed seam between `resolveAuthzContext` + * and a door should use in place of `() => undefined`. + * + * Re-raises {@link AuthzStoreUnavailableError} and swallows everything else to + * `undefined`, so a seam keeps its existing fail-closed behaviour for every + * fault EXCEPT the one the 2026-08-30 ruling requires to stay loud. + * + * ## Why the seams need this at all + * + * Making `tryFind` throw is necessary but NOT sufficient, and that was + * MEASURED rather than assumed. Between the resolver and the package door sit + * three independent nets — `computeExecCtx`'s `try { … } catch { return + * undefined; }`, `resolvePackageRouteExecutionContext`'s `.catch(() => + * undefined)`, and `refusePackageRequest`'s own — and with the throw in place + * but the nets untouched, the door answered **401**: the outage had simply + * changed disguises, from "you hold no capability" (403) into "you are not + * authenticated" (401), which is byte-identical to a genuine anonymous caller. + * Distinguishable from a capability denial, yes — but still not LOUD, and now + * wearing the costume of a different card's defect. + * + * A blanket `catch` cannot tell a fault from a refusal, so each net has to be + * told once, in one shape. This is that shape. + */ +export function rethrowAuthzStoreUnavailable(err: unknown): undefined { + if (isAuthzStoreUnavailableError(err)) throw err; + return undefined; +} diff --git a/packages/core/src/security/index.ts b/packages/core/src/security/index.ts index 31b0d89615..e11750a8f3 100644 --- a/packages/core/src/security/index.ts +++ b/packages/core/src/security/index.ts @@ -85,6 +85,18 @@ export { type TenancyPostureSource, } from './api-key.js'; +// [#13279] The LOUD failure an unreachable permission store raises, and the +// brand predicate a fail-closed `catch` uses to re-raise it instead of +// degrading an outage into a capability denial. Ruled 2026-08-30. +export { + AuthzStoreUnavailableError, + isAuthzStoreUnavailableError, + rethrowAuthzStoreUnavailable, + AUTHZ_STORE_UNAVAILABLE_STATUS, + AUTHZ_STORE_UNAVAILABLE_CODE, + AUTHZ_STORE_UNAVAILABLE_MESSAGE, +} from './authz-store-unavailable.js'; + export { resolveAuthzContext, resolveUserAuthzGrants, diff --git a/packages/core/src/security/resolve-authz-context.ts b/packages/core/src/security/resolve-authz-context.ts index 712b2e9305..7a3f95f7fa 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -20,11 +20,19 @@ * implementation; both entry points are thin adapters that supply `ql` / * `getSession` their own way and delegate here. * - * Fail-closed: every read is defensive. Missing services / tables yield a - * partial context (even `{ positions: [], permissions: [] }`) — enforcement is the - * SecurityPlugin's job, never this resolver's. + * Fail-closed: every read is defensive about a MISSING service — an unwired + * `ql` yields a partial context (even `{ positions: [], permissions: [] }`) and + * enforcement stays the SecurityPlugin's job, never this resolver's. + * + * ⚠️ [#13279] A FAILED read is not a missing service, and since the 2026-08-30 + * ruling the two no longer share an answer. When a permission-store read is + * issued and THROWS, this resolver raises {@link AuthzStoreUnavailableError} + * instead of reporting an empty grant set: an outage must not be answerable as + * a capability denial. See `authz-store-unavailable.ts` for the full reasoning + * and for why the failure is a throw rather than a field on the envelope. */ +import { AuthzStoreUnavailableError } from './authz-store-unavailable.js'; import { mapMembershipRole, BUILTIN_IDENTITY_PLATFORM_ADMIN, @@ -147,14 +155,34 @@ async function tryFind( let rows = await ql.find(object, { where, limit, context } as any); if (rows && (rows as any).value) rows = (rows as any).value; return Array.isArray(rows) ? rows : []; - } catch { - return []; + } catch (err) { + // [#13279] THE loud failure. This `catch` used to `return []`, which made a + // FAILED read and an EMPTY one the same answer — so an outage of the + // permission store resolved as an authenticated principal holding zero + // capabilities, and the door answered a `403` byte-identical to a genuine + // capability denial. Ruled 2026-08-30: distinguish the two, and fail LOUD. + // + // ⚠️ The `!ql` guard ABOVE deliberately still returns `[]`: "no engine is + // wired" is not a failed read, and an embedder that never configured a data + // plane must keep resolving to an empty-but-valid envelope exactly as + // before. Only a read that was ISSUED and THREW reaches here. + throw new AuthzStoreUnavailableError(object, err); } } /** - * Resolve the authorization context for an inbound request. Always resolves — - * never throws. Anonymous requests yield `{ positions: [], permissions: [], ... }`. + * Resolve the authorization context for an inbound request. Anonymous requests + * yield `{ positions: [], permissions: [], ... }`. + * + * ⚠️ [#13279] This function used to document itself as "Always resolves — never + * throws", and that total guarantee WAS the defect: the only way to always + * resolve across a permission-store outage is to report a capability set the + * resolver never actually read. It now throws exactly one error — + * {@link AuthzStoreUnavailableError}, when a permission-store read was issued + * and failed. Every other path still resolves, including every MISSING-service + * path. A transport that fails closed on unexpected throws should re-raise this + * one ({@link isAuthzStoreUnavailableError}) rather than degrade it to a + * refusal — degrading it restores the disguise the ruling removed. */ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise { const { ql, headers } = input; @@ -321,8 +349,14 @@ export interface ResolveUserAuthzGrantsOptions { * automation engine calls this to run the flow's data ops exactly as that user * — not the bare member/everyone fallback the missing grants used to leave it. * - * Fail-closed like its parent: every read is defensive, a missing engine/table - * yields an empty-but-valid envelope, and it never throws. + * Fail-closed like its parent: a missing engine yields an empty-but-valid + * envelope. + * + * ⚠️ [#13279] "and it never throws" was removed from this sentence deliberately. + * A permission-store read that is issued and FAILS now raises + * {@link AuthzStoreUnavailableError} rather than contributing an empty grant + * set, so a `runAs:'user'` automation cannot silently run with the authority of + * a user whose grants were never read. */ export async function resolveUserAuthzGrants( ql: any, diff --git a/packages/plugins/plugin-sharing/src/sharing-plugin.ts b/packages/plugins/plugin-sharing/src/sharing-plugin.ts index 80c9911d24..a5b33b66e7 100644 --- a/packages/plugins/plugin-sharing/src/sharing-plugin.ts +++ b/packages/plugins/plugin-sharing/src/sharing-plugin.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import type { Plugin, PluginContext } from '@objectstack/core'; -import { resolveAuthzContext } from '@objectstack/core'; +import { resolveAuthzContext, isAuthzStoreUnavailableError } from '@objectstack/core'; import type { EngineMiddleware, OperationContext } from '@objectstack/objectql'; import type { AuthSessionApi, @@ -843,7 +843,11 @@ export class SharingServicePlugin implements Plugin { // both sibling transports build — `rest-server.ts` and // `runtime/src/security/resolve-execution-context.ts`. return { ...authz, isSystem: false }; - } catch { + } catch (err) { + // [#13279] Degrading an outage to `{}` answers 401 — the same + // answer a genuine anonymous caller gets. Re-raised so the + // outage is not laundered into an authentication verdict. + if (isAuthzStoreUnavailableError(err)) throw err; return {}; // anonymous → authed routes 401 } }; diff --git a/packages/rest/src/execctx-consumer-census.test.ts b/packages/rest/src/execctx-consumer-census.test.ts index 3ffc2e1a89..8efe709455 100644 --- a/packages/rest/src/execctx-consumer-census.test.ts +++ b/packages/rest/src/execctx-consumer-census.test.ts @@ -71,7 +71,13 @@ type Handler = (req: any, res: any) => any; /** * Every `this.resolveExecCtx(environmentId, req)` invocation, with the line it - * sits on and whether it carries its OWN `.catch(() => undefined)`. + * sits on and whether it carries its OWN `.catch(…)`. + * + * [#13279] The catch ARGUMENT changed — every site now passes + * `rethrowAuthzStoreUnavailable` instead of `() => undefined`, so a + * permission-store outage is re-raised rather than degraded into a refusal. + * The detection below keys on `.catch(` and is deliberately spelling-agnostic, + * so this census still measures WHICH sites are caught, which is its subject. * * ⚠️ The catch may sit on the CONTINUATION line — four of them do. A * single-line grep counts 16 and misses those four, which is how the thread's @@ -499,8 +505,13 @@ describe('[#13160] §6 the boundary of this census', () => { // `package-door-execctx-fault-reading.test.ts` (PR #13153) — // fail-CLOSED, two ablation legs, both rival readings falsified. // ⛔ Recorded as DEFERRED to that file, never as "assumed closed". + // [#13279] The catch argument is now `rethrowAuthzStoreUnavailable` + // (was `() => undefined`): a permission-store OUTAGE must reach the + // door as the 503 it is instead of being laundered into a 401/403. + // This grep tracks the wrapper's CURRENT spelling — the site is still + // deferred to its own file, which is what §6 asserts. const wrapper = SOURCE.split('\n').findIndex((l) => - l.includes('return this.resolveExecCtx(environmentId, req).catch(() => undefined);')); + l.includes('return this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable);')); expect(wrapper).toBeGreaterThan(0); expect(SITES.some((s) => s.line === wrapper + 1)).toBe(true); }); diff --git a/packages/rest/src/package-door-execctx-fault-reachability.test.ts b/packages/rest/src/package-door-execctx-fault-reachability.test.ts index 0c0b7d6725..f95f75d146 100644 --- a/packages/rest/src/package-door-execctx-fault-reachability.test.ts +++ b/packages/rest/src/package-door-execctx-fault-reachability.test.ts @@ -30,23 +30,39 @@ * required to access this endpoint." The caller may hold a valid session; * the fault is elsewhere. * - `GRANTS LOST` (two classes) — identity survives and the CAPABILITY - * aggregation is what faulted, so the door answers **403 `FORBIDDEN`**, + * aggregation is what faulted, so the door answered **403 `FORBIDDEN`**, * "Reading packages requires the `studio.access` or `setup.access` - * capability." An authenticated administrator is told they lack a - * capability while the permission store is down. ⭐ This shape does NOT + * capability." An authenticated administrator was told they lack a + * capability while the permission store was down. ⭐ This shape does NOT * travel through the `.catch(() => undefined)` the card names, nor through * `computeExecCtx`'s own `catch`: it is `tryFind`'s per-read swallow * inside `resolveAuthzContext` (`@objectstack/core`), one layer further - * out. Section 5 pins that both shapes are byte-identical to their - * innocent twins. + * out. + * + * ⭐ **[#13279] REPAIRED, and this file now pins the repair.** Maintainer + * ruling 2026-08-30, verbatim 「第一批其余同意」: `tryFind` distinguishes + * "no rows" from "the read failed", and a read failure fails LOUD. The + * `PERMISSION_STORE_DOWN` class therefore answers **503 + * `SERVICE_UNAVAILABLE`** and is no longer byte-identical to its innocent + * twin — section 5's assertion is INVERTED IN PLACE, with the superseded + * text quoted beside it. ⚠️ The SECOND grants-lost class, + * `DATA_ENGINE_UNRESOLVABLE`, is UNCHANGED and still answers 403: it + * reaches an empty grant set through `tryFind`'s `!ql` guard (there is no + * engine to read) rather than through its `catch` (a read that failed), so + * the ruling's landing point does not see it. That residue is asserted + * explicitly in section 3 rather than left to be rediscovered. * - **Is a fault ever served as ANONYMOUS ACCESS, or as a silent success? NO.** * Every degraded class is REFUSED on every wire-reachable method of all * four routes. The swallow fails CLOSED. (Section 6.) - * - **Does a fault ever reach the caller as the 5xx it is? NO — never, in any - * class.** That zero is read against a WORKING instrument: section 1 shows - * this same door answering **500 `INTERNAL_ERROR`** when the fault is raised - * one layer later, by the package service. So "no 5xx" is a property of the - * degradation, not of the harness. + * - **Does a fault ever reach the caller as the 5xx it is?** It did not, in + * any class — that zero was read against a WORKING instrument: section 1 + * shows this same door answering **500 `INTERNAL_ERROR`** when the fault is + * raised one layer later, by the package service. So "no 5xx" was a property + * of the degradation, not of the harness. ⭐ **[#13279] Now partitioned + * rather than zero**: the ruled permission-store class answers 503 on every + * route, and every class the ruling did not reach still answers a refusal. + * Section 3 asserts both halves, so the test still fails if a door goes + * quiet again OR if an unruled class starts throwing. * - **Does the `.catch(() => undefined)` at * `resolvePackageRouteExecutionContext` ever fire on production input? NO.** * In every class below the private resolver FULFILS. `computeExecCtx` wraps @@ -59,13 +75,18 @@ * lets anything through. Section 4 measures this per class, against a * control that shows the witness CAN report a rejection. * - * ## ⛔ What this file does not do + * ## What this file does, and no longer does not (#13279) + * + * As written for #13255 this file repaired nothing and asserted no verdict — + * distinguishing "no context" from "resolution failed" was a behaviour change + * on a public door and out of that card's scope. #13279 RULED that change for + * the permission-store half, so the assertions covering it are now regression + * pins on the repaired behaviour rather than measurements of a defect. * - * It does not repair anything and it asserts no verdict. Question 3's repair — - * distinguishing "no context" from "resolution failed" — is a behaviour change - * on a public door and is explicitly out of this card's scope. `rest-server.ts` - * is not edited by this change at all (it is under a same-file serial hold); - * everything here is measurement. + * ⛔ The other half is still not repaired and still not asserted as a verdict: + * `CONTEXT LOST` remains byte-identical to a genuine anonymous caller (section + * 5's first case is unchanged), and `DATA_ENGINE_UNRESOLVABLE` remains a 403. + * Both are recorded here as measurements, exactly as before. * * ## Reading discipline * @@ -77,7 +98,11 @@ */ import { describe, it, expect } from 'vitest'; -import { ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_STATUS } from '@objectstack/core'; +import { + ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_STATUS, + // [#13279] The loud permission-store outage, and its brand predicate. + AUTHZ_STORE_UNAVAILABLE_CODE, AUTHZ_STORE_UNAVAILABLE_STATUS, isAuthzStoreUnavailableError, +} from '@objectstack/core'; import type { RouteHandler } from '@objectstack/spec/contracts'; import { registerPackageRoutes } from './package-routes.js'; import { RestServer } from './rest-server.js'; @@ -258,13 +283,25 @@ interface FaultClass { faulted: () => Wiring; /** The request that reaches the door (scoped mount supplies `params`). */ req?: Record; - ctx: 'lost' | 'grants'; + /** + * `lost` — the whole execution context is gone. + * `grants` — identity survives, the capability aggregation is empty. + * `loud` — [#13279] the resolution REFUSES rather than resolving at all. + */ + ctx: 'lost' | 'grants' | 'loud'; read: { status: number; code: string }; write: { status: number; code: string }; } const DENY = { status: ANONYMOUS_DENY_STATUS, code: ANONYMOUS_DENY_CODE }; const FORBID = { status: 403, code: 'FORBIDDEN' }; +/** + * [#13279] The LOUD cohort — a permission-store outage, answered as the outage + * it is. Ruled 2026-08-30 (maintainer, verbatim 「第一批其余同意」): + * `tryFind` distinguishes "no rows" from "the read failed", and a read failure + * fails LOUD, so an outage can no longer be answered as a capability denial. + */ +const UNAVAILABLE = { status: AUTHZ_STORE_UNAVAILABLE_STATUS, code: AUTHZ_STORE_UNAVAILABLE_CODE }; const CLASSES: FaultClass[] = [ { @@ -308,7 +345,12 @@ const CLASSES: FaultClass[] = [ id: 'PERMISSION_STORE_DOWN', what: 'identity resolves, then every permission-store read throws', faulted: () => ({ ...healthy(), objectQLProvider: async () => qlDown() }), - ctx: 'grants', read: FORBID, write: FORBID, + // ⭐ [#13279] INVERTED IN PLACE, not re-baselined. Until the 2026-08-30 + // ruling this row read `ctx: 'grants', read: FORBID, write: FORBID` — an + // authenticated principal with an empty capability set, refused 403. That + // was the DISGUISE the ruling reverses: the store that holds the + // capabilities was down, so no capability judgement was ever reached. + ctx: 'loud', read: UNAVAILABLE, write: UNAVAILABLE, }, { id: 'DATA_ENGINE_UNRESOLVABLE', @@ -323,6 +365,23 @@ describe('[#13255] reachability — each production fault class, driven, with it // ---- the fault ------------------------------------------------------- const rest = serverWith(klass.faulted()); const req = { params: {}, headers: {}, method: 'GET', path: PKGS, ...(klass.req ?? {}) }; + // [#13279] The loud cohort never produces a context to inspect — that IS + // the repair. The resolution REJECTS with the branded outage error instead + // of fabricating an envelope that reports a capability set nobody read. + if (klass.ctx === 'loud') { + const settled = await rest.resolvePackageRouteExecutionContext(req).then( + (v) => ({ ok: true as const, v }), (e) => ({ ok: false as const, e }), + ); + expect(settled.ok).toBe(false); + // The BRAND, not merely "something threw": an incidental throw would + // satisfy a bare `.rejects` and prove nothing about which fault fired. + expect(isAuthzStoreUnavailableError((settled as any).e)).toBe(true); + expect((settled as any).e.status).toBe(AUTHZ_STORE_UNAVAILABLE_STATUS); + // POSITIVE CONTROL, unchanged: the same wiring minus the fault is served. + const served = await drive(mount(serverWith(healthy())), 'GET', PKGS, klass.req ?? {}); + expect(served.status).toBe(200); + return; + } const ctx = await rest.resolvePackageRouteExecutionContext(req); if (klass.ctx === 'lost') { expect(ctx).toBeUndefined(); @@ -370,20 +429,36 @@ describe('[#13255] consequence — the door\'s answer for each fault class', () expect(publish.body?.error?.code).toBe(klass.write.code); }); - it('⭐ NO class, on any route, is answered with the 5xx the fault actually is', async () => { - const seen: number[] = []; + it('⭐ [#13279] a permission-store OUTAGE surfaces as a 5xx on every route; every other class still does not', async () => { + // ⭐ INVERTED IN PLACE, not re-baselined. This test used to assert + // `seen.filter((s) => s >= 500)` was EMPTY across every class and route — + // "a fault never reaches the caller as the 5xx it is", which was the + // card's headline finding. The 2026-08-30 ruling reverses exactly that for + // the permission-store class, so the assertion is inverted rather than + // deleted: the zero becomes a partition, and the classes NOT covered by the + // ruling keep the original reading, which is what makes this still a + // regression test rather than a rubber stamp. + const loud: number[] = []; + const quiet: number[] = []; for (const klass of CLASSES) { const routes = mount(serverWith(klass.faulted())); const extra = klass.req ?? {}; - seen.push((await drive(routes, 'GET', PKGS, extra)).status); - seen.push((await drive(routes, 'DELETE', `${PKGS}/:id`, { ...extra, params: { ...(extra.params ?? {}), id: 'x' } })).status); - seen.push((await drive(routes, 'POST', `${PKGS}/publish`, { ...extra, body: { manifest: { id: 'x', version: '1.0.0' } } })).status); + const bucket = klass.ctx === 'loud' ? loud : quiet; + bucket.push((await drive(routes, 'GET', PKGS, extra)).status); + bucket.push((await drive(routes, 'DELETE', `${PKGS}/:id`, { ...extra, params: { ...(extra.params ?? {}), id: 'x' } })).status); + bucket.push((await drive(routes, 'POST', `${PKGS}/publish`, { ...extra, body: { manifest: { id: 'x', version: '1.0.0' } } })).status); } - // ⚠️ ZERO. Its control is section 1's 500 case, on this same door and this - // same harness — so this reads as "the degradation never surfaces", not as - // "the instrument cannot see a 5xx". - expect(seen.filter((s) => s >= 500)).toEqual([]); - expect(seen.every((s) => s === ANONYMOUS_DENY_STATUS || s === 403)).toBe(true); + // The ruled class: the outage is the answer, on EVERY route — not one door + // taught to be loud while its siblings kept the disguise. + expect(loud.length).toBeGreaterThan(0); + expect(loud.every((s) => s === AUTHZ_STORE_UNAVAILABLE_STATUS)).toBe(true); + // ⚠️ The classes the ruling did NOT reach still answer a refusal, and that + // residue is deliberately left visible rather than asserted away: + // `DATA_ENGINE_UNRESOLVABLE` reaches an empty grant set through `tryFind`'s + // `!ql` guard (no engine to read) rather than through its `catch` (a read + // that failed), so the ruling's landing point does not see it. + expect(quiet.filter((s) => s >= 500)).toEqual([]); + expect(quiet.every((s) => s === ANONYMOUS_DENY_STATUS || s === 403)).toBe(true); }); it('the capability clause, isolated from the anonymous floor, refuses the lost context too', async () => { @@ -414,13 +489,19 @@ describe('[#13255] the private resolver FULFILS on every production fault class' expect(await settle(Promise.resolve(1))).toBe('fulfilled'); }); - it.each(CLASSES)('$id — `resolveExecCtx` resolves; it does not reject', async (klass) => { + it.each(CLASSES)('$id — `resolveExecCtx` settles the way its cohort declares', async (klass) => { const rest = serverWith(klass.faulted()); const req: Record = { params: {}, headers: {}, method: 'GET', path: PKGS, ...(klass.req ?? {}) }; // The PRIVATE resolver, read BEFORE the wrapper's `.catch` can act — so // this reads the supplier, not the net over it. const inner = (rest as any).resolveExecCtx(req.params?.environmentId, req); - expect(await settle(inner)).toBe('fulfilled'); + // ⭐ [#13279] INVERTED IN PLACE for the loud cohort. This assertion used to + // read `'fulfilled'` for EVERY class, and that uniformity was the finding: + // every fault reached the door as a value, so no fault could be told from a + // verdict. A permission-store outage now REJECTS all the way out here — + // which is why `computeExecCtx`'s blanket `catch` had to learn to re-raise + // it. Every other class still fulfils, exactly as measured before. + expect(await settle(inner)).toBe(klass.ctx === 'loud' ? 'rejected' : 'fulfilled'); }); it('CONTROL: when the inner resolve IS made to reject, the wrapper is what absorbs it', async () => { @@ -452,16 +533,40 @@ describe('[#13255] a server-side fault is indistinguishable from the denial it i expect(JSON.stringify(faulted)).toBe(JSON.stringify(anonymous)); }); - it('GRANTS LOST: a permission-store outage answers exactly what "you hold nothing" answers', async () => { + it('⭐ [#13279] GRANTS LOST: a permission-store outage NO LONGER answers what "you hold nothing" answers', async () => { + // ⭐ THE INVERSION. This is the #13282 assertion the 2026-08-30 ruling + // reverses, inverted IN PLACE with its reason recorded — ⛔ not deleted and + // ⛔ not re-baselined. It used to read: + // + // expect(faulted.status).toBe(403); + // expect(faulted.body?.error?.message).toContain('studio.access'); + // expect(JSON.stringify(faulted)).toBe(JSON.stringify(genuinelyEmpty)); + // + // i.e. an outage of the permission store and a caller who genuinely holds + // nothing were ONE answer, byte for byte. Maintainer ruling 2026-08-30, + // verbatim 「第一批其余同意」: 权限库不可达时不再解析为「已认证零能力」, + // 而是响亮拒绝(与真实能力拒绝的 403 可区分). const faulted = await drive( mount(serverWith({ ...healthy(), objectQLProvider: async () => qlDown() })), 'GET', PKGS, ); const genuinelyEmpty = await drive( mount(serverWith({ authServiceProvider: AUTH_OK, objectQLProvider: async () => qlEmpty() })), 'GET', PKGS, ); - expect(faulted.status).toBe(403); - expect(faulted.body?.error?.message).toContain('studio.access'); - expect(JSON.stringify(faulted)).toBe(JSON.stringify(genuinelyEmpty)); + + // The outage is answered as an outage — and says so, in words that cannot + // be read as a permission verdict. + expect(faulted.status).toBe(AUTHZ_STORE_UNAVAILABLE_STATUS); + expect(faulted.body?.error?.code).toBe(AUTHZ_STORE_UNAVAILABLE_CODE); + expect(faulted.body?.error?.message).not.toContain('studio.access'); + + // ⚠️ The other half of the ruling, and the half a one-sided fix would + // break: a GENUINE capability denial is untouched. Making outages loud is + // only correct if real denials still read as denials. + expect(genuinelyEmpty.status).toBe(403); + expect(genuinelyEmpty.body?.error?.message).toContain('studio.access'); + + // The disguise is gone, stated on the same comparison that pinned it. + expect(JSON.stringify(faulted)).not.toBe(JSON.stringify(genuinelyEmpty)); }); it('CONTROL: the same comparison SEPARATES two answers that differ', async () => { diff --git a/packages/rest/src/package-routes.ts b/packages/rest/src/package-routes.ts index 28744f56c6..079ebd21bb 100644 --- a/packages/rest/src/package-routes.ts +++ b/packages/rest/src/package-routes.ts @@ -1,6 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import { IHttpServer, shouldDenyAnonymous, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE } from '@objectstack/core'; +import { IHttpServer, shouldDenyAnonymous, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE, rethrowAuthzStoreUnavailable } from '@objectstack/core'; // [#7020] The read cohort names the READ-ONLY half of the ADR-0106 D4 exemption // on purpose: `OBJECT_SCHEMA_MASK_EXEMPT_CAPABILITIES` became the derived union // (write gate ∪ read-only exemptions) under the 2026-08-10 ruling, while this @@ -77,8 +77,13 @@ async function refusePackageRequest( res: any, kind: 'read' | 'write', ): Promise { + // [#13279] The gate's OWN net. `rethrowAuthzStoreUnavailable` keeps the + // fail-closed default for every fault except a permission-store outage, which + // must reach `handlePackageRouteError` and be answered as the 503 + // `SERVICE_UNAVAILABLE` it is — never as a capability denial the caller could + // mistake for "you lack `studio.access`". const ctx = options.resolveExecutionContext - ? await options.resolveExecutionContext(req).catch(() => undefined) + ? await options.resolveExecutionContext(req).catch(rethrowAuthzStoreUnavailable) : undefined; // Anonymous-deny floor. This direct-mount surface DECLARES the wrapped // BaseResponseSchema envelope — every other body here goes through diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index b9809d0e52..6ce9831bf0 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -2,6 +2,9 @@ import { IHttpServer, resolveAuthzContext, resolveLocalizationContext, isAuthGateAllowlisted, + // [#13279] Re-raise a permission-store OUTAGE through the fail-closed nets + // below instead of degrading it into an anonymous/denied answer. + rethrowAuthzStoreUnavailable, effectiveTenancyPosture, assembleExecutionContext, normalizeAuthGate, type AuthGate, shouldDenyAnonymous, ANONYMOUS_DENY_BODY, ANONYMOUS_DENY_STATUS, @@ -1518,7 +1521,7 @@ export class RestServer { */ resolvePackageRouteExecutionContext(req: any): Promise { const environmentId = req?.params?.environmentId ?? undefined; - return this.resolveExecCtx(environmentId, req).catch(() => undefined); + return this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); } /** @@ -1598,7 +1601,7 @@ export class RestServer { // ignored outright — it is not consulted for user principals, and not // as a fallback for unauthenticated ones either, so there is no shape // in which a caller can choose the name the audit row records. - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); const userId = (ctx as any)?.userId; return typeof userId === 'string' && userId ? userId : undefined; } @@ -1619,7 +1622,7 @@ export class RestServer { req: any, opts: { needPermissionSets: boolean }, ): Promise<{ authenticated: boolean; permissionSets?: string[] }> { - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); const authenticated = !!ctx?.userId; if (!authenticated || !opts.needPermissionSets || !this.securityServiceProvider) { return { authenticated }; @@ -2041,8 +2044,16 @@ export class RestServer { if (isPerfDisclosurePrincipal(execCtx)) allowPerfDisclosure(); return execCtx; - } catch { - return undefined; + } catch (err) { + // [#13279] The FIRST net, and the one that actually fires: every + // seam below this resolves with `undefined` rather than rejecting, + // so a blanket swallow here decides the answer for the whole + // server. A permission-store OUTAGE must not be laundered into + // "no context" — that only swaps the 403 disguise for the 401 one + // (measured: with `tryFind` loud but this net untouched, the + // package door answered 401, byte-identical to a genuine anonymous + // caller). Every OTHER fault still fails closed exactly as before. + return rethrowAuthzStoreUnavailable(err); } } @@ -2805,7 +2816,7 @@ export class RestServer { } // Resolved ONCE per request, not once per item: the list read asks the // same caller about every object it serves. - const context = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const context = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); const security = await this.resolveSecurityService(environmentId, req); const telemetry = { warn: (message: string, meta: Record) => logWarn(message, meta), @@ -3934,7 +3945,7 @@ export class RestServer { // the `isScoped ? req.params.environmentId : undefined` // each `/data` handler derives. const environmentId = req?.params?.environmentId; - const context = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const context = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); // [#3963] `audience: 'public'` is a DECLARED capability, so it // must not depend on a deployment flipping its whole data plane // open. An anonymous read of the @@ -4174,7 +4185,7 @@ export class RestServer { // unauthorized caller cannot use the 501-vs-200 answer to // probe which kernels support drafts (same posture as // `_migrate-stored` below). - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); if (!isObjectSchemaMaskExempt(ctx)) { res.status(403).json({ error: { @@ -4263,7 +4274,7 @@ export class RestServer { // to the caller's own active organization" condition can // never hold here. `manage_metadata`-only, unchanged — // do not copy the item doors' acceptance in. - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); const held = new Set( Array.isArray(ctx?.systemPermissions) ? ctx!.systemPermissions : [], ); @@ -4471,7 +4482,7 @@ export class RestServer { ? ((raw as any).items as any[]) : null; if (list) { - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); if (ctx?.userId) { const sysPerms = new Set( Array.isArray(ctx.systemPermissions) ? ctx.systemPermissions : [], @@ -4578,7 +4589,7 @@ export class RestServer { ? ((raw as any).items as any[]) : null; if (list) { - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); const registered = await this.resolveRegisteredServices((ctx as any)?.__kernel, list); const serviceGate = registered ? (n: string) => registered.has(n) : undefined; if (serviceGate) { @@ -5399,7 +5410,7 @@ export class RestServer { // lacks the app's `requiredPermissions`, and strip // forbidden nav entries from the returned schema. if (isAppType && visible) { - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); if (ctx?.userId) { const sysPerms = new Set( Array.isArray(ctx.systemPermissions) ? ctx.systemPermissions : [], @@ -5473,7 +5484,7 @@ export class RestServer { // ADR's "the server is the authoritative gate" true // rather than merely written down. if (isDashboardType && visible) { - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); const registered = await this.resolveRegisteredServices((ctx as any)?.__kernel, [visible]); const serviceGate = registered ? (n: string) => registered.has(n) : undefined; if (serviceGate) visible = this.filterDashboardForUser(visible, serviceGate); @@ -5627,7 +5638,7 @@ export class RestServer { // very value `organizationIdForMetaWrite` threads below, so // an admitted write can only land org-scoped in the caller's // own partition: never env-wide, never another org's. - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); { const verdict = metaWriteCapabilityVerdict({ isSystem: ctx?.isSystem === true, @@ -5859,7 +5870,7 @@ export class RestServer { // one (see the [#8805] comment below). `?dropStorage=true` // is `object`-only, and `object` is not org-overridable, // so the org capability can never reach it. - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); { const verdict = metaWriteCapabilityVerdict({ isSystem: ctx?.isSystem === true, @@ -6129,7 +6140,7 @@ export class RestServer { // hands back — the same reasoning the `/published` route // states below — not from the request payload. It is still // read on the two lines that need it. - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); // The `(p as any)` casts this door carried came off when // `MetadataProtocol` declared `auditMetaItem` (the #11006 // pattern, same as the publish door below): the literal is @@ -6204,7 +6215,7 @@ export class RestServer { // the draft through `getOverlayRepo(orgId)`, so an admitted // org-presentation publish promotes only the caller's own // org partition. - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); { const verdict = metaWriteCapabilityVerdict({ isSystem: ctx?.isSystem === true, @@ -6397,7 +6408,7 @@ export class RestServer { // org-presentation rollback restores only a version of the // caller's own org overlay — the env-wide row and its // history stay out of reach. - const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); + const ctx = await this.resolveExecCtx(environmentId, req).catch(rethrowAuthzStoreUnavailable); { const verdict = metaWriteCapabilityVerdict({ isSystem: ctx?.isSystem === true, diff --git a/packages/services/service-datasource/src/admin-routes.ts b/packages/services/service-datasource/src/admin-routes.ts index 164d3b00f9..c2ecbb548f 100644 --- a/packages/services/service-datasource/src/admin-routes.ts +++ b/packages/services/service-datasource/src/admin-routes.ts @@ -7,6 +7,7 @@ import type { PluginContext } from '@objectstack/core'; // local check would be the wrong shape even if it were written correctly. import { resolveAuthzContext, + isAuthzStoreUnavailableError, shouldDenyAnonymous, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, @@ -392,9 +393,13 @@ export function registerDatasourceAdminRoutes( // resolution rather than a second reading of `sys_*`. systemPermissions = Array.isArray(authz.systemPermissions) ? authz.systemPermissions : []; } - } catch { - // Fail closed: an identity that could not be resolved is not an identity, - // and grants that could not be read are not grants. + } catch (err) { + // [#13279] "grants that could not be READ are not grants" was the exact + // reasoning the 2026-08-30 ruling reverses: an unreadable store licenses + // no verdict at all, so the outage is re-raised instead of being answered + // as a denial. Every other fault still fails closed, unchanged. + if (isAuthzStoreUnavailableError(err)) throw err; + // Fail closed: an identity that could not be resolved is not an identity. userId = undefined; systemPermissions = []; } diff --git a/packages/services/service-settings/src/settings-service-plugin.ts b/packages/services/service-settings/src/settings-service-plugin.ts index daaa2b21c1..6063f29f26 100644 --- a/packages/services/service-settings/src/settings-service-plugin.ts +++ b/packages/services/service-settings/src/settings-service-plugin.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import type { Plugin, PluginContext } from '@objectstack/core'; -import { resolveAuthzContext } from '@objectstack/core'; +import { resolveAuthzContext, isAuthzStoreUnavailableError } from '@objectstack/core'; import type { IHttpServer, IDataEngine, IHttpRequest } from '@objectstack/spec/contracts'; import type { SettingsContext } from './settings-service.types.js'; import type { SettingsManifest } from '@objectstack/spec/system'; @@ -285,7 +285,11 @@ export class SettingsServicePlugin implements Plugin { permissions: [...(authz.systemPermissions ?? []), ...(authz.permissions ?? [])], enforced: true, }; - } catch { + } catch (err) { + // [#13279] An unreachable permission store is an outage, not a + // caller with no permissions — re-raise it rather than returning an + // enforced-but-empty context the routes read as a denial. + if (isAuthzStoreUnavailableError(err)) throw err; return { enforced: true }; // fail closed } }; diff --git a/packages/services/service-storage/src/storage-service-plugin.ts b/packages/services/service-storage/src/storage-service-plugin.ts index cd97a51e3d..96f307ad68 100644 --- a/packages/services/service-storage/src/storage-service-plugin.ts +++ b/packages/services/service-storage/src/storage-service-plugin.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import type { Plugin, PluginContext } from '@objectstack/core'; -import { resolveAuthzContext } from '@objectstack/core'; +import { resolveAuthzContext, isAuthzStoreUnavailableError } from '@objectstack/core'; import type { IHttpServer, IDataEngine, @@ -770,7 +770,11 @@ function buildFileReadAuthorizer( } } return 'deny'; - } catch { + } catch (err) { + // [#13279] A permission-store outage is not a read verdict. Re-raised so + // the file-read door answers the 503 it is instead of a silent 'deny' + // indistinguishable from a genuine refusal. + if (isAuthzStoreUnavailableError(err)) throw err; return 'deny'; // fail closed } }; From 6f83e6e5cbedc13b8c8231aca984e8fdb120e2be Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 14:16:23 +0000 Subject: [PATCH 2/7] docs(changeset): record the ADR-0087 disposition for #13279 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declared-breaking changesets must answer the ledger question in writing. This change touches no metadata surface in either direction — no Zod schema, no spec declaration, no authorable key, no stored row, no object definition — so `objectstack migrate meta` has nothing to visit and no tombstone exists to mint. `SERVICE_UNAVAILABLE` is an existing `StandardErrorCode` member, so even the wire vocabulary is unchanged; only which declared code an outage selects. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .changeset/authz-read-failure-fails-loud.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.changeset/authz-read-failure-fails-loud.md b/.changeset/authz-read-failure-fails-loud.md index c539015600..1173449bbe 100644 --- a/.changeset/authz-read-failure-fails-loud.md +++ b/.changeset/authz-read-failure-fails-loud.md @@ -57,3 +57,5 @@ later cannot inherit the old silence unnoticed. Callers that treat any throw from `resolveAuthzContext` as "anonymous" should re-raise `isAuthzStoreUnavailableError(err)` instead: degrading it restores the disguise this removes. + + From 85d27f4f9faf0636a0586484dce97e5436137318 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 14:22:41 +0000 Subject: [PATCH 3/7] docs(changeset): retract an overclaim measured false (#13279) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The changeset claimed "embedders without a data plane are unaffected". Measured false: the pinned cases cover an UNWIRED engine and an EMPTY one, not a real engine whose sys_* tables were never created, where find is issued and throws `no such table`. That shape is currently treated as an outage and must not be — with no permission tables provisioned, zero capabilities is the true answer. Records the measured cost (client CRUD 503s, 400s becoming 503s, two silenced diagnostic channels) and why the boundary cannot be drawn inside core. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .changeset/authz-read-failure-fails-loud.md | 35 ++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/.changeset/authz-read-failure-fails-loud.md b/.changeset/authz-read-failure-fails-loud.md index 1173449bbe..8ea6914f4a 100644 --- a/.changeset/authz-read-failure-fails-loud.md +++ b/.changeset/authz-read-failure-fails-loud.md @@ -37,13 +37,40 @@ code an outage selects. Doors that map thrown errors through **What did NOT change**, and is pinned: -- A reachable, genuinely EMPTY store still resolves to zero capabilities. +- A reachable, genuinely EMPTY store (reads return no rows) still resolves to + zero capabilities. - A genuine capability denial still answers `403 FORBIDDEN` with its message. -- An ABSENT engine (`ql` unwired) still resolves to an empty-but-valid envelope - — "no engine" is not a failed read, and embedders without a data plane are - unaffected. +- An ABSENT engine (`ql` unwired, so no read is ever issued) still resolves to + an empty-but-valid envelope. - Anonymous requests never reach the store, so an outage cannot make them loud. +⛔ **NOT YET TRUE, and this is why the PR is blocked.** An earlier revision of +this changeset claimed "embedders without a data plane are unaffected". That +claim was too broad and is retracted. The pinned cases above cover an *unwired* +engine and an *empty* one; they do NOT cover the far commoner unprovisioned +shape — a REAL engine whose `sys_*` tables were never created, where `find` is +issued and THROWS `no such table`. That is currently treated as an outage, and +it must not be: with no permission tables provisioned, "zero capabilities" is +the TRUE answer, not a fabrication. Only an UNREACHABLE store — the ruling's own +word — leaves the capability set unknown. + +Measured cost of the conflation, all from `no such table` on `sys_user`, +`sys_member`, `sys_user_position`, `sys_user_permission_set`: ordinary CRUD in +`@objectstack/client` answers `503`, batch validation errors that owe `400` +answer `503` because authorization refuses before validation runs, and two +`.integration.test.ts` noise guards report that the driver and engine +diagnostics for `sys_position` stopped being emitted — the eager throw aborts +the resolution before that later read is ever issued, so a change made to stop a +failed read being silent made two other channels silent. + +The classifier that draws the boundary correctly already exists and is exactly +right — `isMissingTableError(error, readObject)`, driver-code based rather than +prose-sniffing, documented so that "cannot say" never means "be loud" — but it +lives in `@objectstack/metadata`, which DEPENDS ON `@objectstack/core`, so the +resolver cannot import it; and the SQL driver's `backendStatementFaultError` +deliberately withholds the distinction from the thrown error. Resolving that is +a maintainer decision, not an implementation detail. + **All-transport, not just REST.** Every transport authorizing through `resolveAuthzContext` inherits this. Six of the eight production transports wrapped the call in a fail-closed `catch` that would have re-silenced the From 603c9cbb6bf3d2f007808055e42bc4cbe0a778d7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 14:40:14 +0000 Subject: [PATCH 4/7] docs(permissions): re-anchor the system-context census after the #13279 line shift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure line rot from this PR's transport edits, not a population change — measured both ways before touching the page: merge base 71627f7b4e census EXIT=0, 109 sites / 145 anchors / 27 non-read this branch same 109 sites; rest-server.ts holds 6 read sites on BOTH sides, shifted [1266 4270 5633 5865 6210 6403] -> [1269 4281 5644 5876 6221 6414] Every re-anchored line was checked content-identical between base and head (+3 before the import insertion, +11 after the computeExecCtx expansion). `--fix` repaired package-routes.ts and sharing-plugin.ts mechanically. It REFUSED rest-server.ts, reporting "page anchors 7 distinct read line(s), census finds 6 -- the POPULATION changed". That refusal is a limitation of the --fix heuristic, not a population change: it counts the two ledger-excused non-read citations (`:1234`, `:1263`, the "never settable from inbound HTTP" pair) as read anchors. The census itself reports 6 reads on both sides. Those five anchors were therefore re-anchored by hand to lines proven byte-identical, and NON_READ_ANCHORS needed no edit — it locates rows by needle, so it re-resolved itself once the page pointed at the right lines. No row was written, deleted or reworded; only line numbers move. The gate's verdict line is now byte-identical to the merge base's. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- content/docs/permissions/system-context.mdx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 02f78b850d..3ec8097df0 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -64,7 +64,7 @@ not on any flag. ## How the flag is set `isSystem` is **server-constructed and never client-supplied**. Inbound HTTP -cannot set it (`packages/rest/src/rest-server.ts:1234`, `:1263`), and neither +cannot set it (`packages/rest/src/rest-server.ts:1237`, `:1266`), and neither can an action body (`packages/runtime/src/domains/actions.ts:404`). It is written by internal callers only, as an option on the engine call: @@ -103,7 +103,7 @@ that silently does not happen. | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:246` | | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` | | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` | -| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1266` | +| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1269` | ### 2. Write pipeline and data integrity @@ -134,7 +134,7 @@ The largest single consumer — **20 of the 109 sites**. | 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1179` | | 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1257` (guard at `:1282`) | | 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1309` | -| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1073` | +| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:413`, `:467`, `:471`, `:544`, `:574` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | | 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:157`, `:382` | @@ -158,11 +158,11 @@ The largest single consumer — **20 of the 109 sites**. |:--|:---|:---|:---|:---| | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:136` | | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:399` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4270`, `:5633`, `:5865`, `:6210`, `:6403` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4281`, `:5644`, `:5876`, `:6221`, `:6414` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | -| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:982`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:246`, `external-datasource-routes.ts:302`, `package-routes.ts:92` | +| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:982`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:246`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | -| 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:97` | +| 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:102` | | 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:95`, `:128` | | 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `activation-gate.ts:139`, `:190` | | 57 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `domains/automation.ts:254`, `:545`, `:635` | @@ -199,7 +199,7 @@ assuming `isSystem` covers it is a documented source of bugs. | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1451` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:273` | -| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1234`, `:1263`; `domains/actions.ts:404` | +| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1237`, `:1266`; `domains/actions.ts:404` | --- From 5c7c0b66b932ebe3efe53facf6580838eb0c50ba Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 15:26:10 +0000 Subject: [PATCH 5/7] fix(core,types,metadata): narrow the loud authz read failure to a genuinely unreachable store Implements the maintainer's 2026-08-30 option-A ruling on top of the existing loud-failure implementation. "The read failed" is two facts. A read also throws when the table was never PROVISIONED -- a real engine, wired and reachable, whose sys_* tables were never created. There "zero capabilities" is the true answer, not a fabrication; only an UNREACHABLE store leaves it unknown. Treating them alike turned four CI suites red. - Relocate isMissingTableError (and its sibling isSchemaAlreadyExistsError, which shares its matcher and cannot be separated from it) from @objectstack/metadata to @objectstack/types, the package @objectstack/core already depends on. @objectstack/metadata/errors still exports isMissingTableError by re-export, so no consumer of that published subpath changes. - tryFind raises AuthzStoreUnavailableError only for a read failure that is NOT positively identified as an unprovisioned table. - Record the signed-off false-positive risk beside the predicate, as the ruling requires, and pin both directions by name in authz-store-unavailable.test.ts. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .changeset/authz-read-failure-fails-loud.md | 92 +++++++--- .../security/authz-store-unavailable.test.ts | 168 ++++++++++++++++++ .../src/security/authz-store-unavailable.ts | 15 ++ .../src/security/resolve-authz-context.ts | 68 ++++++- packages/metadata/src/errors.ts | 29 ++- .../metadata/src/loaders/database-loader.ts | 5 +- .../src/external-datasource-service.ts | 3 +- .../src/driver-error-classification.test.ts} | 8 +- .../src/driver-error-classification.ts} | 69 ++++++- packages/types/src/error-leak.test.ts | 3 +- packages/types/src/index.ts | 10 ++ packages/types/src/unique-violation.ts | 4 +- 12 files changed, 426 insertions(+), 48 deletions(-) rename packages/{metadata/src/utils/schema-sync-errors.test.ts => types/src/driver-error-classification.test.ts} (98%) rename packages/{metadata/src/utils/schema-sync-errors.ts => types/src/driver-error-classification.ts} (86%) diff --git a/.changeset/authz-read-failure-fails-loud.md b/.changeset/authz-read-failure-fails-loud.md index 8ea6914f4a..b1fa7b6f3e 100644 --- a/.changeset/authz-read-failure-fails-loud.md +++ b/.changeset/authz-read-failure-fails-loud.md @@ -1,5 +1,7 @@ --- "@objectstack/core": minor +"@objectstack/types": minor +"@objectstack/metadata": patch "@objectstack/rest": patch "@objectstack/service-datasource": patch "@objectstack/service-settings": patch @@ -28,6 +30,12 @@ Maintainer ruling 2026-08-30, verbatim 「第一批其余同意」: `tryFind` 与「读失败」,读失败 fail-loud —— 权限库不可达时不再解析为「已认证零能力」,而是 响亮拒绝(与真实能力拒绝的 403 可区分)。 +Second maintainer ruling the same day (第 5 场总监席决裁批 #9, verbatim 「同意」), +after implementing the first one showed that "the read failed" is two facts: +采**选项 A** —— 把 `isMissingTableError` 从 `@objectstack/metadata` 迁至 +`@objectstack/types`(core 已依赖),metadata 保留 re-export 兼容;`tryFind` 仅对 +**未被判定为「表未 provision」**的读失败抛 `AuthzStoreUnavailableError`。 + **What changed.** A permission-store read that is issued and throws now raises `AuthzStoreUnavailableError`, which carries the EXISTING ADR-0112 wire code `SERVICE_UNAVAILABLE` and status `503`. No code is added to the closed wire @@ -43,33 +51,67 @@ code an outage selects. Doors that map thrown errors through - An ABSENT engine (`ql` unwired, so no read is ever issued) still resolves to an empty-but-valid envelope. - Anonymous requests never reach the store, so an outage cannot make them loud. +- A REAL engine whose `sys_*` tables were never provisioned resolves to zero + capabilities, quietly — pinned to be byte-identical to the empty-store + envelope, in every dialect spelling and in the production wrapper shape where + the driver's phrase is on `cause` rather than the outer message. -⛔ **NOT YET TRUE, and this is why the PR is blocked.** An earlier revision of +**The boundary between the two kinds of read failure.** An earlier revision of this changeset claimed "embedders without a data plane are unaffected". That -claim was too broad and is retracted. The pinned cases above cover an *unwired* -engine and an *empty* one; they do NOT cover the far commoner unprovisioned -shape — a REAL engine whose `sys_*` tables were never created, where `find` is -issued and THROWS `no such table`. That is currently treated as an outage, and -it must not be: with no permission tables provisioned, "zero capabilities" is -the TRUE answer, not a fabrication. Only an UNREACHABLE store — the ruling's own -word — leaves the capability set unknown. - -Measured cost of the conflation, all from `no such table` on `sys_user`, -`sys_member`, `sys_user_position`, `sys_user_permission_set`: ordinary CRUD in -`@objectstack/client` answers `503`, batch validation errors that owe `400` -answer `503` because authorization refuses before validation runs, and two -`.integration.test.ts` noise guards report that the driver and engine -diagnostics for `sys_position` stopped being emitted — the eager throw aborts -the resolution before that later read is ever issued, so a change made to stop a -failed read being silent made two other channels silent. - -The classifier that draws the boundary correctly already exists and is exactly -right — `isMissingTableError(error, readObject)`, driver-code based rather than -prose-sniffing, documented so that "cannot say" never means "be loud" — but it -lives in `@objectstack/metadata`, which DEPENDS ON `@objectstack/core`, so the -resolver cannot import it; and the SQL driver's `backendStatementFaultError` -deliberately withholds the distinction from the thrown error. Resolving that is -a maintainer decision, not an implementation detail. +claim was too broad; it is retracted here, and the gap it named is now closed +rather than merely disclosed. A read also throws when the table was never +PROVISIONED — a real engine, wired and reachable, whose `sys_*` tables were +never created — and that is a supported deployment shape, not an outage. There +"zero capabilities" is the TRUE answer rather than a fabrication: nothing is +provisioned, so nothing was withheld. Only an UNREACHABLE store — the ruling's +own word 不可达 — leaves the capability set unknown, and only an unknown answer +may not be reported as a denial. + +Treating the two alike was measured, not theorised: it turned four CI suites +red, all from `no such table` on `sys_user` / `sys_member` / +`sys_user_position` / `sys_user_permission_set`. Ordinary CRUD in +`@objectstack/client` answered `503`; batch validation errors that owe `400` +answered `503`, because authorization refused before validation ran; runtime +notifications answered `401` where authenticated callers must be served `200`; +and two `.integration.test.ts` noise guards reported that the driver and engine +diagnostics for `sys_position` stopped being emitted — the eager throw aborted +the resolution before that later read was ever issued, so a change made to stop +a failed read being silent had made two other channels silent. + +`tryFind` therefore raises `AuthzStoreUnavailableError` only for a read failure +that is NOT positively identified as an unprovisioned table. + +**`isMissingTableError` moved to `@objectstack/types`.** The classifier that +draws that boundary already existed and was already right — driver-code based +rather than prose-sniffing, documented so that "cannot say" never means "be +loud". It lived in `@objectstack/metadata`, which DEPENDS ON `@objectstack/core`, +so the resolver could not import it. Rather than keep a second copy of a +security-relevant predicate, the ruling relocated the one classifier to +`@objectstack/types` — the package core already depends on, and the repo's own +stated Home rule for a cross-package error predicate ("every consumer of the +question already depends on it, so adopting the predicate never adds an edge", +`packages/types/src/unique-violation.ts`). `@objectstack/metadata/errors` still +exports `isMissingTableError`, re-exported from the new home, so no consumer of +that published subpath changes. + +Its sibling `isSchemaAlreadyExistsError` moved with it — the two are not two +modules but two signatures over one matcher, and separating them would have +meant re-rolling the matcher, which is the duplication the module exists to +prevent. Both are now exported from `@objectstack/types`; the metadata subpath +deliberately still publishes only `isMissingTableError`, which is the only one +anything imports through it. + +⚠️ **Signed-off risk, recorded because it is load-bearing.** Gating loudness on +a driver-error predicate was approved with its false-positive direction stated: +mis-reading a genuine outage as "table not provisioned" silently restores the +quiet 403 this change removes, with no thrown error and no other failing test. +That direction is accepted, not overlooked — the predicate keys on driver codes, +SQLSTATEs and errnos first, excludes the known superstring traps up front, and +returns `false` for anything it does not positively recognise, so an +unrecognised outage stays loud by default. The risk is written beside the +predicate in `resolve-authz-context.ts` and both directions are pinned by name +in `authz-store-unavailable.test.ts`. ⛔ Do not widen `isMissingTableError` to +make a first boot quieter: every widening moves outages into the quiet branch. **All-transport, not just REST.** Every transport authorizing through `resolveAuthzContext` inherits this. Six of the eight production transports diff --git a/packages/core/src/security/authz-store-unavailable.test.ts b/packages/core/src/security/authz-store-unavailable.test.ts index 38a0ed59e8..3116a8bf56 100644 --- a/packages/core/src/security/authz-store-unavailable.test.ts +++ b/packages/core/src/security/authz-store-unavailable.test.ts @@ -246,3 +246,171 @@ describe('[#13279] every transport that authorizes through resolveAuthzContext', }, ); }); + +// --------------------------------------------------------------------------- +// 4. ⭐⭐ THE BOUNDARY, PINNED IN BOTH DIRECTIONS. +// +// Maintainer ruling, 2026-08-30, 第 5 场总监席决裁批 #9, verbatim: +// +// > 签字在案:基于 driver 错误码的表缺失判定获准在安全路径上门控响亮性; +// > 其假阳方向(误判「表缺失」⇒ 静默恢复安静 403)是本裁定接受的已知风险, +// > 须在谓词旁注释写明并以测试钉住两个方向(真 outage ⇒ 响;真未 provision +// > ⇒ 零能力为真答案,不响)。 +// +// This section IS the "以测试钉住两个方向" half; the "谓词旁注释" half is the +// comment beside `isMissingTableError` in `tryFind`'s catch. +// +// Why BOTH are load-bearing, and why neither alone would do: +// - only the LOUD direction ⇒ satisfied by a resolver that throws at +// everything, which refuses service to correctly-configured deployments +// whose `sys_*` tables were never created (measured: it turned four CI +// suites red — client CRUD, runtime notifications, and two integration +// noise guards). +// - only the QUIET direction ⇒ satisfied by the pre-#13279 `return []`, +// i.e. the defect itself: an outage answered as a capability denial. +// +// ⚠️ The accepted risk lives in the first direction. A false POSITIVE from +// `isMissingTableError` — an outage mis-read as "not provisioned" — restores +// the quiet 403 with no other symptom anywhere. Every case below is a shape +// that must NOT be mis-read, so this table is the risk's tripwire. Add to it +// rather than relax it. +// --------------------------------------------------------------------------- + +/** The tables `resolveAuthzContext` reads; a fake engine answers all of them alike. */ +const qlThrowing = (make: (object: string) => unknown) => ({ + find: async (object: string) => { throw make(object); }, +}); + +/** + * Shapes that are NOT "this table was never provisioned", each with the reason + * a naive predicate might have said otherwise. + */ +const OUTAGE_SHAPES: ReadonlyArray unknown]> = [ + ['a bare connection failure', () => new Error('permission store unreachable')], + ['ECONNREFUSED from the driver', () => Object.assign(new Error('connect ECONNREFUSED'), { code: 'ECONNREFUSED' })], + ['a statement timeout', () => Object.assign(new Error('canceling statement due to statement timeout'), { code: '57014' })], + [ + 'a PERMISSION denial on the table — the row exists, we were refused it', + (o: string) => Object.assign(new Error(`permission denied for table ${o}`), { code: '42501' }), + ], + [ + '#6347 the Postgres missing-COLUMN phrase, which CONTAINS a legal missing-table phrase', + (o: string) => Object.assign(new Error(`column "x" of relation "${o}" does not exist`), { code: '42703' }), + ], + [ + '#13324 a missing-table phrase naming a DIFFERENT relation (a view over a dropped base)', + () => new Error('no such table: main.some_other_base'), + ], + ['an unrecognised driver error — unrecognised must never mean benign', () => Object.assign(new Error('opaque'), { code: 'SQLITE_BUSY' })], +]; + +describe('[#13279 option A] ⭐ THE OUTAGE DIRECTION — a read failure that is not an unprovisioned table stays LOUD', () => { + it.each(OUTAGE_SHAPES)('%s ⇒ AuthzStoreUnavailableError (503 SERVICE_UNAVAILABLE)', async (_label, make) => { + const r = await settle(resolveAuthzContext({ ql: qlThrowing(make), headers: {}, ...SESSION })); + expect(r.ok).toBe(false); + expect(isAuthzStoreUnavailableError((r as any).e)).toBe(true); + expect((r as any).e.code).toBe(AUTHZ_STORE_UNAVAILABLE_CODE); + expect((r as any).e.status).toBe(AUTHZ_STORE_UNAVAILABLE_STATUS); + }); + + it('⭐ a store that is provisioned but loses ONE table mid-resolution is still loud', async () => { + // The quiet branch must not leak past the table it is about. Everything + // resolves normally except `sys_user_permission_set`, which fails for a + // reason that is not "never provisioned". + const ql = { + find: async (object: string) => { + if (object === 'sys_user_permission_set') throw new Error('connection terminated unexpectedly'); + return []; + }, + }; + const r = await settle(resolveAuthzContext({ ql, headers: {}, ...SESSION })); + expect(r.ok).toBe(false); + expect(isAuthzStoreUnavailableError((r as any).e)).toBe(true); + expect((r as any).e.object).toBe('sys_user_permission_set'); + }); +}); + +/** + * Genuine "the table was never created", in every spelling a supported driver + * produces. This is the shape a first boot against a fresh database makes, and + * `packages/runtime`'s `notifications.hono.integration.test.ts` names the + * deployment shape `ABSENT_AUTHZ_TABLES`. + */ +const UNPROVISIONED_SHAPES: ReadonlyArray unknown]> = [ + ['SQLite / libsql message', (o: string) => new Error(`no such table: ${o}`)], + ['PostgreSQL SQLSTATE 42P01', (o: string) => Object.assign(new Error(`relation "${o}" does not exist`), { code: '42P01' })], + ['MySQL / MariaDB errno 1146', (o: string) => Object.assign(new Error(`Table 'app.${o}' doesn't exist`), { errno: 1146 })], + [ + 'the PRODUCTION wrapper shape — driver phrase on `cause`, not on the outer message', + // What `SqlDriver.backendStatementFaultError` actually raises: a wrapper + // that deliberately withholds the verdict, with the driver's own error + // attached. The predicate follows `cause`; if it stopped at the outer + // message this would be read as an outage and first boot would 503. + (o: string) => Object.assign(new Error('backend statement fault'), { cause: new Error(`no such table: ${o}`) }), + ], +]; + +describe('[#13279 option A] ⭐ THE UNPROVISIONED DIRECTION — a never-provisioned table resolves QUIETLY', () => { + it.each(UNPROVISIONED_SHAPES)( + '%s ⇒ zero capabilities, because that is the TRUE answer and not a fabrication', + async (_label, make) => { + const ctx = await resolveAuthzContext({ ql: qlThrowing(make), headers: {}, ...SESSION }); + expect(ctx.userId).toBe(USER); + expect(ctx.systemPermissions).toEqual([]); + expect(ctx.permissions).toEqual([]); + // ONLY the unconditional `everyone` audience anchor (ADR-0090 D5), which + // every authenticated member holds without any read happening. Asserted + // exactly rather than as "empty": the point is that NOTHING was invented + // from a read that failed, and `toEqual([])` would have been a claim + // about the anchor rather than about this repair. + expect(ctx.positions).toEqual(['everyone']); + }, + ); + + it('⭐ resolves IDENTICALLY to a reachable, genuinely empty store', async () => { + // The strongest statement of "this is the true answer": an unprovisioned + // deployment and an empty-but-provisioned one are the same fact — nothing + // is granted — so they must be the same envelope, byte for byte. Anything + // less means the quiet branch is quietly different. + const unprovisioned = await resolveAuthzContext({ + ql: qlThrowing((o) => new Error(`no such table: ${o}`)), headers: {}, ...SESSION, + }); + const empty = await resolveAuthzContext({ ql: qlEmpty(), headers: {}, ...SESSION }); + expect(JSON.stringify(unprovisioned)).toBe(JSON.stringify(empty)); + // CONTROL against a vacuous comparison: a HEALTHY store differs from both. + const healthy = await resolveAuthzContext({ ql: qlHealthy(), headers: {}, ...SESSION }); + expect(JSON.stringify(healthy)).not.toBe(JSON.stringify(empty)); + }); + + it('⭐ `resolveUserAuthzGrants` inherits the quiet direction too', async () => { + const grants = await resolveUserAuthzGrants(qlThrowing((o) => new Error(`no such table: ${o}`)), USER); + expect(grants.systemPermissions).toEqual([]); + expect(grants.org_user_ids).toEqual([USER]); + }); +}); + +describe('[#13279 option A] the classifier is the RELOCATED one, not a second copy', () => { + it('⭐ `resolve-authz-context.ts` imports `isMissingTableError` from `@objectstack/types`', () => { + // The ruling's structural half, pinned in source. A local re-spelling of + // the predicate here would pass every behavioural test above and still be + // the duplication-drift the ruling rejected (option B). + const src = readFileSync(join(REPO_ROOT, 'packages/core/src/security/resolve-authz-context.ts'), 'utf8'); + expect(src).toMatch(/import \{ isMissingTableError \} from '@objectstack\/types';/); + expect(src).not.toMatch(/function\s+isMissingTableError/); + // ⛔ core must not IMPORT `@objectstack/metadata` — metadata depends on + // core, and that edge is why the predicate moved rather than being imported. + // (Prose mentions of the old home are fine and deliberate; an import is not.) + expect(src).not.toMatch(/from '@objectstack\/metadata/); + }); + + it('⭐ the SIGNED-OFF RISK is written beside the predicate, as the ruling requires', () => { + // The audit trail is a deliverable of the ruling, not decoration: the + // false-positive direction is accepted only BECAUSE it is recorded where + // the next author will read it before widening the predicate. + const src = readFileSync(join(REPO_ROOT, 'packages/core/src/security/resolve-authz-context.ts'), 'utf8'); + const catchBody = src.slice(src.indexOf('} catch (err) {'), src.indexOf('throw new AuthzStoreUnavailableError(object, err);')); + expect(catchBody).toContain('THE SIGNED-OFF RISK'); + expect(catchBody).toContain('签字在案'); + expect(catchBody).toContain('isMissingTableError(err, object)'); + }); +}); diff --git a/packages/core/src/security/authz-store-unavailable.ts b/packages/core/src/security/authz-store-unavailable.ts index 41b7535a76..6e5525f73f 100644 --- a/packages/core/src/security/authz-store-unavailable.ts +++ b/packages/core/src/security/authz-store-unavailable.ts @@ -20,6 +20,21 @@ * and "the read failed" are different facts, and only one of them licenses * the sentence "this user holds nothing". * + * ## …and "the read failed" turned out to be TWO facts (ruled 2026-08-30, A) + * + * A read ALSO throws when the table was never PROVISIONED — a real engine, + * wired and reachable, whose `sys_*` tables were never created. There "this + * user holds nothing" is TRUE, not invented, so failing loud would refuse + * service to a correctly-configured deployment. The first implementation of + * this card did exactly that and four CI suites measured it. + * + * So `tryFind` raises this error only for a read failure that is NOT + * positively identified as an unprovisioned table, asking the one relocated + * `isMissingTableError` predicate (`@objectstack/types`) rather than a second + * copy. The boundary, the ruling's verbatim text and the false-positive risk + * signed off with it are written beside that call in `resolve-authz-context.ts`; + * both directions are pinned in `authz-store-unavailable.test.ts` §4. + * * ## Maintainer ruling, 2026-08-30, verbatim 「第一批其余同意」 * * > `tryFind` 区分「无行」与「读失败」,读失败 fail-loud —— 权限库不可达时 diff --git a/packages/core/src/security/resolve-authz-context.ts b/packages/core/src/security/resolve-authz-context.ts index 7a3f95f7fa..942d8d2a40 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -30,9 +30,21 @@ * instead of reporting an empty grant set: an outage must not be answerable as * a capability denial. See `authz-store-unavailable.ts` for the full reasoning * and for why the failure is a throw rather than a field on the envelope. + * + * ⚠️ …with ONE positively-identified exception, ruled the same day: a read that + * failed because the table was never PROVISIONED is quiet, because there + * "zero capabilities" is the true answer rather than an invention. The full + * boundary, the ruling's verbatim text and the risk signed off with it are in + * `tryFind`'s `catch` — read that before changing anything on this path. */ import { AuthzStoreUnavailableError } from './authz-store-unavailable.js'; +// [#13279, ruled 2026-08-30] The one "was this READ failure just an +// unprovisioned table?" predicate. It was `@objectstack/metadata`'s until this +// resolver needed it; metadata depends on core, so the ruling relocated it to +// `@objectstack/types` — which core already depends on — rather than let a +// second copy of a security-relevant classifier exist. See `tryFind`'s catch. +import { isMissingTableError } from '@objectstack/types'; import { mapMembershipRole, BUILTIN_IDENTITY_PLATFORM_ADMIN, @@ -166,6 +178,57 @@ async function tryFind( // wired" is not a failed read, and an embedder that never configured a data // plane must keep resolving to an empty-but-valid envelope exactly as // before. Only a read that was ISSUED and THREW reaches here. + // + // ── The boundary, and the risk signed off with it ────────────────────── + // + // "The read failed" is NOT one fact, and the first implementation of this + // ruling proved it by turning four CI suites red. A read also throws when + // the table was never PROVISIONED — a real engine, wired and reachable, + // whose `sys_*` tables were never created. That is a supported, deliberately + // tested deployment shape; `packages/runtime`'s + // `notifications.hono.integration.test.ts` names it `ABSENT_AUTHZ_TABLES`. + // There, "zero capabilities" is the TRUE answer rather than a fabrication: + // nothing is provisioned, so nothing was withheld. Only an UNREACHABLE + // store — the ruling's own word 不可达 — leaves the answer UNKNOWN, and only + // an unknown answer may not be reported as a capability denial. + // + // Maintainer ruling, 2026-08-30, 第 5 场总监席决裁批 #9, verbatim: + // + // > 采选项 A —— 把 `isMissingTableError` 从 `@objectstack/metadata` 迁至 + // > `@objectstack/types`(core 已依赖);metadata 保留 re-export 兼容; + // > `tryFind` 仅对未被判定为「表未 provision」的读失败抛 + // > `AuthzStoreUnavailableError`(SERVICE_UNAVAILABLE / 503)。 + // + // ⚠️⚠️ THE SIGNED-OFF RISK — beside the predicate, as the ruling requires. + // Ruling, verbatim: + // + // > 签字在案:基于 driver 错误码的表缺失判定获准在安全路径上门控响亮性; + // > 其假阳方向(误判「表缺失」⇒ 静默恢复安静 403)是本裁定接受的已知风险, + // > 须在谓词旁注释写明并以测试钉住两个方向(真 outage ⇒ 响;真未 provision + // > ⇒ 零能力为真答案,不响)。 + // + // Plainly: this predicate now GATES loudness on a security path. Its + // false-POSITIVE direction — calling a genuine outage "table not + // provisioned" — silently restores the quiet 403 this card exists to + // remove, with no thrown error, no log line, and no other failing test to + // notice it. That direction is an ACCEPTED, RECORDED risk, not an + // oversight. It is acceptable because the predicate keys on driver CODES, + // SQLSTATEs and errnos first, excludes the known superstring traps up + // front, and returns `false` for anything it does not POSITIVELY + // recognise — an unrecognised outage stays loud by default. + // + // ⛔ Do NOT widen `isMissingTableError` to make a first boot quieter: every + // widening moves outages into this quiet branch. ⛔ Do NOT add a second + // predicate here to hedge the accepted risk — a hedge makes loudness + // conditional on two classifiers agreeing, which is strictly more ways to + // fall silent, and the maintainer signed off the single-classifier design. + // Both directions are pinned by name in `authz-store-unavailable.test.ts` + // ('THE OUTAGE DIRECTION' / 'THE UNPROVISIONED DIRECTION'); keep them. + // + // `object` is passed as `readObject` so the #13324 narrowing applies: a + // phrase that names some OTHER relation is not evidence about the table + // this read asked for, and stays loud. + if (isMissingTableError(err, object)) return []; throw new AuthzStoreUnavailableError(object, err); } } @@ -179,8 +242,9 @@ async function tryFind( * resolve across a permission-store outage is to report a capability set the * resolver never actually read. It now throws exactly one error — * {@link AuthzStoreUnavailableError}, when a permission-store read was issued - * and failed. Every other path still resolves, including every MISSING-service - * path. A transport that fails closed on unexpected throws should re-raise this + * and failed FOR A REASON THAT IS NOT AN UNPROVISIONED TABLE. Every other path + * still resolves, including every MISSING-service path and every + * never-provisioned one. A transport that fails closed on unexpected throws should re-raise this * one ({@link isAuthzStoreUnavailableError}) rather than degrade it to a * refusal — degrading it restores the disguise the ruling removed. */ diff --git a/packages/metadata/src/errors.ts b/packages/metadata/src/errors.ts index 19e3b63e7f..320864d408 100644 --- a/packages/metadata/src/errors.ts +++ b/packages/metadata/src/errors.ts @@ -24,9 +24,23 @@ * `@objectstack/spec/shared`). Architecturally attractive and explicitly * *not* precluded by this module — but out of scope on the round that * needed it (spec was frozen; types was under concurrent change). + * ⇒ **TAKEN, by the maintainer's 2026-08-30 ruling on #13279.** The + * predicate now lives in `@objectstack/types` + * (`driver-error-classification.ts`); what forced it was a consumer this + * file could never serve — `resolveAuthzContext` in `@objectstack/core`, + * which metadata **depends on**, so the edge could not point that way. * 3. **Export it deliberately from its current home** — this file. One * declaration, one implementation, one place a new driver quirk is taught. * + * ## What this file is now + * + * Option 2 is taken, so this is the compatibility seam it always said it would + * become — "a single, greppable seam to delete if the maintainer later takes + * option 2". It is NOT deleted: `@objectstack/metadata/errors` is a published + * subpath with out-of-repo consumers, and #13279 is a fix, not a removal. It + * re-exports the one symbol it always exported, from the new home. Everything + * below still describes why the subpath exists and why it stays narrow. + * * ## Why a subpath and not the package entry * * `@objectstack/metadata`'s root entry pulls the manager, every loader and the @@ -39,12 +53,13 @@ * * ## Scope of the promise * - * Only {@link isMissingTableError} is exported: it has a cross-package - * consumer today. Its sibling `isSchemaAlreadyExistsError` deliberately stays - * internal to this package — it has no consumer outside it, and an exported - * symbol nobody imports is a promise made for nothing (Prime Directive #10, - * pointed at our own API surface). Add it here the day something outside - * `@objectstack/metadata` needs it, not before. + * Only {@link isMissingTableError} is re-exported HERE. Its sibling + * `isSchemaAlreadyExistsError` moved to `@objectstack/types` with it (they are + * two signatures over one matcher and cannot be separated without re-rolling + * it), but it does not need a second door: nothing imports it through + * `@objectstack/metadata/errors`, and an exported symbol nobody imports is a + * promise made for nothing (Prime Directive #10, pointed at our own API + * surface). Anything that needs it reads `@objectstack/types` directly. */ -export { isMissingTableError } from './utils/schema-sync-errors.js'; +export { isMissingTableError } from '@objectstack/types'; diff --git a/packages/metadata/src/loaders/database-loader.ts b/packages/metadata/src/loaders/database-loader.ts index 4aa81dd82a..f376c6bbe2 100644 --- a/packages/metadata/src/loaders/database-loader.ts +++ b/packages/metadata/src/loaders/database-loader.ts @@ -26,7 +26,10 @@ import type { IDataDriver, IDataEngine, DriverQuery } from '@objectstack/spec/co import type { MetadataLoader } from './loader-interface.js'; import { calculateChecksum } from '../utils/metadata-history-utils.js'; import { LRUCache } from '../utils/lru-cache.js'; -import { isMissingTableError, isSchemaAlreadyExistsError } from '../utils/schema-sync-errors.js'; +// [#13279] Both predicates moved to `@objectstack/types` — see its +// `driver-error-classification.ts` `## Home` section. The verdicts are +// byte-identical; only the import path changed. +import { isMissingTableError, isSchemaAlreadyExistsError } from '@objectstack/types'; import { migrateProjectIdToEnvironmentId } from '../migrations/migrate-project-id-to-environment-id.js'; /** diff --git a/packages/services/service-datasource/src/external-datasource-service.ts b/packages/services/service-datasource/src/external-datasource-service.ts index 51a6beaef6..f8547d8e67 100644 --- a/packages/services/service-datasource/src/external-datasource-service.ts +++ b/packages/services/service-datasource/src/external-datasource-service.ts @@ -764,7 +764,8 @@ export class ExternalDatasourceService implements IExternalDatasourceService { * from a successfully read schema in which the table is absent. A throw * means the comparison never ran, and per the repo's read-failure * classification precedent (`READ_FAILURE_DISCRIMINATORS`, - * `packages/metadata/src/utils/schema-sync-errors.ts`: a fact verdict must + * `packages/types/src/driver-error-classification.ts` (#13279 moved it there + * from `packages/metadata/src/utils/schema-sync-errors.ts`): a fact verdict must * be POSITIVELY EARNED, never defaulted to), no signature test on the thrown * value can earn a claim about a remote schema nobody read. Deliberately NOT * a hand-rolled `err.code` allowlist — an unrecognised connection error diff --git a/packages/metadata/src/utils/schema-sync-errors.test.ts b/packages/types/src/driver-error-classification.test.ts similarity index 98% rename from packages/metadata/src/utils/schema-sync-errors.test.ts rename to packages/types/src/driver-error-classification.test.ts index 77be581545..e4f47fc8ff 100644 --- a/packages/metadata/src/utils/schema-sync-errors.test.ts +++ b/packages/types/src/driver-error-classification.test.ts @@ -4,6 +4,12 @@ * #4728 / #4825 — the classifications that decide whether a driver failure may * be silenced. * + * [#13279] Moved here with the module it tests, from + * `packages/metadata/src/utils/schema-sync-errors.test.ts`. Unchanged except + * for the import path: `@objectstack/core`'s authorization resolver now asks + * `isMissingTableError`, so the predicate lives in the package both sides + * already depend on. See the module's own `## Home` section. + * * Both directions are pinned deliberately, for both predicates. A test suite * that only proves the benign case is recognised would pass just as happily on * `() => true`, which is exactly the bug being fixed (one benign reason excusing @@ -11,7 +17,7 @@ */ import { describe, it, expect } from 'vitest'; -import { isMissingTableError, isSchemaAlreadyExistsError } from './schema-sync-errors.js'; +import { isMissingTableError, isSchemaAlreadyExistsError } from './driver-error-classification.js'; describe('isSchemaAlreadyExistsError', () => { describe('benign — the table/column is already provisioned', () => { diff --git a/packages/metadata/src/utils/schema-sync-errors.ts b/packages/types/src/driver-error-classification.ts similarity index 86% rename from packages/metadata/src/utils/schema-sync-errors.ts rename to packages/types/src/driver-error-classification.ts index 21a715fa0a..0cfab63753 100644 --- a/packages/metadata/src/utils/schema-sync-errors.ts +++ b/packages/types/src/driver-error-classification.ts @@ -1,8 +1,55 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * Driver-error classification for the metadata storage seams (#4728, #4825; - * rule from #4632). + * Driver-error classification: "which driver failures may be silenced?" + * (#4728, #4825; rule from #4632). + * + * ## Home — `@objectstack/types`, since #13279 + * + * This module was born in `@objectstack/metadata` and lived there through + * #4728 / #4825 / #5841. `@objectstack/metadata/errors`' own docblock recorded + * the move now made as **option 2** — "sink it into a common dependency + * (`@objectstack/types`) … architecturally attractive and explicitly *not* + * precluded by this module — but out of scope on the round that needed it" — + * and kept its export as "a single, greppable seam to delete if the maintainer + * later takes option 2". The maintainer took option 2 on 2026-08-30. + * + * What forced it: `resolveAuthzContext` (`@objectstack/core`) must ask + * {@link isMissingTableError} to tell a permission-store OUTAGE from a + * deployment whose `sys_*` tables were never provisioned (#13279). Core cannot + * import `@objectstack/metadata` — metadata **depends on** core — so the + * predicate had to move to a package both sides already depend on, or be + * copied. Copying was measured and rejected: two vocabularies of "which driver + * errors are benign", one of them on a security path, is the exact + * duplication-drift this module was built to retire. + * + * `@objectstack/types` is the repo's stated Home rule for a cross-package error + * predicate — see `unique-violation.ts`: "because every consumer of the + * question already depends on it, so adopting the predicate never adds an + * edge", which cites *this* predicate's #5841 move as its own precedent. The + * edge was already there in the other direction too: the front-exclusion below + * has read {@link isRelationSubObjectPhrase} from this package since #6615, so + * the move puts the phrase and the predicate that excludes on it in one place. + * + * `@objectstack/metadata/errors` — the published subpath — remains, and now + * re-exports from here, so no out-of-repo consumer changed. The package's + * INTERNAL `utils/schema-sync-errors.ts` is gone rather than left as a + * forwarding stub: it carried no promise to anyone, and a file that exists only + * to forward is the thing that rots. Its two in-package readers + * (`errors.ts`, `loaders/database-loader.ts`) import this module directly. + * + * ## Both predicates moved, not one + * + * The ruling names {@link isMissingTableError}. Its sibling + * {@link isSchemaAlreadyExistsError} came with it because they are **not two + * modules** — they are two signatures over one {@link matchesDriverError}, and + * that sharing is the point (see the paragraph below). Leaving the sibling + * behind would have meant either exporting the matcher as machinery or + * re-rolling it in `metadata`, and the second is the duplication this module + * exists to prevent. `@objectstack/types` therefore publishes both; the + * "exported symbol nobody imports" objection recorded in + * `@objectstack/metadata/errors` does not apply, because after the move + * `metadata`'s `DatabaseLoader` **is** an outside consumer of it. * * Two questions live here, and they share one mechanism on purpose. A second * hand-rolled `catch`-and-guess elsewhere in this package would be a second @@ -83,10 +130,12 @@ * ``` */ -// [#6615] The Postgres `"x" of relation "y"` phrase, owned once — see the -// module docblock in `@objectstack/types` for the superstring hole it closes -// and for why the exclusion's width deliberately differs from the extractor's. -import { isRelationSubObjectPhrase } from '@objectstack/types'; +// [#6615] The Postgres `"x" of relation "y"` phrase, owned once — see +// `relation-sub-object.ts` next door for the superstring hole it closes and for +// why the exclusion's width deliberately differs from the extractor's. That +// module was already this one's dependency across the package boundary; since +// #13279 moved this file into `@objectstack/types`, the two are siblings. +import { isRelationSubObjectPhrase } from './relation-sub-object.js'; /** * The relation name each missing-table phrase puts on display, one capture per @@ -323,7 +372,8 @@ const MISSING_TABLE: DriverErrorSignature = { * * [#6615] All three now read one home — `@objectstack/types` — instead * of three hand-kept copies, so the phrase can no longer be taught to - * the repo a fourth time or drift in one package only. The **width** + * the repo a fourth time or drift in one package only. [#13279] This + * file now lives in that same home, so the read is a sibling import. The **width** * difference that used to justify the copy is preserved and is the * reason the home exports two functions rather than one: those two * *extract* the column name to phrase a better error, so a miss costs a @@ -462,8 +512,9 @@ export function isSchemaAlreadyExistsError(error: unknown, depth = 0): boolean { * * Pass `readObject` from every in-repo call site. It is **optional** so that * omitting it is exactly the pre-#13324 behaviour rather than a new loud - * failure — this is a published export (`@objectstack/metadata/errors`), and a - * required parameter would be a breaking change to it. The cost of the choice + * failure — this is a published export (`@objectstack/types`, and still + * `@objectstack/metadata/errors` by re-export), and a required parameter would + * be a breaking change to it. The cost of the choice * is that the narrowing is opt-in per call site: a new caller that forgets it * silently gets the old, wider verdict. * diff --git a/packages/types/src/error-leak.test.ts b/packages/types/src/error-leak.test.ts index a3f492a70a..72db245cd6 100644 --- a/packages/types/src/error-leak.test.ts +++ b/packages/types/src/error-leak.test.ts @@ -137,7 +137,8 @@ describe('looksLikeInternalErrorLeak — shipped-dialect phrasings (#8132)', () ['postgres permission denied for relation', 'permission denied for relation sys_user'], // SQLite/libsql message-only errors: the same conditions with NO // `SQLITE_` prefix to trip the existing limb. Measured shapes in this - // repo — `metadata/src/utils/schema-sync-errors.ts` documents both. + // repo — `driver-error-classification.ts` (next door, moved here by + // #13279 from `metadata/src/utils/schema-sync-errors.ts`) documents both. ['sqlite bare missing table', 'no such table: sys_metadata'], ['sqlite bare missing table with a schema prefix', 'no such table: main.sys_metadata_history'], ['sqlite bare missing column', 'no such column: bogus'], diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 0e4a597111..a36698a4bd 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -27,6 +27,16 @@ export * from './relation-sub-object.js'; // Four hand-written vocabularies used to answer it and disagreed about MySQL, // which is why every MySQL conflict came back 500 instead of 409. export * from './unique-violation.js'; +// [#4728/#4825, moved here by #13279] The one "which driver failures may be +// silenced?" vocabulary — `isMissingTableError` (a READ failed because the +// table was never provisioned) and `isSchemaAlreadyExistsError` (a DDL failure +// that was just the table already being there). It was `@objectstack/metadata`'s +// until `@objectstack/core`'s authorization resolver had to ask it, and core +// cannot import metadata — metadata depends on core. Same Home rule as +// `unique-violation.js` above: every consumer already depends on this package, +// so adopting the predicate adds no edge. `@objectstack/metadata/errors` still +// re-exports `isMissingTableError` for its published consumers. +export * from './driver-error-classification.js'; // [#8567] The OPPOSITE question, kept deliberately separate: "is this the // database refusing an ON CONFLICT target that no unique index backs?" One // measured limb per dialect that can raise it (SQLite, Postgres); MySQL cannot, diff --git a/packages/types/src/unique-violation.ts b/packages/types/src/unique-violation.ts index d9b796305d..1e63ff6b56 100644 --- a/packages/types/src/unique-violation.ts +++ b/packages/types/src/unique-violation.ts @@ -119,7 +119,9 @@ interface UniqueViolationSignature { * (from `service-messaging`). * - `1062` — the same MySQL condition on the channel mysql2 *also* sets. The * one addition, and not a new dialect: `@objectstack/metadata`'s - * `schema-sync-errors.ts` already reads `errno` alongside `code` for exactly + * `driver-error-classification.ts` (this package since #13279; it was + * `metadata/src/utils/schema-sync-errors.ts`) already reads `errno` + * alongside `code` for exactly * these drivers, so a code-only read is a known gap rather than a decision. * - `UNIQUE_VIOLATION` — the PLATFORM's own registered code * (`error-code-ledger.zod.ts`), added by #13197 when `driver-memory` grew From 0b71a8a13e122a0ce9572b174ed4bb91ade18542 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 16:23:41 +0000 Subject: [PATCH 6/7] docs(permissions): re-anchor the system-context census after the main merge `rest-server.ts` moved on BOTH sides of the merge, so neither side's anchors were correct afterwards. `--fix` repaired `package-routes.ts:92 -> :97` and then refused `rest-server.ts` with "page anchors 7 distinct read line(s), census finds 6 -- the POPULATION changed". That refusal is FALSE, and the same run says so: its own `[ledger-row-unused]` line reports NON_READ_ANCHORS excusing `rest-server.ts:1238` with no anchor pointing there. The page carries EIGHT anchors into that file -- six elevation reads plus the two `NON_READ_ANCHORS`-excused inbound seams -- and `--fix` counts the excused pair as read anchors. Filed as #13490. Re-anchored by hand, every target proven BY CONTENT rather than by arithmetic: :1235 -> :1238 holds the exact ledger needle '"authenticated". `isSystem` flags are never set on inbound HTTP' :1264 -> :1267 holds the exact ledger needle '`isSystem` is never set on inbound HTTP, so it cannot bypass.' :1267 -> :1270 row 17's READ anchor. The arithmetic could not see this: :1267 was simultaneously the second ledger seam's new home and row 17's stale anchor. :4271 -> :4282, :5634 -> :5645, :5866 -> :5877, :6211 -> :6222, :6404 -> :6415 row 50's five read anchors, each the census's own read site No row was written, deleted or reworded -- only where they point. Population proven unchanged against origin/main in a compare worktree: both verdict lines are byte-identical, "109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read". Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- content/docs/permissions/system-context.mdx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index d32c01255b..0b867113f1 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -64,7 +64,7 @@ not on any flag. ## How the flag is set `isSystem` is **server-constructed and never client-supplied**. Inbound HTTP -cannot set it (`packages/rest/src/rest-server.ts:1235`, `:1264`), and neither +cannot set it (`packages/rest/src/rest-server.ts:1238`, `:1267`), and neither can an action body (`packages/runtime/src/domains/actions.ts:404`). It is written by internal callers only, as an option on the engine call: @@ -103,7 +103,7 @@ that silently does not happen. | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:246` | | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` | | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` | -| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1267` | +| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1270` | ### 2. Write pipeline and data integrity @@ -158,9 +158,9 @@ The largest single consumer — **20 of the 109 sites**. |:--|:---|:---|:---|:---| | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:136` | | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:399` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4271`, `:5634`, `:5866`, `:6211`, `:6404` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4282`, `:5645`, `:5877`, `:6222`, `:6415` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | -| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:246`, `external-datasource-routes.ts:302`, `package-routes.ts:92` | +| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:246`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | | 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:102` | | 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:95`, `:128` | @@ -199,7 +199,7 @@ assuming `isSystem` covers it is a documented source of bugs. | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1451` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:273` | -| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1235`, `:1264`; `domains/actions.ts:404` | +| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1238`, `:1267`; `domains/actions.ts:404` | --- From cfe54c80d08462121f7f51db504f4a5b9f70122d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 17:32:23 +0000 Subject: [PATCH 7/7] fix(rest): convert the four surviving execCtx swallow sites, and enforce the census claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contract review (条款②) on PR #13475 found four `this.resolveExecCtx(environmentId, req)` sites still passing `.catch(() => undefined)` — the CONTINUATION-layout ones the census file's own docblock warns about in as many words ("a single-line grep counts 16 and misses those four"). The conversion had reached the 16 single-line sites only. serveMetaItemLayered, GET meta/:type, GET meta/:type/:name and GET meta/:type/:name/published each swallowed the loud failure, so during a permission-store outage they could serve an org-unscoped, env-wide 200 instead of the declared 503. Converted, same shape as the other 16. Measured after: 20 resolveExecCtx sites guarded (16 inline + 4 continuation), 0 remaining `() => undefined` after a resolver call, plus computeExecCtx's own net = 21 guarded sites. ⛔ The sharper half was a FALSE COMPLETENESS CLAIM this PR shipped in the census file: "every site now passes `rethrowAuthzStoreUnavailable`". It was false when written, in the very file that documents the trap, and no pin failed — the sibling ledger's per-transport check is PRESENCE-based, so one converted site satisfies it for the whole file and a PARTIAL conversion is invisible to it. So the sentence is not merely corrected; it is replaced by measurement: - §7 re-derives the catch ARGUMENT at every site from source, in both layouts, and fails on any `() => undefined` survivor or any local re-spelling of the shared guard. A CONTROL asserts it finds both layouts, so a regex that stopped matching cannot read as a clean pass. - §8 drives the doors: with the resolver rejecting, no route touching a continuation site answers 200, and each keeps the declared 503 or propagates. Its healthy leg is the anti-vacuity control — it proves those sites are reached AND that 200 is their healthy answer, which is exactly what the defect fabricated. - The docblock records the miss, and states the ledger's reach so nobody reads its green as covering this. ⚠️ §8's OWN reach is recorded too, and it was measured by ablation rather than assumed: reverting one of the four sites turns §7 red and leaves §8 GREEN, because the `${metaPath}/:type` handler (4332-4794) resolves the context three times, so under a total outage a later guarded site still refuses. §8 pins what a DOOR answers; §7 pins what each SITE spells. The case neither covers — a PARTIAL outage where only the first read fails, which is where the org-unscoped 200 actually appears — is stated as not measured rather than implied away. Site lines are DERIVED, never transcribed: hardcoded numbers in this file are what went stale and hid the four sites. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .../rest/src/execctx-consumer-census.test.ts | 197 +++++++++++++++++- packages/rest/src/rest-server.ts | 8 +- 2 files changed, 199 insertions(+), 6 deletions(-) diff --git a/packages/rest/src/execctx-consumer-census.test.ts b/packages/rest/src/execctx-consumer-census.test.ts index 8efe709455..e35edd7870 100644 --- a/packages/rest/src/execctx-consumer-census.test.ts +++ b/packages/rest/src/execctx-consumer-census.test.ts @@ -56,7 +56,12 @@ import { describe, it, expect, vi } from 'vitest'; import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { dirname, resolve } from 'node:path'; -import { ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_STATUS } from '@objectstack/core'; +import { + ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_STATUS, + // [#13279] §8 drives the real loud failure rather than a stand-in, so the + // propagation it observes is the one production raises. + AuthzStoreUnavailableError, AUTHZ_STORE_UNAVAILABLE_STATUS, +} from '@objectstack/core'; import { RestServer } from './rest-server.js'; const HERE = dirname(fileURLToPath(import.meta.url)); @@ -73,7 +78,7 @@ type Handler = (req: any, res: any) => any; * Every `this.resolveExecCtx(environmentId, req)` invocation, with the line it * sits on and whether it carries its OWN `.catch(…)`. * - * [#13279] The catch ARGUMENT changed — every site now passes + * [#13279] The catch ARGUMENT changed: a caught site passes * `rethrowAuthzStoreUnavailable` instead of `() => undefined`, so a * permission-store outage is re-raised rather than degraded into a refusal. * The detection below keys on `.catch(` and is deliberately spelling-agnostic, @@ -82,6 +87,25 @@ type Handler = (req: any, res: any) => any; * ⚠️ The catch may sit on the CONTINUATION line — four of them do. A * single-line grep counts 16 and misses those four, which is how the thread's * "16 caught / 52 bare" split came to name two numbers that do not add to 72. + * + * ⛔ THAT WARNING CAUGHT ITS OWN AUTHOR, AND THE RECORD SAYS SO. The first + * revision of this block claimed "**every** site now passes + * `rethrowAuthzStoreUnavailable`". It was FALSE when written: the conversion + * had reached the 16 single-line sites and none of the four continuation ones + * (their `.catch` lines were 2747, 4388, 5226, 6759 at that revision; §7 and §8 + * derive the numbers rather than quoting them) — the exact miss the paragraph + * above describes, committed by the person who had just written it down. + * Contract review found it. The sentence is now QUANTIFIED and, more to the + * point, ENFORCED: §7 below re-derives the argument at every site from source + * and fails on any `() => undefined` survivor, so this is a measurement rather + * than a claim a reader has to trust. + * + * ⚠️ Reach of the sibling ledger, so nobody reads its green as covering this: + * `authz-store-unavailable.test.ts`'s per-transport check is PRESENCE-based — + * it asks whether the file CONTAINS `isAuthzStoreUnavailableError` or + * `rethrowAuthzStoreUnavailable` at all. One converted site satisfies it for + * the whole file, so it cannot see a PARTIAL conversion and no pin there failed + * while those four sites stood. §7 is what closes that gap for this file. */ function siteTable(): { line: number; caught: boolean; nextLine: string }[] { const lines = SOURCE.split('\n'); @@ -516,3 +540,172 @@ describe('[#13160] §6 the boundary of this census', () => { expect(SITES.some((s) => s.line === wrapper + 1)).toBe(true); }); }); + +// --------------------------------------------------------------------------- +// 7. ⭐ The catch ARGUMENT at every site — the half a presence check cannot see +// +// Added after contract review found this file asserting, in prose, that +// "every site now passes `rethrowAuthzStoreUnavailable`" while four sites +// still passed `() => undefined`. Prose cannot rot loudly; this can. +// +// Why the sibling ledger did not catch it: that check is PRESENCE-based +// (does the file mention the guard at all), so one converted site satisfies +// it for the whole file. A partial conversion is exactly what it cannot see, +// and a partial conversion on this surface means a live door still launders +// a store OUTAGE into an org-unscoped 200. +// --------------------------------------------------------------------------- + +/** The catch argument at each caught site, read from source, both layouts. */ +function catchArguments(): { line: number; arg: string; layout: 'inline' | 'continuation' }[] { + const lines = SOURCE.split('\n'); + const out: { line: number; arg: string; layout: 'inline' | 'continuation' }[] = []; + lines.forEach((text, i) => { + if (!text.includes('this.resolveExecCtx(environmentId, req)')) return; + const inline = /\.catch\(([^)]*(?:\)[^)]*)*)\);?\s*$/.exec(text); + if (text.includes('.catch(') && inline) { + out.push({ line: i + 1, arg: inline[1].trim(), layout: 'inline' }); + return; + } + const next = (lines[i + 1] ?? '').trim(); + const cont = /^\.catch\((.*)\);$/.exec(next); + if (cont) out.push({ line: i + 1, arg: cont[1].trim(), layout: 'continuation' }); + }); + return out; +} + +describe('[#13279] §7 every caught resolveExecCtx site re-raises the outage', () => { + it('CONTROL: the reader finds sites in BOTH layouts, so neither can pass vacuously', () => { + // Without this, a regex that silently stopped matching the continuation + // form would report "0 survivors" and read exactly like a clean pass — + // which is the precise failure that let the four sites through. + const args = catchArguments(); + expect(args.filter((a) => a.layout === 'inline').length).toBeGreaterThanOrEqual(16); + expect(args.filter((a) => a.layout === 'continuation').length).toBeGreaterThanOrEqual(4); + expect(args.length).toBe(CAUGHT.length); + }); + + it('⭐ NO caught site swallows with `() => undefined`', () => { + const swallowers = catchArguments().filter((a) => /=>\s*undefined/.test(a.arg)); + // Named, so a failure says WHICH door is still laundering an outage. + expect(swallowers.map((s) => `${s.line} (${s.layout})`)).toEqual([]); + }); + + it('⭐ every caught site passes the shared guard, not a local re-spelling', () => { + // A local `(e) => { if (e?.status === 503) throw e; }` would satisfy the + // test above and still be a second, driftable copy of the predicate. + for (const a of catchArguments()) { + expect({ line: a.line, arg: a.arg }).toEqual({ line: a.line, arg: 'rethrowAuthzStoreUnavailable' }); + } + }); +}); + +// --------------------------------------------------------------------------- +// 8. ⭐ BEHAVIOURAL: the four continuation-layout doors PROPAGATE the outage +// +// §7 proves the source says `rethrowAuthzStoreUnavailable` at every site. +// That is a claim about text. This section drives the doors and observes +// what they ANSWER when the resolver rejects with the real error, because +// the defect these four carried was not a spelling: during a store outage +// they swallowed the loud failure and served an org-unscoped, env-wide +// `200` — a fabricated success, not a refusal. +// +// ⚠️ The site lines are DERIVED from §7, never transcribed. This file's own +// history is that hardcoded line numbers in it went stale and hid four +// sites; a literal here would be the same mistake in the test that exists +// to catch it. +// +// ⛔ REACH OF THIS SECTION, measured by ablation rather than assumed — read +// it before trusting §8 as the tripwire for a single site. Reverting ONE of +// the four sites to `() => undefined` turns §7 red and leaves §8 GREEN. That +// is not a gap in the assertion, it is the shape of the handlers: the +// `${metaPath}/:type` handler registered at line 4332 runs to 4794 and +// resolves the context THREE times (the continuation site, then two guarded +// single-line sites), so under the TOTAL outage this section drives, a later +// site still throws and the door still refuses. §8 therefore pins what a +// door ANSWERS; §7 pins what each SITE spells. Only §7 fails on one reverted +// site, and that is the division of labour to keep. +// +// ⚠️ Which also names what neither section covers: a PARTIAL outage, where +// only the first read fails. There the swallow returns `undefined`, the +// handler proceeds with no `organizationId`, and the door answers the +// org-unscoped env-wide 200 that the conversion exists to prevent. Driving +// that needs a per-read fault injector rather than a rejecting resolver; +// it is NOT MEASURED here, and is stated so rather than implied away. +// --------------------------------------------------------------------------- + +/** As `instrument`, but the resolver REJECTS — the production outage shape. */ +function instrumentRejecting(err: unknown) { + const proto: any = (RestServer as any).prototype; + const original = proto.resolveExecCtx; + const hits = new Map>(); + const state = { route: '' }; + proto.resolveExecCtx = async function () { + const m = (new Error().stack ?? '').match(/rest-server\.ts:(\d+):\d+/); + if (m) { + if (!hits.has(state.route)) hits.set(state.route, new Set()); + hits.get(state.route)!.add(Number(m[1])); + } + throw err; + }; + return { hits, state, restore: () => { proto.resolveExecCtx = original; } }; +} + +async function sweepRejecting(err: unknown, mount: 'FULL' | 'ISOLATED'): Promise { + const probe = instrumentRejecting(err); + const { rs, table } = makeServer(metaProtocol(OBJECT_DOC)); + if (mount === 'FULL') rs.registerRoutes(); else rs.registerMetadataEndpointsInner(BASE); + const rows: Row[] = []; + for (const [key, handler] of table) { + probe.state.route = key; + const [method, pattern] = key.split(' '); + const observed = await call(handler, method, pattern, paramsFor(pattern, 'object', OBJECT_DOC.name)); + rows.push({ route: key, sites: [...(probe.hits.get(key) ?? [])].sort((a, b) => a - b), ...observed }); + } + probe.restore(); + return rows; +} + +describe('[#13279] §8 an outage at the four continuation sites is not served as success', () => { + const CONTINUATION = catchArguments().filter((a) => a.layout === 'continuation').map((a) => a.line); + + // ISOLATED, and the reason is measured rather than stylistic: under the FULL + // mount `registerMetadataEndpoints`'s umbrella guard resolves the context + // FIRST and refuses there, so the handler never reaches these four sites and + // a sweep observes nothing about them. Isolating the floor is the same move + // §6 documents — a counterfactual that reads a site on its OWN behaviour, + // never a production posture. + const MOUNT = 'ISOLATED' as const; + + it('CONTROL: the four sites are REACHED by real routes, and 200 is their HEALTHY answer', async () => { + // Two things at once, and both are needed. Without the reachability half, + // "no 200 during an outage" is satisfied by a door that never runs. + // Without the 200 half, it is satisfied by a door that never returned 200 + // in the first place — and the defect being pinned is precisely that + // these doors served an org-unscoped 200 while the store was down. + expect(CONTINUATION.length).toBeGreaterThanOrEqual(4); + const healthy = await sweep(ENTITLED, MOUNT); + const touched = healthy.filter((r) => r.sites.some((line) => CONTINUATION.includes(line))); + expect(touched.length).toBeGreaterThan(0); + expect(touched.filter((r) => r.status === 200).length).toBeGreaterThan(0); + }); + + it('⭐ no route that touches a continuation site fabricates a 200 during an outage', async () => { + const rows = await sweepRejecting(new AuthzStoreUnavailableError('sys_user_permission_set'), MOUNT); + const touched = rows.filter((r) => r.sites.some((line) => CONTINUATION.includes(line))); + expect(touched.length).toBeGreaterThan(0); + // Named, so a regression says WHICH door started inventing an answer. + expect(touched.filter((r) => r.status === 200).map((r) => r.route)).toEqual([]); + }); + + it('⭐ the outage keeps its declared 503 rather than turning into a refusal', async () => { + const rows = await sweepRejecting(new AuthzStoreUnavailableError('sys_user_permission_set'), MOUNT); + const touched = rows.filter((r) => r.sites.some((line) => CONTINUATION.includes(line))); + for (const r of touched) { + // Either the declared 503 reached the caller, or it propagated out of + // the handler for `handleRouteError` to render. Never a 401/403 — + // that disguise is the whole defect the ruling removed. + const ok = r.threw !== undefined || r.status === AUTHZ_STORE_UNAVAILABLE_STATUS; + expect({ route: r.route, status: r.status, ok }).toEqual({ route: r.route, status: r.status, ok: true }); + } + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index d156433ef3..0350733aec 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -2744,7 +2744,7 @@ export class RestServer { // non-overridable type keeps reading env-wide (see that predicate for // why naming the org unconditionally would resurrect #6190's phantoms). const layeredCtx = await this.resolveExecCtx(environmentId, req) - .catch(() => undefined); + .catch(rethrowAuthzStoreUnavailable); const layeredOrganizationId = organizationIdForMetaRead( // [#10340] FOLDED, not raw — see the PUT door's org-scope comment // for the measurement. @@ -4385,7 +4385,7 @@ export class RestServer { // simply not in the list. Same memoised `resolveExecCtx` // and same registry gate as every other read door here. const listCtx = await this.resolveExecCtx(environmentId, req) - .catch(() => undefined); + .catch(rethrowAuthzStoreUnavailable); const listOrganizationId = organizationIdForMetaRead( // [#10340] FOLDED, not raw — see the PUT door's // org-scope comment for the measurement. @@ -5223,7 +5223,7 @@ export class RestServer { // ⚠️ NOT a new seam: memoised per request, and this // handler resolves the same context again further down. const readCtx = await this.resolveExecCtx(environmentId, req) - .catch(() => undefined); + .catch(rethrowAuthzStoreUnavailable); const readOrganizationId = organizationIdForMetaRead( // [#10340] FOLDED, not raw — see the PUT door's // org-scope comment for the measurement. @@ -6756,7 +6756,7 @@ export class RestServer { if (typeof publishedProtocol?.getMetaItemLayered === 'function') { try { const publishedCtx = await this.resolveExecCtx(environmentId, req) - .catch(() => undefined); + .catch(rethrowAuthzStoreUnavailable); const layered = await publishedProtocol.getMetaItemLayered({ type, name,