From 667656e8f3d6c23a79a4b80f2f6f330024115893 Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Thu, 21 Aug 2025 11:39:57 +0300 Subject: [PATCH] fix(core)!: Use CSP header to sandbox html webhooks instead of iframe (#18602) --- .../config/src/configs/security.config.ts | 7 +- packages/@n8n/config/test/config.test.ts | 2 +- packages/cli/BREAKING-CHANGES.md | 10 + .../src/webhooks/webhook-request-handler.ts | 35 +- .../__snapshots__/html-sandbox.test.ts.snap | 37 -- .../core/src/__tests__/html-sandbox.test.ts | 336 ++---------------- packages/core/src/html-sandbox.ts | 138 +------ .../RespondToWebhook/RespondToWebhook.node.ts | 21 +- .../test/RespondToWebhook.test.ts | 72 +--- .../RespondToWebhook/test/binary.test.ts | 12 +- .../nodes/RespondToWebhook/utils/binary.ts | 22 +- 11 files changed, 66 insertions(+), 626 deletions(-) delete mode 100644 packages/core/src/__tests__/__snapshots__/html-sandbox.test.ts.snap diff --git a/packages/@n8n/config/src/configs/security.config.ts b/packages/@n8n/config/src/configs/security.config.ts index ee42440300..843d0605cd 100644 --- a/packages/@n8n/config/src/configs/security.config.ts +++ b/packages/@n8n/config/src/configs/security.config.ts @@ -39,7 +39,10 @@ export class SecurityConfig { @Env('N8N_CONTENT_SECURITY_POLICY_REPORT_ONLY') contentSecurityPolicyReportOnly: boolean = false; - /** Whether to disable iframe sandboxing for webhooks */ + /** + * Whether to disable HTML sandboxing for webhooks. The sandboxing mechanism uses CSP headers now, + * but the name is kept for backwards compatibility. + */ @Env('N8N_INSECURE_DISABLE_WEBHOOK_IFRAME_SANDBOX') - disableIframeSandboxing: boolean = false; + disableWebhookHtmlSandboxing: boolean = false; } diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index b140c69676..28cc813a30 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -299,7 +299,7 @@ describe('GlobalConfig', () => { daysAbandonedWorkflow: 90, contentSecurityPolicy: '{}', contentSecurityPolicyReportOnly: false, - disableIframeSandboxing: false, + disableWebhookHtmlSandboxing: false, }, executions: { pruneData: true, diff --git a/packages/cli/BREAKING-CHANGES.md b/packages/cli/BREAKING-CHANGES.md index aa87cf17c1..253c46fb07 100644 --- a/packages/cli/BREAKING-CHANGES.md +++ b/packages/cli/BREAKING-CHANGES.md @@ -2,6 +2,16 @@ This list shows all the versions which include breaking changes and how to upgrade. +# 1.109.0 + +### What changed? + +Webhook HTML responses were sandboxed to an iframe starting from 1.103.1 due to security. The sandboxing mechanism is now changed to use `Content-Security-Policy` header instead of an `iframe`. The security guarantees stay the same, but the mechanism is less breaking. + +### When is action necessary? + +If you have workflows that return HTML responses from `Webhook Trigger` node or `Respond to Webhook` node. + # 1.107.0 ## What changed? diff --git a/packages/cli/src/webhooks/webhook-request-handler.ts b/packages/cli/src/webhooks/webhook-request-handler.ts index 97c692d388..e8073420ac 100644 --- a/packages/cli/src/webhooks/webhook-request-handler.ts +++ b/packages/cli/src/webhooks/webhook-request-handler.ts @@ -2,15 +2,13 @@ import { Logger } from '@n8n/backend-common'; import { Container } from '@n8n/di'; import type express from 'express'; import { + isWebhookHtmlSandboxingDisabled, + getWebhookSandboxCSP, isHtmlRenderedContentType, - sandboxHtmlResponse, - createHtmlSandboxTransformStream, } from 'n8n-core'; import { ensureError, type IHttpRequestMethods } from 'n8n-workflow'; import { finished } from 'stream/promises'; -import { WebhookService } from './webhook.service'; - import { WebhookNotFoundError } from '@/errors/response-errors/webhook-not-found.error'; import * as ResponseHelper from '@/response-helper'; import type { @@ -24,6 +22,7 @@ import { isWebhookResponse, isWebhookStreamResponse, } from '@/webhooks/webhook-response'; +import { WebhookService } from '@/webhooks/webhook.service'; import type { IWebhookManager, WebhookOptionsRequest, @@ -126,12 +125,8 @@ class WebhookRequestHandler { this.setResponseStatus(res, code); this.setResponseHeaders(res, headers); - const contentType = res.getHeader('content-type') as string | undefined; - const needsSandbox = contentType && isHtmlRenderedContentType(contentType); - - const streamToSend = needsSandbox ? stream.pipe(createHtmlSandboxTransformStream()) : stream; - streamToSend.pipe(res, { end: false }); - await finished(streamToSend); + stream.pipe(res, { end: false }); + await finished(stream); process.nextTick(() => res.end()); } @@ -142,19 +137,10 @@ class WebhookRequestHandler { this.setResponseStatus(res, code); this.setResponseHeaders(res, headers); - const contentType = res.getHeader('content-type') as string | undefined; - if (typeof body === 'string') { - const needsSandbox = !contentType || isHtmlRenderedContentType(contentType); - const bodyToSend = needsSandbox ? sandboxHtmlResponse(body) : body; - res.send(bodyToSend); + res.send(body); } else { - const needsSandbox = contentType && isHtmlRenderedContentType(contentType); - if (needsSandbox) { - res.send(sandboxHtmlResponse(body)); - } else { - res.json(body); - } + res.json(body); } } @@ -170,6 +156,13 @@ class WebhookRequestHandler { res.setHeader(name, value); } } + + const contentType = res.getHeader('content-type') as string | undefined; + const needsSandbox = !contentType || isHtmlRenderedContentType(contentType); + + if (needsSandbox && !isWebhookHtmlSandboxingDisabled()) { + res.setHeader('Content-Security-Policy', getWebhookSandboxCSP()); + } } private async setupCorsHeaders( diff --git a/packages/core/src/__tests__/__snapshots__/html-sandbox.test.ts.snap b/packages/core/src/__tests__/__snapshots__/html-sandbox.test.ts.snap deleted file mode 100644 index 2da8201f02..0000000000 --- a/packages/core/src/__tests__/__snapshots__/html-sandbox.test.ts.snap +++ /dev/null @@ -1,37 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`sandboxHtmlResponse should always sandbox if forceSandbox is true 1`] = ` -"" -`; - -exports[`sandboxHtmlResponse should handle HTML with special characters 1`] = ` -"" -`; - -exports[`sandboxHtmlResponse should replace ampersands and double quotes in HTML 1`] = ` -"" -`; - -exports[`sandboxHtmlResponse should sandbox even with no tag 1`] = ` -"" -`; - -exports[`sandboxHtmlResponse should sandbox when outside and tags 1`] = ` -"" -`; - -exports[`sandboxHtmlResponse should sandbox when outside tag 1`] = ` -"" -`; diff --git a/packages/core/src/__tests__/html-sandbox.test.ts b/packages/core/src/__tests__/html-sandbox.test.ts index 2194de2fbd..def14bb261 100644 --- a/packages/core/src/__tests__/html-sandbox.test.ts +++ b/packages/core/src/__tests__/html-sandbox.test.ts @@ -1,77 +1,31 @@ import type { SecurityConfig } from '@n8n/config'; import { Container } from '@n8n/di'; import { mock } from 'jest-mock-extended'; -import { Readable } from 'stream'; import { - bufferEscapeHtml, - createHtmlSandboxTransformStream, - hasHtml, + isWebhookHtmlSandboxingDisabled, + getWebhookSandboxCSP, isHtmlRenderedContentType, - sandboxHtmlResponse, -} from '../html-sandbox'; - -// Utility function to consume a stream into a buffer -async function consumeStreamToString(stream: NodeJS.ReadableStream): Promise { - const chunks: Buffer[] = []; - - return await new Promise((resolve, reject) => { - stream.on('data', (chunk: Buffer) => chunks.push(chunk)); - stream.on('end', () => resolve(Buffer.concat(chunks).toString())); - stream.on('error', reject); - }); -} +} from '@/html-sandbox'; const securityConfig = mock(); -describe('sandboxHtmlResponse', () => { +describe('isWebhookHtmlSandboxingDisabled', () => { beforeAll(() => { - securityConfig.disableIframeSandboxing = false; jest.spyOn(Container, 'get').mockReturnValue(securityConfig); }); afterAll(() => { jest.restoreAllMocks(); }); - it('should replace ampersands and double quotes in HTML', () => { - const html = '
Content & more
'; - expect(sandboxHtmlResponse(html)).toMatchSnapshot(); + + it('should return false when sandboxing is enabled', () => { + securityConfig.disableWebhookHtmlSandboxing = false; + expect(isWebhookHtmlSandboxingDisabled()).toBe(false); }); - it('should handle HTML with special characters', () => { - const html = '

Special characters: <>&"\'

'; - expect(sandboxHtmlResponse(html)).toMatchSnapshot(); - }); - - it.each([ - ['Hello World', 'Hello World'], - ['< not html >', '< not html >'], - ['# Test', '# Test'], - ['', ''], - [123, '123'], - [null, 'null'], - ])('should not sandbox if not html', (data, expected) => { - expect(sandboxHtmlResponse(data)).toBe(expected); - }); - - it('should sandbox even with no tag', () => { - const html = 'Test'; - expect(sandboxHtmlResponse(html)).toMatchSnapshot(); - }); - - it('should sandbox when outside and tags', () => { - const html = - 'Test'; - expect(sandboxHtmlResponse(html)).toMatchSnapshot(); - }); - - it('should sandbox when outside tag', () => { - const html = 'Test'; - expect(sandboxHtmlResponse(html)).toMatchSnapshot(); - }); - - it('should always sandbox if forceSandbox is true', () => { - const text = 'Hello World'; - expect(sandboxHtmlResponse(text, true)).toMatchSnapshot(); + it('should return true when sandboxing is disabled', () => { + securityConfig.disableWebhookHtmlSandboxing = true; + expect(isWebhookHtmlSandboxingDisabled()).toBe(true); }); }); @@ -129,266 +83,16 @@ describe('isHtmlRenderedContentType', () => { }); }); -describe('bufferEscapeHtml', () => { - it('should return the same buffer when no escaping is needed', () => { - const input = Buffer.from('Hello World', 'utf8'); - const result = bufferEscapeHtml(input); - - expect(result).toEqual(input); - expect(result.toString()).toBe('Hello World'); +describe('getWebhookSandboxCSP', () => { + it('should return correct CSP sandbox directive', () => { + const csp = getWebhookSandboxCSP(); + expect(csp).toBe( + 'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols', + ); }); - it('should handle empty buffer', () => { - const input = Buffer.alloc(0); - const result = bufferEscapeHtml(input); - - expect(result).toEqual(input); - expect(result.length).toBe(0); - }); - - describe('should escape special characters', () => { - test.each([ - ['&', '&'], - ['"', '"'], - ['&"', '&"'], - ['Hello & World', 'Hello & World'], - ['Hello "World"', 'Hello "World"'], - ['Hello & "World"', 'Hello & "World"'], - ['Hello && World', 'Hello && World'], - ['Hello ""World""', 'Hello ""World""'], - ['&"Hello"&"World"&', '&"Hello"&"World"&'], - ])('should escape %s to %s', (input, expected) => { - const buffer = Buffer.from(input, 'utf8'); - const result = bufferEscapeHtml(buffer); - expect(result.toString()).toBe(expected); - }); - }); - - it('should handle unicode characters with special characters', () => { - const input = Buffer.from('Hello & 世界 "World" & こんにちは', 'utf8'); - const result = bufferEscapeHtml(input); - - expect(result.toString()).toBe('Hello & 世界 "World" & こんにちは'); - }); - - it('should not modify other special characters', () => { - const input = Buffer.from('Hello & "Test"', 'utf8'); - const result = bufferEscapeHtml(input); - - expect(result.toString()).toBe('Hello & "Test"'); - expect(result.toString()).toContain('<'); - expect(result.toString()).toContain('>'); - }); -}); - -describe('createHtmlSandboxTransformStream', () => { - const getComparableHtml = (input: Buffer | string) => - sandboxHtmlResponse(input.toString(), true).replace(/\s+/g, ' '); - - it('should wrap single chunk in iframe with proper escaping', async () => { - const input = Buffer.from('Hello & "World"', 'utf8'); - const transform = createHtmlSandboxTransformStream(); - const readable = new Readable(); - readable.push(input); - readable.push(null); - - const result = await consumeStreamToString(readable.pipe(transform)); - - expect(result).toEqual(getComparableHtml(input)); - }); - - it('should handle multiple chunks correctly', async () => { - const transform = createHtmlSandboxTransformStream(); - const readable = new Readable(); - const inputChunks = ['Hello & ', '"World"', ' & Test']; - - for (const chunk of inputChunks) { - readable.push(Buffer.from(chunk, 'utf8')); - } - readable.push(null); - - const result = await consumeStreamToString(readable.pipe(transform)); - - expect(result).toEqual(getComparableHtml(inputChunks.join(''))); - }); - - it('should handle empty input', async () => { - const transform = createHtmlSandboxTransformStream(); - const readable = new Readable(); - readable.push(null); - - const result = await consumeStreamToString(readable.pipe(transform)); - - expect(result).toEqual(getComparableHtml('')); - }); - - it('should handle empty chunks', async () => { - const transform = createHtmlSandboxTransformStream(); - const readable = new Readable(); - - readable.push(Buffer.alloc(0)); - readable.push(Buffer.from('Hello', 'utf8')); - readable.push(Buffer.alloc(0)); - readable.push(null); - - const result = await consumeStreamToString(readable.pipe(transform)); - - expect(result).toEqual(getComparableHtml('Hello')); - }); - - it('should handle string chunks by converting to buffer', async () => { - const transform = createHtmlSandboxTransformStream(); - const readable = new Readable(); - readable.push('Hello & "World"'); - readable.push(null); - - const result = await consumeStreamToString(readable.pipe(transform)); - - expect(result).toEqual(getComparableHtml('Hello & "World"')); - }); - - it('should handle unicode characters correctly', async () => { - const input = Buffer.from('Hello & 世界 "World" & こんにちは', 'utf8'); - const transform = createHtmlSandboxTransformStream(); - const readable = new Readable(); - readable.push(input); - readable.push(null); - - const result = await consumeStreamToString(readable.pipe(transform)); - - expect(result).toEqual(getComparableHtml(input)); - }); - - it('should handle large content in chunks', async () => { - const baseString = 'Hello & World "Test" & Another "Quote"'; - const largeContent = baseString.repeat(100); - const transform = createHtmlSandboxTransformStream(); - const readable = new Readable(); - - // Split into chunks - const chunkSize = 1000; - for (let i = 0; i < largeContent.length; i += chunkSize) { - const chunk = largeContent.slice(i, i + chunkSize); - readable.push(Buffer.from(chunk, 'utf8')); - } - readable.push(null); - - const result = await consumeStreamToString(readable.pipe(transform)); - - expect(result).toEqual(getComparableHtml(largeContent)); - }); - - it('should handle special HTML characters', async () => { - const input = Buffer.from('
&"Hello"
', 'utf8'); - const transform = createHtmlSandboxTransformStream(); - const readable = new Readable(); - readable.push(input); - readable.push(null); - - const result = await consumeStreamToString(readable.pipe(transform)); - - expect(result).toEqual(getComparableHtml(input)); - }); - - it('should handle mixed content types', async () => { - const transform = createHtmlSandboxTransformStream(); - const readable = new Readable(); - - readable.push(Buffer.from('Hello', 'utf8')); - readable.push(' & World'); - readable.push(Buffer.from(' "Test"', 'utf8')); - readable.push(null); - - const result = await consumeStreamToString(readable.pipe(transform)); - - expect(result).toEqual(getComparableHtml('Hello & World "Test"')); - }); - - it('should produce valid HTML structure', async () => { - const input = Buffer.from('

Hello & "World"

', 'utf8'); - const transform = createHtmlSandboxTransformStream(); - const readable = new Readable(); - readable.push(input); - readable.push(null); - - const result = await consumeStreamToString(readable.pipe(transform)); - - expect(result).toEqual(getComparableHtml(input)); - }); -}); - -describe('sandboxHtmlResponse > not string types', () => { - beforeAll(() => { - securityConfig.disableIframeSandboxing = false; - jest.spyOn(Container, 'get').mockReturnValue(securityConfig); - }); - afterAll(() => { - jest.restoreAllMocks(); - }); - it('should not throw if data is number', () => { - const data = 123; - expect(() => sandboxHtmlResponse(data)).not.toThrow(); - }); - - it('should not throw if data is object', () => { - const data = {}; - expect(() => sandboxHtmlResponse(data)).not.toThrow(); - }); - - it('should not throw if data is boolean', () => { - const data = true; - expect(() => sandboxHtmlResponse(data)).not.toThrow(); - }); -}); - -describe('sandboxHtmlResponse > sandboxing disabled', () => { - beforeAll(() => { - securityConfig.disableIframeSandboxing = true; - jest.spyOn(Container, 'get').mockReturnValue(securityConfig); - }); - afterAll(() => { - jest.restoreAllMocks(); - }); - it('should return unchanged number data', () => { - const data = 123; - expect(sandboxHtmlResponse(data)).toEqual(data); - }); - - it('should return unchanged object data', () => { - const data = {}; - expect(sandboxHtmlResponse(data)).toEqual(data); - }); - - it('should return unchanged boolean data', () => { - const data = true; - expect(sandboxHtmlResponse(data)).toEqual(data); - }); - - it('should return unchanged text data', () => { - const data = 'string data'; - expect(sandboxHtmlResponse(data)).toEqual(data); - }); - - it('should return unchanged html data', () => { - const data = '

html data

'; - expect(sandboxHtmlResponse(data)).toEqual(data); - }); -}); - -describe('hasHtml', () => { - test('returns true for valid HTML', () => { - expect(hasHtml('

Hello

')).toBe(true); - }); - - test('returns true for malformed but still HTML-like content', () => { - expect(hasHtml('
Test')).toBe(true); - }); - - test('returns false for plain text', () => { - expect(hasHtml('Just a string')).toBe(false); - }); - - test('returns false for empty string', () => { - expect(hasHtml('')).toBe(false); + it('should not include allow-same-origin', () => { + const csp = getWebhookSandboxCSP(); + expect(csp).not.toContain('allow-same-origin'); }); }); diff --git a/packages/core/src/html-sandbox.ts b/packages/core/src/html-sandbox.ts index 5b8402558e..8c52e8692d 100644 --- a/packages/core/src/html-sandbox.ts +++ b/packages/core/src/html-sandbox.ts @@ -1,143 +1,15 @@ import { SecurityConfig } from '@n8n/config'; import { Container } from '@n8n/di'; -import { ElementType, parseDocument } from 'htmlparser2'; -import type { TransformCallback } from 'stream'; -import { Transform } from 'stream'; -export const isIframeSandboxDisabled = () => { - return Container.get(SecurityConfig).disableIframeSandboxing; +export const isWebhookHtmlSandboxingDisabled = () => { + return Container.get(SecurityConfig).disableWebhookHtmlSandboxing; }; /** - * Checks if the given string contains HTML. + * Returns the CSP header value that sandboxes the HTML page into a separate origin. */ -export const hasHtml = (str: string) => { - try { - const doc = parseDocument(str); - return doc.children.some((node) => node.type === ElementType.Tag); - } catch { - return false; - } -}; - -/** - * Sandboxes the HTML response to prevent possible exploitation, if the data has HTML. - * If the data does not have HTML, it will be returned as is. - * Otherwise, it embeds the response in an iframe to make sure the HTML has a different origin. - * Env var `N8N_INSECURE_DISABLE_WEBHOOK_IFRAME_SANDBOX` can be used, in this case sandboxing is disabled. - * - * @param data - The data to sandbox. - * @param forceSandbox - Whether to force sandboxing even if the data does not contain HTML. - * @returns The sandboxed HTML response. - */ -export const sandboxHtmlResponse = (data: T, forceSandbox = false) => { - if (isIframeSandboxDisabled()) return data; - - let text; - if (typeof data !== 'string') { - text = JSON.stringify(data); - } else { - text = data; - } - - if (!forceSandbox && !hasHtml(text)) return text; - - // Escape & and " as mentioned in the spec: - // https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element - const escapedHtml = text.replaceAll('&', '&').replaceAll('"', '"'); - - return ``; -}; - -/** - * Converts ampersands and double quotes in a buffer to their HTML entities. - * Does double pass on the buffer to avoid multiple allocations. - * - * @example - * ```ts - * const input = Buffer.from('Hello & "World"', 'utf8'); - * const result = bufferEscapeHtml(input); - * console.log(result.toString()); // 'Hello & "World"' - * ``` - */ -export const bufferEscapeHtml = (input: Buffer) => { - const ampersand = Buffer.from('&', 'utf8').readUInt8(0); - const escapedAmpersand = Buffer.from('&', 'utf8'); - const doublequote = Buffer.from('"', 'utf8').readUInt8(0); - const escapedDoublequote = Buffer.from('"', 'utf8'); - - let ampersandCount = 0; - let doublequoteCount = 0; - - for (let i = 0; i < input.length; i++) { - if (input[i] === ampersand) ampersandCount++; - else if (input[i] === doublequote) doublequoteCount++; - } - - if (ampersandCount === 0 && doublequoteCount === 0) return Buffer.from(input); - - const resultLength = - input.length + - ampersandCount * (escapedAmpersand.length - 1) + - doublequoteCount * (escapedDoublequote.length - 1); - const output = Buffer.alloc(resultLength); - let writeOffset = 0; - - for (let i = 0; i < input.length; i++) { - if (input[i] === ampersand) { - escapedAmpersand.copy(output, writeOffset); - writeOffset += escapedAmpersand.length; - } else if (input[i] === doublequote) { - escapedDoublequote.copy(output, writeOffset); - writeOffset += escapedDoublequote.length; - } else { - output[writeOffset++] = input[i]; - } - } - - return output; -}; - -/** - * Creates a transform stream that sandboxes HTML content by wrapping it in an iframe. - * This is the streaming equivalent of sandboxHtmlResponse. - */ -export const createHtmlSandboxTransformStream = () => { - let isFirstChunk = true; - - const prefix = Buffer.from('', - 'utf8', - ); - - return new Transform({ - transform(chunk: Buffer, encoding: string, done: TransformCallback) { - try { - chunk = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk, encoding as BufferEncoding); - const escapedChunk = bufferEscapeHtml(chunk); - const transformedChunk = isFirstChunk - ? Buffer.concat([prefix, escapedChunk]) - : escapedChunk; - isFirstChunk = false; - - done(null, transformedChunk); - } catch (error) { - done(error as Error); - } - }, - - flush(done: TransformCallback) { - try { - this.push(isFirstChunk ? Buffer.concat([prefix, suffix]) : suffix); - done(); - } catch (error) { - done(error as Error); - } - }, - }); +export const getWebhookSandboxCSP = (): string => { + return 'sandbox allow-downloads allow-forms allow-modals allow-orientation-lock allow-pointer-lock allow-popups allow-presentation allow-scripts allow-top-navigation allow-top-navigation-by-user-activation allow-top-navigation-to-custom-protocols'; }; /** diff --git a/packages/nodes-base/nodes/RespondToWebhook/RespondToWebhook.node.ts b/packages/nodes-base/nodes/RespondToWebhook/RespondToWebhook.node.ts index 74411fdef9..db122978d1 100644 --- a/packages/nodes-base/nodes/RespondToWebhook/RespondToWebhook.node.ts +++ b/packages/nodes-base/nodes/RespondToWebhook/RespondToWebhook.node.ts @@ -1,6 +1,5 @@ import jwt from 'jsonwebtoken'; import set from 'lodash/set'; -import { isHtmlRenderedContentType, sandboxHtmlResponse } from 'n8n-core'; import type { IDataObject, IExecuteFunctions, @@ -402,9 +401,6 @@ 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') { @@ -480,13 +476,9 @@ export class RespondToWebhook implements INodeType { this.sendChunk('end', 0); } } else if (respondWith === 'text') { - // If a user doesn't set the content-type header and uses html, the html can still be rendered on the browser const rawBody = this.getNodeParameter('responseBody', 0) as string; - if (hasHtmlContentType || !headers['content-type']) { - responseBody = sandboxHtmlResponse(rawBody); - } else { - responseBody = rawBody; - } + responseBody = rawBody; + // Send the raw body to the stream if (shouldStream) { this.sendChunk('begin', 0); @@ -564,15 +556,6 @@ export class RespondToWebhook implements INodeType { return [[{ json: {}, sendMessage: message }]]; } - if ( - hasHtmlContentType && - respondWith !== 'text' && - respondWith !== 'binary' && - responseBody - ) { - responseBody = sandboxHtmlResponse(JSON.stringify(responseBody as string)); - } - response = { body: responseBody, headers, diff --git a/packages/nodes-base/nodes/RespondToWebhook/test/RespondToWebhook.test.ts b/packages/nodes-base/nodes/RespondToWebhook/test/RespondToWebhook.test.ts index 808ccd6e20..356c9f1c41 100644 --- a/packages/nodes-base/nodes/RespondToWebhook/test/RespondToWebhook.test.ts +++ b/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, sandboxHtmlResponse } from 'n8n-core'; +import { constructExecutionMetaData } from 'n8n-core'; import { BINARY_ENCODING, WAIT_NODE_TYPE, @@ -236,7 +236,7 @@ describe('RespondToWebhook Node', () => { await expect(respondToWebhook.execute.call(mockExecuteFunctions)).resolves.not.toThrow(); expect(mockExecuteFunctions.sendResponse).toHaveBeenCalledWith({ - body: sandboxHtmlResponse('responseBody'), + body: 'responseBody', headers: {}, statusCode: 200, }); @@ -336,74 +336,6 @@ 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({ typeVersion: 1.1 })); - mockExecuteFunctions.getParentNodes.mockReturnValue([ - mock({ 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({ typeVersion: 1.1 })); - mockExecuteFunctions.getParentNodes.mockReturnValue([ - mock({ 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); diff --git a/packages/nodes-base/nodes/RespondToWebhook/test/binary.test.ts b/packages/nodes-base/nodes/RespondToWebhook/test/binary.test.ts index a770fae19c..64f2a98965 100644 --- a/packages/nodes-base/nodes/RespondToWebhook/test/binary.test.ts +++ b/packages/nodes-base/nodes/RespondToWebhook/test/binary.test.ts @@ -1,11 +1,10 @@ -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', () => { + it('returns { binaryData } when binaryData.id is present', () => { const binaryData = { id: '123', data: '

Hello

', @@ -15,7 +14,7 @@ describe('getBinaryResponse', () => { const result = getBinaryResponse(binaryData, headers); - expect(result).toBe(sandboxHtmlResponse(binaryData.data)); + expect(result).toEqual({ binaryData }); expect(headers['content-type']).toBe('text/html'); }); @@ -33,7 +32,7 @@ describe('getBinaryResponse', () => { expect(headers['content-type']).toBe('application/octet-stream'); }); - it('returns sanitized HTML when binaryData.id is not present and mimeType is text/html', () => { + it('returns Buffer when binaryData.id is not present', () => { const binaryData = { data: '

Hello

', mimeType: 'text/html', @@ -42,9 +41,8 @@ describe('getBinaryResponse', () => { const result = getBinaryResponse(binaryData, headers); - expect(result).toBe( - sandboxHtmlResponse(Buffer.from(binaryData.data, BINARY_ENCODING).toString()), - ); + expect(Buffer.isBuffer(result)).toBe(true); + expect(result.toString()).toBe(Buffer.from(binaryData.data, BINARY_ENCODING).toString()); expect(headers['content-type']).toBe('text/html'); }); diff --git a/packages/nodes-base/nodes/RespondToWebhook/utils/binary.ts b/packages/nodes-base/nodes/RespondToWebhook/utils/binary.ts index 97ba3c02a8..a0acabbd49 100644 --- a/packages/nodes-base/nodes/RespondToWebhook/utils/binary.ts +++ b/packages/nodes-base/nodes/RespondToWebhook/utils/binary.ts @@ -1,4 +1,3 @@ -import { isHtmlRenderedContentType, sandboxHtmlResponse, isIframeSandboxDisabled } from 'n8n-core'; import type { IBinaryData, IDataObject, IN8nHttpResponse } from 'n8n-workflow'; import { BINARY_ENCODING } from 'n8n-workflow'; import type { Readable } from 'stream'; @@ -15,30 +14,13 @@ const setContentLength = (responseBody: IN8nHttpResponse | Readable, headers: ID * 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; - - let shouldSandboxResponseData; - if (isIframeSandboxDisabled()) { - shouldSandboxResponseData = false; - } else { - shouldSandboxResponseData = - isHtmlRenderedContentType(binaryData.mimeType) || - (contentType && isHtmlRenderedContentType(contentType)); - } - let responseBody: IN8nHttpResponse | Readable; if (binaryData.id) { - responseBody = shouldSandboxResponseData - ? sandboxHtmlResponse(binaryData.data) - : { binaryData }; + responseBody = { binaryData }; } else { const responseBuffer = Buffer.from(binaryData.data, BINARY_ENCODING); - - responseBody = shouldSandboxResponseData - ? sandboxHtmlResponse(responseBuffer.toString()) - : responseBuffer; - + responseBody = responseBuffer; setContentLength(responseBody, headers); }