fix(HTTP Request Node): Sanitize secrets of predefined credentials (#9612)

packages/core/src/NodeExecuteFunctions.ts

@@ -2844,7 +2844,8 @@ const getCommonWorkflowFunctions = (
 	getInstanceBaseUrl: () => additionalData.instanceBaseUrl,
 	getInstanceId: () => Container.get(InstanceSettings).instanceId,
 	getTimezone: () => getTimezone(workflow),
-
+	getCredentialsProperties: (type: string) =>
+		additionalData.credentialsHelper.getCredentialsProperties(type),
 	prepareOutputData: async (outputData) => [outputData],
 });
 

packages/nodes-base/nodes/HttpRequest/GenericFunctions.ts

@@ -1,16 +1,20 @@
 import type { SecureContextOptions } from 'tls';
 import type {
+	ICredentialDataDecryptedObject,
 	IDataObject,
 	INodeExecutionData,
+	INodeProperties,
 	IOAuth2Options,
 	IRequestOptions,
 } from 'n8n-workflow';
 
 import set from 'lodash/set';
+import isPlainObject from 'lodash/isPlainObject';
 
 import FormData from 'form-data';
-import type { HttpSslAuthCredentials } from './interfaces';
 import { formatPrivateKey } from '../../utils/utilities';
+import type { HttpSslAuthCredentials } from './interfaces';
+import get from 'lodash/get';
 
 export type BodyParameter = {
 	name: string;
@@ -29,7 +33,33 @@ export const replaceNullValues = (item: INodeExecutionData) => {
 	return item;
 };
 
-export function sanitizeUiMessage(request: IRequestOptions, authDataKeys: IAuthDataSanitizeKeys) {
+export const REDACTED = '**hidden**';
+
+function isObject(obj: unknown): obj is IDataObject {
+	return isPlainObject(obj);
+}
+
+function redact<T = unknown>(obj: T, secrets: string[]): T {
+	if (typeof obj === 'string') {
+		return secrets.reduce((safe, secret) => safe.replace(secret, REDACTED), obj) as T;
+	}
+
+	if (Array.isArray(obj)) {
+		return obj.map((item) => redact(item, secrets)) as T;
+	} else if (isObject(obj)) {
+		for (const [key, value] of Object.entries(obj)) {
+			(obj as IDataObject)[key] = redact(value, secrets);
+		}
+	}
+
+	return obj;
+}
+
+export function sanitizeUiMessage(
+	request: IRequestOptions,
+	authDataKeys: IAuthDataSanitizeKeys,
+	secrets?: string[],
+) {
 	let sendRequest = request as unknown as IDataObject;
 
 	// Protect browser from sending large binary data
@@ -38,7 +68,7 @@ export function sanitizeUiMessage(request: IRequestOptions, authDataKeys: IAuthD
 			...request,
 			body: `Binary data got replaced with this text. Original was a Buffer with a size of ${
 				(request.body as string).length
-			} byte.`,
+			} bytes.`,
 		};
 	}
 
@@ -50,7 +80,7 @@ export function sanitizeUiMessage(request: IRequestOptions, authDataKeys: IAuthD
 				// eslint-disable-next-line @typescript-eslint/no-loop-func
 				(acc: IDataObject, curr) => {
 					acc[curr] = authDataKeys[requestProperty].includes(curr)
-						? '** hidden **'
+						? REDACTED
 						: (sendRequest[requestProperty] as IDataObject)[curr];
 					return acc;
 				},
@@ -59,9 +89,33 @@ export function sanitizeUiMessage(request: IRequestOptions, authDataKeys: IAuthD
 		};
 	}
 
+	if (secrets && secrets.length > 0) {
+		return redact(sendRequest, secrets);
+	}
+
 	return sendRequest;
 }
 
+export function getSecrets(
+	properties: INodeProperties[],
+	credentials: ICredentialDataDecryptedObject,
+): string[] {
+	const sensitivePropNames = new Set(
+		properties.filter((prop) => prop.typeOptions?.password).map((prop) => prop.name),
+	);
+
+	const secrets = Object.entries(credentials)
+		.filter(([propName]) => sensitivePropNames.has(propName))
+		.map(([_, value]) => value)
+		.filter((value): value is string => typeof value === 'string');
+	const oauthAccessToken = get(credentials, 'oauthTokenData.access_token');
+	if (typeof oauthAccessToken === 'string') {
+		secrets.push(oauthAccessToken);
+	}
+
+	return secrets;
+}
+
 export const getOAuth2AdditionalParameters = (nodeCredentialType: string) => {
 	const oAuth2Options: { [credentialType: string]: IOAuth2Options } = {
 		bitlyOAuth2Api: {

packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts

@@ -29,14 +29,15 @@ import type { BodyParameter, IAuthDataSanitizeKeys } from '../GenericFunctions';
 import {
 	binaryContentTypes,
 	getOAuth2AdditionalParameters,
+	getSecrets,
 	prepareRequestBody,
 	reduceAsync,
 	replaceNullValues,
 	sanitizeUiMessage,
 	setAgentOptions,
 } from '../GenericFunctions';
-import { keysToLowercase } from '@utils/utilities';
 import type { HttpSslAuthCredentials } from '../interfaces';
+import { keysToLowercase } from '@utils/utilities';
 
 function toText<T>(data: T) {
 	if (typeof data === 'object' && data !== null) {
@@ -1299,7 +1300,12 @@ export class HttpRequestV3 implements INodeType {
 			requestInterval: number;
 		};
 
-		const sanitazedRequests: IDataObject[] = [];
+		const requests: Array<{
+			options: IRequestOptions;
+			authKeys: IAuthDataSanitizeKeys;
+			credentialType?: string;
+		}> = [];
+
 		for (let itemIndex = 0; itemIndex < items.length; itemIndex++) {
 			if (authentication === 'genericCredentialType') {
 				genericCredentialType = this.getNodeParameter('genericAuthType', 0) as string;
@@ -1696,11 +1702,11 @@ export class HttpRequestV3 implements INodeType {
 				}
 			}
 
-			try {
-				const sanitazedRequestOptions = sanitizeUiMessage(requestOptions, authDataKeys);
-				this.sendMessageToUI(sanitazedRequestOptions);
-				sanitazedRequests.push(sanitazedRequestOptions);
-			} catch (e) {}
+			requests.push({
+				options: requestOptions,
+				authKeys: authDataKeys,
+				credentialType: nodeCredentialType,
+			});
 
 			if (pagination && pagination.paginationMode !== 'off') {
 				let continueExpression = '={{false}}';
@@ -1827,7 +1833,29 @@ export class HttpRequestV3 implements INodeType {
 				requestPromises.push(requestWithAuthentication);
 			}
 		}
-		const promisesResponses = await Promise.allSettled(requestPromises);
+
+		const sanitizedRequests: IDataObject[] = [];
+		const promisesResponses = await Promise.allSettled(
+			requestPromises.map(
+				async (requestPromise, itemIndex) =>
+					await requestPromise.finally(async () => {
+						try {
+							// Secrets need to be read after the request because secrets could have changed
+							// For example: OAuth token refresh, preAuthentication
+							const { options, authKeys, credentialType } = requests[itemIndex];
+							let secrets: string[] = [];
+							if (credentialType) {
+								const properties = this.getCredentialsProperties(credentialType);
+								const credentials = await this.getCredentials(credentialType, itemIndex);
+								secrets = getSecrets(properties, credentials);
+							}
+							const sanitizedRequestOptions = sanitizeUiMessage(options, authKeys, secrets);
+							sanitizedRequests.push(sanitizedRequestOptions);
+							this.sendMessageToUI(sanitizedRequestOptions);
+						} catch (e) {}
+					}),
+			),
+		);
 
 		let responseData: any;
 		for (let itemIndex = 0; itemIndex < items.length; itemIndex++) {
@@ -1842,7 +1870,7 @@ export class HttpRequestV3 implements INodeType {
 						responseData.reason.error = Buffer.from(responseData.reason.error as Buffer).toString();
 					}
 					const error = new NodeApiError(this.getNode(), responseData as JsonObject, { itemIndex });
-					set(error, 'context.request', sanitazedRequests[itemIndex]);
+					set(error, 'context.request', sanitizedRequests[itemIndex]);
 					throw error;
 				} else {
 					removeCircularRefs(responseData.reason as JsonObject);

packages/nodes-base/nodes/HttpRequest/test/utils/utils.test.ts

@@ -1,75 +1,140 @@
 import type { IRequestOptions } from 'n8n-workflow';
-import { prepareRequestBody, setAgentOptions } from '../../GenericFunctions';
+import {
+	REDACTED,
+	prepareRequestBody,
+	sanitizeUiMessage,
+	setAgentOptions,
+} from '../../GenericFunctions';
 import type { BodyParameter, BodyParametersReducer } from '../../GenericFunctions';
 
-describe('HTTP Node Utils, prepareRequestBody', () => {
-	it('should call default reducer', async () => {
-		const bodyParameters: BodyParameter[] = [
-			{
-				name: 'foo.bar',
-				value: 'baz',
-			},
-		];
-		const defaultReducer: BodyParametersReducer = jest.fn();
+describe('HTTP Node Utils', () => {
+	describe('prepareRequestBody', () => {
+		it('should call default reducer', async () => {
+			const bodyParameters: BodyParameter[] = [
+				{
+					name: 'foo.bar',
+					value: 'baz',
+				},
+			];
+			const defaultReducer: BodyParametersReducer = jest.fn();
 
-		await prepareRequestBody(bodyParameters, 'json', 3, defaultReducer);
+			await prepareRequestBody(bodyParameters, 'json', 3, defaultReducer);
 
-		expect(defaultReducer).toBeCalledTimes(1);
-		expect(defaultReducer).toBeCalledWith({}, { name: 'foo.bar', value: 'baz' });
-	});
+			expect(defaultReducer).toBeCalledTimes(1);
+			expect(defaultReducer).toBeCalledWith({}, { name: 'foo.bar', value: 'baz' });
+		});
 
-	it('should call process dot notations', async () => {
-		const bodyParameters: BodyParameter[] = [
-			{
-				name: 'foo.bar.spam',
-				value: 'baz',
-			},
-		];
-		const defaultReducer: BodyParametersReducer = jest.fn();
+		it('should call process dot notations', async () => {
+			const bodyParameters: BodyParameter[] = [
+				{
+					name: 'foo.bar.spam',
+					value: 'baz',
+				},
+			];
+			const defaultReducer: BodyParametersReducer = jest.fn();
 
-		const result = await prepareRequestBody(bodyParameters, 'json', 4, defaultReducer);
+			const result = await prepareRequestBody(bodyParameters, 'json', 4, defaultReducer);
 
-		expect(defaultReducer).toBeCalledTimes(0);
-		expect(result).toBeDefined();
-		expect(result).toEqual({ foo: { bar: { spam: 'baz' } } });
-	});
-});
-
-describe('HTTP Node Utils, setAgentOptions', () => {
-	it("should not have agentOptions as it's undefined", async () => {
-		const requestOptions: IRequestOptions = {
-			method: 'GET',
-			uri: 'https://example.com',
-		};
-
-		const sslCertificates = undefined;
-
-		setAgentOptions(requestOptions, sslCertificates);
-
-		expect(requestOptions).toEqual({
-			method: 'GET',
-			uri: 'https://example.com',
+			expect(defaultReducer).toBeCalledTimes(0);
+			expect(result).toBeDefined();
+			expect(result).toEqual({ foo: { bar: { spam: 'baz' } } });
 		});
 	});
 
-	it('should have agentOptions set', async () => {
-		const requestOptions: IRequestOptions = {
-			method: 'GET',
-			uri: 'https://example.com',
-		};
+	describe('setAgentOptions', () => {
+		it("should not have agentOptions as it's undefined", async () => {
+			const requestOptions: IRequestOptions = {
+				method: 'GET',
+				uri: 'https://example.com',
+			};
 
-		const sslCertificates = {
-			ca: 'mock-ca',
-		};
+			const sslCertificates = undefined;
 
-		setAgentOptions(requestOptions, sslCertificates);
+			setAgentOptions(requestOptions, sslCertificates);
 
-		expect(requestOptions).toStrictEqual({
-			method: 'GET',
-			uri: 'https://example.com',
-			agentOptions: {
+			expect(requestOptions).toEqual({
+				method: 'GET',
+				uri: 'https://example.com',
+			});
+		});
+
+		it('should have agentOptions set', async () => {
+			const requestOptions: IRequestOptions = {
+				method: 'GET',
+				uri: 'https://example.com',
+			};
+
+			const sslCertificates = {
 				ca: 'mock-ca',
-			},
+			};
+
+			setAgentOptions(requestOptions, sslCertificates);
+
+			expect(requestOptions).toStrictEqual({
+				method: 'GET',
+				uri: 'https://example.com',
+				agentOptions: {
+					ca: 'mock-ca',
+				},
+			});
+		});
+	});
+
+	describe('sanitizeUiMessage', () => {
+		it('should remove large Buffers', async () => {
+			const requestOptions: IRequestOptions = {
+				method: 'POST',
+				uri: 'https://example.com',
+				body: Buffer.alloc(900000),
+			};
+
+			expect(sanitizeUiMessage(requestOptions, {}).body).toEqual(
+				'Binary data got replaced with this text. Original was a Buffer with a size of 900000 bytes.',
+			);
+		});
+
+		it('should remove keys that contain sensitive data', async () => {
+			const requestOptions: IRequestOptions = {
+				method: 'POST',
+				uri: 'https://example.com',
+				body: { sessionToken: 'secret', other: 'foo' },
+				headers: { authorization: 'secret', other: 'foo' },
+				auth: { user: 'user', password: 'secret' },
+			};
+
+			expect(
+				sanitizeUiMessage(requestOptions, {
+					headers: ['authorization'],
+					body: ['sessionToken'],
+					auth: ['password'],
+				}),
+			).toEqual({
+				body: { sessionToken: REDACTED, other: 'foo' },
+				headers: { other: 'foo', authorization: REDACTED },
+				auth: { user: 'user', password: REDACTED },
+				method: 'POST',
+				uri: 'https://example.com',
+			});
+		});
+
+		it('should remove secrets', async () => {
+			const requestOptions: IRequestOptions = {
+				method: 'POST',
+				uri: 'https://example.com',
+				body: { nested: { secret: 'secretAccessToken' } },
+				headers: { authorization: 'secretAccessToken', other: 'foo' },
+			};
+
+			expect(sanitizeUiMessage(requestOptions, {}, ['secretAccessToken'])).toEqual({
+				body: {
+					nested: {
+						secret: REDACTED,
+					},
+				},
+				headers: { authorization: REDACTED, other: 'foo' },
+				method: 'POST',
+				uri: 'https://example.com',
+			});
 		});
 	});
 });

packages/workflow/src/Interfaces.ts

@@ -216,6 +216,8 @@ export abstract class ICredentialsHelper {
 		type: string,
 		data: ICredentialDataDecryptedObject,
 	): Promise<void>;
+
+	abstract getCredentialsProperties(type: string): INodeProperties[];
 }
 
 export interface IAuthenticateBase {
@@ -813,6 +815,7 @@ export type NodeTypeAndVersion = {
 export interface FunctionsBase {
 	logger: Logger;
 	getCredentials(type: string, itemIndex?: number): Promise<ICredentialDataDecryptedObject>;
+	getCredentialsProperties(type: string): INodeProperties[];
 	getExecutionId(): string;
 	getNode(): INode;
 	getWorkflow(): IWorkflowMetadata;