diff --git a/.changeset/demoted-code-withheld-on-undeclared-5xx.md b/.changeset/demoted-code-withheld-on-undeclared-5xx.md new file mode 100644 index 0000000000..203399d173 --- /dev/null +++ b/.changeset/demoted-code-withheld-on-undeclared-5xx.md @@ -0,0 +1,53 @@ +--- +"@objectstack/types": patch +--- + +fix(types): a DEMOTED `declaredCode` is withheld on a 5xx the producer did not declare (#12509, ADR-0112) + +**Wire change, for undeclared server faults only.** When a 5xx has its prose +withheld, the producer's demoted `declaredCode` is now withheld with it — +but only when the fallback-to-500 picked that code up from a producer that +declared no HTTP answer. An author-declared code is untouched at every status. + +FROM (`origin/main`, measured through the real routes): + +``` +POST /api/v1/packages/publish → 500 {"error":{"code":"INTERNAL_ERROR", + "message":"Internal server error","declaredCode":"SQLITE_ERROR"}} +POST /api/v1/analytics/query → 500 {"error":{"code":"INTERNAL_ERROR", + "message":"Internal server error","httpStatus":500,"declaredCode":"42P01"}} +``` + +TO: + +``` +POST /api/v1/packages/publish → 500 {"error":{"code":"INTERNAL_ERROR", + "message":"Internal server error"}} +POST /api/v1/analytics/query → 500 {"error":{"code":"INTERNAL_ERROR", + "message":"Internal server error","httpStatus":500}} +``` + +UNCHANGED — the author-authored channel the ADR-0112 amendment wrote +`declaredCode` for: + +``` +{ status: 503, code: 'ACME_LEDGER_OFFLINE' } + → 503 {"error":{"code":"SERVICE_UNAVAILABLE",…,"declaredCode":"ACME_LEDGER_OFFLINE"}} +``` + +`SQLITE_ERROR` vs `42P01` names the backend, which is one of the two +disclosures the 5xx message withhold exists to prevent (the other, +identifiers, was already covered). Maintainer ruling 2026-08-27, option D. + +**What a consumer must know.** A `declaredCode` on a 5xx now means the +producer declared that fault itself, which is a stronger guarantee than the +field carried before; nothing that was a *registered* code moves, and no 4xx +moves. A producer that spells a code but declares no status loses that code +on a 5xx — declare the status the refusal means and the spelling is kept. + +The distinction lives in ONE place, `serverFaultProvenance` +(`packages/types/src/thrown-http-error.ts`), read by `demotedDeclaredCode` — +the read every door already makes — so all five emitting exits inherit it and +no registrar carries a variant. The prose axis of the same ruling (the +dispatcher door adopting the structural withhold for every declared 5xx +message) is #12281 and is deliberately not applied here. diff --git a/docs/adr/0112-error-code-vocabulary-and-ledger.md b/docs/adr/0112-error-code-vocabulary-and-ledger.md index 15cc3ea584..d98471da0c 100644 --- a/docs/adr/0112-error-code-vocabulary-and-ledger.md +++ b/docs/adr/0112-error-code-vocabulary-and-ledger.md @@ -135,6 +135,16 @@ Nine rulings, D1–D9. > > **Where it is pinned.** `packages/rest/src/rest-thrown-code-vocabulary.test.ts`, in both directions — an unregistered code demotes, and a registered code still arrives verbatim in `code`. The second half is the one a regression does not redden, which is why it is written down. +> **Amendment (2026-08-27, [#12509](https://github.com/objectstack-ai/objectstack/issues/12509)) — the channel has a 5xx SCOPE: a demoted code is withheld with the prose when the producer declared no server fault; an author-declared code survives.** The two amendments above say WHICH spelling goes on the wire; neither says whether the open channel is in scope for 5xx sanitisation, and at none of the doors was the answer "no" a decision anyone made — it was what each door's control flow produced. Measured on `origin/main` before this ruling: a bare driver error (`{ code: 'SQLITE_ERROR' }` or `{ code: '42P01' }`, **no declared status**) resolves to `500 INTERNAL_ERROR`, the door withholds the leaky message ([#8086](https://github.com/objectstack-ai/objectstack/issues/8086)) — and then puts the driver's own dialect on the wire in `declaredCode`. `SQLITE_ERROR` vs `42P01` names the backend, which is one of the two disclosures the message withhold exists to prevent (the other, identifiers, was already covered). +> +> **Ruled (maintainer, 2026-08-27).** In 5xx sanitisation a **demoted** code — one the fallback-to-500 picked up from an **undeclared** producer — is withheld along with the prose; an **author-declared** `declaredCode` survives. Implemented **once at the shared resolver layer so all doors inherit one rule**; ⛔ no per-registrar variants. The rejected alternatives, all three named: recording the leak as a decision (it rests on an untested premise that can rot), withholding the author channel too (that drops the channel for exactly the metadata-app 5xx refusals this ADR's amendment wrote it for), and a per-door rule (it re-creates the divergence the two amendments above just closed). +> +> **The discriminator is the STATUS channel, and it could not be anything else.** A driver errno and an app's own spelling both arrive on `.code` as a plain string, so telling them apart by looking at the string would be a heuristic over an open channel — the consumer-side tolerance this ADR exists to forbid, and unfalsifiable besides (nothing stops an app from spelling `SQLITE_ERROR`). "The producer declared an HTTP answer" is the one structural signal, and it is already computed: `ThrownHttpError.declaredStatus`. The cost is stated rather than hidden — **a producer that spells a code but declares no status loses that code on a 5xx**, and keeps it by declaring the status it means, which is the shape this ADR already asks for. The tenant-authored limb is untouched: `SandboxError` answers **400**, so the `DUPLICATE` witness rides a 4xx and is outside this scope entirely. +> +> **Where it is pinned.** `serverFaultProvenance` (`packages/types/src/thrown-http-error.ts`, anchored in `scripts/adr-anchors/`) is the ONE definition, read by `demotedDeclaredCode` — the read every door already makes, which is what makes "all doors inherit" a construction rather than five suites agreeing about literals. Pinned at the doors in `packages/types/src/thrown-http-error-5xx-code-withhold.test.ts` (the rule), `packages/rest/src/package-door-5xx-demoted-code-withhold.test.ts` and `packages/runtime/src/dispatcher-5xx-demoted-code-withhold.test.ts` (each door's wire, driven through its real route). +> +> **Scope, exactly.** This rules the CODE channel only. The PROSE axis of the same ruling — the dispatcher door adopting the structural withhold for every declared 5xx message — is [#12281](https://github.com/objectstack-ai/objectstack/issues/12281), a separate card with its own measurement-first step; it is the `'declared'` limb of the same `serverFaultProvenance` function and deliberately not applied here. + **D5 — One location, eventually: `error.code` carries the semantic code.** Target end-state, recorded here so the follow-ups have a fixed destination: the HTTP status lives on the transport and (optionally) `error.httpStatus`; `error.code` is always the semantic string; `error.details.code` and `error.type` are retired as code carriers. The dispatcher-occupation fix (#3689 sibling) and `ROUTE_NOT_FOUND`-in-`type` retirement land as follow-ups (Rollout, batch 3). The client's three-location probe is deleted only after both. > **Amendment (2026-07-30, [#4007](https://github.com/objectstack-ai/objectstack/issues/4007)) — ruled and done.** With batch 3 landed, the client's parking-spot read (`error.details.code`) is deleted: SDK and server ship as a changesets fixed group, so the "newer SDK, older server" pairing it served is not a supported deployment — and batches 1–2 renamed the code *values* anyway, so a code dug out of an old server's parking spot would match no branch written against the current catalog; location-compat without value-compat protects nothing. The client's two remaining reads are the two *live* envelopes' declared spots (flat top-level `code`, wrapped `error.code`) — a present-tense fact, not a fallback chain; retiring the flat shape itself belongs to the envelope-convergence line (#3843 family), not this ADR. The D9b nesting fix (`category`/`retryable`, [#4006](https://github.com/objectstack-ai/objectstack/issues/4006)) landed in the same change. diff --git a/packages/rest/src/package-door-5xx-demoted-code-withhold.test.ts b/packages/rest/src/package-door-5xx-demoted-code-withhold.test.ts new file mode 100644 index 0000000000..07f2b3ee69 --- /dev/null +++ b/packages/rest/src/package-door-5xx-demoted-code-withhold.test.ts @@ -0,0 +1,363 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#12509] This package's two doors inherit ADR-0112's 5xx code scope from the + * shared resolver — they do not each carry a copy of it. + * + * ## What was measured on the wire before the change + * + * `origin/main` @ `aef1b7e64`, through the REAL registrar rather than by + * reading source: + * + * ``` + * POST /api/v1/packages/publish, packageService.publish throws + * { code: 'SQLITE_ERROR' } → 500 {"error":{"code":"INTERNAL_ERROR", + * "message":"Internal server error","declaredCode":"SQLITE_ERROR"}} + * { code: '42P01' } → 500 {…,"declaredCode":"42P01"} + * ``` + * + * The prose withhold (#8086) had already fired — `message` is the generic + * sentence — and the driver's own dialect went out beside it, naming the + * backend. Ruled 2026-08-27 (option D): a demoted code the fallback-to-500 + * picked up from an UNDECLARED producer is withheld with the prose; an + * AUTHOR-DECLARED one survives. + * + * ## Why this door needs no edit, and why that is the point + * + * `sendThrownError` reads `demotedDeclaredCode(thrown)` and emits whatever it + * answers. The ruling put the judgement inside that function, so this door + * inherits it without a line changing here — and section 3 is what keeps that + * true: it compares the wire against the shared function itself, so a door + * that ever grew a rule of its own turns red. ⛔ Do not "fix" a red in + * section 3 by teaching this door the condition; that is the per-door variant + * the ruling declined. + * + * ## The flat `/data` door is measured, not assumed + * + * Section 4 drives a REAL `ObjectQL` and a driver that fails every access with + * a coded fault through the real CRUD routes. That door answers its own fixed + * sanitised 5xx bodies (`DATA_STORE_FAULT` / `UNCLASSIFIED_FAULT`) and never + * reaches `thrownCodeFields` on that path, so it never emitted the shape and + * nothing about it moves. Measured rather than inferred because "this door + * cannot produce it" is exactly the kind of claim that rots: the section is + * here so the day the classification changes, someone is told. + * + * ## Reverse verification + * + * Predicted before running: reverting the withhold in `demotedDeclaredCode` + * turns sections 1 and 3's undeclared rows RED and leaves sections 2 and 4 + * GREEN (the author-declared rows and the flat door's fixed bodies are + * satisfied by both implementations). Measured: see the PR body. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ApiErrorSchema, BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; +import type { RouteHandler } from '@objectstack/spec/contracts'; +import { ObjectQL } from '@objectstack/objectql'; +import { + resolveThrownHttpError, + demotedDeclaredCode, + INTERNAL_ERROR_MESSAGE, +} from '@objectstack/types'; +import { registerPackageRoutes } from './package-routes.js'; +import { RestServer } from './rest-server.js'; + +const PKGS = '/api/v1/packages'; + +interface Captured { + status: number; + body: any; +} + +/** A caller holding every capability these routes gate on. */ +const CLEARS_THE_GATE = async () => ({ + userId: 'u_pkg', + systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], +}); + +function mount(svc: Record) { + const routes = new Map(); + const server = { + get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, + post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, + put: (p: string, h: RouteHandler) => { routes.set(`PUT:${p}`, h); }, + delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, + patch: () => {}, use: () => {}, listen: async () => {}, close: async () => {}, + } as any; + registerPackageRoutes(server, () => svc as any, '/api/v1', { + resolveExecutionContext: CLEARS_THE_GATE, + } as any); + return routes; +} + +async function drive( + routes: Map, + method: string, + path: string, + req: Record = {}, +): Promise { + const handler = routes.get(`${method}:${path}`); + if (!handler) throw new Error(`no handler for ${method} ${path}`); + const captured: Captured = { status: 0, body: undefined }; + const res: any = { + json(data: any) { captured.body = data; }, send() {}, + status(code: number) { captured.status = code; return res; }, + header() { return res; }, + }; + await handler({ params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any, res); + return captured; +} + +const MANIFEST = { id: 'com.acme.crm', version: '1.0.0' }; + +/** + * The publish seam, with a witness that the throw really travelled through it + * — an absence assertion on a request that never reached the service would + * pass for a reason that has nothing to do with this rule. + */ +async function throughPackageDoor(error: unknown): Promise { + const publish = vi.fn(async () => { throw error; }); + const captured = await drive(mount({ publish }), 'POST', `${PKGS}/publish`, { + body: { manifest: MANIFEST, metadata: { author: 'acme' } }, + }); + expect(publish.mock.calls.length, 'the throwing seam was never called').toBe(1); + return captured; +} + +/** Every assertion an answer from this door must satisfy, from the schemas themselves. */ +function expectDeclaredEnvelope(captured: Captured): any { + expect(BaseResponseSchema.safeParse(captured.body).success).toBe(true); + expect(envelopeViolations(captured.body)).toEqual([]); + expect(captured.body?.success).toBe(false); + const parsed = ApiErrorSchema.safeParse(captured.body?.error); + expect(parsed.error?.issues ?? []).toEqual([]); + expect(parsed.success).toBe(true); + return captured.body.error; +} + +/** A producer's throw, carrying whatever it declares. */ +function thrown(message: string, carried: Record): Error { + return Object.assign(new Error(message), carried); +} + +// --------------------------------------------------------------------------- +// 1. The withhold, on the wire +// --------------------------------------------------------------------------- + +describe('[#12509] the package door withholds an UNDECLARED 5xx spelling', () => { + const WITHHELD: Array<{ name: string; error: unknown }> = [ + { + name: 'a sqlite errno', + error: thrown('SQLITE_ERROR: no such table: sys_metadata', { code: 'SQLITE_ERROR' }), + }, + { + name: 'a postgres errno', + error: thrown('relation "sys_metadata" does not exist', { code: '42P01' }), + }, + { + name: 'an app spelling that declared no status — the ruling splits by provenance', + error: thrown('the widget refused the write', { code: 'WIDGET_REFUSED_THE_WRITE' }), + }, + ]; + + for (const c of WITHHELD) { + it(`${c.name}: 500 INTERNAL_ERROR with NO \`declaredCode\``, async () => { + const captured = await throughPackageDoor(c.error); + const error = expectDeclaredEnvelope(captured); + + // The positive shape first — an absence on a body that came back for + // some other reason would pass vacuously. + expect(captured.status).toBe(500); + expect(error.code).toBe('INTERNAL_ERROR'); + expect('declaredCode' in error).toBe(false); + // And the spelling is not hiding anywhere else in the body. + expect(JSON.stringify(captured.body)).not.toContain((c.error as any).code); + }); + } + + it('the withheld prose is unchanged — this is the CODE channel only', async () => { + const captured = await throughPackageDoor( + thrown('SQLITE_ERROR: no such table: sys_metadata', { code: 'SQLITE_ERROR' }), + ); + expect(captured.body.error.message).toBe(INTERNAL_ERROR_MESSAGE); + }); +}); + +// --------------------------------------------------------------------------- +// 2. The author channel survives — the half the ruling declined to withhold +// --------------------------------------------------------------------------- + +describe('[#12509] an AUTHOR-DECLARED code still reaches the wire', () => { + const SURVIVES: Array<{ name: string; error: unknown; status: number; code: string; declaredCode: string }> = [ + { + name: 'a declared 503', + error: thrown('the acme ledger service is down', { status: 503, code: 'ACME_LEDGER_OFFLINE' }), + status: 503, code: 'SERVICE_UNAVAILABLE', declaredCode: 'ACME_LEDGER_OFFLINE', + }, + { + name: 'a declared 500 — declaring the fallback VALUE is still declaring', + error: thrown('the importer gave up', { status: 500, code: 'ACME_IMPORT_ABORTED' }), + status: 500, code: 'INTERNAL_ERROR', declaredCode: 'ACME_IMPORT_ABORTED', + }, + { + name: 'a declared 501 spelled `statusCode`', + error: thrown('this dialect emits no DDL', { statusCode: 501, code: 'ACME_NOT_BUILT' }), + status: 501, code: 'NOT_IMPLEMENTED', declaredCode: 'ACME_NOT_BUILT', + }, + { + name: 'a declared 409 — below the sanitisation band entirely', + error: thrown('invoices still open', { status: 409, code: 'CLOSE_PERIOD_LOCKED' }), + status: 409, code: 'RESOURCE_CONFLICT', declaredCode: 'CLOSE_PERIOD_LOCKED', + }, + ]; + + for (const c of SURVIVES) { + it(c.name, async () => { + const captured = await throughPackageDoor(c.error); + const error = expectDeclaredEnvelope(captured); + expect(captured.status).toBe(c.status); + expect(error.code).toBe(c.code); + expect(error.declaredCode).toBe(c.declaredCode); + }); + } + + it('the rows above really do produce different answers', () => { + const answers = SURVIVES.map((c) => `${c.status} ${c.code} ${c.declaredCode}`); + expect(new Set(answers).size).toBe(SURVIVES.length); + }); +}); + +// --------------------------------------------------------------------------- +// 3. The wire IS the shared rule — this door adds nothing of its own +// --------------------------------------------------------------------------- + +describe('[#12509] the door reads the shared rule, it does not restate it', () => { + const SHAPES: unknown[] = [ + thrown('sqlite errno, undeclared', { code: 'SQLITE_ERROR' }), + thrown('postgres errno, undeclared', { code: '42P01' }), + thrown('app spelling, undeclared', { code: 'WIDGET_REFUSED_THE_WRITE' }), + thrown('app spelling, declared 503', { status: 503, code: 'ACME_LEDGER_OFFLINE' }), + thrown('app spelling, declared 500', { status: 500, code: 'ACME_IMPORT_ABORTED' }), + thrown('app spelling, declared 409', { status: 409, code: 'CLOSE_PERIOD_LOCKED' }), + thrown('registered code', { status: 409, code: 'DESTRUCTIVE_CHANGE' }), + thrown('a bare fault', {}), + ]; + + for (const shape of SHAPES) { + it(`"${(shape as Error).message}" answers what demotedDeclaredCode says`, async () => { + const expected = demotedDeclaredCode(resolveThrownHttpError(shape)); + const captured = await throughPackageDoor(shape); + expect(captured.body?.error?.declaredCode).toBe(expected); + }); + } + + it('the shapes above do not all answer the same thing', () => { + // Anti-vacuity for the comparison itself: if every shape now demoted to + // `undefined`, each case above would compare `undefined` to `undefined` + // and pass against a door that emits the channel never — which is option + // B, the alternative the ruling declined. + const answers = SHAPES.map((s) => demotedDeclaredCode(resolveThrownHttpError(s))); + expect(answers.filter((a) => a !== undefined).length).toBeGreaterThan(2); + expect(answers.filter((a) => a === undefined).length).toBeGreaterThan(2); + }); +}); + +// --------------------------------------------------------------------------- +// 4. The flat `/data` door: nothing to withhold, measured on a real driver +// --------------------------------------------------------------------------- + +describe('[#12509] the flat `/data` door never carried the shape', () => { + /** A driver that fails every access with a CODED fault, as a real driver does. */ + function codedFailingDriver(message: string, code: string) { + const boom = () => { throw Object.assign(new Error(message), { code }); }; + const driver: any = { + name: 'memory-broken', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, + async find() { boom(); }, async findOne() { boom(); }, + async create() { boom(); }, async update() { boom(); }, async delete() { boom(); }, + async upsert() { boom(); }, async count() { boom(); }, + async bulkCreate() { boom(); }, async bulkUpdate() { boom(); }, async bulkDelete() { boom(); }, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return driver; + } + + function createMockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; + } + + /** The real CRUD routes over a real engine whose driver fails with `code`. */ + async function throughDataDoor(message: string, code: string, method: 'GET' | 'POST') { + const engine = new ObjectQL(); + engine.registerDriver(codedFailingDriver(message, code), true); + engine.registerApp({ + id: 'acme', name: 'Acme', + objects: [{ name: 'leave_request', fields: { title: { type: 'text' } } }], + } as any); + + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' } }), + getMetaTypes: vi.fn().mockResolvedValue([]), + getMetaItems: vi.fn().mockResolvedValue([{ name: 'leave_request' }]), + getMetaItem: vi.fn().mockResolvedValue({}), + findData: vi.fn(async (r: any) => engine.find(r.object, {})), + createData: vi.fn(async (r: any) => engine.insert(r.object, r.data)), + updateData: vi.fn(async (r: any) => engine.update(r.object, { id: r.id, ...r.data })), + deleteData: vi.fn(async (r: any) => engine.delete(r.object, { where: { id: r.id } })), + }; + + const rest = new RestServer(createMockServer() as any, protocol, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'u1' }); + rest.registerRoutes(); + + const route = (rest as any).getRoutes().find( + (r: any) => r.method === method && r.path === '/api/v1/data/:object', + ); + expect(route, `${method} /api/v1/data/:object must be registered`).toBeTruthy(); + + const res: any = { + statusCode: 0, _body: undefined, + status(c: number) { res.statusCode = c; return res; }, + json(b: any) { res._body = b; return res; }, + send() { return res; }, setHeader() { return res; }, header() { return res; }, + end() { return res; }, write() { return res; }, + }; + await route.handler( + { method, path: '/api/v1/data/:object', headers: {}, query: {}, params: { object: 'leave_request' }, body: { title: 'x' } } as any, + res, + ); + return { status: res.statusCode, body: res._body }; + } + + const DIALECTS: Array<{ name: string; message: string; code: string; status: number; code_: string }> = [ + { + name: 'a sqlite fault that names no object → the sanitised store-fault terminal', + message: 'SQLITE_ERROR: database disk image is malformed', code: 'SQLITE_ERROR', + status: 500, code_: 'DATABASE_ERROR', + }, + { + name: 'a postgres fault the classifier does not recognise → the unclassified terminal', + message: 'canceling statement due to statement timeout', code: '57014', + status: 500, code_: 'INTERNAL_ERROR', + }, + ]; + + for (const d of DIALECTS) { + for (const method of ['GET', 'POST'] as const) { + it(`${method}: ${d.name}`, async () => { + const answer = await throughDataDoor(d.message, d.code, method); + expect(answer.status).toBe(d.status); + expect(answer.body?.code).toBe(d.code_); + // The fixed body has no code channel to withhold — and the errno is + // nowhere in it, which is the fact that matters to a caller. + expect(answer.body).not.toHaveProperty('declaredCode'); + expect(JSON.stringify(answer.body)).not.toContain(d.code); + }, 60_000); + } + } +}); diff --git a/packages/rest/src/package-door-declared-code.test.ts b/packages/rest/src/package-door-declared-code.test.ts index f1beea3091..88517682bf 100644 --- a/packages/rest/src/package-door-declared-code.test.ts +++ b/packages/rest/src/package-door-declared-code.test.ts @@ -134,6 +134,10 @@ * the wire) and leaves sections 2 and 5 GREEN, those being the ABSENCE * assertions the unrepaired door also satisfies. * + * ⚠️ [#12509] The numbers below record a run against THIS FILE'S PRE-#12509 + * row set (the third `DEMOTED` row and the `SHAPES` table have both moved + * since), so re-running the ablation today will not reproduce 155. + * * Measured: 30 failed / 125 passed of 155. Sections 1, 3 and 4 went red as * predicted — and so did the CONVERGENCE block of section 5, which the * prediction had lumped in with its real-producer sibling. It was wrong to: @@ -333,9 +337,19 @@ describe('[#12405] an UNREGISTERED producer spelling rides `declaredCode`', () = /** * Two spellings of the status channel, because both are produced in this * repo (`metadata-protocol` throws `status`, runtime action execution and - * the lifecycle hooks throw `statusCode`), and one throw whose status the - * resolver has to default — the shape a producer that names the condition - * but not the band produces. + * the lifecycle hooks throw `statusCode`), and a declared 5xx — the band + * where the demote and the 5xx sanitisation regime meet. + * + * ⚠️ [#12509] The third row used to be a throw whose status the resolver had + * to DEFAULT ("names the condition but not the band"). That shape no longer + * demotes to the wire: the 2026-08-27 ruling withholds a code the + * fallback-to-500 picked up from an undeclared producer, because a driver + * errno arrives on exactly that shape and naming the backend is one of the + * two disclosures the 5xx message withhold exists to prevent. It is replaced + * by a producer that declares the 5xx it means, which keeps its spelling. + * ⛔ Do not restore the old row here — the withhold has its own pin, in + * `package-door-5xx-demoted-code-withhold.test.ts`, and a second copy of the + * shape in this file would make one ruling readable in two voices. */ const DEMOTED: Array<{ name: string; error: unknown; status: number; code: string; declaredCode: string }> = [ { @@ -353,11 +367,11 @@ describe('[#12405] an UNREGISTERED producer spelling rides `declaredCode`', () = declaredCode: 'ORG_LICENCE_INVALID', }, { - name: 'an app spelling with no declared status at all', - error: thrown('the widget refused', { code: 'WIDGET_REFUSED_THE_WRITE' }), + name: 'an app spelling on a DECLARED 500 — the author channel survives 5xx sanitisation (#12509)', + error: thrown('the importer gave up', { status: 500, code: 'ACME_IMPORT_ABORTED' }), status: 500, code: 'INTERNAL_ERROR', - declaredCode: 'WIDGET_REFUSED_THE_WRITE', + declaredCode: 'ACME_IMPORT_ABORTED', }, ]; @@ -642,7 +656,12 @@ describe('[#12405] the wire `declaredCode` IS the shared rule, not a second copy const SHAPES: unknown[] = [ thrown('app spelling, declared 409', { status: 409, code: 'CLOSE_PERIOD_LOCKED' }), thrown('app spelling, declared 403 via statusCode', { statusCode: 403, code: 'ORG_LICENCE_INVALID' }), + // [#12509] Kept — it now exercises the WITHHOLD through the shared rule + // (the comparison below reads `demotedDeclaredCode`, so it followed the + // ruling without an edit, which is the property this block is for). The + // declared 500 beside it is what keeps the anti-vacuity count honest. thrown('app spelling, no declared status', { code: 'WIDGET_REFUSED_THE_WRITE' }), + thrown('app spelling, declared 500', { status: 500, code: 'ACME_IMPORT_ABORTED' }), thrown('registered code', { status: 409, code: 'DESTRUCTIVE_CHANGE' }), thrown('a record-validation failure', { name: 'ValidationError', code: 'VALIDATION_FAILED', fields: [] }), thrown('a bare fault', {}), diff --git a/packages/runtime/src/dispatcher-5xx-demoted-code-withhold.test.ts b/packages/runtime/src/dispatcher-5xx-demoted-code-withhold.test.ts new file mode 100644 index 0000000000..494ec75cff --- /dev/null +++ b/packages/runtime/src/dispatcher-5xx-demoted-code-withhold.test.ts @@ -0,0 +1,291 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#12509] This package's THREE dispatcher exits inherit ADR-0112's 5xx code + * scope from the shared resolver — none of them carries a copy of it. + * + * ## What was measured on the wire before the change + * + * `origin/main` @ `aef1b7e64`, through the REAL route for the exit that has + * one: + * + * ``` + * POST /api/v1/analytics/query, the analytics service throws + * { code: 'SQLITE_ERROR' } → 500 {"error":{"code":"INTERNAL_ERROR", + * "message":"Internal server error","httpStatus":500, + * "declaredCode":"SQLITE_ERROR"}} + * { code: '42P01' } → 500 {…,"declaredCode":"42P01"} + * ``` + * + * ⭐ That measurement is also the ruling's FIRST named reading, answered: the + * card's premise was that a driver errno cannot reach a producing seam, and + * that had been measured at `PackageService`'s four seams ONLY — where the + * service discriminates on the STATUS channel, so a driver fault is converted + * to a declared `503 SERVICE_UNAVAILABLE` and never demotes. This door has no + * such producer in front of it. `errorResponseBase` catches whatever the + * service threw and resolves it directly, so a coded driver fault reaches the + * wire verbatim in `declaredCode`. **The premise does not hold here** — which + * is what makes the ruling a repair rather than a tidy-up. + * + * ## Why no line changes at the exits + * + * All three read `demotedDeclaredCode(thrown)` and emit what it answers. The + * ruling put the judgement inside that function, so the exits inherit it. + * Section 3 compares each exit's body against the shared function itself, so + * an exit that ever grew a rule of its own turns red. ⛔ Do not "fix" a red + * there by teaching an exit the condition — that is the per-door variant the + * 2026-08-27 ruling declined by name. + * + * ## What is deliberately NOT touched here + * + * The MESSAGE. `errorResponseBase` still withholds on `declaresServerFault` + * (which needs a string code) rather than on every declared 5xx; aligning it + * to `/data` is the same ruling's prose axis and it is #12281's card, with its + * own measurement-first step. Section 4 pins the message behaviour AS IT + * STANDS so that card's change is visible as a change rather than as a silent + * drift, and names what will move. + */ + +import { describe, it, expect } from 'vitest'; +import { ApiErrorSchema, BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; +import { resolveThrownHttpError, demotedDeclaredCode, INTERNAL_ERROR_MESSAGE } from '@objectstack/types'; +import { HttpDispatcher } from './http-dispatcher.js'; +import type { DomainHandlerDeps } from './domain-handler-registry.js'; +import { endpointErrorAnswer } from './endpoint-executor.js'; +import { createDispatcherPlugin } from './dispatcher-plugin.js'; + +/** The REAL exit `domains/*.ts` calls, reached through the dispatcher's own seam. */ +const errorFromThrown: DomainHandlerDeps['errorFromThrown'] = (() => { + const dispatcher: any = new HttpDispatcher({ context: { getService: () => null } } as any); + return (dispatcher.domainDeps as DomainHandlerDeps).errorFromThrown; +})(); + +/** A producer's throw, carrying whatever it declares. */ +function thrown(message: string, carried: Record): Error { + return Object.assign(new Error(message), carried); +} + +// --- the third exit, driven through its real HTTP route --------------------- + +function makeFakeServer() { + const handlers: Record any> = {}; + const rec = (verb: string) => (path: string, handler: any) => { handlers[`${verb} ${path}`] = handler; }; + return { + handlers, + server: { get: rec('GET'), post: rec('POST'), put: rec('PUT'), delete: rec('DELETE'), patch: rec('PATCH') }, + }; +} + +function makeCtx(fakeServer: any, analyticsError: unknown) { + const analytics = { + query: async () => { throw analyticsError; }, + getMeta: async () => ({ cubes: [] }), + generateSql: async () => ({ sql: null }), + }; + const kernel = { + getService: (n: string) => (n === 'analytics' ? analytics : undefined), + getServiceAsync: async (n: string) => (n === 'analytics' ? analytics : undefined), + }; + return { + getKernel: () => kernel, + getService: (n: string) => (n === 'http.server' ? fakeServer : undefined), + environmentId: undefined, + logger: { info() {}, warn() {}, error() {}, debug() {} }, + hook: () => {}, on: () => {}, + } as any; +} + +/** `POST /api/v1/analytics/query` with an analytics service that throws. */ +async function postAnalyticsQuery(err: unknown) { + const { server, handlers } = makeFakeServer(); + const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }); + await plugin.start?.(makeCtx(server, err)); + const handler = handlers['POST /api/v1/analytics/query']; + expect(handler, 'POST /api/v1/analytics/query must be mounted').toBeTypeOf('function'); + const res: any = { + statusCode: undefined, body: undefined, + status(c: number) { res.statusCode = c; return res; }, + header() { return res; }, + json(b: any) { res.body = b; return res; }, + }; + // [#3878] The body must pass entry validation, so the SERVICE's throw is + // what reaches the exit rather than an entry 400. + await handler({ body: { cube: 'x', measures: ['count'] }, query: {} }, res); + return { status: res.statusCode as number, body: res.body }; +} + +/** The three exits, each as `(error) => { status, body }`. */ +const EXITS: Array<{ name: string; run: (e: unknown) => Promise<{ status: number; body: any }> }> = [ + { + name: 'HttpDispatcher.errorFromThrown', + run: async (e) => { + const r: any = errorFromThrown(e as any, 500); + return { status: r.status, body: r.body }; + }, + }, + { + name: 'endpoint-executor.endpointErrorAnswer', + run: async (e) => { + const r = endpointErrorAnswer(e as any, 500); + return { status: r.status, body: r.body as any }; + }, + }, + { + name: 'dispatcher-plugin.errorResponseBase (real route POST /api/v1/analytics/query)', + run: (e) => postAnalyticsQuery(e), + }, +]; + +function expectDeclaredEnvelope(body: any): any { + expect(BaseResponseSchema.safeParse(body).success).toBe(true); + expect(envelopeViolations(body)).toEqual([]); + const parsed = ApiErrorSchema.safeParse(body?.error); + expect(parsed.error?.issues ?? []).toEqual([]); + expect(parsed.success).toBe(true); + return body.error; +} + +// --------------------------------------------------------------------------- +// 1. The withhold, at every exit +// --------------------------------------------------------------------------- + +describe('[#12509] every dispatcher exit withholds an UNDECLARED 5xx spelling', () => { + const WITHHELD: Array<{ name: string; error: unknown; spelling: string }> = [ + { + name: 'a sqlite errno', + error: thrown('SQLITE_ERROR: no such table: leave_request', { code: 'SQLITE_ERROR' }), + spelling: 'SQLITE_ERROR', + }, + { + name: 'a postgres errno', + error: thrown('relation "leave_request" does not exist', { code: '42P01' }), + spelling: '42P01', + }, + { + name: 'an app spelling that declared no status', + error: thrown('the widget refused the write', { code: 'WIDGET_REFUSED_THE_WRITE' }), + spelling: 'WIDGET_REFUSED_THE_WRITE', + }, + ]; + + for (const exit of EXITS) { + for (const c of WITHHELD) { + it(`${exit.name}: ${c.name}`, async () => { + const answer = await exit.run(c.error); + const error = expectDeclaredEnvelope(answer.body); + + // Positive shape first, so the absence cannot pass vacuously. + expect(answer.status).toBe(500); + expect(error.code).toBe('INTERNAL_ERROR'); + expect('declaredCode' in error).toBe(false); + expect(JSON.stringify(answer.body)).not.toContain(c.spelling); + }); + } + } +}); + +// --------------------------------------------------------------------------- +// 2. The author channel survives, at every exit +// --------------------------------------------------------------------------- + +describe('[#12509] an AUTHOR-DECLARED code still reaches the wire at every exit', () => { + const SURVIVES: Array<{ name: string; error: unknown; status: number; code: string; declaredCode: string }> = [ + { + name: 'a declared 503', + error: thrown('the acme ledger service is down', { status: 503, code: 'ACME_LEDGER_OFFLINE' }), + status: 503, code: 'SERVICE_UNAVAILABLE', declaredCode: 'ACME_LEDGER_OFFLINE', + }, + { + name: 'a declared 500 — declaring the fallback VALUE is still declaring', + error: thrown('the importer gave up', { status: 500, code: 'ACME_IMPORT_ABORTED' }), + status: 500, code: 'INTERNAL_ERROR', declaredCode: 'ACME_IMPORT_ABORTED', + }, + { + name: 'a declared 409 — below the sanitisation band entirely', + error: thrown('invoices still open', { status: 409, code: 'CLOSE_PERIOD_LOCKED' }), + status: 409, code: 'RESOURCE_CONFLICT', declaredCode: 'CLOSE_PERIOD_LOCKED', + }, + ]; + + for (const exit of EXITS) { + for (const c of SURVIVES) { + it(`${exit.name}: ${c.name}`, async () => { + const answer = await exit.run(c.error); + const error = expectDeclaredEnvelope(answer.body); + expect(answer.status).toBe(c.status); + expect(error.code).toBe(c.code); + expect(error.declaredCode).toBe(c.declaredCode); + }); + } + } +}); + +// --------------------------------------------------------------------------- +// 3. Each exit's wire IS the shared rule +// --------------------------------------------------------------------------- + +describe('[#12509] the exits read the shared rule, they do not restate it', () => { + const SHAPES: unknown[] = [ + thrown('sqlite errno, undeclared', { code: 'SQLITE_ERROR' }), + thrown('postgres errno, undeclared', { code: '42P01' }), + thrown('app spelling, undeclared', { code: 'WIDGET_REFUSED_THE_WRITE' }), + thrown('app spelling, declared 503', { status: 503, code: 'ACME_LEDGER_OFFLINE' }), + thrown('app spelling, declared 500', { status: 500, code: 'ACME_IMPORT_ABORTED' }), + thrown('app spelling, declared 409', { status: 409, code: 'CLOSE_PERIOD_LOCKED' }), + thrown('registered code', { status: 409, code: 'DESTRUCTIVE_CHANGE' }), + thrown('a bare fault', {}), + ]; + + for (const exit of EXITS) { + for (const shape of SHAPES) { + it(`${exit.name}: "${(shape as Error).message}"`, async () => { + const expected = demotedDeclaredCode(resolveThrownHttpError(shape, 500)); + const answer = await exit.run(shape); + expect(answer.body?.error?.declaredCode).toBe(expected); + }); + } + } + + it('the shapes above do not all answer the same thing', () => { + // Anti-vacuity: if every shape demoted to `undefined` the comparisons + // would pass against an exit that emits the channel never — option B, + // which the ruling declined. + const answers = SHAPES.map((s) => demotedDeclaredCode(resolveThrownHttpError(s, 500))); + expect(answers.filter((a) => a !== undefined).length).toBeGreaterThan(2); + expect(answers.filter((a) => a === undefined).length).toBeGreaterThan(2); + }); +}); + +// --------------------------------------------------------------------------- +// 4. The prose axis, pinned AS IT STANDS — #12281's card, not this one +// --------------------------------------------------------------------------- + +describe('[#12509] the MESSAGE is untouched here — #12281 owns that axis', () => { + it('a declared 5xx WITH a code still has its prose withheld at errorResponseBase', async () => { + // `declaresServerFault` needs both a 5xx status and a string code, and + // this shape has both, so the withhold already fires. + const answer = await postAnalyticsQuery( + thrown('the acme ledger service is down', { status: 503, code: 'ACME_LEDGER_OFFLINE' }), + ); + expect(answer.status).toBe(503); + expect(answer.body.error.message).toBe(INTERNAL_ERROR_MESSAGE); + // …and the code channel is the half THIS card rules: author-declared, + // so it survives beside the withheld prose. + expect(answer.body.error.declaredCode).toBe('ACME_LEDGER_OFFLINE'); + }); + + it('⚠️ a declared 5xx with NO code keeps its prose — this is what #12281 changes', async () => { + // The population measurement the ruling asked for is non-empty: + // `action-execution.ts` throws `{ statusCode: 503, message: 'Data + // service not available' }` at six sites and a 501 at a seventh, all + // code-less. `declaresServerFault` is false for them, so their prose + // travels today. When #12281 lands this expectation flips to the + // generic sentence — deliberately pinned so that lands as a CHANGE + // rather than as drift nobody sees. + const answer = await postAnalyticsQuery({ statusCode: 503, message: 'Data service not available' }); + expect(answer.status).toBe(503); + expect(answer.body.error.message).toBe('Data service not available'); + // Nothing to withhold on the code channel: the producer declared none. + expect(answer.body.error).not.toHaveProperty('declaredCode'); + }); +}); diff --git a/packages/types/src/thrown-http-error-5xx-code-withhold.test.ts b/packages/types/src/thrown-http-error-5xx-code-withhold.test.ts new file mode 100644 index 0000000000..8cb6673b0a --- /dev/null +++ b/packages/types/src/thrown-http-error-5xx-code-withhold.test.ts @@ -0,0 +1,228 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#12509] ADR-0112's `declaredCode` channel has a 5xx SCOPE, and this file + * pins the rule itself — the doors are pinned to it from their own packages. + * + * ## What was on the wire before this + * + * Measured on `origin/main` @ `aef1b7e64`, by driving the real routes rather + * than by reading source: + * + * ``` + * POST /api/v1/packages/publish → 500 {"error":{"code":"INTERNAL_ERROR", + * "message":"Internal server error","declaredCode":"SQLITE_ERROR"}} + * POST /api/v1/analytics/query → 500 {"error":{"code":"INTERNAL_ERROR", + * "message":"Internal server error","declaredCode":"42P01"}} + * ``` + * + * The prose withhold (#8086) fired and the driver's own dialect went out + * beside it. `SQLITE_ERROR` vs `42P01` names the backend, which is one of the + * two disclosures the message withhold exists to prevent. + * + * ## The ruling this implements + * + * Maintainer, 2026-08-27, option D: in 5xx sanitisation a DEMOTED code — one + * the fallback-to-500 picked up from an UNDECLARED producer — is withheld + * along with the prose; an AUTHOR-DECLARED code survives. One rule at the + * shared resolver, every door inherits it. Per-door variants, withholding the + * author channel too, and recording the leak as a decision were all declined. + * + * ## Why the STATUS channel is the discriminator + * + * A driver errno and an app's own spelling both arrive on `.code` as a plain + * string. Anything that told them apart by LOOKING at the string would be a + * heuristic over an open channel — and unfalsifiable, since nothing stops an + * app from spelling `SQLITE_ERROR`. Section 3 is that pin: two throws whose + * `.code` strings are IDENTICAL and whose answers differ, so an implementation + * that inspected the spelling cannot pass here. + * + * ## The cost, pinned rather than hidden + * + * Section 2's last row: a producer that spells a code and declares NO status + * loses that code on a 5xx. That is what "the fallback-to-500 picked it up + * from an undeclared producer" means when it is spelled as code, and it is + * kept visible here so nobody discovers it from a bug report. The tenant + * limb the ADR-0112 amendment protects is NOT this shape: `SandboxError` + * answers 400, so `DUPLICATE` rides a 4xx (pinned in + * `packages/runtime/src/domains/actions-validation-envelope.test.ts`). + */ + +import { describe, it, expect } from 'vitest'; +import { + resolveThrownHttpError, + demotedDeclaredCode, + serverFaultProvenance, +} from './thrown-http-error.js'; + +/** A producer's throw, carrying whatever it declares. */ +function thrown(message: string, carried: Record): Error { + return Object.assign(new Error(message), carried); +} + +// --------------------------------------------------------------------------- +// 1. The discriminator +// --------------------------------------------------------------------------- + +describe('[#12509] `serverFaultProvenance` — who named this 5xx', () => { + const CASES: Array<{ name: string; error: unknown; expected: string | undefined }> = [ + { + name: 'a bare driver fault — the fallback supplied the 500', + error: thrown('SQLITE_ERROR: no such table: leave_request', { code: 'SQLITE_ERROR' }), + expected: 'undeclared', + }, + { + name: 'a producer that declared 503 itself', + error: thrown('the ledger service is down', { status: 503, code: 'ACME_LEDGER_OFFLINE' }), + expected: 'declared', + }, + { + name: 'a producer that declared 500 itself — declaring the fallback VALUE is still declaring', + error: thrown('the importer gave up', { status: 500, code: 'ACME_IMPORT_ABORTED' }), + expected: 'declared', + }, + { + name: 'a declared 5xx spelled `statusCode`', + error: thrown('not built yet', { statusCode: 501, code: 'ACME_NOT_BUILT' }), + expected: 'declared', + }, + { + name: 'a declared 4xx — nothing is sanitised below 500', + error: thrown('invoices still open', { status: 409, code: 'CLOSE_PERIOD_LOCKED' }), + expected: undefined, + }, + { + name: 'a validation shape — a 400 by shape, so also below 500', + error: thrown('bad manifest', { name: 'ValidationError', fields: [{ field: 'id' }] }), + expected: undefined, + }, + ]; + + for (const c of CASES) { + it(c.name, () => { + expect(serverFaultProvenance(resolveThrownHttpError(c.error))).toBe(c.expected); + }); + } + + it('the cases above really do produce all three answers', () => { + // Anti-vacuity for the table: rows that collapsed onto one answer would + // agree with any implementation, including one that always says the same + // thing. + const answers = CASES.map((c) => serverFaultProvenance(resolveThrownHttpError(c.error))); + expect(new Set(answers)).toEqual(new Set(['undeclared', 'declared', undefined])); + }); + + it('the fallback the CALLER supplies is what decides, not the number 500', () => { + // A door that resolves against a 4xx fallback is answering a 4xx, and + // nothing here applies. The same throw, two callers, two verdicts. + const bare = new Error('kaboom'); + expect(serverFaultProvenance(resolveThrownHttpError(bare, 500))).toBe('undeclared'); + expect(serverFaultProvenance(resolveThrownHttpError(bare, 404))).toBeUndefined(); + }); +}); + +// --------------------------------------------------------------------------- +// 2. What a boundary is allowed to put on the wire +// --------------------------------------------------------------------------- + +describe('[#12509] `demotedDeclaredCode` withholds an UNDECLARED 5xx spelling', () => { + const CASES: Array<{ name: string; error: unknown; status: number; declaredCode: string | undefined }> = [ + { + name: 'sqlite errno on an undeclared 500 → WITHHELD', + error: thrown('SQLITE_ERROR: no such table: leave_request', { code: 'SQLITE_ERROR' }), + status: 500, + declaredCode: undefined, + }, + { + name: 'postgres errno on an undeclared 500 → WITHHELD', + error: thrown('relation "leave_request" does not exist', { code: '42P01' }), + status: 500, + declaredCode: undefined, + }, + { + name: 'an AUTHOR-declared 503 → survives', + error: thrown('the ledger service is down', { status: 503, code: 'ACME_LEDGER_OFFLINE' }), + status: 503, + declaredCode: 'ACME_LEDGER_OFFLINE', + }, + { + name: 'an AUTHOR-declared 500 → survives; the ruling splits by PROVENANCE, not by status value', + error: thrown('the importer gave up', { status: 500, code: 'ACME_IMPORT_ABORTED' }), + status: 500, + declaredCode: 'ACME_IMPORT_ABORTED', + }, + { + name: 'an app spelling on a declared 409 → untouched, 5xx sanitisation does not reach it', + error: thrown('invoices still open', { status: 409, code: 'CLOSE_PERIOD_LOCKED' }), + status: 409, + declaredCode: 'CLOSE_PERIOD_LOCKED', + }, + { + name: 'a REGISTERED code is already in `code` — absent for the #9106 reason, not this one', + error: thrown('would drop data', { status: 409, code: 'DESTRUCTIVE_CHANGE' }), + status: 409, + declaredCode: undefined, + }, + { + name: '⚠️ the COST: a code spelled with NO declared status loses the channel on a 5xx', + error: thrown('the widget refused the write', { code: 'WIDGET_REFUSED_THE_WRITE' }), + status: 500, + declaredCode: undefined, + }, + ]; + + for (const c of CASES) { + it(c.name, () => { + const resolved = resolveThrownHttpError(c.error); + // The POSITIVE half first: an absence asserted on a throw that resolved + // to some entirely different status would pass for the wrong reason. + expect(resolved.status).toBe(c.status); + expect(demotedDeclaredCode(resolved)).toBe(c.declaredCode); + }); + } + + it('the withhold is a WITHHOLD — the resolver still records what the producer wrote', () => { + // The two fields answer different questions, and a "fix" that stopped + // recording the spelling would break `metadata-protocol`'s row mapper and + // every log that reads it. Only the WIRE read is narrowed. + const resolved = resolveThrownHttpError(thrown('boom', { code: 'SQLITE_ERROR' })); + expect(resolved.declaredCode).toBe('SQLITE_ERROR'); + expect(demotedDeclaredCode(resolved)).toBeUndefined(); + }); + + it('both outcomes are reachable from this table', () => { + // Anti-vacuity: a rule that withheld everything, or nothing, would satisfy + // half of the rows above and this line is what notices. + const answers = CASES.map((c) => demotedDeclaredCode(resolveThrownHttpError(c.error))); + expect(answers.filter((a) => a !== undefined).length).toBeGreaterThan(1); + expect(answers.filter((a) => a === undefined).length).toBeGreaterThan(1); + }); +}); + +// --------------------------------------------------------------------------- +// 3. The spelling is NOT what decides — the pin against a string heuristic +// --------------------------------------------------------------------------- + +describe('[#12509] identical spellings, different answers — provenance, not the string', () => { + it('the SAME `.code` survives when declared and is withheld when not', () => { + const SPELLING = 'SQLITE_ERROR'; + const declared = resolveThrownHttpError(thrown('down for maintenance', { status: 503, code: SPELLING })); + const undeclared = resolveThrownHttpError(thrown('down for maintenance', { code: SPELLING })); + + expect(demotedDeclaredCode(declared)).toBe(SPELLING); + expect(demotedDeclaredCode(undeclared)).toBeUndefined(); + }); + + it('and an app-shaped spelling is withheld on the undeclared side too', () => { + // The mirror image of the row above: nothing here reads the string, so a + // driver-looking code and an app-looking code get the same treatment for + // the same reason. An implementation that pattern-matched errnos would + // pass the first case and fail this one. + const SPELLING = 'ACME_WIDGET_REFUSED'; + const declared = resolveThrownHttpError(thrown('refused', { status: 503, code: SPELLING })); + const undeclared = resolveThrownHttpError(thrown('refused', { code: SPELLING })); + + expect(demotedDeclaredCode(declared)).toBe(SPELLING); + expect(demotedDeclaredCode(undeclared)).toBeUndefined(); + }); +}); diff --git a/packages/types/src/thrown-http-error.ts b/packages/types/src/thrown-http-error.ts index be905ae8a7..ded6de181e 100644 --- a/packages/types/src/thrown-http-error.ts +++ b/packages/types/src/thrown-http-error.ts @@ -70,6 +70,14 @@ * answers come from ONE function, which is what keeps agreement a construction * rather than two suites agreeing about literals. * + * [#12509] And the channel has a SCOPE, ruled 2026-08-27 (option D): on a 5xx + * the producer did not declare, the demoted spelling came off an undeclared + * producer and is withheld with the prose, while an author-declared code + * survives. The discriminator is {@link serverFaultProvenance} — one function, + * read by {@link demotedDeclaredCode}, which every door already calls, so no + * registrar carries a variant. Read that function's note for why the status + * channel is the only honest signal here. + * * ## What this deliberately does NOT decide * * - **Message disclosure.** A 5xx message may name physical tables or carry a @@ -131,6 +139,11 @@ export interface ThrownHttpError { * every door (#9106) — but for the wire's `declaredCode` channel when the * spelling is not a vocabulary member ({@link demotedDeclaredCode}). See the * module note on why there are two. + * + * ⚠️ This field records what the producer WROTE, not what a boundary may + * emit: since #12509 a demoted spelling is withheld on an undeclared 5xx. + * ⛔ Read {@link demotedDeclaredCode}, never this field, when deciding what + * goes on a wire. */ declaredCode?: string; /** The thrown message, UNSANITISED — see the module note on disclosure. */ @@ -259,20 +272,85 @@ export function declaredUserMessage(error: unknown): string | undefined { return typeof declared === 'string' && declared.trim().length > 0 ? declared : undefined; } +/** + * [#12509] WHO named this 5xx — the producer, or this resolver's fallback. + * `undefined` for anything below 500, where nothing is sanitised at all. + * + * This is the ONE definition of the distinction ADR-0112's 5xx-sanitisation + * scope turns on (maintainer ruling 2026-08-27, option D), and it exists as a + * named function rather than as an inline conjunction because TWO rules read + * it and they read opposite limbs: + * + * - `'undeclared'` — the throw declared no HTTP answer, so + * {@link ThrownHttpError.status} is the caller's `fallbackStatus` and + * EVERYTHING this resolver picked up off that throw is the producer's + * internals rather than an answer it composed. A driver errno + * (`SQLITE_ERROR`, `42P01`) is the measured case, and it is why + * {@link demotedDeclaredCode} withholds the code here: the spelling names + * the backend, which is one of the two disclosures the 5xx message + * withhold exists to prevent (`looksLikeInternalErrorLeak`; the other, + * identifiers, is already covered). + * - `'declared'` — the producer named a 5xx ITSELF, so its code is authored + * and survives. #11718's `{ status: 503, code: 'SERVICE_UNAVAILABLE' }` + * relay is this limb, and so is a metadata app's own 5xx refusal spelling + * (#7867), which the ADR-0112 amendment wrote `declaredCode` for. + * + * ⚠️ The DISCRIMINATOR is the status channel, not the code's shape. There is + * no other structural signal: a driver errno and an app's own spelling both + * arrive on `.code` as a plain string, so anything that told them apart by + * LOOKING at the string would be a heuristic over an open channel — the + * consumer-side tolerance ADR-0112 exists to forbid, and unfalsifiable besides + * (nothing stops an app from spelling `SQLITE_ERROR`). The cost of the + * structural answer is stated rather than hidden: a producer that spells a + * code but declares NO status loses that code on a 5xx. It keeps it by + * declaring the status it means, which is the shape the ADR already asks for. + * + * ⛔ NOT gated on whether `looksLikeInternalErrorLeak` actually fired on the + * message. That predicate is a heuristic over a DIFFERENT channel, and gating + * here on it would leak the errno for exactly the dialects whose prose the + * heuristic misses — the ceiling `sendThrownError`'s note records. The 5xx + * sanitisation REGIME is the condition, not one of its two outcomes. + * + * ⭐ #12281 — the prose axis of the same 2026-08-27 ruling — is the + * `'declared'` limb of this same function: the dispatcher door withholds the + * message of EVERY declared 5xx, aligning to `/data`. It is a separate card + * with its own measurement-first step, so nothing here applies it; this + * function is the shape it will read rather than a second copy it would have + * to grow. + */ +export type ServerFaultProvenance = 'declared' | 'undeclared'; + +/** See {@link ServerFaultProvenance}. */ +export function serverFaultProvenance(thrown: ThrownHttpError): ServerFaultProvenance | undefined { + if (thrown.status < 500) return undefined; + return thrown.declaredStatus === undefined ? 'undeclared' : 'declared'; +} + /** * The producer's spelling a boundary should surface as the wire's * `declaredCode` beside the closed `code` — or `undefined` when there is * nothing to surface (#9106). * * Present exactly when the throw spelled a code that did NOT survive into - * {@link ThrownHttpError.code} — i.e. the demote happened. A registered code - * is already in `code`, so emitting it again would put two spellings of one - * fact on every refusal; a throw with no code has nothing to declare. Spelled - * once here rather than as three `!==` comparisons at three exits, so - * "presence means demotion" (`ApiErrorSchema.declaredCode`'s documented - * semantics) has one definition. + * {@link ThrownHttpError.code} — i.e. the demote happened — AND the answer is + * not an undeclared server fault. A registered code is already in `code`, so + * emitting it again would put two spellings of one fact on every refusal; a + * throw with no code has nothing to declare. Spelled once here rather than as + * three `!==` comparisons at three exits, so "presence means demotion" + * (`ApiErrorSchema.declaredCode`'s documented semantics) has one definition. + * + * [#12509] The withhold limb, ruled 2026-08-27 (option D): on a 5xx the + * producer did NOT declare, the spelling this resolver demoted came off an + * undeclared producer — a driver errno, measured on the wire at three of this + * repo's doors — and it is withheld along with the prose. An AUTHOR-declared + * code survives at every status. The judgement lives in + * {@link serverFaultProvenance}; it is applied HERE, in the one read every + * boundary already makes, so all of them inherit it without a door growing a + * rule of its own. ⛔ Do not re-derive the condition at a door: a per-door + * variant is the divergence this channel has now been repaired for twice. */ export function demotedDeclaredCode(thrown: ThrownHttpError): string | undefined { + if (serverFaultProvenance(thrown) === 'undeclared') return undefined; return thrown.declaredCode !== undefined && thrown.declaredCode !== thrown.code ? thrown.declaredCode : undefined; diff --git a/scripts/adr-anchors/packages__types__src__thrown-http-error.ts.json b/scripts/adr-anchors/packages__types__src__thrown-http-error.ts.json index 5cce8ff507..b4aedd022f 100644 --- a/scripts/adr-anchors/packages__types__src__thrown-http-error.ts.json +++ b/scripts/adr-anchors/packages__types__src__thrown-http-error.ts.json @@ -3,5 +3,5 @@ "adrs": [ "ADR-0112" ], - "invariant": "This is the ONE definition of how a thrown error becomes an HTTP answer, and the two spellings it returns are not a redundancy to tidy away. ADR-0112 makes `error.code` a CLOSED vocabulary (`StandardErrorCode` union the registered ledger), and the 2026-08-16 ruling on #9106 extended that from the REST package door to every door this function serves: `code` is always a union member — a throw whose `.code` is unregistered falls to the member the status derives — while `declaredCode` keeps the producer's verbatim string. `demotedDeclaredCode()` is the single rule for which spelling a boundary surfaces beside the closed one, and it answers `undefined` for a registered code on purpose: emitting both would put two spellings of one fact on every refusal, and `ApiErrorSchema.declaredCode`'s documented semantics are that PRESENCE MEANS DEMOTION. Do not 'simplify' a boundary by writing `thrown.declaredCode` into `error.code` — that is the pre-#9106 dispatcher behaviour, and it re-opens the tenant-authored limb #9106 closed: a metadata app's action code crosses the QuickJS sandbox carrying the app's own `.code` (#7867, a capability deliberately granted and preserved), so `error.code` would again carry strings authored by tenants at runtime, which no ledger can enumerate and no gate can sweep. The author's spelling is not dropped — it rides the wire's open `declaredCode` channel instead." + "invariant": "This is the ONE definition of how a thrown error becomes an HTTP answer, and the two spellings it returns are not a redundancy to tidy away. ADR-0112 makes `error.code` a CLOSED vocabulary (`StandardErrorCode` union the registered ledger), and the 2026-08-16 ruling on #9106 extended that from the REST package door to every door this function serves: `code` is always a union member — a throw whose `.code` is unregistered falls to the member the status derives — while `declaredCode` keeps the producer's verbatim string. `demotedDeclaredCode()` is the single rule for which spelling a boundary surfaces beside the closed one, and it answers `undefined` for a registered code on purpose: emitting both would put two spellings of one fact on every refusal, and `ApiErrorSchema.declaredCode`'s documented semantics are that PRESENCE MEANS DEMOTION. Do not 'simplify' a boundary by writing `thrown.declaredCode` into `error.code` — that is the pre-#9106 dispatcher behaviour, and it re-opens the tenant-authored limb #9106 closed: a metadata app's action code crosses the QuickJS sandbox carrying the app's own `.code` (#7867, a capability deliberately granted and preserved), so `error.code` would again carry strings authored by tenants at runtime, which no ledger can enumerate and no gate can sweep. The author's spelling is not dropped — it rides the wire's open `declaredCode` channel instead. [#12509, ruled 2026-08-27] The channel also has a 5xx SCOPE, and it is the SECOND thing `demotedDeclaredCode()` answers: on a 5xx the producer did NOT declare, the demoted spelling came off an undeclared producer -- a driver errno, measured on the wire -- and is withheld along with the prose; an AUTHOR-DECLARED code survives at every status. `serverFaultProvenance()` is the ONE definition of that distinction, and the discriminator is the STATUS channel because it is the only structural one: a driver errno and an app's own spelling both arrive on `.code` as a plain string, so anything that told them apart by LOOKING at the string would be a heuristic over an open channel. Do not re-derive the condition at a door and do not gate it on whether `looksLikeInternalErrorLeak` fired -- that predicate reads a DIFFERENT channel, and gating on it leaks the errno for exactly the dialects whose prose the heuristic misses. The 'declared' limb of the same function is what #12281's prose rule will read; it is deliberately not applied yet." }