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

* fix(types): teach looksLikeInternalErrorLeak the shipped dialects' phrasings (#8132)

* test(rest): flip the #8130 residual pin to the withheld assertion (#8132)

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.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))
   );
 }