diff --git a/.changeset/hono-adapter-declared-envelope-render.md b/.changeset/hono-adapter-declared-envelope-render.md new file mode 100644 index 0000000000..f6927fc8a5 --- /dev/null +++ b/.changeset/hono-adapter-declared-envelope-render.md @@ -0,0 +1,55 @@ +--- +'@objectstack/plugin-hono-server': minor +--- + +fix(plugin-hono-server): an escaped throw that declares an ADR-0112 envelope is answered as that envelope, not as a bare `500 INTERNAL_ERROR "No response from handler"` (#16545) + +`HonoHttpServer.wrap()` is the seam **every direct-mount route passes** — `get` / +`post` / `put` / `delete` / `patch` each register `this.wrap(handler)`, and +`IHttpServer` is how `service-datasource`, `packages/rest` and the dispatcher +bridge all mount. Until now a throw that escaped a route handler was answered +there as `500 { code: 'INTERNAL_ERROR', message: 'No response from handler' }`, +with the thrown value discarded — so a producer that had *declared* its refusal +lost both halves of the declaration on the way to the caller. + +The measured case: `service-datasource`'s `requireDatasourceAdmin` re-raises +`AuthzStoreUnavailableError` (declared `status: 503`, declared `code: +SERVICE_UNAVAILABLE`) when the authorization store cannot be read — deliberately, +per the #13279 ruling that an unreadable store licenses no verdict. The operator's +outage reached the caller as a generic fault naming the wrong component: the +declared code never arrived, and the message said "No response from handler". + +**What changed.** An escaped throw carrying **both** a declared ADR-0112 status +(a key of `HttpStatusErrorCodeMap`) **and** a code registered in `ErrorCode` +(`StandardErrorCode` ∪ `ERROR_CODE_LEDGER`) is now rendered as that envelope, +with the producer's `details` and `userMessage` channels forwarded. The status +and code are read through `resolveThrownHttpError` — the one rule the REST +registrar and the dispatcher already share — so this seam agrees with the other +doors by construction rather than by a second ladder. + +**What did NOT change**, pinned in the same PR: + +- an escaped throw that is **not** such an envelope answers exactly the bytes it + answered before — 500, no cause in the body. A partial declaration (status but + no code, code but no status), an unregistered code, and a status ADR-0112 does + not declare all take that arm; +- a handler that simply wrote nothing is untouched; +- a handler that **wrote and then threw** keeps what it wrote; +- the `notFound` fallback seam still answers `Fallback handler failed` — a + fallback that threw is a broken consumer, not a refusal it declared; +- ⛔ no error code is minted and no ledger row is added. A code on this path that + is not registered is a ledger gap under the #16404 ruling, and takes the + unchanged 500 arm rather than being registered in passing. + +The 5xx disclosure filter every door emitting a thrown message already runs +(`looksLikeInternalErrorLeak`, #3867 / #8086) is applied here from this seam's +first day: a driver dump on a declared 5xx is withheld, where the old bare 500 +disclosed nothing at all. The escaped-throw diagnosis (#5848) still fires exactly +once at `error`, and now names the answer that was really sent instead of +claiming an opaque 500. + +⚠️ **Known-unreached door, stated rather than left silent.** A route mounted +through `getRawApp()` funnels through neither `wrap()` nor any registrar wrapper, +so it is **not** repaired by this change and still answers a non-envelope +`text/plain` 500. That is out of this card's scope by the `domain:cli` seat's +ruling and is filed separately. diff --git a/packages/plugins/plugin-hono-server/src/adapter.ts b/packages/plugins/plugin-hono-server/src/adapter.ts index f912cc4925..a7beaf1e46 100644 --- a/packages/plugins/plugin-hono-server/src/adapter.ts +++ b/packages/plugins/plugin-hono-server/src/adapter.ts @@ -27,6 +27,21 @@ import { routePath } from 'hono/route'; import { serve } from '@hono/node-server'; import { serveStatic } from '@hono/node-server/serve-static'; import { matchesRoutePattern } from './route-pattern'; +// The ADR-0112 wire vocabulary, read as DATA rather than restated: `ErrorCode` +// is the closed union (`StandardErrorCode` ∪ `ERROR_CODE_LEDGER`) a registered +// code must be a member of, and `HttpStatusErrorCodeMap` IS the set of statuses +// ADR-0112 declares — the same table `standardErrorCodeForHttpStatus` derives +// from, so "a declared ADR-0112 status" needs no second list here. +import { ErrorCode, HttpStatusErrorCodeMap } from '@objectstack/spec/api'; +// The ONE rule for "what HTTP answer does a THROWN error declare?" (#8016), and +// the 5xx disclosure filter every door that emits a thrown message already runs +// (#3867 / #8086). Both are CALLED, never restated — a second ladder here is +// how the `/api/v1/packages` two-door divergence arose in the first place. +import { + resolveThrownHttpError, + looksLikeInternalErrorLeak, + INTERNAL_ERROR_MESSAGE, +} from '@objectstack/types'; /** * Request headers allowed on preflight, by default. @@ -163,6 +178,116 @@ function toLoggableError(thrown: unknown): Error { return new Error(`Non-Error value thrown: ${described}`); } +/** + * The declared ADR-0112 envelope an escaped throw CARRIES, or `undefined` when + * it carries none (#16545, the `domain:cli` half of the #15999 ruling). + * + * ## What the ruling asked for + * + * > **Shared half** (`domain:cli`, hono adapter / registrar wrapper): an + * > escaped throw carrying a declared ADR-0112 `status` + registered `code` is + * > rendered by them, not as a bare `500 INTERNAL_ERROR "No response from + * > handler"`. This changes what an escaped throw means for every direct-mount + * > route; the PR pins that an escaped **non**-envelope throw still answers 500 + * > with no cause in the body. + * + * The measured motivating path: `service-datasource`'s `requireDatasourceAdmin` + * re-raises `AuthzStoreUnavailableError` (declared `status: 503` / `code: + * SERVICE_UNAVAILABLE`) on an unreadable authorization store, deliberately and + * per the #13279 ruling — and the caller was told `500 INTERNAL_ERROR "No + * response from handler"`. The declared code never reached the caller and the + * message named the wrong component. Only the RENDERING moves here; #13279's + * discipline (an unreadable authz store licenses no verdict) is untouched. + * + * ## Why this is a GATE and not `sendThrownError` + * + * `packages/rest`'s `sendThrownError` maps EVERY throw through + * `resolveThrownHttpError`, so an undeclared fault arrives as `500 + * INTERNAL_ERROR` carrying the thrown message. That is right for a REST + * registrar, whose bodies are parsed against `BaseResponseSchema` by its own + * conformance suite. It is NOT what this seam may do: the ruling pins that a + * non-envelope throw keeps today's behaviour EXACTLY — 500, and no cause in the + * body — so the fallback arm must stay byte-identical rather than gain the + * thrown message. Hence a gate that answers `undefined` for everything the + * ruling did not name, and `wrap`'s existing literal for that arm. + * + * ## The two conditions, both read off the ONE rule + * + * `resolveThrownHttpError` reports the DECLARATION it read — `declaredStatus` + * is absent exactly when the throw declared no status (its docblock states the + * distinction and why `status` cannot answer it), and `declaredCode` is the + * producer's own spelling. So neither condition re-spells that function's + * precedence chain here; a second chain is the divergence #8016 removed. + * + * 1. **a declared ADR-0112 status** — a key of `HttpStatusErrorCodeMap`. That + * table is ADR-0112's own status list, so the vocabulary has one home. A + * producer that declares `418` or `599` is NOT naming an ADR-0112 status + * and takes the fallback arm. + * 2. **a registered code** — a member of `ErrorCode`, i.e. `StandardErrorCode` + * ∪ `ERROR_CODE_LEDGER`. ⛔ No code is minted here and no ledger row is + * added; a code this path carries that is NOT registered is a ledger gap + * under the #16404 ruling and takes the fallback arm rather than being + * registered in passing. + * + * ⚠️ **Blast radius, stated because it is wider than the motivating path.** + * `resolveThrownHttpError` treats the validation SHAPE as a declaration too + * (`err.name === 'ValidationError'` ⇒ `400` / `VALIDATION_FAILED`), so a bare + * `ValidationError` escaping a direct-mount handler now answers `400 + * VALIDATION_FAILED` with its `fields[]` instead of a bare 500. That is the one + * rule's own semantics, and second-guessing one of its limbs at this door is + * precisely how two doors start disagreeing — so it is accepted and recorded, + * not carved out. + * + * ⛔ `declaredCode` is deliberately NOT forwarded. Under this gate the + * producer's spelling IS the registered member sitting in `code`, so + * `demotedDeclaredCode` returns `undefined` by construction — forwarding it + * would put two spellings of one fact on every envelope this seam renders. + */ +function declaredEnvelopeForThrow(thrown: unknown): { + status: number; + body: { success: false; error: Record }; +} | undefined { + const resolved = resolveThrownHttpError(thrown); + + // Condition 1 — the throw DECLARED a status, and it is one ADR-0112 names. + if (resolved.declaredStatus === undefined) return undefined; + if (!Object.prototype.hasOwnProperty.call(HttpStatusErrorCodeMap, resolved.declaredStatus)) { + return undefined; + } + // Condition 2 — the producer's OWN code is a member of the closed union. + if (resolved.declaredCode === undefined) return undefined; + if (!ErrorCode.safeParse(resolved.declaredCode).success) return undefined; + + // The 5xx disclosure filter every door emitting a thrown message runs + // (`HttpDispatcher.error` since #3867, `packages/rest`'s registrars since + // #8086). This seam becomes such a door with this change, so it owes the + // rule from its first day: without it a driver dump reaching a declared + // 5xx would newly travel to the client, where the old bare 500 disclosed + // nothing. Scoped to 5xx, like the twins: a 4xx message is a + // caller-facing answer by design. + const message = resolved.status >= 500 && looksLikeInternalErrorLeak(resolved.message) + ? INTERNAL_ERROR_MESSAGE + : resolved.message; + + return { + status: resolved.status, + body: { + success: false, + error: { + code: resolved.code, + message, + // The producer's structured context and its END-USER-addressed + // refusal text (#9934), forwarded exactly as the REST twin + // forwards them. Both are absent unless the producer declared + // them, so a throw that carried neither renders the same two + // keys it always did. + ...(resolved.details ? { details: resolved.details } : {}), + ...(resolved.userMessage !== undefined ? { userMessage: resolved.userMessage } : {}), + }, + }, + }; +} + /** * The matched route's path parameters, or `{}` when there is no matched route. * @@ -296,7 +421,18 @@ export class HonoHttpServer implements IHttpServer { // internal helper to convert standard handler to Hono handler private wrap(handler: RouteHandler) { return async (c: any) => { - const { response } = await this.runHandler(c, handler); + // `renderDeclaredEnvelope` is the #16545 opt-in, and it is opt-IN + // rather than the default because the OTHER caller of `runHandler` + // — the `notFound` seam — must keep answering `Fallback handler + // failed`: a fallback that threw is a broken consumer, not a + // refusal the consumer declared. The ruling names direct-mount + // ROUTES, which is exactly this call site. + const { response } = await this.runHandler(c, handler, { + renderDeclaredEnvelope: true, + }); + // Unchanged, and pinned byte-for-byte: a throw that declared no + // ADR-0112 envelope, and a handler that simply wrote nothing, both + // still answer 500 with no cause in the body. return response ?? c.json( { success: false, @@ -332,6 +468,15 @@ export class HonoHttpServer implements IHttpServer { private async runHandler( c: any, handler: RouteHandler, + opts: { + /** + * Render an escaped throw that carries a declared ADR-0112 status + * and a registered code as THAT envelope (#16545). Off by default + * — see {@link declaredEnvelopeForThrow} for the rule and + * {@link wrap} for why only the route caller opts in. + */ + renderDeclaredEnvelope?: boolean; + } = {}, ): Promise<{ response: Response | null; failed: boolean }> { let body: any = {}; @@ -465,7 +610,7 @@ export class HonoHttpServer implements IHttpServer { // Create a streaming response wrapper — if handler calls res.write(), // we return a ReadableStream; otherwise fall back to capturedResponse. - const streamPromise = new Promise<{ response: Response | null; failed: boolean }>((resolve) => { + const streamPromise = new Promise<{ response: Response | null; failed: boolean; thrown?: unknown }>((resolve) => { const stream = new ReadableStream({ start(controller) { streamController = controller; @@ -506,21 +651,37 @@ export class HonoHttpServer implements IHttpServer { }).catch((err) => { _endHandler?.(); closeStream(); - // The ONE place an escaping throw is reported (#5848). Both - // callers turn `failed: true` into a 500 that says nothing - // about the cause — `wrap`'s `No response from handler` and - // the `notFound` seam's `Fallback handler failed` — so if the - // diagnosis is not emitted here it does not exist anywhere. - this.reportHandlerFailure(c, err); - resolve({ response: null, failed: true }); + // [#16545] The throw is CARRIED OUT rather than reported here. + // Reporting moved below so the diagnosis can name the answer + // that was actually sent: since this seam may now render a + // declared envelope, a line hard-coding "answered 500 with no + // cause" would be false for exactly the requests the render + // exists to fix. Still reported exactly once per escaped + // throw, and still the ONLY place it is reported (#5848). + resolve({ response: null, failed: true, thrown: err }); }); }); const outcome = await streamPromise; - return { - response: outcome.response ?? capturedResponse ?? null, - failed: outcome.failed, - }; + // A handler that WROTE and then threw keeps what it wrote — unchanged, + // and the reason the render decision is taken here rather than in the + // `catch`: `capturedResponse` is not visible from inside the executor's + // rejection path, so deciding there would have let a declared envelope + // overwrite a response the handler had already produced. + let response = outcome.response ?? capturedResponse ?? null; + let rendered: { status: number; code: unknown } | undefined; + + if (outcome.failed && response === null && opts.renderDeclaredEnvelope) { + const envelope = declaredEnvelopeForThrow(outcome.thrown); + if (envelope) { + response = c.json(envelope.body, envelope.status); + rendered = { status: envelope.status, code: envelope.body.error.code }; + } + } + + if (outcome.failed) this.reportHandlerFailure(c, outcome.thrown, rendered); + + return { response, failed: outcome.failed }; } /** @@ -567,14 +728,39 @@ export class HonoHttpServer implements IHttpServer { * likely place for credentials and PII to sit, and `message` + `stack` * already locate the failure in the code. */ - private reportHandlerFailure(c: any, thrown: unknown): void { + private reportHandlerFailure( + c: any, + thrown: unknown, + /** + * [#16545] What the caller actually answered, when the throw carried a + * declared ADR-0112 envelope and this seam rendered it. Absent for + * every throw that took the unchanged bare-500 arm. + * + * The log line branches on it because the old sentence is a factual + * CLAIM about the response — "answered 500 with no cause in the body" + * — and it stops being true for precisely the requests this card + * repairs. An operator reading `503 SERVICE_UNAVAILABLE` on the wire + * beside a log line insisting the caller got an opaque 500 would be + * debugging the seam instead of the outage. + */ + rendered?: { status: number; code: unknown }, + ): void { try { const method = typeof c?.req?.method === 'string' ? c.req.method : undefined; const path = typeof c?.req?.path === 'string' ? c.req.path : undefined; + // Still `error`, in BOTH arms. A rendered envelope makes the answer + // honest; it does not make the escape intentional — a handler that + // throws its refusal past its own `catch` is still a server-side + // defect, and the AGENTS.md "handed to the CALLER" exemption does + // not apply to a throw nobody caught. this.logger.error( - '[hono] route handler threw — request answered 500 with no cause in the body', + rendered + ? '[hono] route handler threw — request answered with the throw\'s declared ADR-0112 envelope' + : '[hono] route handler threw — request answered 500 with no cause in the body', toLoggableError(thrown), - { method, path }, + rendered + ? { method, path, status: rendered.status, code: rendered.code } + : { method, path }, ); } catch { // Reporting the failure must never become a second failure: a diff --git a/packages/plugins/plugin-hono-server/src/handler-throw-declared-envelope.test.ts b/packages/plugins/plugin-hono-server/src/handler-throw-declared-envelope.test.ts new file mode 100644 index 0000000000..4177960c20 --- /dev/null +++ b/packages/plugins/plugin-hono-server/src/handler-throw-declared-envelope.test.ts @@ -0,0 +1,403 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16545] A throw that ESCAPES a `RouteHandler` carrying a declared ADR-0112 + * `status` + a registered `code` is answered as THAT envelope; a throw that + * carries no such envelope still answers `500` with no cause in the body. + * + * The `domain:cli` half of the #15999 ruling, verbatim: + * + * > **Shared half** (`domain:cli`, hono adapter / registrar wrapper): an + * > escaped throw carrying a declared ADR-0112 `status` + registered `code` is + * > rendered by them, not as a bare `500 INTERNAL_ERROR "No response from + * > handler"`. This changes what an escaped throw means for every direct-mount + * > route; the PR pins that an escaped **non**-envelope throw still answers 500 + * > with no cause in the body. + * + * ## Why the pin is HERE and not on the raw Hono mount + * + * The `domain:cli` seat's scope ruling on the card (comment `5570159125`, + * restated in `5575064522`) is binding, and its first reason is a measurement + * about where a pin can mean anything: + * + * > ⭐ **本卡自带的那条 pin 在那扇门上是空的** —— 普查实测,raw mount 上「带信封的 + * > 抛出」与「不带信封的抛出」今天答的是**逐字节相同**的 `text/plain` 500,所以 + * > 「非信封抛出仍答 500 且 body 里没有 cause」这条 pin **在任何修复存在之前就已经 + * > 绿了**。⛔ 一条对着未修复的树就绿的 pin,正是本车道拒绝出货的东西。 + * + * `HonoHttpServer.wrap()` is the seam every direct-mount route passes — + * `get`/`post`/`put`/`delete`/`patch` each register `this.wrap(handler)`, and + * `IHttpServer` is how `service-datasource`, `packages/rest` and the dispatcher + * bridge all mount. A route mounted through `getRawApp()` funnels through + * NEITHER `wrap()` nor any registrar wrapper and is out of this card's scope by + * that ruling; it is named in the PR body as a known-unreached door and filed + * separately, ⛔ not folded in here. + * + * ## Both directions, and the ablation that makes them evidence + * + * Every assertion below was measured RED against the unfixed tree before the + * fix existed (the PR body carries the run). The render arm fails as `500 / + * INTERNAL_ERROR / "No response from handler"`; the fallback arms pass on the + * unfixed tree by construction — they are the REGRESSION half, and they are the + * half that goes red if the gate is ever widened. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { Logger } from '@objectstack/spec/contracts'; + +import { HonoHttpServer } from './adapter'; + +/** + * The exact bytes `wrap()` answers for a throw carrying no declared envelope, + * and for a handler that simply wrote nothing. Byte-identical to the constant + * `handler-throw-logging.test.ts` pins — deliberately duplicated rather than + * imported, so that suite and this one cannot drift into agreeing about a + * literal that moved. + */ +const FALLBACK_BODY = + '{"success":false,"error":{"code":"INTERNAL_ERROR","message":"No response from handler"}}'; + +/** A silent logger — this file is about the WIRE, not the diagnosis. */ +function quietLogger(): Logger { + const noop = () => {}; + return { debug: noop, info: noop, warn: noop, error: noop, fatal: noop } as unknown as Logger; +} + +function server(): HonoHttpServer { + const s = new HonoHttpServer(0); + s.setLogger(quietLogger()); + return s; +} + +const call = (s: HonoHttpServer, path: string, init?: RequestInit) => + s.getRawApp().fetch(new Request(`http://localhost${path}`, init)); + +/** A throw shaped exactly like the platform's declared refusals. */ +function declared(status: number, code: string, message: string, extra?: Record) { + return Object.assign(new Error(message), { status, code, ...extra }); +} + +describe('an escaped throw carrying a declared ADR-0112 envelope is rendered as that envelope', () => { + /** + * The motivating path, in the shape `service-datasource` really produces it: + * `AuthzStoreUnavailableError` declares `status: 503` / `code: + * SERVICE_UNAVAILABLE` and `requireDatasourceAdmin` re-raises it (#13279), so + * before this card the operator's outage reached the caller as a generic + * fault naming the wrong component. + */ + it('answers the declared status and the declared code, not 500 INTERNAL_ERROR', async () => { + const s = server(); + s.get('/api/v1/datasources', async () => { + throw declared(503, 'SERVICE_UNAVAILABLE', 'The authorization store could not be read.'); + }); + + const res = await call(s, '/api/v1/datasources'); + + expect(res.status).toBe(503); + expect(await res.json()).toEqual({ + success: false, + error: { + code: 'SERVICE_UNAVAILABLE', + message: 'The authorization store could not be read.', + }, + }); + }); + + it('reads `statusCode` as well as `status` — both spellings are produced in this repo', async () => { + const s = server(); + // `plugin-approvals`' lifecycle hooks and `metadata-protocol` throw + // `statusCode`; reading one spelling is how `/api/v1/data` answered 500 for + // a deliberate `409 RECORD_LOCKED` until #7525. + s.get('/api/v1/conflict', async () => { + throw Object.assign(new Error('locked by another process'), { + statusCode: 409, + code: 'LOCK_CONFLICT', + }); + }); + + const res = await call(s, '/api/v1/conflict'); + expect(res.status).toBe(409); + expect((await res.json()).error).toEqual({ + code: 'LOCK_CONFLICT', + message: 'locked by another process', + }); + }); + + it('renders a code registered in the LEDGER, not only a StandardErrorCode', async () => { + const s = server(); + // `ErrorCode` is `StandardErrorCode` ∪ `ERROR_CODE_LEDGER`; a ledger member + // is registered exactly as much as a standard one, and a gate that admitted + // only the standard half would silently demote every service's own code. + s.get('/api/v1/import', async () => { + throw declared(400, 'EXTERNAL_IMPORT_ERROR', 'remote table has no primary key'); + }); + + const res = await call(s, '/api/v1/import'); + expect(res.status).toBe(400); + expect((await res.json()).error.code).toBe('EXTERNAL_IMPORT_ERROR'); + }); + + it('forwards the producer’s `details` and `userMessage` channels', async () => { + const s = server(); + s.get('/api/v1/conflicted', async () => { + throw declared(409, 'RESOURCE_CONFLICT', 'diagnostic prose', { + issues: [{ path: 'name' }], + userMessage: 'Pick a different name.', + }); + }); + + const res = await call(s, '/api/v1/conflicted'); + const body = await res.json(); + expect(res.status).toBe(409); + // `userMessage` is the producer's END-USER-addressed text (#9934) and + // `details` its structured context — the same two channels the REST twin + // forwards. A door that dropped them would answer a narrower envelope than + // the producer declared. + expect(body.error.userMessage).toBe('Pick a different name.'); + expect(body.error.details).toEqual({ issues: [{ path: 'name' }] }); + }); + + /** + * ⚠️ The BLAST RADIUS this card accepts, pinned so it is a decision rather + * than a surprise. `resolveThrownHttpError` — the ONE rule both HTTP doors + * already call — treats the validation SHAPE as a declaration (`err.name === + * 'ValidationError'` ⇒ `400` / `VALIDATION_FAILED`, and `fields[]` alongside). + * So a bare `ValidationError` escaping a direct-mount handler now answers 400 + * where it used to answer a bare 500. + * + * Carving that limb out HERE would mean this seam disagreeing with the very + * function it delegates to, which is the two-door divergence #8016 removed. + * It is accepted and recorded instead. + */ + it('renders the validation SHAPE as the 400 the one rule says it declares', async () => { + const s = server(); + s.get('/api/v1/shaped', async () => { + throw Object.assign(new Error('name is required'), { + name: 'ValidationError', + fields: [{ field: 'name', code: 'required' }], + }); + }); + + const res = await call(s, '/api/v1/shaped'); + const body = await res.json(); + expect(res.status).toBe(400); + expect(body.error.code).toBe('VALIDATION_FAILED'); + expect(body.error.details).toEqual({ fields: [{ field: 'name', code: 'required' }] }); + }); + + it('withholds a LEAKY 5xx message while keeping the declared status and code', async () => { + const s = server(); + // This seam becomes a door that emits a thrown message with this card, so + // it owes the disclosure filter its twins already run (#3867 / #8086) from + // its first day — otherwise a driver dump on a declared 5xx would newly + // reach the client, where the old bare 500 disclosed nothing. + s.get('/api/v1/leaky', async () => { + throw declared(503, 'SERVICE_UNAVAILABLE', 'SQLITE_ERROR: no such table: sys_metadata'); + }); + + const res = await call(s, '/api/v1/leaky'); + const body = await res.json(); + expect(res.status).toBe(503); + expect(body.error.code).toBe('SERVICE_UNAVAILABLE'); + expect(body.error.message).toBe('Internal server error'); + expect(body.error.message).not.toContain('sys_metadata'); + }); + + it('leaves a 4xx message alone — a caller-facing refusal is not a disclosure', async () => { + const s = server(); + s.get('/api/v1/refused', async () => { + throw declared(409, 'RESOURCE_CONFLICT', 'no such table: pass `?version=` to disambiguate'); + }); + + const body = await (await call(s, '/api/v1/refused')).json(); + expect(body.error.message).toBe('no such table: pass `?version=` to disambiguate'); + }); +}); + +describe('an escaped throw that is NOT such an envelope keeps today’s behaviour exactly', () => { + it('a plain Error answers the byte-identical 500 with no cause in the body', async () => { + const s = server(); + s.get('/api/v1/boom', async () => { throw new Error('datasource exploded'); }); + + const res = await call(s, '/api/v1/boom'); + expect(res.status).toBe(500); + expect(await res.text()).toBe(FALLBACK_BODY); + }); + + it('a declared status with NO code takes the fallback arm', async () => { + const s = server(); + s.get('/api/v1/half', async () => { + throw Object.assign(new Error('half declared'), { status: 503 }); + }); + + const res = await call(s, '/api/v1/half'); + expect(res.status).toBe(500); + expect(await res.text()).toBe(FALLBACK_BODY); + }); + + it('a registered code with NO status takes the fallback arm', async () => { + const s = server(); + s.get('/api/v1/codeonly', async () => { + throw Object.assign(new Error('code only'), { code: 'SERVICE_UNAVAILABLE' }); + }); + + const res = await call(s, '/api/v1/codeonly'); + expect(res.status).toBe(500); + expect(await res.text()).toBe(FALLBACK_BODY); + }); + + it('an UNREGISTERED code takes the fallback arm — ⛔ nothing is registered in passing', async () => { + const s = server(); + // A code on this path that is not registered is a LEDGER GAP under the + // #16404 ruling — reported, never minted here. The card states this as a + // prohibition, so it is pinned as one. + s.get('/api/v1/unregistered', async () => { + throw declared(503, 'TENANCY_SERVICE_EXPLODED', 'not in the ledger'); + }); + + const res = await call(s, '/api/v1/unregistered'); + expect(res.status).toBe(500); + expect(await res.text()).toBe(FALLBACK_BODY); + }); + + it('a status ADR-0112 does not declare takes the fallback arm', async () => { + const s = server(); + // 418 is not a key of `HttpStatusErrorCodeMap`, so it is not "a declared + // ADR-0112 status" however registered the code beside it is. + s.get('/api/v1/teapot', async () => { + throw declared(418, 'INTERNAL_ERROR', 'kaboom'); + }); + + const res = await call(s, '/api/v1/teapot'); + expect(res.status).toBe(500); + expect(await res.text()).toBe(FALLBACK_BODY); + }); + + it('a NON-string `code` is context, never a declaration', async () => { + const s = server(); + // A driver errno. Promoting it would put a number in the field callers + // branch on — the drift #3842 removed. + s.get('/api/v1/errno', async () => { + throw Object.assign(new Error('driver said no'), { status: 503, code: 1 }); + }); + + const res = await call(s, '/api/v1/errno'); + expect(res.status).toBe(500); + expect(await res.text()).toBe(FALLBACK_BODY); + }); + + it('a handler that simply writes nothing is unaffected', async () => { + const s = server(); + // Not a throw — the OTHER way to reach the same body. It never had a + // declaration to read, and this card does not give it one. + s.get('/api/v1/silent', async () => { /* resolves without responding */ }); + + const res = await call(s, '/api/v1/silent'); + expect(res.status).toBe(500); + expect(await res.text()).toBe(FALLBACK_BODY); + }); +}); + +describe('the seams this card deliberately does not move', () => { + it('a handler that WROTE and then threw keeps what it wrote', async () => { + const s = server(); + s.get('/api/v1/wrote-then-threw', async (_req, res) => { + res.status(202); + res.json({ accepted: true }); + throw declared(503, 'SERVICE_UNAVAILABLE', 'too late'); + }); + + const res = await call(s, '/api/v1/wrote-then-threw'); + // The declared envelope must not overwrite a response the handler already + // produced — the render decision is taken where `capturedResponse` is + // visible precisely so this stays true. + expect(res.status).toBe(202); + expect(await res.json()).toEqual({ accepted: true }); + }); + + it('a THROWING FALLBACK handler still answers `Fallback handler failed`', async () => { + const s = server(); + // The `notFound` seam's consumer is a fallback, and a fallback that threw + // is a broken consumer — not a refusal it declared. The ruling names + // direct-mount ROUTES, so this arm is opted OUT of the render. + s.setFallbackHandler(() => { + throw declared(503, 'SERVICE_UNAVAILABLE', 'fallback exploded'); + }); + + const res = await call(s, '/api/v1/nothing-mounted'); + expect(res.status).toBe(500); + expect(await res.json()).toEqual({ + success: false, + error: { code: 'INTERNAL_ERROR', message: 'Fallback handler failed' }, + }); + }); +}); + +describe('the diagnosis names the answer that was really sent', () => { + function recording() { + const errors: Array<{ message: string; meta?: Record }> = []; + const noop = () => {}; + const logger = { + debug: noop, info: noop, warn: noop, fatal: noop, + error: (message: string, _err?: unknown, meta?: Record) => { + errors.push({ message, meta }); + }, + } as unknown as Logger; + return { logger, errors }; + } + + it('reports the rendered envelope instead of claiming an opaque 500', async () => { + const s = new HonoHttpServer(0); + const rec = recording(); + s.setLogger(rec.logger); + s.get('/api/v1/outage', async () => { + throw declared(503, 'SERVICE_UNAVAILABLE', 'store unreadable'); + }); + + await call(s, '/api/v1/outage'); + + // Still reported, still exactly once, still at `error`: a refusal thrown + // past its own `catch` is a server-side defect whatever the wire says. + expect(rec.errors).toHaveLength(1); + expect(rec.errors[0]!.message).toContain('declared ADR-0112 envelope'); + expect(rec.errors[0]!.meta).toMatchObject({ + method: 'GET', + path: '/api/v1/outage', + status: 503, + code: 'SERVICE_UNAVAILABLE', + }); + }); + + it('keeps the old sentence for a throw that took the fallback arm', async () => { + const s = new HonoHttpServer(0); + const rec = recording(); + s.setLogger(rec.logger); + s.get('/api/v1/plain', async () => { throw new Error('plain'); }); + + await call(s, '/api/v1/plain'); + + expect(rec.errors).toHaveLength(1); + expect(rec.errors[0]!.message).toBe( + '[hono] route handler threw — request answered 500 with no cause in the body', + ); + expect(rec.errors[0]!.meta).toEqual({ method: 'GET', path: '/api/v1/plain' }); + }); +}); + +/** + * A guard on the reading above: `vi` is imported by every sibling suite in this + * package and an unused import is a lint failure, so it is used here for the + * one thing this file genuinely needs a spy for — proving the recording logger + * is the object the adapter really wrote through. + */ +describe('the logger under test is the one the adapter uses', () => { + it('routes through `setLogger`, not the package default', async () => { + const s = new HonoHttpServer(0); + const error = vi.fn(); + s.setLogger({ debug: vi.fn(), info: vi.fn(), warn: vi.fn(), fatal: vi.fn(), error } as unknown as Logger); + s.get('/api/v1/probe', async () => { throw new Error('probe'); }); + + await call(s, '/api/v1/probe'); + expect(error).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/services/service-datasource/src/__tests__/admin-routes-authz-outage-envelope.test.ts b/packages/services/service-datasource/src/__tests__/admin-routes-authz-outage-envelope.test.ts new file mode 100644 index 0000000000..a85a2a1ca9 --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/admin-routes-authz-outage-envelope.test.ts @@ -0,0 +1,191 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16545] The END-TO-END proof the card names: `GET /api/v1/datasources` with + * a `tenancy` service registered through a THROWING factory answers `503 + * SERVICE_UNAVAILABLE` on the wire. + * + * ## What this file is, and what it is NOT + * + * It is the card's own probe, and it drives `admin-routes.ts` **read-only** — + * `packages/services/**` is another lane's surface and nothing in it is + * modified by this card. The repair lives one package over, in + * `HonoHttpServer.wrap()` (`@objectstack/plugin-hono-server`), which is the + * seam every direct-mount route passes; the wrapper-level pin for both + * directions lives beside it in `handler-throw-declared-envelope.test.ts`. + * This file exists because a seam repaired in isolation and a REQUEST that + * actually reaches the caller are different facts. + * + * ## The measured chain, and the ruling behind each link + * + * 1. `resolveAdmissionTenancyPosture` asks the kernel for `tenancy`. + * 2. The factory rejects with an UNBRANDED error — registered, and failed to + * construct. `classifyAdmissionTenancyPosture` (#13906 decision 1 option A) + * turns exactly that into `AuthzStoreUnavailableError`: declared `status: + * 503`, declared `code: SERVICE_UNAVAILABLE`. + * 3. `requireDatasourceAdmin`'s own `catch` re-raises it rather than + * laundering an outage into a denial — the #13279 ruling, which this card + * leaves completely untouched. Only the RENDERING moves. + * 4. The throw escapes the handler. Before this card the adapter answered + * `500 { code: 'INTERNAL_ERROR', message: 'No response from handler' }` — + * the declared code never reached the caller and the message named the + * wrong component. It now answers the envelope the throw declared. + * + * ## Why there are three arms and not one + * + * `admin-routes.ts` has a SECOND 503 of its own: `resolve()` answers `503 + * SERVICE_UNAVAILABLE "The datasource-admin service is not available."` when + * the service is missing. A suite that asserted only `status === 503` would + * pass just as well against a fixture whose wiring was simply broken, and would + * have measured nothing about this card. So the subject arm asserts the + * OUTAGE's own message, and two controls stand beside it: + * + * - a HEALTHY tenancy lets the request REACH A VERDICT — the anonymous-deny + * `401 UNAUTHENTICATED` these arms' unauthenticated caller has earned; + * - a tenancy service that was NEVER REGISTERED stays quiet and reaches the + * same verdict, because an embedding with no `plugin-auth` is a SUPPORTED + * composition (#13906 decision 1 option A) and this card must not make it + * an outage. + * + * ⭐ The controls answering `401` rather than `200` is the SHARPER reading, and + * it is the discriminator this card is actually about: a 401 is a VERDICT the + * door reached, while the outage arm reaches no verdict at all — which is + * exactly why #13279 refuses to answer it as a denial. Two different 4xx/5xx + * answers separated by whether a decision was ever taken. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_STATUS } from '@objectstack/core'; +import type { PluginContext } from '@objectstack/core'; +import { HonoHttpServer } from '@objectstack/plugin-hono-server'; +import { registerDatasourceAdminRoutes } from '../admin-routes.js'; + +/** The declared refusal `AuthzStoreUnavailableError` carries (ADR-0112). */ +const OUTAGE_STATUS = 503; +const OUTAGE_CODE = 'SERVICE_UNAVAILABLE'; + +/** How the `tenancy` slot behaves for a given arm. */ +type Tenancy = + | { kind: 'healthy' } + | { kind: 'throws' } + | { kind: 'never-registered' }; + +/** + * The kernel's async accessor, in the three shapes the classification + * distinguishes. The brand is an own property, never `instanceof` — a monorepo + * resolves the same module through more than one path and two copies of a class + * make `instanceof` answer false for a genuine instance. + */ +function kernelFor(tenancy: Tenancy) { + return { + getServiceAsync: async (name: string) => { + if (name !== 'tenancy') throw new Error(`unexpected service: ${name}`); + if (tenancy.kind === 'healthy') return { getTenancyPosture: () => 'single' }; + if (tenancy.kind === 'never-registered') { + // Branded "never registered" ⇒ the quiet `undefined` arm. + throw Object.assign(new Error("Service 'tenancy' is not registered"), { + __objectstackServiceNotRegistered: true, + code: 'SERVICE_NOT_REGISTERED', + serviceName: 'tenancy', + }); + } + // Registered and FAILED TO CONSTRUCT — unbranded, so the classification + // raises the ADR-0112 outage. This is the card's scenario. + throw new Error('tenancy factory exploded while constructing'); + }, + }; +} + +function mount(tenancy: Tenancy) { + const listDatasources = vi.fn(async () => []); + const ctx = { + getService: vi.fn((name: string) => { + // No session and no API key reaches these arms: the outage is raised + // while the posture is being resolved, which happens BEFORE any identity + // verdict — that ordering is the whole reason the throw escapes. + if (name === 'auth') return { api: { getSession: async () => undefined } }; + if (name === 'objectql' || name === 'data') return { find: async () => [] }; + return { listDatasources }; + }), + getKernel: () => kernelFor(tenancy), + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + } as unknown as PluginContext; + + const server = new HonoHttpServer(0); + // The adapter's own diagnostics are not this file's subject; keep them quiet + // so a real escaped throw does not print a stack per arm. + server.setLogger({ + debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn(), + } as any); + registerDatasourceAdminRoutes(server, ctx, '/api/v1'); + return { app: server.getRawApp(), listDatasources }; +} + +async function list(tenancy: Tenancy) { + const { app, listDatasources } = mount(tenancy); + const res = await app.fetch(new Request('http://local/api/v1/datasources')); + const text = await res.text(); + let body: any; + try { body = text ? JSON.parse(text) : undefined; } catch { body = { raw: text }; } + return { status: res.status, body, listDatasources }; +} + +describe('GET /api/v1/datasources — an authz-store outage reaches the caller as its declared envelope', () => { + it('answers 503 SERVICE_UNAVAILABLE, not 500 INTERNAL_ERROR "No response from handler"', async () => { + const { status, body } = await list({ kind: 'throws' }); + + expect(status).toBe(OUTAGE_STATUS); + expect(body.success).toBe(false); + expect(body.error.code).toBe(OUTAGE_CODE); + + // The regression, spelled out: these are the exact bytes the caller used to + // get, and they are what this card removes from this path. + expect(body.error.message).not.toBe('No response from handler'); + expect(body.error.code).not.toBe('INTERNAL_ERROR'); + }); + + it('carries the OUTAGE’s own message — not the missing-service 503 one door over', async () => { + const { body } = await list({ kind: 'throws' }); + + // `resolve()` in the same registrar answers `503 SERVICE_UNAVAILABLE "The + // datasource-admin service is not available."`. Asserting only the status + // would let a broken fixture pass for a repaired outage. + expect(body.error.message).toContain('authorization store could not be read'); + expect(body.error.message).toContain('not a permission denial'); + expect(body.error.message).not.toContain('datasource-admin service is not available'); + }); + + it('never reached the service — an unreadable store licenses no verdict (#13279 intact)', async () => { + const { listDatasources } = await list({ kind: 'throws' }); + + // The card moves the RENDERING only. If the outage started resolving to a + // verdict, this would be non-zero and #13279 would have been reversed as a + // rider. + expect(listDatasources).not.toHaveBeenCalled(); + }); +}); + +describe('the controls that make the arm above readable', () => { + it('a HEALTHY tenancy lets the request reach a VERDICT', async () => { + const { status, body } = await list({ kind: 'healthy' }); + + // The caller in these arms carries no session and no API key, so the + // verdict it has earned is the platform's anonymous deny. What matters is + // that a verdict was REACHED: posture resolution completed and the door ran + // on. Nothing here is 503, and nothing here is the adapter's bare 500. + expect(status).toBe(ANONYMOUS_DENY_STATUS); + expect(body.error.code).toBe(ANONYMOUS_DENY_CODE); + expect(status).not.toBe(OUTAGE_STATUS); + }); + + it('a NEVER-REGISTERED tenancy stays quiet and reaches the same verdict', async () => { + // An embedding with no `plugin-auth` is a SUPPORTED composition (#13906 + // decision 1 option A). This card must not turn it into an outage, and a + // gate that rendered every escaped throw would not be able to tell. + const { status, body } = await list({ kind: 'never-registered' }); + + expect(status).toBe(ANONYMOUS_DENY_STATUS); + expect(body.error.code).toBe(ANONYMOUS_DENY_CODE); + expect(status).not.toBe(OUTAGE_STATUS); + }); +});