diff --git a/packages/core/src/integrations/requestdata.ts b/packages/core/src/integrations/requestdata.ts index eda1603c809d..27d4b05e825a 100644 --- a/packages/core/src/integrations/requestdata.ts +++ b/packages/core/src/integrations/requestdata.ts @@ -7,12 +7,12 @@ import type { Event } from '../types/event'; import type { IntegrationFn } from '../types/integration'; import type { QueryParams, RequestEventData } from '../types/request'; import type { StreamedSpanJSON } from '../types/span'; -import { parseCookie } from '../utils/cookie'; +import { parseCookiePairs } from '../utils/cookie'; import { SENSITIVE_COOKIE_NAME_SNIPPETS } from '../utils/data-collection/filtering-snippets'; import { filterKeyValueData } from '../utils/data-collection/filterKeyValueData'; import { filterQueryParams } from '../utils/data-collection/filterQueryParams'; import { filterUrlQuery } from '../utils/data-collection/filterUrlQuery'; -import { httpHeadersToSpanAttributes } from '../utils/request'; +import { filterCookiePairs, httpHeadersToSpanAttributes } from '../utils/request'; import { getUrlQuery } from '../utils/url'; import { getClientIPAddress, ipHeaderNames } from '../vendor/getIpAddress'; import { safeSetSpanJSONAttributes } from '../tracing/spans/captureSpan'; @@ -186,11 +186,13 @@ function addNormalizedRequestDataToSpan( // Process cookies before headers so normalizedRequest.cookies takes precedence // over the raw cookie header (matching the processEvent path). if (requestData.cookies && Object.keys(requestData.cookies).length > 0) { - const cookieString = Object.entries(requestData.cookies) - .map(([name, value]) => `${name}=${value}`) - .join('; '); - const cookieAttributes = httpHeadersToSpanAttributes({ cookie: cookieString }, dataCollection, 'request'); - safeSetSpanJSONAttributes(span, cookieAttributes); + const cookieAttributes = filterCookiePairs( + Object.entries(requestData.cookies).map(([name, value]) => [name, String(value)]), + dataCollection.cookies, + ); + if (cookieAttributes.length) { + safeSetSpanJSONAttributes(span, { 'http.request.header.cookie': cookieAttributes }); + } } if (requestData.headers) { @@ -245,7 +247,7 @@ function extractNormalizedRequestData( } if (include.cookies) { - const cookies = normalizedRequest.cookies || (headers?.cookie ? parseCookie(headers.cookie) : undefined); + const cookies = normalizedRequest.cookies || (headers?.cookie ? parseCookieRecord(headers.cookie) : undefined); requestData.cookies = cookies || {}; } @@ -260,6 +262,19 @@ function extractNormalizedRequestData( return requestData; } +function parseCookieRecord(cookieString: string): Record { + const parsed: Record = {}; + + for (const [name, value] of parseCookiePairs(cookieString)) { + // only assign once + if (name !== '' && !(name in parsed)) { + parsed[name] = value; + } + } + + return parsed; +} + function resolveFilteringBehavior(isIncluded: boolean, behavior: CollectBehavior): CollectBehavior { return isIncluded && behavior === false ? true : behavior; } diff --git a/packages/core/src/utils/cookie.ts b/packages/core/src/utils/cookie.ts index 218342ae36d3..ad0c11f0dcb0 100644 --- a/packages/core/src/utils/cookie.ts +++ b/packages/core/src/utils/cookie.ts @@ -28,36 +28,51 @@ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +// `Set-Cookie` attributes are metadata, not cookies. Response cookie strings handed to +// the `Cookie`-mode parser may still carry them, so they are dropped by name. +const SET_COOKIE_ATTRIBUTES = new Set([ + 'expires', + 'max-age', + 'domain', + 'path', + 'secure', + 'httponly', + 'samesite', + 'partitioned', +]); + /** - * Parses a cookie string + * Parses a `Cookie` or `Set-Cookie` header value into ordered `[name, value]` pairs. + * + * In `Set-Cookie` mode each header value carries a single cookie, so only the segment + * before the first `;` is parsed. Otherwise every `;`-separated segment is a pair, with + * known `Set-Cookie` attributes dropped by name. + * + * Segments without `=` (e.g. a bare token or a flag like `Secure`) are returned as + * `['', segment]`; values are unquoted and URL-decoded. */ -export function parseCookie(str: string): Record { - const obj: Record = {}; - let index = 0; +export function parseCookiePairs(value: string | string[], setCookie = false): [string, string][] { + const pairs: [string, string][] = []; - while (index < str.length) { - const eqIdx = str.indexOf('=', index); - - // no more cookie pairs - if (eqIdx === -1) { - break; + for (const headerValue of Array.isArray(value) ? value : [value]) { + if (typeof headerValue !== 'string' || headerValue === '') { + continue; } - let endIdx = str.indexOf(';', index); + // A `Set-Cookie` value carries one cookie, so only its first segment is a pair. + // `headerValue` is non-empty here, so the split always yields at least one segment. + const segments = setCookie ? headerValue.split(';', 1) : headerValue.split(';'); - if (endIdx === -1) { - endIdx = str.length; - } else if (endIdx < eqIdx) { - // backtrack on prior semicolon - index = str.lastIndexOf(';', eqIdx - 1) + 1; - continue; - } + for (let segment of segments) { + segment = segment.trim(); - const key = str.slice(index, eqIdx).trim(); + if (segment === '') { + continue; + } - // only assign once - if (undefined === obj[key]) { - let val = str.slice(eqIdx + 1, endIdx).trim(); + const eqIdx = segment.indexOf('='); + const name = (eqIdx === -1 ? '' : segment.slice(0, eqIdx)).trim(); + let val = (eqIdx === -1 ? segment : segment.slice(eqIdx + 1)).trim(); // quoted values if (val.charCodeAt(0) === 0x22) { @@ -65,14 +80,18 @@ export function parseCookie(str: string): Record { } try { - obj[key] = val.indexOf('%') !== -1 ? decodeURIComponent(val) : val; + val = val.indexOf('%') !== -1 ? decodeURIComponent(val) : val; } catch { - obj[key] = val; + // keep the raw value } - } - index = endIdx + 1; + if (!setCookie && SET_COOKIE_ATTRIBUTES.has(name.toLowerCase())) { + continue; + } + + pairs.push([name, val]); + } } - return obj; + return pairs; } diff --git a/packages/core/src/utils/data-collection/filterCookies.ts b/packages/core/src/utils/data-collection/filterCookies.ts index ad18d67fe14a..54b764e21b07 100644 --- a/packages/core/src/utils/data-collection/filterCookies.ts +++ b/packages/core/src/utils/data-collection/filterCookies.ts @@ -1,5 +1,5 @@ import type { CollectBehavior } from '../../types/datacollection'; -import { parseCookie } from '../cookie'; +import { parseCookiePairs } from '../cookie'; import { FILTERED_VALUE as FILTERED, SENSITIVE_COOKIE_NAME_SNIPPETS } from './filtering-snippets'; import { filterKeyValueData } from './filterKeyValueData'; @@ -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 is dropped: a record key cannot carry a `[Filtered]` marker + * without leaking the bare token. */ export function filterCookies(cookieString: string, behavior: CollectBehavior): Record | string { if (behavior === false) { @@ -15,7 +17,14 @@ export function filterCookies(cookieString: string, behavior: CollectBehavior): } try { - const parsed = parseCookie(cookieString); + const parsed: Record = {}; + + for (const [name, value] of parseCookiePairs(cookieString)) { + // only assign once + if (name !== '' && !(name in parsed)) { + parsed[name] = value; + } + } if (Object.keys(parsed).length === 0) { return {}; diff --git a/packages/core/src/utils/request.ts b/packages/core/src/utils/request.ts index 932a10f652b8..f09c4d1433c2 100644 --- a/packages/core/src/utils/request.ts +++ b/packages/core/src/utils/request.ts @@ -1,13 +1,14 @@ /* eslint-disable max-lines-per-function */ import { DEBUG_BUILD } from '../debug-build'; import type { Scope } from '../scope'; -import type { ResolvedDataCollection } from '../types/datacollection'; +import type { CollectBehavior, ResolvedDataCollection } from '../types/datacollection'; import type { PolymorphicRequest } from '../types/polymorphics'; import type { RequestEventData } from '../types/request'; import type { WebFetchHeaders, WebFetchRequest } from '../types/webfetchapi'; import { debug } from './debug-logger'; import { FILTERED_VALUE, SENSITIVE_COOKIE_NAME_SNIPPETS } from './data-collection/filtering-snippets'; import { shouldFilterDataKey } from './data-collection/filterKeyValueData'; +import { parseCookiePairs } from './cookie'; import { safeUnref } from './timer'; import { getUrlQuery } from './url'; @@ -303,14 +304,8 @@ export function httpHeadersToSpanAttributes( continue; } - const cookies = parseCookieHeader(value, lowerKey === 'set-cookie'); - spanAttributes[`${prefix}${lowerKey}`] = cookies.length - ? cookies.map(([cookieKey, cookieValue]) => - shouldFilterDataKey(cookieKey, cookieBehavior, SENSITIVE_COOKIE_NAME_SNIPPETS) - ? `${cookieKey}=${FILTERED_VALUE}` - : `${cookieKey}=${cookieValue}`, - ) - : [FILTERED_VALUE]; + const cookies = parseCookiePairs(value, lowerKey === 'set-cookie'); + spanAttributes[`${prefix}${lowerKey}`] = filterCookiePairs(cookies, cookieBehavior); } else { if (headerBehavior === false) { continue; @@ -338,21 +333,30 @@ export function httpHeadersToSpanAttributes( return spanAttributes; } -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") - const cookies = (Array.isArray(value) ? value : [value]).flatMap(headerValue => { - if (typeof headerValue !== 'string' || headerValue === '') { - return []; - } - return isSetCookie ? [headerValue.split(';')[0]!] : headerValue.split('; '); - }); +/** + * Filter already-parsed cookie pairs and serialize them back to `name=value` strings. + * + * Callers must pass pairs straight through rather than rejoining them into a header + * string first: a value containing `;`, `%`, or quotes survives a single parse, but a + * round trip through a string splits and double-decodes it. + */ +export function filterCookiePairs(pairs: [string, string][], cookieBehavior: CollectBehavior): string[] { + if (cookieBehavior === false) { + return []; + } + + if (!pairs.length) { + return [FILTERED_VALUE]; + } - return cookies.map(cookie => { - const equalSignIndex = cookie.indexOf('='); - return equalSignIndex !== -1 - ? [cookie.substring(0, equalSignIndex), cookie.substring(equalSignIndex + 1)] - : [cookie, '']; + return pairs.map(([cookieKey, cookieValue]) => { + // A nameless segment'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}`; }); } diff --git a/packages/core/test/lib/integrations/requestdata.test.ts b/packages/core/test/lib/integrations/requestdata.test.ts index e5528d6b5816..a33f79b8ad94 100644 --- a/packages/core/test/lib/integrations/requestdata.test.ts +++ b/packages/core/test/lib/integrations/requestdata.test.ts @@ -32,7 +32,7 @@ function baseEvent(overrides: Partial = {}): Event { }; } -/** Rich normalized request (Cookie header only — tests `parseCookie` path). */ +/** Rich normalized request (Cookie header only — tests `parseCookiePairs` path). */ function richNormalizedRequest() { return { method: 'POST', @@ -963,6 +963,21 @@ describe('requestDataIntegration processSegmentSpan', () => { }); }); + it('does not double-decode cookies already parsed from the request', () => { + const integration = requestDataIntegration(); + const span = makeSpan(); + + mockIsolationScope({ + cookies: { theme: '%20' }, + }); + + integration.processSegmentSpan!(span, mockClient({ userInfo: false })); + + expect(span.attributes).toMatchObject({ + 'http.request.header.cookie': ['theme=%20'], + }); + }); + it('falls back to cookie header when normalizedRequest.cookies is not set', () => { const integration = requestDataIntegration({ include: { headers: false } }); const span = makeSpan(); diff --git a/packages/core/test/lib/utils/cookie.test.ts b/packages/core/test/lib/utils/cookie.test.ts index ccec4a9a26dd..6c6594cb6f14 100644 --- a/packages/core/test/lib/utils/cookie.test.ts +++ b/packages/core/test/lib/utils/cookie.test.ts @@ -29,40 +29,140 @@ */ import { describe, expect, it } from 'vitest'; -import { parseCookie } from '../../../src/utils/cookie'; +import { parseCookiePairs } from '../../../src/utils/cookie'; -describe('parseCookie(str)', function () { - it('should parse cookie string to object', function () { - expect(parseCookie('foo=bar')).toEqual({ foo: 'bar' }); - expect(parseCookie('foo=123')).toEqual({ foo: '123' }); +describe('parseCookiePairs(value)', function () { + it('should parse cookie string to ordered pairs', function () { + expect(parseCookiePairs('foo=bar')).toEqual([['foo', 'bar']]); + expect(parseCookiePairs('foo=123')).toEqual([['foo', '123']]); + expect(parseCookiePairs('foo=bar; baz=raz')).toEqual([ + ['foo', 'bar'], + ['baz', 'raz'], + ]); }); it('should ignore OWS', function () { - expect(parseCookie('FOO = bar; baz = raz')).toEqual({ FOO: 'bar', baz: 'raz' }); + expect(parseCookiePairs('FOO = bar; baz = raz')).toEqual([ + ['FOO', 'bar'], + ['baz', 'raz'], + ]); }); it('should parse cookie with empty value', function () { - expect(parseCookie('foo= ; bar=')).toEqual({ foo: '', bar: '' }); + expect(parseCookiePairs('foo= ; bar=')).toEqual([ + ['foo', ''], + ['bar', ''], + ]); }); it('should URL-decode values', function () { - expect(parseCookie('foo="bar=123456789&name=Magic+Mouse"')).toEqual({ foo: 'bar=123456789&name=Magic+Mouse' }); + expect(parseCookiePairs('foo="bar=123456789&name=Magic+Mouse"')).toEqual([ + ['foo', 'bar=123456789&name=Magic+Mouse'], + ]); - expect(parseCookie('email=%20%22%2c%3b%2f')).toEqual({ email: ' ",;/' }); + expect(parseCookiePairs('email=%20%22%2c%3b%2f')).toEqual([['email', ' ",;/']]); }); it('should return original value on escape error', function () { - expect(parseCookie('foo=%1;bar=bar')).toEqual({ foo: '%1', bar: 'bar' }); + expect(parseCookiePairs('foo=%1;bar=bar')).toEqual([ + ['foo', '%1'], + ['bar', 'bar'], + ]); }); - it('should ignore cookies without value', function () { - expect(parseCookie('foo=bar;fizz ; buzz')).toEqual({ foo: 'bar' }); - expect(parseCookie(' fizz; foo= bar')).toEqual({ foo: 'bar' }); + it('should keep duplicate cookies as ordered pairs', function () { + expect(parseCookiePairs('foo=%1;bar=bar;foo=boo')).toEqual([ + ['foo', '%1'], + ['bar', 'bar'], + ['foo', 'boo'], + ]); }); - it('should ignore duplicate cookies', function () { - expect(parseCookie('foo=%1;bar=bar;foo=boo')).toEqual({ foo: '%1', bar: 'bar' }); - expect(parseCookie('foo=false;bar=bar;foo=tre')).toEqual({ foo: 'false', bar: 'bar' }); - expect(parseCookie('foo=;bar=bar;foo=boo')).toEqual({ foo: '', bar: 'bar' }); + it('should return nameless segments with an empty name', function () { + expect(parseCookiePairs('foo=bar;fizz ; buzz')).toEqual([ + ['foo', 'bar'], + ['', 'fizz'], + ['', 'buzz'], + ]); + expect(parseCookiePairs(' fizz; foo= bar')).toEqual([ + ['', 'fizz'], + ['foo', 'bar'], + ]); + }); + + it('should split on ";" even without a trailing space', function () { + expect(parseCookiePairs('foo=bar;baz=raz')).toEqual([ + ['foo', 'bar'], + ['baz', 'raz'], + ]); + }); + + it('should skip empty segments', function () { + expect(parseCookiePairs('foo=bar;;;baz=raz;')).toEqual([ + ['foo', 'bar'], + ['baz', 'raz'], + ]); + expect(parseCookiePairs('')).toEqual([]); + }); + + it('should only split on the first "="', function () { + expect(parseCookiePairs('data=base64==')).toEqual([['data', 'base64==']]); + }); + + it('should decode a percent-encoded value only once', function () { + expect(parseCookiePairs('token=%2520')).toEqual([['token', '%20']]); + }); + + it('should accept an array of header values', function () { + expect(parseCookiePairs(['foo=bar', 'baz=raz'])).toEqual([ + ['foo', 'bar'], + ['baz', 'raz'], + ]); + expect(parseCookiePairs(['foo=bar', '', 'baz=raz'])).toEqual([ + ['foo', 'bar'], + ['baz', 'raz'], + ]); + }); + + describe('Set-Cookie mode', function () { + it('should only parse the first segment of each value', function () { + expect(parseCookiePairs('sid=1; Max-Age=3600; Path=/', true)).toEqual([['sid', '1']]); + expect(parseCookiePairs('theme=dark; Expires=Wed, 21 Oct 2026 07:28:00 GMT; Domain=example.com', true)).toEqual([ + ['theme', 'dark'], + ]); + }); + + it('should parse one cookie per array value', function () { + expect(parseCookiePairs(['theme=dark; HttpOnly', 'session=abc123; Secure'], true)).toEqual([ + ['theme', 'dark'], + ['session', 'abc123'], + ]); + }); + + it('should return nameless first segments with an empty name', function () { + expect(parseCookiePairs('auth_required; HttpOnly', true)).toEqual([['', 'auth_required']]); + }); + }); + + describe('Set-Cookie attribute handling', function () { + it('should drop known Set-Cookie attributes by name', function () { + expect(parseCookiePairs('sid=1; Max-Age=3600; Path=/')).toEqual([['sid', '1']]); + expect(parseCookiePairs('theme=dark; Expires=Wed, 21 Oct 2026 07:28:00 GMT; Domain=example.com')).toEqual([ + ['theme', 'dark'], + ]); + expect(parseCookiePairs('a=1; SameSite=Lax; Max-Age=60')).toEqual([['a', '1']]); + }); + + it('should return bare flag attributes as nameless pairs (dropped or filtered downstream)', () => { + expect(parseCookiePairs('a=1; Secure; HttpOnly')).toEqual([ + ['a', '1'], + ['', 'Secure'], + ['', 'HttpOnly'], + ]); + }); + + it('should match attribute names case-insensitively', function () { + expect(parseCookiePairs('sid=1; max-age=3600; PATH=/')).toEqual([['sid', '1']]); + }); }); }); 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..e00406d4f978 100644 --- a/packages/core/test/lib/utils/data-collection/filterCookies.test.ts +++ b/packages/core/test/lib/utils/data-collection/filterCookies.test.ts @@ -83,6 +83,22 @@ describe('filterCookies', () => { }); }); + describe('Set-Cookie attribute handling', () => { + it('does not report Set-Cookie attributes as cookie pairs', () => { + expect(filterCookies('sid=1; Max-Age=3600; Path=/', true)).toEqual({ sid: '[Filtered]' }); + }); + + it('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', + }); + }); + + it('drops nameless segments', () => { + expect(filterCookies('opaque-blob; theme=dark', 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/core/test/lib/utils/request.test.ts b/packages/core/test/lib/utils/request.test.ts index 2041e06ec3da..7968afebcaa4 100644 --- a/packages/core/test/lib/utils/request.test.ts +++ b/packages/core/test/lib/utils/request.test.ts @@ -660,7 +660,7 @@ describe('request utils', () => { 'http.request.header.cookie': [ 'session=[Filtered]', 'tracking=enabled', - 'cookie-authentication-key-without-value=[Filtered]', + '[Filtered]', 'theme=dark', 'lang=en', 'user_session=[Filtered]', @@ -728,7 +728,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 }; @@ -754,6 +754,12 @@ describe('request utils', () => { expect(result).toEqual({ 'http.request.header.cookie': ['random-string=eyJhbGc=.eyJzdWI=.SflKxw'] }); }); + it('URL-decodes and unquotes cookie values', () => { + const headers = { Cookie: 'theme=%22dark%20mode%22' }; + const result = httpHeadersToSpanAttributes(headers, resolveDataCollectionOptions({})); + expect(result).toEqual({ 'http.request.header.cookie': ['theme="dark mode"'] }); + }); + it.each([ { dataCollection: resolveDataCollectionOptions({