Skip to content

Commit a81fdff

Browse files
authored
feat(core)!: Align request data collection with v11 defaults (#22853)
Cookies, request and response headers, and query parameters are now collected by default with sensitive values filtered, instead of being gated on `sendDefaultPii`. The RequestData integration remains a second configuration layer: explicit `include` values control whether a category is attached, while `dataCollection` continues to define its allowlist or denylist. When `include` explicitly enables a category disabled globally, the default sensitive-value denylist applies. This also allows options such as `include.ip: true` to override `dataCollection.userInfo: false` for that integration. Also added tests for the different cases. **Review Tip: Use "Hide Whitespace" as there were some indentation changes.** Closes #21260
1 parent 81140e1 commit a81fdff

15 files changed

Lines changed: 522 additions & 271 deletions

File tree

MIGRATION.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,17 @@ Sentry.init({
206206
Each key-value field (`cookies`, `urlQueryParams`, `httpHeaders.request`, `httpHeaders.response`) accepts
207207
`true`, `false`, `{ allow: string[] }`, or `{ deny: string[] }` for fine-grained control.
208208

209+
#### RequestData
210+
211+
The `requestDataIntegration`'s `include` options remain an integration-level override. An explicit `false`
212+
prevents that category from being attached, while an explicit `true` enables it even when the corresponding
213+
`dataCollection` category is disabled. For cookies, headers, and query parameters, any configured `allow` or
214+
`deny` filtering continues to apply: When `include` enables a category which `dataCollection` disabled, the
215+
default sensitive-value denylist is applied.
216+
209217
User IP address inference, which was previously gated on `sendDefaultPii`, is now controlled by
210-
`dataCollection.userInfo`.
218+
`dataCollection.userInfo`. An explicit `requestDataIntegration({ include: { ip: true } })` overrides
219+
`dataCollection.userInfo: false` for data collected by that integration.
211220

212221
### Browser sessions use `unhandled` instead of `crashed`
213222

dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/init.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,4 @@ Sentry.init({
88
dsn: 'https://public@dsn.ingest.sentry.io/1337',
99
integrations: [httpClientIntegration()],
1010
tracesSampleRate: 1,
11-
// todo(v11): remove together with the sendDefaultPii guard in httpclient.ts (JS-2580)
12-
sendDefaultPii: true,
1311
});

dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withSensitiveHeaders/test.ts

Lines changed: 36 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,51 +3,48 @@ import type { Event } from '@sentry/core';
33
import { sentryTest } from '../../../../../utils/fixtures';
44
import { envelopeRequestParser, waitForErrorRequest } from '../../../../../utils/helpers';
55

6-
sentryTest(
7-
'should filter sensitive header and cookie values with sendDefaultPii',
8-
async ({ getLocalTestUrl, page }) => {
9-
const url = await getLocalTestUrl({ testDir: __dirname });
10-
11-
await page.route('**/foo', route => {
12-
return route.fulfill({
13-
status: 500,
14-
body: JSON.stringify({
15-
error: {
16-
message: 'Internal Server Error',
17-
},
18-
}),
19-
headers: {
20-
'Content-Type': 'text/html',
21-
'X-Auth-Token': 'secret-response-token',
22-
'X-Request-Id': 'abc-123',
6+
sentryTest('filters sensitive header and cookie values by default', async ({ getLocalTestUrl, page }) => {
7+
const url = await getLocalTestUrl({ testDir: __dirname });
8+
9+
await page.route('**/foo', route => {
10+
return route.fulfill({
11+
status: 500,
12+
body: JSON.stringify({
13+
error: {
14+
message: 'Internal Server Error',
2315
},
24-
});
16+
}),
17+
headers: {
18+
'Content-Type': 'text/html',
19+
'X-Auth-Token': 'secret-response-token',
20+
'X-Request-Id': 'abc-123',
21+
},
2522
});
23+
});
2624

27-
const req = await Promise.all([waitForErrorRequest(page), page.goto(url)]).then(([r]) => r);
28-
const eventData = envelopeRequestParser<Event>(req);
25+
const req = await Promise.all([waitForErrorRequest(page), page.goto(url)]).then(([r]) => r);
26+
const eventData = envelopeRequestParser<Event>(req);
2927

30-
expect(eventData.exception?.values).toHaveLength(1);
28+
expect(eventData.exception?.values).toHaveLength(1);
3129

32-
const reqHeaders = eventData.request?.headers || {};
33-
const resHeaders = (eventData.contexts?.response?.headers as Record<string, string>) || {};
30+
const reqHeaders = eventData.request?.headers || {};
31+
const resHeaders = (eventData.contexts?.response?.headers as Record<string, string>) || {};
3432

35-
// Non-sensitive request headers should be present with their values
36-
expect(reqHeaders['accept']).toBe('application/json');
37-
expect(reqHeaders['content-type']).toBe('application/json');
38-
expect(reqHeaders['x-custom-header']).toBe('safe-value');
33+
// Non-sensitive request headers should be present with their values
34+
expect(reqHeaders['accept']).toBe('application/json');
35+
expect(reqHeaders['content-type']).toBe('application/json');
36+
expect(reqHeaders['x-custom-header']).toBe('safe-value');
3937

40-
// Sensitive request headers should have their values filtered
41-
// 'authorization' matches the 'auth' snippet
42-
expect(reqHeaders['authorization']).toBe('[Filtered]');
43-
// 'x-api-key' matches the 'key' snippet
44-
expect(reqHeaders['x-api-key']).toBe('[Filtered]');
38+
// Sensitive request headers should have their values filtered
39+
// 'authorization' matches the 'auth' snippet
40+
expect(reqHeaders['authorization']).toBe('[Filtered]');
41+
// 'x-api-key' matches the 'key' snippet
42+
expect(reqHeaders['x-api-key']).toBe('[Filtered]');
4543

46-
// Non-sensitive response headers should be present with their values
47-
expect(resHeaders['x-request-id']).toBe('abc-123');
44+
// Non-sensitive response headers should be present with their values
45+
expect(resHeaders['x-request-id']).toBe('abc-123');
4846

49-
// Sensitive response headers should have their values filtered
50-
// 'x-auth-token' matches 'auth' and 'token' snippets
51-
expect(resHeaders['x-auth-token']).toBe('[Filtered]');
52-
},
53-
);
47+
// Sensitive response headers should have their values filtered
48+
// 'x-auth-token' matches 'auth' and 'token' snippets
49+
expect(resHeaders['x-auth-token']).toBe('[Filtered]');
50+
});

dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/init.js

Lines changed: 0 additions & 13 deletions
This file was deleted.

dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/subject.js

Lines changed: 0 additions & 9 deletions
This file was deleted.

dev-packages/browser-integration-tests/suites/integrations/httpclient/fetch/withoutSendDefaultPii/test.ts

Lines changed: 0 additions & 45 deletions
This file was deleted.

dev-packages/browser-integration-tests/suites/integrations/httpclient/init.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,4 @@ Sentry.init({
88
dsn: 'https://public@dsn.ingest.sentry.io/1337',
99
integrations: [httpClientIntegration()],
1010
tracesSampleRate: 1,
11-
// todo(v11): remove together with the sendDefaultPii guard in httpclient.ts (JS-2580)
12-
sendDefaultPii: true,
1311
});

dev-packages/node-integration-tests/suites/express/tracing/withError/test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,21 @@ describe('express tracing with error', () => {
2626
runner.makeRequest('get', '/test/123/abc?q=1');
2727
await runner.completed();
2828
});
29+
30+
test('preserves encoded query parameters while filtering sensitive values on events', async () => {
31+
const runner = createRunner()
32+
.ignore('transaction')
33+
.expect({
34+
event: {
35+
request: {
36+
query_string: 'q=hello%20world&token=[Filtered]',
37+
},
38+
},
39+
})
40+
.start();
41+
42+
await runner.makeRequest('get', '/test/123/abc?q=hello%20world&token=secret');
43+
await runner.completed();
44+
});
2945
});
3046
});

packages/browser/src/integrations/httpclient.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -427,17 +427,6 @@ function _getDataCollectionSettings() {
427427
return { cookies: false, requestHeaders: false, responseHeaders: false };
428428
}
429429

430-
// todo(v11): Always use granular dataCollection settings and remove this legacy guard.
431-
// Currently, when dataCollection is not explicitly set, we gate all collection on
432-
// sendDefaultPii to avoid sending more data than before (the spec defaults would
433-
// collect headers/cookies with deny-list filtering even without sendDefaultPii).
434-
const options = client.getOptions();
435-
if (options.dataCollection == null) {
436-
// eslint-disable-next-line typescript/no-deprecated
437-
const enabled = Boolean(options.sendDefaultPii);
438-
return { cookies: enabled, requestHeaders: enabled, responseHeaders: enabled };
439-
}
440-
441430
const { cookies, httpHeaders } = client.getDataCollectionOptions();
442431
return { cookies, requestHeaders: httpHeaders.request, responseHeaders: httpHeaders.response };
443432
}

packages/browser/test/integrations/httpclient.test.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -142,21 +142,24 @@ describe('httpClientIntegration', () => {
142142
});
143143
});
144144

145-
// TODO(v11): also collect safe headers by default (but no PII headers) to align with server behavior
146-
it('does not collect headers or cookies without sendDefaultPii or dataCollection', () => {
145+
it('collects safe headers and filters sensitive headers by default', () => {
147146
const { fetchHandler, captureEventSpy } = setup();
148147

149148
triggerFetch(fetchHandler, {
150-
requestHeaders: { Authorization: 'Bearer x', Accept: 'application/json' },
151-
responseHeaders: { 'Content-Type': 'text/html' },
149+
requestHeaders: { Authorization: 'Bearer x', Accept: 'application/json', Cookie: 'theme=dark; session=secret' },
150+
responseHeaders: { 'Content-Type': 'text/html', 'Set-Cookie': 'locale=en; session=secret' },
152151
});
153152

154153
expect(captureEventSpy).toHaveBeenCalledTimes(1);
155154
const event = getEvent(captureEventSpy);
156-
expect(event.request?.headers).toBeUndefined();
157-
expect(event.request?.cookies).toBeUndefined();
158-
expect(event.contexts?.response?.headers).toBeUndefined();
159-
expect(event.contexts?.response?.cookies).toBeUndefined();
155+
expect(event.request?.headers).toEqual({
156+
accept: 'application/json',
157+
authorization: '[Filtered]',
158+
cookie: '[Filtered]',
159+
});
160+
expect(event.request?.cookies).toEqual({ theme: 'dark', session: '[Filtered]' });
161+
expect(event.contexts?.response?.headers).toEqual({ 'content-type': 'text/html', 'set-cookie': '[Filtered]' });
162+
expect(event.contexts?.response?.cookies).toEqual({ locale: 'en', session: '[Filtered]' });
160163
});
161164

162165
it('filters PII headers when an explicit deny list is configured', () => {
@@ -252,7 +255,7 @@ describe('httpClientIntegration', () => {
252255
});
253256
});
254257

255-
it('does not collect response headers or cookies without sendDefaultPii or dataCollection', () => {
258+
it('collects response headers and filters response cookies by default', () => {
256259
const { xhrHandler, captureEventSpy } = setup();
257260

258261
triggerXhr(xhrHandler, {
@@ -263,9 +266,9 @@ describe('httpClientIntegration', () => {
263266

264267
expect(captureEventSpy).toHaveBeenCalledTimes(1);
265268
const event = getEvent(captureEventSpy);
266-
expect(event.request?.headers).toBeUndefined();
267-
expect(event.contexts?.response?.headers).toBeUndefined();
268-
expect(event.contexts?.response?.cookies).toBeUndefined();
269+
expect(event.request?.headers).toEqual({ Authorization: '[Filtered]' });
270+
expect(event.contexts?.response?.headers).toEqual({ 'content-type': 'text/html' });
271+
expect(event.contexts?.response?.cookies).toEqual({ session: '[Filtered]', theme: 'dark' });
269272
});
270273
});
271274
});

0 commit comments

Comments
 (0)