Skip to content

Commit 8ac2323

Browse files
Jack Qclaude
andauthored
docs(types): correct error-leak.ts's false MySQL dialect claim, and pin the uncovered dialect (#8824)
`DIALECT_LEAK_PHRASINGS` said MySQL/MSSQL/Oracle "nobody here runs them". Measurably false for MySQL on the same tree: driver-sql branches on mysql/mysql2, CI stands up a live mysql:8.0 for the required Temporal Conformance check, and live MySQL 8.0.46 measurements landed driver fixes (#8621, #8622). The claim is a security-reasoning input and was consumed as one on PR #8737, so the correction replaces the reachability claim with a COVERAGE statement and adds the pins that keep it honest. Behaviour is unchanged: no keyword is added, and whether MySQL is a supported deployment target or a tested dialect stays an open product question on #8739. Part of #8739 Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn Co-authored-by: Claude <noreply@anthropic.com>
1 parent b79b294 commit 8ac2323

2 files changed

Lines changed: 105 additions & 13 deletions

File tree

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

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,13 @@ describe('looksLikeInternalErrorLeak', () => {
9696
* FALSE and shipped a physical table name from every boundary that applies the
9797
* predicate.
9898
*
99-
* Scope is deliberately the dialects this repo actually RUNS (SQLite/libsql and
100-
* Postgres, via `driver-sql`), not a census of MySQL/MSSQL/Oracle spellings
101-
* nobody here has met — the unbounded-list trap the module note argues against.
99+
* Scope is deliberately the two dialects the list COVERS (SQLite/libsql and
100+
* Postgres, via `driver-sql`), not a census of every dialect's spelling — the
101+
* unbounded-list trap the module note argues against. ⚠️ Covered is not the
102+
* same as reachable: #8739 measured that MySQL runs here (a live `mysql:8.0`
103+
* behind a required check) while this list does not cover it, and the
104+
* "uncovered dialects, uncovered on purpose" block below pins that gap so the
105+
* scope sentence cannot quietly go false again.
102106
*
103107
* The negative half is the load-bearing half. A bare `includes('does not
104108
* exist')` would have matched "user does not exist" and started replacing
@@ -162,6 +166,62 @@ describe('looksLikeInternalErrorLeak — shipped-dialect phrasings (#8132)', ()
162166
});
163167
});
164168

169+
/**
170+
* [#8739] The uncovered dialect, pinned as a MEASUREMENT rather than a
171+
* paragraph.
172+
*
173+
* The module used to say nobody here runs MySQL, and a reviewer sizing a
174+
* disclosure residual on PR #8737 quoted it in good faith. The claim was false:
175+
* `driver-sql` branches on `mysql`/`mysql2`, CI stands up a live `mysql:8.0`
176+
* for a required check, and live MySQL 8.0.46 measurements landed driver fixes
177+
* (#8621, #8622). What survived correction is the narrower, still-true fact —
178+
* this predicate does not COVER MySQL — and that is exactly the fact a reader
179+
* needs and cannot get from a comment they might not read.
180+
*
181+
* ⛔ These assert `false`, and a `false` here is NOT a verdict that the text is
182+
* safe: it is the predicate being SILENT on a dialect it never learned. The
183+
* phrasing-independent answer is {@link declaresServerFault}, which is dialect-
184+
* blind by construction.
185+
*
186+
* ⛔ **If a future PR teaches the list MySQL's spellings, these go red — that is
187+
* the tripwire, not a broken test.** Do not delete the case to make it green.
188+
* Come back here, and to `DIALECT_LEAK_PHRASINGS`' note, and update both to say
189+
* what is then true. Extending the list is a behaviour change at three
190+
* boundaries (it moves what gets suppressed), and #8739 leaves it parked behind
191+
* an open product question: is MySQL a supported deployment target, or merely a
192+
* tested dialect? Same shape as `metadata-protocol`'s
193+
* `protocol.driver-text-disclosure.test.ts`, which pins its unmet dialects for
194+
* the same reason.
195+
*/
196+
describe('looksLikeInternalErrorLeak — dialects the list does NOT cover (#8739)', () => {
197+
it.each([
198+
// The tail PR #8737 keeps verbatim in the write-path log. Names an
199+
// IDENTIFIER on MySQL, as SQLite's and Postgres' spellings do — which is
200+
// why that PR's conclusion held even though its stated reason did not.
201+
['mysql unknown column', "Unknown column 'zzz_nonexistent_field' in 'field list'"],
202+
// The same condition the Postgres/SQLite limbs above DO catch.
203+
['mysql missing table', "Table 'crm.sys_metadata' doesn't exist"],
204+
// Value-bearing, and the reason "uncovered" is worth pinning: MySQL puts
205+
// a CALLER'S VALUE in this diagnostic. `isUniqueViolationError`
206+
// (`unique-violation.ts`) is the predicate that does recognise it; this
207+
// one does not, and the two answer different questions on purpose.
208+
['mysql duplicate entry', "Duplicate entry 'acme@example.com' for key 'idx_email_unique'"],
209+
])('is silent on %s — false here means UNCOVERED, never "safe"', (_label, message) => {
210+
expect(looksLikeInternalErrorLeak(message)).toBe(false);
211+
});
212+
213+
/**
214+
* The other half of the same measurement: the dialect is uncovered, but the
215+
* declaration channel is not. A boundary that also asks
216+
* {@link declaresServerFault} withholds the identical text.
217+
*/
218+
it('withholds the same uncovered text through the declaration channel', () => {
219+
const mysqlDump = { status: 500, code: 'DATABASE_ERROR', message: "Table 'crm.sys_metadata' doesn't exist" };
220+
expect(looksLikeInternalErrorLeak(mysqlDump.message)).toBe(false);
221+
expect(declaresServerFault(mysqlDump)).toBe(true);
222+
});
223+
});
224+
165225
/**
166226
* [#5811] The declaration half. `looksLikeInternalErrorLeak` asks whether a
167227
* message SOUNDS internal; this asks whether the producer SAID it was a server

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

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,11 @@
3636
export const INTERNAL_ERROR_MESSAGE = 'Internal server error';
3737

3838
/**
39-
* [#8132] The phrasings of the dialects this repo actually RUNS, each anchored
40-
* on the driver's own errmsg template rather than on its tail.
39+
* [#8132] The dialect phrasings this list COVERS — the SQLite family and
40+
* Postgres — each anchored on the driver's own errmsg template rather than on
41+
* its tail. Coverage, not a census of what this repo runs: see "What this list
42+
* covers, and what it does not" below, which is the load-bearing half for
43+
* anyone sizing a disclosure residual.
4144
*
4245
* The gap that forced these: the keyword set below caught SQLite's
4346
* `SQLITE_ERROR: no such table: sys_metadata` through the `sqlite_` limb, while
@@ -53,13 +56,39 @@ export const INTERNAL_ERROR_MESSAGE = 'Internal server error';
5356
* not — a quoted identifier, or the trailing colon of SQLite's template. The
5457
* negative cases in `error-leak.test.ts` pin that distinction.
5558
*
56-
* **Why the list stops here.** The module note above argues against growing a
57-
* driver taxonomy, and it is right that the list is unbounded *across dialects*
58-
* — MySQL/MSSQL/Oracle each phrase all of this differently and nobody here runs
59-
* them. These are not a census: they are the two engines `driver-sql`,
60-
* `driver-turso` and `driver-sqlite-wasm` actually reach. A dialect this repo
61-
* does not run gets no entry, and {@link declaresServerFault} remains the
62-
* answer that does not depend on phrasing at all.
59+
* **What this list covers, and what it does not.** The module note above argues
60+
* against growing a driver taxonomy, and that reason still holds on its own: a
61+
* phrasing list is unbounded *across dialects*, because every dialect spells
62+
* every one of these conditions its own way. So these entries are a COVERAGE
63+
* statement, not a census — they are the two spellings #8132 measured the gap
64+
* on, and {@link declaresServerFault} remains the answer that does not depend
65+
* on phrasing at all.
66+
*
67+
* ⚠️ **This list's silence is NOT evidence that a dialect is unreachable.**
68+
* Until #8739 this paragraph said "nobody here runs" MySQL/MSSQL/Oracle, and a
69+
* reviewer sizing a disclosure residual read it as one. It was false for MySQL,
70+
* measurably, on the same tree:
71+
*
72+
* - `driver-sql` branches on `mysql`/`mysql2` — the `isMysql` getter,
73+
* `withUtcSession`, and the `dialect === 'mysql'` arms of
74+
* `textMatchPredicate` / `likePatternPredicate`.
75+
* - CI stands up a live `mysql:8.0` service for the job named
76+
* `Temporal Conformance (live PG + MySQL)`, which IS a required check, and
77+
* its `OS_EXPECT_LIVE_DIALECT_MATRIX` flag turns a missing MySQL URL into a
78+
* named red rather than a quiet skip.
79+
* - Live MySQL 8.0.46 measurements produced merged driver fixes (#8621,
80+
* #8622), and `unique-violation.ts` — one file over — names sqlite /
81+
* postgres / mysql as the three dialect families `sql-driver.ts` recognises.
82+
*
83+
* Whether MySQL is a **supported deployment target** or merely a **tested
84+
* dialect** is an open product question (#8739); this file does not answer it,
85+
* and neither answer changes the rule a reader needs. What is true either way,
86+
* and is the only thing to carry away: on a MySQL deployment this predicate is
87+
* SILENT, not clearing. Its phrasings of these same conditions —
88+
* `Unknown column 'c' in 'field list'`, `Table 'app.t' doesn't exist`,
89+
* `Duplicate entry 'x' for key 'i'` — all return FALSE here, pinned in
90+
* `error-leak.test.ts`. Adding them would change what gets suppressed at three
91+
* boundaries, so it belongs to that decision, not to a comment.
6392
*
6493
* ⚠️ Related but NOT reusable: `relation-sub-object.ts` owns the same Postgres
6594
* sentence for two other questions (which column? / is this a sub-object?), and
@@ -94,7 +123,10 @@ const DIALECT_LEAK_PHRASINGS: readonly RegExp[] = [
94123
* (a message that *starts* as `SELECT`/`INSERT INTO`/`UPDATE`/`DELETE FROM` —
95124
* drivers prefix the offending SQL to their message), constraint-violation
96125
* dumps, which name physical tables and columns, and the
97-
* {@link DIALECT_LEAK_PHRASINGS} of the engines this repo ships.
126+
* {@link DIALECT_LEAK_PHRASINGS} the list covers — the SQLite family and
127+
* Postgres. A dialect outside that coverage (MySQL is reachable here, and is
128+
* not covered) makes this return FALSE without meaning the text is safe; read
129+
* {@link DIALECT_LEAK_PHRASINGS}' note before sizing anything on a `false`.
98130
*
99131
* Does NOT match ordinary business or validation messages, which is why the
100132
* statement forms are anchored with `startsWith` and the dialect phrasings on

0 commit comments

Comments
 (0)