Skip to content

Commit bf324ae

Browse files
committed
pass allow/denylists
1 parent 8babbb5 commit bf324ae

8 files changed

Lines changed: 61 additions & 29 deletions

File tree

‎dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/instrument.mjs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,17 @@ Sentry.init({
77
release: '1.0',
88
tracesSampleRate: 1.0,
99
transport: loggingTransport,
10+
dataCollection: {
11+
httpHeaders: {
12+
request: { deny: ['x-tenant-id'] },
13+
response: { deny: ['content-length'] },
14+
},
15+
},
1016
integrations: [
1117
Sentry.nativeNodeFetchIntegration({
1218
headersToSpanAttributes: {
13-
requestHeaders: ['x-test-header', 'authorization'],
14-
responseHeaders: ['x-powered-by'],
19+
requestHeaders: ['x-test-header', 'authorization', 'x-tenant-id'],
20+
responseHeaders: ['x-powered-by', 'content-length'],
1521
},
1622
}),
1723
],

‎dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,6 @@ import * as Sentry from '@sentry/node';
33
// eslint-disable-next-line @typescript-eslint/no-floating-promises
44
Sentry.startSpan({ name: 'test_transaction' }, async () => {
55
await fetch(`${process.env.SERVER_URL}/api/v0`, {
6-
headers: { 'x-test-header': 'test-value', authorization: 'Bearer super-secret' },
6+
headers: { 'x-test-header': 'test-value', authorization: 'Bearer super-secret', 'x-tenant-id': 'acme-corp' },
77
});
88
});

‎dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,12 @@ describe('outgoing fetch spans - headers to span attributes', () => {
2929
origin: 'auto.http.node_fetch',
3030
data: expect.objectContaining({
3131
'http.request.header.x-test-header': ['test-value'],
32-
// Listed in `headersToSpanAttributes`, but the denylist still wins.
32+
// Listed in `headersToSpanAttributes`, but the built-in denylist still wins.
3333
'http.request.header.authorization': ['[Filtered]'],
34+
// Listed in `headersToSpanAttributes`, but denied via `dataCollection.httpHeaders`.
35+
'http.request.header.x-tenant-id': ['[Filtered]'],
3436
'http.response.header.x-powered-by': ['Express'],
37+
'http.response.header.content-length': ['[Filtered]'],
3538
}),
3639
}),
3740
]),

‎packages/core/src/index.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,7 @@ export {
7676
_INTERNAL_shouldSkipAiProviderWrapping,
7777
_INTERNAL_clearAiProviderSkips,
7878
} from './utils/ai/providerSkip';
79-
export {
80-
filterKeyValueData as _INTERNAL_filterKeyValueData,
81-
shouldFilterDataKey as _INTERNAL_shouldFilterDataKey,
82-
} from './utils/data-collection/filterKeyValueData';
83-
export { FILTERED_VALUE as _INTERNAL_FILTERED_VALUE } from './utils/data-collection/filtering-snippets';
79+
export { filterKeyValueData as _INTERNAL_filterKeyValueData } from './utils/data-collection/filterKeyValueData';
8480
export { filterCookies as _INTERNAL_filterCookies } from './utils/data-collection/filterCookies';
8581
export { filterQueryParams as _INTERNAL_filterQueryParams } from './utils/data-collection/filterQueryParams';
8682
export { filterCollectedUrl, filterCollectedUrlQuery } from './utils/data-collection/filterCollectedUrl';

‎packages/core/src/utils/data-collection/filterCookies.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import { filterKeyValueData } from './filterKeyValueData';
88
*
99
* When individual cookies can be parsed, each key-value pair is filtered
1010
* independently. When parsing fails, the entire string is replaced with `[Filtered]`.
11+
* A nameless segment inside an otherwise parseable string (`"opaque-blob; theme=dark"`) is
12+
* dropped, since a record key cannot carry a `[Filtered]` marker without leaking the token.
1113
*/
1214
export function filterCookies(cookieString: string, behavior: CollectBehavior): Record<string, string> | string {
1315
if (behavior === false) {

‎packages/core/test/lib/utils/data-collection/filterCookies.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,21 @@ describe('filterCookies', () => {
8484
});
8585
});
8686

87+
// Intended behavior for the cookie parsing consolidation follow-up: `Set-Cookie` attributes are
88+
// metadata, not cookies, so they must not show up as key-value pairs. Marked `fails` until the
89+
// shared parser handles them.
90+
describe('Set-Cookie attribute handling (known gaps)', () => {
91+
it.fails('does not report Set-Cookie attributes as cookie pairs', () => {
92+
expect(filterCookies('sid=1; Max-Age=3600; Path=/', true)).toEqual({ sid: '[Filtered]' });
93+
});
94+
95+
it.fails('does not report Expires/Domain attributes as cookie pairs', () => {
96+
expect(filterCookies('theme=dark; Expires=Wed, 21 Oct 2026 07:28:00 GMT; Domain=example.com', true)).toEqual({
97+
theme: 'dark',
98+
});
99+
});
100+
});
101+
87102
describe('edge cases', () => {
88103
it('handles cookies with = in the value', () => {
89104
const result = filterCookies('data=base64==; theme=light', true);

‎packages/node/src/integrations/node-fetch/types.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,16 @@ export interface UndiciInstrumentationConfig<RequestType = UndiciRequest, Respon
8787
requestHook?: RequestHookFunction<RequestType>;
8888
/** Function called once response headers have been received */
8989
responseHook?: ResponseHookFunction<RequestType, ResponseType>;
90-
/** Map the following HTTP headers to span attributes. */
90+
/**
91+
* Capture the listed HTTP headers as span attributes
92+
* (`http.request.header.<name>` / `http.response.header.<name>`).
93+
*
94+
* Privacy filtering still applies to every header listed here. A header keeps its value only if
95+
* `dataCollection.httpHeaders` permits it:
96+
* - Sensitive names (`authorization`, `cookie`, ...) always show up as `[Filtered]`.
97+
* - Names on the `deny` list show up as `[Filtered]`.
98+
* - If an `allow` list is configured, a header must appear there as well, or it shows up as `[Filtered]`.
99+
*/
91100
headersToSpanAttributes?: {
92101
requestHeaders?: string[];
93102
responseHeaders?: string[];

‎packages/node/src/integrations/node-fetch/undici-instrumentation.ts‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ import {
4040
getUrlQuery,
4141
filterCollectedUrl,
4242
filterCollectedUrlQuery,
43-
_INTERNAL_shouldFilterDataKey,
44-
_INTERNAL_FILTERED_VALUE,
43+
httpHeadersToSpanAttributes,
4544
} from '@sentry/core';
4645
import { addFetchRequestBreadcrumb, addTracePropagationHeadersToFetchRequest } from '../../utils/outgoingFetchRequest';
4746
import {
@@ -314,20 +313,21 @@ function onRequestHeaders(config: NodeFetchOptions, { request, socket }: Request
314313

315314
// After hooks have been processed (which may modify request headers)
316315
// we can collect the headers based on the configuration
317-
if (config.headersToSpanAttributes?.requestHeaders) {
316+
const client = getClient();
317+
if (config.headersToSpanAttributes?.requestHeaders && client) {
318318
const headersToAttribs = new Set(config.headersToSpanAttributes.requestHeaders.map(n => n.toLowerCase()));
319319
const headersMap = parseRequestHeaders(request);
320320

321+
const allowlisted: Record<string, string | string[]> = {};
321322
for (const [name, value] of headersMap.entries()) {
322323
if (headersToAttribs.has(name)) {
323-
// An allowlist entry does not exempt a header from the denylist.
324-
spanAttributes[`http.request.header.${name}`] = _INTERNAL_shouldFilterDataKey(name, true)
325-
? [_INTERNAL_FILTERED_VALUE]
326-
: Array.isArray(value)
327-
? value
328-
: [value];
324+
allowlisted[name] = value;
329325
}
330326
}
327+
328+
// An entry in `headersToSpanAttributes` does not exempt a header from the `dataCollection`
329+
// filtering, so the allowlisted subset goes through the same pipeline as any other header.
330+
Object.assign(spanAttributes, httpHeadersToSpanAttributes(allowlisted, client.getDataCollectionOptions()));
331331
}
332332

333333
span.setAttributes(spanAttributes);
@@ -360,30 +360,31 @@ function onResponseHeaders(config: NodeFetchOptions, { request, response }: Resp
360360
() => undefined,
361361
);
362362

363-
if (config.headersToSpanAttributes?.responseHeaders) {
363+
const client = getClient();
364+
if (config.headersToSpanAttributes?.responseHeaders && client) {
364365
const headersToAttribs = new Set<string>();
365366
config.headersToSpanAttributes?.responseHeaders.forEach(name => headersToAttribs.add(name.toLowerCase()));
366367

368+
const allowlisted: Record<string, string[]> = {};
367369
for (let idx = 0; idx < response.headers.length; idx = idx + 2) {
368370
const nameBuf = response.headers[idx];
369371
const valueBuf = response.headers[idx + 1];
370372
if (nameBuf === undefined || valueBuf === undefined) {
371373
continue;
372374
}
373375
const name = nameBuf.toString().toLowerCase();
374-
const value = valueBuf;
375376

376377
if (headersToAttribs.has(name)) {
377-
const attrName = `http.response.header.${name}`;
378-
if (_INTERNAL_shouldFilterDataKey(name, true)) {
379-
spanAttributes[attrName] = [_INTERNAL_FILTERED_VALUE];
380-
} else if (!Object.prototype.hasOwnProperty.call(spanAttributes, attrName)) {
381-
spanAttributes[attrName] = [value.toString()];
382-
} else {
383-
(spanAttributes[attrName] as string[]).push(value.toString());
384-
}
378+
(allowlisted[name] ??= []).push(valueBuf.toString());
385379
}
386380
}
381+
382+
// An entry in `headersToSpanAttributes` does not exempt a header from the `dataCollection`
383+
// filtering, so the allowlisted subset goes through the same pipeline as any other header.
384+
Object.assign(
385+
spanAttributes,
386+
httpHeadersToSpanAttributes(allowlisted, client.getDataCollectionOptions(), 'response'),
387+
);
387388
}
388389

389390
span.setAttributes(spanAttributes);

0 commit comments

Comments
 (0)