From 0f51828d75b64b6d4e039b20277b78a2e1249985 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:07:07 +0200 Subject: [PATCH 1/5] fix(core): Apply the sensitive denylist to cookie headers and configured fetch headers Co-Authored-By: Claude Opus 5 --- .../instrument.mjs | 2 +- .../scenario.mjs | 4 +++- .../fetch-headers-to-span-attributes/test.ts | 5 +++- packages/core/src/index.ts | 6 ++++- .../utils/data-collection/filterCookies.ts | 4 +++- packages/core/src/utils/request.ts | 23 +++++++++++++------ .../data-collection/filterCookies.test.ts | 5 ++-- packages/core/test/lib/utils/request.test.ts | 7 +++--- .../node-fetch/undici-instrumentation.ts | 15 +++++++++--- 9 files changed, 51 insertions(+), 20 deletions(-) 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..9e3aa401939d 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 @@ -10,7 +10,7 @@ Sentry.init({ integrations: [ Sentry.nativeNodeFetchIntegration({ headersToSpanAttributes: { - requestHeaders: ['x-test-header'], + requestHeaders: ['x-test-header', 'authorization'], responseHeaders: ['x-powered-by'], }, }), 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..4d0731416fc2 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' }, + }); }); 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..624ade1e2157 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 @@ -9,11 +9,12 @@ describe('outgoing fetch spans - headers to span attributes', () => { createCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { test('maps configured request & response headers to span attributes', async () => { - expect.assertions(2); + expect.assertions(3); const [SERVER_URL, closeTestServer] = await createTestServer() .get('/api/v0', headers => { expect(headers['x-test-header']).toBe('test-value'); + expect(headers['authorization']).toBe('Bearer super-secret'); }) .start(); @@ -29,6 +30,8 @@ describe('outgoing fetch spans - headers to span attributes', () => { origin: 'auto.http.node_fetch', data: expect.objectContaining({ 'http.request.header.x-test-header': ['test-value'], + // Listing a header explicitly does not exempt it from the sensitive denylist. + 'http.request.header.authorization': '[Filtered]', 'http.response.header.x-powered-by': ['Express'], }), }), diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index dc01c23fda8f..c6c77ad9893f 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -76,7 +76,11 @@ export { _INTERNAL_shouldSkipAiProviderWrapping, _INTERNAL_clearAiProviderSkips, } from './utils/ai/providerSkip'; -export { filterKeyValueData as _INTERNAL_filterKeyValueData } from './utils/data-collection/filterKeyValueData'; +export { + filterKeyValueData as _INTERNAL_filterKeyValueData, + shouldFilterDataKey as _INTERNAL_shouldFilterDataKey, +} from './utils/data-collection/filterKeyValueData'; +export { FILTERED_VALUE as _INTERNAL_FILTERED_VALUE } from './utils/data-collection/filtering-snippets'; export { filterCookies as _INTERNAL_filterCookies } from './utils/data-collection/filterCookies'; export { filterQueryParams as _INTERNAL_filterQueryParams } from './utils/data-collection/filterQueryParams'; export { filterCollectedUrl, filterCollectedUrlQuery } from './utils/data-collection/filterCollectedUrl'; diff --git a/packages/core/src/utils/data-collection/filterCookies.ts b/packages/core/src/utils/data-collection/filterCookies.ts index ad18d67fe14a..714abcf25ad4 100644 --- a/packages/core/src/utils/data-collection/filterCookies.ts +++ b/packages/core/src/utils/data-collection/filterCookies.ts @@ -17,8 +17,10 @@ export function filterCookies(cookieString: string, behavior: CollectBehavior): try { const parsed = parseCookie(cookieString); + // An opaque or malformed cookie string yields no pairs; the spec requires the whole value to be + // filtered rather than silently dropped. 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 b013f09e8ce6..32c716f1c98f 100644 --- a/packages/core/src/utils/request.ts +++ b/packages/core/src/utils/request.ts @@ -303,14 +303,16 @@ export function httpHeadersToSpanAttributes( continue; } - if (typeof value === 'string' && value !== '') { - const parsed = parseCookieHeader(value, lowerKey === 'set-cookie'); + const parsed = + typeof value === 'string' && value !== '' ? parseCookieHeader(value, lowerKey === 'set-cookie') : undefined; + if (parsed) { const filtered = filterKeyValueData(parsed, cookieBehavior, SENSITIVE_COOKIE_NAME_SNIPPETS); for (const [cookieKey, cookieValue] of Object.entries(filtered)) { spanAttributes[`${prefix}${normalizeAttributeKey(lowerKey)}.${normalizeAttributeKey(cookieKey)}`] = cookieValue; } } else { + // Per spec, a cookie header we cannot split into key-value pairs is filtered as a whole. spanAttributes[`${prefix}${normalizeAttributeKey(lowerKey)}`] = FILTERED_VALUE; } } else { @@ -343,7 +345,14 @@ function normalizeAttributeKey(key: string): string { return key.replace(/-/g, '_'); } -function parseCookieHeader(value: string, isSetCookie: boolean): Record { +/** + * Splits a `Cookie` / `Set-Cookie` header into its individual name-value pairs. + * + * Segments that are not a `name=value` pair are dropped rather than emitted as a key: an opaque + * cookie string used as an attribute key cannot be scrubbed by any denylist. When nothing parses, + * `undefined` signals the caller to filter the header as a whole. + */ +function parseCookieHeader(value: string, isSetCookie: boolean): Record | undefined { // Set-Cookie: single cookie with attributes ("name=value; HttpOnly; Secure") // Cookie: multiple cookies separated by "; " ("cookie1=value1; cookie2=value2") const semicolonIndex = value.indexOf(';'); @@ -353,11 +362,11 @@ function parseCookieHeader(value: string, isSetCookie: boolean): Record = {}; for (const cookie of cookies) { const equalSignIndex = cookie.indexOf('='); - const cookieKey = (equalSignIndex !== -1 ? cookie.substring(0, equalSignIndex) : cookie).toLowerCase(); - const cookieValue = equalSignIndex !== -1 ? cookie.substring(equalSignIndex + 1) : ''; - result[cookieKey] = cookieValue; + if (equalSignIndex > 0) { + result[cookie.substring(0, equalSignIndex).toLowerCase()] = cookie.substring(equalSignIndex + 1); + } } - return result; + return Object.keys(result).length > 0 ? result : undefined; } /** 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..4d5e00aadd46 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,9 @@ 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]'); }); }); diff --git a/packages/core/test/lib/utils/request.test.ts b/packages/core/test/lib/utils/request.test.ts index 4db75d5a96ff..06ffc5793e23 100644 --- a/packages/core/test/lib/utils/request.test.ts +++ b/packages/core/test/lib/utils/request.test.ts @@ -648,7 +648,7 @@ describe('request utils', () => { }); }); - it('attaches and filters sensitive cookie headers', () => { + it('attaches and filters sensitive cookie headers, dropping segments that are not key-value pairs', () => { const headers = { Cookie: 'session=abc123; tracking=enabled; cookie-authentication-key-without-value; theme=dark; lang=en; user_session=xyz789; pref=1', @@ -656,13 +656,13 @@ describe('request utils', () => { const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({})); + // The valueless segment is dropped: as an attribute key it could not be scrubbed. expect(result).toEqual({ 'http.request.header.cookie.session': '[Filtered]', 'http.request.header.cookie.tracking': 'enabled', 'http.request.header.cookie.theme': 'dark', 'http.request.header.cookie.lang': 'en', 'http.request.header.cookie.user_session': '[Filtered]', - 'http.request.header.cookie.cookie_authentication_key_without_value': '[Filtered]', 'http.request.header.cookie.pref': '1', }); }); @@ -725,7 +725,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]' }], + // No `name=value` pair to extract, so the whole header falls back to the filtered 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/undici-instrumentation.ts b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts index b2989e6c6bdf..77f92785a821 100644 --- a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts +++ b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts @@ -40,6 +40,8 @@ import { getUrlQuery, filterCollectedUrl, filterCollectedUrlQuery, + _INTERNAL_shouldFilterDataKey, + _INTERNAL_FILTERED_VALUE, } from '@sentry/core'; import { addFetchRequestBreadcrumb, addTracePropagationHeadersToFetchRequest } from '../../utils/outgoingFetchRequest'; import { @@ -319,8 +321,13 @@ function onRequestHeaders(config: NodeFetchOptions, { request, socket }: Request for (const [name, value] of headersMap.entries()) { if (headersToAttribs.has(name)) { - const attrValue = Array.isArray(value) ? value : [value]; - spanAttributes[`http.request.header.${name}`] = attrValue; + // The sensitive denylist applies even to explicitly listed headers, so an `authorization` + // entry in `headersToSpanAttributes` still reports as `[Filtered]`. + spanAttributes[`http.request.header.${name}`] = _INTERNAL_shouldFilterDataKey(name, true) + ? _INTERNAL_FILTERED_VALUE + : Array.isArray(value) + ? value + : [value]; } } } @@ -370,7 +377,9 @@ function onResponseHeaders(config: NodeFetchOptions, { request, response }: Resp if (headersToAttribs.has(name)) { const attrName = `http.response.header.${name}`; - if (!Object.prototype.hasOwnProperty.call(spanAttributes, attrName)) { + if (_INTERNAL_shouldFilterDataKey(name, true)) { + spanAttributes[attrName] = _INTERNAL_FILTERED_VALUE; + } else if (!Object.prototype.hasOwnProperty.call(spanAttributes, attrName)) { spanAttributes[attrName] = [value.toString()]; } else { (spanAttributes[attrName] as string[]).push(value.toString()); From cde83e3656fc69f6e7d3ef4b60b2a7b2155cbf1c Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:36:53 +0200 Subject: [PATCH 2/5] test: Split the cookie-segment cases and trim comments Co-Authored-By: Claude Opus 5 --- .../fetch-headers-to-span-attributes/test.ts | 5 ++-- .../utils/data-collection/filterCookies.ts | 3 +-- packages/core/src/utils/request.ts | 9 +++---- packages/core/test/lib/utils/request.test.ts | 27 +++++++++++++++---- .../node-fetch/undici-instrumentation.ts | 3 +-- 5 files changed, 30 insertions(+), 17 deletions(-) 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 624ade1e2157..59badb025916 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 @@ -9,12 +9,11 @@ describe('outgoing fetch spans - headers to span attributes', () => { createCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { test('maps configured request & response headers to span attributes', async () => { - expect.assertions(3); + expect.assertions(2); const [SERVER_URL, closeTestServer] = await createTestServer() .get('/api/v0', headers => { expect(headers['x-test-header']).toBe('test-value'); - expect(headers['authorization']).toBe('Bearer super-secret'); }) .start(); @@ -30,7 +29,7 @@ describe('outgoing fetch spans - headers to span attributes', () => { origin: 'auto.http.node_fetch', data: expect.objectContaining({ 'http.request.header.x-test-header': ['test-value'], - // Listing a header explicitly does not exempt it from the sensitive denylist. + // Listed in `headersToSpanAttributes`, but the denylist still wins. 'http.request.header.authorization': '[Filtered]', 'http.response.header.x-powered-by': ['Express'], }), diff --git a/packages/core/src/utils/data-collection/filterCookies.ts b/packages/core/src/utils/data-collection/filterCookies.ts index 714abcf25ad4..1c2de81e236b 100644 --- a/packages/core/src/utils/data-collection/filterCookies.ts +++ b/packages/core/src/utils/data-collection/filterCookies.ts @@ -17,8 +17,7 @@ export function filterCookies(cookieString: string, behavior: CollectBehavior): try { const parsed = parseCookie(cookieString); - // An opaque or malformed cookie string yields no pairs; the spec requires the whole value to be - // filtered rather than silently dropped. + // 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 cookieString ? FILTERED : {}; } diff --git a/packages/core/src/utils/request.ts b/packages/core/src/utils/request.ts index 32c716f1c98f..a486d1796e9d 100644 --- a/packages/core/src/utils/request.ts +++ b/packages/core/src/utils/request.ts @@ -312,7 +312,6 @@ export function httpHeadersToSpanAttributes( cookieValue; } } else { - // Per spec, a cookie header we cannot split into key-value pairs is filtered as a whole. spanAttributes[`${prefix}${normalizeAttributeKey(lowerKey)}`] = FILTERED_VALUE; } } else { @@ -346,11 +345,11 @@ function normalizeAttributeKey(key: string): string { } /** - * Splits a `Cookie` / `Set-Cookie` header into its individual name-value pairs. + * Splits a `Cookie` / `Set-Cookie` header into its name-value pairs, or returns `undefined` when it + * holds none. * - * Segments that are not a `name=value` pair are dropped rather than emitted as a key: an opaque - * cookie string used as an attribute key cannot be scrubbed by any denylist. When nothing parses, - * `undefined` signals the caller to filter the header as a whole. + * A segment without an `=` is dropped. It would otherwise become the attribute key itself, and no + * denylist can scrub a key. */ function parseCookieHeader(value: string, isSetCookie: boolean): Record | undefined { // Set-Cookie: single cookie with attributes ("name=value; HttpOnly; Secure") diff --git a/packages/core/test/lib/utils/request.test.ts b/packages/core/test/lib/utils/request.test.ts index 06ffc5793e23..f3b822f945fa 100644 --- a/packages/core/test/lib/utils/request.test.ts +++ b/packages/core/test/lib/utils/request.test.ts @@ -648,15 +648,13 @@ describe('request utils', () => { }); }); - it('attaches and filters sensitive cookie headers, dropping segments that are not key-value pairs', () => { + 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({})); - // The valueless segment is dropped: as an attribute key it could not be scrubbed. expect(result).toEqual({ 'http.request.header.cookie.session': '[Filtered]', 'http.request.header.cookie.tracking': 'enabled', @@ -667,6 +665,26 @@ describe('request utils', () => { }); }); + it('drops cookie segments that are not a name=value pair', () => { + // The segment would become the attribute key, and keys are never scrubbed. + const headers = { Cookie: 'session=abc123; theme=dark; y7Uu0Rk2QpLmXv3' }; + + const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({})); + + expect(result).toEqual({ + 'http.request.header.cookie.session': '[Filtered]', + 'http.request.header.cookie.theme': 'dark', + }); + }); + + it('filters the whole cookie header when it holds no name=value pair', () => { + const headers = { Cookie: 'y7Uu0Rk2QpLmXv3' }; + + const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({})); + + expect(result).toEqual({ 'http.request.header.cookie': '[Filtered]' }); + }); + it('filters common framework and provider session-style cookie names', () => { const headers = { Cookie: @@ -725,7 +743,6 @@ 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]' }], - // No `name=value` pair to extract, so the whole header falls back to the filtered 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) => { diff --git a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts index 77f92785a821..ac113c52484c 100644 --- a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts +++ b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts @@ -321,8 +321,7 @@ function onRequestHeaders(config: NodeFetchOptions, { request, socket }: Request for (const [name, value] of headersMap.entries()) { if (headersToAttribs.has(name)) { - // The sensitive denylist applies even to explicitly listed headers, so an `authorization` - // entry in `headersToSpanAttributes` still reports as `[Filtered]`. + // An allowlist entry does not exempt a header from the denylist. spanAttributes[`http.request.header.${name}`] = _INTERNAL_shouldFilterDataKey(name, true) ? _INTERNAL_FILTERED_VALUE : Array.isArray(value) From 7ec524dd53ee67bad5cd17c26950a5b6379fa847 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 7 Sep 2026 09:50:52 +0200 Subject: [PATCH 3/5] docs: Explain why a cookie segment can lack an `=` Co-Authored-By: Claude Opus 5 --- packages/core/src/utils/request.ts | 4 ++-- packages/core/test/lib/utils/request.test.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/src/utils/request.ts b/packages/core/src/utils/request.ts index a486d1796e9d..9e028152439d 100644 --- a/packages/core/src/utils/request.ts +++ b/packages/core/src/utils/request.ts @@ -348,8 +348,8 @@ function normalizeAttributeKey(key: string): string { * Splits a `Cookie` / `Set-Cookie` header into its name-value pairs, or returns `undefined` when it * holds none. * - * A segment without an `=` is dropped. It would otherwise become the attribute key itself, and no - * denylist can scrub a key. + * A segment without an `=` is a nameless cookie, so the bare token is its value. Dropping it keeps + * that value out of the attribute key, where no denylist could reach it. */ function parseCookieHeader(value: string, isSetCookie: boolean): Record | undefined { // Set-Cookie: single cookie with attributes ("name=value; HttpOnly; Secure") diff --git a/packages/core/test/lib/utils/request.test.ts b/packages/core/test/lib/utils/request.test.ts index f3b822f945fa..6ae1e1c5294f 100644 --- a/packages/core/test/lib/utils/request.test.ts +++ b/packages/core/test/lib/utils/request.test.ts @@ -666,7 +666,7 @@ describe('request utils', () => { }); it('drops cookie segments that are not a name=value pair', () => { - // The segment would become the attribute key, and keys are never scrubbed. + // The bare token is a nameless cookie's value, so it must not become the attribute key. const headers = { Cookie: 'session=abc123; theme=dark; y7Uu0Rk2QpLmXv3' }; const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({})); From 8babbb54544b1cd1c6eabf3f7f37b7cd1a43e5cb Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Thu, 17 Sep 2026 11:46:54 +0200 Subject: [PATCH 4/5] updates for array cookies --- .../fetch-headers-to-span-attributes/test.ts | 2 +- .../browser/src/integrations/httpclient.ts | 12 +--- packages/core/src/utils/request.ts | 67 +++++++------------ packages/core/test/lib/utils/request.test.ts | 25 ++++--- .../node-fetch/undici-instrumentation.ts | 4 +- 5 files changed, 47 insertions(+), 63 deletions(-) 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 59badb025916..a98e50d69aea 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 @@ -30,7 +30,7 @@ describe('outgoing fetch spans - headers to span attributes', () => { data: expect.objectContaining({ 'http.request.header.x-test-header': ['test-value'], // Listed in `headersToSpanAttributes`, but the denylist still wins. - 'http.request.header.authorization': '[Filtered]', + 'http.request.header.authorization': ['[Filtered]'], 'http.response.header.x-powered-by': ['Express'], }), }), 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/request.ts b/packages/core/src/utils/request.ts index 3c8290cd5cc8..7c6ae29a3304 100644 --- a/packages/core/src/utils/request.ts +++ b/packages/core/src/utils/request.ts @@ -303,28 +303,18 @@ export function httpHeadersToSpanAttributes( continue; } - /* previous approach - - const parsed = - typeof value === 'string' && value !== '' ? parseCookieHeader(value, lowerKey === 'set-cookie') : undefined; - if (parsed) { - const filtered = filterKeyValueData(parsed, cookieBehavior, SENSITIVE_COOKIE_NAME_SNIPPETS); - for (const [cookieKey, cookieValue] of Object.entries(filtered)) { - spanAttributes[`${prefix}${normalizeAttributeKey(lowerKey)}.${normalizeAttributeKey(cookieKey)}`] = - cookieValue; - } - } else { - spanAttributes[`${prefix}${normalizeAttributeKey(lowerKey)}`] = FILTERED_VALUE; - } - */ - 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) { @@ -353,40 +343,31 @@ export function httpHeadersToSpanAttributes( return spanAttributes; } -/** TODO: update this as this is the description before we sent cookie header strings - * Splits a `Cookie` / `Set-Cookie` header into its name-value pairs, or returns `undefined` when it - * holds none. +/** + * 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. Dropping it keeps - * that value out of the attribute key, where no denylist could reach it. + * 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 cookies.map(cookie => { - const equalSignIndex = cookie.indexOf('='); - return equalSignIndex !== -1 - ? [cookie.substring(0, equalSignIndex), cookie.substring(equalSignIndex + 1)] - : [cookie, '']; + return isSetCookie ? [headerValue.split(';')[0]!] : headerValue.split(';'); }); - /* previous - const result: Record = {}; - for (const cookie of cookies) { - const equalSignIndex = cookie.indexOf('='); - if (equalSignIndex > 0) { - result[cookie.substring(0, equalSignIndex).toLowerCase()] = cookie.substring(equalSignIndex + 1); - } - } - return Object.keys(result).length > 0 ? result : undefined; - */ + 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/request.test.ts b/packages/core/test/lib/utils/request.test.ts index 3de4eb76d960..58e29ad12ddd 100644 --- a/packages/core/test/lib/utils/request.test.ts +++ b/packages/core/test/lib/utils/request.test.ts @@ -659,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]', @@ -668,24 +667,33 @@ describe('request utils', () => { }); }); - it('drops cookie segments that are not a name=value pair', () => { - // The bare token is a nameless cookie's value, so it must not become the attribute key. + 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]', - 'http.request.header.cookie.theme': 'dark', + 'http.request.header.cookie': ['session=[Filtered]', 'theme=dark', '[Filtered]'], }); }); - it('filters the whole cookie header when it holds no name=value pair', () => { + 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]' }); + 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', () => { @@ -747,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/undici-instrumentation.ts b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts index 06c96be2ed43..c5ab15159eae 100644 --- a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts +++ b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts @@ -322,7 +322,7 @@ function onRequestHeaders(config: NodeFetchOptions, { request, socket }: Request if (headersToAttribs.has(name)) { // An allowlist entry does not exempt a header from the denylist. spanAttributes[`http.request.header.${name}`] = _INTERNAL_shouldFilterDataKey(name, true) - ? _INTERNAL_FILTERED_VALUE + ? [_INTERNAL_FILTERED_VALUE] : Array.isArray(value) ? value : [value]; @@ -376,7 +376,7 @@ function onResponseHeaders(config: NodeFetchOptions, { request, response }: Resp if (headersToAttribs.has(name)) { const attrName = `http.response.header.${name}`; if (_INTERNAL_shouldFilterDataKey(name, true)) { - spanAttributes[attrName] = _INTERNAL_FILTERED_VALUE; + spanAttributes[attrName] = [_INTERNAL_FILTERED_VALUE]; } else if (!Object.prototype.hasOwnProperty.call(spanAttributes, attrName)) { spanAttributes[attrName] = [value.toString()]; } else { From bf324ae0ee555fc71b95320e548df9708295cae5 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 18 Sep 2026 13:41:55 +0200 Subject: [PATCH 5/5] pass allow/denylists --- .../instrument.mjs | 10 ++++- .../scenario.mjs | 2 +- .../fetch-headers-to-span-attributes/test.ts | 5 ++- packages/core/src/index.ts | 6 +-- .../utils/data-collection/filterCookies.ts | 2 + .../data-collection/filterCookies.test.ts | 15 +++++++ .../node/src/integrations/node-fetch/types.ts | 11 +++++- .../node-fetch/undici-instrumentation.ts | 39 ++++++++++--------- 8 files changed, 61 insertions(+), 29 deletions(-) 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 9e3aa401939d..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', 'authorization'], - 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 4d0731416fc2..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 @@ -3,6 +3,6 @@ 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', authorization: 'Bearer super-secret' }, + 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 a98e50d69aea..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,9 +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 denylist still wins. + // 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/core/src/index.ts b/packages/core/src/index.ts index be4a8b35bfab..0146e82a11fa 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -76,11 +76,7 @@ export { _INTERNAL_shouldSkipAiProviderWrapping, _INTERNAL_clearAiProviderSkips, } from './utils/ai/providerSkip'; -export { - filterKeyValueData as _INTERNAL_filterKeyValueData, - shouldFilterDataKey as _INTERNAL_shouldFilterDataKey, -} from './utils/data-collection/filterKeyValueData'; -export { FILTERED_VALUE as _INTERNAL_FILTERED_VALUE } from './utils/data-collection/filtering-snippets'; +export { filterKeyValueData as _INTERNAL_filterKeyValueData } from './utils/data-collection/filterKeyValueData'; export { filterCookies as _INTERNAL_filterCookies } from './utils/data-collection/filterCookies'; export { filterQueryParams as _INTERNAL_filterQueryParams } from './utils/data-collection/filterQueryParams'; export { filterCollectedUrl, filterCollectedUrlQuery } from './utils/data-collection/filterCollectedUrl'; diff --git a/packages/core/src/utils/data-collection/filterCookies.ts b/packages/core/src/utils/data-collection/filterCookies.ts index 1c2de81e236b..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) { 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 4d5e00aadd46..4f8ed3d57fba 100644 --- a/packages/core/test/lib/utils/data-collection/filterCookies.test.ts +++ b/packages/core/test/lib/utils/data-collection/filterCookies.test.ts @@ -84,6 +84,21 @@ describe('filterCookies', () => { }); }); + // 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', + }); + }); + }); + describe('edge cases', () => { it('handles cookies with = in the value', () => { const result = filterCookies('data=base64==; theme=light', true); 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 c5ab15159eae..89eed4800d46 100644 --- a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts +++ b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts @@ -40,8 +40,7 @@ import { getUrlQuery, filterCollectedUrl, filterCollectedUrlQuery, - _INTERNAL_shouldFilterDataKey, - _INTERNAL_FILTERED_VALUE, + httpHeadersToSpanAttributes, } from '@sentry/core'; import { addFetchRequestBreadcrumb, addTracePropagationHeadersToFetchRequest } from '../../utils/outgoingFetchRequest'; import { @@ -314,20 +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)) { - // An allowlist entry does not exempt a header from the denylist. - spanAttributes[`http.request.header.${name}`] = _INTERNAL_shouldFilterDataKey(name, true) - ? [_INTERNAL_FILTERED_VALUE] - : Array.isArray(value) - ? value - : [value]; + 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); @@ -360,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]; @@ -371,19 +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 (_INTERNAL_shouldFilterDataKey(name, true)) { - spanAttributes[attrName] = [_INTERNAL_FILTERED_VALUE]; - } else 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);