From f4665c8ce38621b6438b162324cc2eb8512fdfb1 Mon Sep 17 00:00:00 2001 From: Shireen Missi <shireen@n8n.io> Date: Thu, 29 Aug 2024 13:53:39 +0100 Subject: [PATCH 1/4] Redact authorization headers --- .../nodes/HttpRequest/GenericFunctions.ts | 9 ++++- .../HttpRequest/test/utils/utils.test.ts | 36 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts b/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts index a6994be2552fa..63e0a658e72c3 100644 --- a/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts +++ b/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts @@ -88,7 +88,14 @@ export function sanitizeUiMessage( ), }; } - + //redact headers with Base64 encoded credentials + const headers = sendRequest.headers as IDataObject; + if (headers) { + const headerKey = Object.keys(headers).find((key) => key.toLowerCase() === 'authorization'); + if (headerKey) { + headers[headerKey] = REDACTED; + } + } if (secrets && secrets.length > 0) { return redact(sendRequest, secrets); } diff --git a/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts b/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts index b3e9d5fbd7170..f307e8c39b83f 100644 --- a/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts +++ b/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts @@ -136,5 +136,41 @@ describe('HTTP Node Utils', () => { uri: 'https://example.com', }); }); + it('should redact the Authorization header', () => { + const requestOptions: IRequestOptions = { + method: 'POST', + uri: 'https://example.com', + body: { sessionToken: 'secret', other: 'foo' }, + headers: { authorization: 'Bearer some-sensitive-token', other: 'foo' }, + auth: { user: 'user', password: 'secret' }, + }; + const authDataKeys = {}; + const sanitizedRequest = sanitizeUiMessage(requestOptions, authDataKeys); + + expect(sanitizedRequest.headers).toEqual({ authorization: REDACTED, other: 'foo' }); + }); + + it('should leave headers unchanged if Authorization header is not present', () => { + const requestOptions: IRequestOptions = { + method: 'POST', + uri: 'https://example.com', + body: { sessionToken: 'secret', other: 'foo' }, + headers: { other: 'foo' }, + auth: { user: 'user', password: 'secret' }, + }; + const authDataKeys = {}; + const sanitizedRequest = sanitizeUiMessage(requestOptions, authDataKeys); + + expect(sanitizedRequest.headers).toEqual({ other: 'foo' }); + }); + + it('should handle case when headers are undefined', () => { + const requestOptions: IRequestOptions = {}; + + const authDataKeys = {}; + const sanitizedRequest = sanitizeUiMessage(requestOptions, authDataKeys); + + expect(sanitizedRequest.headers).toBeUndefined(); + }); }); }); From dbbfdafe9ea1e1fb166a13379d8d09a4c60efb98 Mon Sep 17 00:00:00 2001 From: Shireen Missi <shireen@n8n.io> Date: Thu, 29 Aug 2024 14:19:59 +0100 Subject: [PATCH 2/4] Add one more tests --- .../nodes/HttpRequest/test/utils/utils.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts b/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts index f307e8c39b83f..c0e550d6101de 100644 --- a/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts +++ b/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts @@ -150,6 +150,20 @@ describe('HTTP Node Utils', () => { expect(sanitizedRequest.headers).toEqual({ authorization: REDACTED, other: 'foo' }); }); + it('should redact the Authorization header when the key starts with an uppercase letter', () => { + const requestOptions: IRequestOptions = { + method: 'POST', + uri: 'https://example.com', + body: { sessionToken: 'secret', other: 'foo' }, + headers: { Authorization: 'Basic another-sensitive-token', other: 'foo' }, + auth: { user: 'user', password: 'secret' }, + }; + const authDataKeys = {}; + const sanitizedRequest = sanitizeUiMessage(requestOptions, authDataKeys); + + expect(sanitizedRequest.headers).toEqual({ Authorization: REDACTED, other: 'foo' }); + }); + it('should leave headers unchanged if Authorization header is not present', () => { const requestOptions: IRequestOptions = { method: 'POST', From 34bda8bf9ec8a8d2da93e7ef79e9a17b04b1abf8 Mon Sep 17 00:00:00 2001 From: Shireen Missi <shireen@n8n.io> Date: Thu, 29 Aug 2024 14:51:20 +0100 Subject: [PATCH 3/4] Remove comment --- packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts b/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts index 63e0a658e72c3..ef3c2a05c4893 100644 --- a/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts +++ b/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts @@ -88,7 +88,6 @@ export function sanitizeUiMessage( ), }; } - //redact headers with Base64 encoded credentials const headers = sendRequest.headers as IDataObject; if (headers) { const headerKey = Object.keys(headers).find((key) => key.toLowerCase() === 'authorization'); From 5b4f1b5b18a6e6770cad9d1bf73ab5b170630ffc Mon Sep 17 00:00:00 2001 From: Shireen Missi <shireen@n8n.io> Date: Thu, 29 Aug 2024 15:10:01 +0100 Subject: [PATCH 4/4] Redact more header keys --- .../nodes/HttpRequest/GenericFunctions.ts | 17 ++++- .../HttpRequest/test/utils/utils.test.ts | 69 +++++++++++-------- 2 files changed, 55 insertions(+), 31 deletions(-) diff --git a/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts b/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts index ef3c2a05c4893..07d25b8b49f25 100644 --- a/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts +++ b/packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts @@ -88,11 +88,22 @@ export function sanitizeUiMessage( ), }; } + const HEADER_BLOCKLIST = new Set([ + 'authorization', + 'x-api-key', + 'x-auth-token', + 'cookie', + 'proxy-authorization', + 'sslclientcert', + ]); + const headers = sendRequest.headers as IDataObject; + if (headers) { - const headerKey = Object.keys(headers).find((key) => key.toLowerCase() === 'authorization'); - if (headerKey) { - headers[headerKey] = REDACTED; + for (const headerName of Object.keys(headers)) { + if (HEADER_BLOCKLIST.has(headerName.toLowerCase())) { + headers[headerName] = REDACTED; + } } } if (secrets && secrets.length > 0) { diff --git a/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts b/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts index c0e550d6101de..0ad0bf35d19ca 100644 --- a/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts +++ b/packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts @@ -136,32 +136,47 @@ describe('HTTP Node Utils', () => { uri: 'https://example.com', }); }); - it('should redact the Authorization header', () => { - const requestOptions: IRequestOptions = { - method: 'POST', - uri: 'https://example.com', - body: { sessionToken: 'secret', other: 'foo' }, - headers: { authorization: 'Bearer some-sensitive-token', other: 'foo' }, - auth: { user: 'user', password: 'secret' }, - }; - const authDataKeys = {}; - const sanitizedRequest = sanitizeUiMessage(requestOptions, authDataKeys); - - expect(sanitizedRequest.headers).toEqual({ authorization: REDACTED, other: 'foo' }); - }); - it('should redact the Authorization header when the key starts with an uppercase letter', () => { - const requestOptions: IRequestOptions = { - method: 'POST', - uri: 'https://example.com', - body: { sessionToken: 'secret', other: 'foo' }, - headers: { Authorization: 'Basic another-sensitive-token', other: 'foo' }, - auth: { user: 'user', password: 'secret' }, - }; - const authDataKeys = {}; - const sanitizedRequest = sanitizeUiMessage(requestOptions, authDataKeys); + const headersToTest = [ + 'authorization', + 'x-api-key', + 'x-auth-token', + 'cookie', + 'proxy-authorization', + 'sslclientcert', + ]; + + headersToTest.forEach((header) => { + it(`should redact the ${header} header when the key is lowercase`, () => { + const requestOptions: IRequestOptions = { + method: 'POST', + uri: 'https://example.com', + body: { sessionToken: 'secret', other: 'foo' }, + headers: { [header]: 'some-sensitive-token', other: 'foo' }, + auth: { user: 'user', password: 'secret' }, + }; + + const sanitizedRequest = sanitizeUiMessage(requestOptions, {}); + + expect(sanitizedRequest.headers).toEqual({ [header]: REDACTED, other: 'foo' }); + }); - expect(sanitizedRequest.headers).toEqual({ Authorization: REDACTED, other: 'foo' }); + it(`should redact the ${header} header when the key is uppercase`, () => { + const requestOptions: IRequestOptions = { + method: 'POST', + uri: 'https://example.com', + body: { sessionToken: 'secret', other: 'foo' }, + headers: { [header.toUpperCase()]: 'some-sensitive-token', other: 'foo' }, + auth: { user: 'user', password: 'secret' }, + }; + + const sanitizedRequest = sanitizeUiMessage(requestOptions, {}); + + expect(sanitizedRequest.headers).toEqual({ + [header.toUpperCase()]: REDACTED, + other: 'foo', + }); + }); }); it('should leave headers unchanged if Authorization header is not present', () => { @@ -172,8 +187,7 @@ describe('HTTP Node Utils', () => { headers: { other: 'foo' }, auth: { user: 'user', password: 'secret' }, }; - const authDataKeys = {}; - const sanitizedRequest = sanitizeUiMessage(requestOptions, authDataKeys); + const sanitizedRequest = sanitizeUiMessage(requestOptions, {}); expect(sanitizedRequest.headers).toEqual({ other: 'foo' }); }); @@ -181,8 +195,7 @@ describe('HTTP Node Utils', () => { it('should handle case when headers are undefined', () => { const requestOptions: IRequestOptions = {}; - const authDataKeys = {}; - const sanitizedRequest = sanitizeUiMessage(requestOptions, authDataKeys); + const sanitizedRequest = sanitizeUiMessage(requestOptions, {}); expect(sanitizedRequest.headers).toBeUndefined(); });