Skip to content

Commit f6d1a96

Browse files
committed
fix(server-utils): Sanitize PostgreSQL dollar-quoted strings in db.query.text
1 parent 002809a commit f6d1a96

2 files changed

Lines changed: 222 additions & 18 deletions

File tree

‎packages/server-utils/src/utils/sql.ts‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,17 @@ let integerLiteralRE: RegExp | undefined;
152152

153153
/**
154154
* SQL dialect variants that matter for finding the end of a string literal:
155-
* - `standard` (PostgreSQL, SQLite): `"` quotes identifiers and `''` is the only in-string escape.
155+
* - `standard` (PostgreSQL, SQLite, SQL Server): `"` quotes identifiers, `''` is the only
156+
* in-string escape, and PostgreSQL's `$$…$$` dollar quoting opens a literal.
156157
* - `mysql`: `"` quotes a string literal unless `ANSI_QUOTES` is set, and `\` escapes the next
157158
* character unless `NO_BACKSLASH_ESCAPES` is set. Both default to off, and mysql/mysql2 escape
158159
* inlined values with backslashes, so this is the mode their statements arrive in.
159160
*/
160161
export type SqlDialect = 'standard' | 'mysql';
161162

163+
// Sticky, so the scanner can test one position without slicing the query on every `$`.
164+
const DOLLAR_QUOTE_RE = /\$(?:[A-Za-z_]\w*)?\$/y;
165+
162166
/**
163167
* Returns the index just past the run's closing `delimiter`, or the end of the query if the run is
164168
* never closed — an unterminated literal must swallow the remainder rather than let it through.
@@ -213,6 +217,19 @@ function stripLiteralsAndComments(sql: string, dialect: SqlDialect): string {
213217
continue;
214218
}
215219

220+
// In a dollar-quoted body (`$$body$$`, `$tag$body$tag$`) nothing has syntax meaning. Read the
221+
// previous character from the query, not from `out`, where a dropped comment would leave `$$`
222+
// looking like part of an identifier. MySQL has no dollar quoting and allows `$` in names.
223+
if (!isMysql && char === '$' && !isIdentifierChar(sql[i - 1])) {
224+
const tag = matchDollarQuoteTag(sql, i);
225+
if (tag) {
226+
const bodyEnd = sql.indexOf(tag, i + tag.length);
227+
i = bodyEnd === -1 ? sql.length : bodyEnd + tag.length;
228+
out += '?';
229+
continue;
230+
}
231+
}
232+
216233
// Quoted identifiers: backticks in MySQL, double quotes everywhere else
217234
if (char === '`' || (char === '"' && !isMysql)) {
218235
const runEnd = findQuotedRunEnd(sql, i, char, false);
@@ -222,7 +239,7 @@ function stripLiteralsAndComments(sql: string, dialect: SqlDialect): string {
222239
}
223240

224241
if (char === "'" || (char === '"' && isMysql)) {
225-
// A prefix like `X'1A'`, `B'01'` or PostgreSQL's `E'a\nb'` is part of the literal, so it has
242+
// A prefix like `X'1A'`, `B'01'`, `N'…'` or PostgreSQL's `E'a\nb'` is part of the literal, so it has
226243
// to collapse into the same `?` instead of being left behind as a bare identifier.
227244
const prefix = char === "'" ? getLiteralPrefix(out, isMysql) : undefined;
228245
out = prefix ? out.slice(0, -1) : out;
@@ -238,18 +255,30 @@ function stripLiteralsAndComments(sql: string, dialect: SqlDialect): string {
238255
return out;
239256
}
240257

258+
/** Whether `char` can appear inside an identifier. `undefined` (start of query) counts as a break. */
259+
function isIdentifierChar(char: string | undefined): boolean {
260+
return char !== undefined && /[\w$]/.test(char);
261+
}
262+
263+
/** Returns the opening dollar-quote tag at `start` (`$$` or `$tag$`), or undefined if there is none. */
264+
function matchDollarQuoteTag(sql: string, start: number): string | undefined {
265+
DOLLAR_QUOTE_RE.lastIndex = start;
266+
return DOLLAR_QUOTE_RE.exec(sql)?.[0];
267+
}
268+
241269
/**
242270
* Returns the literal-prefix character immediately before a `'`, if there is one: `X`/`B` for
243-
* hex/binary literals, or `E` for a PostgreSQL escape string (which honors backslash escapes).
271+
* hex/binary literals, `N` for a national-character literal (SQL Server, MySQL), or `E` for a
272+
* PostgreSQL escape string (which honors backslash escapes).
244273
*/
245-
function getLiteralPrefix(out: string, isMysql: boolean): 'X' | 'B' | 'E' | undefined {
274+
function getLiteralPrefix(out: string, isMysql: boolean): 'X' | 'B' | 'N' | 'E' | undefined {
246275
// A prefix only counts when it stands alone — the `X` in `MAX'...'` belongs to the identifier
247-
if (/[\w$]/.test(out.slice(-2, -1))) {
276+
if (isIdentifierChar(out.slice(-2, -1))) {
248277
return undefined;
249278
}
250279

251280
const prefix = out.slice(-1).toUpperCase();
252-
if (prefix === 'X' || prefix === 'B') {
281+
if (prefix === 'X' || prefix === 'B' || prefix === 'N') {
253282
return prefix;
254283
}
255284
return prefix === 'E' && !isMysql ? 'E' : undefined;
@@ -259,8 +288,9 @@ function getLiteralPrefix(out: string, isMysql: boolean): 'X' | 'B' | 'E' | unde
259288
* Sanitize SQL query as per the OTEL semantic conventions
260289
* https://opentelemetry.io/docs/specs/semconv/database/database-spans/#sanitization-of-dbquerytext
261290
*
262-
* PostgreSQL $n placeholders are preserved per OTEL spec - they're parameterized queries,
263-
* not sensitive literals. Only actual values (strings, numbers, booleans) are sanitized.
291+
* Parameter placeholders survive: PostgreSQL `$n`, SQLite `?n`, and named forms like `:name` and
292+
* `@name`. Per the OTEL spec they mark a parameterized query, so only values (strings, numbers,
293+
* booleans) are sanitized.
264294
*
265295
* Pass `dialect` when the statement comes from a driver whose literals are not standard-quoted;
266296
* see {@link SqlDialect}.
@@ -274,7 +304,7 @@ export function sanitizeSqlQuery(sqlQuery: string | undefined, dialect: SqlDiale
274304
// on import and crash Safari <16.4 browser bundles that reach this file via
275305
// the core barrel. Building it on first call keeps the cost off the import path.
276306
if (!integerLiteralRE) {
277-
integerLiteralRE = new RegExp('(?<!\\$)-?\\b\\d+\\b', 'g');
307+
integerLiteralRE = new RegExp('(?<![$?])-?\\b\\d+\\b', 'g');
278308
}
279309

280310
return (

‎packages/server-utils/test/utils/sql.test.ts‎

Lines changed: 183 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -615,18 +615,180 @@ describe('sanitizeSqlQuery', () => {
615615
});
616616
});
617617

618+
describe("dialect: 'standard'", () => {
619+
it.each([
620+
// `"..."` quotes an identifier, so it survives as a summary target
621+
['SELECT * FROM "User" WHERE "email" = \'jane@example.com\'', 'SELECT * FROM "User" WHERE "email" = ?'],
622+
['SELECT "col""umn" FROM t WHERE a = 1', 'SELECT "col""umn" FROM t WHERE a = ?'],
623+
// PostgreSQL reads `#` as a bitwise operator, not a comment
624+
['SELECT * FROM t WHERE a # 1 = 2', 'SELECT * FROM t WHERE a # ? = ?'],
625+
// `\` is an ordinary character, so the literal ends at the next quote
626+
[String.raw`SELECT * FROM t WHERE path = 'C:\' AND b = 2`, 'SELECT * FROM t WHERE path = ? AND b = ?'],
627+
// dollar-quoted strings (PostgreSQL), tagged and untagged
628+
["SELECT * FROM t WHERE a = $$O'Brien$$", 'SELECT * FROM t WHERE a = ?'],
629+
['SELECT * FROM t WHERE a = $tag$secret$tag$ AND b = $1', 'SELECT * FROM t WHERE a = ? AND b = $1'],
630+
['SELECT $$a$$, $$b$$ FROM t', 'SELECT ?, ? FROM t'],
631+
// a `$` inside an identifier does not open a dollar quote, even when a dropped comment is
632+
// what separates the two
633+
['SELECT a$$b FROM t WHERE c = 1', 'SELECT a$$b FROM t WHERE c = ?'],
634+
['SELECT a /* c */ $$secret$$ FROM t', 'SELECT a ? FROM t'],
635+
['SELECT a/* c */$$secret$$ FROM t', 'SELECT a? FROM t'],
636+
// `$name` is a SQLite parameter placeholder, not a dollar quote
637+
['INSERT INTO t (a, b) VALUES ($name, $email)', 'INSERT INTO t (a, b) VALUES ($name, $email)'],
638+
// national-character literals collapse with their prefix: `N'...'` in SQL Server (which
639+
// requires the uppercase N) and in MySQL (which takes either case)
640+
["SELECT * FROM t WHERE name = N'Jane'", 'SELECT * FROM t WHERE name = ?'],
641+
["SELECT * FROM t WHERE name = n'Jane'", 'SELECT * FROM t WHERE name = ?'],
642+
// ... unless the `N` belongs to the identifier before it. `MIN'x'` parses in no dialect. It
643+
// guards against a name ending in N eating the quote after it.
644+
["SELECT MIN'x' FROM t", 'SELECT MIN? FROM t'],
645+
])('sanitizes %p', (input, expected) => {
646+
expect(sanitizeSqlQuery(input)).toBe(expected);
647+
expect(sanitizeSqlQuery(input, 'standard')).toBe(expected);
648+
});
649+
650+
// Known limit: SQLite reads `"..."` as a string when the name matches no column
651+
// (`SQLITE_DQS`), and only the schema tells that apart from an identifier. Drivers that bind
652+
// their values never emit this shape.
653+
it('leaves a SQLite double-quoted string in place', () => {
654+
expect(sanitizeSqlQuery('SELECT * FROM t WHERE a = "jane@example.com"')).toBe(
655+
'SELECT * FROM t WHERE a = "jane@example.com"',
656+
);
657+
});
658+
});
659+
660+
describe('dialect divergence', () => {
661+
it.each([
662+
// [input, standard, mysql]
663+
['SELECT * FROM t WHERE name = "Jane"', 'SELECT * FROM t WHERE name = "Jane"', 'SELECT * FROM t WHERE name = ?'],
664+
['SELECT * FROM t WHERE a = 1 # 2', 'SELECT * FROM t WHERE a = ? # ?', 'SELECT * FROM t WHERE a = ?'],
665+
[
666+
String.raw`SELECT * FROM t WHERE a = 'x\' AND b = 'Jane'`,
667+
'SELECT * FROM t WHERE a = ? AND b = ?',
668+
// MySQL reads `\'` as an escaped quote, so the literal runs to the quote before `Jane` and
669+
// swallows the statement text. The server lexes it the same way, which leaves `Jane` a bare
670+
// token here, not the value it looks like.
671+
'SELECT * FROM t WHERE a = ?Jane?',
672+
],
673+
// MySQL has no dollar quoting and allows `$` in identifiers, so it leaves `$...$` alone
674+
['SELECT $col$x FROM t WHERE a = 1', 'SELECT ?', 'SELECT $col$x FROM t WHERE a = ?'],
675+
['SELECT `a` FROM t WHERE b = 1', 'SELECT `a` FROM t WHERE b = ?', 'SELECT `a` FROM t WHERE b = ?'],
676+
])('%p sanitizes to %p (standard) and %p (mysql)', (input, standard, mysql) => {
677+
expect(sanitizeSqlQuery(input, 'standard')).toBe(standard);
678+
expect(sanitizeSqlQuery(input, 'mysql')).toBe(mysql);
679+
});
680+
});
681+
682+
describe('unterminated literals swallow the rest of the statement', () => {
683+
it.each([
684+
["SELECT * FROM t WHERE a = 'jane@example.com AND b = 2", 'standard' as const],
685+
["SELECT * FROM t WHERE a = N'jane@example.com AND b = 2", 'standard' as const],
686+
['SELECT * FROM t WHERE a = $$jane@example.com AND b = 2', 'standard' as const],
687+
['SELECT * FROM t WHERE a = "jane@example.com AND b = 2', 'mysql' as const],
688+
["SELECT * FROM t WHERE a = 'jane@example.com AND b = 2", 'mysql' as const],
689+
])('drops the unterminated value in %p (%s)', (input, dialect) => {
690+
expect(sanitizeSqlQuery(input, dialect)).toBe('SELECT * FROM t WHERE a = ?');
691+
});
692+
});
693+
694+
describe('representative statements per driver', () => {
695+
it.each([
696+
// pg / postgres-js: parameterized text passes through unchanged
697+
[
698+
'SELECT "User"."id" FROM "public"."User" WHERE "User"."email" = $1 AND "User"."age" > $2 LIMIT $3',
699+
'SELECT "User"."id" FROM "public"."User" WHERE "User"."email" = $1 AND "User"."age" > $2 LIMIT $3',
700+
],
701+
// pg: values inlined by the caller instead of bound
702+
[
703+
"SELECT * FROM users WHERE email = 'jane@example.com' AND created_at > '2024-01-01' ORDER BY id DESC LIMIT 10",
704+
'SELECT * FROM users WHERE email = ? AND created_at > ? ORDER BY id DESC LIMIT ?',
705+
],
706+
[
707+
"UPDATE accounts SET balance = balance - 42.50, note = 'rent for jane' WHERE owner_email = 'jane@example.com'",
708+
'UPDATE accounts SET balance = balance - ?, note = ? WHERE owner_email = ?',
709+
],
710+
[
711+
"DELETE FROM sessions WHERE token = 'sk_live_abc123' OR expires_at < NOW() - INTERVAL '7 days'",
712+
'DELETE FROM sessions WHERE token = ? OR expires_at < NOW() - INTERVAL ?',
713+
],
714+
])('sanitizes PostgreSQL statement %p', (input, expected) => {
715+
expect(sanitizeSqlQuery(input)).toBe(expected);
716+
});
717+
718+
it.each([
719+
// mysql/mysql2 escape inlined values with backslashes and quote identifiers with backticks
720+
[
721+
"SELECT * FROM `users` WHERE `email` = 'o\\'brien@example.com' AND `active` = 1",
722+
'SELECT * FROM `users` WHERE `email` = ? AND `active` = ?',
723+
],
724+
[
725+
"INSERT INTO `users` (`name`, `bio`) VALUES ('Jane', 'says \\\"hi\\\" a lot')",
726+
'INSERT INTO `users` (`name`, `bio`) VALUES (?, ?)',
727+
],
728+
[
729+
'SELECT * FROM `users` WHERE `id` = ? AND `status` = ?',
730+
'SELECT * FROM `users` WHERE `id` = ? AND `status` = ?',
731+
],
732+
[
733+
'UPDATE `orders` SET `note` = "customer said: don\'t ship" WHERE `id` = 7',
734+
'UPDATE `orders` SET `note` = ? WHERE `id` = ?',
735+
],
736+
])('sanitizes MySQL statement %p', (input, expected) => {
737+
expect(sanitizeSqlQuery(input, 'mysql')).toBe(expected);
738+
});
739+
740+
it.each([
741+
// SQLite (D1, Nitro/Nuxt): `?`, `?n` and named placeholders survive, inlined values do not
742+
[
743+
"INSERT INTO users (name, email) VALUES ('Jane', 'jane@example.com')",
744+
'INSERT INTO users (name, email) VALUES (?, ?)',
745+
],
746+
[
747+
'INSERT OR REPLACE INTO users (id, email) VALUES (?1, ?2)',
748+
'INSERT OR REPLACE INTO users (id, email) VALUES (?1, ?2)',
749+
],
750+
[
751+
'SELECT * FROM users WHERE email = :email AND age > @minAge',
752+
'SELECT * FROM users WHERE email = :email AND age > @minAge',
753+
],
754+
['PRAGMA table_info(users)', 'PRAGMA table_info(users)'],
755+
])('sanitizes SQLite statement %p', (input, expected) => {
756+
expect(sanitizeSqlQuery(input)).toBe(expected);
757+
});
758+
759+
it.each([
760+
// tedious (SQL Server): `@P1` placeholders survive, `N'...'` literals do not
761+
[
762+
'SELECT [id], [email] FROM [dbo].[users] WHERE [email] = @P1 AND [age] > @P2',
763+
'SELECT [id], [email] FROM [dbo].[users] WHERE [email] = @P1 AND [age] > @P2',
764+
],
765+
["SELECT TOP 10 * FROM users WHERE email = N'jane@example.com'", 'SELECT TOP ? * FROM users WHERE email = ?'],
766+
[
767+
"INSERT INTO users (name, email) VALUES (N'Jane', N'jane@example.com')",
768+
'INSERT INTO users (name, email) VALUES (?, ?)',
769+
],
770+
])('sanitizes SQL Server statement %p', (input, expected) => {
771+
expect(sanitizeSqlQuery(input)).toBe(expected);
772+
});
773+
});
774+
618775
describe('regression: values must not survive as summary targets', () => {
619776
// A literal that survives sanitization and happens to contain `from`/`join`/`select` is read
620777
// as a table name by getSqlQuerySummary, which puts it in `db.query.summary` and — with span
621778
// streaming — in the span name.
622779
it.each([
623-
['SELECT * FROM users WHERE name = "from bob@secret.com"', 'bob@secret.com'],
624-
['SELECT * FROM users WHERE bio = "i come from Berlin and join clubs"', 'Berlin'],
625-
['INSERT INTO t (c) VALUES ("select from s3cret-token")', 's3cret-token'],
626-
[String.raw`SELECT * FROM users WHERE name = 'O\'Brien from ACME'`, 'ACME'],
627-
[String.raw`UPDATE t SET a = 'x\'y from Z' WHERE id = 5`, 'from Z'],
628-
])('strips the value out of %p', (input, value) => {
629-
const sanitized = sanitizeSqlQuery(input, 'mysql');
780+
['mysql' as const, 'SELECT * FROM users WHERE name = "from bob@secret.com"', 'bob@secret.com'],
781+
['mysql' as const, 'SELECT * FROM users WHERE bio = "i come from Berlin and join clubs"', 'Berlin'],
782+
['mysql' as const, 'INSERT INTO t (c) VALUES ("select from s3cret-token")', 's3cret-token'],
783+
['mysql' as const, String.raw`SELECT * FROM users WHERE name = 'O\'Brien from ACME'`, 'ACME'],
784+
['mysql' as const, String.raw`UPDATE t SET a = 'x\'y from Z' WHERE id = 5`, 'from Z'],
785+
['standard' as const, "SELECT * FROM users WHERE name = 'from bob@secret.com'", 'bob@secret.com'],
786+
['standard' as const, 'SELECT * FROM users WHERE bio = $$i come from Berlin and join clubs$$', 'Berlin'],
787+
['standard' as const, 'INSERT INTO t (c) VALUES ($tag$select from s3cret-token$tag$)', 's3cret-token'],
788+
['standard' as const, "SELECT * FROM users WHERE name = N'from ACME'", 'ACME'],
789+
['standard' as const, String.raw`UPDATE t SET a = E'x\'y from Z' WHERE id = 5`, 'from Z'],
790+
])('strips the value out of %s statement %p', (dialect, input, value) => {
791+
const sanitized = sanitizeSqlQuery(input, dialect);
630792
expect(sanitized).not.toContain(value);
631793
expect(getSqlQuerySummary(sanitized)).not.toContain(value);
632794
});
@@ -641,13 +803,25 @@ describe('sanitizeSqlQueryWithSummary', () => {
641803
});
642804
});
643805

644-
it('passes the dialect through to the sanitizer', () => {
645-
expect(sanitizeSqlQueryWithSummary('SELECT * FROM users WHERE email = "jane@example.com"', 'mysql')).toEqual({
806+
it.each([
807+
['mysql' as const, 'SELECT * FROM users WHERE email = "jane@example.com"'],
808+
['standard' as const, "SELECT * FROM users WHERE email = 'jane@example.com'"],
809+
['standard' as const, 'SELECT * FROM users WHERE email = $$jane@example.com$$'],
810+
['standard' as const, "SELECT * FROM users WHERE email = N'jane@example.com'"],
811+
])('passes the %s dialect through to the sanitizer: %p', (dialect, input) => {
812+
expect(sanitizeSqlQueryWithSummary(input, dialect)).toEqual({
646813
queryText: 'SELECT * FROM users WHERE email = ?',
647814
querySummary: 'SELECT users',
648815
});
649816
});
650817

818+
it('derives the summary from the sanitized statement, not the raw one', () => {
819+
expect(sanitizeSqlQueryWithSummary("SELECT * FROM users WHERE bio = 'from secret_table'")).toEqual({
820+
queryText: 'SELECT * FROM users WHERE bio = ?',
821+
querySummary: 'SELECT users',
822+
});
823+
});
824+
651825
it('returns undefined for both when there is no statement', () => {
652826
expect(sanitizeSqlQueryWithSummary(undefined)).toEqual({ queryText: undefined, querySummary: undefined });
653827
expect(sanitizeSqlQueryWithSummary('')).toEqual({ queryText: undefined, querySummary: undefined });

0 commit comments

Comments
 (0)