Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8" />
</head>
<body>
<img src="https://sentry-test-site.example/path/to/image.svg?token=sensitive-token-value&size=2" />
</body>
</html>
Original file line number Diff line number Diff line change
@@ -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: '<svg xmlns="http://www.w3.org/2000/svg"/>', 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<Event>(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');
},
);
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
</head>
<body>
<div id="content"></div>
<img src="https://sentry-test-site.example/my/image.png" />
<img src="https://sentry-test-site.example/my/image.png?token=sensitive-token-value&v=1" />
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
});
Expand All @@ -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']);
Expand Down
2 changes: 1 addition & 1 deletion dev-packages/browser-integration-tests/utils/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ const sentryTest = base.extend<TestFixtures>({
}

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)
Expand Down
20 changes: 11 additions & 9 deletions packages/browser-utils/src/performance/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } : {}),
},
});
Expand Down Expand Up @@ -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,
},
});
}
Expand Down Expand Up @@ -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
Expand All @@ -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,
});
Expand Down
3 changes: 2 additions & 1 deletion packages/browser-utils/src/web-vitals/spans.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { Client, Span, SpanAttributes } from '@sentry/core';
import {
browserPerformanceTimeOrigin,
debug,
filterCollectedUrl,
getActiveSpan,
getClient,
getRootSpan,
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions packages/browser-utils/src/web-vitals/tracking.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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) {
Expand Down
40 changes: 40 additions & 0 deletions packages/browser-utils/test/performance/browserMetrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];

Expand Down Expand Up @@ -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'],
Expand Down
5 changes: 3 additions & 2 deletions packages/browser-utils/test/web-vitals/spans.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ describe('_sendLcpSpan', () => {
vi.mocked(SentryCore.getClient).mockReturnValue({
getOptions: () => ({ traceLifecycle: 'static' }),
getIntegrationByName: () => undefined,
getDataCollectionOptions: () => ({ urlQueryParams: true }),
} as any);
});

Expand All @@ -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,
Expand All @@ -424,7 +425,7 @@ describe('_sendLcpSpan', () => {
'sentry.pageload.span_id': 'pageload-123',
'browser.web_vital.lcp.element': '<img>',
'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,
Expand Down
3 changes: 1 addition & 2 deletions packages/browser/src/integrations/httpcontext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }),
};
Expand Down
5 changes: 3 additions & 2 deletions packages/browser/src/tracing/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -574,8 +574,9 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption

// We store the normalized request data on the scope, so we get the request data at time of span creation
// otherwise, the URL etc. may already be of the following navigation, and we'd report the wrong URL
const requestData = getHttpRequestData();
scope.setSDKProcessingMetadata({
normalizedRequest: getHttpRequestData(),
normalizedRequest: { ...requestData, url: filterCollectedUrl(requestData.url, client) },
});

_createRouteSpan(client, {
Expand Down Expand Up @@ -815,7 +816,7 @@ export function startBrowserTracingNavigationSpan(
scope.setSDKProcessingMetadata({
normalizedRequest: {
...getHttpRequestData(),
url,
url: filterCollectedUrl(url, client),
},
});
}
Expand Down
23 changes: 23 additions & 0 deletions packages/browser/test/integrations/httpcontext.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,29 @@ describe('httpContextIntegration', () => {
});
});

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'] } } } }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ describe('pagesRouterInstrumentNavigation', () => {
const client = {
emit,
getOptions: () => ({}),
getDataCollectionOptions: () => ({ urlQueryParams: true }),
} as unknown as Client;

pagesRouterInstrumentNavigation(client);
Expand Down
Loading