diff --git a/CHANGELOG.md b/CHANGELOG.md index e64ab64a45719..467ad3db0b619 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## [1.103.1](https://github.com/n8n-io/n8n/compare/n8n@1.103.0...n8n@1.103.1) (2025-07-16) + + +### Bug Fixes + +* Fix issue with restricted file access order ([#17329](https://github.com/n8n-io/n8n/issues/17329)) ([0aebdb1](https://github.com/n8n-io/n8n/commit/0aebdb14ed2d92db3ce9419e4c92438fb3042165)) +* **Webhook Trigger Node:** Change html responses to be embedded an iframe ([#17312](https://github.com/n8n-io/n8n/issues/17312)) ([77535e6](https://github.com/n8n-io/n8n/commit/77535e68d49dedc65b01ab5c1265af54fd1fc7b6)) + + + # [1.103.0](https://github.com/n8n-io/n8n/compare/n8n@1.102.0...n8n@1.103.0) (2025-07-14) diff --git a/package.json b/package.json index 0cb468602a211..3b243cfb46a35 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "n8n-monorepo", - "version": "1.103.0", + "version": "1.103.1", "private": true, "engines": { "node": ">=22.16", diff --git a/packages/@n8n/db/package.json b/packages/@n8n/db/package.json index 298218c8b7af7..d0ee7d802a92f 100644 --- a/packages/@n8n/db/package.json +++ b/packages/@n8n/db/package.json @@ -1,6 +1,6 @@ { "name": "@n8n/db", - "version": "0.14.0", + "version": "0.14.1", "scripts": { "clean": "rimraf dist .turbo", "dev": "pnpm watch", diff --git a/packages/@n8n/task-runner/package.json b/packages/@n8n/task-runner/package.json index 7d2af5f3591d6..655c8c665005b 100644 --- a/packages/@n8n/task-runner/package.json +++ b/packages/@n8n/task-runner/package.json @@ -1,6 +1,6 @@ { "name": "@n8n/task-runner", - "version": "1.39.0", + "version": "1.39.1", "scripts": { "clean": "rimraf dist .turbo", "start": "node dist/start.js", diff --git a/packages/cli/BREAKING-CHANGES.md b/packages/cli/BREAKING-CHANGES.md index 5af3513422ff2..dc0fc37dab427 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.103.0 + +### What changed? + +We will no longer be allowing users to use `responseData` within the Webhook node since this is now sandboxed in an iframe, which may break workflows relying on browser APIs like `localStorage` and `fetch` from within custom code. + +### When is action necessary? + +If your workflow is using the Webhook node and uses JavaScript in `responseData` to make `fetch` calls or access `localStorage`, you may need to refactor it due to the new iframe sandboxing. + ## 1.102.0 ### What changed? @@ -12,8 +22,8 @@ with libraries like `puppeteer` at the cost of security. ### When is action necessary? -If you are using the `N8N_RUNNERS_ALLOW_PROTOTYPE_MUTATION` flag, or if you find that the task runner does not -currently support an external module that you rely on, then consider setting `N8N_RUNNERS_INSECURE_MODE=true`, +If you are using the `N8N_RUNNERS_ALLOW_PROTOTYPE_MUTATION` flag, or if you find that the task runner does not +currently support an external module that you rely on, then consider setting `N8N_RUNNERS_INSECURE_MODE=true`, at your own risk. ## 1.98.0 diff --git a/packages/cli/package.json b/packages/cli/package.json index 01d7ccb06cafa..c53bb98935f56 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "n8n", - "version": "1.103.0", + "version": "1.103.1", "description": "n8n Workflow Automation Tool", "main": "dist/index", "types": "dist/index.d.ts", diff --git a/packages/cli/src/webhooks/__tests__/webhook-helpers.test.ts b/packages/cli/src/webhooks/__tests__/webhook-helpers.test.ts index 72f0305ee5b18..268ee29cb6b5a 100644 --- a/packages/cli/src/webhooks/__tests__/webhook-helpers.test.ts +++ b/packages/cli/src/webhooks/__tests__/webhook-helpers.test.ts @@ -21,7 +21,6 @@ import { finished } from 'stream/promises'; import { autoDetectResponseMode, handleFormRedirectionCase, - getResponseOnReceived, setupResponseNodePromise, prepareExecutionData, } from '../webhook-helpers'; @@ -140,49 +139,6 @@ describe('handleFormRedirectionCase', () => { }); }); -describe('getResponseOnReceived', () => { - const responseCode = 200; - const webhookResultData = mock(); - - beforeEach(() => { - jest.resetAllMocks(); - }); - - test('should return response with no data when responseData is "noData"', () => { - const callbackData = getResponseOnReceived('noData', webhookResultData, responseCode); - - expect(callbackData).toEqual({ responseCode }); - }); - - test('should return response with responseData when it is defined', () => { - const responseData = JSON.stringify({ foo: 'bar' }); - - const callbackData = getResponseOnReceived(responseData, webhookResultData, responseCode); - - expect(callbackData).toEqual({ data: responseData, responseCode }); - }); - - test('should return response with webhookResponse when responseData is falsy but webhookResponse exists', () => { - const webhookResponse = { success: true }; - webhookResultData.webhookResponse = webhookResponse; - - const callbackData = getResponseOnReceived(undefined, webhookResultData, responseCode); - - expect(callbackData).toEqual({ data: webhookResponse, responseCode }); - }); - - test('should return default response message when responseData and webhookResponse are falsy', () => { - webhookResultData.webhookResponse = undefined; - - const callbackData = getResponseOnReceived(undefined, webhookResultData, responseCode); - - expect(callbackData).toEqual({ - data: { message: 'Workflow was started' }, - responseCode, - }); - }); -}); - describe('setupResponseNodePromise', () => { const workflowId = 'test-workflow-id'; const executionId = 'test-execution-id'; diff --git a/packages/cli/src/webhooks/__tests__/webhook-last-node-response-extractor.test.ts b/packages/cli/src/webhooks/__tests__/webhook-last-node-response-extractor.test.ts new file mode 100644 index 0000000000000..83da033e47677 --- /dev/null +++ b/packages/cli/src/webhooks/__tests__/webhook-last-node-response-extractor.test.ts @@ -0,0 +1,346 @@ +import { Container } from '@n8n/di'; +import { mock, type MockProxy } from 'jest-mock-extended'; +import { BinaryDataService } from 'n8n-core'; +import type { ITaskData, INodeExecutionData, IBinaryData } from 'n8n-workflow'; +import { BINARY_ENCODING, OperationalError } from 'n8n-workflow'; +import assert from 'node:assert'; +import { Readable } from 'node:stream'; + +import { extractWebhookLastNodeResponse } from '../webhook-last-node-response-extractor'; + +import type { WebhookExecutionContext } from '@/webhooks/webhook-execution-context'; + +describe('extractWebhookLastNodeResponse', () => { + let context: MockProxy; + let lastNodeTaskData: MockProxy; + let binaryDataService: MockProxy; + + beforeEach(() => { + context = mock(); + lastNodeTaskData = mock(); + binaryDataService = mock(); + + Container.set(BinaryDataService, binaryDataService); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('responseDataType: firstEntryJson', () => { + it('should return first entry JSON data', async () => { + const jsonData = { foo: 'bar', test: 123 }; + lastNodeTaskData.data = { + main: [[{ json: jsonData }]], + }; + + const result = await extractWebhookLastNodeResponse( + context, + 'firstEntryJson', + lastNodeTaskData, + ); + + expect(result).toEqual({ + ok: true, + result: { + type: 'static', + body: jsonData, + contentType: undefined, + }, + }); + }); + + it('should return error when no item to return', async () => { + lastNodeTaskData.data = { + main: [[]], + }; + + const result = await extractWebhookLastNodeResponse( + context, + 'firstEntryJson', + lastNodeTaskData, + ); + + assert(!result.ok); + expect(result.error).toBeInstanceOf(OperationalError); + }); + + it('should extract specific property when responsePropertyName is set', async () => { + const jsonData = { foo: 'bar', nested: { value: 'test' } }; + lastNodeTaskData.data = { + main: [[{ json: jsonData }]], + }; + + context.evaluateSimpleWebhookDescriptionExpression + .mockReturnValueOnce('nested.value') + .mockReturnValueOnce(undefined); + + const result = await extractWebhookLastNodeResponse( + context, + 'firstEntryJson', + lastNodeTaskData, + ); + + expect(result).toEqual({ + ok: true, + result: { + type: 'static', + body: 'test', + contentType: undefined, + }, + }); + }); + + it('should set content type when responseContentType is provided', async () => { + const jsonData = { foo: 'bar' }; + lastNodeTaskData.data = { + main: [[{ json: jsonData }]], + }; + + context.evaluateSimpleWebhookDescriptionExpression + .mockReturnValueOnce(undefined) + .mockReturnValueOnce('application/xml'); + + const result = await extractWebhookLastNodeResponse( + context, + 'firstEntryJson', + lastNodeTaskData, + ); + + expect(result).toEqual({ + ok: true, + result: { + type: 'static', + body: jsonData, + contentType: 'application/xml', + }, + }); + }); + }); + + describe('responseDataType: firstEntryBinary', () => { + it('should return binary data as buffer when no ID is present', async () => { + const binaryData: IBinaryData = { + data: Buffer.from('test binary data').toString(BINARY_ENCODING), + mimeType: 'text/plain', + }; + const nodeExecutionData: INodeExecutionData = { + json: {}, + binary: { data: binaryData }, + }; + lastNodeTaskData.data = { + main: [[nodeExecutionData]], + }; + + context.evaluateSimpleWebhookDescriptionExpression.mockReturnValue('data'); + + const result = await extractWebhookLastNodeResponse( + context, + 'firstEntryBinary', + lastNodeTaskData, + ); + + expect(result).toEqual({ + ok: true, + result: { + type: 'static', + body: Buffer.from('test binary data'), + contentType: 'text/plain', + }, + }); + }); + + it('should return binary data as stream when ID is present', async () => { + const mockStream = new Readable(); + binaryDataService.getAsStream.mockResolvedValue(mockStream); + + const binaryData: IBinaryData = { + id: 'binary-123', + mimeType: 'image/jpeg', + data: '', + }; + const nodeExecutionData: INodeExecutionData = { + json: {}, + binary: { data: binaryData }, + }; + lastNodeTaskData.data = { + main: [[nodeExecutionData]], + }; + + context.evaluateSimpleWebhookDescriptionExpression.mockReturnValue('data'); + + const result = await extractWebhookLastNodeResponse( + context, + 'firstEntryBinary', + lastNodeTaskData, + ); + + expect(result).toEqual({ + ok: true, + result: { + type: 'stream', + stream: mockStream, + contentType: 'image/jpeg', + }, + }); + assert(binaryDataService.getAsStream.mock.calls[0][0] === 'binary-123'); + }); + + it('should return error when no item found', async () => { + lastNodeTaskData.data = { + main: [[]], + }; + + const result = await extractWebhookLastNodeResponse( + context, + 'firstEntryBinary', + lastNodeTaskData, + ); + + assert(!result.ok); + expect(result.error).toBeInstanceOf(OperationalError); + expect(result.error.message).toBe('No item was found to return'); + }); + + it('should return error when no binary data found', async () => { + const nodeExecutionData: INodeExecutionData = { + json: { foo: 'bar' }, + }; + lastNodeTaskData.data = { + main: [[nodeExecutionData]], + }; + + const result = await extractWebhookLastNodeResponse( + context, + 'firstEntryBinary', + lastNodeTaskData, + ); + + assert(!result.ok); + expect(result.error).toBeInstanceOf(OperationalError); + expect(result.error.message).toBe('No binary data was found to return'); + }); + + it('should return error when responseBinaryPropertyName is undefined', async () => { + const nodeExecutionData: INodeExecutionData = { + json: {}, + binary: { data: { data: 'test', mimeType: 'text/plain' } }, + }; + lastNodeTaskData.data = { + main: [[nodeExecutionData]], + }; + + context.evaluateSimpleWebhookDescriptionExpression.mockReturnValue(undefined); + + const result = await extractWebhookLastNodeResponse( + context, + 'firstEntryBinary', + lastNodeTaskData, + ); + + assert(!result.ok); + expect(result.error).toBeInstanceOf(OperationalError); + expect(result.error.message).toBe("No 'responseBinaryPropertyName' is set"); + }); + + it('should return error when responseBinaryPropertyName is not a string', async () => { + const nodeExecutionData: INodeExecutionData = { + json: {}, + binary: { data: { data: 'test', mimeType: 'text/plain' } }, + }; + lastNodeTaskData.data = { + main: [[nodeExecutionData]], + }; + + context.evaluateSimpleWebhookDescriptionExpression.mockReturnValue(123); + + const result = await extractWebhookLastNodeResponse( + context, + 'firstEntryBinary', + lastNodeTaskData, + ); + + assert(!result.ok); + expect(result.error).toBeInstanceOf(OperationalError); + expect(result.error.message).toBe("'responseBinaryPropertyName' is not a string"); + }); + + it('should return error when specified binary property does not exist', async () => { + const nodeExecutionData: INodeExecutionData = { + json: {}, + binary: { otherProperty: { data: 'test', mimeType: 'text/plain' } }, + }; + lastNodeTaskData.data = { + main: [[nodeExecutionData]], + }; + + context.evaluateSimpleWebhookDescriptionExpression.mockReturnValue('nonExistentProperty'); + + const result = await extractWebhookLastNodeResponse( + context, + 'firstEntryBinary', + lastNodeTaskData, + ); + + assert(!result.ok); + expect(result.error).toBeInstanceOf(OperationalError); + expect(result.error.message).toBe( + "The binary property 'nonExistentProperty' which should be returned does not exist", + ); + }); + }); + + describe('responseDataType: noData', () => { + it('should return undefined body and contentType', async () => { + const result = await extractWebhookLastNodeResponse(context, 'noData', lastNodeTaskData); + + expect(result).toEqual({ + ok: true, + result: { + type: 'static', + body: undefined, + contentType: undefined, + }, + }); + }); + }); + + describe('responseDataType: default (allEntries)', () => { + it('should return all entries as JSON array', async () => { + const jsonData1 = { foo: 'bar' }; + const jsonData2 = { test: 123 }; + const jsonData3 = { nested: { value: 'test' } }; + lastNodeTaskData.data = { + main: [[{ json: jsonData1 }, { json: jsonData2 }, { json: jsonData3 }]], + }; + + const result = await extractWebhookLastNodeResponse(context, 'allEntries', lastNodeTaskData); + + expect(result).toEqual({ + ok: true, + result: { + type: 'static', + body: [jsonData1, jsonData2, jsonData3], + contentType: undefined, + }, + }); + }); + + it('should return empty array when no entries', async () => { + lastNodeTaskData.data = { + main: [[]], + }; + + const result = await extractWebhookLastNodeResponse(context, 'allEntries', lastNodeTaskData); + + expect(result).toEqual({ + ok: true, + result: { + type: 'static', + body: [], + contentType: undefined, + }, + }); + }); + }); +}); diff --git a/packages/cli/src/webhooks/__tests__/webhook-on-received-response-extractor.test.ts b/packages/cli/src/webhooks/__tests__/webhook-on-received-response-extractor.test.ts new file mode 100644 index 0000000000000..d82d502000409 --- /dev/null +++ b/packages/cli/src/webhooks/__tests__/webhook-on-received-response-extractor.test.ts @@ -0,0 +1,43 @@ +import { mock } from 'jest-mock-extended'; +import type { IWebhookResponseData } from 'n8n-workflow'; + +import { extractWebhookOnReceivedResponse } from '@/webhooks/webhook-on-received-response-extractor'; + +describe('extractWebhookOnReceivedResponse', () => { + const webhookResultData = mock(); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + test('should return response with no data when responseData is "noData"', () => { + const callbackData = extractWebhookOnReceivedResponse('noData', webhookResultData); + + expect(callbackData).toEqual(undefined); + }); + + test('should return response with responseData when it is defined', () => { + const responseData = JSON.stringify({ foo: 'bar' }); + + const callbackData = extractWebhookOnReceivedResponse(responseData, webhookResultData); + + expect(callbackData).toEqual(responseData); + }); + + test('should return response with webhookResponse when responseData is falsy but webhookResponse exists', () => { + const webhookResponse = { success: true }; + webhookResultData.webhookResponse = webhookResponse; + + const callbackData = extractWebhookOnReceivedResponse(undefined, webhookResultData); + + expect(callbackData).toEqual(webhookResponse); + }); + + test('should return default response message when responseData and webhookResponse are falsy', () => { + webhookResultData.webhookResponse = undefined; + + const callbackData = extractWebhookOnReceivedResponse(undefined, webhookResultData); + + expect(callbackData).toEqual({ message: 'Workflow was started' }); + }); +}); diff --git a/packages/cli/src/webhooks/webhook-execution-context.ts b/packages/cli/src/webhooks/webhook-execution-context.ts new file mode 100644 index 0000000000000..3470397bf955c --- /dev/null +++ b/packages/cli/src/webhooks/webhook-execution-context.ts @@ -0,0 +1,60 @@ +import type { + IWebhookData, + INode, + IWorkflowDataProxyAdditionalKeys, + Workflow, + WorkflowExecuteMode, + IExecuteData, + IWebhookDescription, + NodeParameterValueType, +} from 'n8n-workflow'; + +/** + * A helper class that holds the context for the webhook execution. + * Provides quality of life methods for evaluating expressions. + */ +export class WebhookExecutionContext { + constructor( + readonly workflow: Workflow, + readonly workflowStartNode: INode, + readonly webhookData: IWebhookData, + readonly executionMode: WorkflowExecuteMode, + readonly additionalKeys: IWorkflowDataProxyAdditionalKeys, + ) {} + + /** + * Evaluates a simple expression from the webhook description. + */ + evaluateSimpleWebhookDescriptionExpression( + propertyName: keyof IWebhookDescription, + executeData?: IExecuteData, + defaultValue?: T, + ): T | undefined { + return this.workflow.expression.getSimpleParameterValue( + this.workflowStartNode, + this.webhookData.webhookDescription[propertyName], + this.executionMode, + this.additionalKeys, + executeData, + defaultValue, + ) as T | undefined; + } + + /** + * Evaluates a complex expression from the webhook description. + */ + evaluateComplexWebhookDescriptionExpression( + propertyName: keyof IWebhookDescription, + executeData?: IExecuteData, + defaultValue?: T, + ): T | undefined { + return this.workflow.expression.getComplexParameterValue( + this.workflowStartNode, + this.webhookData.webhookDescription[propertyName], + this.executionMode, + this.additionalKeys, + executeData, + defaultValue, + ) as T | undefined; + } +} diff --git a/packages/cli/src/webhooks/webhook-helpers.ts b/packages/cli/src/webhooks/webhook-helpers.ts index c7af885dbf450..4392115b38965 100644 --- a/packages/cli/src/webhooks/webhook-helpers.ts +++ b/packages/cli/src/webhooks/webhook-helpers.ts @@ -1,21 +1,18 @@ /* eslint-disable @typescript-eslint/no-unsafe-argument */ -/* eslint-disable @typescript-eslint/prefer-optional-chain */ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ /* eslint-disable id-denylist */ /* eslint-disable prefer-spread */ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ -/* eslint-disable @typescript-eslint/restrict-template-expressions */ + import { Logger } from '@n8n/backend-common'; import { GlobalConfig } from '@n8n/config'; import type { Project } from '@n8n/db'; import { Container } from '@n8n/di'; import type express from 'express'; -import get from 'lodash/get'; import { BinaryDataService, ErrorReporter } from 'n8n-core'; import type { IBinaryData, - IBinaryKeyData, IDataObject, IDeferredPromise, IExecuteData, @@ -35,7 +32,6 @@ import type { WebhookResponseData, } from 'n8n-workflow'; import { - BINARY_ENCODING, createDeferredPromise, ExecutionCancelledError, FORM_NODE_TYPE, @@ -47,6 +43,14 @@ import { } from 'n8n-workflow'; import { finished } from 'stream/promises'; +import { WebhookService } from './webhook.service'; +import type { + IWebhookResponseCallbackData, + WebhookRequest, + WebhookNodeResponseHeaders, + WebhookResponseHeaders, +} from './webhook.types'; + import { ActiveExecutions } from '@/active-executions'; import { MCP_TRIGGER_NODE_TYPE } from '@/constants'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; @@ -56,14 +60,16 @@ import { parseBody } from '@/middlewares'; import { OwnershipService } from '@/services/ownership.service'; import { WorkflowStatisticsService } from '@/services/workflow-statistics.service'; import { WaitTracker } from '@/wait-tracker'; +import { WebhookExecutionContext } from '@/webhooks/webhook-execution-context'; import { createMultiFormDataParser } from '@/webhooks/webhook-form-data'; +import { extractWebhookLastNodeResponse } from '@/webhooks/webhook-last-node-response-extractor'; +import { extractWebhookOnReceivedResponse } from '@/webhooks/webhook-on-received-response-extractor'; +import type { WebhookResponse } from '@/webhooks/webhook-response'; +import { createStaticResponse, createStreamResponse } from '@/webhooks/webhook-response'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; import * as WorkflowHelpers from '@/workflow-helpers'; import { WorkflowRunner } from '@/workflow-runner'; -import { WebhookService } from './webhook.service'; -import type { IWebhookResponseCallbackData, WebhookRequest } from './webhook.types'; - /** * Returns all the webhooks which should be created for the given workflow */ @@ -200,28 +206,6 @@ export const handleFormRedirectionCase = ( const { formDataFileSizeMax } = Container.get(GlobalConfig).endpoints; const parseFormData = createMultiFormDataParser(formDataFileSizeMax); -/** Return webhook response when responseMode is set to "onReceived" */ -export function getResponseOnReceived( - responseData: WebhookResponseData | string | undefined, - webhookResultData: IWebhookResponseData, - responseCode: number, -): IWebhookResponseCallbackData { - const callbackData: IWebhookResponseCallbackData = { responseCode }; - // Return response directly and do not wait for the workflow to finish - if (responseData === 'noData') { - // Return without data - } else if (responseData) { - // Return the data specified in the response data option - callbackData.data = responseData as unknown as IDataObject; - } else if (webhookResultData.webhookResponse !== undefined) { - // Data to respond with is given - callbackData.data = webhookResultData.webhookResponse; - } else { - callbackData.data = { message: 'Workflow was started' }; - } - return callbackData; -} - export function setupResponseNodePromise( responsePromise: IDeferredPromise, res: express.Response, @@ -344,7 +328,10 @@ export async function executeWebhook( executionId: string | undefined, req: WebhookRequest, res: express.Response, - responseCallback: (error: Error | null, data: IWebhookResponseCallbackData) => void, + responseCallback: ( + error: Error | null, + data: IWebhookResponseCallbackData | WebhookResponse, + ) => void, destinationNode?: string, ): Promise { // Get the nodeType to know which responseMode is set @@ -357,6 +344,14 @@ export async function executeWebhook( $executionId: executionId, }; + const context = new WebhookExecutionContext( + workflow, + workflowStartNode, + webhookData, + executionMode, + additionalKeys, + ); + let project: Project | undefined = undefined; try { project = await Container.get(OwnershipService).getWorkflowProjectCached(workflowData.id); @@ -486,30 +481,12 @@ export async function executeWebhook( }; } - if (webhookData.webhookDescription.responseHeaders !== undefined) { - const responseHeaders = workflow.expression.getComplexParameterValue( - workflowStartNode, - webhookData.webhookDescription.responseHeaders, - executionMode, - additionalKeys, - undefined, - undefined, - ) as { - entries?: - | Array<{ - name: string; - value: string; - }> - | undefined; - }; + const responseHeaders = evaluateResponseHeaders(context); - if (!res.headersSent) { - // Only set given headers if they haven't been sent yet, e.g. for streaming - if (responseHeaders?.entries !== undefined) { - for (const item of responseHeaders.entries) { - res.setHeader(item.name, item.value); - } - } + if (!res.headersSent && responseHeaders) { + // Only set given headers if they haven't been sent yet, e.g. for streaming + for (const [name, value] of responseHeaders.entries()) { + res.setHeader(name, value); } } @@ -551,8 +528,9 @@ export async function executeWebhook( // Now that we know that the workflow should run we can return the default response // directly if responseMode it set to "onReceived" and a response should be sent if (responseMode === 'onReceived' && !didSendResponse) { - const callbackData = getResponseOnReceived(responseData, webhookResultData, responseCode); - responseCallback(null, callbackData); + const responseBody = extractWebhookOnReceivedResponse(responseData, webhookResultData); + const webhookResponse = createStaticResponse(responseBody, responseCode, responseHeaders); + responseCallback(null, webhookResponse); didSendResponse = true; } @@ -643,9 +621,8 @@ export async function executeWebhook( if (!didSendResponse) { executePromise - // eslint-disable-next-line complexity - .then(async (data) => { - if (data === undefined) { + .then(async (runData) => { + if (runData === undefined) { if (!didSendResponse) { responseCallback(null, { data: { @@ -659,11 +636,11 @@ export async function executeWebhook( } if (pinData) { - data.data.resultData.pinData = pinData; + runData.data.resultData.pinData = pinData; } - const returnData = WorkflowHelpers.getDataLastExecutedNodeData(data); - if (data.data.resultData.error || returnData?.error !== undefined) { + const lastNodeTaskData = WorkflowHelpers.getDataLastExecutedNodeData(runData); + if (runData.data.resultData.error || lastNodeTaskData?.error !== undefined) { if (!didSendResponse) { responseCallback(null, { data: { @@ -673,7 +650,7 @@ export async function executeWebhook( }); } didSendResponse = true; - return data; + return runData; } // in `responseNode` mode `responseCallback` is called by `responsePromise` @@ -682,7 +659,7 @@ export async function executeWebhook( return undefined; } - if (returnData === undefined) { + if (lastNodeTaskData === undefined) { if (!didSendResponse) { responseCallback(null, { data: { @@ -693,148 +670,38 @@ export async function executeWebhook( }); } didSendResponse = true; - return data; + return runData; } - if (!didSendResponse) { - let data: IDataObject | IDataObject[] | undefined; - - if (responseData === 'firstEntryJson') { - // Return the JSON data of the first entry - - if (returnData.data!.main[0]![0] === undefined) { - responseCallback(new OperationalError('No item to return got found'), {}); - didSendResponse = true; - return undefined; - } - - data = returnData.data!.main[0]![0].json; - - const responsePropertyName = workflow.expression.getSimpleParameterValue( - workflowStartNode, - webhookData.webhookDescription.responsePropertyName, - executionMode, - additionalKeys, - undefined, - undefined, - ); - - if (responsePropertyName !== undefined) { - data = get(data, responsePropertyName as string) as IDataObject; - } - - const responseContentType = workflow.expression.getSimpleParameterValue( - workflowStartNode, - webhookData.webhookDescription.responseContentType, - executionMode, - additionalKeys, - undefined, - undefined, - ); - - if (responseContentType !== undefined) { - // Send the webhook response manually to be able to set the content-type - res.setHeader('Content-Type', responseContentType as string); - - // Returning an object, boolean, number, ... causes problems so make sure to stringify if needed - if ( - data !== null && - data !== undefined && - ['Buffer', 'String'].includes(data.constructor.name) - ) { - res.end(data); - } else { - res.end(JSON.stringify(data)); - } - - responseCallback(null, { - noWebhookResponse: true, - }); - didSendResponse = true; - } - } else if (responseData === 'firstEntryBinary') { - // Return the binary data of the first entry - data = returnData.data!.main[0]![0]; - - if (data === undefined) { - responseCallback(new OperationalError('No item was found to return'), {}); - didSendResponse = true; - return undefined; - } - - if (data.binary === undefined) { - responseCallback(new OperationalError('No binary data was found to return'), {}); - didSendResponse = true; - return undefined; - } - - const responseBinaryPropertyName = workflow.expression.getSimpleParameterValue( - workflowStartNode, - webhookData.webhookDescription.responseBinaryPropertyName, - executionMode, - additionalKeys, - undefined, - 'data', - ); - - if (responseBinaryPropertyName === undefined && !didSendResponse) { - responseCallback( - new OperationalError("No 'responseBinaryPropertyName' is set"), - {}, - ); - didSendResponse = true; - } - - const binaryData = (data.binary as IBinaryKeyData)[ - responseBinaryPropertyName as string - ]; - if (binaryData === undefined && !didSendResponse) { - responseCallback( - new OperationalError( - `The binary property '${responseBinaryPropertyName}' which should be returned does not exist`, - ), - {}, - ); - didSendResponse = true; - } - - if (!didSendResponse) { - // Send the webhook response manually - res.setHeader('Content-Type', binaryData.mimeType); - if (binaryData.id) { - const stream = await Container.get(BinaryDataService).getAsStream(binaryData.id); - stream.pipe(res, { end: false }); - await finished(stream); - } else { - res.write(Buffer.from(binaryData.data, BINARY_ENCODING)); - } - - responseCallback(null, { - noWebhookResponse: true, - }); - process.nextTick(() => res.end()); - } - } else if (responseData === 'noData') { - // Return without data - data = undefined; - } else { - // Return the JSON data of all the entries - data = []; - for (const entry of returnData.data!.main[0]!) { - data.push(entry.json); - } - } + if (didSendResponse) { + return runData; + } - if (!didSendResponse) { - responseCallback(null, { - data, - responseCode, - }); - } + const result = await extractWebhookLastNodeResponse( + context, + responseData as WebhookResponseData, + lastNodeTaskData, + ); + if (!result.ok) { + responseCallback(result.error, {}); + didSendResponse = true; + return runData; } - didSendResponse = true; - return data; + const response = result.result; + // Apply potential content-type override + if (response.contentType) { + responseHeaders.set('content-type', response.contentType); + } + + responseCallback( + null, + response.type === 'static' + ? createStaticResponse(response.body, responseCode, responseHeaders) + : createStreamResponse(response.stream, responseCode, responseHeaders), + ); + didSendResponse = true; + return runData; }) .catch((e) => { if (!didSendResponse) { @@ -963,3 +830,28 @@ async function parseRequestBody( } } } + +/** + * Evaluates the `responseHeaders` parameter of a webhook node + */ +function evaluateResponseHeaders(context: WebhookExecutionContext): WebhookResponseHeaders { + const headers = new Map(); + + if (context.webhookData.webhookDescription.responseHeaders === undefined) { + return headers; + } + + const evaluatedHeaders = + context.evaluateComplexWebhookDescriptionExpression( + 'responseHeaders', + ); + if (evaluatedHeaders?.entries === undefined) { + return headers; + } + + for (const entry of evaluatedHeaders.entries) { + headers.set(entry.name.toLowerCase(), entry.value); + } + + return headers; +} diff --git a/packages/cli/src/webhooks/webhook-last-node-response-extractor.ts b/packages/cli/src/webhooks/webhook-last-node-response-extractor.ts new file mode 100644 index 0000000000000..4fafbddb72507 --- /dev/null +++ b/packages/cli/src/webhooks/webhook-last-node-response-extractor.ts @@ -0,0 +1,160 @@ +import { Container } from '@n8n/di'; +import get from 'lodash/get'; +import { BinaryDataService } from 'n8n-core'; +import type { INodeExecutionData, ITaskData, Result, WebhookResponseData } from 'n8n-workflow'; +import { BINARY_ENCODING, createResultError, createResultOk, OperationalError } from 'n8n-workflow'; +import type { Readable } from 'node:stream'; + +import type { WebhookExecutionContext } from '@/webhooks/webhook-execution-context'; + +/** Response that is not a stream */ +type StaticResponse = { + type: 'static'; + body: unknown; + contentType: string | undefined; +}; + +type StreamResponse = { + type: 'stream'; + stream: Readable; + contentType: string | undefined; +}; + +/** ++ * Extracts the response for a webhook when the response mode is set to + * `lastNode`. + */ +export async function extractWebhookLastNodeResponse( + context: WebhookExecutionContext, + responseDataType: WebhookResponseData | undefined, + lastNodeTaskData: ITaskData, +): Promise> { + if (responseDataType === 'firstEntryJson') { + return extractFirstEntryJsonFromTaskData(context, lastNodeTaskData); + } + + if (responseDataType === 'firstEntryBinary') { + return await extractFirstEntryBinaryFromTaskData(context, lastNodeTaskData); + } + + if (responseDataType === 'noData') { + return createResultOk({ + type: 'static', + body: undefined, + contentType: undefined, + }); + } + + // Default to all entries JSON + return extractAllEntriesJsonFromTaskData(lastNodeTaskData); +} + +/** + * Extracts the JSON data of the first item of the last node + */ +function extractFirstEntryJsonFromTaskData( + context: WebhookExecutionContext, + lastNodeTaskData: ITaskData, +): Result { + if (lastNodeTaskData.data!.main[0]![0] === undefined) { + return createResultError(new OperationalError('No item to return was found')); + } + + let lastNodeFirstJsonItem: unknown = lastNodeTaskData.data!.main[0]![0].json; + + const responsePropertyName = + context.evaluateSimpleWebhookDescriptionExpression('responsePropertyName'); + + if (responsePropertyName !== undefined) { + lastNodeFirstJsonItem = get(lastNodeFirstJsonItem, responsePropertyName); + } + + // User can set the content type of the response and also the headers. + // The `responseContentType` only applies to `firstEntryJson` mode. + const responseContentType = + context.evaluateSimpleWebhookDescriptionExpression('responseContentType'); + + return createResultOk({ + type: 'static', + body: lastNodeFirstJsonItem, + contentType: responseContentType, + }); +} + +/** + * Extracts the binary data of the first item of the last node + */ +async function extractFirstEntryBinaryFromTaskData( + context: WebhookExecutionContext, + lastNodeTaskData: ITaskData, +): Promise> { + // Return the binary data of the first entry + const lastNodeFirstJsonItem: INodeExecutionData = lastNodeTaskData.data!.main[0]![0]; + + if (lastNodeFirstJsonItem === undefined) { + return createResultError(new OperationalError('No item was found to return')); + } + + if (lastNodeFirstJsonItem.binary === undefined) { + return createResultError(new OperationalError('No binary data was found to return')); + } + + const responseBinaryPropertyName = context.evaluateSimpleWebhookDescriptionExpression( + 'responseBinaryPropertyName', + undefined, + 'data', + ); + + if (responseBinaryPropertyName === undefined) { + return createResultError(new OperationalError("No 'responseBinaryPropertyName' is set")); + } else if (typeof responseBinaryPropertyName !== 'string') { + return createResultError(new OperationalError("'responseBinaryPropertyName' is not a string")); + } + + const binaryData = lastNodeFirstJsonItem.binary[responseBinaryPropertyName]; + if (binaryData === undefined) { + return createResultError( + new OperationalError( + `The binary property '${responseBinaryPropertyName}' which should be returned does not exist`, + ), + ); + } + + // In binary data's case, the mime type takes precedence over any manually + // set content type header + if (binaryData.id) { + const stream = await Container.get(BinaryDataService).getAsStream(binaryData.id); + return createResultOk({ + type: 'stream', + stream, + contentType: binaryData.mimeType, + }); + } else { + return createResultOk({ + type: 'static', + body: Buffer.from(binaryData.data, BINARY_ENCODING), + contentType: binaryData.mimeType, + }); + } +} + +/** + * Extracts the JSON data of all the items of the last node + */ +function extractAllEntriesJsonFromTaskData( + lastNodeTaskData: ITaskData, +): Result { + const data: unknown[] = []; + for (const entry of lastNodeTaskData.data!.main[0]!) { + data.push(entry.json); + } + + return createResultOk({ + type: 'static', + body: data, + // No content-type override in this case. User can set the content-type + // header if they wish. We default to application/json later on when the + // response is sent. + contentType: undefined, + }); +} diff --git a/packages/cli/src/webhooks/webhook-on-received-response-extractor.ts b/packages/cli/src/webhooks/webhook-on-received-response-extractor.ts new file mode 100644 index 0000000000000..ce39122cd440a --- /dev/null +++ b/packages/cli/src/webhooks/webhook-on-received-response-extractor.ts @@ -0,0 +1,33 @@ +import type { IWebhookResponseData, WebhookResponseData } from 'n8n-workflow'; + +/** ++ * Creates the response for a webhook when the response mode is set to + * `onReceived`. + * + * @param context - The webhook execution context + * @param responseData - The evaluated `responseData` option of the webhook node + * @param webhookResultData - The webhook result data that the webhook might have returned when it was ran + * + * @returns The response body + */ +export function extractWebhookOnReceivedResponse( + // eslint-disable-next-line @typescript-eslint/no-redundant-type-constituents + responseData: Extract | string | undefined, + webhookResultData: IWebhookResponseData, +): unknown { + // Return response directly and do not wait for the workflow to finish + if (responseData === 'noData') { + return undefined; + } + + if (responseData) { + return responseData; + } + + if (webhookResultData.webhookResponse !== undefined) { + // Data to respond with is given + return webhookResultData.webhookResponse as unknown; + } + + return { message: 'Workflow was started' }; +} diff --git a/packages/cli/src/webhooks/webhook-request-handler.ts b/packages/cli/src/webhooks/webhook-request-handler.ts index 18d659a26dcd2..dd2eb671ad1be 100644 --- a/packages/cli/src/webhooks/webhook-request-handler.ts +++ b/packages/cli/src/webhooks/webhook-request-handler.ts @@ -1,18 +1,36 @@ import { Logger } from '@n8n/backend-common'; import { Container } from '@n8n/di'; import type express from 'express'; +import { + 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 { + WebhookStaticResponse, + WebhookResponse, + WebhookResponseStream, +} from '@/webhooks/webhook-response'; +import { + isWebhookNoResponse, + isWebhookStaticResponse, + isWebhookResponse, + isWebhookStreamResponse, +} from '@/webhooks/webhook-response'; import type { IWebhookManager, WebhookOptionsRequest, WebhookRequest, + WebhookResponseHeaders, } from '@/webhooks/webhook.types'; -import { WebhookService } from './webhook.service'; - const WEBHOOK_METHODS: IHttpRequestMethods[] = ['DELETE', 'GET', 'HEAD', 'PATCH', 'POST', 'PUT']; class WebhookRequestHandler { @@ -47,15 +65,23 @@ class WebhookRequestHandler { try { const response = await this.webhookManager.executeWebhook(req, res); - // Don't respond, if already responded - if (response.noWebhookResponse !== true) { - ResponseHelper.sendSuccessResponse( - res, - response.data, - true, - response.responseCode, - response.headers, - ); + // Modern way of responding to webhooks + if (isWebhookResponse(response)) { + await this.sendWebhookResponse(res, response); + } else { + // Legacy way of responding to webhooks. `WebhookResponse` should be used to + // pass the response from the webhookManager. However, we still have code + // that doesn't use that yet. We need to keep this here until all codepaths + // return a `WebhookResponse` instead. + if (response.noWebhookResponse !== true) { + ResponseHelper.sendSuccessResponse( + res, + response.data, + true, + response.responseCode, + response.headers, + ); + } } } catch (e) { const error = ensureError(e); @@ -78,6 +104,74 @@ class WebhookRequestHandler { } } + private async sendWebhookResponse(res: express.Response, webhookResponse: WebhookResponse) { + if (isWebhookNoResponse(webhookResponse)) { + return; + } + + if (isWebhookStaticResponse(webhookResponse)) { + this.sendStaticResponse(res, webhookResponse); + return; + } + + if (isWebhookStreamResponse(webhookResponse)) { + await this.sendStreamResponse(res, webhookResponse); + return; + } + } + + private async sendStreamResponse(res: express.Response, webhookResponse: WebhookResponseStream) { + const { stream, code, headers } = webhookResponse; + + 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); + + process.nextTick(() => res.end()); + } + + private sendStaticResponse(res: express.Response, webhookResponse: WebhookStaticResponse) { + const { body, code, headers } = webhookResponse; + + 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); + } else { + const needsSandbox = contentType && isHtmlRenderedContentType(contentType); + if (needsSandbox) { + res.send(sandboxHtmlResponse(JSON.stringify(body))); + } else { + res.json(body); + } + } + } + + private setResponseStatus(res: express.Response, statusCode?: number) { + if (statusCode !== undefined) { + res.status(statusCode); + } + } + + private setResponseHeaders(res: express.Response, headers?: WebhookResponseHeaders) { + if (headers) { + for (const [name, value] of headers.entries()) { + res.setHeader(name, value); + } + } + } + private async setupCorsHeaders( req: WebhookRequest | WebhookOptionsRequest, res: express.Response, diff --git a/packages/cli/src/webhooks/webhook-response.ts b/packages/cli/src/webhooks/webhook-response.ts new file mode 100644 index 0000000000000..5437c4561006a --- /dev/null +++ b/packages/cli/src/webhooks/webhook-response.ts @@ -0,0 +1,83 @@ +import type { Readable } from 'stream'; + +import type { WebhookResponseHeaders } from './webhook.types'; + +export const WebhookResponseTag = Symbol('WebhookResponse'); + +/** + * Result that indicates that no response needs to be sent. This is used + * when the node or something else has already sent a response. + */ +export type WebhookNoResponse = { + [WebhookResponseTag]: 'noResponse'; +}; + +/** + * Result that indicates that a non-stream response needs to be sent. + */ +export type WebhookStaticResponse = { + [WebhookResponseTag]: 'static'; + body: unknown; + headers: WebhookResponseHeaders | undefined; + code: number | undefined; +}; + +/** + * Result that indicates that a stream response needs to be sent. + */ +export type WebhookResponseStream = { + [WebhookResponseTag]: 'stream'; + stream: Readable; + code: number | undefined; + headers: WebhookResponseHeaders | undefined; +}; + +export type WebhookResponse = WebhookNoResponse | WebhookStaticResponse | WebhookResponseStream; + +export const isWebhookResponse = (response: unknown): response is WebhookResponse => { + return typeof response === 'object' && response !== null && WebhookResponseTag in response; +}; + +export const isWebhookNoResponse = (response: unknown): response is WebhookNoResponse => { + return isWebhookResponse(response) && response[WebhookResponseTag] === 'noResponse'; +}; + +export const isWebhookStaticResponse = (response: unknown): response is WebhookStaticResponse => { + return isWebhookResponse(response) && response[WebhookResponseTag] === 'static'; +}; + +export const isWebhookStreamResponse = (response: unknown): response is WebhookResponseStream => { + return isWebhookResponse(response) && response[WebhookResponseTag] === 'stream'; +}; + +export const createNoResponse = (): WebhookNoResponse => { + return { + [WebhookResponseTag]: 'noResponse', + }; +}; + +export const createStaticResponse = ( + body: unknown, + code: number | undefined, + headers: WebhookResponseHeaders | undefined, +): WebhookStaticResponse => { + return { + [WebhookResponseTag]: 'static', + body, + code, + headers, + }; +}; + +export const createStreamResponse = ( + stream: Readable, + code: number | undefined, + headers: WebhookResponseHeaders | undefined, +): WebhookResponseStream => { + return { + [WebhookResponseTag]: 'stream', + stream, + code, + headers, + }; +}; diff --git a/packages/cli/src/webhooks/webhook.types.ts b/packages/cli/src/webhooks/webhook.types.ts index c34d15ddd9eb2..7e10f57b7a154 100644 --- a/packages/cli/src/webhooks/webhook.types.ts +++ b/packages/cli/src/webhooks/webhook.types.ts @@ -37,3 +37,16 @@ export interface IWebhookResponseCallbackData { } export type Method = NonNullable; + +/** Response headers. Keys are always lower-cased. */ +export type WebhookResponseHeaders = Map; + +/** + * The headers object that node's `responseHeaders` property can return + */ +export type WebhookNodeResponseHeaders = { + entries?: Array<{ + name: string; + value: string; + }>; +}; diff --git a/packages/core/package.json b/packages/core/package.json index a13e82fb2da2e..5bf22bff9a02b 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "n8n-core", - "version": "1.102.0", + "version": "1.102.1", "description": "Core functionality of n8n", "main": "dist/index", "types": "dist/index.d.ts", diff --git a/packages/core/src/__tests__/__snapshots__/html-sandbox.test.ts.snap b/packages/core/src/__tests__/__snapshots__/html-sandbox.test.ts.snap new file mode 100644 index 0000000000000..e1eed34e3a9ab --- /dev/null +++ b/packages/core/src/__tests__/__snapshots__/html-sandbox.test.ts.snap @@ -0,0 +1,19 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`sandboxHtmlResponse should handle HTML with special characters 1`] = ` +"" +`; + +exports[`sandboxHtmlResponse should handle empty HTML 1`] = ` +"" +`; + +exports[`sandboxHtmlResponse should replace ampersands and double quotes in HTML 1`] = ` +"" +`; diff --git a/packages/core/src/__tests__/html-sandbox.test.ts b/packages/core/src/__tests__/html-sandbox.test.ts new file mode 100644 index 0000000000000..a85013e08049f --- /dev/null +++ b/packages/core/src/__tests__/html-sandbox.test.ts @@ -0,0 +1,278 @@ +import { Readable } from 'stream'; + +import { + bufferEscapeHtml, + createHtmlSandboxTransformStream, + 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); + }); +} + +describe('sandboxHtmlResponse', () => { + it('should replace ampersands and double quotes in HTML', () => { + const html = '
Content & more
'; + expect(sandboxHtmlResponse(html)).toMatchSnapshot(); + }); + + it('should handle empty HTML', () => { + const html = ''; + expect(sandboxHtmlResponse(html)).toMatchSnapshot(); + }); + + it('should handle HTML with special characters', () => { + const html = '

Special characters: <>&"\'

'; + expect(sandboxHtmlResponse(html)).toMatchSnapshot(); + }); +}); + +describe('isHtmlRenderedContentType', () => { + it('should return true for text/html content type', () => { + const contentType = 'text/html'; + expect(isHtmlRenderedContentType(contentType)).toBe(true); + }); + + it('should return true for application/xhtml+xml content type', () => { + const contentType = 'application/xhtml+xml'; + expect(isHtmlRenderedContentType(contentType)).toBe(true); + }); + + it('should return false for other content types', () => { + const contentType = 'application/json'; + expect(isHtmlRenderedContentType(contentType)).toBe(false); + }); + + describe('should handle various HTML content types', () => { + test.each([ + 'text/html', + 'TEXT/HTML', + 'text/html; charset=utf-8', + 'text/html; charset=iso-8859-1', + 'application/xhtml+xml', + 'APPLICATION/XHTML+XML', + 'application/xhtml+xml; charset=utf-8', + ])('should return true for %s', (contentType) => { + expect(isHtmlRenderedContentType(contentType)).toBe(true); + }); + }); + + describe('should handle non-HTML content types', () => { + test.each([ + 'text/plain', + 'application/xml', + 'text/css', + 'application/javascript', + 'image/png', + 'application/pdf', + '', + 'html', + 'xhtml', + ])('should return false for %s', (contentType) => { + expect(isHtmlRenderedContentType(contentType)).toBe(false); + }); + }); + + it('should handle edge cases', () => { + expect(isHtmlRenderedContentType('text/htmlsomething')).toBe(true); + expect(isHtmlRenderedContentType('application/xhtml+xmlsomething')).toBe(true); + expect(isHtmlRenderedContentType(' text/html')).toBe(false); + expect(isHtmlRenderedContentType('text/html ')).toBe(true); + }); +}); + +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'); + }); + + 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()).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)); + }); +}); diff --git a/packages/core/src/execution-engine/node-execution-context/__tests__/__snapshots__/html-sandbox.test.ts.snap b/packages/core/src/execution-engine/node-execution-context/__tests__/__snapshots__/html-sandbox.test.ts.snap deleted file mode 100644 index ae207902537ca..0000000000000 --- a/packages/core/src/execution-engine/node-execution-context/__tests__/__snapshots__/html-sandbox.test.ts.snap +++ /dev/null @@ -1,25 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`sandboxHtmlResponse should handle HTML with special characters 1`] = ` -" - " -`; - -exports[`sandboxHtmlResponse should handle empty HTML 1`] = ` -" - " -`; - -exports[`sandboxHtmlResponse should replace ampersands and double quotes in HTML 1`] = ` -" - " -`; diff --git a/packages/core/src/execution-engine/node-execution-context/__tests__/html-sandbox.test.ts b/packages/core/src/execution-engine/node-execution-context/__tests__/html-sandbox.test.ts deleted file mode 100644 index b33a7b2196938..0000000000000 --- a/packages/core/src/execution-engine/node-execution-context/__tests__/html-sandbox.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { isHtmlRenderedContentType, sandboxHtmlResponse } from '@/html-sandbox'; - -describe('sandboxHtmlResponse', () => { - it('should replace ampersands and double quotes in HTML', () => { - const html = '
Content & more
'; - expect(sandboxHtmlResponse(html)).toMatchSnapshot(); - }); - - it('should handle empty HTML', () => { - const html = ''; - expect(sandboxHtmlResponse(html)).toMatchSnapshot(); - }); - - it('should handle HTML with special characters', () => { - const html = '

Special characters: <>&"\'

'; - expect(sandboxHtmlResponse(html)).toMatchSnapshot(); - }); -}); - -describe('isHtmlRenderedContentType', () => { - it('should return true for text/html content type', () => { - const contentType = 'text/html'; - expect(isHtmlRenderedContentType(contentType)).toBe(true); - }); - - it('should return true for application/xhtml+xml content type', () => { - const contentType = 'application/xhtml+xml'; - expect(isHtmlRenderedContentType(contentType)).toBe(true); - }); - - it('should return false for other content types', () => { - const contentType = 'application/json'; - expect(isHtmlRenderedContentType(contentType)).toBe(false); - }); - - describe('should handle various HTML content types', () => { - test.each([ - 'text/html', - 'TEXT/HTML', - 'text/html; charset=utf-8', - 'text/html; charset=iso-8859-1', - 'application/xhtml+xml', - 'APPLICATION/XHTML+XML', - 'application/xhtml+xml; charset=utf-8', - ])('should return true for %s', (contentType) => { - expect(isHtmlRenderedContentType(contentType)).toBe(true); - }); - }); - - describe('should handle non-HTML content types', () => { - test.each([ - 'text/plain', - 'application/xml', - 'text/css', - 'application/javascript', - 'image/png', - 'application/pdf', - '', - 'html', - 'xhtml', - ])('should return false for %s', (contentType) => { - expect(isHtmlRenderedContentType(contentType)).toBe(false); - }); - }); - - it('should handle edge cases', () => { - expect(isHtmlRenderedContentType('text/htmlsomething')).toBe(true); - expect(isHtmlRenderedContentType('application/xhtml+xmlsomething')).toBe(true); - expect(isHtmlRenderedContentType(' text/html')).toBe(false); - expect(isHtmlRenderedContentType('text/html ')).toBe(true); - }); -}); diff --git a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/file-system-helper-functions.test.ts b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/file-system-helper-functions.test.ts index d031fb4993e32..8972b1210ee4d 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/file-system-helper-functions.test.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/file-system-helper-functions.test.ts @@ -119,6 +119,36 @@ describe('isFilePathBlocked', () => { expect(isFilePathBlocked(invitePath)).toBe(true); expect(isFilePathBlocked(pwResetPath)).toBe(true); }); + + it('should block access to n8n files if restrict and block are set', () => { + const homeVarName = process.platform === 'win32' ? 'USERPROFILE' : 'HOME'; + const userHome = process.env.N8N_USER_FOLDER ?? process.env[homeVarName] ?? process.cwd(); + + process.env[RESTRICT_FILE_ACCESS_TO] = userHome; + process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'true'; + const restrictedPath = instanceSettings.n8nFolder; + expect(isFilePathBlocked(restrictedPath)).toBe(true); + }); + + it('should allow access to parent folder if restrict and block are set', () => { + const homeVarName = process.platform === 'win32' ? 'USERPROFILE' : 'HOME'; + const userHome = process.env.N8N_USER_FOLDER ?? process.env[homeVarName] ?? process.cwd(); + + process.env[RESTRICT_FILE_ACCESS_TO] = userHome; + process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'true'; + const restrictedPath = join(userHome, 'somefile.txt'); + expect(isFilePathBlocked(restrictedPath)).toBe(false); + }); + + it('should not block similar paths', () => { + const homeVarName = process.platform === 'win32' ? 'USERPROFILE' : 'HOME'; + const userHome = process.env.N8N_USER_FOLDER ?? process.env[homeVarName] ?? process.cwd(); + + process.env[RESTRICT_FILE_ACCESS_TO] = userHome; + process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] = 'true'; + const restrictedPath = join(userHome, '.n8n_x'); + expect(isFilePathBlocked(restrictedPath)).toBe(false); + }); }); describe('getFileSystemHelperFunctions', () => { diff --git a/packages/core/src/execution-engine/node-execution-context/utils/file-system-helper-functions.ts b/packages/core/src/execution-engine/node-execution-context/utils/file-system-helper-functions.ts index 175f780a41e0a..e575b298454ab 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/file-system-helper-functions.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/file-system-helper-functions.ts @@ -1,4 +1,4 @@ -import { safeJoinPath } from '@n8n/backend-common'; +import { isContainedWithin, safeJoinPath } from '@n8n/backend-common'; import { Container } from '@n8n/di'; import type { FileSystemHelperFunctions, INode } from 'n8n-workflow'; import { NodeOperationError } from 'n8n-workflow'; @@ -34,52 +34,17 @@ export function isFilePathBlocked(filePath: string): boolean { const resolvedFilePath = resolve(filePath); const blockFileAccessToN8nFiles = process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] !== 'false'; - //if allowed paths are defined, allow access only to those paths - if (allowedPaths.length) { - for (const path of allowedPaths) { - if (resolvedFilePath.startsWith(path)) { - return false; - } - } - + const restrictedPaths = blockFileAccessToN8nFiles ? getN8nRestrictedPaths() : []; + if ( + restrictedPaths.some((restrictedPath) => isContainedWithin(restrictedPath, resolvedFilePath)) + ) { return true; } - //restrict access to .n8n folder, ~/.cache/n8n/public, and other .env config related paths - if (blockFileAccessToN8nFiles) { - const { n8nFolder, staticCacheDir } = Container.get(InstanceSettings); - const restrictedPaths = [n8nFolder, staticCacheDir]; - - if (process.env[CONFIG_FILES]) { - restrictedPaths.push(...process.env[CONFIG_FILES].split(',')); - } - - if (process.env[CUSTOM_EXTENSION_ENV]) { - const customExtensionFolders = process.env[CUSTOM_EXTENSION_ENV].split(';'); - restrictedPaths.push(...customExtensionFolders); - } - - if (process.env[BINARY_DATA_STORAGE_PATH]) { - restrictedPaths.push(process.env[BINARY_DATA_STORAGE_PATH]); - } - - if (process.env[UM_EMAIL_TEMPLATES_INVITE]) { - restrictedPaths.push(process.env[UM_EMAIL_TEMPLATES_INVITE]); - } - - if (process.env[UM_EMAIL_TEMPLATES_PWRESET]) { - restrictedPaths.push(process.env[UM_EMAIL_TEMPLATES_PWRESET]); - } - - //check if the file path is restricted - for (const path of restrictedPaths) { - if (resolvedFilePath.startsWith(path)) { - return true; - } - } + if (allowedPaths.length) { + return !allowedPaths.some((allowedPath) => isContainedWithin(allowedPath, resolvedFilePath)); } - //path is not restricted return false; } @@ -120,3 +85,34 @@ export const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunct return await fsWriteFile(filePath, content, { encoding: 'binary', flag }); }, }); + +/** + * @returns The restricted paths for the n8n instance. + */ +function getN8nRestrictedPaths() { + const { n8nFolder, staticCacheDir } = Container.get(InstanceSettings); + const restrictedPaths = [n8nFolder, staticCacheDir]; + + if (process.env[CONFIG_FILES]) { + restrictedPaths.push(...process.env[CONFIG_FILES].split(',')); + } + + if (process.env[CUSTOM_EXTENSION_ENV]) { + const customExtensionFolders = process.env[CUSTOM_EXTENSION_ENV].split(';'); + restrictedPaths.push(...customExtensionFolders); + } + + if (process.env[BINARY_DATA_STORAGE_PATH]) { + restrictedPaths.push(process.env[BINARY_DATA_STORAGE_PATH]); + } + + if (process.env[UM_EMAIL_TEMPLATES_INVITE]) { + restrictedPaths.push(process.env[UM_EMAIL_TEMPLATES_INVITE]); + } + + if (process.env[UM_EMAIL_TEMPLATES_PWRESET]) { + restrictedPaths.push(process.env[UM_EMAIL_TEMPLATES_PWRESET]); + } + + return restrictedPaths; +} diff --git a/packages/core/src/html-sandbox.ts b/packages/core/src/html-sandbox.ts index 5581cb307e065..2ecae9f8aaeef 100644 --- a/packages/core/src/html-sandbox.ts +++ b/packages/core/src/html-sandbox.ts @@ -1,3 +1,6 @@ +import type { TransformCallback } from 'stream'; +import { Transform } from 'stream'; + /** * Sandboxes the HTML response to prevent possible exploitation. Embeds the * response in an iframe to make sure the HTML has a different origin. @@ -7,11 +10,98 @@ export const sandboxHtmlResponse = (html: string) => { // https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element const escapedHtml = html.replaceAll('&', '&').replaceAll('"', '"'); - return ` - `; + allowtransparency="true">`; +}; + +/** + * 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); + } + }, + }); }; /** diff --git a/packages/node-dev/package.json b/packages/node-dev/package.json index 0a685a64c3fc8..45acd732fb8fb 100644 --- a/packages/node-dev/package.json +++ b/packages/node-dev/package.json @@ -1,6 +1,6 @@ { "name": "n8n-node-dev", - "version": "1.101.0", + "version": "1.101.1", "description": "CLI to simplify n8n credentials/node development", "main": "dist/src/index", "types": "dist/src/index.d.ts",