diff --git a/.changeset/23532-hono-undeclared-500-leak-filter.md b/.changeset/23532-hono-undeclared-500-leak-filter.md new file mode 100644 index 00000000000..cd2bdcd6f64 --- /dev/null +++ b/.changeset/23532-hono-undeclared-500-leak-filter.md @@ -0,0 +1,17 @@ +--- +'@objectstack/hono': patch +--- + +fix(hono): `createHonoApp`'s catch arms withhold a driver diagnostic from an undeclared `500` instead of shipping it to the client (#23532) + +Clause-②: no + +`@objectstack/hono`'s `createHonoApp` has two catch arms: the auth mount's, and the `${prefix}/*` catch-all's around `HttpDispatcher.dispatch()`. A throw that does not declare an ADR-0112 envelope is answered from its own `message`, and that message was shipped unread. A driver diagnostic therefore reached the client verbatim: on a host whose object `ghost` was registered but whose table was never synced, `POST /api/v1/data/ghost` answered `500 INTERNAL_ERROR` with the message `insert into [statement and bound values redacted] - no such table: ghost`, naming the SQLite error and the physical table. + +**What changed.** Before it ships that message, the arm now asks `looksLikeInternalErrorLeak` (`@objectstack/types`), as every other door that emits a thrown message already does: `HttpDispatcher.error`, the REST registrars, `@objectstack/plugin-hono-server`'s direct-mount routes and this adapter's own `/auth/config` arm. When the answer is a `5xx` and the message looks like a driver or SQL dump, the message is replaced by the same text those doors use, `Internal server error`. The status and the `code` are unchanged. + +**What did NOT change:** + +- a `5xx` whose message does not look like a dump keeps it: a bare `new Error('boom')` is still `500 INTERNAL_ERROR` with the message `boom`; +- a `4xx` keeps its message, as on the other doors, because a `4xx` message is an answer meant for the caller; +- a throw that declares an ADR-0112 envelope is answered as declared, as before. diff --git a/packages/adapters/hono/src/hono-thrown-message-disclosure.test.ts b/packages/adapters/hono/src/hono-thrown-message-disclosure.test.ts new file mode 100644 index 00000000000..fce0c51db78 --- /dev/null +++ b/packages/adapters/hono/src/hono-thrown-message-disclosure.test.ts @@ -0,0 +1,154 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23532] `createHonoApp`'s two catch arms run the 5xx disclosure filter + * before they ship an UNDECLARED throw's message. + * + * ## The defect + * + * Both arms (the `${authMount}/*` mount's and the `${prefix}/*` catch-all's + * around `HttpDispatcher.dispatch()`) end in `thrownJson`. A throw the ADR-0112 + * recogniser does not accept fell to `errorJson(c, err.message, …)` with the + * message unread, so a driver diagnostic reached the client verbatim. Measured + * through a real host (an object registered, its table never synced): + * + * POST /api/v1/data/ghost + * 500 {"success":false,"error":{"code":"INTERNAL_ERROR", + * "message":"insert into [statement and bound values redacted] - no such table: ghost"}} + * + * The driver had already redacted the statement and its values; the SQLite + * diagnostic and the physical table name still went out. Every other door that + * emits a thrown message asks `looksLikeInternalErrorLeak` first + * (`HttpDispatcher.error`, `packages/rest`'s registrars, the recogniser's own + * 5xx limb, and this file's `/auth/config` arm), so this one now does too. + * + * ## What is pinned, per arm + * + * - the probe above, as the throw: `500`, `INTERNAL_ERROR`, and the twins' + * `INTERNAL_ERROR_MESSAGE` in place of the message; neither the table name + * nor the diagnostic is anywhere in the body; + * - a plain `new Error('boom')`: `500`, `INTERNAL_ERROR`, `boom` — an ordinary + * undeclared 5xx still names itself. + * + * And the filter's scope on the catch-all, the same as the twins': a 5xx that + * declares its status through `statusCode` is filtered at that status, and a + * 4xx keeps its message, because a 4xx message is a caller-facing answer. + * + * ## What this file is + * + * This package's vitest config aliases `@objectstack/runtime` to a stub, so the + * dispatcher here is a fixture whose methods throw what a case hands them; + * `createHonoApp`, both of its arms and `thrownJson` are the real source. + * The composed-host sibling, a real kernel and the real dispatcher, is + * `packages/qa/http-conformance/src/hono-declared-envelope.conformance.test.ts`. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { INTERNAL_ERROR_MESSAGE } from '@objectstack/types'; + +const mockDispatcher = { + getDiscoveryInfo: vi.fn().mockReturnValue({ version: '1.0', routes: {} }), + handleAuth: vi.fn(), + dispatch: vi.fn(), +}; + +vi.mock('@objectstack/runtime', () => ({ + HttpDispatcher: function HttpDispatcher() { return mockDispatcher; }, +})); + +import { createHonoApp } from './index'; + +const PREFIX = '/api/v1'; +/** The card's probe: the message the SQL driver threw for an unsynced table. */ +const PROBE = 'insert into [statement and bound values redacted] - no such table: ghost'; +const PLAIN = 'boom'; + +interface Answer { status: number; body: any; text: string } + +const send = async (app: ReturnType, method: string, path: string): Promise => { + const res = await app.request(`http://localhost${path}`, { + method, + ...(method === 'GET' ? {} : { headers: { 'content-type': 'application/json' }, body: JSON.stringify({ name: 'x' }) }), + }); + const text = await res.text(); + return { status: res.status, body: JSON.parse(text), text }; +}; + +/** A kernel whose `auth` service's `handleRequest` throws `thrown`. */ +const kernelWithThrowingAuth = (thrown: unknown) => ({ + name: 'test-kernel', + getServiceAsync: vi.fn(async () => ({ handleRequest: vi.fn(async () => { throw thrown; }) })), +}); + +const withheld = { success: false, error: { code: 'INTERNAL_ERROR', message: INTERNAL_ERROR_MESSAGE } }; +const kept = (message: string) => ({ success: false, error: { code: 'INTERNAL_ERROR', message } }); + +/** The disclosure itself, independent of the replacement text. */ +const expectNoDiagnostic = (a: Answer) => { + expect(a.text).not.toContain('ghost'); + expect(a.text).not.toContain('no such table'); +}; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('[#23532] the dispatch() catch-all arm', () => { + const app = () => createHonoApp({ kernel: { name: 'test-kernel' } as any, prefix: PREFIX, cors: false }); + + it('a driver diagnostic thrown undeclared: 500 INTERNAL_ERROR with the withheld message', async () => { + mockDispatcher.dispatch.mockRejectedValueOnce(new Error(PROBE)); + const answer = await send(app(), 'POST', `${PREFIX}/data/ghost`); + expect(answer.status, answer.text).toBe(500); + expect(answer.body).toEqual(withheld); + expectNoDiagnostic(answer); + }); + + it('control: a plain bare Error keeps its message', async () => { + mockDispatcher.dispatch.mockRejectedValueOnce(new Error(PLAIN)); + const answer = await send(app(), 'POST', `${PREFIX}/data/ghost`); + expect(answer.status, answer.text).toBe(500); + expect(answer.body).toEqual(kept(PLAIN)); + }); + + it('a 5xx declared through `statusCode` alone is filtered at that status', async () => { + mockDispatcher.dispatch.mockRejectedValueOnce(Object.assign(new Error(PROBE), { statusCode: 503 })); + const answer = await send(app(), 'GET', `${PREFIX}/data/ghost`); + expect(answer.status, answer.text).toBe(503); + expect(answer.body).toEqual({ success: false, error: { code: 'SERVICE_UNAVAILABLE', message: INTERNAL_ERROR_MESSAGE } }); + expectNoDiagnostic(answer); + }); + + it('scope: a 4xx keeps its message, as on the twin doors', async () => { + mockDispatcher.dispatch.mockRejectedValueOnce(Object.assign(new Error(PROBE), { statusCode: 400 })); + const answer = await send(app(), 'GET', `${PREFIX}/data/ghost`); + expect(answer.status, answer.text).toBe(400); + expect(answer.body.error.message).toBe(PROBE); + }); +}); + +describe('[#23532] the auth-mount arm', () => { + it('an auth service that throws a driver diagnostic: 500 INTERNAL_ERROR with the withheld message', async () => { + const app = createHonoApp({ kernel: kernelWithThrowingAuth(new Error(PROBE)) as any, prefix: PREFIX, cors: false }); + const answer = await send(app, 'GET', `${PREFIX}/auth/get-session`); + expect(answer.status, answer.text).toBe(500); + expect(answer.body).toEqual(withheld); + expectNoDiagnostic(answer); + }); + + it('control: an auth service that throws a plain bare Error keeps its message', async () => { + const app = createHonoApp({ kernel: kernelWithThrowingAuth(new Error(PLAIN)) as any, prefix: PREFIX, cors: false }); + const answer = await send(app, 'GET', `${PREFIX}/auth/get-session`); + expect(answer.status, answer.text).toBe(500); + expect(answer.body).toEqual(kept(PLAIN)); + }); + + it('the legacy dispatcher fallback (no auth service) throwing a driver diagnostic: withheld', async () => { + mockDispatcher.handleAuth.mockRejectedValueOnce(new Error(PROBE)); + const app = createHonoApp({ kernel: { name: 'test-kernel' } as any, prefix: PREFIX, cors: false }); + const answer = await send(app, 'POST', `${PREFIX}/auth/sign-in/email`); + expect(answer.status, answer.text).toBe(500); + expect(answer.body).toEqual(withheld); + expectNoDiagnostic(answer); + }); +}); diff --git a/packages/adapters/hono/src/index.ts b/packages/adapters/hono/src/index.ts index 3529a20128d..446655b1e9b 100644 --- a/packages/adapters/hono/src/index.ts +++ b/packages/adapters/hono/src/index.ts @@ -529,11 +529,37 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono { * before: a bare `Error` is still `500` carrying `code` and `message`, with * no `cause`, and a throw declaring `statusCode` with no registered code * keeps that status. + * + * [#23532] …with one exception, the disclosure filter. That answer ships the + * thrown message, so this arm is a door emitting a thrown message, and it + * runs `looksLikeInternalErrorLeak` (`@objectstack/types`) exactly as the + * other such doors do: `HttpDispatcher.error`, `packages/rest`'s registrars, + * the recogniser's own 5xx limb above and this file's `/auth/config` arm. + * Without it a driver diagnostic reached the client verbatim. Measured + * through this host (an object registered, its table never synced): + * + * POST /api/v1/data/ghost + * 500 {"success":false,"error":{"code":"INTERNAL_ERROR", + * "message":"insert into [statement and bound values redacted] - no such table: ghost"}} + * + * Same scope as the twins: only a 5xx, because a 4xx message is a + * caller-facing answer by design, and only a message the predicate flags, + * so an ordinary bare `Error` still names itself. The replacement is the + * twins' `INTERNAL_ERROR_MESSAGE`, never a new literal. ⛔ This does not + * withhold every undeclared 5xx: whether an undeclared 5xx stays legible is + * a standing decision this arm does not reopen. The status and `code` are + * unchanged; only the message is replaced. */ const thrownJson = (c: any, err: any) => { const declared = declaredEnvelopeForThrow(err); if (declared) return c.json(declared.body, declared.status); - return errorJson(c, err.message || 'Internal Server Error', err.statusCode || 500); + const status = err.statusCode || 500; + const message = err.message || 'Internal Server Error'; + return errorJson( + c, + status >= 500 && looksLikeInternalErrorLeak(message) ? INTERNAL_ERROR_MESSAGE : message, + status, + ); }; const toResponse = (c: any, result: HttpDispatcherResult) => {