diff --git a/dev-packages/browser-integration-tests/suites/integrations/httpContext/test.ts b/dev-packages/browser-integration-tests/suites/integrations/httpContext/test.ts index db8a384510a4..e50a658b4c9a 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/httpContext/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/httpContext/test.ts @@ -25,6 +25,18 @@ sentryTest('httpContextIntegration captures user-agent and referrer', async ({ g }); }); +sentryTest('httpContextIntegration filters sensitive query params in the URL', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + const errorEventPromise = waitForErrorRequest(page); + + await page.goto(`${url}?jwt=sensitive-jwt-value&page=1`); + + const errorEvent = envelopeRequestParser(await errorEventPromise); + + expect(errorEvent.request?.url).toBe(`${url}?jwt=[Filtered]&page=1`); +}); + sentryTest('httpContextIntegration captures url, user-agent, and referer', async ({ getLocalTestUrl, page }) => { sentryTest.skip(shouldSkipTracingTest()); const url = await getLocalTestUrl({ testDir: __dirname }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-sensitive-query-params-static/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-sensitive-query-params-static/init.js new file mode 100644 index 000000000000..bb6d4918d1bf --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-sensitive-query-params-static/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + traceLifecycle: 'static', + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-sensitive-query-params-static/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-sensitive-query-params-static/template.html new file mode 100644 index 000000000000..3d3b49a9951f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-sensitive-query-params-static/template.html @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-sensitive-query-params-static/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-sensitive-query-params-static/test.ts new file mode 100644 index 000000000000..8bfc5a6df874 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/pageload-sensitive-query-params-static/test.ts @@ -0,0 +1,32 @@ +import { BROWSER_REQUEST, PAGELOAD, RESOURCE_IMG } from '@sentry/conventions/op'; +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/core'; +import { sentryTest } from '../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers'; + +sentryTest( + 'filters sensitive query params in the pageload transaction URL and span descriptions', + async ({ getLocalTestUrl, page }) => { + sentryTest.skip(shouldSkipTracingTest()); + + await page.route('https://sentry-test-site.example/path/to/image.svg?*', route => + route.fulfill({ body: '', contentType: 'image/svg+xml' }), + ); + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === PAGELOAD); + await page.goto(`${url}?jwt=sensitive-jwt-value&page=1`); + const eventData = envelopeRequestParser(await pageloadRequestPromise); + + expect(eventData.request?.url).toBe(`${url}?jwt=[Filtered]&page=1`); + expect(eventData.spans?.find(({ op }) => op === BROWSER_REQUEST)?.description).toBe(`${url}?jwt=[Filtered]&page=1`); + expect(eventData.spans?.find(({ op }) => op === RESOURCE_IMG)?.description).toBe( + 'https://sentry-test-site.example/path/to/image.svg?token=[Filtered]&size=2', + ); + + const serializedEvent = JSON.stringify(eventData); + expect(serializedEvent).not.toContain('sensitive-jwt-value'); + expect(serializedEvent).not.toContain('sensitive-token-value'); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-static/template.html b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-static/template.html index b613a556aca4..164dbf51f486 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-static/template.html +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-static/template.html @@ -5,6 +5,6 @@
- + diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-static/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-static/test.ts index 3c0b3c5af990..712a06a21b04 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-static/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp-static/test.ts @@ -16,7 +16,7 @@ sentryTest('captures LCP vitals with element details', async ({ browserName, get } page.route('**', route => route.continue()); - page.route('**/my/image.png', async (route: Route) => { + page.route('**/my/image.png?*', async (route: Route) => { return route.fulfill({ path: `${__dirname}/assets/sentry-logo-600x179.png`, }); @@ -36,6 +36,9 @@ sentryTest('captures LCP vitals with element details', async ({ browserName, get expect(eventData.contexts?.trace?.data?.['lcp.size']).toBeGreaterThan(0); expect(eventData.contexts?.trace?.data?.['lcp.loadTime']).toBeGreaterThan(0); expect(eventData.contexts?.trace?.data?.['lcp.renderTime']).toBeGreaterThan(0); + expect(eventData.contexts?.trace?.data?.['lcp.url']).toBe( + 'https://sentry-test-site.example/my/image.png?token=[Filtered]&v=1', + ); // The LCP value should be the renderTime because the renderTime is set expect(eventData.measurements?.lcp?.value).toBeCloseTo(eventData.contexts?.trace?.data?.['lcp.renderTime']); diff --git a/dev-packages/browser-integration-tests/utils/fixtures.ts b/dev-packages/browser-integration-tests/utils/fixtures.ts index f856fa64ce23..7436f2c35d9a 100644 --- a/dev-packages/browser-integration-tests/utils/fixtures.ts +++ b/dev-packages/browser-integration-tests/utils/fixtures.ts @@ -90,7 +90,7 @@ const sentryTest = base.extend({ } await page.route(`${TEST_HOST}/*.*`, route => { - const file = route.request().url().split('/').pop(); + const file = new URL(route.request().url()).pathname.split('/').pop(); const filePath = path.resolve(tmpDir, `./${file}`); return fs.existsSync(filePath) diff --git a/packages/browser-utils/src/performance/entries.ts b/packages/browser-utils/src/performance/entries.ts index 668a0fb3a077..905fa44426f7 100644 --- a/packages/browser-utils/src/performance/entries.ts +++ b/packages/browser-utils/src/performance/entries.ts @@ -360,14 +360,15 @@ function _addPerformanceNavigationTiming( return; } const op = NAVIGATION_TIMING_SPAN_OPS[event]; + const filteredUrl = filterCollectedUrl(entry.name); startAndEndSpan(span, _navigationTimeToSeconds(entry, start), _navigationTimeToSeconds(entry, end), { // With span streaming, span names have to be low cardinality, so we can't fall back to the // document URL. `url.full` keeps it, and is what Relay derives the description from. - name: spanStreamingEnabled ? BROWSER_NAVIGATION_TIMING_SPAN_NAMES[op] : entry.name, + name: spanStreamingEnabled ? BROWSER_NAVIGATION_TIMING_SPAN_NAMES[op] : filteredUrl, attributes: { [SENTRY_OP]: op, [SENTRY_ORIGIN]: 'auto.ui.browser.metrics', - [URL_FULL]: filterCollectedUrl(entry.name), + [URL_FULL]: filteredUrl, ...(event === 'redirect' && entry.redirectCount != null ? { 'http.redirect_count': entry.redirectCount } : {}), }, }); @@ -396,23 +397,23 @@ function _addRequest(span: Span, entry: PerformanceNavigationTiming, spanStreami // With span streaming, span names have to be low cardinality, so we can't fall back to the // document URL. `url.full` keeps it, and is what Relay derives the description from. - const url = filterCollectedUrl(entry.name); + const filteredUrl = filterCollectedUrl(entry.name); startAndEndSpan(span, requestStartTimestamp, responseEndTimestamp, { - name: spanStreamingEnabled ? BROWSER_NAVIGATION_TIMING_SPAN_NAMES[BROWSER_REQUEST] : entry.name, + name: spanStreamingEnabled ? BROWSER_NAVIGATION_TIMING_SPAN_NAMES[BROWSER_REQUEST] : filteredUrl, attributes: { [SENTRY_OP]: BROWSER_REQUEST, [SENTRY_ORIGIN]: 'auto.ui.browser.metrics', - [URL_FULL]: url, + [URL_FULL]: filteredUrl, }, }); startAndEndSpan(span, responseStartTimestamp, responseEndTimestamp, { - name: spanStreamingEnabled ? BROWSER_NAVIGATION_TIMING_SPAN_NAMES[BROWSER_RESPONSE] : entry.name, + name: spanStreamingEnabled ? BROWSER_NAVIGATION_TIMING_SPAN_NAMES[BROWSER_RESPONSE] : filteredUrl, attributes: { [SENTRY_OP]: BROWSER_RESPONSE, [SENTRY_ORIGIN]: 'auto.ui.browser.metrics', - [URL_FULL]: url, + [URL_FULL]: filteredUrl, }, }); } @@ -468,7 +469,8 @@ export function _addResourceSpans( attributes[HTTP_REQUEST_SAME_ORIGIN] = resourceUrl.includes(WINDOW.location.origin); - attributes[URL_FULL] = filterCollectedUrl(resourceUrl); + const filteredUrl = filterCollectedUrl(resourceUrl); + attributes[URL_FULL] = filteredUrl; _setResourceRequestAttributes(entry, attributes, [ // https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStatus @@ -493,7 +495,7 @@ export function _addResourceSpans( // With span streaming, span names have to be low cardinality, so we can't fall back to the URL. name: spanStreamingEnabled ? domain || RESOURCE_SPAN_NAME_FALLBACK - : resourceUrl.replace(WINDOW.location.origin, ''), + : filteredUrl.replace(WINDOW.location.origin, ''), op, attributes: attributesWithResourceTiming, }); diff --git a/packages/browser-utils/src/web-vitals/spans.ts b/packages/browser-utils/src/web-vitals/spans.ts index 5c44ea8d1078..1660a44cf273 100644 --- a/packages/browser-utils/src/web-vitals/spans.ts +++ b/packages/browser-utils/src/web-vitals/spans.ts @@ -3,6 +3,7 @@ import type { Client, Span, SpanAttributes } from '@sentry/core'; import { browserPerformanceTimeOrigin, debug, + filterCollectedUrl, getActiveSpan, getClient, getRootSpan, @@ -219,7 +220,7 @@ export function _sendLcpSpan( attributes[UI_COMPONENT_NAME] = componentName; } entry?.id && (attributes['browser.web_vital.lcp.id'] = entry.id); - entry?.url && (attributes['browser.web_vital.lcp.url'] = entry.url); + entry?.url && (attributes['browser.web_vital.lcp.url'] = filterCollectedUrl(entry.url, client)); entry?.loadTime != null && (attributes['browser.web_vital.lcp.load_time'] = entry.loadTime); entry?.renderTime != null && (attributes['browser.web_vital.lcp.render_time'] = entry.renderTime); entry?.size != null && (attributes['browser.web_vital.lcp.size'] = entry.size); diff --git a/packages/browser-utils/src/web-vitals/tracking.ts b/packages/browser-utils/src/web-vitals/tracking.ts index a8dc27fde867..c83fd40f02f0 100644 --- a/packages/browser-utils/src/web-vitals/tracking.ts +++ b/packages/browser-utils/src/web-vitals/tracking.ts @@ -1,5 +1,5 @@ import type { Client, Measurements, Span } from '@sentry/core'; -import { browserPerformanceTimeOrigin, debug, setMeasurement, spanToJSON } from '@sentry/core'; +import { browserPerformanceTimeOrigin, debug, filterCollectedUrl, setMeasurement, spanToJSON } from '@sentry/core'; import { BROWSER_NAVIGATION_TYPE, SENTRY_OP } from '@sentry/conventions/attributes'; import { DEBUG_BUILD } from '../debug-build'; import { htmlTreeAsString } from '../htmlTreeAsString'; @@ -253,7 +253,7 @@ function _setWebVitalAttributes(span: Span, options: AddWebVitalsToSpanOptions): if (_lcpEntry.url) { // Trim URL to the first 200 characters. - span.setAttribute('lcp.url', _lcpEntry.url.trim().slice(0, 200)); + span.setAttribute('lcp.url', filterCollectedUrl(_lcpEntry.url.trim()).slice(0, 200)); } if (_lcpEntry.loadTime != null) { diff --git a/packages/browser-utils/test/performance/browserMetrics.test.ts b/packages/browser-utils/test/performance/browserMetrics.test.ts index 7a5455aa491d..6ea603aa6311 100644 --- a/packages/browser-utils/test/performance/browserMetrics.test.ts +++ b/packages/browser-utils/test/performance/browserMetrics.test.ts @@ -289,6 +289,26 @@ describe('_addResourceSpans', () => { expect(json.attributes['url.full']).toBe('https://example.com/assets/app.js?v=42#main'); }); + it('filters sensitive query params in the description and url.full', () => { + const spans: Span[] = []; + + getClient()?.on('spanEnd', span => { + spans.push(span); + }); + + const entry = mockPerformanceResourceTiming({ + initiatorType: 'script', + nextHopProtocol: 'http/1.1', + }); + + _addResourceSpans(span, entry, 'https://example.com/assets/app.js?token=abc&v=42', 100, 23); + + expect(spans).toHaveLength(1); + const json = spanToJSON(spans[0]!); + expect(json.name).toBe('/assets/app.js?token=[Filtered]&v=42'); + expect(json.attributes['url.full']).toBe('https://example.com/assets/app.js?token=[Filtered]&v=42'); + }); + it('sets url.full to the full cross-origin URL', () => { const spans: Span[] = []; @@ -790,6 +810,26 @@ describe('_addNavigationSpans', () => { ); }); + it('filters sensitive query params in span names and url.full', () => { + const spans: Span[] = []; + + getClient()?.on('spanEnd', span => { + spans.push(span); + }); + + _addNavigationSpans(pageloadSpan, { ...entry, name: 'https://santry.com/test?jwt=baz&q=1' }); + + const filteredUrl = 'https://santry.com/test?jwt=[Filtered]&q=1'; + expect(spans.map(spanToJSON)).toEqual( + Array(9).fill( + expect.objectContaining({ + name: filteredUrl, + attributes: expect.objectContaining({ 'url.full': filteredUrl }), + }), + ), + ); + }); + describe('with span streaming enabled', () => { it.each([ ['browser.unload_event', 'Unload event'], diff --git a/packages/browser-utils/test/web-vitals/spans.test.ts b/packages/browser-utils/test/web-vitals/spans.test.ts index 05a20d5f2d2d..92c86637cbf2 100644 --- a/packages/browser-utils/test/web-vitals/spans.test.ts +++ b/packages/browser-utils/test/web-vitals/spans.test.ts @@ -392,6 +392,7 @@ describe('_sendLcpSpan', () => { vi.mocked(SentryCore.getClient).mockReturnValue({ getOptions: () => ({ traceLifecycle: 'static' }), getIntegrationByName: () => undefined, + getDataCollectionOptions: () => ({ urlQueryParams: true }), } as any); }); @@ -403,7 +404,7 @@ describe('_sendLcpSpan', () => { const mockEntry = { element: { tagName: 'img' } as Element, id: 'hero', - url: 'https://example.com/hero.jpg', + url: 'https://example.com/hero.jpg?token=abc&size=2', loadTime: 100, renderTime: 150, size: 50000, @@ -424,7 +425,7 @@ describe('_sendLcpSpan', () => { 'sentry.pageload.span_id': 'pageload-123', 'browser.web_vital.lcp.element': '', 'browser.web_vital.lcp.id': 'hero', - 'browser.web_vital.lcp.url': 'https://example.com/hero.jpg', + 'browser.web_vital.lcp.url': 'https://example.com/hero.jpg?token=[Filtered]&size=2', 'browser.web_vital.lcp.load_time': 100, 'browser.web_vital.lcp.render_time': 150, 'browser.web_vital.lcp.size': 50000, diff --git a/packages/browser/src/integrations/httpcontext.ts b/packages/browser/src/integrations/httpcontext.ts index b90c9c44ae2e..45961e307b15 100644 --- a/packages/browser/src/integrations/httpcontext.ts +++ b/packages/browser/src/integrations/httpcontext.ts @@ -35,8 +35,7 @@ export const httpContextIntegration = defineIntegration(() => { }; event.request = { - // The URL isn't gated by `dataCollection`, same as on the server. - url, + url: filterCollectedUrl(url, client), ...event.request, ...(Object.keys(headers).length > 0 ? { headers } : { headers: undefined }), }; diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index b791dddecefc..a900309bf992 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -574,8 +574,9 @@ export const browserTracingIntegration = ((options: Partial { }); }); + it('filters sensitive query params in the URL attached to events', () => { + const client = new BrowserClient(getDefaultBrowserClientOptions()); + const event: Event = {}; + + document.location.href = 'https://example.com/?jwt=baz&page=1'; + try { + httpContextIntegration().preprocessEvent!(event, {}, client); + } finally { + document.location.href = 'https://example.com'; + } + + expect(event.request?.url).toBe('https://example.com/?jwt=[Filtered]&page=1'); + }); + + it('leaves a user-supplied event URL untouched', () => { + const client = new BrowserClient(getDefaultBrowserClientOptions()); + const event: Event = { request: { url: 'https://example.com/?jwt=user-set' } }; + + httpContextIntegration().preprocessEvent!(event, {}, client); + + expect(event.request?.url).toBe('https://example.com/?jwt=user-set'); + }); + it('applies an allowlist to event headers', () => { const client = new BrowserClient( getDefaultBrowserClientOptions({ dataCollection: { httpHeaders: { request: { allow: ['user-agent'] } } } }), diff --git a/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts index 55e794dda04e..fb7dc10c3019 100644 --- a/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts +++ b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts @@ -320,6 +320,7 @@ describe('pagesRouterInstrumentNavigation', () => { const client = { emit, getOptions: () => ({}), + getDataCollectionOptions: () => ({ urlQueryParams: true }), } as unknown as Client; pagesRouterInstrumentNavigation(client);