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..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 @@ -29,6 +29,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'], + // 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/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..1c2de81e236b 100644 --- a/packages/core/src/utils/data-collection/filterCookies.ts +++ b/packages/core/src/utils/data-collection/filterCookies.ts @@ -17,8 +17,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 b013f09e8ce6..a486d1796e9d 100644 --- a/packages/core/src/utils/request.ts +++ b/packages/core/src/utils/request.ts @@ -303,8 +303,9 @@ 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)}`] = @@ -343,7 +344,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 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. + */ +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 +361,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..f3b822f945fa 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({})); @@ -662,11 +661,30 @@ describe('request utils', () => { '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', }); }); + 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,7 @@ 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]' }], + ['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..ac113c52484c 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,12 @@ 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; + // 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]; } } } @@ -370,7 +376,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());