Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/astro/src/server/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,10 @@ async function instrumentRequestStartHttpServerSpan(
// This is here for backwards compatibility, we used to set this here before
method,
url: stripUrlQueryAndFragment(ctx.url.href),
...httpHeadersToSpanAttributes(winterCGHeadersToDict(request.headers)),
...httpHeadersToSpanAttributes(
winterCGHeadersToDict(request.headers),
getClient()?.getOptions().sendDefaultPii ?? false,
),
};

if (parametrizedRoute) {
Expand Down
6 changes: 5 additions & 1 deletion packages/bun/src/integrations/bunserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
captureException,
continueTrace,
defineIntegration,
getClient,
httpHeadersToSpanAttributes,
isURLObjectRelative,
parseStringToURLObject,
Expand Down Expand Up @@ -206,7 +207,10 @@ function wrapRequestHandler<T extends RouteHandler = RouteHandler>(
routeName = route;
}

Object.assign(attributes, httpHeadersToSpanAttributes(request.headers.toJSON()));
Object.assign(
attributes,
httpHeadersToSpanAttributes(request.headers.toJSON(), getClient()?.getOptions().sendDefaultPii ?? false),
);

isolationScope.setSDKProcessingMetadata({
normalizedRequest: {
Expand Down
9 changes: 8 additions & 1 deletion packages/cloudflare/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
captureException,
continueTrace,
flush,
getClient,
getHttpSpanDetailsFromUrlObject,
httpHeadersToSpanAttributes,
parseStringToURLObject,
Expand Down Expand Up @@ -66,7 +67,13 @@ export function wrapRequestHandler(
attributes['user_agent.original'] = userAgentHeader;
}

Object.assign(attributes, httpHeadersToSpanAttributes(winterCGHeadersToDict(request.headers)));
Object.assign(
attributes,
httpHeadersToSpanAttributes(
winterCGHeadersToDict(request.headers),
getClient()?.getOptions().sendDefaultPii ?? false,
),
);

attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] = 'http.server';

Expand Down
64 changes: 52 additions & 12 deletions packages/core/src/utils/request.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getClient } from '../currentScopes';
import type { PolymorphicRequest } from '../types-hoist/polymorphics';
import type { RequestEventData } from '../types-hoist/request';
import type { WebFetchHeaders, WebFetchRequest } from '../types-hoist/webfetchapi';
Expand Down Expand Up @@ -128,21 +129,29 @@ function getAbsoluteUrl({
return undefined;
}

// "-user" because otherwise it would match "user-agent"
const SENSITIVE_HEADER_SNIPPETS = [
'auth',
'token',
'secret',
'cookie',
'-user',
'session', // for the user_session cookie
'password',
'passwd',
'pwd',
'key',
'jwt',
'bearer',
'sso',
'saml',
'crsf',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Typo in sensitive header snippet breaks CSRF filtering

The sensitive header snippet 'crsf' is a typo that should be 'csrf' (Cross-Site Request Forgery). This prevents proper filtering of CSRF-related headers and cookies that don't contain other sensitive keywords. For example, a header like X-CSRF or a cookie named csrf-token would not be filtered because 'x-csrf'.includes('crsf') returns false. The existing test passes only because X-CSRF-Token also contains token, which masks this bug.

Fix in Cursor Fix in Web

'xsrf',
'credentials',
// Always treat cookie headers as sensitive in case individual key-value cookie pairs cannot properly be extracted
'set-cookie',
'cookie',
];

const PII_HEADER_SNIPPETS = ['x-forwarded-', '-user'];

/**
* Converts incoming HTTP request headers to OpenTelemetry span attributes following semantic conventions.
* Header names are converted to the format: http.request.header.<key>
Expand All @@ -152,6 +161,7 @@ const SENSITIVE_HEADER_SNIPPETS = [
*/
export function httpHeadersToSpanAttributes(
headers: Record<string, string | string[] | undefined>,
sendDefaultPii: boolean = false,
): Record<string, string> {
const spanAttributes: Record<string, string> = {};

Expand All @@ -161,16 +171,26 @@ export function httpHeadersToSpanAttributes(
return;
}

const lowerCasedKey = key.toLowerCase();
const isSensitive = SENSITIVE_HEADER_SNIPPETS.some(snippet => lowerCasedKey.includes(snippet));
const normalizedKey = `http.request.header.${lowerCasedKey.replace(/-/g, '_')}`;
const lowerCasedHeaderKey = key.toLowerCase();
const isCookieHeader = lowerCasedHeaderKey === 'cookie' || lowerCasedHeaderKey === 'set-cookie';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: probably saves us a few bytes:

Suggested change
const isCookieHeader = lowerCasedHeaderKey === 'cookie' || lowerCasedHeaderKey === 'set-cookie';
const isCookieHeader = /^(set-)cookie$?/.test(lowerCasedHeaderKey)


if (isCookieHeader && typeof value === 'string' && value !== '') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: do we handle arrays of cookie headers? (or is this not relevant for cookie/set-cookie?)

const cookies = value.split('; ');

for (const cookie of cookies) {
// Split only at the first '=' to preserve '=' characters in cookie values
const equalSignIndex = cookie.indexOf('=');
const cookieKey = equalSignIndex !== -1 ? cookie.substring(0, equalSignIndex) : cookie;
const cookieValue = equalSignIndex !== -1 ? cookie.substring(equalSignIndex + 1) : '';

const lowerCasedCookieKey = String(cookieKey).toLowerCase();
const normalizedKey = `http.request.header.${normalizeAttributeKey(lowerCasedHeaderKey)}.${normalizeAttributeKey(lowerCasedCookieKey)}`;

if (isSensitive) {
spanAttributes[normalizedKey] = '[Filtered]';
} else if (Array.isArray(value)) {
spanAttributes[normalizedKey] = value.map(v => (v != null ? String(v) : v)).join(';');
} else if (typeof value === 'string') {
spanAttributes[normalizedKey] = value;
spanAttributes[normalizedKey] = handleHttpHeader(lowerCasedCookieKey, cookieValue, sendDefaultPii);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Set-Cookie attributes incorrectly parsed as separate cookies

The cookie parsing logic treats Cookie and Set-Cookie headers identically by splitting on '; ', but these headers have fundamentally different formats. Cookie headers contain multiple cookies (name1=value1; name2=value2), while Set-Cookie headers contain a single cookie with attributes (name=value; Path=/; HttpOnly). When processing a Set-Cookie header like session=abc; Path=/; HttpOnly, the code incorrectly creates span attributes for path and httponly as if they were cookie names rather than attributes of the session cookie.

Fix in Cursor Fix in Web

} else {
const normalizedKey = `http.request.header.${normalizeAttributeKey(lowerCasedHeaderKey)}`;
spanAttributes[normalizedKey] = handleHttpHeader(lowerCasedHeaderKey, value, sendDefaultPii);
}
});
} catch {
Expand All @@ -180,6 +200,26 @@ export function httpHeadersToSpanAttributes(
return spanAttributes;
}

This comment was marked as outdated.


function normalizeAttributeKey(key: string): string {
return key.replace(/-/g, '_');
}

function handleHttpHeader(lowerCasedKey: string, value: string | string[] | undefined, sendPii: boolean): string {
const isSensitive = sendPii
? SENSITIVE_HEADER_SNIPPETS.some(snippet => lowerCasedKey.includes(snippet))
: [...PII_HEADER_SNIPPETS, ...SENSITIVE_HEADER_SNIPPETS].some(snippet => lowerCasedKey.includes(snippet));

if (isSensitive) {
return '[Filtered]';
} else if (Array.isArray(value)) {
return value.map(v => (v != null ? String(v) : v)).join(';');
} else if (typeof value === 'string') {
return value;
}

return ''; // Fallback for unexpected types
}

/** Extract the query params from an URL. */
export function extractQueryParamsFromUrl(url: string): string | undefined {
// url is path and query string
Expand Down
97 changes: 93 additions & 4 deletions packages/core/test/lib/utils/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ describe('request utils', () => {
});
});

describe('PII filtering', () => {
describe('PII/Sensitive data filtering', () => {
it('filters sensitive headers case-insensitively', () => {
const headers = {
AUTHORIZATION: 'Bearer secret-token',
Expand All @@ -625,12 +625,101 @@ describe('request utils', () => {

expect(result).toEqual({
'http.request.header.content_type': 'application/json',
'http.request.header.cookie': '[Filtered]',
'http.request.header.cookie.session': '[Filtered]',
'http.request.header.x_api_key': '[Filtered]',
'http.request.header.authorization': '[Filtered]',
});
});

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',
};

const result = httpHeadersToSpanAttributes(headers);

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',
});
});

it('adds a filtered cookie header when cookie header is present, but has no valid key=value pairs', () => {
const headers1 = { Cookie: ['key', 'val'] };
const result1 = httpHeadersToSpanAttributes(headers1);
expect(result1).toEqual({ 'http.request.header.cookie': '[Filtered]' });

const headers3 = { Cookie: '' };
const result3 = httpHeadersToSpanAttributes(headers3);
expect(result3).toEqual({ 'http.request.header.cookie': '[Filtered]' });
});

it('attaches and filters sensitive a set-cookie header', () => {
const headers1 = { 'Set-Cookie': 'user_session=def456' };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: let's add or adjust a test here for a set-cookie header with additional properties (e.g. like max-age)

const result1 = httpHeadersToSpanAttributes(headers1);
expect(result1).toEqual({ 'http.request.header.set_cookie.user_session': '[Filtered]' });

const headers2 = { 'Set-Cookie': 'preferred-color-mode=light' };
const result2 = httpHeadersToSpanAttributes(headers2);
expect(result2).toEqual({ 'http.request.header.set_cookie.preferred_color_mode': 'light' });

const headers3 = { 'Set-Cookie': 'lang=en' };
const result3 = httpHeadersToSpanAttributes(headers3);
expect(result3).toEqual({ 'http.request.header.set_cookie.lang': 'en' });

const headers4 = { 'Set-Cookie': 'timezone=UTC' };
const result4 = httpHeadersToSpanAttributes(headers4);
expect(result4).toEqual({ 'http.request.header.set_cookie.timezone': 'UTC' });
});

it('only splits cookies once between key and value, even when more equals signs are present', () => {
const headers = { Cookie: 'random-string=eyJhbGc=.eyJzdWI=.SflKxw' };
const result = httpHeadersToSpanAttributes(headers);
expect(result).toEqual({ 'http.request.header.cookie.random_string': 'eyJhbGc=.eyJzdWI=.SflKxw' });
});

it.each([
{ sendDefaultPii: false, description: 'sendDefaultPii is false (default)' },
{ sendDefaultPii: true, description: 'sendDefaultPii is true' },
])('does not include PII headers when $description', ({ sendDefaultPii }) => {
const headers = {
'Content-Type': 'application/json',
'User-Agent': 'Mozilla/5.0',
'x-user': 'my-personal-username',
'X-Forwarded-For': '192.168.1.1',
'X-Forwarded-Host': 'example.com',
'X-Forwarded-Proto': 'https',
};

const result = httpHeadersToSpanAttributes(headers, sendDefaultPii);

if (sendDefaultPii) {
expect(result).toEqual({
'http.request.header.content_type': 'application/json',
'http.request.header.user_agent': 'Mozilla/5.0',
'http.request.header.x_user': 'my-personal-username',
'http.request.header.x_forwarded_for': '192.168.1.1',
'http.request.header.x_forwarded_host': 'example.com',
'http.request.header.x_forwarded_proto': 'https',
});
} else {
expect(result).toEqual({
'http.request.header.content_type': 'application/json',
'http.request.header.user_agent': 'Mozilla/5.0',
'http.request.header.x_user': '[Filtered]',
'http.request.header.x_forwarded_for': '[Filtered]',
'http.request.header.x_forwarded_host': '[Filtered]',
'http.request.header.x_forwarded_proto': '[Filtered]',
});
}
});

it('always filters comprehensive list of sensitive headers', () => {
const headers = {
'Content-Type': 'application/json',
Expand Down Expand Up @@ -671,8 +760,8 @@ describe('request utils', () => {
'http.request.header.accept': 'application/json',
'http.request.header.host': 'example.com',
'http.request.header.authorization': '[Filtered]',
'http.request.header.cookie': '[Filtered]',
'http.request.header.set_cookie': '[Filtered]',
'http.request.header.cookie.session': '[Filtered]',
'http.request.header.set_cookie.session': '[Filtered]',
'http.request.header.x_api_key': '[Filtered]',
'http.request.header.x_auth_token': '[Filtered]',
'http.request.header.x_secret': '[Filtered]',
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/common/utils/addHeadersAsAttributes.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Span, WebFetchHeaders } from '@sentry/core';
import { httpHeadersToSpanAttributes, winterCGHeadersToDict } from '@sentry/core';
import { getClient, httpHeadersToSpanAttributes, winterCGHeadersToDict } from '@sentry/core';

/**
* Extracts HTTP request headers as span attributes and optionally applies them to a span.
Expand All @@ -17,7 +17,7 @@ export function addHeadersAsAttributes(
? winterCGHeadersToDict(headers as Headers)
: headers;

const headerAttributes = httpHeadersToSpanAttributes(headersDict);
const headerAttributes = httpHeadersToSpanAttributes(headersDict, getClient()?.getOptions().sendDefaultPii ?? false);

if (span) {
span.setAttributes(headerAttributes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions
'http.flavor': httpVersion,
'net.transport': httpVersion?.toUpperCase() === 'QUIC' ? 'ip_udp' : 'ip_tcp',
...getRequestContentLengthAttribute(request),
...httpHeadersToSpanAttributes(normalizedRequest.headers || {}),
...httpHeadersToSpanAttributes(
normalizedRequest.headers || {},
client.getOptions().sendDefaultPii ?? false,
),
},
});

Expand Down
3 changes: 2 additions & 1 deletion packages/nuxt/src/runtime/hooks/wrapMiddlewareHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
captureException,
debug,
flushIfServerless,
getClient,
httpHeadersToSpanAttributes,
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
Expand Down Expand Up @@ -172,7 +173,7 @@ function getSpanAttributes(

// Get headers from the Node.js request object
const headers = event.node?.req?.headers || {};
const headerAttributes = httpHeadersToSpanAttributes(headers);
const headerAttributes = httpHeadersToSpanAttributes(headers, getClient()?.getOptions().sendDefaultPii ?? false);

// Merge header attributes with existing attributes
Object.assign(attributes, headerAttributes);
Expand Down
5 changes: 4 additions & 1 deletion packages/remix/src/server/instrumentServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,10 @@ function wrapRequestHandler<T extends ServerBuild | (() => ServerBuild | Promise
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
method: request.method,
...httpHeadersToSpanAttributes(winterCGHeadersToDict(request.headers)),
...httpHeadersToSpanAttributes(
winterCGHeadersToDict(request.headers),
clientOptions.sendDefaultPii ?? false,
),
},
},
async span => {
Expand Down
6 changes: 5 additions & 1 deletion packages/sveltekit/src/server-common/handle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
winterCGRequestToRequestData,
withIsolationScope,
} from '@sentry/core';
import { getClient } from '@sentry/svelte';
import type { Handle, ResolveOptions } from '@sveltejs/kit';
import { DEBUG_BUILD } from '../common/debug-build';
import { getTracePropagationData, sendErrorToSentry } from './utils';
Expand Down Expand Up @@ -204,7 +205,10 @@ async function instrumentHandle(
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit',
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: routeId ? 'route' : 'url',
'http.method': event.request.method,
...httpHeadersToSpanAttributes(winterCGHeadersToDict(event.request.headers)),
...httpHeadersToSpanAttributes(
winterCGHeadersToDict(event.request.headers),
getClient()?.getOptions().sendDefaultPii ?? false,
),
},
name: routeName,
},
Expand Down
Loading