diff --git a/packages/server-utils/src/integrations/knex.ts b/packages/server-utils/src/integrations/knex.ts index a11d83eec9fe..e326a2e7a7ec 100644 --- a/packages/server-utils/src/integrations/knex.ts +++ b/packages/server-utils/src/integrations/knex.ts @@ -33,7 +33,7 @@ import { DB } from '@sentry/conventions/op'; import { DEBUG_BUILD } from '../debug-build'; import { CHANNELS } from '../orchestrion/channels'; import { bindTracingChannelToSpan } from '../tracing-channel'; -import { sanitizeSqlQueryWithSummary } from '../utils/sql'; +import { sanitizeSqlQueryWithSummary, toSqlDialect } from '../utils/sql'; // NOTE: this uses the same name as the OTel integration by design. `@sentry/node`'s `knexIntegration` // picks this subscriber over the vendored OTel path when orchestrion injection is active. @@ -174,7 +174,7 @@ function subscribeQuery(): void { connection?.filename || connection?.database || extractDatabaseFromConnectionString(connectionString); const dbSystem = mapSystem(client?.driverName); - const dialect = client?.driverName === 'mysql' || client?.driverName === 'mysql2' ? 'mysql' : undefined; + const dialect = toSqlDialect(client?.driverName); const { queryText: dbStatement, querySummary } = sanitizeSqlQueryWithSummary( query?.sql ? truncate(query.sql, MAX_QUERY_LENGTH) : undefined, dialect, diff --git a/packages/server-utils/src/integrations/prisma/tracing-helper.ts b/packages/server-utils/src/integrations/prisma/tracing-helper.ts index efa2f279e17a..bb17556efea1 100644 --- a/packages/server-utils/src/integrations/prisma/tracing-helper.ts +++ b/packages/server-utils/src/integrations/prisma/tracing-helper.ts @@ -35,7 +35,7 @@ import { SENTRY_KIND, SENTRY_OP, } from '@sentry/conventions/attributes'; -import { getSqlQuerySummary, sanitizeSqlQuery, type SqlDialect } from '../../utils/sql'; +import { getSqlQuerySummary, sanitizeSqlQuery, type SqlDialect, toSqlDialect } from '../../utils/sql'; // Reading `process.env` can throw in runtimes that gate env access (e.g. Deno without `--allow-env`) // and `process` may be absent altogether (edge runtimes), so this degrades to `false` in those cases. @@ -128,10 +128,10 @@ function buildSpanAttributes(name: string, attributes: Record | * a string literal rather than a quoted identifier, so sanitizing it as standard SQL leaves the value * in place — and a literal containing `FROM`/`JOIN` then reads as a table name in the summary. */ -function getSqlDialect(attributes: SpanAttributes): SqlDialect | undefined { +function getSqlDialect(attributes: SpanAttributes): SqlDialect { // oxlint-disable-next-line typescript/no-deprecated const system = attributes[DB_SYSTEM_NAME] ?? attributes[DB_SYSTEM]; - return system === 'mysql' || system === 'mariadb' ? 'mysql' : undefined; + return toSqlDialect(system); } /** diff --git a/packages/server-utils/src/integrations/tedious.ts b/packages/server-utils/src/integrations/tedious.ts index 6ed5ab70af1d..0c1cf4ca4927 100644 --- a/packages/server-utils/src/integrations/tedious.ts +++ b/packages/server-utils/src/integrations/tedious.ts @@ -132,7 +132,7 @@ function subscribeQuery(channelName: string, operation: string): void { const databaseName = connection[currentDatabaseSymbol]; const sql = extractSql(request); - const queryText = sql ? sanitizeSqlQuery(sql) : undefined; + const queryText = sql ? sanitizeSqlQuery(sql, 'mssql') : undefined; const querySummary = queryText && operation !== 'callProcedure' ? getSqlQuerySummary(queryText) : undefined; const attributes: SpanAttributes = { diff --git a/packages/server-utils/src/utils/sql.ts b/packages/server-utils/src/utils/sql.ts index a015f895ca0a..fb8ecd5ca8a8 100644 --- a/packages/server-utils/src/utils/sql.ts +++ b/packages/server-utils/src/utils/sql.ts @@ -149,23 +149,62 @@ function truncate(summary: string): string { } let integerLiteralRE: RegExp | undefined; +let mssqlIntegerLiteralRE: RegExp | undefined; + +/** + * The lookbehind keeps `$n` and `?n` parameter placeholders out of the match. T-SQL has neither, + * and `$1000` there is a money literal, so `$` only guards a placeholder outside `mssql`. + * + * Lazy init: constructing a lookbehind at module scope evaluates it on import and crashes + * Safari <16.4 browser bundles that reach this file via the core barrel. + */ +function getIntegerLiteralRE(dialect: SqlDialect): RegExp { + if (dialect === 'mssql') { + if (!mssqlIntegerLiteralRE) { + mssqlIntegerLiteralRE = new RegExp('(? { it.each([undefined, ''])('returns undefined for %j', input => { @@ -679,6 +679,26 @@ describe('sanitizeSqlQuery', () => { }); }); + describe("dialect: 'mssql'", () => { + it.each([ + // `[...]` quotes an identifier, so a `'` inside one belongs to the name + ["SELECT * FROM [dbo].[user's] WHERE email = 'jane@example.com'", "SELECT * FROM [dbo].[user's] WHERE email = ?"], + // `]]` escapes a `]` inside the name + ['SELECT [a]]b] FROM [t] WHERE c = 1', 'SELECT [a]]b] FROM [t] WHERE c = ?'], + ['SELECT * FROM [dbo].[Customer Orders] WHERE id = @P1', 'SELECT * FROM [dbo].[Customer Orders] WHERE id = @P1'], + ["INSERT INTO [dbo].[users] ([name]) VALUES (N'Jane')", 'INSERT INTO [dbo].[users] ([name]) VALUES (?)'], + // T-SQL has no `E'...'` escape strings, so the `E` stays an identifier + ["SELECT * FROM t WHERE a = E'x'", 'SELECT * FROM t WHERE a = E?'], + // ... and no dollar quoting, where `$` is an ordinary identifier character + ['SELECT * FROM t WHERE a = $$x$$', 'SELECT * FROM t WHERE a = $$x$$'], + // `$n` is a money literal here, not the placeholder it is in PostgreSQL + ['SELECT * FROM t WHERE price = $1000', 'SELECT * FROM t WHERE price = $?'], + ['SELECT * FROM t WHERE price = $10.50', 'SELECT * FROM t WHERE price = $?'], + ])('sanitizes %p', (input, expected) => { + expect(sanitizeSqlQuery(input, 'mssql')).toBe(expected); + }); + }); + describe('unterminated literals swallow the rest of the statement', () => { it.each([ ["SELECT * FROM t WHERE a = 'jane@example.com AND b = 2", 'standard' as const], @@ -689,6 +709,25 @@ describe('sanitizeSqlQuery', () => { ])('drops the unterminated value in %p (%s)', (input, dialect) => { expect(sanitizeSqlQuery(input, dialect)).toBe('SELECT * FROM t WHERE a = ?'); }); + + // An unterminated identifier quote is the same hazard: the rest of the statement is not a + // name, so copying it through would carry the literals in it out unlexed. + it.each([ + ["SELECT * FROM [users WHERE email = 'jane@example.com'", 'mssql' as const], + ["SELECT * FROM \"users WHERE email = 'jane@example.com'", 'standard' as const], + ["SELECT * FROM `users WHERE email = 'jane@example.com'", 'mysql' as const], + ])('drops the unterminated identifier in %p (%s)', (input, dialect) => { + expect(sanitizeSqlQuery(input, dialect)).toBe('SELECT * FROM ?'); + }); + + // The run ends on a doubled closer, which escapes the character rather than closing the name + it.each([ + ["SELECT * FROM [t WHERE email = 'jane@example.com' AND x = [a]]", 'mssql' as const], + ['SELECT * FROM "t WHERE email = \'jane@example.com\' AND x = a""', 'standard' as const], + ["SELECT * FROM `t WHERE email = 'jane@example.com' AND x = a``", 'mysql' as const], + ])('drops an identifier left open by an escaped closer in %p (%s)', (input, dialect) => { + expect(sanitizeSqlQuery(input, dialect)).toBe('SELECT * FROM ?'); + }); }); describe('representative statements per driver', () => { @@ -768,7 +807,7 @@ describe('sanitizeSqlQuery', () => { 'INSERT INTO users (name, email) VALUES (?, ?)', ], ])('sanitizes SQL Server statement %p', (input, expected) => { - expect(sanitizeSqlQuery(input)).toBe(expected); + expect(sanitizeSqlQuery(input, 'mssql')).toBe(expected); }); }); @@ -787,6 +826,7 @@ describe('sanitizeSqlQuery', () => { ['standard' as const, 'INSERT INTO t (c) VALUES ($tag$select from s3cret-token$tag$)', 's3cret-token'], ['standard' as const, "SELECT * FROM users WHERE name = N'from ACME'", 'ACME'], ['standard' as const, String.raw`UPDATE t SET a = E'x\'y from Z' WHERE id = 5`, 'from Z'], + ['mssql' as const, "SELECT * FROM [dbo].[users] WHERE note = 'from bob@secret.com'", 'bob@secret.com'], ])('strips the value out of %s statement %p', (dialect, input, value) => { const sanitized = sanitizeSqlQuery(input, dialect); expect(sanitized).not.toContain(value); @@ -795,6 +835,44 @@ describe('sanitizeSqlQuery', () => { }); }); +describe('sanitizeSqlQueryWithSummary', () => { + it('returns the sanitized statement and its summary', () => { + expect(sanitizeSqlQueryWithSummary("SELECT * FROM users WHERE email = 'jane@example.com'")).toEqual({ + queryText: 'SELECT * FROM users WHERE email = ?', + querySummary: 'SELECT users', + }); + }); + + it('passes the dialect through to the sanitizer', () => { + expect(sanitizeSqlQueryWithSummary('SELECT * FROM users WHERE email = "jane@example.com"', 'mysql')).toEqual({ + queryText: 'SELECT * FROM users WHERE email = ?', + querySummary: 'SELECT users', + }); + }); + + it('returns undefined for both when there is no statement', () => { + expect(sanitizeSqlQueryWithSummary(undefined)).toEqual({ queryText: undefined, querySummary: undefined }); + expect(sanitizeSqlQueryWithSummary('')).toEqual({ queryText: undefined, querySummary: undefined }); + }); +}); + +describe('toSqlDialect', () => { + it.each([ + ['mysql', 'mysql'], + ['mysql2', 'mysql'], + ['mariadb', 'mysql'], + ['mssql', 'mssql'], + ['sqlserver', 'mssql'], + ['microsoft.sql_server', 'mssql'], + ])('maps %j to %j', (system, expected) => { + expect(toSqlDialect(system)).toBe(expected); + }); + + it.each(['postgresql', 'sqlite', 'oracle', '', undefined])('falls back to standard for %j', system => { + expect(toSqlDialect(system)).toBe('standard'); + }); +}); + describe('sanitizeSqlQueryWithSummary', () => { it('returns the sanitized statement and its summary', () => { expect(sanitizeSqlQueryWithSummary("SELECT * FROM users WHERE email = 'jane@example.com'")).toEqual({ @@ -815,6 +893,13 @@ describe('sanitizeSqlQueryWithSummary', () => { }); }); + it('passes the mssql dialect through, so a quote inside a bracketed name stays part of the name', () => { + expect(sanitizeSqlQueryWithSummary("SELECT * FROM [users] WHERE [email] = 'jane@example.com'", 'mssql')).toEqual({ + queryText: 'SELECT * FROM [users] WHERE [email] = ?', + querySummary: 'SELECT [users]', + }); + }); + it('derives the summary from the sanitized statement, not the raw one', () => { expect(sanitizeSqlQueryWithSummary("SELECT * FROM users WHERE bio = 'from secret_table'")).toEqual({ queryText: 'SELECT * FROM users WHERE bio = ?',