diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/instrument.mjs index bd934b7a9c2b..11ff9c8eb451 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/instrument.mjs @@ -7,11 +7,17 @@ Sentry.init({ release: '1.0', tracesSampleRate: 1.0, transport: loggingTransport, + dataCollection: { + httpHeaders: { + request: { deny: ['x-tenant-id'] }, + response: { deny: ['content-length'] }, + }, + }, integrations: [ Sentry.nativeNodeFetchIntegration({ headersToSpanAttributes: { - requestHeaders: ['x-test-header'], - responseHeaders: ['x-powered-by'], + requestHeaders: ['x-test-header', 'authorization', 'x-tenant-id'], + responseHeaders: ['x-powered-by', 'content-length'], }, }), ], diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.mjs index 0edf81a9a50a..9f5650834bd4 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.mjs @@ -2,5 +2,7 @@ import * as Sentry from '@sentry/node'; // eslint-disable-next-line @typescript-eslint/no-floating-promises Sentry.startSpan({ name: 'test_transaction' }, async () => { - await fetch(`${process.env.SERVER_URL}/api/v0`, { headers: { 'x-test-header': 'test-value' } }); + await fetch(`${process.env.SERVER_URL}/api/v0`, { + headers: { 'x-test-header': 'test-value', authorization: 'Bearer super-secret', 'x-tenant-id': 'acme-corp' }, + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts index d17d0a4132fe..6bfb18909525 100644 --- a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts @@ -29,7 +29,12 @@ describe('outgoing fetch spans - headers to span attributes', () => { origin: 'auto.http.node_fetch', data: expect.objectContaining({ 'http.request.header.x-test-header': ['test-value'], + // Listed in `headersToSpanAttributes`, but the built-in denylist still wins. + 'http.request.header.authorization': ['[Filtered]'], + // Listed in `headersToSpanAttributes`, but denied via `dataCollection.httpHeaders`. + 'http.request.header.x-tenant-id': ['[Filtered]'], 'http.response.header.x-powered-by': ['Express'], + 'http.response.header.content-length': ['[Filtered]'], }), }), ]), diff --git a/packages/browser/src/integrations/httpclient.ts b/packages/browser/src/integrations/httpclient.ts index f9d01c5719b5..a47725b3d37d 100644 --- a/packages/browser/src/integrations/httpclient.ts +++ b/packages/browser/src/integrations/httpclient.ts @@ -93,16 +93,12 @@ function _fetchResponseHandler( const reqCookieStr = request.headers.get('Cookie') || undefined; if (reqCookieStr) { const filtered = _INTERNAL_filterCookies(reqCookieStr, dc.cookies); - if (typeof filtered === 'object') { - requestCookies = filtered; - } + requestCookies = typeof filtered === 'string' ? { cookie: filtered } : filtered; } const resCookieStr = response.headers.get('Set-Cookie') || undefined; if (resCookieStr) { const filtered = _INTERNAL_filterCookies(resCookieStr, dc.cookies); - if (typeof filtered === 'object') { - responseCookies = filtered; - } + responseCookies = typeof filtered === 'string' ? { 'set-cookie': filtered } : filtered; } } @@ -146,9 +142,7 @@ function _xhrResponseHandler( const cookieString = xhr.getResponseHeader('Set-Cookie') || xhr.getResponseHeader('set-cookie') || undefined; if (cookieString) { const filtered = _INTERNAL_filterCookies(cookieString, dc.cookies); - if (typeof filtered === 'object') { - responseCookies = filtered; - } + responseCookies = typeof filtered === 'string' ? { 'set-cookie': filtered } : filtered; } } catch { // ignore it if parsing fails diff --git a/packages/core/src/utils/data-collection/filterCookies.ts b/packages/core/src/utils/data-collection/filterCookies.ts index ad18d67fe14a..0fc373f4bfce 100644 --- a/packages/core/src/utils/data-collection/filterCookies.ts +++ b/packages/core/src/utils/data-collection/filterCookies.ts @@ -8,6 +8,8 @@ import { filterKeyValueData } from './filterKeyValueData'; * * When individual cookies can be parsed, each key-value pair is filtered * independently. When parsing fails, the entire string is replaced with `[Filtered]`. + * A nameless segment inside an otherwise parseable string (`"opaque-blob; theme=dark"`) is + * dropped, since a record key cannot carry a `[Filtered]` marker without leaking the token. */ export function filterCookies(cookieString: string, behavior: CollectBehavior): Record | string { if (behavior === false) { @@ -17,8 +19,9 @@ export function filterCookies(cookieString: string, behavior: CollectBehavior): try { const parsed = parseCookie(cookieString); + // A non-empty string we cannot parse may still hold a session token, so it counts as sensitive. if (Object.keys(parsed).length === 0) { - return {}; + return cookieString ? FILTERED : {}; } return filterKeyValueData(parsed, behavior, SENSITIVE_COOKIE_NAME_SNIPPETS); diff --git a/packages/core/src/utils/request.ts b/packages/core/src/utils/request.ts index 932a10f652b8..7c6ae29a3304 100644 --- a/packages/core/src/utils/request.ts +++ b/packages/core/src/utils/request.ts @@ -305,11 +305,16 @@ export function httpHeadersToSpanAttributes( const cookies = parseCookieHeader(value, lowerKey === 'set-cookie'); spanAttributes[`${prefix}${lowerKey}`] = cookies.length - ? cookies.map(([cookieKey, cookieValue]) => - shouldFilterDataKey(cookieKey, cookieBehavior, SENSITIVE_COOKIE_NAME_SNIPPETS) + ? cookies.map(([cookieKey, cookieValue]) => { + // A nameless cookie's bare token is its value; no denylist could match it, so it is + // always filtered. + if (cookieKey === '') { + return FILTERED_VALUE; + } + return shouldFilterDataKey(cookieKey, cookieBehavior, SENSITIVE_COOKIE_NAME_SNIPPETS) ? `${cookieKey}=${FILTERED_VALUE}` - : `${cookieKey}=${cookieValue}`, - ) + : `${cookieKey}=${cookieValue}`; + }) : [FILTERED_VALUE]; } else { if (headerBehavior === false) { @@ -338,22 +343,31 @@ export function httpHeadersToSpanAttributes( return spanAttributes; } +/** + * Splits a `Cookie` / `Set-Cookie` header into its name-value pairs. + * + * A segment without an `=` is a nameless cookie, so the bare token is its value (RFC 6265bis): + * it is returned as a pair with an empty name. + */ function parseCookieHeader(value: string | string[], isSetCookie: boolean): [string, string][] { // Set-Cookie: one cookie per value, with attributes ("name=value; HttpOnly; Secure") - // Cookie: multiple cookies separated by "; " ("cookie1=value1; cookie2=value2") + // Cookie: multiple cookies separated by ";" (the space after ";" is not guaranteed on the wire) const cookies = (Array.isArray(value) ? value : [value]).flatMap(headerValue => { if (typeof headerValue !== 'string' || headerValue === '') { return []; } - return isSetCookie ? [headerValue.split(';')[0]!] : headerValue.split('; '); + return isSetCookie ? [headerValue.split(';')[0]!] : headerValue.split(';'); }); - return cookies.map(cookie => { - const equalSignIndex = cookie.indexOf('='); - return equalSignIndex !== -1 - ? [cookie.substring(0, equalSignIndex), cookie.substring(equalSignIndex + 1)] - : [cookie, '']; - }); + return cookies + .map(cookie => cookie.trim()) + .filter(cookie => cookie !== '') + .map(cookie => { + const equalSignIndex = cookie.indexOf('='); + return equalSignIndex !== -1 + ? [cookie.substring(0, equalSignIndex), cookie.substring(equalSignIndex + 1)] + : ['', cookie]; + }); } /** Extract the query params from an URL. */ diff --git a/packages/core/test/lib/utils/data-collection/filterCookies.test.ts b/packages/core/test/lib/utils/data-collection/filterCookies.test.ts index 11e5a660c1e6..4f8ed3d57fba 100644 --- a/packages/core/test/lib/utils/data-collection/filterCookies.test.ts +++ b/packages/core/test/lib/utils/data-collection/filterCookies.test.ts @@ -78,8 +78,24 @@ describe('filterCookies', () => { expect(filterCookies('', true)).toEqual({}); }); - it('returns empty record for string with no key-value pairs', () => { - expect(filterCookies(';;;', true)).toEqual({}); + it('filters the whole string when no key-value pairs can be extracted', () => { + expect(filterCookies(';;;', true)).toBe('[Filtered]'); + expect(filterCookies('opaque-session-blob', true)).toBe('[Filtered]'); + }); + }); + + // Intended behavior for the cookie parsing consolidation follow-up: `Set-Cookie` attributes are + // metadata, not cookies, so they must not show up as key-value pairs. Marked `fails` until the + // shared parser handles them. + describe('Set-Cookie attribute handling (known gaps)', () => { + it.fails('does not report Set-Cookie attributes as cookie pairs', () => { + expect(filterCookies('sid=1; Max-Age=3600; Path=/', true)).toEqual({ sid: '[Filtered]' }); + }); + + it.fails('does not report Expires/Domain attributes as cookie pairs', () => { + expect(filterCookies('theme=dark; Expires=Wed, 21 Oct 2026 07:28:00 GMT; Domain=example.com', true)).toEqual({ + theme: 'dark', + }); }); }); diff --git a/packages/core/test/lib/utils/request.test.ts b/packages/core/test/lib/utils/request.test.ts index 2041e06ec3da..58e29ad12ddd 100644 --- a/packages/core/test/lib/utils/request.test.ts +++ b/packages/core/test/lib/utils/request.test.ts @@ -650,8 +650,7 @@ describe('request utils', () => { it('attaches and filters sensitive cookie headers', () => { const headers = { - Cookie: - 'session=abc123; tracking=enabled; cookie-authentication-key-without-value; theme=dark; lang=en; user_session=xyz789; pref=1', + Cookie: 'session=abc123; tracking=enabled; theme=dark; lang=en; user_session=xyz789; pref=1', }; const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({})); @@ -660,7 +659,6 @@ describe('request utils', () => { 'http.request.header.cookie': [ 'session=[Filtered]', 'tracking=enabled', - 'cookie-authentication-key-without-value=[Filtered]', 'theme=dark', 'lang=en', 'user_session=[Filtered]', @@ -669,6 +667,35 @@ describe('request utils', () => { }); }); + it('filters cookie segments that are not a name=value pair', () => { + // The bare token is a nameless cookie's value, so it must be filtered. + const headers = { Cookie: 'session=abc123; theme=dark; y7Uu0Rk2QpLmXv3' }; + + const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({})); + + expect(result).toEqual({ + 'http.request.header.cookie': ['session=[Filtered]', 'theme=dark', '[Filtered]'], + }); + }); + + it('filters a cookie header that holds no name=value pair', () => { + const headers = { Cookie: 'y7Uu0Rk2QpLmXv3' }; + + const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({})); + + expect(result).toEqual({ 'http.request.header.cookie': ['[Filtered]'] }); + }); + + it('splits cookies on ";" without a following space', () => { + const headers = { Cookie: 'theme=dark;__Secure-session=abc123' }; + + const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({})); + + expect(result).toEqual({ + 'http.request.header.cookie': ['theme=dark', '__Secure-session=[Filtered]'], + }); + }); + it('filters common framework and provider session-style cookie names', () => { const headers = { Cookie: @@ -728,7 +755,8 @@ describe('request utils', () => { ['pref=1; Max-Age=3600', { 'http.request.header.set-cookie': ['pref=1'] }], ['color=blue; Path=/dashboard', { 'http.request.header.set-cookie': ['color=blue'] }], ['token=eyJhbGc=.eyJzdWI=.SflKxw; Secure', { 'http.request.header.set-cookie': ['token=[Filtered]'] }], - ['auth_required; HttpOnly', { 'http.request.header.set-cookie': ['auth_required=[Filtered]'] }], + // A set-cookie string without "=" is a nameless cookie: the bare token is its value. + ['auth_required; HttpOnly', { 'http.request.header.set-cookie': ['[Filtered]'] }], ['empty=; Secure', { 'http.request.header.set-cookie': ['empty='] }], ])('should parse and filter Set-Cookie header: %s', (setCookieValue, expected) => { const headers = { 'Set-Cookie': setCookieValue }; diff --git a/packages/node/src/integrations/node-fetch/types.ts b/packages/node/src/integrations/node-fetch/types.ts index 4a5d38c5bc86..3f0e6a477537 100644 --- a/packages/node/src/integrations/node-fetch/types.ts +++ b/packages/node/src/integrations/node-fetch/types.ts @@ -87,7 +87,16 @@ export interface UndiciInstrumentationConfig; /** Function called once response headers have been received */ responseHook?: ResponseHookFunction; - /** Map the following HTTP headers to span attributes. */ + /** + * Capture the listed HTTP headers as span attributes + * (`http.request.header.` / `http.response.header.`). + * + * Privacy filtering still applies to every header listed here. A header keeps its value only if + * `dataCollection.httpHeaders` permits it: + * - Sensitive names (`authorization`, `cookie`, ...) always show up as `[Filtered]`. + * - Names on the `deny` list show up as `[Filtered]`. + * - If an `allow` list is configured, a header must appear there as well, or it shows up as `[Filtered]`. + */ headersToSpanAttributes?: { requestHeaders?: string[]; responseHeaders?: string[]; diff --git a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts index 62fc4992ef4f..89eed4800d46 100644 --- a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts +++ b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts @@ -40,6 +40,7 @@ import { getUrlQuery, filterCollectedUrl, filterCollectedUrlQuery, + httpHeadersToSpanAttributes, } from '@sentry/core'; import { addFetchRequestBreadcrumb, addTracePropagationHeadersToFetchRequest } from '../../utils/outgoingFetchRequest'; import { @@ -312,16 +313,21 @@ function onRequestHeaders(config: NodeFetchOptions, { request, socket }: Request // After hooks have been processed (which may modify request headers) // we can collect the headers based on the configuration - if (config.headersToSpanAttributes?.requestHeaders) { + const client = getClient(); + if (config.headersToSpanAttributes?.requestHeaders && client) { const headersToAttribs = new Set(config.headersToSpanAttributes.requestHeaders.map(n => n.toLowerCase())); const headersMap = parseRequestHeaders(request); + const allowlisted: Record = {}; for (const [name, value] of headersMap.entries()) { if (headersToAttribs.has(name)) { - const attrValue = Array.isArray(value) ? value : [value]; - spanAttributes[`http.request.header.${name}`] = attrValue; + allowlisted[name] = value; } } + + // An entry in `headersToSpanAttributes` does not exempt a header from the `dataCollection` + // filtering, so the allowlisted subset goes through the same pipeline as any other header. + Object.assign(spanAttributes, httpHeadersToSpanAttributes(allowlisted, client.getDataCollectionOptions())); } span.setAttributes(spanAttributes); @@ -354,10 +360,12 @@ function onResponseHeaders(config: NodeFetchOptions, { request, response }: Resp () => undefined, ); - if (config.headersToSpanAttributes?.responseHeaders) { + const client = getClient(); + if (config.headersToSpanAttributes?.responseHeaders && client) { const headersToAttribs = new Set(); config.headersToSpanAttributes?.responseHeaders.forEach(name => headersToAttribs.add(name.toLowerCase())); + const allowlisted: Record = {}; for (let idx = 0; idx < response.headers.length; idx = idx + 2) { const nameBuf = response.headers[idx]; const valueBuf = response.headers[idx + 1]; @@ -365,17 +373,18 @@ function onResponseHeaders(config: NodeFetchOptions, { request, response }: Resp continue; } const name = nameBuf.toString().toLowerCase(); - const value = valueBuf; if (headersToAttribs.has(name)) { - const attrName = `http.response.header.${name}`; - if (!Object.prototype.hasOwnProperty.call(spanAttributes, attrName)) { - spanAttributes[attrName] = [value.toString()]; - } else { - (spanAttributes[attrName] as string[]).push(value.toString()); - } + (allowlisted[name] ??= []).push(valueBuf.toString()); } } + + // An entry in `headersToSpanAttributes` does not exempt a header from the `dataCollection` + // filtering, so the allowlisted subset goes through the same pipeline as any other header. + Object.assign( + spanAttributes, + httpHeadersToSpanAttributes(allowlisted, client.getDataCollectionOptions(), 'response'), + ); } span.setAttributes(spanAttributes);