Skip to content

fix(types): teach the internal-leak predicate MySQL's three error templates (#8739) - #9030

Merged
qq9340100 merged 1 commit into
mainfrom
claude/issue-8739-mysql-leak-phrasings
Aug 16, 2026
Merged

fix(types): teach the internal-leak predicate MySQL's three error templates (#8739)#9030
qq9340100 merged 1 commit into
mainfrom
claude/issue-8739-mysql-leak-phrasings

Conversation

@qq9340100

Copy link
Copy Markdown
Collaborator

Part of #8739 — ruling item 1 only. Deliberately not a closing keyword: three of the ruling's four consequences live elsewhere (see "What this PR does not do"), so the card should survive this merge.

The ruling

Maintainer, 2026-08-15, comment 5302926756, provenance quoted verbatim and untranslated:

接受你的所有建议。

Option A — MySQL is a supported deployment target, not merely a tested dialect. The reasoning the dev report and the re-grade reached independently: the docs are the promise already madeOS_DATABASE_DRIVER=mysql is a published deployment knob, MysqlConfig is authorable datasource config, types.mdx specifies per-field MySQL DDL, and MySQL-specific defects are accepted and fixed rather than closed as unsupported.

Item 1 of the four consequences: teach DIALECT_LEAK_PHRASINGS MySQL's phrasings.

What changed

looksLikeInternalErrorLeak decides whether a message is a driver dump that must not reach an API client. It is applied at three HTTP boundaries (rest's mapDataError, runtime's dispatcher-plugin and endpoint-executor, the hono adapter) and gates objectql's log redactor. Its dialect list covered the SQLite family and Postgres; on a MySQL deployment it answered false to every one of the same conditions — silent, not clearing.

Three limbs added, one per condition the other two dialects were already covered for, each anchored on MySQL's own errmsg template rather than a bare substring:

conditionMySQL templatecode
missing tableTable 'app.t' doesn't existER_NO_SUCH_TABLE 1146
missing columnUnknown column 'c' in 'field list'ER_BAD_FIELD_ERROR 1054
unique violationDuplicate entry 'x' for key 'i'ER_DUP_ENTRY 1062

Anchoring detail, since over-matching is the expensive direction here: the unknown-column pattern requires both quoted parts — the second is MySQL's clause name (field list, where clause, order clause, on clause) and is what separates the driver's template from a sentence that merely calls a column unknown. The duplicate-entry pattern anchors on the for key tail plus a quoted index, and matches the value half loosely because it is the caller's own text and MySQL does not escape a quote inside it.

Deliberately still NOT recognised, on the record rather than inferred:

  • MySQL's ACL familyAccess 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 here 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 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: …, a pinned negative case), so a guessed pattern would over-match — and over-matching suppresses diagnostics an operator needs.
  • MSSQL and Oracle — still false.
  • Prose sharing 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. All pinned as new negative cases.

The four pins went red by design, and are rewritten rather than deleted

PR #8824 planted four toBe(false) pins as a deliberate tripwire for exactly this decision. They fired. Measured first, before any test was touched — source edited alone, landed pins run against it:

Test Files 1 failed | 11 passed (12)
Tests 4 failed | 333 passed (337)
× is silent on mysql unknown column — false here means UNCOVERED, never "safe"
× is silent on mysql missing table — false here means UNCOVERED, never "safe"
× is silent on mysql duplicate entry — false here means UNCOVERED, never "safe"
× withholds the same uncovered text through the declaration channel

Zero collateral inside packages/types, which is the evidence that the three limbs match the templates and nothing else in that suite.

The rewrite keeps them equally capable of going red, which was the point of building them:

  • The same three measured messages now assert true, plus three more (MySQL's where clause spelling, a qualified table.column key, and a value containing its own quote). A future change that silently drops MySQL coverage fails here, on templates measured off real MySQL text.
  • Six new negative cases guard the other direction — if someone relaxes a MySQL anchor to a bare includes(...), they go red before a deployment starts answering "Internal server error" to real questions.
  • The false-means-UNCOVERED lesson is not retired with them. It was never about MySQL: it is the reading a reviewer on PR fix(objectql): refuse undeclared insert fields at the schema, and keep bound values out of the write-path logs (#8682) #8737 got wrong while sizing a disclosure residual, which is what produced this card. A second block keeps the original toBe(false) shape pointed at MSSQL and Oracle, so the distinction keeps a live subject instead of becoming a paragraph.

Reverse verification, direction predicted in writing first

Both legs were predicted before running, and the prediction file recorded the inverse direction this card requires.

Leg A — the original pins must fail against the change. Predicted 4 red, zero collateral. Observed exactly that (above).

Leg B — the rewritten pins must go red against pre-ruling behaviour. Fix committed first, then git checkout origin/main -- packages/types/src/error-leak.ts alone:

Tests 5 failed | 79 passed (84)
× covers mysql unknown column
× covers mysql unknown column, where clause
× covers mysql missing table
× covers mysql duplicate entry
× covers mysql duplicate entry, qualified key
✓ covers mysql duplicate entry behind a knex statement prefix
✓ is silent on mssql invalid object name …
✓ is silent on oracle missing table or view …
✓ withholds the same uncovered text through the declaration channel

One missed prediction, kept rather than tidied away: leg B was predicted at 3 red and came back 5. The direction was right; the count was stale because two positive cases were added after the prediction was written. Recording it because the useful half is the two that stayed green: the knex-prefixed case is green in both directions (it reaches true through the pre-existing insert into limb, independently of the MySQL limbs — which is exactly why it was added), and the MSSQL/Oracle block is green in both directions by design, making it a guard rather than evidence.

Restored by git checkout of the branch, byte-identity proved rather than assumed: worktree blob 4e2d8a49ab83996acddb646e4a75cff6ebfb302f equals git rev-parse HEAD:packages/types/src/error-leak.ts.

The card's four measurements, re-measured through the shipped functions

Not inherited from the round-8 report. Driven through the real redactStatementFromMessage, resolving @objectstack/types from its built dist:

mysql unknown column => "Unknown column 'zzz' in 'field list' [statement and bound values redacted]"
mysql duplicate entry => "Duplicate entry 'acme@example.com' for key 'crm_account.email' [statement and bound values redacted]"
sqlite unique violation => "UNIQUE constraint failed: crm_account.email [statement and bound values redacted]"
pg unique violation => "duplicate key value violates unique constraint \"crm_account_email_key\" [statement and bound values redacted]"

Byte-identical to the recorded values. Three keep an identifier; one keeps a caller's value, and it is the dialect the false claim excluded. Postgres' own value was confirmed to live on error.detailKey (email)=(acme@example.com) already exists. — which Logger does not serialize, and which the redactor's rebuilt Error drops as well; so Postgres escapes for a reason unrelated to the cut, as recorded.

No status mapping moves, and the log does not move either

Measured, not reasoned:

Consumption-radius sweep, not a package-scoped one

The predicate has consumers in seven packages, so the sweep followed the rule's consumers rather than the edited package. Full suites run:

packageresult
types348 passed
objectql3721 passed
rest2001 passed
runtime2456 passed
metadata-protocol1524 passed
service-analytics1722 passed
service-package38 passed
hono73 passed

Exactly two verdicts moved outside packages/types, and both are files that measure this predicate directly rather than files that merely use it:

  • rest/src/rest-unique-violation-dialects.test.ts — the #6250 block pinned "the fix did not widen the internal-leak classifier". Rewritten to assert the invariant it was always for: the 409 does not depend on the leak verdict. The rewrite is 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.
  • metadata-protocol/src/protocol.driver-text-disclosure.test.ts — the DIALECTS matrix row and its uncovered-count. That file's own header instructs the next author to flip rather than delete, and that is what was done. Every withhold case in it passed before and after the flip, which is the cleanest available demonstration of its thesis: a producer that withholds by DECLARATION is unaffected when the phrasing list moves underneath it.

Two neighbouring comments that state the old verdict as history were given a dated qualifier so the next reader cannot re-derive a false present-tense claim from them — which is the exact defect class this card exists for.

What this PR does not do

Gates

Derived after the final commit against the actual changed paths, and run at that HEAD — 05e85a759.

The derivation named more families than the dispatch prompt predicted: it expected check:nul-bytes, check:query-options-erasure, check:type-check-coverage, check:type-check-debt, and the re-derivation added check:changeset-gate-self-tests, check:cross-package-test-inputs, check:durability-log-level, check:objectui-changeset, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-cross-package-test-inputs, plus convention-triggered check:engine-double-contract and check:where-matcher (this PR edits three test files). The union was run, output redirected to files, never piped.

14 of 14 EXIT=0.check:type-check-debt --re-measure ran on the built workspace closure — 33 ledger entries re-measured in 294.7s, 1926 raw errors, none above its recorded number — so it is MEASURED, not the unbuilt-worktree refusal. Nothing was NOT MEASURED.


Generated by Claude Code

…plates (#8739)
`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn
@vercel

vercelBot commented Aug 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 16, 2026 7:05am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/types.

1 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/plugins/packages.mdx(via @objectstack/types)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@qq9340100@claude