diff --git a/MIGRATION.md b/MIGRATION.md index 438b2572f682..6fec6e35e279 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -206,8 +206,17 @@ Sentry.init({ Each key-value field (`cookies`, `urlQueryParams`, `httpHeaders.request`, `httpHeaders.response`) accepts `true`, `false`, `{ allow: string[] }`, or `{ deny: string[] }` for fine-grained control. +#### RequestData + +The `requestDataIntegration`'s `include` options remain an integration-level override. An explicit `false` +prevents that category from being attached, while an explicit `true` enables it even when the corresponding +`dataCollection` category is disabled. For cookies, headers, and query parameters, any configured `allow` or +`deny` filtering continues to apply: When `include` enables a category which `dataCollection` disabled, the +default sensitive-value denylist is applied. + User IP address inference, which was previously gated on `sendDefaultPii`, is now controlled by -`dataCollection.userInfo`. +`dataCollection.userInfo`. An explicit `requestDataIntegration({ include: { ip: true } })` overrides +`dataCollection.userInfo: false` for data collected by that integration. ### Browser sessions use `unhandled` instead of `crashed` diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/init.js b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/init.js index d884ae8eb04c..43248f09b63d 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/init.js @@ -8,6 +8,4 @@ Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', integrations: [httpClientIntegration()], tracesSampleRate: 1, - // todo(v11): remove together with the sendDefaultPii guard in httpclient.ts (JS-2580) - sendDefaultPii: true, }); diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withSensitiveHeaders/test.ts b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withSensitiveHeaders/test.ts index b5eeb799381b..635fb085e586 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withSensitiveHeaders/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withSensitiveHeaders/test.ts @@ -3,51 +3,48 @@ import type { Event } from '@sentry/core'; import { sentryTest } from '../../../../../utils/fixtures'; import { envelopeRequestParser, waitForErrorRequest } from '../../../../../utils/helpers'; -sentryTest( - 'should filter sensitive header and cookie values with sendDefaultPii', - async ({ getLocalTestUrl, page }) => { - const url = await getLocalTestUrl({ testDir: __dirname }); - - await page.route('**/foo', route => { - return route.fulfill({ - status: 500, - body: JSON.stringify({ - error: { - message: 'Internal Server Error', - }, - }), - headers: { - 'Content-Type': 'text/html', - 'X-Auth-Token': 'secret-response-token', - 'X-Request-Id': 'abc-123', +sentryTest('filters sensitive header and cookie values by default', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.route('**/foo', route => { + return route.fulfill({ + status: 500, + body: JSON.stringify({ + error: { + message: 'Internal Server Error', }, - }); + }), + headers: { + 'Content-Type': 'text/html', + 'X-Auth-Token': 'secret-response-token', + 'X-Request-Id': 'abc-123', + }, }); + }); - const req = await Promise.all([waitForErrorRequest(page), page.goto(url)]).then(([r]) => r); - const eventData = envelopeRequestParser(req); + const req = await Promise.all([waitForErrorRequest(page), page.goto(url)]).then(([r]) => r); + const eventData = envelopeRequestParser(req); - expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values).toHaveLength(1); - const reqHeaders = eventData.request?.headers || {}; - const resHeaders = (eventData.contexts?.response?.headers as Record) || {}; + const reqHeaders = eventData.request?.headers || {}; + const resHeaders = (eventData.contexts?.response?.headers as Record) || {}; - // Non-sensitive request headers should be present with their values - expect(reqHeaders['accept']).toBe('application/json'); - expect(reqHeaders['content-type']).toBe('application/json'); - expect(reqHeaders['x-custom-header']).toBe('safe-value'); + // Non-sensitive request headers should be present with their values + expect(reqHeaders['accept']).toBe('application/json'); + expect(reqHeaders['content-type']).toBe('application/json'); + expect(reqHeaders['x-custom-header']).toBe('safe-value'); - // Sensitive request headers should have their values filtered - // 'authorization' matches the 'auth' snippet - expect(reqHeaders['authorization']).toBe('[Filtered]'); - // 'x-api-key' matches the 'key' snippet - expect(reqHeaders['x-api-key']).toBe('[Filtered]'); + // Sensitive request headers should have their values filtered + // 'authorization' matches the 'auth' snippet + expect(reqHeaders['authorization']).toBe('[Filtered]'); + // 'x-api-key' matches the 'key' snippet + expect(reqHeaders['x-api-key']).toBe('[Filtered]'); - // Non-sensitive response headers should be present with their values - expect(resHeaders['x-request-id']).toBe('abc-123'); + // Non-sensitive response headers should be present with their values + expect(resHeaders['x-request-id']).toBe('abc-123'); - // Sensitive response headers should have their values filtered - // 'x-auth-token' matches 'auth' and 'token' snippets - expect(resHeaders['x-auth-token']).toBe('[Filtered]'); - }, -); + // Sensitive response headers should have their values filtered + // 'x-auth-token' matches 'auth' and 'token' snippets + expect(resHeaders['x-auth-token']).toBe('[Filtered]'); +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/init.js b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/init.js deleted file mode 100644 index 1da8a0e59ade..000000000000 --- a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/init.js +++ /dev/null @@ -1,13 +0,0 @@ -import * as Sentry from '@sentry/browser'; -import { httpClientIntegration } from '@sentry/browser'; - -window.Sentry = Sentry; - -Sentry.init({ - traceLifecycle: 'static', - dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [httpClientIntegration()], - tracesSampleRate: 1, - // sendDefaultPii is not set (defaults to false) - // dataCollection is not set -}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/subject.js b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/subject.js deleted file mode 100644 index 93da60f0e2e6..000000000000 --- a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/subject.js +++ /dev/null @@ -1,9 +0,0 @@ -fetch('http://sentry-test.io/foo', { - method: 'GET', - credentials: 'include', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - Cache: 'no-cache', - }, -}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/test.ts b/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/test.ts deleted file mode 100644 index cd7d9494ae0d..000000000000 --- a/dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/test.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { expect } from '@playwright/test'; -import type { Event } from '@sentry/core'; -import { sentryTest } from '../../../../../utils/fixtures'; -import { envelopeRequestParser, waitForErrorRequest } from '../../../../../utils/helpers'; - -sentryTest( - 'should not capture request/response headers or cookies without sendDefaultPii', - async ({ getLocalTestUrl, page }) => { - const url = await getLocalTestUrl({ testDir: __dirname }); - - await page.route('**/foo', route => { - return route.fulfill({ - status: 500, - body: JSON.stringify({ - error: { - message: 'Internal Server Error', - }, - }), - headers: { - 'Content-Type': 'text/html', - }, - }); - }); - - const req = await Promise.all([waitForErrorRequest(page), page.goto(url)]).then(([r]) => r); - const eventData = envelopeRequestParser(req); - - expect(eventData.exception?.values).toHaveLength(1); - expect(eventData.message).toBe('HTTP Client Error with status code: 500'); - - // Request URL and method are always present - expect(eventData.request?.url).toBe('http://sentry-test.io/foo'); - expect(eventData.request?.method).toBe('GET'); - - // Without sendDefaultPii, no request headers should be captured by the integration - expect(eventData.request?.headers?.accept).toBeUndefined(); - expect(eventData.request?.headers?.cache).toBeUndefined(); - expect(eventData.request?.headers?.['content-type']).toBeUndefined(); - expect(eventData.request?.cookies).toBeUndefined(); - - // Response headers should not be captured either - expect(eventData.contexts?.response?.headers?.['content-type']).toBeUndefined(); - expect(eventData.contexts?.response?.cookies).toBeUndefined(); - }, -); diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpclient/init.js b/dev-packages/browser-integration-tests/suites/integrations/httpclient/init.js index d884ae8eb04c..43248f09b63d 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/httpclient/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/httpclient/init.js @@ -8,6 +8,4 @@ Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', integrations: [httpClientIntegration()], tracesSampleRate: 1, - // todo(v11): remove together with the sendDefaultPii guard in httpclient.ts (JS-2580) - sendDefaultPii: true, }); 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/browser/src/integrations/httpclient.ts b/packages/browser/src/integrations/httpclient.ts index 1371556e9f06..8ab3c11fccd4 100644 --- a/packages/browser/src/integrations/httpclient.ts +++ b/packages/browser/src/integrations/httpclient.ts @@ -427,17 +427,6 @@ function _getDataCollectionSettings() { return { cookies: false, requestHeaders: false, responseHeaders: false }; } - // todo(v11): Always use granular dataCollection settings and remove this legacy guard. - // Currently, when dataCollection is not explicitly set, we gate all collection on - // sendDefaultPii to avoid sending more data than before (the spec defaults would - // collect headers/cookies with deny-list filtering even without sendDefaultPii). - const options = client.getOptions(); - if (options.dataCollection == null) { - // eslint-disable-next-line typescript/no-deprecated - const enabled = Boolean(options.sendDefaultPii); - return { cookies: enabled, requestHeaders: enabled, responseHeaders: enabled }; - } - const { cookies, httpHeaders } = client.getDataCollectionOptions(); return { cookies, requestHeaders: httpHeaders.request, responseHeaders: httpHeaders.response }; } diff --git a/packages/browser/test/integrations/httpclient.test.ts b/packages/browser/test/integrations/httpclient.test.ts index 3a6e8a527853..67b811cb1aa6 100644 --- a/packages/browser/test/integrations/httpclient.test.ts +++ b/packages/browser/test/integrations/httpclient.test.ts @@ -142,21 +142,24 @@ describe('httpClientIntegration', () => { }); }); - // TODO(v11): also collect safe headers by default (but no PII headers) to align with server behavior - it('does not collect headers or cookies without sendDefaultPii or dataCollection', () => { + it('collects safe headers and filters sensitive headers by default', () => { const { fetchHandler, captureEventSpy } = setup(); triggerFetch(fetchHandler, { - requestHeaders: { Authorization: 'Bearer x', Accept: 'application/json' }, - responseHeaders: { 'Content-Type': 'text/html' }, + requestHeaders: { Authorization: 'Bearer x', Accept: 'application/json', Cookie: 'theme=dark; session=secret' }, + responseHeaders: { 'Content-Type': 'text/html', 'Set-Cookie': 'locale=en; session=secret' }, }); expect(captureEventSpy).toHaveBeenCalledTimes(1); const event = getEvent(captureEventSpy); - expect(event.request?.headers).toBeUndefined(); - expect(event.request?.cookies).toBeUndefined(); - expect(event.contexts?.response?.headers).toBeUndefined(); - expect(event.contexts?.response?.cookies).toBeUndefined(); + expect(event.request?.headers).toEqual({ + accept: 'application/json', + authorization: '[Filtered]', + cookie: '[Filtered]', + }); + expect(event.request?.cookies).toEqual({ theme: 'dark', session: '[Filtered]' }); + expect(event.contexts?.response?.headers).toEqual({ 'content-type': 'text/html', 'set-cookie': '[Filtered]' }); + expect(event.contexts?.response?.cookies).toEqual({ locale: 'en', session: '[Filtered]' }); }); it('filters PII headers when an explicit deny list is configured', () => { @@ -252,7 +255,7 @@ describe('httpClientIntegration', () => { }); }); - it('does not collect response headers or cookies without sendDefaultPii or dataCollection', () => { + it('collects response headers and filters response cookies by default', () => { const { xhrHandler, captureEventSpy } = setup(); triggerXhr(xhrHandler, { @@ -263,9 +266,9 @@ describe('httpClientIntegration', () => { expect(captureEventSpy).toHaveBeenCalledTimes(1); const event = getEvent(captureEventSpy); - expect(event.request?.headers).toBeUndefined(); - expect(event.contexts?.response?.headers).toBeUndefined(); - expect(event.contexts?.response?.cookies).toBeUndefined(); + expect(event.request?.headers).toEqual({ Authorization: '[Filtered]' }); + expect(event.contexts?.response?.headers).toEqual({ 'content-type': 'text/html' }); + expect(event.contexts?.response?.cookies).toEqual({ session: '[Filtered]', theme: 'dark' }); }); }); }); diff --git a/packages/core/src/integrations/requestdata.ts b/packages/core/src/integrations/requestdata.ts index 3470ec5ceb00..454edb9a58c7 100644 --- a/packages/core/src/integrations/requestdata.ts +++ b/packages/core/src/integrations/requestdata.ts @@ -2,26 +2,29 @@ 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 { SENSITIVE_COOKIE_NAME_SNIPPETS } from '../utils/data-collection/filtering-snippets'; +import { filterKeyValueData } from '../utils/data-collection/filterKeyValueData'; +import { filterQueryParams } from '../utils/data-collection/filterQueryParams'; import { httpHeadersToSpanAttributes } from '../utils/request'; import { getUrlQuery } from '../utils/url'; import { getClientIPAddress, ipHeaderNames } from '../vendor/getIpAddress'; import { safeSetSpanJSONAttributes } from '../tracing/spans/captureSpan'; import { URL_FULL, URL_QUERY } from '@sentry/conventions/attributes'; -interface RequestDataIncludeOptions { +type RequestDataIncludeOptions = { cookies?: boolean; data?: boolean; headers?: boolean; ip?: boolean; query_string?: boolean; url?: boolean; -} +}; type RequestDataIntegrationOptions = { /** @@ -30,38 +33,37 @@ type RequestDataIntegrationOptions = { include?: RequestDataIncludeOptions; }; +type ResolvedRequestDataOptions = { + include: Required; + dataCollection: ResolvedDataCollection; +}; + const INTEGRATION_NAME = 'RequestData' as const; const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) => { - // Per spec, integration-level options override global dataCollection. - // When include overrides a category back on that dataCollection turned off, - // we flip the dataCollection behavior to true (default denylist filtering). - function resolveIncludeAndDataCollection(client: Client): { - include: RequestDataIncludeOptions; - dataCollection: ResolvedDataCollection; - } { - const dc = client.getDataCollectionOptions(); - const dataCollection: ResolvedDataCollection = { - ...dc, - ...(options.include?.cookies === true && dc.cookies === false && { cookies: true as const }), - ...(options.include?.headers === true && - dc.httpHeaders.request === false && { - httpHeaders: { ...dc.httpHeaders, request: true as const }, - }), + function resolveRequestDataOptions(client: Client): ResolvedRequestDataOptions { + const dataCollection = client.getDataCollectionOptions(); + const include = { + cookies: options.include?.cookies ?? dataCollection.cookies !== false, + // Always attach body data that's already on the scope — dataCollection.httpBodies gates write-time, not read-time + data: options.include?.data ?? true, + headers: options.include?.headers ?? dataCollection.httpHeaders.request !== false, + ip: options.include?.ip ?? dataCollection.userInfo, + query_string: options.include?.query_string ?? dataCollection.urlQueryParams !== false, + // No dataCollection equivalent — URL is always included + url: options.include?.url ?? true, }; return { - dataCollection, - include: { - cookies: dataCollection.cookies !== false, - // Always attach body data that's already on the scope — dataCollection.httpBodies gates write-time, not read-time - data: true, - headers: dataCollection.httpHeaders.request !== false, - ip: dataCollection.userInfo, - query_string: dataCollection.urlQueryParams !== false, - // No dataCollection equivalent — URL is always included - url: true, - ...options.include, + include, + dataCollection: { + ...dataCollection, + cookies: resolveFilteringBehavior(include.cookies, dataCollection.cookies), + httpHeaders: { + ...dataCollection.httpHeaders, + request: resolveFilteringBehavior(include.headers, dataCollection.httpHeaders.request), + }, + urlQueryParams: resolveFilteringBehavior(include.query_string, dataCollection.urlQueryParams), }, }; } @@ -72,12 +74,13 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) = const { sdkProcessingMetadata = {} } = event; const { normalizedRequest, ipAddress } = sdkProcessingMetadata; - const { include } = resolveIncludeAndDataCollection(client); - - if (normalizedRequest) { - addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress }, include); + if (!normalizedRequest) { + return event; } + const { include, dataCollection } = resolveRequestDataOptions(client); + addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress }, include, dataCollection); + return event; }, processSegmentSpan(span, client) { @@ -88,7 +91,7 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) = return; } - const { include, dataCollection } = resolveIncludeAndDataCollection(client); + const { include, dataCollection } = resolveRequestDataOptions(client); addNormalizedRequestDataToSpan(span, normalizedRequest, ipAddress, include, dataCollection); }, @@ -111,10 +114,26 @@ 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.cookies) { + requestData.cookies = filterKeyValueData( + requestData.cookies, + dataCollection.cookies, + SENSITIVE_COOKIE_NAME_SNIPPETS, + ); + } + if (requestData.headers) { + requestData.headers = filterKeyValueData(requestData.headers, dataCollection.httpHeaders.request); + } + if (requestData.query_string) { + requestData.query_string = normalizeAndFilterQueryString(requestData.query_string, dataCollection.urlQueryParams); + } + event.request = { ...event.request, - ...extractNormalizedRequestData(req, include), + ...requestData, }; if (include.ip) { @@ -147,7 +166,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); @@ -229,13 +248,21 @@ function extractNormalizedRequestData( return requestData; } +function resolveFilteringBehavior(isIncluded: boolean, behavior: CollectBehavior): CollectBehavior { + return isIncluded && behavior === false ? true : behavior; +} + +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 getUrlQuery(queryString); } 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/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/test/lib/integrations/requestdata.test.ts b/packages/core/test/lib/integrations/requestdata.test.ts index 10f12c3c3c66..d65a35e8ec1a 100644 --- a/packages/core/test/lib/integrations/requestdata.test.ts +++ b/packages/core/test/lib/integrations/requestdata.test.ts @@ -98,13 +98,14 @@ describe('requestDataIntegration', () => { }); }); - it('keeps IP headers when include.ip is true even if userInfo is false', () => { + it('includes user IP when include.ip is true and dataCollection.userInfo is false', () => { const integration = requestDataIntegration({ include: { ip: true } }); const event = baseEvent(); - integration.processEvent?.(event, {}, mockClient(false)); + integration.processEvent?.(event, {}, mockClient(false, { userInfo: false })); expect(event.request?.headers?.['X-Forwarded-For']).toBe('192.168.1.1'); + expect(event.user?.ip_address).toBe('192.168.1.1'); }); it('strips IP headers when include.ip is false even if userInfo is true', () => { @@ -276,6 +277,42 @@ describe('requestDataIntegration', () => { expect(event.request?.cookies).toEqual({ id: '42' }); }); + it('omits headers when include.headers is false and dataCollection enables headers', () => { + const integration = requestDataIntegration({ include: { headers: false } }); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { headers: { Accept: 'application/json' } }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false, { httpHeaders: { request: true, response: true } })); + + expect(event.request?.headers).toBeUndefined(); + }); + + it('applies the configured header allowlist when include.headers is true', () => { + const integration = requestDataIntegration({ include: { headers: true } }); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { + headers: { Accept: 'application/json', 'X-Request-Id': 'req-123', Authorization: 'Bearer secret' }, + }, + }, + }; + + integration.processEvent?.( + event, + {}, + mockClient(false, { httpHeaders: { request: { allow: ['accept'] }, response: true } }), + ); + + expect(event.request?.headers).toEqual({ + Accept: 'application/json', + 'X-Request-Id': '[Filtered]', + Authorization: '[Filtered]', + }); + }); + it('with include.headers false, still sets user.ip_address from original headers when userInfo is true', () => { const integration = requestDataIntegration({ include: { headers: false } }); const event: Event = { @@ -295,6 +332,21 @@ describe('requestDataIntegration', () => { }); }); + it('filters sensitive request headers', () => { + const integration = requestDataIntegration(); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { + headers: { Accept: 'application/json', Authorization: 'Bearer secret' }, + }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false)); + + expect(event.request?.headers).toEqual({ Accept: 'application/json', Authorization: '[Filtered]' }); + }); + describe('include.cookies', () => { it('removes the cookie header from event.request.headers when include.cookies is false', () => { const integration = requestDataIntegration({ @@ -342,6 +394,66 @@ describe('requestDataIntegration', () => { expect(event.request?.cookies).toBeUndefined(); }); + it('omits cookies when include.cookies is false and dataCollection enables cookies', () => { + const integration = requestDataIntegration({ include: { cookies: false } }); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { cookies: { theme: 'dark' } }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false, { cookies: true })); + + expect(event.request?.cookies).toBeUndefined(); + }); + + it('applies the default denylist when include.cookies overrides dataCollection.cookies=false', () => { + const integration = requestDataIntegration({ include: { cookies: true } }); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { cookies: { theme: 'dark', session: 'secret' } }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false, { cookies: false })); + + expect(event.request?.cookies).toEqual({ theme: 'dark', session: '[Filtered]' }); + }); + + it('preserves the configured cookie denylist when include.cookies is true', () => { + const integration = requestDataIntegration({ include: { cookies: true } }); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { cookies: { theme: 'dark', experiment: 'variant-a', session: 'secret' } }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false, { cookies: { deny: ['experiment'] } })); + + expect(event.request?.cookies).toEqual({ + theme: 'dark', + experiment: '[Filtered]', + session: '[Filtered]', + }); + }); + + it('preserves the configured cookie allowlist when include.cookies is true', () => { + const integration = requestDataIntegration({ include: { cookies: true } }); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { cookies: { theme: 'dark', locale: 'en', session: 'secret' } }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false, { cookies: { allow: ['theme'] } })); + + expect(event.request?.cookies).toEqual({ + theme: 'dark', + locale: '[Filtered]', + session: '[Filtered]', + }); + }); + it('uses normalizedRequest.cookies when set', () => { const integration = requestDataIntegration(); const event: Event = { @@ -350,14 +462,14 @@ describe('requestDataIntegration', () => { method: 'GET', url: 'https://example.com/', headers: { Host: 'example.com' }, - cookies: { session_id: 'abc' }, + cookies: { preference: 'abc' }, }, }, }; integration.processEvent?.(event, {}, mockClient(false)); - expect(event.request?.cookies).toEqual({ session_id: 'abc' }); + expect(event.request?.cookies).toEqual({ preference: 'abc' }); }); it('prefers normalizedRequest.cookies over the Cookie header when both are present', () => { @@ -395,6 +507,40 @@ describe('requestDataIntegration', () => { expect(event.request?.cookies).toEqual({ a: '1', b: 'two' }); }); + it('filters sensitive cookies with the default denylist', () => { + const integration = requestDataIntegration(); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { + headers: { cookie: 'theme=dark; session=secret; connect.sid=secret' }, + }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false)); + + expect(event.request?.cookies).toEqual({ + theme: 'dark', + session: '[Filtered]', + 'connect.sid': '[Filtered]', + }); + }); + + it('applies a custom cookie denylist', () => { + const integration = requestDataIntegration(); + const event: Event = { + sdkProcessingMetadata: { + normalizedRequest: { + cookies: { theme: 'dark', experiment: 'variant-a' }, + }, + }, + }; + + integration.processEvent?.(event, {}, mockClient(false, { cookies: { deny: ['experiment'] } })); + + expect(event.request?.cookies).toEqual({ theme: 'dark', experiment: '[Filtered]' }); + }); + it('sets event.request.cookies to an empty object when include.cookies is true but no cookies are present', () => { const integration = requestDataIntegration(); const event: Event = { @@ -428,6 +574,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 = { @@ -470,11 +668,11 @@ describe('requestDataIntegration', () => { data: { body: 'payload' }, headers: { Host: 'example.com', - cookie: 'session=from-header', + cookie: '[Filtered]', 'X-Forwarded-For': '192.168.1.1', 'X-Custom': 'keep', }, - cookies: { session: 'from-header' }, + cookies: { session: '[Filtered]' }, }); expect(event.user?.ip_address).toBe('192.168.1.1'); }); @@ -489,10 +687,10 @@ describe('requestDataIntegration', () => { expect(event.request?.headers).toEqual({ Host: 'example.com', - cookie: 'session=from-header', + cookie: '[Filtered]', 'X-Custom': 'keep', }); - expect(event.request?.cookies).toEqual({ session: 'from-header' }); + expect(event.request?.cookies).toEqual({ session: '[Filtered]' }); expect(event.user?.ip_address).toBeUndefined(); }); @@ -789,6 +987,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 } }); @@ -871,6 +1101,17 @@ describe('requestDataIntegration processSegmentSpan', () => { expect(span.attributes).not.toHaveProperty('url.query'); }); + it('include.ip overrides dataCollection.userInfo=false on spans', () => { + const integration = requestDataIntegration({ include: { ip: true } }); + const span = makeSpan(); + + mockIsolationScope({ headers: { 'x-forwarded-for': '203.0.113.50' } }); + + integration.processSegmentSpan!(span, mockClient(false, { userInfo: false })); + + expect(span.attributes?.['user.ip_address']).toBe('203.0.113.50'); + }); + it('include.headers overrides dataCollection.httpHeaders.request=false on spans', () => { const integration = requestDataIntegration({ include: { headers: true } }); const span = makeSpan(); @@ -887,6 +1128,21 @@ describe('requestDataIntegration processSegmentSpan', () => { }); }); + it('preserves configured header filtering when include.headers is true on spans', () => { + const integration = requestDataIntegration({ include: { headers: true } }); + const span = makeSpan(); + + mockIsolationScope({ headers: { accept: 'application/json', 'x-request-id': 'req-123' } }); + + integration.processSegmentSpan!( + span, + mockClient(false, { httpHeaders: { request: { allow: ['accept'] }, response: true } }), + ); + + expect(span.attributes?.['http.request.header.accept']).toBe('application/json'); + expect(span.attributes?.['http.request.header.x_request_id']).toBe('[Filtered]'); + }); + it('include.cookies overrides dataCollection.cookies=false on spans', () => { const integration = requestDataIntegration({ include: { cookies: true } }); const span = makeSpan(); @@ -902,6 +1158,30 @@ describe('requestDataIntegration processSegmentSpan', () => { 'http.request.header.cookie.locale': 'en', }); }); + + it('preserves configured cookie filtering when include.cookies is true on spans', () => { + const integration = requestDataIntegration({ include: { cookies: true } }); + const span = makeSpan(); + + mockIsolationScope({ cookies: { theme: 'dark', locale: 'en', session: 'secret' } }); + + integration.processSegmentSpan!(span, mockClient(false, { cookies: { allow: ['theme'] } })); + + expect(span.attributes?.['http.request.header.cookie.theme']).toBe('dark'); + expect(span.attributes?.['http.request.header.cookie.locale']).toBe('[Filtered]'); + expect(span.attributes?.['http.request.header.cookie.session']).toBe('[Filtered]'); + }); + + 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/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]'); }); }); });