diff --git a/.cursor/BUGBOT.md b/.cursor/BUGBOT.md index 9bf2d8040791..2a17708c2752 100644 --- a/.cursor/BUGBOT.md +++ b/.cursor/BUGBOT.md @@ -68,6 +68,11 @@ Keep reviews high-signal. Prefer actionable, high-confidence findings over specu - `consoleSandbox(() => { console.warn(...) })` for intentional user-facing warnings (e.g. init-time misconfiguration messages). The `consoleSandbox` wrapper prevents the SDK's own console instrumentation from intercepting the call. Bare `console.*` calls outside very early init paths (e.g. before the logger is available) should be flagged. - Flag `url.full`, `url.query`, `http.target` or `request.query_string` being set from a URL that isn't filtered. Wrap the value in `filterCollectedUrl()` (or `filterCollectedUrlQuery()` for a bare query string), passing the `client` if one is in scope, so `dataCollection.urlQueryParams` applies. Values that can't contain a query (a bare pathname, a queue URL) are fine. The `sdk/no-unfiltered-url-attributes` lint rule catches direct attribute writes, so look for what it can't: URLs passed through a helper or variable first, deprecated aliases set next to a filtered attribute, and URLs on breadcrumbs or events instead of spans. - Flag span names built from a raw URL. Names follow `METHOD scheme://host/path` and must never contain a query string, so they need `stripUrlQueryAndFragment()`, not `filterCollectedUrl()`. +- Flag a SQL statement that reaches telemetry unsanitized. `db.query.text`, `db.query.summary`, a DB span name, and a breadcrumb carrying a statement all have to come from `sanitizeSqlQuery()` or `sanitizeSqlQueryWithSummary()` (`@sentry/server-utils`). Inline literals are user data, and OTel allows collecting query text only once they are replaced with `?`. This is deliberately not gated on `dataCollection.databaseQueryData`, which does not cover query text. + - Cover every place the statement lands, not only the span attribute. The two that get forgotten are the breadcrumb beside the span and the span name used when span streaming is off. + - Sanitize each statement of a batch before joining them. + - Pass the dialect. `toSqlDialect()` maps a driver or `db.system.name` value to one, and a missing dialect leaves MySQL and SQL Server values in the statement. + - Leave Redis command text alone. It is not SQL and has its own redaction path. - Flag usage of the following APIs: `getCurrentScope()`, `getIsolationScope()`, `getClient()` if they are avoidable. Flag it with severity Low and acknowledge from the start that this is more a "is this necessary" check, rather than a rule violation. - Reason for flagging: Usage of these APIs is problematic for multi-client setups where either there is no "current" client/scope, or the wrong client might be used. Calling these APIs would create a current scope, thereby misleading any future calls to these APIs. - What to do instead: Use an existing reference to the scope or client. For example, this is possible in most `Integration` hooks. diff --git a/packages/nuxt/src/runtime/utils/instrumentDatabase.ts b/packages/nuxt/src/runtime/utils/instrumentDatabase.ts index 9700f4bf3e35..8584d7ec123c 100644 --- a/packages/nuxt/src/runtime/utils/instrumentDatabase.ts +++ b/packages/nuxt/src/runtime/utils/instrumentDatabase.ts @@ -14,7 +14,7 @@ import { type StartSpanOptions, } from '@sentry/core'; import { flushIfServerless } from '@sentry/core/server'; -import { getSqlQuerySummary, sanitizeSqlQuery, type SqlDialect } from '@sentry/server-utils'; +import { sanitizeSqlQuery, sanitizeSqlQueryWithSummary, type SqlDialect } from '@sentry/server-utils'; import type { Database, PreparedStatement } from 'db0'; import { type DatabaseConnectionConfig, type DatabaseSpanData, getDatabaseSpanData } from './database-span-data'; import { DB_NAMESPACE, DB_QUERY_SUMMARY, DB_QUERY_TEXT, DB_SYSTEM_NAME } from '@sentry/conventions/attributes'; @@ -262,8 +262,7 @@ function createStartSpanOptions( data: DatabaseSpanData, dialect: SqlDialect | undefined, ): StartSpanOptions { - const queryText = query ? sanitizeSqlQuery(query, dialect) : undefined; - const querySummary = queryText ? getSqlQuerySummary(queryText) : undefined; + const { queryText, querySummary } = sanitizeSqlQueryWithSummary(query, dialect); const client = getClient(); const name = diff --git a/packages/server-utils/src/exports.ts b/packages/server-utils/src/exports.ts index 380d302712c9..3f14e69d93ff 100644 --- a/packages/server-utils/src/exports.ts +++ b/packages/server-utils/src/exports.ts @@ -3,7 +3,7 @@ export { setHttpServerSpanRouteAttribute } from './utils/setHttpServerSpanRouteA export { setAsyncLocalStorageAsyncContextStrategy } from './async-context'; export { openTelemetryIntegration, getOtlpTracesEndpoint } from './opentelemetry'; export * from './ai'; -export { getSqlQuerySummary, sanitizeSqlQuery } from './utils/sql'; +export { getSqlQuerySummary, sanitizeSqlQuery, sanitizeSqlQueryWithSummary } from './utils/sql'; export type { SqlDialect } from './utils/sql'; export { instrumentPostgresJsSql } from './integrations/postgresjs'; export type { PostgresConnectionContext } from './integrations/postgresjs'; diff --git a/packages/server-utils/src/integrations/knex.ts b/packages/server-utils/src/integrations/knex.ts index fc9e9d162a84..a11d83eec9fe 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 { getSqlQuerySummary, sanitizeSqlQuery } from '../utils/sql'; +import { sanitizeSqlQueryWithSummary } 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. @@ -175,8 +175,10 @@ function subscribeQuery(): void { const dbSystem = mapSystem(client?.driverName); const dialect = client?.driverName === 'mysql' || client?.driverName === 'mysql2' ? 'mysql' : undefined; - const dbStatement = query?.sql ? sanitizeSqlQuery(truncate(query.sql, MAX_QUERY_LENGTH), dialect) : undefined; - const querySummary = dbStatement ? getSqlQuerySummary(dbStatement) : undefined; + const { queryText: dbStatement, querySummary } = sanitizeSqlQueryWithSummary( + query?.sql ? truncate(query.sql, MAX_QUERY_LENGTH) : undefined, + dialect, + ); const attributes: SpanAttributes = { [SENTRY_OP]: DB, [SENTRY_KIND]: 'client', diff --git a/packages/server-utils/src/integrations/mysql.ts b/packages/server-utils/src/integrations/mysql.ts index ba77251bd7c4..f8a9b73464c8 100644 --- a/packages/server-utils/src/integrations/mysql.ts +++ b/packages/server-utils/src/integrations/mysql.ts @@ -22,7 +22,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan, } from '@sentry/core'; -import { getSqlQuerySummary, sanitizeSqlQuery } from '../utils/sql'; +import { sanitizeSqlQueryWithSummary } from '../utils/sql'; import { CHANNELS } from '../orchestrion/channels'; import { bindTracingChannelToSpan } from '../tracing-channel'; import { mysqlModuleNames } from '../orchestrion/config/mysql'; @@ -88,8 +88,7 @@ function instrumentMysql(): void { // handler with the caller's context lost. `deferSpanEnd` replays this scope onto the emitter. data._sentryCallerScope = getCurrentScope(); - const queryText = sql ? sanitizeSqlQuery(sql, 'mysql') : undefined; - const querySummary = queryText ? getSqlQuerySummary(queryText) : undefined; + const { queryText, querySummary } = sanitizeSqlQueryWithSummary(sql, 'mysql'); const client = getClient(); const name = diff --git a/packages/server-utils/src/integrations/mysql2/index.ts b/packages/server-utils/src/integrations/mysql2/index.ts index 2d9342878049..751a3ae9c2c3 100644 --- a/packages/server-utils/src/integrations/mysql2/index.ts +++ b/packages/server-utils/src/integrations/mysql2/index.ts @@ -9,7 +9,7 @@ import { startInactiveSpan, waitForTracingChannelBinding, } from '@sentry/core'; -import { getSqlQuerySummary, sanitizeSqlQuery } from '../../utils/sql'; +import { sanitizeSqlQueryWithSummary } from '../../utils/sql'; import { subscribeMysql2DiagnosticChannels } from './mysql2-dc-subscriber'; import type { ChannelName } from '../../orchestrion/channels'; import { CHANNELS } from '../../orchestrion/channels'; @@ -84,8 +84,7 @@ function subscribeQueryChannel(channelName: ChannelName): void { data => { const statement = getQueryText(data.arguments); const connectionAttributes = getConnectionAttributes(data.self?.config); - const queryText = statement ? sanitizeSqlQuery(statement, 'mysql') : undefined; - const querySummary = queryText ? getSqlQuerySummary(queryText) : undefined; + const { queryText, querySummary } = sanitizeSqlQueryWithSummary(statement, 'mysql'); const client = getClient(); const name = diff --git a/packages/server-utils/src/integrations/mysql2/mysql2-dc-subscriber.ts b/packages/server-utils/src/integrations/mysql2/mysql2-dc-subscriber.ts index 9920ac7850a5..71264c73565f 100644 --- a/packages/server-utils/src/integrations/mysql2/mysql2-dc-subscriber.ts +++ b/packages/server-utils/src/integrations/mysql2/mysql2-dc-subscriber.ts @@ -12,7 +12,7 @@ import { import { DB } from '@sentry/conventions/op'; import { getClient, hasSpanStreamingEnabled, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core'; import { bindTracingChannelToSpan } from '../../tracing-channel'; -import { getSqlQuerySummary, sanitizeSqlQuery } from '../../utils/sql'; +import { sanitizeSqlQueryWithSummary } from '../../utils/sql'; // Channel names published by mysql2 >= 3.20.0 (see mysql2 `lib/tracing.js`). // Hardcoded so the subscriber does not have to import mysql2 — the channels @@ -97,9 +97,8 @@ function setupQueryChannel(tracingChannel: MySQL2TracingChannelFactory, channelN // mysql2 does not sanitize its channel payload, so the statement may carry // raw user values (on the `query` channel they are inlined). Strip every // literal before it leaves the process; `values` is never attached. - const queryText = data.query ? sanitizeSqlQuery(data.query, 'mysql') : undefined; + const { queryText, querySummary } = sanitizeSqlQueryWithSummary(data.query, 'mysql'); const operation = queryText?.match(SQL_OPERATION_RE)?.[1]?.toUpperCase(); - const querySummary = getSqlQuerySummary(queryText); const client = getClient(); const name = diff --git a/packages/server-utils/src/integrations/postgres.ts b/packages/server-utils/src/integrations/postgres.ts index 57b3350e454c..350e7d5d41a2 100644 --- a/packages/server-utils/src/integrations/postgres.ts +++ b/packages/server-utils/src/integrations/postgres.ts @@ -22,7 +22,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan, } from '@sentry/core'; -import { getSqlQuerySummary, sanitizeSqlQuery } from '../utils/sql'; +import { sanitizeSqlQueryWithSummary } from '../utils/sql'; import { CHANNELS } from '../orchestrion/channels'; import { bindTracingChannelToSpan } from '../tracing-channel'; import { pgModuleNames } from '../orchestrion/config/pg'; @@ -180,8 +180,7 @@ function querySpanOptions(ctx: PgChannelContext): { name: string; attributes: Sp const params = (ctx.self as { connectionParameters?: PgConnectionParams } | undefined)?.connectionParameters ?? {}; const queryConfig = extractQueryConfig(ctx.arguments); const client = getClient(); - const queryText = queryConfig?.text ? sanitizeSqlQuery(queryConfig.text) : undefined; - const querySummary = queryText ? getSqlQuerySummary(queryText) : undefined; + const { queryText, querySummary } = sanitizeSqlQueryWithSummary(queryConfig?.text); const name = client && hasSpanStreamingEnabled(client) ? querySummary || params.database || DB_SYSTEM_POSTGRESQL diff --git a/packages/server-utils/src/utils/sql.ts b/packages/server-utils/src/utils/sql.ts index f9f2c02600f1..2ae8ef4450ad 100644 --- a/packages/server-utils/src/utils/sql.ts +++ b/packages/server-utils/src/utils/sql.ts @@ -299,3 +299,16 @@ export function sanitizeSqlQuery(sqlQuery: string | undefined, dialect: SqlDiale .replace(/\bIN\b\s*\(\s*\$\d+(?:\s*,\s*\$\d+)*\s*\)/gi, 'IN ($?)') ); } + +/** + * Sanitizes a collected SQL statement and derives the matching `db.query.summary`, the pair the SQL + * integrations attach to their spans. Both come back `undefined` when there is no statement, so an + * empty query omits the attributes instead of reporting the sanitizer's fallback text. + */ +export function sanitizeSqlQueryWithSummary( + sqlQuery: string | undefined, + dialect?: SqlDialect, +): { queryText: string | undefined; querySummary: string | undefined } { + const queryText = sqlQuery ? sanitizeSqlQuery(sqlQuery, dialect) : undefined; + return { queryText, querySummary: getSqlQuerySummary(queryText) }; +} diff --git a/packages/server-utils/test/utils/sql.test.ts b/packages/server-utils/test/utils/sql.test.ts index a4e20d993c22..46bbe2560002 100644 --- a/packages/server-utils/test/utils/sql.test.ts +++ b/packages/server-utils/test/utils/sql.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { getSqlQuerySummary, sanitizeSqlQuery } from '../../src/utils/sql'; +import { getSqlQuerySummary, sanitizeSqlQuery, sanitizeSqlQueryWithSummary } from '../../src/utils/sql'; describe('getSqlQuerySummary', () => { it.each([undefined, ''])('returns undefined for %j', input => { @@ -632,3 +632,24 @@ 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 }); + }); +});