Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions .changeset/error-leak-mysql-phrasings.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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 },
];
Expand DownExpand Up@@ -273,14 +288,17 @@ async function captureThrow(run: () => Promise<unknown>): Promise<any> {
// ---------------------------------------------------------------------------

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

Expand Down
39 changes: 28 additions & 11 deletions packages/rest/src/rest-unique-violation-dialects.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -99,8 +99,10 @@ function driverError(message: string, extra: Record<string, unknown> = {}): 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)',
Expand DownExpand Up@@ -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', () => {
Expand Down
Loading
Loading