diff --git a/.size-limit.js b/.size-limit.js index 48466c55abb3..3786d07b38ac 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -480,7 +480,7 @@ module.exports = [ ignore: [...builtinModules, ...nodePrefixedBuiltinModules], gzip: false, brotli: false, - limit: '490 KiB', + limit: '492 KiB', disablePlugins: ['@size-limit/webpack'], webpack: false, modifyEsbuildConfig: function (config) { diff --git a/dev-packages/node-integration-tests/suites/express/tracing/withError/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/withError/test.ts index 08de03b3358e..608351589310 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/withError/test.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/withError/test.ts @@ -26,5 +26,21 @@ describe('express tracing with error', () => { runner.makeRequest('get', '/test/123/abc?q=1'); await runner.completed(); }); + + test('preserves encoded query parameters while filtering sensitive values on events', async () => { + const runner = createRunner() + .ignore('transaction') + .expect({ + event: { + request: { + query_string: 'q=hello%20world&token=[Filtered]', + }, + }, + }) + .start(); + + await runner.makeRequest('get', '/test/123/abc?q=hello%20world&token=secret'); + await runner.completed(); + }); }); }); diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index 3825e199c742..a025b92be782 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -22,7 +22,13 @@ import { getAbsoluteUrl, } from '@sentry/browser'; import type { Integration, Span } from '@sentry/core'; -import { debug, parseStringToURLObject, stripUrlQueryAndFragment, timestampInSeconds } from '@sentry/core'; +import { + debug, + parseStringToURLObject, + stripUrlQueryAndFragment, + timestampInSeconds, + filterCollectedUrl, +} from '@sentry/core'; import type { Observable } from 'rxjs'; import { Subscription } from 'rxjs'; import { filter, tap } from 'rxjs/operators'; @@ -71,7 +77,7 @@ export function _updateSpanAttributesForParametrizedUrl(route: string, url: stri span.setAttributes({ [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.${op}.angular`, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [URL_FULL]: absoluteUrl, + [URL_FULL]: filterCollectedUrl(absoluteUrl), [URL_PATH]: parseStringToURLObject(absoluteUrl)?.pathname, [URL_TEMPLATE]: route, }); diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 2e833d86f6ef..133398ecf498 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -11,6 +11,8 @@ import { spanToJSON, stripUrlQueryAndFragment, winterCGRequestToRequestData, + filterCollectedUrl, + filterCollectedUrlQuery, } from '@sentry/core'; import { captureException, @@ -219,7 +221,7 @@ async function instrumentRequestStartHttpServerSpan( [SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD]: method, // This is here for backwards compatibility, we used to set this here before method, - [URL_FULL]: ctx.url.href, + [URL_FULL]: filterCollectedUrl(ctx.url.href), [URL_PATH]: ctx.url.pathname, url: stripUrlQueryAndFragment(ctx.url.href), ...httpHeadersToSpanAttributes( @@ -233,7 +235,7 @@ async function instrumentRequestStartHttpServerSpan( } if (ctx.url.search) { - attributes['http.query'] = ctx.url.search; + attributes['http.query'] = filterCollectedUrlQuery(ctx.url.search); } if (ctx.url.hash) { diff --git a/packages/aws-serverless/src/requestSpanOptions.ts b/packages/aws-serverless/src/requestSpanOptions.ts index 1cd782f83d54..023bf0b977bf 100644 --- a/packages/aws-serverless/src/requestSpanOptions.ts +++ b/packages/aws-serverless/src/requestSpanOptions.ts @@ -18,7 +18,7 @@ */ import { CLOUD_ACCOUNT_ID, FAAS_COLDSTART, URL_FULL } from '@sentry/conventions/attributes'; import type { SpanAttributes, StartSpanOptions } from '@sentry/core'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_KIND } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_KIND, filterCollectedUrl } from '@sentry/core'; import type { Context } from 'aws-lambda'; import { ATTR_FAAS_EXECUTION, ATTR_FAAS_ID } from './semconv'; @@ -62,7 +62,7 @@ function extractOtherEventFields(event: unknown): SpanAttributes { const answer: SpanAttributes = {}; const fullUrl = extractFullUrl(event as ApiGatewayLikeEvent); if (fullUrl) { - answer[URL_FULL] = fullUrl; + answer[URL_FULL] = filterCollectedUrl(fullUrl); } return answer; } diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts index 4f6f48f00cd2..2d07c4d07f1e 100644 --- a/packages/browser-utils/src/metrics/browserMetrics.ts +++ b/packages/browser-utils/src/metrics/browserMetrics.ts @@ -11,6 +11,7 @@ import { setMeasurement, spanToJSON, stringMatchesSomePattern, + filterCollectedUrl, } from '@sentry/core'; import { htmlTreeAsString } from '../htmlTreeAsString'; import { WINDOW } from '../types'; @@ -775,7 +776,7 @@ export function _addResourceSpans( attributes['url.same_origin'] = resourceUrl.includes(WINDOW.location.origin); - attributes[URL_FULL] = resourceUrl; + attributes[URL_FULL] = filterCollectedUrl(resourceUrl); _setResourceRequestAttributes(entry, attributes, [ // https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStatus diff --git a/packages/browser/src/integrations/fetchStreamPerformance.ts b/packages/browser/src/integrations/fetchStreamPerformance.ts index 6481e2783b7c..d7a935d9dba3 100644 --- a/packages/browser/src/integrations/fetchStreamPerformance.ts +++ b/packages/browser/src/integrations/fetchStreamPerformance.ts @@ -9,6 +9,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan, stripDataUrlContent, + filterCollectedUrl, } from '@sentry/core'; const responseToStreamSpan = new WeakMap(); @@ -80,7 +81,7 @@ export const fetchStreamPerformanceIntegration = defineIntegration(() => { name: `${method} ${sanitizedUrl}`, startTime: handlerData.endTimestamp, attributes: { - url: stripDataUrlContent(url), + url: filterCollectedUrl(stripDataUrlContent(url)), 'http.method': method, type: 'fetch', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client.stream', diff --git a/packages/browser/src/integrations/httpcontext.ts b/packages/browser/src/integrations/httpcontext.ts index 6e379d492967..55d135034e89 100644 --- a/packages/browser/src/integrations/httpcontext.ts +++ b/packages/browser/src/integrations/httpcontext.ts @@ -1,4 +1,4 @@ -import { defineIntegration, safeSetSpanJSONAttributes } from '@sentry/core/browser'; +import { defineIntegration, filterCollectedUrl, safeSetSpanJSONAttributes } from '@sentry/core/browser'; import { getHttpRequestData, WINDOW } from '../helpers'; import { HTTP_REQUEST_HEADER_KEY_BASE, SENTRY_OP, URL_FULL, USER_AGENT_ORIGINAL } from '@sentry/conventions/attributes'; @@ -45,7 +45,7 @@ export const httpContextIntegration = defineIntegration(() => { ...(span.is_segment && { // Coerce empty string to undefined so the helper's nullish check drops it, // rather than writing an empty `url.full` attribute onto the span. - [URL_FULL]: span.attributes?.[SENTRY_OP] !== 'http.client' ? reqData.url : undefined, + [URL_FULL]: span.attributes?.[SENTRY_OP] !== 'http.client' ? filterCollectedUrl(reqData.url) : undefined, [`${HTTP_REQUEST_HEADER_KEY_BASE}.referer`]: reqData.headers['Referer'], }), }); diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 9566c68ffbbe..90bba3fdd514 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -45,6 +45,7 @@ import { startTrackingLongTasks, } from '@sentry/browser-utils'; import { DEBUG_BUILD } from '../debug-build'; +import { filterCollectedUrl } from '@sentry/core'; import { getHttpRequestData, WINDOW } from '../helpers'; import { fetchStreamPerformanceIntegration } from '../integrations/fetchStreamPerformance'; import { WEB_VITALS_INTEGRATION_NAME, webVitalsIntegration } from '../integrations/webVitals'; @@ -431,7 +432,7 @@ export const browserTracingIntegration = ((options: Partial[0] { // Data URLs need special handling because parseStringToURLObject treats them as "relative" // (no "://"), causing getSanitizedUrlStringFromUrlObject to return just the pathname @@ -366,7 +369,7 @@ function getSpanStartOptions( const sanitizedUrl = stripDataUrlContent(url); return { name: `${method} ${sanitizedUrl}`, - attributes: getFetchSpanAttributes(url, undefined, method, spanOrigin), + attributes: getFetchSpanAttributes(url, undefined, method, spanOrigin, client), }; } @@ -374,7 +377,7 @@ function getSpanStartOptions( const sanitizedUrl = parsedUrl ? getSanitizedUrlStringFromUrlObject(parsedUrl) : url; return { name: `${method} ${sanitizedUrl}`, - attributes: getFetchSpanAttributes(url, parsedUrl, method, spanOrigin), + attributes: getFetchSpanAttributes(url, parsedUrl, method, spanOrigin, client), }; } @@ -383,9 +386,10 @@ function getFetchSpanAttributes( parsedUrl: ReturnType, method: string, spanOrigin: SpanOrigin, + client: Client | undefined, ): SpanAttributes { const attributes: SpanAttributes = { - url: stripDataUrlContent(url), + url: filterCollectedUrl(stripDataUrlContent(url), client), type: 'fetch', 'http.method': method, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin, @@ -394,12 +398,12 @@ function getFetchSpanAttributes( if (parsedUrl) { if (!isURLObjectRelative(parsedUrl)) { // oxlint-disable-next-line typescript/no-deprecated - attributes[HTTP_URL] = stripDataUrlContent(parsedUrl.href); - attributes[URL_FULL] = stripDataUrlContent(parsedUrl.href); + attributes[HTTP_URL] = filterCollectedUrl(stripDataUrlContent(parsedUrl.href), client); + attributes[URL_FULL] = filterCollectedUrl(stripDataUrlContent(parsedUrl.href), client); attributes['server.address'] = parsedUrl.host; } if (parsedUrl.search) { - attributes['http.query'] = parsedUrl.search; + attributes['http.query'] = filterCollectedUrlQuery(parsedUrl.search, client); } if (parsedUrl.hash) { attributes['http.fragment'] = parsedUrl.hash; diff --git a/packages/core/src/integrations/http/add-outgoing-request-breadcrumb.ts b/packages/core/src/integrations/http/add-outgoing-request-breadcrumb.ts index 251dbfc540a6..f815adeaca3b 100644 --- a/packages/core/src/integrations/http/add-outgoing-request-breadcrumb.ts +++ b/packages/core/src/integrations/http/add-outgoing-request-breadcrumb.ts @@ -1,5 +1,6 @@ import { addBreadcrumb } from '../../breadcrumbs'; import { getBreadcrumbLogLevelFromHttpStatusCode } from '../../utils/breadcrumb-log-level'; +import { filterCollectedUrlQuery } from '../../utils/data-collection/filterCollectedUrl'; import { getSanitizedUrlString, parseUrl } from '../../utils/url'; import { getRequestUrlFromClientRequest } from './get-request-url'; import type { HttpClientRequest, HttpIncomingMessage } from './types'; @@ -24,7 +25,7 @@ export function addOutgoingRequestBreadcrumb( status_code: statusCode, url: getSanitizedUrlString(parsedUrl), 'http.method': request.method || 'GET', - ...(parsedUrl.search ? { 'http.query': parsedUrl.search } : {}), + ...(parsedUrl.search ? { 'http.query': filterCollectedUrlQuery(parsedUrl.search) } : {}), ...(parsedUrl.hash ? { 'http.fragment': parsedUrl.hash } : {}), }, type: 'http', diff --git a/packages/core/src/integrations/http/get-outgoing-span-data.ts b/packages/core/src/integrations/http/get-outgoing-span-data.ts index 8092bec8c5bf..d41b99e8c912 100644 --- a/packages/core/src/integrations/http/get-outgoing-span-data.ts +++ b/packages/core/src/integrations/http/get-outgoing-span-data.ts @@ -1,5 +1,6 @@ import type { Span, SpanAttributes } from '../../types/span'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP } from '../../semanticAttributes'; +import { filterCollectedUrl } from '../../utils/data-collection/filterCollectedUrl'; import { getHttpSpanDetailsFromUrlObject, parseStringToURLObject } from '../../utils/url'; import type { HttpClientRequest, HttpIncomingMessage } from './types'; import { getRequestUrlFromClientRequest } from './get-request-url'; @@ -27,9 +28,9 @@ export function getOutgoingRequestSpanData(request: HttpClientRequest): StartSpa // https://getsentry.github.io/sentry-conventions/attributes/ [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.client', 'otel.kind': 'CLIENT', - 'http.url': url, + 'http.url': filterCollectedUrl(url), 'http.method': request.method, - 'http.target': request.path || '/', + 'http.target': filterCollectedUrl(request.path || '/'), 'net.peer.name': request.host, 'http.host': request.getHeader('host') as string | undefined, ...(userAgent ? { 'user_agent.original': userAgent as string } : {}), diff --git a/packages/core/src/integrations/http/server-subscription.ts b/packages/core/src/integrations/http/server-subscription.ts index 1e93166c9de4..85c047909256 100644 --- a/packages/core/src/integrations/http/server-subscription.ts +++ b/packages/core/src/integrations/http/server-subscription.ts @@ -42,6 +42,7 @@ import { SPAN_KIND } from '../../spanKind'; import type { SpanAttributes } from '../../types/span'; import type { SpanStatus } from '../../types/spanStatus'; import { HTTP_URL, URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; +import { filterCollectedUrl } from '../../utils/data-collection/filterCollectedUrl'; // Tree-shakable guard to remove all code related to tracing declare const __SENTRY_TRACING__: boolean; @@ -300,12 +301,15 @@ function buildServerSpanWrap( 'net.peer.port': remotePort, 'sentry.http.prefetch': isKnownPrefetchRequest(request) || undefined, // Old Semantic Conventions attributes for compatibility - [URL_FULL]: urlObj && !isURLObjectRelative(urlObj) ? urlObj.href : undefined, + [URL_FULL]: urlObj && !isURLObjectRelative(urlObj) ? filterCollectedUrl(urlObj.href, client) : undefined, [URL_PATH]: urlObj?.pathname ?? httpTargetWithoutQueryFragment, // oxlint-disable-next-line typescript-eslint(no-deprecated) - [HTTP_URL]: fullUrl, + [HTTP_URL]: filterCollectedUrl(fullUrl, client), 'http.method': method, - 'http.target': urlObj ? `${urlObj.pathname}${urlObj.search}` : httpTargetWithoutQueryFragment, + 'http.target': filterCollectedUrl( + urlObj ? `${urlObj.pathname}${urlObj.search}` : httpTargetWithoutQueryFragment, + client, + ), 'http.host': host, 'net.host.name': hostname, 'http.client_ip': typeof ips === 'string' ? ips.split(',')[0] : undefined, diff --git a/packages/core/src/integrations/requestdata.ts b/packages/core/src/integrations/requestdata.ts index 8c4f0690073f..ca1bd7130f14 100644 --- a/packages/core/src/integrations/requestdata.ts +++ b/packages/core/src/integrations/requestdata.ts @@ -2,12 +2,14 @@ import type { Client } from '../client'; import { getIsolationScope } from '../currentScopes'; import { defineIntegration } from '../integration'; import { SEMANTIC_ATTRIBUTE_USER_IP_ADDRESS } from '../semanticAttributes'; -import type { ResolvedDataCollection } from '../types/datacollection'; +import type { CollectBehavior, ResolvedDataCollection } from '../types/datacollection'; import type { Event } from '../types/event'; import type { IntegrationFn } from '../types/integration'; import type { QueryParams, RequestEventData } from '../types/request'; import type { StreamedSpanJSON } from '../types/span'; import { parseCookie } from '../utils/cookie'; +import { filterQueryParams } from '../utils/data-collection/filterQueryParams'; +import { filterUrlQuery } from '../utils/data-collection/filterUrlQuery'; import { httpHeadersToSpanAttributes } from '../utils/request'; import { getClientIPAddress, ipHeaderNames } from '../vendor/getIpAddress'; import { safeSetSpanJSONAttributes } from '../tracing/spans/captureSpan'; @@ -47,6 +49,7 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) = dc.httpHeaders.request === false && { httpHeaders: { ...dc.httpHeaders, request: true as const }, }), + ...(options.include?.query_string === true && dc.urlQueryParams === false && { urlQueryParams: true as const }), }; return { @@ -71,10 +74,10 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) = const { sdkProcessingMetadata = {} } = event; const { normalizedRequest, ipAddress } = sdkProcessingMetadata; - const { include } = resolveIncludeAndDataCollection(client); + const { include, dataCollection } = resolveIncludeAndDataCollection(client); if (normalizedRequest) { - addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress }, include); + addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress }, include, dataCollection); } return event; @@ -110,10 +113,19 @@ function addNormalizedRequestDataToEvent( // Data that should not go into `event.request` but is somehow related to requests additionalData: { ipAddress?: string }, include: RequestDataIncludeOptions, + dataCollection: ResolvedDataCollection, ): void { + const requestData = extractNormalizedRequestData(req, include); + if (requestData.query_string) { + requestData.query_string = normalizeAndFilterQueryString(requestData.query_string, dataCollection.urlQueryParams); + } + if (requestData.url) { + requestData.url = filterUrlQuery(requestData.url, dataCollection.urlQueryParams); + } + event.request = { ...event.request, - ...extractNormalizedRequestData(req, include), + ...requestData, }; if (include.ip) { @@ -138,7 +150,7 @@ function addNormalizedRequestDataToSpan( const attributes: Record = {}; if (requestData.url) { - attributes[URL_FULL] = requestData.url; + attributes[URL_FULL] = filterUrlQuery(requestData.url, dataCollection.urlQueryParams); } if (requestData.method) { @@ -146,7 +158,7 @@ function addNormalizedRequestDataToSpan( } if (requestData.query_string) { - attributes[URL_QUERY] = normalizeQueryString(requestData.query_string); + attributes[URL_QUERY] = normalizeAndFilterQueryString(requestData.query_string, dataCollection.urlQueryParams); } safeSetSpanJSONAttributes(span, attributes); @@ -228,13 +240,17 @@ function extractNormalizedRequestData( return requestData; } +function normalizeAndFilterQueryString(queryString: QueryParams, behavior: CollectBehavior): string | undefined { + const normalized = normalizeQueryString(queryString); + return normalized ? filterQueryParams(normalized, behavior) : undefined; +} + function normalizeQueryString(queryString: QueryParams): string | undefined { if (typeof queryString === 'string') { return queryString || undefined; } const pairs = Array.isArray(queryString) ? queryString : Object.entries(queryString); - const result = pairs.map(([key, value]) => `${key}=${value}`).join('&'); - - return result || undefined; + const normalized = new URLSearchParams(pairs).toString(); + return normalized || undefined; } diff --git a/packages/core/src/shared-exports.ts b/packages/core/src/shared-exports.ts index fd836f5299f7..4a6a1a4accd0 100644 --- a/packages/core/src/shared-exports.ts +++ b/packages/core/src/shared-exports.ts @@ -79,6 +79,7 @@ export { export { filterKeyValueData as _INTERNAL_filterKeyValueData } from './utils/data-collection/filterKeyValueData'; export { filterCookies as _INTERNAL_filterCookies } from './utils/data-collection/filterCookies'; export { filterQueryParams as _INTERNAL_filterQueryParams } from './utils/data-collection/filterQueryParams'; +export { filterCollectedUrl, filterCollectedUrlQuery } from './utils/data-collection/filterCollectedUrl'; export { envToBool } from './utils/envToBool'; export { applyScopeDataToEvent, mergeScopeData, getCombinedScopeData } from './utils/scopeData'; export { prepareEvent } from './utils/prepareEvent'; diff --git a/packages/core/src/utils/data-collection/filterCollectedUrl.ts b/packages/core/src/utils/data-collection/filterCollectedUrl.ts new file mode 100644 index 000000000000..c8f58b4751f2 --- /dev/null +++ b/packages/core/src/utils/data-collection/filterCollectedUrl.ts @@ -0,0 +1,44 @@ +import type { Client } from '../../client'; +import { getClient } from '../../currentScopes'; +import type { CollectBehavior } from '../../types/datacollection'; +import { filterQueryParams } from './filterQueryParams'; +import { filterUrlQuery } from './filterUrlQuery'; + +function urlQueryParamsBehavior(client: Client | undefined): CollectBehavior { + // Instrumentation can run before a client exists; the denylist default is the safe fallback. + return (client ?? getClient())?.getDataCollectionOptions().urlQueryParams ?? true; +} + +/** + * Applies `dataCollection.urlQueryParams` to a URL the SDK collected itself, for use as `url.full`. + * + * Call this at every site where instrumentation records a URL. Routing the SDK's own URLs through a + * helper is what makes the filtering provenance-correct: a URL a user attaches themselves never passes + * through here, and `dataCollection` is only meant to gate automatically collected data. + * + * Pass the `client` the URL belongs to whenever one is at hand — falling back to `getClient()` resolves + * against the current scope, which is the wrong client in a multi-client setup. + */ +export function filterCollectedUrl(url: string, client?: Client): string; +export function filterCollectedUrl(url: string | undefined, client?: Client): string | undefined; +export function filterCollectedUrl(url: string | undefined, client?: Client): string | undefined { + return url === undefined ? undefined : filterUrlQuery(url, urlQueryParamsBehavior(client)); +} + +/** + * Applies `dataCollection.urlQueryParams` to a query string the SDK collected itself, for use as + * `url.query`. Returns `undefined` when the query must not be collected at all. + * + * See {@link filterCollectedUrl} for why this is a helper and why passing `client` is preferred. + */ +export function filterCollectedUrlQuery(query: string | undefined, client?: Client): string | undefined { + // An empty query is passed through as-is so call sites keep emitting the same shape as before. + if (!query) { + return query; + } + + // v10 records `URL.search` verbatim, so the leading `?` has to survive filtering. + const hasPrefix = query.startsWith('?'); + const filtered = filterQueryParams(hasPrefix ? query.slice(1) : query, urlQueryParamsBehavior(client)); + return filtered && hasPrefix ? `?${filtered}` : filtered; +} diff --git a/packages/core/src/utils/data-collection/filterKeyValueData.ts b/packages/core/src/utils/data-collection/filterKeyValueData.ts index 3cc85ca8eb75..ff8012327ab5 100644 --- a/packages/core/src/utils/data-collection/filterKeyValueData.ts +++ b/packages/core/src/utils/data-collection/filterKeyValueData.ts @@ -5,6 +5,28 @@ function isSensitiveKey(lower: string, denySnippets: string[]): boolean { return denySnippets.some(snippet => lower.includes(snippet)); } +export function shouldFilterDataKey(key: string, behavior: CollectBehavior, additionalDenyTerms?: string[]): boolean { + if (behavior === false) { + return true; + } + + const lowerKey = key.toLowerCase(); + const denySnippets = + additionalDenyTerms != null ? [...SENSITIVE_KEY_SNIPPETS, ...additionalDenyTerms] : SENSITIVE_KEY_SNIPPETS; + + if (isSensitiveKey(lowerKey, denySnippets)) { + return true; + } + + if (behavior === true) { + return false; + } + + const terms = 'deny' in behavior ? behavior.deny : behavior.allow; + const matchesConfiguredTerm = terms.some(term => lowerKey.includes(term.toLowerCase())); + return 'deny' in behavior ? matchesConfiguredTerm : !matchesConfiguredTerm; +} + /** * Filters a key-value record according to a `CollectBehavior`. * @@ -22,37 +44,9 @@ export function filterKeyValueData( return {}; } - const denySnippets = - additionalDenyTerms != null ? [...SENSITIVE_KEY_SNIPPETS, ...additionalDenyTerms] : SENSITIVE_KEY_SNIPPETS; const result: Record = {}; - - if (behavior === true) { - for (const key of Object.keys(data)) { - result[key] = isSensitiveKey(key.toLowerCase(), denySnippets) ? FILTERED : data[key]!; - } - return result; - } - - if ('deny' in behavior) { - const lowerTerms = behavior.deny.map(t => t.toLowerCase()); - for (const key of Object.keys(data)) { - const lower = key.toLowerCase(); - const isDenied = isSensitiveKey(lower, denySnippets) || lowerTerms.some(term => lower.includes(term)); - result[key] = isDenied ? FILTERED : data[key]!; - } - return result; - } - - // allowList mode - const lowerTerms = behavior.allow.map(t => t.toLowerCase()); for (const key of Object.keys(data)) { - const lower = key.toLowerCase(); - if (isSensitiveKey(lower, denySnippets)) { - result[key] = FILTERED; - } else { - const isAllowed = lowerTerms.some(term => lower.includes(term)); - result[key] = isAllowed ? data[key]! : FILTERED; - } + result[key] = shouldFilterDataKey(key, behavior, additionalDenyTerms) ? FILTERED : data[key]!; } return result; } diff --git a/packages/core/src/utils/data-collection/filterQueryParams.ts b/packages/core/src/utils/data-collection/filterQueryParams.ts index 1eb61e753984..6383f758aacd 100644 --- a/packages/core/src/utils/data-collection/filterQueryParams.ts +++ b/packages/core/src/utils/data-collection/filterQueryParams.ts @@ -1,31 +1,25 @@ import type { CollectBehavior } from '../../types/datacollection'; -import { FILTERED_VALUE as FILTERED } from './filtering-snippets'; -import { filterKeyValueData } from './filterKeyValueData'; +import { FILTERED_VALUE } from './filtering-snippets'; +import { shouldFilterDataKey } from './filterKeyValueData'; /** * Filters a query parameter string according to a `CollectBehavior`. * - * When individual params can be parsed, each key-value pair is filtered - * independently. When parsing fails, the entire string is replaced with `[Filtered]`. + * Parameter names are decoded for filtering, while the original encoding, order, and duplicate keys are preserved. */ -export function filterQueryParams(queryString: string, behavior: CollectBehavior): Record | string { - if (behavior === false) { - return {}; +export function filterQueryParams(queryString: string, behavior: CollectBehavior): string | undefined { + if (!queryString || behavior === false) { + return undefined; } - try { - const params = new URLSearchParams(queryString); - const parsed: Record = {}; - params.forEach((value, key) => { - parsed[key] = value; - }); + return queryString + .split('&') + .map(pair => { + const separatorIndex = pair.indexOf('='); + const encodedKey = separatorIndex === -1 ? pair : pair.slice(0, separatorIndex); + const key = new URLSearchParams(`${encodedKey}=`).keys().next().value; - if (Object.keys(parsed).length === 0) { - return {}; - } - - return filterKeyValueData(parsed, behavior); - } catch { - return FILTERED; - } + return key !== undefined && shouldFilterDataKey(key, behavior) ? `${encodedKey}=${FILTERED_VALUE}` : pair; + }) + .join('&'); } diff --git a/packages/core/src/utils/data-collection/filterUrlQuery.ts b/packages/core/src/utils/data-collection/filterUrlQuery.ts new file mode 100644 index 000000000000..712f3e8808c3 --- /dev/null +++ b/packages/core/src/utils/data-collection/filterUrlQuery.ts @@ -0,0 +1,31 @@ +import type { CollectBehavior } from '../../types/datacollection'; +import { filterQueryParams } from './filterQueryParams'; + +/** + * Applies a `CollectBehavior` to the query string of a full URL, leaving every other URL component + * (scheme, host, path, fragment) untouched. + * + * The query is located by string offsets rather than by parsing, so the URL is returned byte-for-byte + * apart from the query itself. This keeps relative URLs, non-HTTP schemes and unusual encodings intact, + * none of which survive a `URL` round-trip. + * + * Returns the URL with its query filtered, or with the query removed entirely when collection is off. + */ +export function filterUrlQuery(url: string, behavior: CollectBehavior): string { + // The fragment is delimited first: a `?` after a `#` belongs to the fragment, not the query. + const fragmentStart = url.indexOf('#'); + const queryEnd = fragmentStart === -1 ? url.length : fragmentStart; + + const queryStart = url.indexOf('?'); + if (queryStart === -1 || queryStart > queryEnd) { + return url; + } + + const prefix = url.slice(0, queryStart); + const query = url.slice(queryStart + 1, queryEnd); + const suffix = url.slice(queryEnd); + + const filtered = filterQueryParams(query, behavior); + + return filtered ? `${prefix}?${filtered}${suffix}` : `${prefix}${suffix}`; +} diff --git a/packages/core/src/utils/url.ts b/packages/core/src/utils/url.ts index 82d85b5bdc1f..639d2c65a53a 100644 --- a/packages/core/src/utils/url.ts +++ b/packages/core/src/utils/url.ts @@ -4,7 +4,9 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, } from '../semanticAttributes'; +import type { Client } from '../client'; import type { SpanAttributes } from '../types/span'; +import { filterCollectedUrl, filterCollectedUrlQuery } from './data-collection/filterCollectedUrl'; type PartialURL = { host?: string; @@ -149,6 +151,8 @@ function getHttpSpanNameFromUrlObject( * @param spanOrigin - The origin of the span * @param request - The request object, see {@link PartialRequest} * @param routeName - The name of the route, must be low cardinality + * @param client - The client the span belongs to, used to resolve `dataCollection.urlQueryParams`. + * Falls back to the current scope's client when omitted, which is the wrong one in a multi-client setup. * @returns The span name and attributes for the HTTP operation */ export function getHttpSpanDetailsFromUrlObject( @@ -157,6 +161,7 @@ export function getHttpSpanDetailsFromUrlObject( spanOrigin: string, request?: PartialRequest, routeName?: string, + client?: Client, ): [name: string, attributes: SpanAttributes] { const attributes: SpanAttributes = { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin, @@ -175,7 +180,7 @@ export function getHttpSpanDetailsFromUrlObject( if (urlObject) { if (urlObject.search) { - attributes['url.query'] = urlObject.search; + attributes['url.query'] = filterCollectedUrlQuery(urlObject.search, client); } if (urlObject.hash) { attributes['url.fragment'] = urlObject.hash; @@ -188,7 +193,7 @@ export function getHttpSpanDetailsFromUrlObject( } if (!isURLObjectRelative(urlObject)) { - attributes[URL_FULL] = urlObject.href; + attributes[URL_FULL] = filterCollectedUrl(urlObject.href, client); if (urlObject.port) { attributes['url.port'] = urlObject.port; } diff --git a/packages/core/test/lib/integrations/http/add-outgoing-request-breadcrumb.test.ts b/packages/core/test/lib/integrations/http/add-outgoing-request-breadcrumb.test.ts index 8ed6a1f3d660..6475614f84c5 100644 --- a/packages/core/test/lib/integrations/http/add-outgoing-request-breadcrumb.test.ts +++ b/packages/core/test/lib/integrations/http/add-outgoing-request-breadcrumb.test.ts @@ -1,7 +1,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as breadcrumbsModule from '../../../../src/breadcrumbs'; +import { withScope } from '../../../../src/currentScopes'; import { addOutgoingRequestBreadcrumb } from '../../../../src/integrations/http/add-outgoing-request-breadcrumb'; import type { HttpClientRequest, HttpIncomingMessage } from '../../../../src/integrations/http/types'; +import type { CollectBehavior } from '../../../../src/types/datacollection'; +import { getDefaultTestClientOptions, TestClient } from '../../../mocks/client'; function makeMockRequest(overrides: Partial> = {}): HttpClientRequest { return { @@ -164,4 +167,44 @@ describe('addOutgoingRequestBreadcrumb', () => { const callArg = vi.mocked(breadcrumbsModule.addBreadcrumb).mock.calls[0]![0]; expect(callArg.data?.['http.method']).toBe('GET'); }); + + // Breadcrumbs never reach the span pipeline, so this is the only place `urlQueryParams` is applied to them. + describe('dataCollection.urlQueryParams', () => { + function breadcrumbQuery(path: string, urlQueryParams?: CollectBehavior): unknown { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + ...(urlQueryParams !== undefined ? { dataCollection: { urlQueryParams } } : {}), + }), + ); + + return withScope(scope => { + scope.setClient(client); + addOutgoingRequestBreadcrumb(makeMockRequest({ path }), makeMockResponse()); + + const callArg = vi.mocked(breadcrumbsModule.addBreadcrumb).mock.calls.at(-1)![0]; + return callArg.data?.['http.query']; + }); + } + + it('filters sensitive params and preserves encoding by default', () => { + expect(breadcrumbQuery('/api/test?token=abc123&q=a%20b%26c&page=5')).toBe('?token=[Filtered]&q=a%20b%26c&page=5'); + }); + + it('omits the query entirely when collection is off', () => { + expect(breadcrumbQuery('/api/test?token=abc123&page=5', false)).toBeUndefined(); + }); + + it('honors allowList mode', () => { + expect(breadcrumbQuery('/api/test?page=1&ref=x&sort=name', { allow: ['page', 'sort'] })).toBe( + '?page=1&ref=[Filtered]&sort=name', + ); + }); + + it('honors extra deny terms', () => { + expect(breadcrumbQuery('/api/test?page=1&utm_source=email', { deny: ['utm'] })).toBe( + '?page=1&utm_source=[Filtered]', + ); + }); + }); }); diff --git a/packages/core/test/lib/integrations/http/server-subscription.test.ts b/packages/core/test/lib/integrations/http/server-subscription.test.ts index e5be2e7f4e9b..136e06312905 100644 --- a/packages/core/test/lib/integrations/http/server-subscription.test.ts +++ b/packages/core/test/lib/integrations/http/server-subscription.test.ts @@ -144,6 +144,25 @@ describe('getHttpServerSubscriptions', () => { expect(transaction.contexts?.trace?.data).not.toHaveProperty(URL_FULL); }); + // `http.target` is the deprecated alias of `url.full` and carries the same query string, so it has to + // respect `dataCollection.urlQueryParams` too. + it('filters sensitive query params in `http.target` and `url.full`', async () => { + server = http.createServer((_req, res) => res.end('ok')); + await new Promise(resolve => server.listen(0, '127.0.0.1', () => resolve())); + instrument(true); + + await makeRequest('/users/42?token=abc123&foo=bar'); + const transaction = await waitForTransaction(); + + expect(transaction.contexts?.trace?.data).toEqual( + expect.objectContaining({ + 'http.target': '/users/42?token=[Filtered]&foo=bar', + [URL_FULL]: expect.stringMatching(/\/users\/42\?token=\[Filtered\]&foo=bar$/), + [URL_PATH]: '/users/42', + }), + ); + }); + it('reports a 500 status with internal_error span status', async () => { server = http.createServer((_req, res) => { res.statusCode = 500; diff --git a/packages/core/test/lib/integrations/requestdata.test.ts b/packages/core/test/lib/integrations/requestdata.test.ts index 10f12c3c3c66..fe317c5bf8e9 100644 --- a/packages/core/test/lib/integrations/requestdata.test.ts +++ b/packages/core/test/lib/integrations/requestdata.test.ts @@ -49,6 +49,42 @@ function richNormalizedRequest() { } describe('requestDataIntegration', () => { + // `event.request.url` carries the same query string as `request.query_string`, so it has to respect + // `dataCollection.urlQueryParams` too. + describe('event.request.url query params', () => { + function processWith(urlQueryParams?: DataCollection['urlQueryParams']): Event { + const integration = requestDataIntegration(); + const event = baseEvent({ + sdkProcessingMetadata: { + normalizedRequest: { + method: 'GET', + url: 'https://example.com/reset?token=secret&id=1', + query_string: 'token=secret&id=1', + }, + }, + }); + + integration.processEvent?.(event, {}, mockClient(false, { userInfo: false, urlQueryParams })); + + return event; + } + + it('filters sensitive params by default', () => { + expect(processWith().request?.url).toBe('https://example.com/reset?token=[Filtered]&id=1'); + }); + + it('strips the query entirely when collection is off', () => { + const event = processWith(false); + + expect(event.request?.url).toBe('https://example.com/reset'); + expect(event.request?.query_string).toBeUndefined(); + }); + + it('honors allowList mode', () => { + expect(processWith({ allow: ['id'] }).request?.url).toBe('https://example.com/reset?token=[Filtered]&id=1'); + }); + }); + describe('IP-related headers on event.request', () => { it('removes known IP headers from event.request.headers when userInfo is false', () => { const integration = requestDataIntegration(); @@ -428,6 +464,58 @@ describe('requestDataIntegration', () => { }); describe('include.query_string', () => { + it('omits query string when include.query_string is false and dataCollection enables query params', () => { + const integration = requestDataIntegration({ include: { query_string: false } }); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { query_string: 'page=1' }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false, { urlQueryParams: true })); + + expect(event.request?.query_string).toBeUndefined(); + }); + + it('applies the default denylist when include.query_string overrides dataCollection.urlQueryParams=false', () => { + const integration = requestDataIntegration({ include: { query_string: true } }); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { query_string: 'page=1&token=secret' }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false, { urlQueryParams: false })); + + expect(event.request?.query_string).toBe('page=1&token=[Filtered]'); + }); + + it('preserves encoded query parameter values while filtering sensitive parameters', () => { + const integration = requestDataIntegration(); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { query_string: 'q=hello%20world&token=secret' }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false)); + + expect(event.request?.query_string).toBe('q=hello%20world&token=[Filtered]'); + }); + + it('preserves the configured query allowlist when include.query_string is true', () => { + const integration = requestDataIntegration({ include: { query_string: true } }); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { query_string: 'page=1&sort=name&token=secret' }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false, { urlQueryParams: { allow: ['page'] } })); + + expect(event.request?.query_string).toBe('page=1&sort=[Filtered]&token=[Filtered]'); + }); + it('omits event.request.query_string when include.query_string is false', () => { const integration = requestDataIntegration({ include: { query_string: false } }); const event: Event = { @@ -776,6 +864,24 @@ describe('requestDataIntegration processSegmentSpan', () => { }); }); + it('filters sensitive query params in `url.full` on the segment span', () => { + const integration = requestDataIntegration(); + const span = makeSpan(); + + mockIsolationScope({ + url: 'https://example.com/api/users?token=secret&page=1', + method: 'GET', + query_string: 'token=secret&page=1', + }); + + integration.processSegmentSpan!(span, mockClient(false, { userInfo: false })); + + expect(span.attributes).toMatchObject({ + 'url.full': 'https://example.com/api/users?token=[Filtered]&page=1', + 'url.query': 'token=[Filtered]&page=1', + }); + }); + it('handles query_string in object format', () => { const integration = requestDataIntegration(); const span = makeSpan(); @@ -789,6 +895,38 @@ describe('requestDataIntegration processSegmentSpan', () => { }); }); + it('encodes query_string in object format before filtering', () => { + const integration = requestDataIntegration(); + const span = makeSpan(); + + mockIsolationScope({ query_string: { redirect: '/home?tab=one&sort=asc', token: 'secret' } }); + + integration.processSegmentSpan!(span, mockClient(false)); + + expect(span.attributes).toMatchObject({ + 'url.query': 'redirect=%2Fhome%3Ftab%3Done%26sort%3Dasc&token=[Filtered]', + }); + }); + + it('encodes query_string in tuple format and preserves duplicate keys', () => { + const integration = requestDataIntegration(); + const span = makeSpan(); + + mockIsolationScope({ + query_string: [ + ['page', 'hello world'], + ['page', 'second&value'], + ['token', 'secret'], + ], + }); + + integration.processSegmentSpan!(span, mockClient(false)); + + expect(span.attributes).toMatchObject({ + 'url.query': 'page=hello+world&page=second%26value&token=[Filtered]', + }); + }); + describe('respects include options', () => { it('excludes url when include.url is false', () => { const integration = requestDataIntegration({ include: { url: false } }); @@ -902,6 +1040,17 @@ describe('requestDataIntegration processSegmentSpan', () => { 'http.request.header.cookie.locale': 'en', }); }); + + it('filters query params when include.query_string overrides dataCollection.urlQueryParams=false on spans', () => { + const integration = requestDataIntegration({ include: { query_string: true } }); + const span = makeSpan(); + + mockIsolationScope({ query_string: 'page=1&token=secret' }); + + integration.processSegmentSpan!(span, mockClient(false, { urlQueryParams: false })); + + expect(span.attributes?.['url.query']).toBe('page=1&token=[Filtered]'); + }); }); }); diff --git a/packages/core/test/lib/tracing/spans/captureSpan.test.ts b/packages/core/test/lib/tracing/spans/captureSpan.test.ts index 28a3f0d48e0b..da9de8f49c81 100644 --- a/packages/core/test/lib/tracing/spans/captureSpan.test.ts +++ b/packages/core/test/lib/tracing/spans/captureSpan.test.ts @@ -803,4 +803,42 @@ describe('applyScopeToSegmentSpan integration', () => { expect(serializedChild?.is_segment).toBe(false); expect(serializedChild?.attributes).not.toHaveProperty('http.response.status_code'); }); + + // `dataCollection` only gates automatically collected data. URL attributes the SDK collects are + // filtered at their write sites (see `filterCollectedUrl`), so anything reaching a span here is + // either already filtered or was set by the user and must be left alone. + describe('dataCollection.urlQueryParams', () => { + function captureUserSetUrl(attributeValue: unknown, dataCollection?: object): unknown { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + tracesSampleRate: 1, + ...(dataCollection ? { dataCollection } : {}), + }), + ); + + const span = withScope(scope => { + scope.setClient(client); + const span = startInactiveSpan({ name: 'my-span' }); + span.setAttribute('url.full', attributeValue as string); + span.end(); + return span; + }); + + const attributes = captureSpan(span, client).attributes as Record | undefined; + return attributes?.['url.full']?.value; + } + + it('does not filter a `url.full` the user set themselves', () => { + expect(captureUserSetUrl('https://example.com/api?token=abc123&page=5')).toBe( + 'https://example.com/api?token=abc123&page=5', + ); + }); + + it('does not strip a user-set query even when collection is off', () => { + expect(captureUserSetUrl('https://example.com/api?token=abc123', { urlQueryParams: false })).toBe( + 'https://example.com/api?token=abc123', + ); + }); + }); }); diff --git a/packages/core/test/lib/utils/data-collection/filterCollectedUrl.test.ts b/packages/core/test/lib/utils/data-collection/filterCollectedUrl.test.ts new file mode 100644 index 000000000000..8f37cef7470e --- /dev/null +++ b/packages/core/test/lib/utils/data-collection/filterCollectedUrl.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from 'vitest'; +import { withScope } from '../../../../src/currentScopes'; +import type { CollectBehavior } from '../../../../src/types/datacollection'; +import { filterCollectedUrl, filterCollectedUrlQuery } from '../../../../src/utils/data-collection/filterCollectedUrl'; +import { getDefaultTestClientOptions, TestClient } from '../../../mocks/client'; + +function withUrlQueryParams(urlQueryParams: CollectBehavior | undefined, fn: () => T): T { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + ...(urlQueryParams !== undefined ? { dataCollection: { urlQueryParams } } : {}), + }), + ); + + return withScope(scope => { + scope.setClient(client); + return fn(); + }); +} + +describe('filterCollectedUrl', () => { + it('filters sensitive params and preserves encoding by default', () => { + const result = withUrlQueryParams(undefined, () => + filterCollectedUrl('https://example.com/api/users?token=abc123&q=a%20b%26c&page=5'), + ); + + expect(result).toBe('https://example.com/api/users?token=[Filtered]&q=a%20b%26c&page=5'); + }); + + it('strips the query entirely when collection is off', () => { + const result = withUrlQueryParams(false, () => filterCollectedUrl('https://example.com/api?token=abc&page=5')); + + expect(result).toBe('https://example.com/api'); + }); + + it('honors allowList mode', () => { + const result = withUrlQueryParams({ allow: ['page'] }, () => + filterCollectedUrl('https://example.com/s?page=1&ref=x'), + ); + + expect(result).toBe('https://example.com/s?page=1&ref=[Filtered]'); + }); + + it('leaves a URL without a query untouched', () => { + expect(withUrlQueryParams(undefined, () => filterCollectedUrl('https://example.com/api'))).toBe( + 'https://example.com/api', + ); + }); + + it('passes `undefined` through', () => { + expect(withUrlQueryParams(undefined, () => filterCollectedUrl(undefined))).toBeUndefined(); + }); + + it('falls back to the denylist when no client is set', () => { + expect(filterCollectedUrl('https://example.com/api?token=abc&page=5')).toBe( + 'https://example.com/api?token=[Filtered]&page=5', + ); + }); + + // The scope's client is the wrong one in a multi-client setup, so an explicitly passed client wins. + it('prefers the passed client over the one on the scope', () => { + const passedClient = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + dataCollection: { urlQueryParams: false }, + }), + ); + + const result = withUrlQueryParams(true, () => + filterCollectedUrl('https://example.com/api?token=abc&page=5', passedClient), + ); + + expect(result).toBe('https://example.com/api'); + }); + + it('passes the client through in `filterCollectedUrlQuery` too', () => { + const passedClient = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + dataCollection: { urlQueryParams: { allow: ['page'] } }, + }), + ); + + const result = withUrlQueryParams(true, () => filterCollectedUrlQuery('page=1&ref=x', passedClient)); + + expect(result).toBe('page=1&ref=[Filtered]'); + }); +}); + +describe('filterCollectedUrlQuery', () => { + it('filters sensitive params by default', () => { + expect(withUrlQueryParams(undefined, () => filterCollectedUrlQuery('token=abc&page=5'))).toBe( + 'token=[Filtered]&page=5', + ); + }); + + it('returns undefined when collection is off', () => { + expect(withUrlQueryParams(false, () => filterCollectedUrlQuery('token=abc'))).toBeUndefined(); + }); + + it('passes an empty or missing query through unchanged', () => { + expect(withUrlQueryParams(undefined, () => filterCollectedUrlQuery(''))).toBe(''); + expect(withUrlQueryParams(undefined, () => filterCollectedUrlQuery(undefined))).toBeUndefined(); + }); + + it('honors extra deny terms', () => { + expect(withUrlQueryParams({ deny: ['utm'] }, () => filterCollectedUrlQuery('page=1&utm_source=email'))).toBe( + 'page=1&utm_source=[Filtered]', + ); + }); +}); diff --git a/packages/core/test/lib/utils/data-collection/filterQueryParams.test.ts b/packages/core/test/lib/utils/data-collection/filterQueryParams.test.ts index fc73fdf6bee3..2dd20b4ffc27 100644 --- a/packages/core/test/lib/utils/data-collection/filterQueryParams.test.ts +++ b/packages/core/test/lib/utils/data-collection/filterQueryParams.test.ts @@ -3,8 +3,8 @@ import { filterQueryParams } from '../../../../src/utils/data-collection/filterQ describe('filterQueryParams', () => { describe('off mode (false)', () => { - it('returns empty record', () => { - expect(filterQueryParams('page=1&token=abc', false)).toEqual({}); + it('returns undefined', () => { + expect(filterQueryParams('page=1&token=abc', false)).toBeUndefined(); }); }); @@ -12,20 +12,13 @@ describe('filterQueryParams', () => { it('filters sensitive param names and preserves safe ones', () => { const result = filterQueryParams('page=1&api_key=secret&sort=name', true); - expect(result).toEqual({ - page: '1', - api_key: '[Filtered]', // matches "key" - sort: 'name', - }); + expect(result).toBe('page=1&api_key=[Filtered]&sort=name'); }); it('filters auth-related params', () => { const result = filterQueryParams('auth=abc&redirect=/home', true); - expect(result).toEqual({ - auth: '[Filtered]', // matches "auth" - redirect: '/home', - }); + expect(result).toBe('auth=[Filtered]&redirect=/home'); }); }); @@ -33,10 +26,7 @@ describe('filterQueryParams', () => { it('applies extra deny terms on top of built-in denylist', () => { const result = filterQueryParams('page=1&utm_source=email', { deny: ['utm'] }); - expect(result).toEqual({ - page: '1', - utm_source: '[Filtered]', - }); + expect(result).toBe('page=1&utm_source=[Filtered]'); }); }); @@ -46,53 +36,76 @@ describe('filterQueryParams', () => { allow: ['page', 'sort'], }); - expect(result).toEqual({ - page: '1', - token: '[Filtered]', // sensitive denylist - sort: 'name', - }); + expect(result).toBe('page=1&token=[Filtered]&sort=name'); }); it('sensitive denylist overrides allowlist', () => { const result = filterQueryParams('token=secret', { allow: ['token'] }); - expect(result).toEqual({ - token: '[Filtered]', // "token" matches sensitive denylist - }); + // "token" matches sensitive denylist + expect(result).toBe('token=[Filtered]'); }); }); describe('empty input', () => { - it('returns empty record for empty string', () => { - expect(filterQueryParams('', true)).toEqual({}); + it('returns undefined for empty string', () => { + expect(filterQueryParams('', true)).toBeUndefined(); }); }); describe('edge cases', () => { - it('handles URL-encoded values', () => { + it('preserves URL-encoded values', () => { const result = filterQueryParams('name=hello%20world&page=1', true); - expect(result).toEqual({ - name: 'hello world', - page: '1', - }); + expect(result).toBe('name=hello%20world&page=1'); + }); + + it('preserves plus-encoded spaces', () => { + const result = filterQueryParams('name=hello+world&page=1', true); + + expect(result).toBe('name=hello+world&page=1'); + }); + + it('filters URL-encoded sensitive param names', () => { + const result = filterQueryParams('to%6Ben=secret&page=1', true); + + expect(result).toBe('to%6Ben=[Filtered]&page=1'); + }); + + it('filters empty param names in allowlist mode', () => { + const result = filterQueryParams('=secret&page=1', { allow: ['page'] }); + + expect(result).toBe('=[Filtered]&page=1'); }); - it('handles params with no value', () => { + it('preserves empty param names in denylist mode', () => { + const result = filterQueryParams('=secret&page=1', { deny: [] }); + + expect(result).toBe('=secret&page=1'); + }); + + it('preserves params with no value', () => { const result = filterQueryParams('debug&page=1', true); - expect(result).toEqual({ - debug: '', - page: '1', - }); + expect(result).toBe('debug&page=1'); }); - it('handles duplicate params (last value wins via URLSearchParams)', () => { - const result = filterQueryParams('page=1&page=2', true); + it('filters sensitive params with no value', () => { + const result = filterQueryParams('debug&token&page=1', true); - expect(result).toEqual({ - page: '2', - }); + expect(result).toBe('debug&token=[Filtered]&page=1'); + }); + + it('preserves duplicate params and their order', () => { + const result = filterQueryParams('page=1&page=2&token=first&token=second', true); + + expect(result).toBe('page=1&page=2&token=[Filtered]&token=[Filtered]'); + }); + + it('preserves encoded delimiters in values', () => { + const result = filterQueryParams('redirect=%2Fhome%3Ftab%3Done%26sort%3Dasc&token=a%26b', true); + + expect(result).toBe('redirect=%2Fhome%3Ftab%3Done%26sort%3Dasc&token=[Filtered]'); }); }); }); diff --git a/packages/core/test/lib/utils/data-collection/filterUrlQuery.test.ts b/packages/core/test/lib/utils/data-collection/filterUrlQuery.test.ts new file mode 100644 index 000000000000..92be879fff90 --- /dev/null +++ b/packages/core/test/lib/utils/data-collection/filterUrlQuery.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from 'vitest'; +import { filterUrlQuery } from '../../../../src/utils/data-collection/filterUrlQuery'; + +describe('filterUrlQuery', () => { + describe('no query string', () => { + it('returns the URL unchanged', () => { + expect(filterUrlQuery('https://example.com/api/users', true)).toBe('https://example.com/api/users'); + }); + + it('returns a URL with only a fragment unchanged', () => { + expect(filterUrlQuery('https://example.com/docs#section', true)).toBe('https://example.com/docs#section'); + }); + + it('leaves a trailing `?` with no params alone', () => { + expect(filterUrlQuery('https://example.com/api?', true)).toBe('https://example.com/api'); + }); + }); + + describe('denyList mode (true)', () => { + it('filters sensitive params and preserves the rest', () => { + const result = filterUrlQuery('https://example.com/api/users?token=abc123&q=a%20b%26c&page=5', true); + + expect(result).toBe('https://example.com/api/users?token=[Filtered]&q=a%20b%26c&page=5'); + }); + + it('preserves the fragment', () => { + const result = filterUrlQuery('https://example.com/api?token=abc&page=5#results', true); + + expect(result).toBe('https://example.com/api?token=[Filtered]&page=5#results'); + }); + + it('preserves userinfo, port and path', () => { + const result = filterUrlQuery('https://user:pw@example.com:8443/a/b?secret=x&ok=1', true); + + expect(result).toBe('https://user:pw@example.com:8443/a/b?secret=[Filtered]&ok=1'); + }); + }); + + describe('off mode (false)', () => { + it('removes the query entirely', () => { + expect(filterUrlQuery('https://example.com/api/users?token=abc&page=5', false)).toBe( + 'https://example.com/api/users', + ); + }); + + it('removes the query but keeps the fragment', () => { + expect(filterUrlQuery('https://example.com/api?token=abc#results', false)).toBe( + 'https://example.com/api#results', + ); + }); + }); + + describe('allow / deny behaviors', () => { + it('supports allowList mode', () => { + const result = filterUrlQuery('https://example.com/s?page=1&ref=x&sort=name', { allow: ['page', 'sort'] }); + + expect(result).toBe('https://example.com/s?page=1&ref=[Filtered]&sort=name'); + }); + + it('supports extra deny terms', () => { + const result = filterUrlQuery('https://example.com/s?page=1&utm_source=email', { deny: ['utm'] }); + + expect(result).toBe('https://example.com/s?page=1&utm_source=[Filtered]'); + }); + }); + + describe('non-standard URLs', () => { + it('handles relative URLs', () => { + expect(filterUrlQuery('/api/users?token=abc&page=5', true)).toBe('/api/users?token=[Filtered]&page=5'); + }); + + it('preserves duplicate params and their order', () => { + const result = filterUrlQuery('https://example.com/s?page=1&token=a&page=2', true); + + expect(result).toBe('https://example.com/s?page=1&token=[Filtered]&page=2'); + }); + + it('does not treat a `?` inside a fragment as a query', () => { + const result = filterUrlQuery('https://example.com/docs#/route?token=abc', true); + + expect(result).toBe('https://example.com/docs#/route?token=abc'); + }); + }); +}); diff --git a/packages/deno/src/wrap-deno-request-handler.ts b/packages/deno/src/wrap-deno-request-handler.ts index f6b9ccb7de78..abeb5e71c9cc 100644 --- a/packages/deno/src/wrap-deno-request-handler.ts +++ b/packages/deno/src/wrap-deno-request-handler.ts @@ -57,7 +57,14 @@ export const wrapDenoRequestHandler = ( } const urlObject = parseStringToURLObject(request.url); - const [name, attributes] = getHttpSpanDetailsFromUrlObject(urlObject, 'server', 'auto.http.deno', request); + const [name, attributes] = getHttpSpanDetailsFromUrlObject( + urlObject, + 'server', + 'auto.http.deno', + request, + undefined, + client, + ); const contentLength = request.headers.get('content-length'); assignIfSet(attributes, 'http.request.body.size', contentLength && parseInt(contentLength, 10)); diff --git a/packages/elysia/src/withElysia.ts b/packages/elysia/src/withElysia.ts index 1258f193adeb..4302ace63783 100644 --- a/packages/elysia/src/withElysia.ts +++ b/packages/elysia/src/withElysia.ts @@ -16,6 +16,7 @@ import { updateSpanName, winterCGRequestToRequestData, withIsolationScope, + filterCollectedUrl, } from '@sentry/core'; import type { AnyElysia, Elysia, ErrorContext, TraceHandler, TraceListener } from 'elysia'; @@ -205,7 +206,7 @@ export function withElysia(app: T, options: ElysiaHandlerOp attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ELYSIA_ORIGIN, [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', - [URL_FULL]: request.url, + [URL_FULL]: filterCollectedUrl(request.url), [URL_PATH]: new URL(request.url).pathname, }, }, diff --git a/packages/ember/addon/utils/instrumentEmberAppInstanceForPerformance.ts b/packages/ember/addon/utils/instrumentEmberAppInstanceForPerformance.ts index 826912a46c22..940f26e26eed 100644 --- a/packages/ember/addon/utils/instrumentEmberAppInstanceForPerformance.ts +++ b/packages/ember/addon/utils/instrumentEmberAppInstanceForPerformance.ts @@ -12,6 +12,7 @@ import { startInactiveSpan, } from '@sentry/browser'; import type { Client, Span } from '@sentry/core'; +import { filterCollectedUrl } from '@sentry/core'; import type { EmberRouterMain } from '../types'; import { getBackburner } from './performance'; import { URL_FULL, URL_PATH, URL_TEMPLATE } from '@sentry/conventions/attributes'; @@ -110,7 +111,7 @@ export function _getRouteUrlAttributes( // fragment (e.g. `https://host/#/tracing`), which would otherwise be lost by `getUrlPathFromEmberLocation`. return { [URL_PATH]: path, - [URL_FULL]: getAbsoluteUrl(fullUrl), + [URL_FULL]: filterCollectedUrl(getAbsoluteUrl(fullUrl)), [URL_TEMPLATE]: buildUrlTemplate(path, params), }; } diff --git a/packages/google-cloud-serverless/src/integrations/google-cloud-http.ts b/packages/google-cloud-serverless/src/integrations/google-cloud-http.ts index d3e6fe11655f..ee5910fdd248 100644 --- a/packages/google-cloud-serverless/src/integrations/google-cloud-http.ts +++ b/packages/google-cloud-serverless/src/integrations/google-cloud-http.ts @@ -6,6 +6,7 @@ import { getClient, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SentryNonRecordingSpan, + stripUrlQueryAndFragment, } from '@sentry/core'; import { startInactiveSpan } from '@sentry/node'; @@ -52,7 +53,8 @@ function wrapRequestFunction(orig: RequestFunction): RequestFunction { const httpMethod = reqOpts.method || 'GET'; const span = SETUP_CLIENTS.has(getClient() as Client) ? startInactiveSpan({ - name: `${httpMethod} ${reqOpts.uri}`, + // Span names must not contain a query string, and callers can pass any URI they want. + name: `${httpMethod} ${stripUrlQueryAndFragment(reqOpts.uri)}`, onlyIfParent: true, op: `http.client.${identifyService(this.apiEndpoint)}`, attributes: { diff --git a/packages/google-cloud-serverless/test/integrations/google-cloud-http.test.ts b/packages/google-cloud-serverless/test/integrations/google-cloud-http.test.ts index f569a6980158..33b2450ad5e8 100644 --- a/packages/google-cloud-serverless/test/integrations/google-cloud-http.test.ts +++ b/packages/google-cloud-serverless/test/integrations/google-cloud-http.test.ts @@ -92,5 +92,25 @@ describe('GoogleCloudHttp tracing', () => { }, }); }); + + // Span names follow `METHOD scheme://host/path`, so a query string must never reach the name, + // whatever the caller passes as `uri`. + test('strips the query string from the span name', async () => { + nock('https://bigquery.googleapis.com') + .get('/bigquery/v2/projects/project-id/datasets') + .query(true) + .reply(200, '{}'); + + await new Promise((resolve, reject) => { + (bigquery as unknown as { request: (o: unknown, cb: (e: unknown) => void) => void }).request( + { uri: '/datasets?key=SECRET_TOKEN_VALUE&alt=json', method: 'GET' }, + (err: unknown) => (err ? reject(err) : resolve()), + ); + }); + + expect(mockStartInactiveSpan).toBeCalledWith(expect.objectContaining({ name: 'GET /datasets' })); + const names = mockStartInactiveSpan.mock.calls.map(([args]) => (args as { name: string }).name); + expect(names.join('\n')).not.toContain('SECRET_TOKEN_VALUE'); + }); }); }); diff --git a/packages/nestjs/src/integrations/wrap-route.ts b/packages/nestjs/src/integrations/wrap-route.ts index c910e658fcf4..d4fa84917faf 100644 --- a/packages/nestjs/src/integrations/wrap-route.ts +++ b/packages/nestjs/src/integrations/wrap-route.ts @@ -1,6 +1,6 @@ import { HTTP_ROUTE } from '@sentry/conventions/attributes'; import type { SpanAttributes } from '@sentry/core'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan, filterCollectedUrl } from '@sentry/core'; import type { AnyFn } from './helpers'; import { copyReflectMetadata, httpOrigin, isWrapped, markWrapped } from './helpers'; import { AttributeNames, NestType } from './vendored/enums'; @@ -96,7 +96,7 @@ export function wrapRequestContextHandler( [AttributeNames.VERSION]: moduleVersion || undefined, [HTTP_ROUTE]: httpRoute || undefined, ['http.method']: req.method || undefined, - ['http.url']: req.originalUrl || req.url || undefined, + ['http.url']: filterCollectedUrl(req.originalUrl || req.url || undefined), }; return startSpan({ name: spanName, op: `${NestType.REQUEST_CONTEXT}.nestjs`, attributes }, () => handler.apply(this, handlerArgs), diff --git a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts index 57bc248410b6..8ea906b06f30 100644 --- a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts @@ -5,6 +5,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + filterCollectedUrl, } from '@sentry/core'; import { startBrowserTracingNavigationSpan, @@ -26,7 +27,7 @@ function stripTrailingSlash(pathname: string): string { function setNavigationSpanUrlAttributes(span: Span, urlPath: string, urlOrPath: string): void { span.setAttributes({ [URL_PATH]: urlPath, - [URL_FULL]: getAbsoluteUrl(urlOrPath), + [URL_FULL]: filterCollectedUrl(getAbsoluteUrl(urlOrPath)), }); } diff --git a/packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts index 1785789483ed..8b8910fbf090 100644 --- a/packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts @@ -1,4 +1,5 @@ import { + filterCollectedUrl, captureException, continueTrace, debug, @@ -92,7 +93,8 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs', - [URL_FULL]: urlObject && !isURLObjectRelative(urlObject) ? urlObject.href : undefined, + [URL_FULL]: + urlObject && !isURLObjectRelative(urlObject) ? filterCollectedUrl(urlObject.href) : undefined, [URL_PATH]: urlObject?.pathname, [HTTP_ROUTE]: parameterizedRoute, }, diff --git a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts index 9429a7c4b1a0..aec66e8d2379 100644 --- a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts @@ -1,4 +1,5 @@ import { + filterCollectedUrl, captureException, getActiveSpan, getCurrentScope, @@ -55,7 +56,7 @@ export function wrapApiHandlerWithSentry( const urlObject = req instanceof Request ? parseStringToURLObject(req.url) : undefined; const urlAttributes = { - [URL_FULL]: urlObject && !isURLObjectRelative(urlObject) ? urlObject.href : undefined, + [URL_FULL]: urlObject && !isURLObjectRelative(urlObject) ? filterCollectedUrl(urlObject.href) : undefined, [URL_PATH]: urlObject?.pathname, }; diff --git a/packages/node-core/src/integrations/http/httpServerSpansIntegration.ts b/packages/node-core/src/integrations/http/httpServerSpansIntegration.ts index fdf350214f9b..34fc2e4cfd6c 100644 --- a/packages/node-core/src/integrations/http/httpServerSpansIntegration.ts +++ b/packages/node-core/src/integrations/http/httpServerSpansIntegration.ts @@ -53,6 +53,7 @@ import { startInactiveSpan, withActiveSpan, SPAN_KIND, + filterCollectedUrl, } from '@sentry/core'; import { DEBUG_BUILD } from '../../debug-build'; import type { NodeClient } from '../../sdk/client'; @@ -175,13 +176,16 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.http', [SENTRY_HTTP_PREFETCH]: isKnownPrefetchRequest(request) || undefined, - [URL_FULL]: urlObj && !isURLObjectRelative(urlObj) ? urlObj.href : undefined, + [URL_FULL]: urlObj && !isURLObjectRelative(urlObj) ? filterCollectedUrl(urlObj.href, client) : undefined, [URL_PATH]: urlObj?.pathname ?? httpTargetWithoutQueryFragment, // Old Semantic Conventions attributes - added for compatibility with what `@opentelemetry/instrumentation-http` output before /* eslint-disable typescript/no-deprecated */ - [HTTP_URL]: fullUrl, + [HTTP_URL]: filterCollectedUrl(fullUrl, client), [HTTP_METHOD]: normalizedRequest.method, - [HTTP_TARGET]: urlObj ? `${urlObj.pathname}${urlObj.search}` : httpTargetWithoutQueryFragment, + [HTTP_TARGET]: filterCollectedUrl( + urlObj ? `${urlObj.pathname}${urlObj.search}` : httpTargetWithoutQueryFragment, + client, + ), [HTTP_HOST]: host, [NET_HOST_NAME]: hostname, [HTTP_CLIENT_IP]: typeof ips === 'string' ? ips.split(',')[0] : undefined, diff --git a/packages/node-core/src/integrations/node-fetch/undici-instrumentation.ts b/packages/node-core/src/integrations/node-fetch/undici-instrumentation.ts index 89a754b36274..2a364ad31727 100644 --- a/packages/node-core/src/integrations/node-fetch/undici-instrumentation.ts +++ b/packages/node-core/src/integrations/node-fetch/undici-instrumentation.ts @@ -33,6 +33,8 @@ import { SPAN_STATUS_ERROR, startInactiveSpan, stripDataUrlContent, + filterCollectedUrl, + filterCollectedUrlQuery, } from '@sentry/core'; import { addFetchRequestBreadcrumb, addTracePropagationHeadersToFetchRequest } from '../../utils/outgoingFetchRequest'; import { @@ -220,9 +222,9 @@ function onRequestCreated(config: NodeFetchOptions, { request }: RequestMessage) const attributes: SpanAttributes = { [HTTP_REQUEST_METHOD]: requestMethod, [ATTR_HTTP_REQUEST_METHOD_ORIGINAL]: request.method, - [URL_FULL]: requestUrl.toString(), + [URL_FULL]: filterCollectedUrl(requestUrl.toString()), [URL_PATH]: requestUrl.pathname, - [URL_QUERY]: requestUrl.search, + [URL_QUERY]: filterCollectedUrlQuery(requestUrl.search), [URL_SCHEME]: urlScheme, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.node_fetch', }; diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index a94779633f43..75a89e42c00e 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -1,5 +1,6 @@ import type { LRUMap, SanitizedRequestData, Span } from '@sentry/core'; import { + filterCollectedUrlQuery, addBreadcrumb, getActiveSpan, getBreadcrumbLogLevelFromHttpStatusCode, @@ -258,7 +259,7 @@ function getBreadcrumbData(request: UndiciRequest): Partial { + const actual = (await vi.importActual('@sentry/core')) as Record; + return { ...actual, addBreadcrumb: vi.fn() }; +}); + +function makeRequest(path: string): UndiciRequest { + return { method: 'GET', origin: 'https://example.com', path, headers: {} } as unknown as UndiciRequest; +} + +const RESPONSE = { statusCode: 200 } as unknown as UndiciResponse; + +/** + * Breadcrumbs never reach the span pipeline, so `addFetchRequestBreadcrumb` is the only place + * `dataCollection.urlQueryParams` is applied to outgoing fetch breadcrumbs. + */ +describe('addFetchRequestBreadcrumb', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + function breadcrumbQuery(path: string, urlQueryParams?: CollectBehavior): unknown { + const client = new NodeClient( + getDefaultNodeClientOptions(urlQueryParams !== undefined ? { dataCollection: { urlQueryParams } } : {}), + ); + + return withScope(scope => { + scope.setClient(client); + getCurrentScope().setClient(client); + addFetchRequestBreadcrumb(makeRequest(path), RESPONSE); + + const callArg = vi.mocked(addBreadcrumb).mock.calls.at(-1)![0]; + return callArg.data?.['http.query']; + }); + } + + it('filters sensitive params and preserves encoding by default', () => { + expect(breadcrumbQuery('/api?token=abc123&q=a%20b%26c&page=5')).toBe('?token=[Filtered]&q=a%20b%26c&page=5'); + }); + + it('omits the query entirely when collection is off', () => { + expect(breadcrumbQuery('/api?token=abc123&page=5', false)).toBeUndefined(); + }); + + it('honors allowList mode', () => { + expect(breadcrumbQuery('/api?page=1&ref=x&sort=name', { allow: ['page', 'sort'] })).toBe( + '?page=1&ref=[Filtered]&sort=name', + ); + }); + + it('honors extra deny terms', () => { + expect(breadcrumbQuery('/api?page=1&utm_source=email', { deny: ['utm'] })).toBe('?page=1&utm_source=[Filtered]'); + }); +}); diff --git a/packages/opentelemetry/src/utils/getRequestSpanData.ts b/packages/opentelemetry/src/utils/getRequestSpanData.ts index e94e8fb2a362..6ed00520c449 100644 --- a/packages/opentelemetry/src/utils/getRequestSpanData.ts +++ b/packages/opentelemetry/src/utils/getRequestSpanData.ts @@ -2,7 +2,7 @@ import type { Span } from '@opentelemetry/api'; import type { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import { HTTP_METHOD, HTTP_REQUEST_METHOD, HTTP_URL, URL_FULL } from '@sentry/conventions/attributes'; import type { SanitizedRequestData } from '@sentry/core'; -import { getSanitizedUrlString, parseUrl } from '@sentry/core'; +import { filterCollectedUrlQuery, getSanitizedUrlString, parseUrl } from '@sentry/core'; import { spanHasAttributes } from './spanTypes'; /** @@ -34,8 +34,10 @@ export function getRequestSpanData(span: Span | ReadableSpan): Partial = {}; if (url) { - data.url = url; + data.url = filterCollectedUrl(url); } - if (query) { + const filteredQuery = filterCollectedUrlQuery(query); + if (filteredQuery) { // Strip the leading `?`/`#` (the `URL.search`/`URL.hash` prefix) so the attribute matches the // canonical format the OTel SDK exporter emits (`getData` in `spanExporter.ts` slices these too). // TODO(v11): emit `url.query`/`url.fragment` (OTel-standard, no leading `?`/`#`) and drop // this stripping + `http.query`/`http.fragment`; `http.query` is specced to keep the leading `?`. - data['http.query'] = query.slice(1); + data['http.query'] = filteredQuery.slice(1); } if (fragment) { data['http.fragment'] = fragment.slice(1); diff --git a/packages/opentelemetry/test/utils/getRequestSpanData.test.ts b/packages/opentelemetry/test/utils/getRequestSpanData.test.ts index 6dab75fec8a3..d0771c112efb 100644 --- a/packages/opentelemetry/test/utils/getRequestSpanData.test.ts +++ b/packages/opentelemetry/test/utils/getRequestSpanData.test.ts @@ -49,6 +49,22 @@ describe('getRequestSpanData', () => { }); }); + it('filters sensitive query params according to dataCollection.urlQueryParams', () => { + const span = createSpan('test-span'); + span.setAttributes({ + [HTTP_URL]: 'http://example.com/reset?token=secret&page=1', + [HTTP_METHOD]: 'GET', + }); + + const data = getRequestSpanData(span); + + expect(data).toEqual({ + url: 'http://example.com/reset', + 'http.method': 'GET', + 'http.query': '?token=[Filtered]&page=1', + }); + }); + it('works without method', () => { const span = createSpan('test-span'); span.setAttributes({ diff --git a/packages/react-router/src/client/createClientInstrumentation.ts b/packages/react-router/src/client/createClientInstrumentation.ts index 3af6aec890f0..5fd46072a2da 100644 --- a/packages/react-router/src/client/createClientInstrumentation.ts +++ b/packages/react-router/src/client/createClientInstrumentation.ts @@ -14,6 +14,7 @@ import { SPAN_STATUS_ERROR, startSpan, updateSpanName, + filterCollectedUrl, } from '@sentry/core'; import type { ClientInstrumentation } from 'react-router'; import { DEBUG_BUILD } from '../common/debug-build'; @@ -127,7 +128,7 @@ export function createSentryClientInstrumentation( const result = await callNavigate(); if (result.status === 'error' && result.error instanceof Error) { captureInstrumentationError(result, captureErrors, 'react_router.navigate', { - 'http.url': info.currentUrl, + 'http.url': filterCollectedUrl(info.currentUrl), }); } return; @@ -174,7 +175,7 @@ export function createSentryClientInstrumentation( navigationSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); } captureInstrumentationError(result, captureErrors, 'react_router.navigate', { - 'http.url': WINDOW.location?.pathname || info.currentUrl, + 'http.url': WINDOW.location?.pathname || filterCollectedUrl(info.currentUrl), }); } } finally { @@ -210,7 +211,7 @@ export function createSentryClientInstrumentation( navigationSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); } captureInstrumentationError(result, captureErrors, 'react_router.navigate', { - 'http.url': toPath, + 'http.url': filterCollectedUrl(toPath), }); } return; @@ -230,7 +231,7 @@ export function createSentryClientInstrumentation( if (result.status === 'error' && result.error instanceof Error) { span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); captureInstrumentationError(result, captureErrors, 'react_router.fetcher', { - 'http.url': info.href, + 'http.url': filterCollectedUrl(info.href), }); } }, diff --git a/packages/react-router/src/client/utils.ts b/packages/react-router/src/client/utils.ts index 0c670f454c3d..343fcf040490 100644 --- a/packages/react-router/src/client/utils.ts +++ b/packages/react-router/src/client/utils.ts @@ -1,6 +1,6 @@ import { getAbsoluteUrl } from '@sentry/browser'; import type { Span } from '@sentry/core'; -import { GLOBAL_OBJ, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; +import { GLOBAL_OBJ, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, filterCollectedUrl } from '@sentry/core'; import { URL_FULL, URL_PATH, URL_TEMPLATE } from '@sentry/conventions/attributes'; import type { DataRouter, RouterState } from 'react-router'; @@ -105,7 +105,7 @@ export function updateNavigationSpanUrlFromLocation(span: Span): void { span.updateName(pathname); span.setAttributes({ [URL_PATH]: pathname, - [URL_FULL]: destinationUrl, + [URL_FULL]: filterCollectedUrl(destinationUrl), }); } diff --git a/packages/react-router/src/server/createServerInstrumentation.ts b/packages/react-router/src/server/createServerInstrumentation.ts index 36ba55e28a68..19b614f4fefc 100644 --- a/packages/react-router/src/server/createServerInstrumentation.ts +++ b/packages/react-router/src/server/createServerInstrumentation.ts @@ -1,6 +1,7 @@ import { context, createContextKey } from '@opentelemetry/api'; import { HTTP_REQUEST_METHOD, HTTP_ROUTE, URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; import { + filterCollectedUrl, debug, flushIfServerless, getActiveSpan, @@ -65,7 +66,7 @@ export function createSentryServerInstrumentation( [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react_router.instrumentation_api', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', - [URL_FULL]: info.request.url, + [URL_FULL]: filterCollectedUrl(info.request.url), [URL_PATH]: pathname, }); @@ -92,7 +93,7 @@ export function createSentryServerInstrumentation( [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', [HTTP_REQUEST_METHOD]: info.request.method, [URL_PATH]: pathname, - [URL_FULL]: info.request.url, + [URL_FULL]: filterCollectedUrl(info.request.url), }, }, async span => { diff --git a/packages/react-router/test/client/createClientInstrumentation.test.ts b/packages/react-router/test/client/createClientInstrumentation.test.ts index d5c44f4cf6ca..3921df3ae491 100644 --- a/packages/react-router/test/client/createClientInstrumentation.test.ts +++ b/packages/react-router/test/client/createClientInstrumentation.test.ts @@ -297,6 +297,34 @@ describe('createSentryClientInstrumentation', () => { ); }); + // `navigate('/x?token=y')` is app-supplied, so the query has to go through `dataCollection.urlQueryParams`. + it('filters sensitive query params in the `http.url` reported for a failed navigate', async () => { + const mockError = new Error('Navigate failed'); + const mockCallNavigate = vi.fn().mockResolvedValue({ status: 'error', error: mockError }); + const mockInstrument = vi.fn(); + + (core.getClient as any).mockReturnValue({}); + (globalThis as any).location = { + href: 'https://example.com/home', + origin: 'https://example.com', + pathname: '/home', + }; + + const instrumentation = createSentryClientInstrumentation(); + instrumentation.router?.({ instrument: mockInstrument }); + const hooks = mockInstrument.mock.calls[0]![0]; + + await hooks.navigate(mockCallNavigate, { currentUrl: '/home', to: '/search?token=secret&page=1' }); + + expect(core.captureException).toHaveBeenCalledWith(mockError, { + mechanism: { + type: 'react_router.navigate', + handled: false, + data: { 'http.url': '/search?token=[Filtered]&page=1' }, + }, + }); + }); + it('should capture errors when captureErrors is true (default)', async () => { const mockError = new Error('Test error'); // React Router returns an error result, not a rejection diff --git a/packages/react/src/tanstackrouter.ts b/packages/react/src/tanstackrouter.ts index de6e8623db90..6073de1688d6 100644 --- a/packages/react/src/tanstackrouter.ts +++ b/packages/react/src/tanstackrouter.ts @@ -6,6 +6,7 @@ import { WINDOW, } from '@sentry/browser'; import type { Integration } from '@sentry/core/browser'; +import { filterCollectedUrl } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, @@ -185,7 +186,7 @@ function locationToSpanUrlAttributes( return { [URL_PATH]: location.pathname, - [URL_FULL]: absoluteUrl, + [URL_FULL]: filterCollectedUrl(absoluteUrl), }; } diff --git a/packages/remix/src/server/instrumentServer.ts b/packages/remix/src/server/instrumentServer.ts index c4eb4ed7e420..b720376a9fcf 100644 --- a/packages/remix/src/server/instrumentServer.ts +++ b/packages/remix/src/server/instrumentServer.ts @@ -34,6 +34,7 @@ import { winterCGHeadersToDict, winterCGRequestToRequestData, withIsolationScope, + filterCollectedUrl, } from '@sentry/core'; import { DEBUG_BUILD } from '../utils/debug-build'; import { createRoutes, getTransactionName, isCloudflareEnv } from '../utils/utils'; @@ -133,7 +134,7 @@ function makeWrappedDocumentRequestFunction(instrumentTracing?: boolean) { onlyIfParent: true, attributes: { method: request.method, - url: request.url, + url: filterCollectedUrl(request.url), [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.remix', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.remix.document_request', }, @@ -385,7 +386,7 @@ function wrapRequestHandler ServerBuild | Promise [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.remix', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', - [URL_FULL]: url.href, + [URL_FULL]: filterCollectedUrl(url.href), [URL_PATH]: url.pathname, method: request.method, ...(source === 'route' && { diff --git a/packages/remix/src/server/integrations/tracing-channel.ts b/packages/remix/src/server/integrations/tracing-channel.ts index 04d5e4a21269..282dc902199a 100644 --- a/packages/remix/src/server/integrations/tracing-channel.ts +++ b/packages/remix/src/server/integrations/tracing-channel.ts @@ -10,6 +10,7 @@ import { SPAN_KIND, startInactiveSpan, waitForTracingChannelBinding, + filterCollectedUrl, } from '@sentry/core'; import { bindTracingChannelToSpan } from '@sentry/server-utils'; import { @@ -73,9 +74,11 @@ function getRequestAttributes(request: unknown): SpanAttributes { } if (typeof url === 'string') { // oxlint-disable-next-line typescript/no-deprecated - attributes[HTTP_URL] = url; + attributes[HTTP_URL] = filterCollectedUrl(url); const urlObject = parseStringToURLObject(url); - attributes[URL_FULL] = urlObject && !isURLObjectRelative(urlObject) ? urlObject.href : undefined; + attributes[URL_FULL] = filterCollectedUrl( + urlObject && !isURLObjectRelative(urlObject) ? urlObject.href : undefined, + ); attributes[URL_PATH] = urlObject?.pathname; } return attributes; diff --git a/packages/remix/src/vendor/instrumentation.ts b/packages/remix/src/vendor/instrumentation.ts index e12253b9a380..73fb9c257841 100644 --- a/packages/remix/src/vendor/instrumentation.ts +++ b/packages/remix/src/vendor/instrumentation.ts @@ -28,7 +28,7 @@ import type * as remixRunServerRuntimeData from '@remix-run/server-runtime/dist/ import type * as remixRunServerRuntimeRouteMatching from '@remix-run/server-runtime/dist/routeMatching'; import type { RouteMatch } from '@remix-run/server-runtime/dist/routeMatching'; import type { ServerRoute } from '@remix-run/server-runtime/dist/routes'; -import { SDK_VERSION } from '@sentry/core'; +import { filterCollectedUrl, SDK_VERSION } from '@sentry/core'; const RemixSemanticAttributes = { MATCH_PARAMS: 'match.params', @@ -334,7 +334,7 @@ export class RemixInstrumentation extends InstrumentationBase { const addRequestAttributesToSpan = (span: Span, request: Request): void => { span.setAttributes({ [HTTP_METHOD]: request.method, - [HTTP_URL]: request.url, + [HTTP_URL]: filterCollectedUrl(request.url), }); }; diff --git a/packages/solid/src/solidrouter.ts b/packages/solid/src/solidrouter.ts index 6041253a2d28..1cbf5ff382a9 100644 --- a/packages/solid/src/solidrouter.ts +++ b/packages/solid/src/solidrouter.ts @@ -19,6 +19,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + filterCollectedUrl, } from '@sentry/core'; import type { BeforeLeaveEventArgs, @@ -40,7 +41,7 @@ function locationToSpanUrlAttributes(pathname: string, search: string = '', hash return { [URL_PATH]: pathname, - [URL_FULL]: getAbsoluteUrl(pathWithSearch), + [URL_FULL]: filterCollectedUrl(getAbsoluteUrl(pathWithSearch)), }; } diff --git a/packages/solid/src/tanstackrouter.ts b/packages/solid/src/tanstackrouter.ts index bc03ba841ff6..caee2291d190 100644 --- a/packages/solid/src/tanstackrouter.ts +++ b/packages/solid/src/tanstackrouter.ts @@ -17,6 +17,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + filterCollectedUrl, } from '@sentry/core'; import type { AnyRouter } from '@tanstack/solid-router'; @@ -177,7 +178,7 @@ function locationToSpanUrlAttributes(router: AnyRouter, location: TanstackRouter return { [URL_PATH]: location.pathname, - [URL_FULL]: absoluteUrl, + [URL_FULL]: filterCollectedUrl(absoluteUrl), }; } diff --git a/packages/sveltekit/src/server-common/handle.ts b/packages/sveltekit/src/server-common/handle.ts index 20965b8e6ddb..cf36c16e1002 100644 --- a/packages/sveltekit/src/server-common/handle.ts +++ b/packages/sveltekit/src/server-common/handle.ts @@ -19,6 +19,7 @@ import { winterCGHeadersToDict, winterCGRequestToRequestData, withIsolationScope, + filterCollectedUrl, } from '@sentry/core'; import type { Handle, ResolveOptions } from '@sveltejs/kit'; import { DEBUG_BUILD } from '../common/debug-build'; @@ -180,8 +181,10 @@ async function instrumentHandle( [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: routeName ? 'route' : 'url', 'sveltekit.tracing.original_name': originalName, - // oxlint-disable-next-line typescript-eslint(no-deprecated) - [URL_FULL]: kitRootSpanAttributes[URL_FULL] ?? kitRootSpanAttributes[HTTP_URL] ?? event.url.href, + [URL_FULL]: filterCollectedUrl( + // oxlint-disable-next-line typescript-eslint(no-deprecated) + (kitRootSpanAttributes[URL_FULL] ?? kitRootSpanAttributes[HTTP_URL] ?? event.url.href) as string, + ), [URL_PATH]: kitRootSpanAttributes[URL_PATH] ?? event.url.pathname, ...(routeName && { [HTTP_ROUTE]: routeName, @@ -215,7 +218,7 @@ async function instrumentHandle( [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: routeId ? 'route' : 'url', 'http.method': event.request.method, - [URL_FULL]: event.url.href, + [URL_FULL]: filterCollectedUrl(event.url.href), [URL_PATH]: event.url.pathname, ...(routeId && { [HTTP_ROUTE]: routeId, diff --git a/packages/sveltekit/test/server-common/handle.test.ts b/packages/sveltekit/test/server-common/handle.test.ts index 025e8fd09f1e..e261d4aa676c 100644 --- a/packages/sveltekit/test/server-common/handle.test.ts +++ b/packages/sveltekit/test/server-common/handle.test.ts @@ -177,6 +177,25 @@ describe('sentryHandle', () => { kitRootSpan.end(); }); + it('filters sensitive query params in the URL taken over from the sveltekit root span', async () => { + const kitRootSpan = SentryCore.startInactiveSpan({ + name: 'sveltekit.handle.root', + attributes: { 'url.full': 'https://example.com/reset?token=secret&page=1' }, + }); + + try { + await sentryHandle()({ + event: mockEvent({ tracing: { enabled: true, root: kitRootSpan } }), + resolve: resolve(type, isError), + }); + } catch { + // + } + + expect(spanToJSON(kitRootSpan).data?.['url.full']).toEqual('https://example.com/reset?token=[Filtered]&page=1'); + kitRootSpan.end(); + }); + it('starts a child span for nested server calls (i.e. if there is an active span)', async () => { let _span: Span | undefined = undefined; let txnCount = 0; diff --git a/packages/vue/src/tanstackrouter.ts b/packages/vue/src/tanstackrouter.ts index e2508f2754df..841a625695f6 100644 --- a/packages/vue/src/tanstackrouter.ts +++ b/packages/vue/src/tanstackrouter.ts @@ -17,6 +17,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + filterCollectedUrl, } from '@sentry/core'; import type { AnyRouter } from '@tanstack/vue-router'; @@ -186,7 +187,7 @@ function locationToSpanUrlAttributes( return { [URL_PATH]: location.pathname, - [URL_FULL]: absoluteUrl, + [URL_FULL]: filterCollectedUrl(absoluteUrl), }; }