fix(types): withhold the Postgres and bare-SQLite phrasings of a driver failure (#8132)

.changeset/error-leak-shipped-dialect-phrasings.md

@@ -0,0 +1,24 @@
+---
+'@objectstack/types': patch
+---
+
+Withhold the Postgres and bare-SQLite phrasings of a driver failure from HTTP error bodies
+
+`looksLikeInternalErrorLeak` recognised SQLite's `SQLITE_ERROR: no such table: sys_metadata`
+but not the Postgres phrasing of the same condition, `relation "sys_metadata" does not exist`.
+The result was that one failure disclosed a physical table name or not depending on which
+engine was underneath, from every boundary that applies the predicate — `HttpDispatcher.error`,
+the declarative endpoint executor, the dispatcher plugin, the direct-mount package door and the
+Hono auth-config route.
+
+The predicate now also recognises, for the engines this repo actually runs:
+
+- Postgres `relation "…" does not exist` and `column "…" does not exist` (42P01/42703), which
+ covers the `… of relation "…"` sub-object family as a superstring;
+- Postgres `permission denied for table|relation|sequence|database …` (42501);
+- SQLite/libsql `no such table:` / `no such column:` in their bare, un-prefixed form.
+
+Each phrasing is anchored on the driver's own template — a quoted identifier, or the trailing
+colon — never on the bare tail, so ordinary business messages such as "user does not exist" are
+still returned to the caller unchanged. The predicate is applied only where the outcome is
+already a 5xx, and the full text still reaches the server log and the error reporter.

packages/rest/src/package-door-5xx-message-sanitization.test.ts

@@ -232,29 +232,31 @@ describe('[#8086] a real sys_metadata failure, walked in process through this do
  }, 60_000);
 
  /**
- * ⚠️ The CEILING of option B, measured and pinned rather than papered over.
+ * [#8132] Was the residual; now the second green.
  *
- * The ruled fix applies the SHARED predicate, which is a heuristic over the
- * message and recognises no Postgres "relation … does not exist" phrasing —
- * measured false, asserted below. So that dialect's line still travels
- * through this door after this change, exactly as it travels through the
- * dispatcher twin, which runs the same predicate (#3867). The two doors
- * therefore still AGREE, which is what this card was about; what remains is a
- * property of the heuristic, shared by every boundary that applies it.
+ * This case was added by #8086 as a deliberately-red-in-future pin: the
+ * shared predicate was a heuristic over the message and knew no Postgres
+ * `relation … does not exist` phrasing, so that dialect's line still
+ * travelled through this door while SQLite's was withheld — the same
+ * condition, disclosed or not depending on which engine was underneath. It
+ * asserted that gap positively so the day it closed would be visible.
  *
- * This is not an argument for widening the predicate here — that would be a
- * new rule at one door, re-creating the divergence this closes. It is the
- * argument for **option C**: `metadata-protocol` should not interpolate
- * driver text into client-facing messages at all, which is the only fix that
- * does not depend on recognising a dialect's phrasing. Filed separately.
+ * #8132 closed it in the predicate, where it belonged — so the assertion is
+ * INVERTED here rather than deleted, and the pair above/below now proves the
+ * property that actually matters: this door answers the same withheld
+ * envelope for BOTH dialects of one failure.
  *
- * This case goes RED the day the shared predicate learns this phrasing or C
- * lands — which is precisely when a reader should come back and re-read the
- * paragraph above, instead of consuming a green suite as proof that the door
- * is covered.
+ * ⚠️ Still not the structural cure, and this comment is the reason the
+ * pointer survives the flip. The predicate now recognises the two engines
+ * this repo runs; it is a phrasing test, and a phrasing test can only ever
+ * know the dialects someone has met. **Option C** — `metadata-protocol` not
+ * interpolating driver text into client-facing messages at all — is the fix
+ * whose correctness does not depend on that, and is tracked as #8136. The
+ * anti-vacuity guard at the top of this describe block goes red when C
+ * lands, which is the intended signal to revisit this whole section.
  */
- it('the residual: Postgres phrasing trips no keyword, so it still travels (option C is the cure)', async () => {
- expect(looksLikeInternalErrorLeak(PG_NO_RELATION)).toBe(false);
+ it('the Postgres phrasing of the same failure is withheld too, by the shared predicate', async () => {
+ expect(looksLikeInternalErrorLeak(PG_NO_RELATION)).toBe(true);
 
  const protocol = await bootRealProtocol(PG_NO_RELATION);
  const captured = await drive(
@@ -267,10 +269,13 @@ describe('[#8086] a real sys_metadata failure, walked in process through this do
  const error = expectDeclaredEnvelope(captured);
  expect(captured.status).toBe(500);
  expect(error.code).toBe('INTERNAL_ERROR');
- // Stated as the fact it is: withheld would be better, and the predicate
- // cannot tell. Asserted positively so the day it changes is visible.
- expect(error.message).toContain('does not exist');
- expect(error.message).not.toBe(INTERNAL_ERROR_MESSAGE);
+ // The same positive shape the SQLite case asserts, which is the point of
+ // the flip: one door, one envelope, regardless of the engine underneath.
+ expect(error.message).toBe(INTERNAL_ERROR_MESSAGE);
+
+ const wire = JSON.stringify(captured.body);
+ expect(wire).not.toContain('does not exist');
+ expect(wire).not.toContain('sys_metadata');
  }, 60_000);
 });
 

packages/types/src/error-leak.test.ts

@@ -87,6 +87,81 @@ describe('looksLikeInternalErrorLeak', () => {
  });
 });
 
+/**
+ * [#8132] The shipped dialects' phrasings, both directions.
+ *
+ * The gap this pins: the keyword set caught SQLite's `SQLITE_ERROR: no such
+ * table: sys_metadata` (via the `sqlite_` limb) while the Postgres phrasing of
+ * *the same condition* — `relation "sys_metadata" does not exist` — returned
+ * FALSE and shipped a physical table name from every boundary that applies the
+ * predicate.
+ *
+ * Scope is deliberately the dialects this repo actually RUNS (SQLite/libsql and
+ * Postgres, via `driver-sql`), not a census of MySQL/MSSQL/Oracle spellings
+ * nobody here has met — the unbounded-list trap the module note argues against.
+ *
+ * The negative half is the load-bearing half. A bare `includes('does not
+ * exist')` would have matched "user does not exist" and started replacing
+ * ordinary business answers with "Internal server error", so every phrasing is
+ * anchored on the driver's own template (a QUOTED identifier, or the trailing
+ * colon) and the near-miss cases below are what prove that anchor is real
+ * rather than incidental.
+ */
+describe('looksLikeInternalErrorLeak — shipped-dialect phrasings (#8132)', () => {
+ it.each([
+ // Postgres 42P01. The exact string measured false on the shipping predicate.
+ ['postgres missing relation', 'relation "sys_metadata" does not exist'],
+ [
+ 'postgres missing relation wrapped in a producer sentence',
+ 'Failed to delete customization overlay: relation "sys_metadata" does not exist',
+ ],
+ // Postgres 42703, read path — no relation named, so the sub-object
+ // helpers in `relation-sub-object.ts` deliberately do not see it.
+ ['postgres missing column (read path)', 'column "bogus" does not exist'],
+ // Postgres 42703 write path / 42704: these carry a complete missing-TABLE
+ // phrase as a substring. For a LEAK verdict that overlap is harmless —
+ // both spellings are a leak — which is why this predicate needs none of
+ // the ordering care `matchMissingColumnOfRelation` exists to provide.
+ ['postgres missing column of relation', 'column "label" of relation "sys_team" does not exist'],
+ [
+ 'postgres missing constraint of relation',
+ 'constraint "uq_sys_team_name" of relation "sys_team" does not exist',
+ ],
+ // Postgres 42501 — names a physical table the caller never asked about.
+ ['postgres permission denied for table', 'permission denied for table sys_user'],
+ ['postgres permission denied for relation', 'permission denied for relation sys_user'],
+ // SQLite/libsql message-only errors: the same conditions with NO
+ // `SQLITE_` prefix to trip the existing limb. Measured shapes in this
+ // repo — `metadata/src/utils/schema-sync-errors.ts` documents both.
+ ['sqlite bare missing table', 'no such table: sys_metadata'],
+ ['sqlite bare missing table with a schema prefix', 'no such table: main.sys_metadata_history'],
+ ['sqlite bare missing column', 'no such column: bogus'],
+ ])('catches %s', (_label, message) => {
+ expect(looksLikeInternalErrorLeak(message)).toBe(true);
+ });
+
+ /**
+ * ⛔ The false-positive guard. Every one of these contains the tail of a
+ * phrasing above and is an ordinary message a caller is entitled to read.
+ * If someone later relaxes an anchor to a bare `includes(...)`, these go red
+ * — which is the whole point of writing them down.
+ */
+ it.each([
+ ['a business message about a missing user', 'user does not exist'],
+ ['a business message about a missing record', 'record does not exist'],
+ ['a sentence a hook author wrote', 'The customer you selected does not exist'],
+ // The quote anchor, stated as a test: unquoted prose that uses the same
+ // NOUN is not a driver line. The looser `includes('relation') &&
+ // includes('does not exist')` reading would match this one.
+ ['prose merely using the word relation', 'This relation does not exist in the diagram'],
+ ['prose merely using the word column', 'The column layout does not exist'],
+ // No physical object kind, so not Postgres' ACL template.
+ ['an ordinary permission refusal', 'Permission denied for this operation'],
+ ])('leaves %s alone', (_label, message) => {
+ expect(looksLikeInternalErrorLeak(message)).toBe(false);
+ });
+});
+
 /**
  * [#5811] The declaration half. `looksLikeInternalErrorLeak` asks whether a
  * message SOUNDS internal; this asks whether the producer SAID it was a server

packages/types/src/error-leak.ts

@@ -35,18 +35,71 @@
 /** Generic replacement text for a message that trips {@link looksLikeInternalErrorLeak}. */
 export const INTERNAL_ERROR_MESSAGE = 'Internal server error';
 
+/**
+ * [#8132] The phrasings of the dialects this repo actually RUNS, each anchored
+ * on the driver's own errmsg template rather than on its tail.
+ *
+ * The gap that forced these: the keyword set below caught SQLite's
+ * `SQLITE_ERROR: no such table: sys_metadata` through the `sqlite_` limb, while
+ * the Postgres phrasing of *the same condition* —
+ * `relation "sys_metadata" does not exist` — matched nothing and shipped a
+ * physical table name to the client from every boundary that applies the
+ * predicate.
+ *
+ * **Why anchored, and never on the bare tail.** `does not exist` is ordinary
+ * business English: "user does not exist", "record does not exist". Matching
+ * that substring would replace legitimate answers with `Internal server error`,
+ * so each pattern requires what the DRIVER always emits and prose usually does
+ * not — a quoted identifier, or the trailing colon of SQLite's template. The
+ * negative cases in `error-leak.test.ts` pin that distinction.
+ *
+ * **Why the list stops here.** The module note above argues against growing a
+ * driver taxonomy, and it is right that the list is unbounded *across dialects*
+ * — MySQL/MSSQL/Oracle each phrase all of this differently and nobody here runs
+ * them. These are not a census: they are the two engines `driver-sql`,
+ * `driver-turso` and `driver-sqlite-wasm` actually reach. A dialect this repo
+ * does not run gets no entry, and {@link declaresServerFault} remains the
+ * answer that does not depend on phrasing at all.
+ *
+ * ⚠️ Related but NOT reusable: `relation-sub-object.ts` owns the same Postgres
+ * sentence for two other questions (which column? / is this a sub-object?), and
+ * its note warns that its two widths must never be collapsed. Neither answers
+ * "is this a leak", and its central problem does not arise here: a message like
+ * `column "label" of relation "sys_team" does not exist` contains a complete
+ * missing-TABLE phrase as a substring, which is a hazard when you are deciding
+ * WHICH object is missing and a non-issue when the verdict is "leak" either way.
+ * That is why this asks its own question with its own patterns.
+ */
+const DIALECT_LEAK_PHRASINGS: readonly RegExp[] = [
+ // Postgres 42P01 / 42703 (and, as a superstring, the `… of relation "…"`
+ // sub-object family: 42704 and friends). The quotes are required because
+ // Postgres always emits them here.
+ /\b(?:relation|column)\s+["'`][^"'`]+["'`]\s+does not exist/i,
+ // Postgres 42501. Restricted to physical object kinds: `schema`, `view`,
+ // `function` and `column` are all ObjectStack AUTHORING vocabulary, so a
+ // product message could legitimately use them and a miss is the cheap
+ // direction (the outcome is already a 5xx).
+ /\bpermission denied for (?:table|relation|sequence|database)\b/i,
+ // SQLite/libsql, message-only form. The `sqlite_` limb below catches these
+ // only when the driver prefixed its code; `better-sqlite3` and libsql both
+ // raise them bare, which is the shape measured across this repo.
+ /\bno such (?:table|column):/i,
+];
+
 /**
  * Whether `message` looks like a raw SQL statement or driver/engine dump that
  * must not be returned to an API client.
  *
  * Matches: dialect error codes (`SQLSTATE`, `sqlite_*`), bare statements
  * (a message that *starts* as `SELECT`/`INSERT INTO`/`UPDATE`/`DELETE FROM` —
- * drivers prefix the offending SQL to their message), and constraint-violation
- * dumps, which name physical tables and columns.
+ * drivers prefix the offending SQL to their message), constraint-violation
+ * dumps, which name physical tables and columns, and the
+ * {@link DIALECT_LEAK_PHRASINGS} of the engines this repo ships.
  *
  * Does NOT match ordinary business or validation messages, which is why the
- * statement forms are anchored with `startsWith`: a legitimate message may
- * *mention* "update" without being one.
+ * statement forms are anchored with `startsWith` and the dialect phrasings on
+ * the driver's template: a legitimate message may *mention* "update", or say
+ * "does not exist" about a business record, without being either.
  */
 export function looksLikeInternalErrorLeak(message: string | undefined | null): boolean {
  if (!message) return false;
@@ -60,7 +113,8 @@ export function looksLikeInternalErrorLeak(message: string | undefined | null):
  lower.startsWith('delete from ') ||
  lower.includes('constraint failed') ||
  lower.includes('unique constraint') ||
- lower.includes('foreign key')
+ lower.includes('foreign key') ||
+ DIALECT_LEAK_PHRASINGS.some((pattern) => pattern.test(lower))
  );
 }