fix(runtime,hono): close the remaining raw-driver-message exits on the HTTP boundary (#3867 follow-up)

.changeset/dispatcher-returned-error-leak.md

@@ -0,0 +1,42 @@
+---
+"@objectstack/runtime": patch
+"@objectstack/hono": patch
+---
+
+fix(runtime,hono): close the remaining raw-driver-message exits on the HTTP boundary (#3867 follow-up)
+
+#3867 sanitised `dispatcher-plugin`'s `errorResponseBase`. That covers errors
+**thrown** out of `dispatch()` — but not the ones it **returns**. A
+`{handled: true, response}` result goes to `sendResult`, never through that
+catch, and those bodies are built by `HttpDispatcher.error()`, which passed the
+message through verbatim. Sweeping the boundary for the same defect class (the
+follow-up #3867 called for) turned up two more live exits:
+
+**`HttpDispatcher.error()`** — the single construction point for every returned
+error response. Reachable with a raw driver message today through
+`errorFromThrown` (`/meta` save, `/packages` install) and the MCP transport's
+`deps.error(err?.message, 500)`. Pinned by a test that drives
+`PUT /meta/:type/:name` with a throwing `protocol.saveMetaItem`: without the
+guard the response body is the driver's `insert into \`sys_team\` … UNIQUE
+constraint failed: sys_team.id`, naming a physical table and column.
+
+**`@objectstack/hono`'s auth-config route** — a 500 built from a caught
+error with `message: err.message`. The auth service reads from the database, so
+that message can carry a driver dump.
+
+Both apply the same `looksLikeInternalErrorLeak` predicate #3867 put in
+`@objectstack/types`, and both are scoped to **5xx** for the same reason: a 4xx
+message is a deliberate business/validation answer (`Path must be
+/actions/:object/:action`, a hook's own `throw`, a `saveMetaItem` field error)
+and must reach the caller intact. Structured `details` — the semantic `code` and
+per-field `issues` the Studio maps back to inputs — is never touched, so a
+sanitised 500 still carries everything a client can act on.
+
+Diagnostics are unaffected: callers that threw still hand the original error to
+`errorReporter` via `__obsRecordedError`, and every 5xx is logged server-side.
+
+Audited in the same pass and deliberately left alone: the inline error bodies in
+the `ai` / `mcp` domains (static literal strings, no interpolated error text) and
+`plugin-hono-server`'s 403s (4xx, deliberate messages). With this change every
+dynamic message on both dispatcher exits and the REST data routes goes through
+one predicate.

packages/adapters/hono/src/index.ts

@@ -7,7 +7,11 @@ import {
  HttpDispatcher,
  HttpDispatcherResult,
 } from '@objectstack/runtime';
-import { readEnvWithDeprecation } from '@objectstack/types';
+import {
+ readEnvWithDeprecation,
+ looksLikeInternalErrorLeak,
+ INTERNAL_ERROR_MESSAGE,
+} from '@objectstack/types';
 
 /**
  * Re-export the `Hono` type from the copy of `hono` this adapter owns.
@@ -308,11 +312,17 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {
  }
  } catch (error) {
  const err = error instanceof Error ? error : new Error(String(error));
+ // [#3867 follow-up] Same guard the two dispatcher error exits apply.
+ // This is a 500 built from a caught error, and the auth service reads
+ // from the database, so its message can carry a driver dump. Only the
+ // leak case is replaced — an ordinary config error still names itself.
  return c.json({
  success: false,
  error: {
  code: 'auth_config_error',
- message: err.message,
+ message: looksLikeInternalErrorLeak(err.message)
+ ? INTERNAL_ERROR_MESSAGE
+ : err.message,
  },
  }, 500);
  }

packages/runtime/src/http-dispatcher.error-leak.test.ts

@@ -0,0 +1,103 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #3867 follow-up — the OTHER dispatcher error exit.
+ *
+ * #3867 sanitised `dispatcher-plugin`'s `errorResponseBase`, which handles
+ * errors THROWN out of `dispatch()`. It does not cover the errors `dispatch()`
+ * RETURNS: a `{handled: true, response}` result goes to `sendResult`, never
+ * through that catch. Those bodies are built by `HttpDispatcher.error()` — the
+ * single construction point for every returned error — and it passed the
+ * message through verbatim.
+ *
+ * That path is reachable with a raw driver message today via `errorFromThrown`
+ * (`/meta` save, `/packages` install) and the MCP transport's
+ * `deps.error(err?.message, 500)`.
+ *
+ * `error()` is private, so these drive it the way real traffic does: through
+ * `dispatch()` on routes whose service throws.
+ */
+
+import { describe, it, expect } from 'vitest';
+
+import { HttpDispatcher } from './http-dispatcher.js';
+
+const SQL_DUMP = 'insert into `sys_team` (`id`) values (?) - UNIQUE constraint failed: sys_team.id';
+
+/**
+ * A kernel whose `protocol` service throws on `saveMetaItem` — the `/meta` PUT
+ * route catches it and RETURNS `deps.errorFromThrown(e, 400)`, which is the
+ * returned-error path this guard covers (it never reaches the plugin's
+ * throw-side `errorResponseBase`).
+ */
+function makeDispatcher(saveError: unknown) {
+ const protocol = {
+ saveMetaItem: async () => { throw saveError; },
+ };
+ const kernel: any = {
+ getService: (name: string) => (name === 'protocol' ? protocol : undefined),
+ getServiceAsync: async (name: string) => (name === 'protocol' ? protocol : undefined),
+ };
+ return new HttpDispatcher(kernel);
+}
+
+async function putMeta(saveError: unknown) {
+ const dispatcher = makeDispatcher(saveError);
+ return dispatcher.dispatch(
+ 'PUT',
+ '/meta/object/widget',
+ { name: 'widget' },
+ {},
+ {} as any,
+ );
+}
+
+describe('#3867 follow-up — HttpDispatcher.error() does not return raw driver messages', () => {
+ it('replaces a SQL dump on a returned 5xx', async () => {
+ const err = Object.assign(new Error(SQL_DUMP), { status: 500 });
+ const result: any = await putMeta(err);
+
+ expect(result.response.status).toBe(500);
+ expect(result.response.body.error.message).toBe('Internal server error');
+ expect(String(result.response.body.error.message)).not.toContain('sys_team');
+ });
+
+ it('leaves a deliberate 4xx message intact even when it resembles SQL', async () => {
+ // The tier that matters: `errorFromThrown` defaults `/meta` saves to
+ // 400, and a validation message is the caller's answer — swallowing it
+ // would be a worse bug than the leak.
+ const err = Object.assign(new Error('unique constraint on name — pick another'), {
+ status: 422,
+ });
+ const result: any = await putMeta(err);
+
+ expect(result.response.status).toBe(422);
+ expect(result.response.body.error.message).toBe('unique constraint on name — pick another');
+ });
+
+ it('leaves an ordinary 5xx message intact — only leaks are replaced', async () => {
+ const err = Object.assign(new Error('metadata store is unavailable'), { status: 503 });
+ const result: any = await putMeta(err);
+
+ expect(result.response.status).toBe(503);
+ expect(result.response.body.error.message).toBe('metadata store is unavailable');
+ });
+
+ it('preserves structured `details` (code / issues) while sanitising the message', async () => {
+ // `details` carries the semantic code and per-field `issues` the UI maps
+ // back to inputs; it is never free-form driver prose, so the guard must
+ // not touch it.
+ const err = Object.assign(new Error(SQL_DUMP), {
+ status: 500,
+ code: 'STORAGE_FAILURE',
+ issues: [{ path: 'name', message: 'taken', code: 'duplicate' }],
+ });
+ const result: any = await putMeta(err);
+
+ expect(result.response.body.error.message).toBe('Internal server error');
+ expect(result.response.body.error.details).toMatchObject({
+ code: 'STORAGE_FAILURE',
+ issues: [{ path: 'name', message: 'taken', code: 'duplicate' }],
+ });
+ });
+});

packages/runtime/src/http-dispatcher.ts

@@ -3,7 +3,7 @@
 import {
  ObjectKernel, getEnv, evaluateAuthGate, isAuthGateAllowlisted,
 } from '@objectstack/core';
-import { isMcpServerEnabled } from '@objectstack/types';
+import { isMcpServerEnabled, looksLikeInternalErrorLeak, INTERNAL_ERROR_MESSAGE } from '@objectstack/types';
 import { measureServerTiming, allowPerfDisclosure, isPerfDisclosurePrincipal } from '@objectstack/observability';
 import { CoreServiceName } from '@objectstack/spec/system';
 import { readServiceSelfInfo } from '@objectstack/spec/api';
@@ -448,10 +448,37 @@ export class HttpDispatcher {
  };
  }
 
+ /**
+ * The single construction point for every error response this dispatcher
+ * RETURNS. Distinct from `dispatcher-plugin`'s `errorResponseBase`, which
+ * covers the errors that are THROWN out of `dispatch()` — a returned
+ * `{handled: true, response}` goes to `sendResult`, never through that
+ * catch. #3867 sanitised the thrown path; this is the returned one, and it
+ * needs the same guard for the same reason.
+ *
+ * Reachable with a raw driver/engine message today via
+ * {@link errorFromThrown} (`/meta` save, `/packages` install) and the MCP
+ * transport's `deps.error(err?.message, 500)` — any of which can be
+ * carrying a SQL dump naming physical tables and columns.
+ *
+ * Scoped to 5xx: a 4xx message is a deliberate business/validation answer
+ * (`Path must be /actions/:object/:action`, a hook's own `throw`, a
+ * `saveMetaItem` field error) and must reach the caller intact. `details`
+ * is left alone — it carries structured `code`/`issues` the UI maps to
+ * fields, never free-form driver prose.
+ *
+ * The unsanitised error is not lost: callers that THREW still hand the
+ * original to `errorReporter` via `__obsRecordedError`, and every 5xx is
+ * logged server-side.
+ */
  private error(message: string, code: number = 500, details?: any) {
+ const safe =
+ code >= 500 && looksLikeInternalErrorLeak(message)
+ ? INTERNAL_ERROR_MESSAGE
+ : message;
  return {
  status: code,
- body: { success: false, error: { message, code, details } }
+ body: { success: false, error: { message: safe, code, details } }
  };
  }