From 05e85a759545c35b860f00b2d94ec70ccd9d26fc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 06:44:17 +0000 Subject: [PATCH] fix(types): teach the internal-leak predicate MySQL's three error templates (#8739) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `looksLikeInternalErrorLeak` covered the SQLite family and Postgres; on a MySQL deployment it returned false for every one of the same conditions — silent, not clearing. Under the maintainer's 2026-08-15 ruling on #8739 MySQL is a supported deployment target, so its driver text reaches these boundaries in production and its templates belong in the list. Adds three limbs, each anchored on MySQL's own errmsg template rather than a bare substring: ER_NO_SUCH_TABLE (1146), ER_BAD_FIELD_ERROR (1054) and ER_DUP_ENTRY (1062) — the last being the only one of the three whose text embeds a caller's value rather than an identifier. MySQL's ACL family is deliberately left uncovered and the reason is recorded: nothing here has raised one off a live server, and `Access denied` collides with this platform's own security prose. The four toBe(false) pins PR #8824 planted as a tripwire went red as designed and are rewritten, not deleted — same three measured messages, opposite verdict — and a second block keeps the original false-means-UNCOVERED shape pointed at MSSQL and Oracle so that distinction keeps a live subject. Two cross-package measurements of this predicate moved with it: rest's #6250 pin (rewritten to assert the invariant it was always for — the 409 is independent of the leak verdict, which is now demonstrated more strongly than before) and metadata-protocol's dialect matrix row and count. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn --- .changeset/error-leak-mysql-phrasings.md | 85 ++++++++++ .../protocol.driver-text-disclosure.test.ts | 38 +++-- .../rest-unique-violation-dialects.test.ts | 39 +++-- packages/types/src/error-leak.test.ts | 153 +++++++++++++----- packages/types/src/error-leak.ts | 119 +++++++++++--- 5 files changed, 350 insertions(+), 84 deletions(-) create mode 100644 .changeset/error-leak-mysql-phrasings.md diff --git a/.changeset/error-leak-mysql-phrasings.md b/.changeset/error-leak-mysql-phrasings.md new file mode 100644 index 0000000000..248dc57f10 --- /dev/null +++ b/.changeset/error-leak-mysql-phrasings.md @@ -0,0 +1,85 @@ +--- +"@objectstack/types": patch +--- + +fix(types): teach the internal-leak predicate MySQL's three error templates (#8739) + +`looksLikeInternalErrorLeak` decides whether a message is a driver dump that +must not reach an API client. It is applied at three HTTP boundaries +(`@objectstack/rest`'s `mapDataError`, `@objectstack/runtime`'s +dispatcher-plugin and endpoint-executor, the hono adapter) and by +`@objectstack/objectql`'s log redactor. Its dialect list covered the SQLite +family and Postgres; on a MySQL deployment it returned `false` for every one of +these conditions — **silent, not clearing**. + +Under the maintainer's 2026-08-15 ruling on #8739, **MySQL is a supported +deployment target**, not merely a tested dialect — the answer already implied by +what is published (`OS_DATABASE_DRIVER=mysql` as a documented deployment knob, +`MysqlConfig` as authorable datasource config, per-field MySQL DDL in +`types.mdx`) and by a required CI check that stands up a live `mysql:8.0`. A +supported target's driver text reaches those boundaries in production, so its +templates belong in the list. + +**Now recognised** — one per condition the other two dialects were already +covered for, each anchored on MySQL's own errmsg template rather than on a bare +substring: + +- `Table 'app.t' doesn't exist` (ER_NO_SUCH_TABLE 1146). MySQL's contracted + spelling quotes `db.table` as one identifier, so the Postgres + `relation "t" does not exist` limb could never reach it. +- `Unknown column 'c' in 'field list'` (ER_BAD_FIELD_ERROR 1054). Both quoted + parts are required; the second is MySQL's clause name (`field list`, + `where clause`, `order clause`, `on clause`), and it is what distinguishes the + driver's template from a sentence that merely calls a column unknown. +- `Duplicate entry 'x' for key 'i'` (ER_DUP_ENTRY 1062). The `for key` tail plus + a quoted index is the anchor. This is the one MySQL template whose text embeds + a **caller's value** rather than an identifier — SQLite's + `UNIQUE constraint failed: t.c` and Postgres' `violates unique constraint "…"` + both name only an index — which is why closing this gap was worth a behaviour + change rather than another comment. + +**Deliberately still NOT recognised**, so the boundary of the change is on the +record rather than inferred: + +- **MySQL's ACL family** — `Access denied for user 'u'@'h' to database 'd'` + (1044), `SELECT command denied to user … for table 't'` (1142) — the + counterpart of the Postgres `permission denied for table` limb. Nothing in + this repo has raised one off a live server, and the standing rule in this + neighbourhood (`unique-violation.ts`) is that a dialect's spelling is added + once it has been MEASURED off a thrown error, never from a reading of the + manual. `Access denied` also collides with this platform's own security prose + (`[Security] Access denied: …`), so a guessed pattern here would over-match — + and over-matching suppresses diagnostics an operator needs. +- **MSSQL and Oracle** — `Invalid object name 'sys_metadata'.`, + `ORA-00942: table or view does not exist` still return `false`. +- **Prose that shares the keywords without the driver's anchoring** — an import + summary saying `duplicate entry in the uploaded file`, a mapping message + saying `Unknown column in the uploaded CSV header`, `The table you selected + does not exist`. Pinned as negative cases, because a phrasing list that says + "leak" too often replaces real answers with `Internal server error`. + +**The `false`-means-UNCOVERED rule survives the change and keeps a live +subject.** A `false` here has never meant the text is safe, only that the +predicate never learned that dialect — the reading a reviewer on PR #8737 got +wrong while sizing a disclosure residual, which is what produced this card. The +four `toBe(false)` pins PR #8824 planted as a tripwire for this exact moment +went red as designed and are rewritten, not deleted: the same three measured +messages now assert `true`, so a future change that silently drops MySQL +coverage fails there, and a second block keeps the original `false`-means- +uncovered shape pointed at MSSQL and Oracle. `declaresServerFault` remains the +phrasing-independent answer. + +**No status mapping moves.** `@objectstack/rest` answers the 409 conflict +question with `isUniqueViolationError`, above and independently of this +predicate (#6250), so a MySQL duplicate-entry error is still `409 +UNIQUE_VIOLATION` and a MySQL unknown-column error is still `400 INVALID_FIELD` +— both decided before the leak branch is reached. The log redactor is unchanged +too: a bare MySQL diagnostic carries no knex ` - ` separator, so there is no +statement to cut. Measured across the predicate's full consumer set — types, +objectql, rest, runtime, metadata-protocol, hono, service-package, +service-analytics — the only verdicts that moved are the two that measure this +predicate directly. + +No live MySQL deployment leaking through these boundaries was measured; this +closes a gap in what the boundary recognises, and the card is explicit that no +leak was demonstrated. diff --git a/packages/metadata-protocol/src/protocol.driver-text-disclosure.test.ts b/packages/metadata-protocol/src/protocol.driver-text-disclosure.test.ts index 95fa76d2b9..0c823ee32b 100644 --- a/packages/metadata-protocol/src/protocol.driver-text-disclosure.test.ts +++ b/packages/metadata-protocol/src/protocol.driver-text-disclosure.test.ts @@ -31,9 +31,12 @@ * ## Why this file does NOT test a phrasing heuristic * * Three downstream boundaries run `looksLikeInternalErrorLeak` — a heuristic - * over the message. #8132 measured its hole for Postgres and #8263 taught it - * the two dialects it COVERS. That is an interim by construction: a - * phrasing test can only ever know the dialects someone has met. + * over the message. #8132 measured its hole for Postgres, #8263 taught it the + * two dialects it then COVERED, and #8739 added a third (MySQL, under the + * 2026-08-15 supported-target ruling). That is an interim by construction: a + * phrasing test can only ever know the dialects someone has met, and the count + * moving from two to three without moving a single case in this file is the + * cleanest available demonstration of why. * * So the dialect matrix below deliberately includes engines the predicate does * NOT recognise, and **asserts that it does not** before asserting the text is @@ -106,14 +109,26 @@ import { ObjectStackProtocolImplementation } from './protocol.js'; * each, measured against the shipping predicate in the first test below rather * than asserted from memory. * - * The three `false` rows are the reason this card is not "add the phrasing": - * MySQL, MSSQL and Oracle each say it differently again, and the list of + * The remaining `false` rows are the reason this card is not "add the + * phrasing": MSSQL and Oracle each say it differently again, and the list of * dialects this predicate does not cover is unbounded. + * + * ⚠️ **The MySQL row moved, and the move is the argument, not a counter-example + * to it.** It read `false` until #8739, when the maintainer's 2026-08-15 ruling + * made MySQL a supported deployment target and the shared list learned its three + * templates. Nothing in THIS file changed to accommodate that — every withhold + * below passed before the flip and passes after, because option C withholds by + * DECLARATION and never asks the predicate anything. That is exactly what a + * phrasing-independent producer is supposed to look like when the phrasing list + * moves underneath it. The row is updated here because this array claims to be + * a MEASUREMENT of the shared predicate; leaving a stale `false` would make it + * a memory, which is the defect #8739 was filed about. */ const DIALECTS: ReadonlyArray<{ engine: string; text: string; knownToPredicate: boolean }> = [ { engine: 'sqlite', text: 'SQLITE_ERROR: no such table: sys_metadata', knownToPredicate: true }, { engine: 'postgres', text: 'relation "sys_metadata" does not exist', knownToPredicate: true }, - { engine: 'mysql', text: "Table 'crm.sys_metadata' doesn't exist", knownToPredicate: false }, + // [#8739] Covered since the 2026-08-15 ruling — ER_NO_SUCH_TABLE's template. + { engine: 'mysql', text: "Table 'crm.sys_metadata' doesn't exist", knownToPredicate: true }, { engine: 'mssql', text: "Invalid object name 'sys_metadata'.", knownToPredicate: false }, { engine: 'oracle', text: 'ORA-00942: table or view does not exist', knownToPredicate: false }, ]; @@ -273,14 +288,17 @@ async function captureThrow(run: () => Promise): Promise { // --------------------------------------------------------------------------- describe('[#8136] the shared leak heuristic is dialect-bounded, which is why the cure is at the producer', () => { - it('recognises the two engines the predicate covers, and none of the three it does not', () => { + it('recognises the three engines the predicate covers, and neither of the two it does not', () => { for (const { engine, text, knownToPredicate } of DIALECTS) { expect(looksLikeInternalErrorLeak(text), `${engine}: ${text}`).toBe(knownToPredicate); } // Stated positively so the asymmetry cannot be read as an accident: - // three of five phrasings of ONE condition are invisible to every - // boundary that runs the predicate. - expect(DIALECTS.filter((d) => !d.knownToPredicate)).toHaveLength(3); + // two of five phrasings of ONE condition are still invisible to every + // boundary that runs the predicate. [#8739] This count was 3 until MySQL + // was covered. It is a live measurement, not a constant — and the fact + // that it can move while every withhold case below stays green is the + // reason this file tests a producer rather than a phrasing list. + expect(DIALECTS.filter((d) => !d.knownToPredicate)).toHaveLength(2); }); }); diff --git a/packages/rest/src/rest-unique-violation-dialects.test.ts b/packages/rest/src/rest-unique-violation-dialects.test.ts index 8ecbcdfb1b..21055c9ef4 100644 --- a/packages/rest/src/rest-unique-violation-dialects.test.ts +++ b/packages/rest/src/rest-unique-violation-dialects.test.ts @@ -99,8 +99,10 @@ function driverError(message: string, extra: Record = {}): Erro const DIALECT_SAMPLES: readonly DialectSample[] = [ // ---------------------------------------------------------------- MySQL // The reported defect. Before #6250 this was `500 INTERNAL_ERROR`: the - // message matches no limb of `looksLikeInternalErrorLeak`, so it never - // reached the 409 branch nested inside it. + // message matched no limb of `looksLikeInternalErrorLeak` AS IT THEN STOOD, + // so it never reached the 409 branch nested inside it. ⚠️ Read that as + // history, not as a present-tense fact about the predicate — #8739 taught it + // MySQL's templates, and the 409 stopped depending on the answer at #6250. { dialect: 'mysql', label: 'ER_DUP_ENTRY — bare driver message (the #6250 report)', @@ -597,21 +599,36 @@ describe('#7821 face 3 — the conflicting field on the wire', () => { }); /** - * The leak classifier was deliberately left byte-identical (#6250's security - * flag): the fix hoists the conflict question OUT of it rather than widening - * its criteria, so nothing else it guards can be reclassified as safe-to-expose - * as a side effect. These pin the two halves of that. + * #6250's security flag, restated as the invariant it always was: the CONFLICT + * question is answered independently of the LEAK question. The fix hoisted the + * conflict test OUT of the leak classifier's true-branch rather than widening + * the classifier's criteria, so nothing the classifier guards could be + * reclassified as safe-to-expose as a side effect. + * + * ⚠️ #8739 later widened that classifier on purpose — under the maintainer's + * 2026-08-15 ruling that MySQL is a supported deployment target, it now covers + * `Duplicate entry 'x' for key 'i'` along with two other MySQL templates. The + * first case below was written as "the classifier still says false" and has + * been rewritten to assert the invariant directly, because that is what it was + * always for. The rewrite makes it a STRONGER demonstration than the original: + * the message now IS classified as a leak, and the 409 is returned anyway — + * which can only be true if the conflict branch runs above and independently of + * the leak branch, exactly as #6250 arranged. ⛔ Do not "restore" the `false`; + * the classifier's MySQL coverage is `error-leak.test.ts`' pin, not this file's. */ -describe('#6250 — the fix did not widen the internal-leak classifier', () => { - it('a MySQL conflict is still not classified as a leak — it no longer has to be', () => { +describe('#6250 — the conflict verdict does not depend on the leak classifier', () => { + it('a MySQL conflict is a 409 even though the classifier now calls the text a leak', () => { const err = driverError( `ER_DUP_ENTRY: Duplicate entry '${OFFENDING_VALUE}' for key '${OFFENDING_INDEX}'`, { code: 'ER_DUP_ENTRY', errno: 1062 }, ); - // Unchanged: the heuristic still does not recognise this phrasing… - expect(looksLikeInternalErrorLeak(err.message)).toBe(false); - // …and that no longer decides whether the conflict is seen. + // [#8739] The classifier covers MySQL's template since the ruling. Before + // it did, this was `false` — and the 409 below was already independent + // of which way it answered, which is the whole point. + expect(looksLikeInternalErrorLeak(err.message)).toBe(true); + // The verdict that matters, unchanged across both eras of the line above. expect(mapDataError(err, 'sys_user').status).toBe(409); + expect(mapDataError(err, 'sys_user').body.code).toBe('UNIQUE_VIOLATION'); }); it('driver text that is a leak but NOT a conflict still gets the sanitised 500', () => { diff --git a/packages/types/src/error-leak.test.ts b/packages/types/src/error-leak.test.ts index dce6f7617d..a3f492a70a 100644 --- a/packages/types/src/error-leak.test.ts +++ b/packages/types/src/error-leak.test.ts @@ -96,13 +96,14 @@ describe('looksLikeInternalErrorLeak', () => { * FALSE and shipped a physical table name from every boundary that applies the * predicate. * - * Scope is deliberately the two dialects the list COVERS (SQLite/libsql and - * Postgres, via `driver-sql`), not a census of every dialect's spelling — the - * unbounded-list trap the module note argues against. ⚠️ Covered is not the - * same as reachable: #8739 measured that MySQL runs here (a live `mysql:8.0` - * behind a required check) while this list does not cover it, and the - * "uncovered dialects, uncovered on purpose" block below pins that gap so the - * scope sentence cannot quietly go false again. + * Scope is deliberately the dialects the list COVERS — SQLite/libsql and + * Postgres here, MySQL in the #8739 block below — not a census of every + * dialect's spelling, which is the unbounded-list trap the module note argues + * against. ⚠️ Covered is still not the same as reachable, and the distinction + * survived the thing that motivated it: #8739 measured MySQL as reachable while + * uncovered, that gap is now closed, and the "dialects the list does NOT cover" + * block below keeps the same tripwire pointed at MSSQL and Oracle so the scope + * sentence cannot quietly go false again. * * The negative half is the load-bearing half. A bare `includes('does not * exist')` would have matched "user does not exist" and started replacing @@ -167,45 +168,117 @@ describe('looksLikeInternalErrorLeak — shipped-dialect phrasings (#8132)', () }); /** - * [#8739] The uncovered dialect, pinned as a MEASUREMENT rather than a - * paragraph. + * [#8739] MySQL, now COVERED — and the same tripwire, re-pointed. * - * The module used to say nobody here runs MySQL, and a reviewer sizing a - * disclosure residual on PR #8737 quoted it in good faith. The claim was false: - * `driver-sql` branches on `mysql`/`mysql2`, CI stands up a live `mysql:8.0` - * for a required check, and live MySQL 8.0.46 measurements landed driver fixes - * (#8621, #8622). What survived correction is the narrower, still-true fact — - * this predicate does not COVER MySQL — and that is exactly the fact a reader - * needs and cannot get from a comment they might not read. + * ## What these cases were, and why they are not deleted * - * ⛔ These assert `false`, and a `false` here is NOT a verdict that the text is - * safe: it is the predicate being SILENT on a dialect it never learned. The - * phrasing-independent answer is {@link declaresServerFault}, which is dialect- - * blind by construction. + * These began as `toBe(false)` pins. The module used to say nobody here runs + * MySQL, and a reviewer sizing a disclosure residual on PR #8737 quoted it in + * good faith; the claim was false (`driver-sql` branches on `mysql`/`mysql2`, + * CI stands up a live `mysql:8.0` for a required check, live MySQL 8.0.46 + * measurements landed driver fixes #8621/#8622). PR #8824 corrected the + * sentence and pinned the narrower, then-true fact — the predicate did not + * COVER MySQL — as a deliberate tripwire for the decision that was still open. + * + * **That decision has been taken.** Maintainer ruling of 2026-08-15 on #8739: + * MySQL is a SUPPORTED DEPLOYMENT TARGET, not merely a tested dialect — the + * only answer consistent with what the docs already publish + * (`OS_DATABASE_DRIVER=mysql`, `MysqlConfig`, per-field MySQL DDL). So the + * tripwire fired as designed, and these cases are REWRITTEN rather than + * removed: same three measured messages, opposite expected verdict. + * + * ⛔ **They must stay equally capable of going red.** Their whole value is that + * a future change which silently drops MySQL coverage — deleting a limb, + * "simplifying" a pattern, loosening an anchor until it stops matching the + * driver's template — fails HERE, on the three templates that were measured off + * real MySQL text, rather than in a deployment. Do not soften them into + * `expect(...).toBeDefined()` or fold them into the #8132 block, where the + * reason they exist would be lost. * - * ⛔ **If a future PR teaches the list MySQL's spellings, these go red — that is - * the tripwire, not a broken test.** Do not delete the case to make it green. - * Come back here, and to `DIALECT_LEAK_PHRASINGS`' note, and update both to say - * what is then true. Extending the list is a behaviour change at three - * boundaries (it moves what gets suppressed), and #8739 leaves it parked behind - * an open product question: is MySQL a supported deployment target, or merely a - * tested dialect? Same shape as `metadata-protocol`'s - * `protocol.driver-text-disclosure.test.ts`, which pins its unmet dialects for - * the same reason. + * ⛔ **And the `false`-means-UNCOVERED lesson is NOT retired with them.** It was + * never about MySQL specifically: a `false` from this predicate is the + * predicate being SILENT on a dialect it never learned, and is never a verdict + * that the text is safe. The second block below keeps that pinned on MSSQL and + * Oracle, which are uncovered today, so the distinction that stopped PR #8737's + * near-miss from repeating keeps a live subject. The phrasing-independent + * answer remains {@link declaresServerFault}, dialect-blind by construction — + * same shape as `metadata-protocol`'s `protocol.driver-text-disclosure.test.ts`, + * which withholds by DECLARATION and therefore needs no dialect list at all. */ -describe('looksLikeInternalErrorLeak — dialects the list does NOT cover (#8739)', () => { +describe('looksLikeInternalErrorLeak — MySQL, covered under the #8739 ruling', () => { it.each([ // The tail PR #8737 keeps verbatim in the write-path log. Names an // IDENTIFIER on MySQL, as SQLite's and Postgres' spellings do — which is // why that PR's conclusion held even though its stated reason did not. ['mysql unknown column', "Unknown column 'zzz_nonexistent_field' in 'field list'"], - // The same condition the Postgres/SQLite limbs above DO catch. + // The clause name is not always `field list`; the pattern requires the + // second quoted part but not any particular word in it. This spelling is + // the one `packages/spec`'s migration registry records for MySQL. + ['mysql unknown column, where clause', "Unknown column 'stage' in 'where clause'"], + // The same condition the Postgres/SQLite limbs above catch, in MySQL's + // own contracted spelling — `doesn't`, and `db.table` as ONE identifier. ['mysql missing table', "Table 'crm.sys_metadata' doesn't exist"], - // Value-bearing, and the reason "uncovered" is worth pinning: MySQL puts - // a CALLER'S VALUE in this diagnostic. `isUniqueViolationError` - // (`unique-violation.ts`) is the predicate that does recognise it; this - // one does not, and the two answer different questions on purpose. + // Value-bearing, and the reason covering MySQL mattered rather than + // merely documenting the gap: MySQL puts a CALLER'S VALUE in this + // diagnostic where SQLite and Postgres put an identifier. ['mysql duplicate entry', "Duplicate entry 'acme@example.com' for key 'idx_email_unique'"], + // MySQL 8 spells the key `table.column`; same template, and the value + // half may itself contain a quote, which the pattern tolerates. + ['mysql duplicate entry, qualified key', "Duplicate entry 'O'Brien' for key 'crm_account.email'"], + // knex prefixes the statement. Already caught by the `insert into ` limb + // before #8739 — pinned so the two routes to `true` stay distinguishable + // if one of them is ever removed. + [ + 'mysql duplicate entry behind a knex statement prefix', + "insert into `crm_account` (`email`) values ('acme@example.com') - Duplicate entry 'acme@example.com' for key 'crm_account.email'", + ], + ])('covers %s', (_label, message) => { + expect(looksLikeInternalErrorLeak(message)).toBe(true); + }); + + /** + * ⛔ The false-positive guard for the three MySQL limbs, in the same spirit + * as #8132's. Each of these contains the KEYWORDS of a MySQL template + * without the driver's anchoring — the quoted identifier, the clause name, + * the `for key` tail — and each is a sentence a product surface may + * legitimately write. If someone relaxes a MySQL anchor to a bare + * `includes(...)`, these go red before a deployment starts answering + * "Internal server error" to real questions. + */ + it.each([ + ['a dedup rule speaking plainly', 'Duplicate entry rejected by the deduplication rule'], + ['an import summary', 'Skipped 3 rows: duplicate entry in the uploaded file'], + ['a mapping message about an unknown column', 'Unknown column in the uploaded CSV header'], + ['an unquoted mapping message', 'Unknown column stage in your mapping'], + ['prose about a missing table, unquoted', 'The table you selected does not exist'], + ['a business message that merely quotes a name', "'crm.sys_metadata' is not available in this environment"], + ])('leaves %s alone', (_label, message) => { + expect(looksLikeInternalErrorLeak(message)).toBe(false); + }); +}); + +/** + * [#8739] The dialects the list still does NOT cover — the surviving half of + * the tripwire above, and the reason the `false`-means-UNCOVERED rule outlives + * any one dialect. + * + * ⛔ These assert `false`, and a `false` here is NOT a verdict that the text is + * safe: it is the predicate being SILENT on a dialect it never learned. That is + * the exact reading PR #8737 got wrong — it survived on unrelated grounds — and + * the distinction needs a live subject, not a retired one, which is what these + * two provide now that MySQL is covered. + * + * ⛔ If a future PR teaches the list one of these, do NOT delete the case: flip + * it, cite the reason the way the MySQL block above cites its ruling, and check + * that {@link DIALECT_LEAK_PHRASINGS}' note still says what is then true. Both + * texts are the ones `metadata-protocol`'s `protocol.driver-text-disclosure.test.ts` + * carries in its dialect matrix, deliberately, so the two files measure the + * same predicate on the same strings. + */ +describe('looksLikeInternalErrorLeak — dialects the list does NOT cover (#8739)', () => { + it.each([ + ['mssql invalid object name', "Invalid object name 'sys_metadata'."], + ['oracle missing table or view', 'ORA-00942: table or view does not exist'], ])('is silent on %s — false here means UNCOVERED, never "safe"', (_label, message) => { expect(looksLikeInternalErrorLeak(message)).toBe(false); }); @@ -213,12 +286,14 @@ describe('looksLikeInternalErrorLeak — dialects the list does NOT cover (#8739 /** * The other half of the same measurement: the dialect is uncovered, but the * declaration channel is not. A boundary that also asks - * {@link declaresServerFault} withholds the identical text. + * {@link declaresServerFault} withholds the identical text — which is why + * closing the MySQL gap was an improvement to defence in depth and never + * the thing standing between an uncovered dialect and disclosure. */ it('withholds the same uncovered text through the declaration channel', () => { - const mysqlDump = { status: 500, code: 'DATABASE_ERROR', message: "Table 'crm.sys_metadata' doesn't exist" }; - expect(looksLikeInternalErrorLeak(mysqlDump.message)).toBe(false); - expect(declaresServerFault(mysqlDump)).toBe(true); + const mssqlDump = { status: 500, code: 'DATABASE_ERROR', message: "Invalid object name 'sys_metadata'." }; + expect(looksLikeInternalErrorLeak(mssqlDump.message)).toBe(false); + expect(declaresServerFault(mssqlDump)).toBe(true); }); }); diff --git a/packages/types/src/error-leak.ts b/packages/types/src/error-leak.ts index f813509823..4e2d8a49ab 100644 --- a/packages/types/src/error-leak.ts +++ b/packages/types/src/error-leak.ts @@ -36,11 +36,11 @@ export const INTERNAL_ERROR_MESSAGE = 'Internal server error'; /** - * [#8132] The dialect phrasings this list COVERS — the SQLite family and - * Postgres — each anchored on the driver's own errmsg template rather than on - * its tail. Coverage, not a census of what this repo runs: see "What this list - * covers, and what it does not" below, which is the load-bearing half for - * anyone sizing a disclosure residual. + * [#8132, #8739] The dialect phrasings this list COVERS — the SQLite family, + * Postgres and MySQL/MariaDB — each anchored on the driver's own errmsg + * template rather than on its tail. Coverage, not a census of what this repo + * runs: see "What this list covers, and what it does not" below, which is the + * load-bearing half for anyone sizing a disclosure residual. * * The gap that forced these: the keyword set below caught SQLite's * `SQLITE_ERROR: no such table: sys_metadata` through the `sqlite_` limb, while @@ -60,13 +60,46 @@ export const INTERNAL_ERROR_MESSAGE = 'Internal server error'; * against growing a driver taxonomy, and that reason still holds on its own: a * phrasing list is unbounded *across dialects*, because every dialect spells * every one of these conditions its own way. So these entries are a COVERAGE - * statement, not a census — they are the two spellings #8132 measured the gap - * on, and {@link declaresServerFault} remains the answer that does not depend - * on phrasing at all. + * statement, not a census — the two spellings #8132 measured the gap on, plus + * the three MySQL templates #8739 added — and {@link declaresServerFault} + * remains the answer that does not depend on phrasing at all. * - * ⚠️ **This list's silence is NOT evidence that a dialect is unreachable.** - * Until #8739 this paragraph said "nobody here runs" MySQL/MSSQL/Oracle, and a - * reviewer sizing a disclosure residual read it as one. It was false for MySQL, + * **Covered as of #8739: MySQL/MariaDB.** Under the maintainer's 2026-08-15 + * ruling on #8739, MySQL is a SUPPORTED DEPLOYMENT TARGET, not merely a tested + * dialect — the answer the published surface already implied + * (`OS_DATABASE_DRIVER=mysql` is a documented deployment knob, `MysqlConfig` is + * authorable datasource config, `types.mdx` specifies per-field MySQL DDL) and + * the one CI's required live-MySQL check already behaves as if. A supported + * target's driver text reaches these boundaries in production, so its + * templates belong here. Three are covered, one per condition the other two + * dialects are already covered for: + * + * - `Table 'app.t' doesn't exist` (ER_NO_SUCH_TABLE 1146) — the missing-object + * condition SQLite spells `no such table:` and Postgres spells + * `relation "t" does not exist`. + * - `Unknown column 'c' in 'field list'` (ER_BAD_FIELD_ERROR 1054) — the same + * condition for a column. The clause name varies (`field list`, + * `where clause`, `order clause`, `on clause`) and is REQUIRED by the + * pattern; it is what separates the driver's template from prose. + * - `Duplicate entry 'x' for key 'i'` (ER_DUP_ENTRY 1062) — the + * unique-violation condition the `constraint failed` / `unique constraint` + * keyword limbs already catch for SQLite and Postgres and cannot catch here, + * because MySQL's spelling shares no word with either. It is also the ONLY + * one of the three whose text embeds a CALLER'S VALUE rather than an + * identifier, which is what made the pre-#8739 gap worth closing rather than + * documenting. + * + * ⛔ Adding this limb does NOT re-open #6250's decision one package over. + * `@objectstack/rest` answers the 409 conflict question with + * `isUniqueViolationError` (`unique-violation.ts`), ABOVE and independently of + * this predicate, precisely so a disclosure rule never decides a status. That + * ordering is what keeps the two unentangled now that both recognise the same + * MySQL sentence; `rest-unique-violation-dialects.test.ts` pins it. + * + * ⚠️ **This list's silence is STILL NOT evidence that a dialect is + * unreachable, and MySQL is why the warning is worded that way.** Until #8739 + * this paragraph said "nobody here runs" MySQL/MSSQL/Oracle, and a reviewer + * sizing a disclosure residual read it as one. It was false for MySQL, * measurably, on the same tree: * * - `driver-sql` branches on `mysql`/`mysql2` — the `isMysql` getter, @@ -80,15 +113,30 @@ export const INTERNAL_ERROR_MESSAGE = 'Internal server error'; * #8622), and `unique-violation.ts` — one file over — names sqlite / * postgres / mysql as the three dialect families `sql-driver.ts` recognises. * - * Whether MySQL is a **supported deployment target** or merely a **tested - * dialect** is an open product question (#8739); this file does not answer it, - * and neither answer changes the rule a reader needs. What is true either way, - * and is the only thing to carry away: on a MySQL deployment this predicate is - * SILENT, not clearing. Its phrasings of these same conditions — - * `Unknown column 'c' in 'field list'`, `Table 'app.t' doesn't exist`, - * `Duplicate entry 'x' for key 'i'` — all return FALSE here, pinned in - * `error-leak.test.ts`. Adding them would change what gets suppressed at three - * boundaries, so it belongs to that decision, not to a comment. + * The rule that outlives any particular dialect: **a `false` from this + * predicate means UNCOVERED, never "safe"**, and the reachability of an + * uncovered dialect is a separate question this file cannot answer. MSSQL and + * Oracle are uncovered today — `Invalid object name 'sys_metadata'.`, + * `ORA-00942: table or view does not exist` both return FALSE — and that is a + * statement about this list, not about them; `error-leak.test.ts` pins those + * two as the standing example so the distinction keeps a live subject. + * {@link declaresServerFault} is the phrasing-independent answer, and + * `metadata-protocol`'s `protocol.driver-text-disclosure.test.ts` is the worked + * demonstration that a producer which withholds by DECLARATION needs no dialect + * list at all. + * + * ⛔ **What is deliberately NOT added here, and why.** MySQL's ACL family + * (`Access denied for user 'u'@'h' to database 'd'`, ER_DBACCESS_DENIED_ERROR + * 1044; `SELECT command denied to user … for table 't'`, + * ER_TABLEACCESS_DENIED_ERROR 1142) is the counterpart of the Postgres + * `permission denied for table` limb above and is NOT covered: nothing in this + * repo has raised one off a live server, and `unique-violation.ts`' standing + * rule for this neighbourhood is that a dialect's spelling is added when it has + * been MEASURED off a thrown error, never on a plausible reading of the + * dialect's manual. `Access denied` also collides with this platform's own + * security prose (`[Security] Access denied: …`, pinned as a negative case), so + * a guessed pattern here is the over-match direction, which suppresses + * diagnostics an operator needs. Measure one, then add it. * * ⚠️ 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 @@ -113,6 +161,28 @@ const DIALECT_LEAK_PHRASINGS: readonly RegExp[] = [ // 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, + // [#8739] MySQL/MariaDB ER_NO_SUCH_TABLE (1146): `Table 'app.t' doesn't + // exist`. Its own template, not a spelling of the Postgres one — MySQL + // contracts the verb and quotes `db.table` as a single identifier — so the + // `relation|column … does not exist` limb above cannot reach it. The quotes + // are required for the same reason they are there: the driver always emits + // them and prose about a table usually does not. + /\btable\s+["'`][^"'`]+["'`]\s+doesn't exist/i, + // [#8739] MySQL/MariaDB ER_BAD_FIELD_ERROR (1054): `Unknown column 'c' in + // 'field list'`. BOTH quoted parts are required. The second is MySQL's clause + // name — `field list`, `where clause`, `order clause`, `on clause` — and it + // is the half that makes this the driver's template rather than a sentence + // that merely calls a column unknown, which an import or mapping feature has + // every right to say. + /\bunknown column\s+["'`][^"'`]+["'`]\s+in\s+["'`][^"'`]+["'`]/i, + // [#8739] MySQL/MariaDB ER_DUP_ENTRY (1062): `Duplicate entry + // 'acme@example.com' for key 'crm_account.email'`. `for key` + a quoted index + // is the anchor; the VALUE half is matched loosely and lazily because it is + // the caller's own text and MySQL does not escape a quote inside it + // (`Duplicate entry 'O'Brien' for key 'i'` is a real shape). A bare + // `duplicate entry` with no `for key '…'` tail is not this template and is + // left alone. + /\bduplicate entry\s+["'`].*?["'`]\s+for key\s+["'`][^"'`]+["'`]/i, ]; /** @@ -123,10 +193,11 @@ const DIALECT_LEAK_PHRASINGS: readonly RegExp[] = [ * (a message that *starts* as `SELECT`/`INSERT INTO`/`UPDATE`/`DELETE FROM` — * drivers prefix the offending SQL to their message), constraint-violation * dumps, which name physical tables and columns, and the - * {@link DIALECT_LEAK_PHRASINGS} the list covers — the SQLite family and - * Postgres. A dialect outside that coverage (MySQL is reachable here, and is - * not covered) makes this return FALSE without meaning the text is safe; read - * {@link DIALECT_LEAK_PHRASINGS}' note before sizing anything on a `false`. + * {@link DIALECT_LEAK_PHRASINGS} the list covers — the SQLite family, Postgres + * and, since #8739, MySQL/MariaDB. A dialect outside that coverage (MSSQL and + * Oracle are the standing examples) makes this return FALSE without meaning the + * text is safe; read {@link DIALECT_LEAK_PHRASINGS}' note before sizing + * anything on a `false`. * * Does NOT match ordinary business or validation messages, which is why the * statement forms are anchored with `startsWith` and the dialect phrasings on