Skip to content

Commit 566e06a

Browse files
s1gr1dclaude
andauthored
fix(core): Apply the sensitive denylist to cookie headers and configured fetch headers (#24090)
Three ways a sensitive value slipped past the denylist, now that cookies ship as one array attribute (#24231). A cookie segment without an `=` is a nameless cookie, so the bare token is its value (RFC 6265bis). The SDK treated it as a name, and no name-based denylist can match a value, so `Cookie: <session-token>` shipped the token in the clear. Such segments now become a `[Filtered]` array element. The `Cookie` header was also split on `"; "`, but the space is not guaranteed on the wire, so a cookie glued on with a bare `;` leaked inside the previous cookie's value. The split is now on `";"`. Headers listed in `headersToSpanAttributes` skipped the denylist entirely, so `authorization` went out in the clear. The spec says an allowlist never exempts a sensitive name, so those now emit `['[Filtered]']`. Fixes #24085 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 9c5dd8c commit 566e06a

10 files changed

Lines changed: 129 additions & 43 deletions

File tree

‎dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/instrument.mjs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,17 @@ Sentry.init({
77
release: '1.0',
88
tracesSampleRate: 1.0,
99
transport: loggingTransport,
10+
dataCollection: {
11+
httpHeaders: {
12+
request: { deny: ['x-tenant-id'] },
13+
response: { deny: ['content-length'] },
14+
},
15+
},
1016
integrations: [
1117
Sentry.nativeNodeFetchIntegration({
1218
headersToSpanAttributes: {
13-
requestHeaders: ['x-test-header'],
14-
responseHeaders: ['x-powered-by'],
19+
requestHeaders: ['x-test-header', 'authorization', 'x-tenant-id'],
20+
responseHeaders: ['x-powered-by', 'content-length'],
1521
},
1622
}),
1723
],

‎dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.mjs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,7 @@ import * as Sentry from '@sentry/node';
22

33
// eslint-disable-next-line @typescript-eslint/no-floating-promises
44
Sentry.startSpan({ name: 'test_transaction' }, async () => {
5-
await fetch(`${process.env.SERVER_URL}/api/v0`, { headers: { 'x-test-header': 'test-value' } });
5+
await fetch(`${process.env.SERVER_URL}/api/v0`, {
6+
headers: { 'x-test-header': 'test-value', authorization: 'Bearer super-secret', 'x-tenant-id': 'acme-corp' },
7+
});
68
});

‎dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@ describe('outgoing fetch spans - headers to span attributes', () => {
2929
origin: 'auto.http.node_fetch',
3030
data: expect.objectContaining({
3131
'http.request.header.x-test-header': ['test-value'],
32+
// Listed in `headersToSpanAttributes`, but the built-in denylist still wins.
33+
'http.request.header.authorization': ['[Filtered]'],
34+
// Listed in `headersToSpanAttributes`, but denied via `dataCollection.httpHeaders`.
35+
'http.request.header.x-tenant-id': ['[Filtered]'],
3236
'http.response.header.x-powered-by': ['Express'],
37+
'http.response.header.content-length': ['[Filtered]'],
3338
}),
3439
}),
3540
]),

‎packages/browser/src/integrations/httpclient.ts‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,16 +93,12 @@ function _fetchResponseHandler(
9393
const reqCookieStr = request.headers.get('Cookie') || undefined;
9494
if (reqCookieStr) {
9595
const filtered = _INTERNAL_filterCookies(reqCookieStr, dc.cookies);
96-
if (typeof filtered === 'object') {
97-
requestCookies = filtered;
98-
}
96+
requestCookies = typeof filtered === 'string' ? { cookie: filtered } : filtered;
9997
}
10098
const resCookieStr = response.headers.get('Set-Cookie') || undefined;
10199
if (resCookieStr) {
102100
const filtered = _INTERNAL_filterCookies(resCookieStr, dc.cookies);
103-
if (typeof filtered === 'object') {
104-
responseCookies = filtered;
105-
}
101+
responseCookies = typeof filtered === 'string' ? { 'set-cookie': filtered } : filtered;
106102
}
107103
}
108104

@@ -146,9 +142,7 @@ function _xhrResponseHandler(
146142
const cookieString = xhr.getResponseHeader('Set-Cookie') || xhr.getResponseHeader('set-cookie') || undefined;
147143
if (cookieString) {
148144
const filtered = _INTERNAL_filterCookies(cookieString, dc.cookies);
149-
if (typeof filtered === 'object') {
150-
responseCookies = filtered;
151-
}
145+
responseCookies = typeof filtered === 'string' ? { 'set-cookie': filtered } : filtered;
152146
}
153147
} catch {
154148
// ignore it if parsing fails

‎packages/core/src/utils/data-collection/filterCookies.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import { filterKeyValueData } from './filterKeyValueData';
88
*
99
* When individual cookies can be parsed, each key-value pair is filtered
1010
* independently. When parsing fails, the entire string is replaced with `[Filtered]`.
11+
* A nameless segment inside an otherwise parseable string (`"opaque-blob; theme=dark"`) is
12+
* dropped, since a record key cannot carry a `[Filtered]` marker without leaking the token.
1113
*/
1214
export function filterCookies(cookieString: string, behavior: CollectBehavior): Record<string, string> | string {
1315
if (behavior === false) {
@@ -17,8 +19,9 @@ export function filterCookies(cookieString: string, behavior: CollectBehavior):
1719
try {
1820
const parsed = parseCookie(cookieString);
1921

22+
// A non-empty string we cannot parse may still hold a session token, so it counts as sensitive.
2023
if (Object.keys(parsed).length === 0) {
21-
return {};
24+
return cookieString ? FILTERED : {};
2225
}
2326

2427
return filterKeyValueData(parsed, behavior, SENSITIVE_COOKIE_NAME_SNIPPETS);

‎packages/core/src/utils/request.ts‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -305,11 +305,16 @@ export function httpHeadersToSpanAttributes(
305305

306306
const cookies = parseCookieHeader(value, lowerKey === 'set-cookie');
307307
spanAttributes[`${prefix}${lowerKey}`] = cookies.length
308-
? cookies.map(([cookieKey, cookieValue]) =>
309-
shouldFilterDataKey(cookieKey, cookieBehavior, SENSITIVE_COOKIE_NAME_SNIPPETS)
308+
? cookies.map(([cookieKey, cookieValue]) => {
309+
// A nameless cookie's bare token is its value; no denylist could match it, so it is
310+
// always filtered.
311+
if (cookieKey === '') {
312+
return FILTERED_VALUE;
313+
}
314+
return shouldFilterDataKey(cookieKey, cookieBehavior, SENSITIVE_COOKIE_NAME_SNIPPETS)
310315
? `${cookieKey}=${FILTERED_VALUE}`
311-
: `${cookieKey}=${cookieValue}`,
312-
)
316+
: `${cookieKey}=${cookieValue}`;
317+
})
313318
: [FILTERED_VALUE];
314319
} else {
315320
if (headerBehavior === false) {
@@ -338,22 +343,31 @@ export function httpHeadersToSpanAttributes(
338343
return spanAttributes;
339344
}
340345

346+
/**
347+
* Splits a `Cookie` / `Set-Cookie` header into its name-value pairs.
348+
*
349+
* A segment without an `=` is a nameless cookie, so the bare token is its value (RFC 6265bis):
350+
* it is returned as a pair with an empty name.
351+
*/
341352
function parseCookieHeader(value: string | string[], isSetCookie: boolean): [string, string][] {
342353
// Set-Cookie: one cookie per value, with attributes ("name=value; HttpOnly; Secure")
343-
// Cookie: multiple cookies separated by "; " ("cookie1=value1; cookie2=value2")
354+
// Cookie: multiple cookies separated by ";" (the space after ";" is not guaranteed on the wire)
344355
const cookies = (Array.isArray(value) ? value : [value]).flatMap(headerValue => {
345356
if (typeof headerValue !== 'string' || headerValue === '') {
346357
return [];
347358
}
348-
return isSetCookie ? [headerValue.split(';')[0]!] : headerValue.split('; ');
359+
return isSetCookie ? [headerValue.split(';')[0]!] : headerValue.split(';');
349360
});
350361

351-
return cookies.map(cookie => {
352-
const equalSignIndex = cookie.indexOf('=');
353-
return equalSignIndex !== -1
354-
? [cookie.substring(0, equalSignIndex), cookie.substring(equalSignIndex + 1)]
355-
: [cookie, ''];
356-
});
362+
return cookies
363+
.map(cookie => cookie.trim())
364+
.filter(cookie => cookie !== '')
365+
.map(cookie => {
366+
const equalSignIndex = cookie.indexOf('=');
367+
return equalSignIndex !== -1
368+
? [cookie.substring(0, equalSignIndex), cookie.substring(equalSignIndex + 1)]
369+
: ['', cookie];
370+
});
357371
}
358372

359373
/** Extract the query params from an URL. */

‎packages/core/test/lib/utils/data-collection/filterCookies.test.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,24 @@ describe('filterCookies', () => {
7878
expect(filterCookies('', true)).toEqual({});
7979
});
8080

81-
it('returns empty record for string with no key-value pairs', () => {
82-
expect(filterCookies(';;;', true)).toEqual({});
81+
it('filters the whole string when no key-value pairs can be extracted', () => {
82+
expect(filterCookies(';;;', true)).toBe('[Filtered]');
83+
expect(filterCookies('opaque-session-blob', true)).toBe('[Filtered]');
84+
});
85+
});
86+
87+
// Intended behavior for the cookie parsing consolidation follow-up: `Set-Cookie` attributes are
88+
// metadata, not cookies, so they must not show up as key-value pairs. Marked `fails` until the
89+
// shared parser handles them.
90+
describe('Set-Cookie attribute handling (known gaps)', () => {
91+
it.fails('does not report Set-Cookie attributes as cookie pairs', () => {
92+
expect(filterCookies('sid=1; Max-Age=3600; Path=/', true)).toEqual({ sid: '[Filtered]' });
93+
});
94+
95+
it.fails('does not report Expires/Domain attributes as cookie pairs', () => {
96+
expect(filterCookies('theme=dark; Expires=Wed, 21 Oct 2026 07:28:00 GMT; Domain=example.com', true)).toEqual({
97+
theme: 'dark',
98+
});
8399
});
84100
});
85101

‎packages/core/test/lib/utils/request.test.ts‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -650,8 +650,7 @@ describe('request utils', () => {
650650

651651
it('attaches and filters sensitive cookie headers', () => {
652652
const headers = {
653-
Cookie:
654-
'session=abc123; tracking=enabled; cookie-authentication-key-without-value; theme=dark; lang=en; user_session=xyz789; pref=1',
653+
Cookie: 'session=abc123; tracking=enabled; theme=dark; lang=en; user_session=xyz789; pref=1',
655654
};
656655

657656
const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({}));
@@ -660,7 +659,6 @@ describe('request utils', () => {
660659
'http.request.header.cookie': [
661660
'session=[Filtered]',
662661
'tracking=enabled',
663-
'cookie-authentication-key-without-value=[Filtered]',
664662
'theme=dark',
665663
'lang=en',
666664
'user_session=[Filtered]',
@@ -669,6 +667,35 @@ describe('request utils', () => {
669667
});
670668
});
671669

670+
it('filters cookie segments that are not a name=value pair', () => {
671+
// The bare token is a nameless cookie's value, so it must be filtered.
672+
const headers = { Cookie: 'session=abc123; theme=dark; y7Uu0Rk2QpLmXv3' };
673+
674+
const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({}));
675+
676+
expect(result).toEqual({
677+
'http.request.header.cookie': ['session=[Filtered]', 'theme=dark', '[Filtered]'],
678+
});
679+
});
680+
681+
it('filters a cookie header that holds no name=value pair', () => {
682+
const headers = { Cookie: 'y7Uu0Rk2QpLmXv3' };
683+
684+
const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({}));
685+
686+
expect(result).toEqual({ 'http.request.header.cookie': ['[Filtered]'] });
687+
});
688+
689+
it('splits cookies on ";" without a following space', () => {
690+
const headers = { Cookie: 'theme=dark;__Secure-session=abc123' };
691+
692+
const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({}));
693+
694+
expect(result).toEqual({
695+
'http.request.header.cookie': ['theme=dark', '__Secure-session=[Filtered]'],
696+
});
697+
});
698+
672699
it('filters common framework and provider session-style cookie names', () => {
673700
const headers = {
674701
Cookie:
@@ -728,7 +755,8 @@ describe('request utils', () => {
728755
['pref=1; Max-Age=3600', { 'http.request.header.set-cookie': ['pref=1'] }],
729756
['color=blue; Path=/dashboard', { 'http.request.header.set-cookie': ['color=blue'] }],
730757
['token=eyJhbGc=.eyJzdWI=.SflKxw; Secure', { 'http.request.header.set-cookie': ['token=[Filtered]'] }],
731-
['auth_required; HttpOnly', { 'http.request.header.set-cookie': ['auth_required=[Filtered]'] }],
758+
// A set-cookie string without "=" is a nameless cookie: the bare token is its value.
759+
['auth_required; HttpOnly', { 'http.request.header.set-cookie': ['[Filtered]'] }],
732760
['empty=; Secure', { 'http.request.header.set-cookie': ['empty='] }],
733761
])('should parse and filter Set-Cookie header: %s', (setCookieValue, expected) => {
734762
const headers = { 'Set-Cookie': setCookieValue };

‎packages/node/src/integrations/node-fetch/types.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,16 @@ export interface UndiciInstrumentationConfig<RequestType = UndiciRequest, Respon
8787
requestHook?: RequestHookFunction<RequestType>;
8888
/** Function called once response headers have been received */
8989
responseHook?: ResponseHookFunction<RequestType, ResponseType>;
90-
/** Map the following HTTP headers to span attributes. */
90+
/**
91+
* Capture the listed HTTP headers as span attributes
92+
* (`http.request.header.<name>` / `http.response.header.<name>`).
93+
*
94+
* Privacy filtering still applies to every header listed here. A header keeps its value only if
95+
* `dataCollection.httpHeaders` permits it:
96+
* - Sensitive names (`authorization`, `cookie`, ...) always show up as `[Filtered]`.
97+
* - Names on the `deny` list show up as `[Filtered]`.
98+
* - If an `allow` list is configured, a header must appear there as well, or it shows up as `[Filtered]`.
99+
*/
91100
headersToSpanAttributes?: {
92101
requestHeaders?: string[];
93102
responseHeaders?: string[];

‎packages/node/src/integrations/node-fetch/undici-instrumentation.ts‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {
4040
getUrlQuery,
4141
filterCollectedUrl,
4242
filterCollectedUrlQuery,
43+
httpHeadersToSpanAttributes,
4344
} from '@sentry/core';
4445
import { addFetchRequestBreadcrumb, addTracePropagationHeadersToFetchRequest } from '../../utils/outgoingFetchRequest';
4546
import {
@@ -312,16 +313,21 @@ function onRequestHeaders(config: NodeFetchOptions, { request, socket }: Request
312313

313314
// After hooks have been processed (which may modify request headers)
314315
// we can collect the headers based on the configuration
315-
if (config.headersToSpanAttributes?.requestHeaders) {
316+
const client = getClient();
317+
if (config.headersToSpanAttributes?.requestHeaders && client) {
316318
const headersToAttribs = new Set(config.headersToSpanAttributes.requestHeaders.map(n => n.toLowerCase()));
317319
const headersMap = parseRequestHeaders(request);
318320

321+
const allowlisted: Record<string, string | string[]> = {};
319322
for (const [name, value] of headersMap.entries()) {
320323
if (headersToAttribs.has(name)) {
321-
const attrValue = Array.isArray(value) ? value : [value];
322-
spanAttributes[`http.request.header.${name}`] = attrValue;
324+
allowlisted[name] = value;
323325
}
324326
}
327+
328+
// An entry in `headersToSpanAttributes` does not exempt a header from the `dataCollection`
329+
// filtering, so the allowlisted subset goes through the same pipeline as any other header.
330+
Object.assign(spanAttributes, httpHeadersToSpanAttributes(allowlisted, client.getDataCollectionOptions()));
325331
}
326332

327333
span.setAttributes(spanAttributes);
@@ -354,28 +360,31 @@ function onResponseHeaders(config: NodeFetchOptions, { request, response }: Resp
354360
() => undefined,
355361
);
356362

357-
if (config.headersToSpanAttributes?.responseHeaders) {
363+
const client = getClient();
364+
if (config.headersToSpanAttributes?.responseHeaders && client) {
358365
const headersToAttribs = new Set<string>();
359366
config.headersToSpanAttributes?.responseHeaders.forEach(name => headersToAttribs.add(name.toLowerCase()));
360367

368+
const allowlisted: Record<string, string[]> = {};
361369
for (let idx = 0; idx < response.headers.length; idx = idx + 2) {
362370
const nameBuf = response.headers[idx];
363371
const valueBuf = response.headers[idx + 1];
364372
if (nameBuf === undefined || valueBuf === undefined) {
365373
continue;
366374
}
367375
const name = nameBuf.toString().toLowerCase();
368-
const value = valueBuf;
369376

370377
if (headersToAttribs.has(name)) {
371-
const attrName = `http.response.header.${name}`;
372-
if (!Object.prototype.hasOwnProperty.call(spanAttributes, attrName)) {
373-
spanAttributes[attrName] = [value.toString()];
374-
} else {
375-
(spanAttributes[attrName] as string[]).push(value.toString());
376-
}
378+
(allowlisted[name] ??= []).push(valueBuf.toString());
377379
}
378380
}
381+
382+
// An entry in `headersToSpanAttributes` does not exempt a header from the `dataCollection`
383+
// filtering, so the allowlisted subset goes through the same pipeline as any other header.
384+
Object.assign(
385+
spanAttributes,
386+
httpHeadersToSpanAttributes(allowlisted, client.getDataCollectionOptions(), 'response'),
387+
);
379388
}
380389

381390
span.setAttributes(spanAttributes);

0 commit comments

Comments
 (0)