fix(core): Track quote state when sanitizing SQL literals - #23659
Open
Lms24 wants to merge 3 commits into
Open
Conversation
`_sanitizeSqlQuery` stripped string literals with a single regex over `'...'`, which misses two shapes MySQL produces. Double-quoted values are string literals in MySQL unless `ANSI_QUOTES` is set, and backslashes escape the next character unless `NO_BACKSLASH_ESCAPES` is set — and `mysql`/`mysql2` escape inlined values with backslashes, so `WHERE name = ?` with `O'Brien` arrives as `'O\'Brien'`. In both cases the value survived into `db.query.text`, and `getSqlQuerySummary` then read any `from`/`join` inside it as a table name, putting it in `db.query.summary` and — with span streaming — in the span name. Replace the literal and comment regexes with a single scanning pass, so quote state and comment state are no longer decided independently: `--` inside a literal no longer truncates it (which also leaked, in every dialect), and a literal's `X`/`B`/`E` prefix collapses into the same `?`. Quoted identifiers are still preserved, since the summary is built from them. The dialect is a parameter because the same characters mean different things per driver: `"` quotes identifiers in PostgreSQL and SQLite, and backslash is literal there. PostgreSQL dollar-quoted strings (`$$...$$`) are still unhandled — telling them apart from `$n` placeholders is ambiguous, and existing behavior for `$1$2$3` is pinned by tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cb2b8f7. Configure here.
`_sanitizeSqlQuery` lived in the postgres.js integration, but it is dialect-generic and now serves the mysql, mysql2, postgres, postgres.js and Cloudflare D1 paths. Move it next to `getSqlQuerySummary`, which is its only downstream consumer and already lives there, so the sanitize-then-summarize pipeline reads as one unit and D1 no longer reaches into a postgres integration module for it. Rename `_sanitizeSqlQuery` to `sanitizeSqlQuery` to match its neighbour; the public `_INTERNAL_sanitizeSqlQuery` alias is unchanged, so nothing outside core moves. Tests move with it, and the leak regressions can now assert the summary directly instead of only the sanitized statement. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lms24
marked this pull request as ready for review
August 26, 2026 16:21
Lms24
requested review from
JPeer264,
msonnb and
stephanie-anderson
and removed request for
a team and
stephanie-anderson
August 26, 2026 16:21
JPeer264
approved these changes
Aug 27, 2026
JPeer264
left a comment
JPeer264
left a comment
Member
There was a problem hiding this comment.
before
SELECT * FROM lgtm WHERE feedback = "Nice work"after
SELECT lgtm
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes a couple of edge cases, most prominently that double quotes can be used as string literals in MySQL (
"...").Changes:
db.query.summarybeforeSELECT * FROM users WHERE name = 'O\'Brien from ACME'SELECT users ACME'SELECT usersSELECT * FROM users WHERE bio = "i come from Berlin and join clubs"SELECT users Berlin clubs"SELECT usersINSERT INTO t (c) VALUES ("select from s3cret-token")INSERT t select s3cret-token"INSERT tSELECT * FROM t WHERE a = 'from secret--x'SELECT t secretSELECT tComments and literals are now stripped in one scanning pass rather than by regexes that each decide quote state on their own.
This also moves the sql sanitization logic to the
utils/sql.tsfile since it's now reused across multiple sql libraries (msql, postgres, cloudflare d1)Stacked on #23601