From f5d884e7f2c4e666d74a901310cb0d7eb21d7a6b Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 4 Apr 2023 18:12:50 +0200 Subject: [PATCH 1/3] fix(node): Redact URL authority only in breadcrumbs and spans --- packages/node/src/integrations/utils/http.ts | 6 +++--- packages/node/test/integrations/http.test.ts | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/node/src/integrations/utils/http.ts b/packages/node/src/integrations/utils/http.ts index 86902425914d..741188b9899e 100644 --- a/packages/node/src/integrations/utils/http.ts +++ b/packages/node/src/integrations/utils/http.ts @@ -48,7 +48,8 @@ export function extractUrl(requestOptions: RequestOptions): string { !requestOptions.port || requestOptions.port === 80 || requestOptions.port === 443 ? '' : `:${requestOptions.port}`; // do not include search or hash in span descriptions, per https://develop.sentry.dev/sdk/data-handling/#structuring-data const path = requestOptions.pathname || '/'; - const authority = requestOptions.auth ? `${requestOptions.auth}@` : ''; + // // always filter authority, see https://develop.sentry.dev/sdk/data-handling/#structuring-data + const authority = requestOptions.auth ? '[Filtered]:[Filtered]@' : ''; return `${protocol}//${authority}${hostname}${port}${path}`; } @@ -123,8 +124,7 @@ export function urlToOptions(url: URL): RequestOptions { options.port = Number(url.port); } if (url.username || url.password) { - // always filter authority, see https://develop.sentry.dev/sdk/data-handling/#structuring-data - options.auth = '[Filtered]:[Filtered]'; + options.auth = `${url.username}:${url.password}`; } return options; } diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 555aad7f44b3..cb93efb14cf5 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -229,6 +229,20 @@ describe('tracing', () => { expect(spans[1].description).toEqual('GET http://[Filtered]:[Filtered]@dogs.are.great/'); }); + it.only('filters the authority (password) in span description', () => { + nock('http://username:password@dogs.are.great').get('/').reply(200); + + const transaction = createTransactionOnScope(); + const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; + + http.get('http://:password@dogs.are.great/'); + + expect(spans.length).toEqual(2); + + // our span is at index 1 because the transaction itself is at index 0 + expect(spans[1].description).toEqual('GET http://[Filtered]:[Filtered]@dogs.are.great/'); + }); + describe('Tracing options', () => { beforeEach(() => { // hacky way of restoring monkey patched functions From bdf8bc760881ea1866a6ba1823fb10f5aa9d9235 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 5 Apr 2023 11:06:01 +0200 Subject: [PATCH 2/3] Update packages/node/src/integrations/utils/http.ts Co-authored-by: Luca Forstner --- packages/node/src/integrations/utils/http.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/integrations/utils/http.ts b/packages/node/src/integrations/utils/http.ts index 741188b9899e..1ebafce197a1 100644 --- a/packages/node/src/integrations/utils/http.ts +++ b/packages/node/src/integrations/utils/http.ts @@ -48,7 +48,7 @@ export function extractUrl(requestOptions: RequestOptions): string { !requestOptions.port || requestOptions.port === 80 || requestOptions.port === 443 ? '' : `:${requestOptions.port}`; // do not include search or hash in span descriptions, per https://develop.sentry.dev/sdk/data-handling/#structuring-data const path = requestOptions.pathname || '/'; - // // always filter authority, see https://develop.sentry.dev/sdk/data-handling/#structuring-data + // always filter authority, see https://develop.sentry.dev/sdk/data-handling/#structuring-data const authority = requestOptions.auth ? '[Filtered]:[Filtered]@' : ''; return `${protocol}//${authority}${hostname}${port}${path}`; From f02f5f1ce727911d14ccf34156ff0c25d4cf8ef2 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 5 Apr 2023 12:06:48 +0200 Subject: [PATCH 3/3] refine authority redaction --- packages/node/src/integrations/utils/http.ts | 7 ++++- packages/node/test/integrations/http.test.ts | 28 +++++++------------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/packages/node/src/integrations/utils/http.ts b/packages/node/src/integrations/utils/http.ts index 1ebafce197a1..1ef1be2549b5 100644 --- a/packages/node/src/integrations/utils/http.ts +++ b/packages/node/src/integrations/utils/http.ts @@ -49,11 +49,16 @@ export function extractUrl(requestOptions: RequestOptions): string { // do not include search or hash in span descriptions, per https://develop.sentry.dev/sdk/data-handling/#structuring-data const path = requestOptions.pathname || '/'; // always filter authority, see https://develop.sentry.dev/sdk/data-handling/#structuring-data - const authority = requestOptions.auth ? '[Filtered]:[Filtered]@' : ''; + const authority = requestOptions.auth ? redactAuthority(requestOptions.auth) : ''; return `${protocol}//${authority}${hostname}${port}${path}`; } +function redactAuthority(auth: string): string { + const [user, password] = auth.split(':'); + return `${user ? '[Filtered]' : ''}:${password ? '[Filtered]' : ''}@`; +} + /** * Handle various edge cases in the span description (for spans representing http(s) requests). * diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index cb93efb14cf5..cf4f979d2753 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -215,32 +215,24 @@ describe('tracing', () => { expect(spans[1].data['http.fragment']).toEqual('learn-more'); }); - it('filters the authority (username and password) in span description', () => { - nock('http://username:password@dogs.are.great').get('/').reply(200); + it.each([ + ['user:pwd', '[Filtered]:[Filtered]@'], + ['user:', '[Filtered]:@'], + ['user', '[Filtered]:@'], + [':pwd', ':[Filtered]@'], + ['', ''], + ])('filters the authority %s in span description', (auth, redactedAuth) => { + nock(`http://${auth}@dogs.are.great`).get('/').reply(200); const transaction = createTransactionOnScope(); const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; - http.get('http://username:password@dogs.are.great/'); + http.get(`http://${auth}@dogs.are.great/`); expect(spans.length).toEqual(2); // our span is at index 1 because the transaction itself is at index 0 - expect(spans[1].description).toEqual('GET http://[Filtered]:[Filtered]@dogs.are.great/'); - }); - - it.only('filters the authority (password) in span description', () => { - nock('http://username:password@dogs.are.great').get('/').reply(200); - - const transaction = createTransactionOnScope(); - const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; - - http.get('http://:password@dogs.are.great/'); - - expect(spans.length).toEqual(2); - - // our span is at index 1 because the transaction itself is at index 0 - expect(spans[1].description).toEqual('GET http://[Filtered]:[Filtered]@dogs.are.great/'); + expect(spans[1].description).toEqual(`GET http://${redactedAuth}dogs.are.great/`); }); describe('Tracing options', () => {