fix(Respond to Webhook Node)!: Surround HTML in iframe (#16978)

Co-authored-by: Tomi Turtiainen <10324676+tomi@users.noreply.github.com>

packages/nodes-base/nodes/RespondToWebhook/RespondToWebhook.node.ts

@@ -1,5 +1,6 @@
 import jwt from 'jsonwebtoken';
 import set from 'lodash/set';
+import { isHtmlRenderedContentType, sandboxHtmlResponse } from 'n8n-core';
 import type {
 	IDataObject,
 	IExecuteFunctions,
@@ -12,7 +13,6 @@ import type {
 } from 'n8n-workflow';
 import {
 	jsonParse,
-	BINARY_ENCODING,
 	NodeOperationError,
 	NodeConnectionTypes,
 	WEBHOOK_NODE_TYPE,
@@ -22,8 +22,9 @@ import {
 } from 'n8n-workflow';
 import type { Readable } from 'stream';
 
-import { configuredOutputs } from './utils';
 import { formatPrivateKey, generatePairedItemData } from '../../utils/utilities';
+import { configuredOutputs } from './utils/outputs';
+import { getBinaryResponse } from './utils/binary';
 
 const respondWithProperty: INodeProperties = {
 	displayName: 'Respond With',
@@ -359,6 +360,9 @@ export class RespondToWebhook implements INodeType {
 				}
 			}
 
+			const hasHtmlContentType =
+				headers['content-type'] && isHtmlRenderedContentType(headers['content-type'] as string);
+
 			let statusCode = (options.responseCode as number) || 200;
 			let responseBody: IN8nHttpResponse | Readable;
 			if (respondWith === 'json') {
@@ -412,7 +416,12 @@ export class RespondToWebhook implements INodeType {
 					? set({}, options.responseKey as string, items[0].json)
 					: items[0].json;
 			} else if (respondWith === 'text') {
-				responseBody = this.getNodeParameter('responseBody', 0) as string;
+				// If a user doesn't set the content-type header and uses html, the html can still be rendered on the browser
+				if (hasHtmlContentType || !headers['content-type']) {
+					responseBody = sandboxHtmlResponse(this.getNodeParameter('responseBody', 0) as string);
+				} else {
+					responseBody = this.getNodeParameter('responseBody', 0) as string;
+				}
 			} else if (respondWith === 'binary') {
 				const item = items[0];
 
@@ -438,16 +447,8 @@ export class RespondToWebhook implements INodeType {
 				}
 
 				const binaryData = this.helpers.assertBinaryData(0, responseBinaryPropertyName);
-				if (binaryData.id) {
-					responseBody = { binaryData };
-				} else {
-					responseBody = Buffer.from(binaryData.data, BINARY_ENCODING);
-					headers['content-length'] = (responseBody as Buffer).length;
-				}
 
-				if (!headers['content-type']) {
-					headers['content-type'] = binaryData.mimeType;
-				}
+				responseBody = getBinaryResponse(binaryData, headers);
 			} else if (respondWith === 'redirect') {
 				headers.location = this.getNodeParameter('redirectURL', 0) as string;
 				statusCode = (options.responseCode as number) ?? 307;
@@ -458,6 +459,15 @@ export class RespondToWebhook implements INodeType {
 				);
 			}
 
+			if (
+				hasHtmlContentType &&
+				respondWith !== 'text' &&
+				respondWith !== 'binary' &&
+				responseBody
+			) {
+				responseBody = sandboxHtmlResponse(JSON.stringify(responseBody as string));
+			}
+
 			response = {
 				body: responseBody,
 				headers,

packages/nodes-base/nodes/RespondToWebhook/test/RespondToWebhook.test.ts

@@ -1,6 +1,6 @@
 import type { DeepMockProxy } from 'jest-mock-extended';
 import { mock, mockDeep } from 'jest-mock-extended';
-import { constructExecutionMetaData } from 'n8n-core';
+import { constructExecutionMetaData, sandboxHtmlResponse } from 'n8n-core';
 import {
 	BINARY_ENCODING,
 	WAIT_NODE_TYPE,
@@ -163,7 +163,7 @@ describe('RespondToWebhook Node', () => {
 
 			await expect(respondToWebhook.execute.call(mockExecuteFunctions)).resolves.not.toThrow();
 			expect(mockExecuteFunctions.sendResponse).toHaveBeenCalledWith({
-				body: 'responseBody',
+				body: sandboxHtmlResponse('responseBody'),
 				headers: {},
 				statusCode: 200,
 			});
@@ -263,6 +263,74 @@ describe('RespondToWebhook Node', () => {
 			expect(mockExecuteFunctions.sendResponse).not.toHaveBeenCalled();
 		});
 
+		describe('HTML content sandboxing', () => {
+			it('should sandbox HTML content for json response with HTML content-type', async () => {
+				const inputItems = [
+					{ json: { index: 0, input: true } },
+					{ json: { index: 1, input: true } },
+				];
+				mockExecuteFunctions.getInputData.mockReturnValue(inputItems);
+				mockExecuteFunctions.getNode.mockReturnValue(mock<INode>({ typeVersion: 1.1 }));
+				mockExecuteFunctions.getParentNodes.mockReturnValue([
+					mock<NodeTypeAndVersion>({ type: WAIT_NODE_TYPE }),
+				]);
+				mockExecuteFunctions.getNodeParameter.mockImplementation((paramName) => {
+					if (paramName === 'respondWith') return 'allIncomingItems';
+					if (paramName === 'options')
+						return {
+							responseHeaders: {
+								entries: [{ name: 'content-type', value: 'application/xhtml+xml' }],
+							},
+						};
+				});
+				mockExecuteFunctions.sendResponse.mockReturnValue();
+
+				const result = await respondToWebhook.execute.call(mockExecuteFunctions);
+				expect(mockExecuteFunctions.sendResponse).toHaveBeenCalledWith({
+					body: sandboxHtmlResponse(JSON.stringify(inputItems.map((item) => item.json))),
+					headers: { 'content-type': 'application/xhtml+xml' },
+					statusCode: 200,
+				});
+				expect(result).toHaveLength(1);
+				expect(result[0]).toHaveLength(2);
+				expect(result[0]).toEqual(inputItems);
+			});
+
+			it('should NOT sandbox HTML content for non-HTML content-type', async () => {
+				const inputItems = [
+					{ json: { index: 0, input: true } },
+					{ json: { index: 1, input: true } },
+				];
+				mockExecuteFunctions.getInputData.mockReturnValue(inputItems);
+				mockExecuteFunctions.getNode.mockReturnValue(mock<INode>({ typeVersion: 1.1 }));
+				mockExecuteFunctions.getParentNodes.mockReturnValue([
+					mock<NodeTypeAndVersion>({ type: WAIT_NODE_TYPE }),
+				]);
+				mockExecuteFunctions.getNodeParameter.mockImplementation((paramName) => {
+					if (paramName === 'respondWith') return 'allIncomingItems';
+					if (paramName === 'options') return {};
+				});
+				mockExecuteFunctions.sendResponse.mockReturnValue();
+
+				const result = await respondToWebhook.execute.call(mockExecuteFunctions);
+				expect(mockExecuteFunctions.sendResponse).toHaveBeenCalledWith({
+					body: inputItems.map((item) => item.json),
+					headers: {},
+					statusCode: 200,
+				});
+				expect(result).toHaveLength(1);
+				expect(result[0]).toHaveLength(2);
+				expect(result[0]).toEqual(inputItems);
+
+				await expect(respondToWebhook.execute.call(mockExecuteFunctions)).resolves.not.toThrow();
+				expect(mockExecuteFunctions.sendResponse).toHaveBeenCalledWith({
+					body: inputItems.map((item) => item.json),
+					headers: {},
+					statusCode: 200,
+				});
+			});
+		});
+
 		it('should have two outputs in version 1.3', async () => {
 			const inputItems = [{ json: { index: 0, input: true } }, { json: { index: 1, input: true } }];
 			mockExecuteFunctions.getInputData.mockReturnValue(inputItems);

packages/nodes-base/nodes/RespondToWebhook/test/binary.test.ts

@@ -0,0 +1,65 @@
+import { sandboxHtmlResponse } from 'n8n-core';
+import type { IDataObject } from 'n8n-workflow';
+import { BINARY_ENCODING } from 'n8n-workflow';
+
+import { getBinaryResponse } from '../utils/binary';
+
+describe('getBinaryResponse', () => {
+	it('returns sanitized HTML when binaryData.id is present and mimeType is text/html', () => {
+		const binaryData = {
+			id: '123',
+			data: '<h1>Hello</h1>',
+			mimeType: 'text/html',
+		};
+		const headers: IDataObject = {};
+
+		const result = getBinaryResponse(binaryData, headers);
+
+		expect(result).toBe(sandboxHtmlResponse(binaryData.data));
+		expect(headers['content-type']).toBe('text/html');
+	});
+
+	it('returns { binaryData } when binaryData.id is present and mimeType is not text/html', () => {
+		const binaryData = {
+			id: '123',
+			data: 'some-binary-data',
+			mimeType: 'application/octet-stream',
+		};
+		const headers: IDataObject = {};
+
+		const result = getBinaryResponse(binaryData, headers);
+
+		expect(result).toEqual({ binaryData });
+		expect(headers['content-type']).toBe('application/octet-stream');
+	});
+
+	it('returns sanitized HTML when binaryData.id is not present and mimeType is text/html', () => {
+		const binaryData = {
+			data: '<h1>Hello</h1>',
+			mimeType: 'text/html',
+		};
+		const headers: IDataObject = {};
+
+		const result = getBinaryResponse(binaryData, headers);
+
+		expect(result).toBe(
+			sandboxHtmlResponse(Buffer.from(binaryData.data, BINARY_ENCODING).toString()),
+		);
+		expect(headers['content-type']).toBe('text/html');
+	});
+
+	it('returns Buffer when binaryData.id is not present and mimeType is not text/html', () => {
+		const binaryData = {
+			data: 'some-binary-data',
+			mimeType: 'application/octet-stream',
+		};
+		const headers: IDataObject = {};
+
+		const result = getBinaryResponse(binaryData, headers);
+
+		expect(Buffer.isBuffer(result)).toBe(true);
+		expect(result.toString()).toBe(Buffer.from(binaryData.data, BINARY_ENCODING).toString());
+		expect(headers['content-type']).toBe('application/octet-stream');
+		expect(headers['content-length']).toBe(Buffer.from(binaryData.data, BINARY_ENCODING).length);
+	});
+});

packages/nodes-base/nodes/RespondToWebhook/test/outputs.test.ts

@@ -1,4 +1,4 @@
-import { configuredOutputs } from '../utils';
+import { configuredOutputs } from '../utils/outputs';
 
 describe('configuredOutputs', () => {
 	it('returns array of objects when version >= 1.3', () => {

packages/nodes-base/nodes/RespondToWebhook/utils/binary.ts

@@ -0,0 +1,42 @@
+import { isHtmlRenderedContentType, sandboxHtmlResponse } from 'n8n-core';
+import type { IBinaryData, IDataObject, IN8nHttpResponse } from 'n8n-workflow';
+import { BINARY_ENCODING } from 'n8n-workflow';
+import type { Readable } from 'stream';
+
+const setContentLength = (responseBody: IN8nHttpResponse | Readable, headers: IDataObject) => {
+	if (Buffer.isBuffer(responseBody)) {
+		headers['content-length'] = responseBody.length;
+	} else if (typeof responseBody === 'string') {
+		headers['content-length'] = Buffer.byteLength(responseBody, 'utf8');
+	}
+};
+
+/**
+ * Returns a response body for a binary data and sets the content-type header.
+ */
+export const getBinaryResponse = (binaryData: IBinaryData, headers: IDataObject) => {
+	const contentType = headers['content-type'] as string;
+	const shouldSandboxResponseData =
+		isHtmlRenderedContentType(binaryData.mimeType) ||
+		(contentType && isHtmlRenderedContentType(contentType));
+
+	let responseBody: IN8nHttpResponse | Readable;
+
+	if (binaryData.id) {
+		responseBody = shouldSandboxResponseData
+			? sandboxHtmlResponse(binaryData.data)
+			: { binaryData };
+	} else {
+		const responseBuffer = Buffer.from(binaryData.data, BINARY_ENCODING);
+
+		responseBody = shouldSandboxResponseData
+			? sandboxHtmlResponse(responseBuffer.toString())
+			: responseBuffer;
+
+		setContentLength(responseBody, headers);
+	}
+
+	headers['content-type'] ??= binaryData.mimeType;
+
+	return responseBody;
+};