diff --git a/.changeset/authz-read-failure-fails-loud.md b/.changeset/authz-read-failure-fails-loud.md new file mode 100644 index 0000000000..b1fa7b6f3e --- /dev/null +++ b/.changeset/authz-read-failure-fails-loud.md @@ -0,0 +1,130 @@ +--- +"@objectstack/core": minor +"@objectstack/types": minor +"@objectstack/metadata": patch +"@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 可区分)。 + +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 +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 (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, 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. + +**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; 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 +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/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 32fe1684d7..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 @@ -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: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: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: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` | --- 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..3116a8bf56 --- /dev/null +++ b/packages/core/src/security/authz-store-unavailable.test.ts @@ -0,0 +1,416 @@ +// 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|\{\})/); + } + }, + ); +}); + +// --------------------------------------------------------------------------- +// 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 new file mode 100644 index 0000000000..6e5525f73f --- /dev/null +++ b/packages/core/src/security/authz-store-unavailable.ts @@ -0,0 +1,188 @@ +// 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". + * + * ## …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 —— 权限库不可达时 + * > 不再解析为「已认证零能力」,而是响亮拒绝(与真实能力拒绝的 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 e0ae8b7b1c..2f38dc0a6a 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -20,11 +20,31 @@ * 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. + * + * ⚠️ …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, @@ -148,14 +168,86 @@ 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. + // + // ── 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); } } /** - * 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 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. */ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise { const { ql, headers } = input; @@ -341,8 +433,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/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/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..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)); @@ -71,11 +76,36 @@ 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: 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, + * 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 * "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'); @@ -499,9 +529,183 @@ 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); }); }); + +// --------------------------------------------------------------------------- +// 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/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 2f63997669..0350733aec 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, @@ -1519,7 +1522,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); } /** @@ -1599,7 +1602,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; } @@ -1620,7 +1623,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 }; @@ -2042,8 +2045,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); } } @@ -2733,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. @@ -2806,7 +2817,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), @@ -3935,7 +3946,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 @@ -4175,7 +4186,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: { @@ -4264,7 +4275,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 : [], ); @@ -4374,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. @@ -4472,7 +4483,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 : [], @@ -4579,7 +4590,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) { @@ -5212,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. @@ -5400,7 +5411,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 : [], @@ -5474,7 +5485,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); @@ -5628,7 +5639,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, @@ -5860,7 +5871,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, @@ -6130,7 +6141,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 @@ -6205,7 +6216,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, @@ -6398,7 +6409,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, @@ -6745,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, 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-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/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 } }; 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