n8n has Unauthenticated Expression Evaluation via Form Node
Description
n8n is an open source workflow automation platform. Prior to versions 2.10.1, 2.9.3, and 1.123.22, a second-order expression injection vulnerability existed in n8n's Form nodes that could allow an unauthenticated attacker to inject and evaluate arbitrary n8n expressions by submitting crafted form data. When chained with an expression sandbox escape, this could escalate to remote code execution on the n8n host. The vulnerability requires a specific workflow configuration to be exploitable. First, a form node with a field interpolating a value provided by an unauthenticated user, e.g. a form submitted value. Second, the field value must begin with an = character, which caused n8n to treat it as an expression and triggered a double-evaluation of the field content. There is no practical reason for a workflow designer to prefix a field with = intentionally — the character is not rendered in the output, so the result would not match the designer's expectations. If added accidentally, it would be noticeable and very unlikely to persist. An unauthenticated attacker would need to either know about this specific circumstance on a target instance or discover a matching form by chance. Even when the preconditions are met, the expression injection alone is limited to data accessible within the n8n expression context. Escalation to remote code execution requires chaining with a separate sandbox escape vulnerability. The issue has been fixed in n8n versions 2.10.1, 2.9.3, and 1.123.22. Users should upgrade to one of these versions or later to remediate the vulnerability. If upgrading is not immediately possible, administrators should consider the following temporary mitigations. Review usage of form nodes manually for above mentioned preconditions, disable the Form node by adding n8n-nodes-base.form to the NODES_EXCLUDE environment variable, and/or disable the Form Trigger node by adding n8n-nodes-base.formTrigger to the NODES_EXCLUDE environment variable. These workarounds do not fully remediate the risk and should only be used as short-term mitigation measures.
Affected packages
Versions sourced from the GitHub Security Advisory.
| Package | Affected versions | Patched versions |
|---|---|---|
n8nnpm | < 1.123.22 | 1.123.22 |
n8nnpm | >= 2.0.0, < 2.9.3 | 2.9.3 |
n8nnpm | >= 2.10.0, < 2.10.1 | 2.10.1 |
Affected products
1Patches
1562d867483e8chore: Bundle 2026-W7 (#26214)
37 files changed · +2484 −294
packages/cli/src/auth/auth.service.ts+11 −0 modified@@ -231,6 +231,17 @@ export class AuthService { }); } + /** + * Validate a cookie auth token: checks revocation, JWT signature/expiry, + * user existence, and hash consistency. Skips browser-id and MFA checks + * since those are not applicable to webhook cookie validation. + */ + async validateCookieToken(token: string): Promise<void> { + const isInvalid = await this.invalidAuthTokenRepository.existsBy({ token }); + if (isInvalid) throw new AuthError('Unauthorized'); + await this.validateToken(token); + } + async authenticateUserBasedOnToken( token: string, method: string,
packages/cli/src/auth/__tests__/auth.service.test.ts+54 −0 modified@@ -937,6 +937,60 @@ describe('AuthService', () => { }); }); + describe('validateCookieToken', () => { + beforeEach(() => { + jest.resetAllMocks(); + }); + + it('should resolve for a valid token', async () => { + const token = authService.issueJWT(user, false, browserId); + invalidAuthTokenRepository.existsBy.mockResolvedValue(false); + userRepository.findOne.mockResolvedValue(user); + + await expect(authService.validateCookieToken(token)).resolves.toBeUndefined(); + expect(invalidAuthTokenRepository.existsBy).toHaveBeenCalledWith({ token }); + expect(userRepository.findOne).toHaveBeenCalled(); + }); + + it('should throw when token has been revoked', async () => { + const token = authService.issueJWT(user, false, browserId); + invalidAuthTokenRepository.existsBy.mockResolvedValue(true); + + await expect(authService.validateCookieToken(token)).rejects.toThrow('Unauthorized'); + expect(userRepository.findOne).not.toHaveBeenCalled(); + }); + + it('should throw when token is malformed', async () => { + invalidAuthTokenRepository.existsBy.mockResolvedValue(false); + + await expect(authService.validateCookieToken('fake-value')).rejects.toThrow('jwt malformed'); + }); + + it('should throw when token is expired', async () => { + const token = authService.issueJWT(user, false, browserId); + invalidAuthTokenRepository.existsBy.mockResolvedValue(false); + jest.advanceTimersByTime(365 * Time.days.toMilliseconds); + + await expect(authService.validateCookieToken(token)).rejects.toThrow('jwt expired'); + }); + + it('should throw when user is disabled', async () => { + const token = authService.issueJWT(user, false, browserId); + invalidAuthTokenRepository.existsBy.mockResolvedValue(false); + userRepository.findOne.mockResolvedValue(mock<User>({ ...userData, disabled: true })); + + await expect(authService.validateCookieToken(token)).rejects.toThrow('Unauthorized'); + }); + + it('should throw when user no longer exists', async () => { + const token = authService.issueJWT(user, false, browserId); + invalidAuthTokenRepository.existsBy.mockResolvedValue(false); + userRepository.findOne.mockResolvedValue(null); + + await expect(authService.validateCookieToken(token)).rejects.toThrow('Unauthorized'); + }); + }); + describe('authenticateUserBasedOnToken', () => { const method = 'POST'; const endpoint = '/api/users';
packages/cli/src/webhooks/__tests__/webhook-helpers.test.ts+2 −2 modified@@ -270,7 +270,7 @@ describe('setupResponseNodePromise', () => { await new Promise(process.nextTick); expect(binaryDataService.getAsStream).toHaveBeenCalledWith('binary-123'); - expect(res.header).toHaveBeenCalledWith({ 'content-type': 'image/jpeg' }); + expect(res.setHeaders).toHaveBeenCalledWith(new Map([['content-type', 'image/jpeg']])); expect(mockStream.pipe).toHaveBeenCalledWith(res, { end: false }); expect(finished).toHaveBeenCalledWith(mockStream); expect(responseCallback).toHaveBeenCalledWith(null, { noWebhookResponse: true }); @@ -294,7 +294,7 @@ describe('setupResponseNodePromise', () => { }); await new Promise(process.nextTick); - expect(res.header).toHaveBeenCalledWith({ 'content-type': 'text/plain' }); + expect(res.setHeaders).toHaveBeenCalledWith(new Map([['content-type', 'text/plain']])); expect(res.end).toHaveBeenCalledWith(buffer); expect(responseCallback).toHaveBeenCalledWith(null, { noWebhookResponse: true }); });
packages/cli/src/webhooks/__tests__/webhook-request-handler.test.ts+102 −1 modified@@ -1,5 +1,6 @@ import { type Response } from 'express'; import { mock } from 'jest-mock-extended'; +import { isWebhookHtmlSandboxingDisabled, getWebhookSandboxCSP } from 'n8n-core'; import { randomString } from 'n8n-workflow'; import type { IHttpRequestMethods } from 'n8n-workflow'; @@ -12,6 +13,14 @@ import type { WebhookRequest, } from '@/webhooks/webhook.types'; +jest.mock('n8n-core', () => ({ + ...jest.requireActual('n8n-core'), + isWebhookHtmlSandboxingDisabled: jest.fn().mockReturnValue(false), + getWebhookSandboxCSP: jest + .fn() + .mockReturnValue('sandbox allow-downloads allow-forms allow-modals'), +})); + describe('WebhookRequestHandler', () => { const webhookManager = mock<Required<IWebhookManager>>(); const handler = createWebhookHandlerFor(webhookManager); @@ -163,7 +172,7 @@ describe('WebhookRequestHandler', () => { expect(webhookManager.executeWebhook).toHaveBeenCalledWith(req, res); expect(res.status).toHaveBeenCalledWith(200); - expect(res.setHeader).toHaveBeenCalledWith('x-custom-header', 'test'); + expect(res.setHeaders).toHaveBeenCalledWith(new Map([['x-custom-header', 'test']])); expect(res.json).toHaveBeenCalledWith(executeWebhookResponse.data); }); @@ -192,6 +201,54 @@ describe('WebhookRequestHandler', () => { }); }); + it('should not throw when legacy response headers contain invalid names', async () => { + const req = mock<WebhookRequest>({ + method: 'GET', + params: { path: 'test' }, + }); + + const res = mock<Response>(); + + const executeWebhookResponse: IWebhookResponseCallbackData = { + responseCode: 200, + data: { ok: true }, + headers: { + '<img src=x onerror=alert(1)>': 'xss', + 'x-valid': 'value', + }, + }; + webhookManager.executeWebhook.mockResolvedValueOnce(executeWebhookResponse); + + await handler(req, res); + + expect(res.setHeaders).toHaveBeenCalledTimes(1); + expect(res.setHeaders).toHaveBeenCalledWith(new Map([['x-valid', 'value']])); + expect(res.json).toHaveBeenCalledWith(executeWebhookResponse.data); + }); + + it('should not allow user to override CSP via response headers', async () => { + const req = mock<WebhookRequest>({ + method: 'GET', + params: { path: 'test' }, + }); + + const res = mock<Response>(); + + const executeWebhookResponse: IWebhookResponseCallbackData = { + responseCode: 200, + data: { ok: true }, + headers: { + 'Content-Security-Policy': "default-src 'unsafe-inline'", + }, + }; + webhookManager.executeWebhook.mockResolvedValueOnce(executeWebhookResponse); + + await handler(req, res); + + expect(res.setHeaders).not.toHaveBeenCalled(); + expect(res.setHeader).toHaveBeenCalledWith('Content-Security-Policy', getWebhookSandboxCSP()); + }); + test.each<IHttpRequestMethods>(['DELETE', 'GET', 'HEAD', 'PATCH', 'POST', 'PUT'])( "should handle '%s' method", async (method) => { @@ -215,4 +272,48 @@ describe('WebhookRequestHandler', () => { }, ); }); + + describe('CSP sandbox header', () => { + it('should set CSP sandbox header on all webhook responses', async () => { + const req = mock<WebhookRequest>({ + method: 'GET', + params: { path: 'test' }, + }); + + const res = mock<Response>(); + + const executeWebhookResponse: IWebhookResponseCallbackData = { + responseCode: 200, + data: {}, + headers: { 'content-type': 'image/svg+xml' }, + }; + webhookManager.executeWebhook.mockResolvedValueOnce(executeWebhookResponse); + + await handler(req, res); + + expect(res.setHeader).toHaveBeenCalledWith('Content-Security-Policy', getWebhookSandboxCSP()); + }); + + it('should not set CSP sandbox header when sandboxing is disabled', async () => { + jest.mocked(isWebhookHtmlSandboxingDisabled).mockReturnValueOnce(true); + + const req = mock<WebhookRequest>({ + method: 'GET', + params: { path: 'test' }, + }); + + const res = mock<Response>(); + + const executeWebhookResponse: IWebhookResponseCallbackData = { + responseCode: 200, + data: {}, + headers: { 'content-type': 'text/html' }, + }; + webhookManager.executeWebhook.mockResolvedValueOnce(executeWebhookResponse); + + await handler(req, res); + + expect(res.setHeader).not.toHaveBeenCalledWith('Content-Security-Policy', expect.anything()); + }); + }); });
packages/cli/src/webhooks/__tests__/webhook-response-headers.test.ts+270 −0 added@@ -0,0 +1,270 @@ +import type { Response } from 'express'; +import { mock } from 'jest-mock-extended'; + +import { WebhookResponseHeaders } from '@/webhooks/webhook-response-headers'; + +describe('WebhookResponseHeaders', () => { + describe('set()', () => { + it('should store and apply valid headers', () => { + const headers = new WebhookResponseHeaders(); + headers.set('X-Custom', 'value'); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).toHaveBeenCalledWith(new Map([['x-custom', 'value']])); + }); + + it('should silently skip invalid header names containing HTML', () => { + const headers = new WebhookResponseHeaders(); + headers.set('<img src=x onerror=alert(1)>', 'value'); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).not.toHaveBeenCalled(); + }); + + it('should silently skip header names with control characters', () => { + const headers = new WebhookResponseHeaders(); + headers.set('invalid\x00name', 'value'); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).not.toHaveBeenCalled(); + }); + + it('should silently skip invalid header values', () => { + const headers = new WebhookResponseHeaders(); + headers.set('x-valid-name', 'invalid\x00value'); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).not.toHaveBeenCalled(); + }); + + it('should block content-security-policy header', () => { + const headers = new WebhookResponseHeaders(); + headers.set('Content-Security-Policy', "default-src 'none'"); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).not.toHaveBeenCalled(); + }); + + it('should block content-security-policy regardless of case', () => { + const headers = new WebhookResponseHeaders(); + headers.set('CONTENT-SECURITY-POLICY', "default-src 'none'"); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).not.toHaveBeenCalled(); + }); + + it('should lower-case header names', () => { + const headers = new WebhookResponseHeaders(); + headers.set('X-My-Header', 'test'); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).toHaveBeenCalledWith(new Map([['x-my-header', 'test']])); + }); + }); + + describe('fromObject()', () => { + it('should create an instance with valid headers from an object', () => { + const headers = WebhookResponseHeaders.fromObject({ + 'x-valid': 'value1', + 'x-also-valid': 'value2', + }); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).toHaveBeenCalledWith( + new Map([ + ['x-valid', 'value1'], + ['x-also-valid', 'value2'], + ]), + ); + }); + + it('should skip invalid entries and keep valid ones', () => { + const headers = WebhookResponseHeaders.fromObject({ + 'x-valid': 'value', + '<script>': 'xss', + 'content-security-policy': "default-src 'none'", + }); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).toHaveBeenCalledTimes(1); + expect(res.setHeaders).toHaveBeenCalledWith(new Map([['x-valid', 'value']])); + }); + + it('should convert non-string values to strings', () => { + const headers = WebhookResponseHeaders.fromObject({ + 'x-number': 42, + 'x-bool': true, + }); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).toHaveBeenCalledWith( + new Map([ + ['x-number', '42'], + ['x-bool', 'true'], + ]), + ); + }); + }); + + describe('addFromObject()', () => { + it('should add valid entries from an object', () => { + const headers = new WebhookResponseHeaders(); + headers.addFromObject({ + 'x-valid': 'value1', + 'x-also-valid': 'value2', + }); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).toHaveBeenCalledWith( + new Map([ + ['x-valid', 'value1'], + ['x-also-valid', 'value2'], + ]), + ); + }); + + it('should skip invalid entries and keep valid ones', () => { + const headers = new WebhookResponseHeaders(); + headers.addFromObject({ + 'x-valid': 'value', + '<script>': 'xss', + 'content-security-policy': "default-src 'none'", + }); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).toHaveBeenCalledTimes(1); + expect(res.setHeaders).toHaveBeenCalledWith(new Map([['x-valid', 'value']])); + }); + + it('should convert non-string values to strings', () => { + const headers = new WebhookResponseHeaders(); + headers.addFromObject({ + 'x-number': 42, + 'x-bool': true, + }); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).toHaveBeenCalledWith( + new Map([ + ['x-number', '42'], + ['x-bool', 'true'], + ]), + ); + }); + }); + + describe('addFromNodeHeaders()', () => { + it('should add valid entries from node response headers', () => { + const headers = new WebhookResponseHeaders(); + headers.addFromNodeHeaders({ + entries: [ + { name: 'X-Custom', value: 'test' }, + { name: 'X-Another', value: 'value' }, + ], + }); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).toHaveBeenCalledWith( + new Map([ + ['x-custom', 'test'], + ['x-another', 'value'], + ]), + ); + }); + + it('should skip invalid entries from node response headers', () => { + const headers = new WebhookResponseHeaders(); + headers.addFromNodeHeaders({ + entries: [ + { name: '<img src=x onerror=alert(1)>', value: 'xss' }, + { name: 'x-valid', value: 'ok' }, + ], + }); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).toHaveBeenCalledTimes(1); + expect(res.setHeaders).toHaveBeenCalledWith(new Map([['x-valid', 'ok']])); + }); + + it('should handle undefined entries gracefully', () => { + const headers = new WebhookResponseHeaders(); + headers.addFromNodeHeaders({}); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).not.toHaveBeenCalled(); + }); + + it('should block CSP from node response headers', () => { + const headers = new WebhookResponseHeaders(); + headers.addFromNodeHeaders({ + entries: [{ name: 'Content-Security-Policy', value: "default-src 'unsafe-inline'" }], + }); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).not.toHaveBeenCalled(); + }); + }); + + describe('applyToResponse()', () => { + it('should be a no-op for an empty instance', () => { + const headers = new WebhookResponseHeaders(); + const res = mock<Response>(); + + headers.applyToResponse(res); + + expect(res.setHeaders).not.toHaveBeenCalled(); + }); + + it('should call setHeader for each valid header', () => { + const headers = new WebhookResponseHeaders(); + headers.set('x-one', '1'); + headers.set('x-two', '2'); + headers.set('x-three', '3'); + + const res = mock<Response>(); + headers.applyToResponse(res); + + expect(res.setHeaders).toHaveBeenCalledWith( + new Map([ + ['x-one', '1'], + ['x-two', '2'], + ['x-three', '3'], + ]), + ); + }); + }); +});
packages/cli/src/webhooks/webhook-helpers.ts+16 −17 modified@@ -50,14 +50,14 @@ import { import { finished } from 'stream/promises'; import { WebhookService } from './webhook.service'; -import type { - IWebhookResponseCallbackData, - WebhookRequest, - WebhookNodeResponseHeaders, +import { WebhookResponseHeaders, -} from './webhook.types'; + type WebhookNodeResponseHeaders, +} from './webhook-response-headers'; +import type { IWebhookResponseCallbackData, WebhookRequest } from './webhook.types'; import { ActiveExecutions } from '@/active-executions'; +import { AuthService } from '@/auth/auth.service'; import { MCP_TRIGGER_NODE_TYPE } from '@/constants'; import { EventService } from '@/events/event.service'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; @@ -309,13 +309,13 @@ export function setupResponseNodePromise( .then(async (response: IN8nHttpFullResponse) => { const binaryData = (response.body as IDataObject)?.binaryData as IBinaryData; if (binaryData?.id) { - res.header(response.headers); + WebhookResponseHeaders.fromObject(response.headers).applyToResponse(res); const stream = await Container.get(BinaryDataService).getAsStream(binaryData.id); stream.pipe(res, { end: false }); await finished(stream); responseCallback(null, { noWebhookResponse: true }); } else if (Buffer.isBuffer(response.body)) { - res.header(response.headers); + WebhookResponseHeaders.fromObject(response.headers).applyToResponse(res); res.end(response.body); responseCallback(null, { noWebhookResponse: true }); } else { @@ -486,6 +486,11 @@ export async function executeWebhook( additionalData.httpRequest = req; additionalData.httpResponse = res; + const authService = Container.get(AuthService); + additionalData.validateCookieAuth = async (token: string) => { + await authService.validateCookieToken(token); + }; + let didSendResponse = false; let runExecutionDataMerge = {}; try { @@ -574,9 +579,7 @@ export async function executeWebhook( 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); - } + responseHeaders.applyToResponse(res); } if (webhookResultData.noWebhookResponse === true && !didSendResponse) { @@ -1026,7 +1029,7 @@ async function parseRequestBody( * Evaluates the `responseHeaders` parameter of a webhook node */ function evaluateResponseHeaders(context: WebhookExecutionContext): WebhookResponseHeaders { - const headers = new Map<string, string>(); + const headers = new WebhookResponseHeaders(); if (context.webhookData.webhookDescription.responseHeaders === undefined) { return headers; @@ -1036,12 +1039,8 @@ function evaluateResponseHeaders(context: WebhookExecutionContext): WebhookRespo context.evaluateComplexWebhookDescriptionExpression<WebhookNodeResponseHeaders>( 'responseHeaders', ); - if (evaluatedHeaders?.entries === undefined) { - return headers; - } - - for (const entry of evaluatedHeaders.entries) { - headers.set(entry.name.toLowerCase(), entry.value); + if (evaluatedHeaders) { + headers.addFromNodeHeaders(evaluatedHeaders); } return headers;
packages/cli/src/webhooks/webhook-request-handler.ts+5 −16 modified@@ -1,11 +1,7 @@ import { Logger } from '@n8n/backend-common'; import { Container } from '@n8n/di'; import type express from 'express'; -import { - isWebhookHtmlSandboxingDisabled, - getWebhookSandboxCSP, - isHtmlRenderedContentType, -} from 'n8n-core'; +import { isWebhookHtmlSandboxingDisabled, getWebhookSandboxCSP } from 'n8n-core'; import { ensureError, type IHttpRequestMethods } from 'n8n-workflow'; import { Readable } from 'stream'; import { finished } from 'stream/promises'; @@ -23,11 +19,11 @@ import { isWebhookResponse, isWebhookStreamResponse, } from '@/webhooks/webhook-response'; +import { WebhookResponseHeaders } from '@/webhooks/webhook-response-headers'; import type { IWebhookManager, WebhookOptionsRequest, WebhookRequest, - WebhookResponseHeaders, } from '@/webhooks/webhook.types'; const WEBHOOK_METHODS: IHttpRequestMethods[] = ['DELETE', 'GET', 'HEAD', 'PATCH', 'POST', 'PUT']; @@ -140,16 +136,9 @@ class WebhookRequestHandler { } private setResponseHeaders(res: express.Response, headers?: WebhookResponseHeaders) { - if (headers) { - for (const [name, value] of headers.entries()) { - res.setHeader(name, value); - } - } - - const contentType = res.getHeader('content-type') as string | undefined; - const needsSandbox = !contentType || isHtmlRenderedContentType(contentType); + headers?.applyToResponse(res); - if (needsSandbox && !isWebhookHtmlSandboxingDisabled()) { + if (!isWebhookHtmlSandboxingDisabled()) { res.setHeader('Content-Security-Policy', getWebhookSandboxCSP()); } } @@ -167,7 +156,7 @@ class WebhookRequestHandler { ) { this.setResponseStatus(res, responseCode); if (responseHeader) { - this.setResponseHeaders(res, new Map(Object.entries(responseHeader))); + this.setResponseHeaders(res, WebhookResponseHeaders.fromObject(responseHeader)); } if (data instanceof Readable) {
packages/cli/src/webhooks/webhook-response-headers.ts+70 −0 added@@ -0,0 +1,70 @@ +import { Logger } from '@n8n/backend-common'; +import { Container } from '@n8n/di'; +import type { Response } from 'express'; +import { validateHeaderName, validateHeaderValue } from 'node:http'; +import { ensureError } from 'n8n-workflow'; + +/** + * The headers object that node's `responseHeaders` property can return + */ +export type WebhookNodeResponseHeaders = { + entries?: Array<{ + name: string; + value: string; + }>; +}; + +/** Headers that users are not allowed to set via webhook response config */ +const PROTECTED_HEADERS = new Set(['content-security-policy']); + +/** Response headers. Keys are always lower-cased. Invalid headers are silently skipped. */ +export class WebhookResponseHeaders { + private headers = new Map<string, string>(); + + /** Create an instance from a plain object, validating each entry. */ + static fromObject(obj: object): WebhookResponseHeaders { + const instance = new WebhookResponseHeaders(); + instance.addFromObject(obj); + return instance; + } + + /** Add a single header. Silently skips invalid or protected headers. */ + set(name: string, value: string): void { + const lowerName = name.toLowerCase(); + if (PROTECTED_HEADERS.has(lowerName)) return; + try { + validateHeaderName(lowerName); + validateHeaderValue(lowerName, value); + } catch (e) { + Container.get(Logger).warn('Dropping invalid webhook response header', { + headerName: name, + error: ensureError(e).message, + }); + return; + } + this.headers.set(lowerName, value); + } + + /** Add headers from a plain object (e.g. IDataObject from RespondToWebhook node). */ + addFromObject(obj: object): void { + for (const [name, value] of Object.entries(obj)) { + this.set(name, String(value)); + } + } + + /** Add headers from a webhook node's `responseHeaders` parameter. */ + addFromNodeHeaders(nodeHeaders: WebhookNodeResponseHeaders): void { + if (nodeHeaders.entries === undefined) return; + + for (const entry of nodeHeaders.entries) { + this.set(entry.name, entry.value); + } + } + + /** Apply all validated headers to an Express response. */ + applyToResponse(res: Response): void { + if (this.headers.size === 0) return; + + res.setHeaders(this.headers); + } +}
packages/cli/src/webhooks/webhook-response.ts+1 −1 modified@@ -1,6 +1,6 @@ import type { Readable } from 'stream'; -import type { WebhookResponseHeaders } from './webhook.types'; +import type { WebhookResponseHeaders } from './webhook-response-headers'; export const WebhookResponseTag = Symbol('WebhookResponse');
packages/cli/src/webhooks/webhook.types.ts+0 −13 modified@@ -37,16 +37,3 @@ export interface IWebhookResponseCallbackData { } export type Method = NonNullable<IHttpRequestMethods>; - -/** Response headers. Keys are always lower-cased. */ -export type WebhookResponseHeaders = Map<string, string>; - -/** - * The headers object that node's `responseHeaders` property can return - */ -export type WebhookNodeResponseHeaders = { - entries?: Array<{ - name: string; - value: string; - }>; -};
packages/core/src/execution-engine/node-execution-context/webhook-context.ts+7 −0 modified@@ -132,6 +132,13 @@ export class WebhookContext extends NodeExecutionContext implements IWebhookFunc return this.webhookData.webhookDescription.name; } + async validateCookieAuth(cookieValue: string): Promise<void> { + if (!this.additionalData.validateCookieAuth) { + throw new ApplicationError('Cookie auth validation is not available'); + } + await this.additionalData.validateCookieAuth(cookieValue); + } + async getInputConnectionData( connectionType: AINodeConnectionType, itemIndex: number,
packages/core/src/html-sandbox.ts+0 −13 modified@@ -11,16 +11,3 @@ export const isWebhookHtmlSandboxingDisabled = () => { 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'; }; - -/** - * Checks if the given content type is something a browser might render - * as HTML. - */ -export const isHtmlRenderedContentType = (contentType: string) => { - const contentTypeLower = contentType.trim().toLowerCase(); - - return ( - // The content-type can also contain a charset, e.g. "text/html; charset=utf-8" - contentTypeLower.startsWith('text/html') || contentTypeLower.startsWith('application/xhtml+xml') - ); -};
packages/core/src/__tests__/html-sandbox.test.ts+1 −63 modified@@ -2,11 +2,7 @@ import type { SecurityConfig } from '@n8n/config'; import { Container } from '@n8n/di'; import { mock } from 'jest-mock-extended'; -import { - isWebhookHtmlSandboxingDisabled, - getWebhookSandboxCSP, - isHtmlRenderedContentType, -} from '@/html-sandbox'; +import { isWebhookHtmlSandboxingDisabled, getWebhookSandboxCSP } from '@/html-sandbox'; const securityConfig = mock<SecurityConfig>(); @@ -29,64 +25,6 @@ describe('isWebhookHtmlSandboxingDisabled', () => { }); }); -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 content type with extra spaces', () => { - expect(isHtmlRenderedContentType(' text/html')).toBe(true); - expect(isHtmlRenderedContentType('text/html ')).toBe(true); - expect(isHtmlRenderedContentType(' text/html ')).toBe(true); - }); - - it('should handle edge cases', () => { - expect(isHtmlRenderedContentType('text/htmlsomething')).toBe(true); - expect(isHtmlRenderedContentType('application/xhtml+xmlsomething')).toBe(true); - }); -}); - describe('getWebhookSandboxCSP', () => { it('should return correct CSP sandbox directive', () => { const csp = getWebhookSandboxCSP();
packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/GenericFunctions.ts+18 −11 modified@@ -37,20 +37,27 @@ export async function validateAuth(context: IWebhookFunctions) { } else if (authentication === 'n8nUserAuth') { const webhookName = context.getWebhookName(); - function getCookie(name: string) { - const value = `; ${headers.cookie}`; - const parts = value.split(`; ${name}=`); + if (webhookName !== 'setup') { + function getCookie(name: string) { + const value = `; ${headers.cookie}`; + const parts = value.split(`; ${name}=`); - if (parts.length === 2) { - return parts.pop()?.split(';').shift(); + if (parts.length === 2) { + return parts.pop()?.split(';').shift(); + } + return ''; } - return ''; - } - const authCookie = getCookie('n8n-auth'); - if (!authCookie && webhookName !== 'setup') { - // Data is not defined on node so can not authenticate - throw new ChatTriggerAuthorizationError(500, 'User not authenticated!'); + const authCookie = getCookie('n8n-auth'); + if (!authCookie) { + throw new ChatTriggerAuthorizationError(401, 'User not authenticated!'); + } + + try { + await context.validateCookieAuth(authCookie); + } catch { + throw new ChatTriggerAuthorizationError(401, 'Invalid authentication token'); + } } }
packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/templates.ts+1 −1 modified@@ -77,7 +77,7 @@ export function createPage({ : 'none'; const sanitizedShowWelcomeScreen = !!showWelcomeScreen; const sanitizedAllowFileUploads = !!allowFileUploads; - const sanitizedAllowedFilesMimeTypes = allowedFilesMimeTypes?.toString() ?? ''; + const sanitizedAllowedFilesMimeTypes = sanitizeUserInput(allowedFilesMimeTypes?.toString() ?? ''); const sanitizedCustomCss = sanitizeHtml(`<style>${customCss?.toString() ?? ''}</style>`, { allowedTags: ['style'], allowedAttributes: false,
packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/GenericFunctions.test.ts+156 −0 added@@ -0,0 +1,156 @@ +import { mock } from 'jest-mock-extended'; +import type { ICredentialDataDecryptedObject, IWebhookFunctions } from 'n8n-workflow'; + +import { ChatTriggerAuthorizationError } from '../error'; +import { validateAuth } from '../GenericFunctions'; + +describe('validateAuth', () => { + const mockContext = mock<IWebhookFunctions>(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('authentication = none', () => { + it('should pass without error', async () => { + mockContext.getNodeParameter.calledWith('authentication').mockReturnValue('none'); + + await expect(validateAuth(mockContext)).resolves.toBeUndefined(); + }); + }); + + describe('authentication = basicAuth', () => { + beforeEach(() => { + mockContext.getNodeParameter.calledWith('authentication').mockReturnValue('basicAuth'); + }); + + it('should throw 500 when credentials are not defined', async () => { + mockContext.getCredentials.mockRejectedValue(new Error('No credentials')); + + await expect(validateAuth(mockContext)).rejects.toThrow(ChatTriggerAuthorizationError); + await expect(validateAuth(mockContext)).rejects.toMatchObject({ + responseCode: 500, + }); + }); + + it('should throw 401 when no auth header is provided', async () => { + mockContext.getCredentials.mockResolvedValue({ + user: 'admin', + password: 'secret', + } as ICredentialDataDecryptedObject); + mockContext.getRequestObject.mockReturnValue({ + headers: {}, + } as never); + + await expect(validateAuth(mockContext)).rejects.toThrow(ChatTriggerAuthorizationError); + await expect(validateAuth(mockContext)).rejects.toMatchObject({ + responseCode: 401, + }); + }); + + it('should throw 403 when credentials are wrong', async () => { + mockContext.getCredentials.mockResolvedValue({ + user: 'admin', + password: 'secret', + } as ICredentialDataDecryptedObject); + mockContext.getRequestObject.mockReturnValue({ + headers: { + authorization: 'Basic ' + Buffer.from('admin:wrong').toString('base64'), + }, + } as never); + + await expect(validateAuth(mockContext)).rejects.toThrow(ChatTriggerAuthorizationError); + await expect(validateAuth(mockContext)).rejects.toMatchObject({ + responseCode: 403, + }); + }); + + it('should pass with correct credentials', async () => { + mockContext.getCredentials.mockResolvedValue({ + user: 'admin', + password: 'secret', + } as ICredentialDataDecryptedObject); + mockContext.getRequestObject.mockReturnValue({ + headers: { + authorization: 'Basic ' + Buffer.from('admin:secret').toString('base64'), + }, + } as never); + + await expect(validateAuth(mockContext)).resolves.toBeUndefined(); + }); + }); + + describe('authentication = n8nUserAuth', () => { + beforeEach(() => { + mockContext.getNodeParameter.calledWith('authentication').mockReturnValue('n8nUserAuth'); + }); + + it('should skip validation for setup webhook', async () => { + mockContext.getWebhookName.mockReturnValue('setup'); + mockContext.getHeaderData.mockReturnValue({}); + + await expect(validateAuth(mockContext)).resolves.toBeUndefined(); + }); + + it('should throw 401 when no n8n-auth cookie is present', async () => { + mockContext.getWebhookName.mockReturnValue('default'); + mockContext.getHeaderData.mockReturnValue({}); + + await expect(validateAuth(mockContext)).rejects.toThrow(ChatTriggerAuthorizationError); + await expect(validateAuth(mockContext)).rejects.toMatchObject({ + responseCode: 401, + message: 'User not authenticated!', + }); + }); + + it('should throw 401 when cookie has a fake/invalid token', async () => { + mockContext.getWebhookName.mockReturnValue('default'); + mockContext.getHeaderData.mockReturnValue({ + cookie: 'n8n-auth=anything', + }); + mockContext.validateCookieAuth.mockRejectedValue(new Error('Unauthorized')); + + await expect(validateAuth(mockContext)).rejects.toThrow(ChatTriggerAuthorizationError); + await expect(validateAuth(mockContext)).rejects.toMatchObject({ + responseCode: 401, + message: 'Invalid authentication token', + }); + }); + + it('should throw 401 when validateCookieAuth rejects (revoked token)', async () => { + mockContext.getWebhookName.mockReturnValue('default'); + mockContext.getHeaderData.mockReturnValue({ + cookie: 'n8n-auth=some.revoked.token', + }); + mockContext.validateCookieAuth.mockRejectedValue(new Error('Unauthorized')); + + await expect(validateAuth(mockContext)).rejects.toThrow(ChatTriggerAuthorizationError); + await expect(validateAuth(mockContext)).rejects.toMatchObject({ + responseCode: 401, + message: 'Invalid authentication token', + }); + }); + + it('should pass with a valid token', async () => { + mockContext.getWebhookName.mockReturnValue('default'); + mockContext.getHeaderData.mockReturnValue({ + cookie: 'n8n-auth=valid.jwt.token', + }); + mockContext.validateCookieAuth.mockResolvedValue(undefined); + + await expect(validateAuth(mockContext)).resolves.toBeUndefined(); + expect(mockContext.validateCookieAuth).toHaveBeenCalledWith('valid.jwt.token'); + }); + + it('should pass when cookie has other cookies alongside n8n-auth', async () => { + mockContext.getWebhookName.mockReturnValue('default'); + mockContext.getHeaderData.mockReturnValue({ + cookie: 'other=value; n8n-auth=valid.jwt.token; another=thing', + }); + mockContext.validateCookieAuth.mockResolvedValue(undefined); + + await expect(validateAuth(mockContext)).resolves.toBeUndefined(); + expect(mockContext.validateCookieAuth).toHaveBeenCalledWith('valid.jwt.token'); + }); + }); +});
packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/templates.test.ts+49 −0 modified@@ -15,6 +15,7 @@ describe('ChatTrigger Templates Security', () => { allowedFilesMimeTypes: '', customCss: '', enableStreaming: false, + initialMessages: '', }; describe('XSS Prevention in initialMessages', () => { @@ -213,6 +214,54 @@ describe('ChatTrigger Templates Security', () => { }); }); + describe('XSS Prevention in allowedFilesMimeTypes', () => { + it('should prevent script injection through allowedFilesMimeTypes', () => { + const maliciousInput = '</script><script>alert(document.cookie)</script>'; + + const result = createPage({ + ...defaultParams, + allowFileUploads: true, + allowedFilesMimeTypes: maliciousInput, + }); + + expect(result).not.toContain('<script>alert(document.cookie)</script>'); + expect(result).not.toContain('</script><script>'); + expect(result).not.toContain('alert(document.cookie)'); + }); + + it('should sanitize common XSS payloads in allowedFilesMimeTypes', () => { + const xssPayloads = [ + { input: '<img src=x onerror=alert(1)>', dangerous: ['onerror=', '<img'] }, + { input: '<svg onload=alert(1)>', dangerous: ['onload=', '<svg'] }, + { input: 'javascript:alert(1)', dangerous: ['javascript:'] }, + ]; + + xssPayloads.forEach(({ input, dangerous }) => { + const result = createPage({ + ...defaultParams, + allowFileUploads: true, + allowedFilesMimeTypes: input, + }); + + dangerous.forEach((dangerousContent) => { + expect(result).not.toContain(dangerousContent); + }); + }); + }); + + it('should preserve legitimate MIME types', () => { + const legitimateMimeTypes = 'image/*,text/plain,application/pdf'; + + const result = createPage({ + ...defaultParams, + allowFileUploads: true, + allowedFilesMimeTypes: legitimateMimeTypes, + }); + + expect(result).toContain(legitimateMimeTypes); + }); + }); + describe('getSanitizedInitialMessages function', () => { it('should sanitize XSS payloads', () => { const maliciousInput = '</script>"%09<script>alert(document.cookie)</script>';
packages/@n8n/task-runner-python/src/constants.py+1 −0 modified@@ -149,6 +149,7 @@ "obj", "__thisclass__", "__self_class__", + "__objclass__", # introspection attributes "__base__", "__class__",
packages/@n8n/task-runner-python/tests/unit/test_task_analyzer.py+13 −0 modified@@ -150,6 +150,19 @@ def test_name_mangled_attributes_blocked(self, analyzer: TaskAnalyzer) -> None: analyzer.validate(code) assert "name-mangled" in exc_info.value.description.lower() + def test_objclass_attribute_blocked(self, analyzer: TaskAnalyzer) -> None: + exploit_attempts = [ + "str.__or__.__objclass__", + "str.__init__.__objclass__", + "type_ref = str.__or__.__objclass__", + "object_ref = str.__init__.__objclass__", + ] + + for code in exploit_attempts: + with pytest.raises(SecurityViolationError) as exc_info: + analyzer.validate(code) + assert "__objclass__" in exc_info.value.description + def test_attribute_error_obj_blocked(self, analyzer: TaskAnalyzer) -> None: exploit_attempts = [ "e.obj",
packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts+48 −20 modified@@ -94,11 +94,40 @@ export interface JsTaskData { additionalData: PartialAdditionalData; } +type GlobalFunctionWithPrototype = ((...args: unknown[]) => unknown) & { + prototype?: object; +}; + type CustomConsole = { log: (...args: unknown[]) => void; }; export class JsTaskRunner extends TaskRunner { + private static readonly CONSOLE_METHODS = [ + 'log', + 'warn', + 'error', + 'info', + 'debug', + 'trace', + 'dir', + 'time', + 'timeEnd', + 'timeLog', + 'assert', + 'clear', + 'count', + 'countReset', + 'group', + 'groupEnd', + 'groupCollapsed', + 'table', + 'dirxml', + 'profile', + 'profileEnd', + 'timeStamp', + ] as const; + private readonly requireResolver: RequireResolver; private readonly builtInsParser = new BuiltInsParser(); @@ -161,15 +190,22 @@ export class JsTaskRunner extends TaskRunner { } } + // Overwrite unsafe Buffer allocations on the real constructor + const safeAlloc = Buffer.alloc.bind(Buffer); + Buffer.allocUnsafe = safeAlloc as typeof Buffer.allocUnsafe; + Buffer.allocUnsafeSlow = safeAlloc as typeof Buffer.allocUnsafeSlow; + // Freeze globals, except in tests because Jest needs to be able to mutate prototypes if (process.env.NODE_ENV !== 'test') { Object.getOwnPropertyNames(globalThis) - // @ts-expect-error globalThis does not have string in index signature - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - .map((name) => globalThis[name]) - .filter((value) => typeof value === 'function') - // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access - .forEach((fn) => Object.freeze(fn.prototype)); + .map((name) => Reflect.get(globalThis, name) as unknown) + .filter((value): value is GlobalFunctionWithPrototype => typeof value === 'function') + .forEach((fn) => { + if (typeof fn.prototype === 'object') Object.freeze(fn.prototype); + Object.freeze(fn); + }); + + [Reflect, JSON, Math].forEach(Object.freeze); } // Freeze internal classes @@ -239,19 +275,9 @@ export class JsTaskRunner extends TaskRunner { } private getNativeVariables() { - const { mode } = this; return { // Exposed Node.js globals - Buffer: new Proxy(Buffer, { - get(target, prop) { - if (mode === 'insecure') return target[prop as keyof typeof Buffer]; - if (prop === 'allocUnsafe' || prop === 'allocUnsafeSlow') { - // eslint-disable-next-line @typescript-eslint/unbound-method - return Buffer.alloc; - } - return target[prop as keyof typeof Buffer]; - }, - }), + Buffer, setTimeout, setInterval, setImmediate, @@ -278,7 +304,6 @@ export class JsTaskRunner extends TaskRunner { async runCode(settings: JSExecSettings, abortSignal: AbortSignal): Promise<unknown> { const context = createContext({ __isExecutionContext: true, - module: { exports: {} }, ...settings.additionalProperties, }); @@ -603,7 +628,7 @@ export class JsTaskRunner extends TaskRunner { private buildCustomConsole(taskId: string): CustomConsole { return { // all except `log` are dummy methods that disregard without throwing, following existing Code node behavior - ...Object.keys(console).reduce<Record<string, () => void>>((acc, name) => { + ...JsTaskRunner.CONSOLE_METHODS.reduce<Record<string, () => void>>((acc, name) => { acc[name] = noOp; return acc; }, {}), @@ -640,7 +665,6 @@ export class JsTaskRunner extends TaskRunner { return createContext({ __isExecutionContext: true, require: this.requireResolver, - module: {}, console: this.buildCustomConsole(taskId), $getWorkflowStaticData: (type: 'global' | 'node') => workflow.getStaticData(type, node), ...this.getNativeVariables(), @@ -654,6 +678,7 @@ export class JsTaskRunner extends TaskRunner { return [ // shim for `global` compatibility 'globalThis.global = globalThis', + 'var module = { exports: {} }', // prevent prototype manipulation 'Object.getPrototypeOf = () => ({})', @@ -673,6 +698,9 @@ export class JsTaskRunner extends TaskRunner { 'Object.defineProperty = () => ({})', 'Object.defineProperties = () => ({})', + // freeze constructors to prevent static method mutation + '[Object, Function, Array, String, Number, Boolean, RegExp, Error, TypeError, RangeError, SyntaxError, ReferenceError, Promise, Symbol, Map, Set, WeakMap, WeakSet, Date, JSON, Math, Reflect, ArrayBuffer, DataView, Int8Array, Uint8Array, Float32Array, Float64Array].forEach((constructor) => { try { Object.freeze(constructor); } catch {} })', + // wrap user code `module.exports = async function VmCodeWrapper() {${code}\n}()`, ].join('; ');
packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts+112 −0 modified@@ -151,6 +151,85 @@ describe('JsTaskRunner', () => { expect(outcome.result).toEqual([wrapIntoJson({ allZeros: true })]); }); + + it('should prevent bypass via Object.getOwnPropertyDescriptor', async () => { + const outcome = await executeForAllItems({ + code: ` + const desc = Object.getOwnPropertyDescriptor(Buffer, 'allocUnsafe'); + const fn = desc && desc.value; + const buf = fn(10); + return [{ json: { allZeros: buf.every(b => b === 0) } }] + `, + inputItems: [{ a: 1 }], + }); + + expect(outcome.result).toEqual([wrapIntoJson({ allZeros: true })]); + }); + + it('should prevent bypass via Object.getOwnPropertyDescriptor for allocUnsafeSlow', async () => { + const outcome = await executeForAllItems({ + code: ` + const desc = Object.getOwnPropertyDescriptor(Buffer, 'allocUnsafeSlow'); + const fn = desc && desc.value; + const buf = fn(10); + return [{ json: { allZeros: buf.every(b => b === 0) } }] + `, + inputItems: [{ a: 1 }], + }); + + expect(outcome.result).toEqual([wrapIntoJson({ allZeros: true })]); + }); + + it('should prevent bypass via Buffer.from([]).constructor.allocUnsafe', async () => { + const outcome = await executeForAllItems({ + code: ` + const RealBuffer = Buffer.from([]).constructor; + const buf = RealBuffer.allocUnsafe(100); + return [{ json: { allZeros: buf.every(b => b === 0) } }] + `, + inputItems: [{ a: 1 }], + }); + + expect(outcome.result).toEqual([wrapIntoJson({ allZeros: true })]); + }); + + it('should support Buffer.from and Buffer.concat', async () => { + const outcome = await executeForAllItems({ + code: ` + const buf1 = Buffer.from('hello'); + const buf2 = Buffer.from(' world'); + const combined = Buffer.concat([buf1, buf2]); + return [{ json: { result: combined.toString() } }] + `, + inputItems: [{ a: 1 }], + }); + + expect(outcome.result).toEqual([wrapIntoJson({ result: 'hello world' })]); + }); + + it('should support Buffer.isBuffer', async () => { + const outcome = await executeForAllItems({ + code: ` + const buf = Buffer.from('test'); + return [{ json: { isBuf: Buffer.isBuffer(buf), notBuf: Buffer.isBuffer('str') } }] + `, + inputItems: [{ a: 1 }], + }); + + expect(outcome.result).toEqual([wrapIntoJson({ isBuf: true, notBuf: false })]); + }); + + it('should support instanceof Buffer', async () => { + const outcome = await executeForAllItems({ + code: ` + const buf = Buffer.from('test'); + return [{ json: { isInstance: buf instanceof Buffer } }] + `, + inputItems: [{ a: 1 }], + }); + + expect(outcome.result).toEqual([wrapIntoJson({ isInstance: true })]); + }); }); describe('console', () => { @@ -1476,6 +1555,39 @@ describe('JsTaskRunner', () => { expect(Duration.fromObject({ hours: 1 }).maliciousKey).toBeUndefined(); }); + test('should prevent overwriting Object.keys via module.constructor', async () => { + const outcome = await executeForAllItems({ + code: ` + module.constructor.keys = () => ['polluted']; + return [{ json: { keyCount: Object.keys({ a: 1, b: 2 }).length } }]; + `, + inputItems: [{ a: 1 }], + }); + + expect(outcome.result).toEqual([wrapIntoJson({ keyCount: 2 })]); + }); + + test('should keep module.constructor in the sandbox realm', async () => { + const outcome = await executeForAllItems({ + code: ` + return [{ + json: { + isSandboxObject: module.constructor === Object, + isFrozen: Object.isFrozen(module.constructor), + }, + }]; + `, + inputItems: [{ a: 1 }], + }); + + expect(outcome.result).toEqual([ + wrapIntoJson({ + isSandboxObject: true, + isFrozen: true, + }), + ]); + }); + it('should allow prototype mutation when `insecureMode` is true', async () => { const runner = createRunnerWithOpts({ insecureMode: true,
packages/nodes-base/nodes/Form/Form.node.ts+17 −21 modified@@ -8,19 +8,14 @@ import type { IWebhookResponseData, } from 'n8n-workflow'; import { - Node, - updateDisplayOptions, - NodeOperationError, FORM_NODE_TYPE, FORM_TRIGGER_NODE_TYPE, - tryToParseJsonToFormFields, + Node, NodeConnectionTypes, + NodeOperationError, + updateDisplayOptions, } from 'n8n-workflow'; -import { cssVariables } from './cssVariables'; -import { renderFormCompletion } from './utils/formCompletionUtils'; -import { getFormTriggerNode, renderFormNode } from './utils/formNodeUtils'; -import { prepareFormReturnItem, resolveRawData } from './utils/utils'; import { configureWaitTillDate } from '../../utils/sendAndWait/configureWaitTillDate.util'; import { limitWaitTimeProperties } from '../../utils/sendAndWait/descriptions'; import { @@ -29,6 +24,10 @@ import { formFieldsDynamic, formTitle, } from '../Form/common.descriptions'; +import { cssVariables } from './cssVariables'; +import { renderFormCompletion } from './utils/formCompletionUtils'; +import { getFormTriggerNode, renderFormNode } from './utils/formNodeUtils'; +import { parseFormFields, prepareFormReturnItem } from './utils/utils'; const waitTimeProperties: INodeProperties[] = [ { @@ -366,20 +365,17 @@ export class Form extends Node { let fields: FormFieldsParameter = []; if (defineForm === 'json') { - try { - const jsonOutput = context.getNodeParameter('jsonOutput', '', { - rawExpressions: true, - }) as string; - - fields = tryToParseJsonToFormFields(resolveRawData(context, jsonOutput)); - } catch (error) { - throw new NodeOperationError(context.getNode(), error.message, { - description: error.message, - type: mode === 'test' ? 'manual-form-test' : undefined, - }); - } + fields = parseFormFields(context, { + defineForm: 'json', + fieldsParameterName: 'jsonOutput', + mode, + }); } else { - fields = context.getNodeParameter('formFields.values', []) as FormFieldsParameter; + fields = parseFormFields(context, { + defineForm: 'fields', + fieldsParameterName: 'formFields.values', + mode, + }); } const method = context.getRequestObject().method;
packages/nodes-base/nodes/Form/test/Form.node.test.ts+77 −0 modified@@ -408,6 +408,83 @@ describe('Form Node', () => { ); }); + it.each(['json', 'fields'])( + 'should evaluate expressions only once in %s mode, preserving nested braces', + async (defineForm) => { + const formFields = [ + { + fieldLabel: 'Custom HTML', + fieldType: 'html', + elementName: 'test', + html: '<h2>Hello {{ $json.world }} </h2>', + }, + ]; + + const mockResponseObject = { + render: jest.fn(), + setHeader: jest.fn(), + }; + mockWebhookFunctions.getResponseObject.mockReturnValue( + mockResponseObject as unknown as Response, + ); + mockWebhookFunctions.getRequestObject.mockReturnValue({ method: 'GET' } as Request); + mockWebhookFunctions.getParentNodes.mockReturnValue([ + { + type: 'n8n-nodes-base.formTrigger', + name: 'Form Trigger', + typeVersion: 2.1, + disabled: false, + }, + ]); + mockWebhookFunctions.evaluateExpression.mockImplementation((expression: string) => { + console.log('expression', expression); + if (expression.includes('formMode')) { + return 'test'; + } + if (expression === '{{ $json.world }}') { + return "{{ 'World' }}"; + } + if (expression === "{{ 'World' }}") { + fail('Should not be called'); + } + return expression; + }); + mockWebhookFunctions.getNode.mockReturnValue(mock<INode>()); + mockWebhookFunctions.getNodeParameter.mockImplementation((paramName: string) => { + if (paramName === 'operation') return 'page'; + if (paramName === 'defineForm') return defineForm; + if (paramName === 'jsonOutput') return `=${JSON.stringify(formFields)}`; + if (paramName === 'formFields.values') return formFields; + if (paramName === 'options') { + return { + formTitle: 'Form Title', + formDescription: 'Form Description', + buttonLabel: 'Form Button', + }; + } + return undefined; + }); + + mockWebhookFunctions.getChildNodes.mockReturnValue([]); + + await form.webhook(mockWebhookFunctions); + + expect(mockWebhookFunctions.evaluateExpression).not.toHaveBeenCalledWith("{{ 'World' }}"); + expect(mockResponseObject.render).toHaveBeenCalledWith( + 'form-trigger', + expect.objectContaining({ + formFields: expect.arrayContaining([ + expect.objectContaining({ + html: "<h2>Hello {{ 'World' }} </h2>", + }), + ]), + }), + ); + }, + ); + }); + + describe('webhook method - completion and redirect', () => { it('should handle completion operation and redirect', async () => { mockWebhookFunctions.getRequestObject.mockReturnValue({ method: 'GET' } as Request); mockWebhookFunctions.getNodeParameter.mockImplementation((paramName) => {
packages/nodes-base/nodes/Form/test/utils.test.ts+185 −16 modified@@ -27,6 +27,7 @@ import { addFormResponseDataToReturnItem, validateSafeRedirectUrl, handleNewlines, + parseFormFields, } from '../utils/utils'; import { isIpAllowed } from '../../Webhook/utils'; @@ -2498,37 +2499,71 @@ describe('validateResponseModeConfiguration', () => { }); describe('prepareFormFields', () => { - it('should resolve expressions in html fields', async () => { - webhookFunctions.evaluateExpression.mockImplementation((expression) => { - if (expression === '{{ $json.formMode }}') { - return 'Title'; - } + it('should prepare hiddenField', async () => { + const result = prepareFormFields([ + { + fieldLabel: '', + fieldName: 'test', + fieldType: 'hiddenField', + }, + ]); + + expect(result[0]).toEqual({ + fieldLabel: 'test', + fieldName: 'test', + fieldType: 'hiddenField', }); + }); - const result = prepareFormFields(webhookFunctions, [ + it('should sanitize html fields', async () => { + const result = prepareFormFields([ { fieldLabel: 'Custom HTML', fieldType: 'html', elementName: 'test', - html: '<h1>{{ $json.formMode }}</h1>', + html: '<div>Safe content</div><script>alert("XSS")</script>', }, ]); - expect(result[0].html).toBe('<h1>Title</h1>'); + expect(result[0].html).toBe('<div>Safe content</div>'); }); - it('should prepare hiddenField', async () => { - const result = prepareFormFields(webhookFunctions, [ + + it('should not modify html fields when html is empty', async () => { + const result = prepareFormFields([ { - fieldLabel: '', - fieldName: 'test', - fieldType: 'hiddenField', + fieldLabel: 'Custom HTML', + fieldType: 'html', + elementName: 'test', + html: '', + }, + ]); + + expect(result[0].html).toBe(''); + }); + + it('should not modify html fields when html is undefined', async () => { + const result = prepareFormFields([ + { + fieldLabel: 'Custom HTML', + fieldType: 'html', + elementName: 'test', + }, + ]); + + expect(result[0].html).toBeUndefined(); + }); + + it('should not process non-html fields', async () => { + const result = prepareFormFields([ + { + fieldLabel: 'Text Field', + fieldType: 'text', }, ]); expect(result[0]).toEqual({ - fieldLabel: 'test', - fieldName: 'test', - fieldType: 'hiddenField', + fieldLabel: 'Text Field', + fieldType: 'text', }); }); }); @@ -2875,3 +2910,137 @@ describe('handleNewlines', () => { expect(result).toBe(expected); }); }); + +describe('parseFormFields - HTML field expression resolution', () => { + let mockWebhookFunctions: ReturnType<typeof mock<IWebhookFunctions>>; + + beforeEach(() => { + mockWebhookFunctions = mock<IWebhookFunctions>(); + }); + + it('should resolve expressions in html fields', () => { + mockWebhookFunctions.getNodeParameter.mockImplementation((paramName: string) => { + if (paramName === 'formFields.values') { + return [ + { + fieldLabel: 'Custom HTML', + fieldType: 'html', + elementName: 'test', + html: '<h1>{{ $json.formMode }}</h1>', + }, + ]; + } + return undefined; + }); + + mockWebhookFunctions.evaluateExpression.mockImplementation((expression: string) => { + if (expression === '{{ $json.formMode }}') { + return 'Title'; + } + if (expression.includes('formMode')) { + return 'test'; + } + return expression; + }); + + const result = parseFormFields(mockWebhookFunctions, { + defineForm: 'fields', + fieldsParameterName: 'formFields.values', + }); + + expect(mockWebhookFunctions.evaluateExpression).toHaveBeenCalledWith('{{ $json.formMode }}'); + expect(result[0].html).toBe('<h1>Title</h1>'); + }); + + it('should handle multiple expressions in html fields', () => { + mockWebhookFunctions.getNodeParameter.mockImplementation((paramName: string) => { + if (paramName === 'formFields.values') { + return [ + { + fieldLabel: 'Custom HTML', + fieldType: 'html', + elementName: 'test', + html: '<h1>{{ $json.title }}</h1><p>{{ $json.description }}</p>', + }, + ]; + } + return undefined; + }); + + mockWebhookFunctions.evaluateExpression.mockImplementation((expression: string) => { + if (expression === '{{ $json.title }}') return 'Welcome'; + if (expression === '{{ $json.description }}') return 'Please fill out the form'; + if (expression.includes('formMode')) { + return 'test'; + } + return expression; + }); + + const result = parseFormFields(mockWebhookFunctions, { + defineForm: 'fields', + fieldsParameterName: 'formFields.values', + }); + + expect(result[0].html).toBe('<h1>Welcome</h1><p>Please fill out the form</p>'); + }); + + it('should not modify html fields without expressions', () => { + mockWebhookFunctions.getNodeParameter.mockImplementation((paramName: string) => { + if (paramName === 'formFields.values') { + return [ + { + fieldLabel: 'Custom HTML', + fieldType: 'html', + elementName: 'test', + html: '<h1>Static Title</h1>', + }, + ]; + } + return undefined; + }); + + mockWebhookFunctions.evaluateExpression.mockImplementation((expression: string) => { + if (expression.includes('formMode')) { + return 'test'; + } + return expression; + }); + + const result = parseFormFields(mockWebhookFunctions, { + defineForm: 'fields', + fieldsParameterName: 'formFields.values', + }); + + expect(result[0].html).toBe('<h1>Static Title</h1>'); + }); + + it('should handle empty html fields', () => { + mockWebhookFunctions.getNodeParameter.mockImplementation((paramName: string) => { + if (paramName === 'formFields.values') { + return [ + { + fieldLabel: 'Custom HTML', + fieldType: 'html', + elementName: 'test', + html: '', + }, + ]; + } + return undefined; + }); + + mockWebhookFunctions.evaluateExpression.mockImplementation((expression: string) => { + if (expression.includes('formMode')) { + return 'test'; + } + return expression; + }); + + const result = parseFormFields(mockWebhookFunctions, { + defineForm: 'fields', + fieldsParameterName: 'formFields.values', + }); + + expect(result[0].html).toBe(''); + }); +});
packages/nodes-base/nodes/Form/utils/utils.ts+40 −13 modified@@ -20,6 +20,7 @@ import { jsonParse, tryToParseUrl, BINARY_MODE_COMBINED, + tryToParseJsonToFormFields, } from 'n8n-workflow'; import * as a from 'node:assert'; import sanitize from 'sanitize-html'; @@ -116,20 +117,11 @@ export const handleNewlines = (text: string) => { return text.replace(/\\n|\\\\n/g, (match) => (match === '\\\\n' ? '\\n' : '\n')); }; -export const prepareFormFields = (context: IWebhookFunctions, fields: FormFieldsParameter) => { +export const prepareFormFields = (fields: FormFieldsParameter) => { return fields.map((field) => { - if (field.fieldType === 'html') { - let { html } = field; - - if (!html) return field; - - for (const resolvable of getResolvables(html)) { - html = html.replace(resolvable, context.evaluateExpression(resolvable) as string); - } - - field.html = sanitizeHtml(html); + if (field.fieldType === 'html' && field.html) { + field.html = sanitizeHtml(field.html); } - if (field.fieldType === 'hiddenField') { field.fieldLabel = field.fieldName as string; } @@ -552,7 +544,7 @@ export function renderForm({ } catch (error) {} } - formFields = prepareFormFields(context, formFields); + formFields = prepareFormFields(formFields); const data = prepareFormData({ formTitle, @@ -743,3 +735,38 @@ export function resolveRawData(context: IWebhookFunctions, rawData: string) { } return returnData; } + +type ParseFormFieldsOptions = { + defineForm: 'json' | 'fields'; + fieldsParameterName: string; + mode?: 'test' | 'production'; +}; +export function parseFormFields(context: IWebhookFunctions, options: ParseFormFieldsOptions) { + let fields: FormFieldsParameter = []; + if (options.defineForm === 'json') { + try { + const jsonOutput = context.getNodeParameter(options.fieldsParameterName, '', { + rawExpressions: true, + }) as string; + + fields = tryToParseJsonToFormFields(resolveRawData(context, jsonOutput)); + } catch (error) { + throw new NodeOperationError(context.getNode(), error.message, { + description: error.message, + type: options.mode === 'test' ? 'manual-form-test' : undefined, + }); + } + } else { + fields = context.getNodeParameter(options.fieldsParameterName, []) as FormFieldsParameter; + for (const field of fields) { + if (field.fieldType === 'html') { + let html = field.html ?? ''; + for (const resolvable of getResolvables(html)) { + html = html.replace(resolvable, context.evaluateExpression(resolvable) as string); + } + field.html = html; + } + } + } + return fields; +}
packages/nodes-base/nodes/Merge/test/v3/combineBtSql.test.ts+186 −0 added@@ -0,0 +1,186 @@ +import type { AlaSQLExtended } from '../../v3/actions/mode/combineBySql'; +import { disableUnsafeAccess, freezeAlasql } from '../../v3/actions/mode/combineBySql'; + +describe('combineBySql security functions', () => { + describe('disableUnsafeAccess', () => { + it('should disable all file access operations on a fresh instance', async () => { + // Load a fresh AlaSQL instance for this test + const alasqlModule = await import('alasql'); + const mockAlasql = (alasqlModule.default || alasqlModule) as AlaSQLExtended; + + disableUnsafeAccess(mockAlasql); + + // Test FROM handlers are disabled + const fromHandlers = ['FILE', 'JSON', 'CSV', 'XLSX', 'HTML']; + fromHandlers.forEach((handler) => { + if (mockAlasql.from?.[handler]) { + // @ts-expect-error - mockAlasql.from is of type Record<string, unknown> + expect(() => mockAlasql.from).toThrow( + 'File access operations are disabled for security reasons', + ); + } + }); + + // Test INTO handlers are disabled + const intoHandlers = ['FILE', 'JSON', 'CSV', 'XLSX', 'HTML']; + intoHandlers.forEach((handler) => { + if (mockAlasql.into?.[handler]) { + // @ts-expect-error - mockAlasql.into is of type Record<string, unknown> + expect(() => mockAlasql.into).toThrow( + 'File access operations are disabled for security reasons', + ); + } + }); + + // Test file-based database engines are disabled + const engines = ['FILE', 'FILESTORAGE', 'LOCALSTORAGE', 'INDEXEDDB', 'SQLITE']; + engines.forEach((engine) => { + if (mockAlasql.engines?.[engine]) { + // @ts-expect-error - mockAlasql.engines is of type Record<string, unknown> + expect(() => mockAlasql.engines).toThrow( + 'File access operations are disabled for security reasons', + ); + } + }); + + // Test file system utility functions are disabled + const utils = [ + 'loadFile', + 'loadBinaryFile', + 'saveFile', + 'removeFile', + 'deleteFile', + 'fileExists', + 'require', + ]; + utils.forEach((util) => { + if (mockAlasql.utils?.[util]) { + // @ts-expect-error - mockAlasql.utils is of type Record<string, unknown> + expect(() => mockAlasql.utils).toThrow( + 'File access operations are disabled for security reasons', + ); + } + }); + + // Test fn handlers are disabled + const fnHandlers = ['FILE', 'JSON', 'TXT', 'CSV', 'XLSX', 'XLS', 'LOAD', 'SAVE', 'REQUIRE']; + fnHandlers.forEach((handler) => { + if (mockAlasql.fn?.[handler]) { + expect(() => mockAlasql.fn).toThrow( + 'File access operations are disabled for security reasons', + ); + } + }); + + // Test that fn is frozen + expect(Object.isFrozen(mockAlasql.fn)).toBe(true); + + // Test REQUIRE statement execution is disabled + if (mockAlasql.yy?.Require?.prototype?.execute) { + expect(() => mockAlasql.yy!.Require!.prototype!.execute!()).toThrow( + 'File access operations are disabled for security reasons', + ); + } + }); + + it('should handle missing optional properties gracefully', async () => { + // Load a fresh instance + const alasqlModule = await import('alasql'); + const baseAlasql = (alasqlModule.default || alasqlModule) as AlaSQLExtended; + + // Create a minimal mock that's missing some optional properties + const minimalAlasql = { + ...baseAlasql, + from: undefined, + into: undefined, + engines: undefined, + utils: undefined, + fn: undefined, + yy: undefined, + } as unknown as AlaSQLExtended; + + // Should not throw when properties are missing + expect(() => disableUnsafeAccess(minimalAlasql)).not.toThrow(); + }); + }); + + describe('freezeAlasql', () => { + it('should freeze fn object', async () => { + // Load a fresh AlaSQL instance for this test + const alasqlModule = await import('alasql'); + const mockAlasql = (alasqlModule.default || alasqlModule) as AlaSQLExtended; + + freezeAlasql(mockAlasql); + + expect(Object.isFrozen(mockAlasql.fn)).toBe(true); + }); + + it('should freeze yy object if present', async () => { + // Load a fresh AlaSQL instance for this test + const alasqlModule = await import('alasql'); + const mockAlasql = (alasqlModule.default || alasqlModule) as AlaSQLExtended; + + freezeAlasql(mockAlasql); + + if (mockAlasql.yy) { + expect(Object.isFrozen(mockAlasql.yy)).toBe(true); + } + }); + + it('should prevent modifications to fn after freezing', async () => { + // Load a fresh AlaSQL instance for this test + const alasqlModule = await import('alasql'); + const mockAlasql = (alasqlModule.default || alasqlModule) as AlaSQLExtended; + + freezeAlasql(mockAlasql); + + expect(() => { + (mockAlasql.fn as Record<string, unknown>).newFunction = () => {}; + }).toThrow(); + }); + + it('should prevent modifications to yy after freezing', async () => { + // Load a fresh AlaSQL instance for this test + const alasqlModule = await import('alasql'); + const mockAlasql = (alasqlModule.default || alasqlModule) as AlaSQLExtended; + + freezeAlasql(mockAlasql); + + if (mockAlasql.yy) { + expect(() => { + (mockAlasql.yy as Record<string, unknown>).newProperty = {}; + }).toThrow(); + } + }); + + it('should handle missing yy object gracefully', async () => { + // Load a fresh AlaSQL instance for this test + const alasqlModule = await import('alasql'); + const baseAlasql = (alasqlModule.default || alasqlModule) as AlaSQLExtended; + + // Create a mock without yy property + const mockAlasql = { + ...baseAlasql, + yy: undefined, + } as unknown as AlaSQLExtended; + + // Should not throw when yy is missing + expect(() => freezeAlasql(mockAlasql)).not.toThrow(); + expect(Object.isFrozen(mockAlasql.fn)).toBe(true); + }); + + it('should freeze both fn and yy in one call', async () => { + // Load a fresh AlaSQL instance for this test + const alasqlModule = await import('alasql'); + const mockAlasql = (alasqlModule.default || alasqlModule) as AlaSQLExtended; + + freezeAlasql(mockAlasql); + + // Verify both are frozen after calling freezeAlasql + expect(Object.isFrozen(mockAlasql.fn)).toBe(true); + if (mockAlasql.yy) { + expect(Object.isFrozen(mockAlasql.yy)).toBe(true); + } + }); + }); +});
packages/nodes-base/nodes/Merge/v3/actions/mode/combineBySql.ts+75 −10 modified@@ -1,5 +1,4 @@ import { Container } from '@n8n/di'; -import alasqlImport from 'alasql'; import { ErrorReporter } from 'n8n-core'; import type { @@ -16,25 +15,43 @@ import { getResolvables, updateDisplayOptions } from '@utils/utilities'; import { numberInputsProperty } from '../../helpers/descriptions'; import { modifySelectQuery, rowToExecutionData } from '../../helpers/utils'; -type AlaSQLBase = typeof alasqlImport; -type AlaSQLExtended = AlaSQLBase & { +// Type for AlaSQL - use type-only import to avoid runtime import +// We import the type statically for type checking, but use dynamic import at runtime +import type AlasqlType from 'alasql'; +type AlaSQLBase = typeof AlasqlType; +export type AlaSQLExtended = AlaSQLBase & { // Access `engines` internal structure to override file access engines engines?: Record<string, unknown>; // Access `into` handlers to override file write operations into?: Record<string, unknown>; // Access `utils` for utility functions utils?: Record<string, unknown>; + // Access `yy` for statement types like REQUIRE + yy?: { + Require?: { + prototype?: { + execute?: (...args: unknown[]) => unknown; + }; + }; + [key: string]: unknown; + }; // Fix Database constructor typing Database: AlaSQLBase['Database'] & { new (databaseId: string): AlaSQLBase['Database'] }; }; -const alasql = alasqlImport as AlaSQLExtended; +// Cache for the loaded and secured alasql instance +let cachedAlaSql: AlaSQLExtended | null = null; -function disableAlasqlFileAccess() { - const disabledFunction = () => { - throw new Error('File access operations are disabled for security reasons'); - }; +// Export for testing - allows resetting the cache +export function resetAlaSqlCache() { + cachedAlaSql = null; +} + +const disabledFunction = () => { + throw new Error('File access operations are disabled for security reasons'); +}; +export function disableUnsafeAccess(alasql: AlaSQLExtended) { // Block ALL FROM handlers that can read files or external resources if (alasql.from) { const fromHandlers = [ @@ -109,15 +126,60 @@ function disableAlasqlFileAccess() { alasql.utils.removeFile = disabledFunction; alasql.utils.deleteFile = disabledFunction; alasql.utils.fileExists = disabledFunction; + alasql.utils.require = disabledFunction; } // Block fn handlers if present if (alasql.fn) { - const fnHandlers = ['FILE', 'JSON', 'TXT', 'CSV', 'XLSX', 'XLS', 'LOAD', 'SAVE']; + const fnHandlers = ['FILE', 'JSON', 'TXT', 'CSV', 'XLSX', 'XLS', 'LOAD', 'SAVE', 'REQUIRE']; fnHandlers.forEach((handler) => { alasql.fn[handler] = disabledFunction; }); + alasql.fn = Object.freeze(alasql.fn); } + + if (alasql.yy) { + alasql.yy.JavaScript = disabledFunction; + } + + // Block REQUIRE statement execution + // REQUIRE is a statement type (like SELECT, INSERT) that can load and execute arbitrary code + // We need to override yy.Require.prototype.execute before freezing + // The yy object contains statement constructors and is accessible via alasql.yy + if (alasql.yy?.Require?.prototype) { + alasql.yy.Require.prototype.execute = disabledFunction; + } +} + +export function freezeAlasql(alasql: AlaSQLExtended) { + /* + * we freeze these elements of alasql to avoid users being able to create new functions as part of an execution + * by creating and immediately executing functions with CREATE FUNCTION or with AGGREGATE manipulation. + */ + alasql.fn = Object.freeze(alasql.fn); + if (alasql.yy) { + alasql.yy = Object.freeze(alasql.yy); + } +} + +/** + * Lazy loads AlaSQL, disables file access, and freezes it in one go. + * This ensures AlaSQL is only loaded when needed and is immediately secured. + */ +export async function loadAlaSql(): Promise<AlaSQLExtended> { + if (cachedAlaSql) { + return cachedAlaSql; + } + + const alasqlImport = await import('alasql'); + const alasql = (alasqlImport.default || alasqlImport) as AlaSQLExtended; + + disableUnsafeAccess(alasql); + freezeAlasql(alasql); + + cachedAlaSql = alasql; + + return alasql; } type OperationOptions = { @@ -200,6 +262,7 @@ async function executeSelectWithMappedPairedItems( inputsData: INodeExecutionData[][], query: string, returnSuccessItemIfEmpty: boolean, + alasql: AlaSQLExtended, ): Promise<INodeExecutionData[][]> { const returnData: INodeExecutionData[] = []; @@ -250,7 +313,8 @@ export async function execute( this: IExecuteFunctions, inputsData: INodeExecutionData[][], ): Promise<INodeExecutionData[][]> { - disableAlasqlFileAccess(); + // Lazy load AlaSQL, disable file access, and freeze it + const alasql = await loadAlaSql(); const node = this.getNode(); const returnData: INodeExecutionData[] = []; @@ -274,6 +338,7 @@ export async function execute( inputsData, query, returnSuccessItemIfEmpty, + alasql, ); } catch (error) { Container.get(ErrorReporter).error(error, {
packages/nodes-base/utils/sendAndWait/utils.ts+22 −31 modified@@ -1,19 +1,23 @@ import isbot from 'isbot'; import { getWebhookSandboxCSP } from 'n8n-core'; -import { - NodeOperationError, - SEND_AND_WAIT_OPERATION, - tryToParseJsonToFormFields, - updateDisplayOptions, -} from 'n8n-workflow'; import type { - INodeProperties, + FormFieldsParameter, + IDataObject, IExecuteFunctions, + INodeProperties, IWebhookFunctions, - IDataObject, - FormFieldsParameter, } from 'n8n-workflow'; +import { NodeOperationError, SEND_AND_WAIT_OPERATION, updateDisplayOptions } from 'n8n-workflow'; +import { cssVariables } from '../../nodes/Form/cssVariables'; +import { formFieldsProperties } from '../../nodes/Form/Form.node'; +import { + parseFormFields, + prepareFormData, + prepareFormFields, + prepareFormReturnItem, +} from '../../nodes/Form/utils/utils'; +import { escapeHtml } from '../utilities'; import { limitWaitTimeOption } from './descriptions'; import { ACTION_RECORDED_PAGE, @@ -23,15 +27,6 @@ import { createEmailBodyWithoutN8nAttribution, } from './email-templates'; import type { IEmail } from './interfaces'; -import { cssVariables } from '../../nodes/Form/cssVariables'; -import { formFieldsProperties } from '../../nodes/Form/Form.node'; -import { - prepareFormData, - prepareFormFields, - prepareFormReturnItem, - resolveRawData, -} from '../../nodes/Form/utils/utils'; -import { escapeHtml } from '../utilities'; export type SendAndWaitConfig = { title: string; @@ -400,26 +395,22 @@ export async function sendAndWaitWebhook(this: IWebhookFunctions) { let fields: FormFieldsParameter = []; if (defineForm === 'json') { - try { - const jsonOutput = this.getNodeParameter('jsonOutput', '', { - rawExpressions: true, - }) as string; - - fields = tryToParseJsonToFormFields(resolveRawData(this, jsonOutput)); - } catch (error) { - throw new NodeOperationError(this.getNode(), error.message, { - description: error.message, - }); - } + fields = parseFormFields(this, { + defineForm: 'json', + fieldsParameterName: 'jsonOutput', + }); } else { - fields = this.getNodeParameter('formFields.values', []) as FormFieldsParameter; + fields = parseFormFields(this, { + defineForm: 'fields', + fieldsParameterName: 'formFields.values', + }); } if (method === 'GET') { const { formTitle, formDescription, buttonLabel, customCss } = getFormResponseCustomizations(this); - fields = prepareFormFields(this, fields); + fields = prepareFormFields(fields); const data = prepareFormData({ formTitle,
packages/workflow/src/expression-sandboxing.ts+200 −12 modified@@ -17,10 +17,69 @@ const DATA_NODE_NAME = '___n8n_data'; const RESERVED_VARIABLE_NAMES = new Set([DATA_NODE_NAME, sanitizerName]); +type AstNode = { type: string } & Record<string, unknown>; + +const isAstNode = (value: unknown): value is AstNode => + typeof value === 'object' && value !== null && 'type' in value && typeof value.type === 'string'; + +const getBoundIdentifiers = (node: unknown, acc: string[] = []): string[] => { + if (!isAstNode(node)) return acc; + + switch (node.type) { + case 'Identifier': { + if (typeof node.name === 'string') acc.push(node.name); + break; + } + case 'ObjectPattern': { + if (!Array.isArray(node.properties)) break; + for (const property of node.properties) { + if (!isAstNode(property)) continue; + if (property.type === 'Property') { + getBoundIdentifiers(property.value, acc); + } else if (property.type === 'RestElement') { + getBoundIdentifiers(property.argument, acc); + } + } + break; + } + case 'ArrayPattern': { + if (!Array.isArray(node.elements)) break; + for (const element of node.elements) { + getBoundIdentifiers(element, acc); + } + break; + } + case 'AssignmentPattern': { + getBoundIdentifiers(node.left, acc); + break; + } + case 'RestElement': { + getBoundIdentifiers(node.argument, acc); + break; + } + case 'VariableDeclaration': { + if (!Array.isArray(node.declarations)) break; + for (const declaration of node.declarations) { + if (!isAstNode(declaration) || declaration.type !== 'VariableDeclarator') continue; + getBoundIdentifiers(declaration.id, acc); + } + break; + } + } + + return acc; +}; + +const getReservedIdentifier = (node: unknown): string | undefined => + getBoundIdentifiers(node).find((name) => RESERVED_VARIABLE_NAMES.has(name)); + export const DOLLAR_SIGN_ERROR = 'Cannot access "$" without calling it as a function'; const EMPTY_CONTEXT = b.objectExpression([ b.property('init', b.identifier('process'), b.objectExpression([])), + b.property('init', b.identifier('require'), b.objectExpression([])), + b.property('init', b.identifier('module'), b.objectExpression([])), + b.property('init', b.identifier('Buffer'), b.objectExpression([])), ]); const SAFE_GLOBAL = b.objectExpression([]); @@ -70,6 +129,8 @@ const isValidDollarPropertyAccess = (expr: unknown): boolean => { const GLOBAL_IDENTIFIERS = new Set(['globalThis']); +const BLOCKED_SPREAD_GLOBALS = new Set(['process', 'global', 'globalThis', 'Buffer']); + /** * Prevents regular functions from binding their `this` to the Node.js global. */ @@ -247,24 +308,75 @@ const blockedBaseClasses = new Set([ 'AsyncGeneratorFunction', ]); +/** + * Builds an AST node that safely resolves a spread argument like `...process`. + * + * Tournament's VariablePolyfill rewrites plain identifiers (e.g. `process`) + * to look them up from the data context, but it does NOT handle identifiers + * inside SpreadElement / SpreadProperty nodes. Without this fix, `{...process}` + * would resolve to the real Node.js `process` object. + * + * The generated code checks the data context first, falling back to a throw: + * + * ("process" in data) ? data.process : (() => { throw new Error("...") })() + * + * - If the workflow has a variable called "process" → spread that (safe, user-defined) + * - Otherwise → throw at runtime, blocking access to the real global + */ +const buildSafeSpreadArg = (name: string, dataNode: Parameters<ASTAfterHook>[1]) => { + // "process" in ___n8n_data + const isInDataContext = b.binaryExpression('in', b.literal(name), dataNode); + + // ___n8n_data.process + const readFromDataContext = b.memberExpression(dataNode, b.identifier(name)); + + // (() => { throw new Error('Cannot spread "process" ...') })() + // + // This is an IIFE because `throw` is a statement, not an expression, + // so it cannot appear directly inside a ternary's falsy branch. + const throwSecurityError = b.callExpression( + b.arrowFunctionExpression( + [], + b.blockStatement([ + b.throwStatement( + b.newExpression(b.identifier('Error'), [ + b.literal(`Cannot spread "${name}" due to security concerns`), + ]), + ), + ]), + ), + [], + ); + + // Full result: + // ("process" in ___n8n_data) ? ___n8n_data.process : (() => { throw ... })() + return b.conditionalExpression(isInDataContext, readFromDataContext, throwSecurityError); +}; + export const PrototypeSanitizer: ASTAfterHook = (ast, dataNode) => { astVisit(ast, { visitVariableDeclarator(path) { this.traverse(path); const node = path.node; - if (node.id.type === 'Identifier' && RESERVED_VARIABLE_NAMES.has(node.id.name)) { - throw new ExpressionReservedVariableError(node.id.name); - } + const reservedIdentifier = getReservedIdentifier(node.id); + if (reservedIdentifier === undefined) return; + throw new ExpressionReservedVariableError(reservedIdentifier); }, visitFunction(path) { this.traverse(path); const node = path.node; + const functionName = getReservedIdentifier(node.id); + if (functionName !== undefined) { + throw new ExpressionReservedVariableError(functionName); + } + for (const param of node.params) { - if (param.type === 'Identifier' && RESERVED_VARIABLE_NAMES.has(param.name)) { - throw new ExpressionReservedVariableError(param.name); + const paramName = getReservedIdentifier(param); + if (paramName !== undefined) { + throw new ExpressionReservedVariableError(paramName); } } }, @@ -273,29 +385,87 @@ export const PrototypeSanitizer: ASTAfterHook = (ast, dataNode) => { this.traverse(path); const node = path.node; - if (node.param?.type === 'Identifier' && RESERVED_VARIABLE_NAMES.has(node.param.name)) { - throw new ExpressionReservedVariableError(node.param.name); - } + const catchParamName = getReservedIdentifier(node.param); + if (catchParamName === undefined) return; + throw new ExpressionReservedVariableError(catchParamName); }, visitClassDeclaration(path) { this.traverse(path); const node = path.node; - if (node.superClass?.type === 'Identifier' && blockedBaseClasses.has(node.superClass.name)) { - throw new ExpressionClassExtensionError(node.superClass.name); + const className = getReservedIdentifier(node.id); + if (className !== undefined) { + throw new ExpressionReservedVariableError(className); + } + + if (node.superClass) { + if (node.superClass.type === 'Identifier') { + if (blockedBaseClasses.has(node.superClass.name)) { + throw new ExpressionClassExtensionError(node.superClass.name); + } + } else { + throw new ExpressionError('Cannot use dynamic class extension due to security concerns'); + } } }, visitClassExpression(path) { this.traverse(path); const node = path.node; - if (node.superClass?.type === 'Identifier' && blockedBaseClasses.has(node.superClass.name)) { - throw new ExpressionClassExtensionError(node.superClass.name); + const className = getReservedIdentifier(node.id); + if (className !== undefined) { + throw new ExpressionReservedVariableError(className); + } + + if (node.superClass) { + if (node.superClass.type === 'Identifier') { + if (blockedBaseClasses.has(node.superClass.name)) { + throw new ExpressionClassExtensionError(node.superClass.name); + } + } else { + throw new ExpressionError('Cannot use dynamic class extension due to security concerns'); + } } }, + visitAssignmentExpression(path) { + this.traverse(path); + const node = path.node; + + const assignedIdentifier = getReservedIdentifier(node.left); + if (assignedIdentifier === undefined) return; + throw new ExpressionReservedVariableError(assignedIdentifier); + }, + + visitUpdateExpression(path) { + this.traverse(path); + const node = path.node; + + const updatedIdentifier = getReservedIdentifier(node.argument); + if (updatedIdentifier === undefined) return; + throw new ExpressionReservedVariableError(updatedIdentifier); + }, + + visitForOfStatement(path) { + this.traverse(path); + const node = path.node; + + const loopBinding = getReservedIdentifier(node.left); + if (loopBinding === undefined) return; + throw new ExpressionReservedVariableError(loopBinding); + }, + + visitForInStatement(path) { + this.traverse(path); + const node = path.node; + + const loopBinding = getReservedIdentifier(node.left); + if (loopBinding === undefined) return; + throw new ExpressionReservedVariableError(loopBinding); + }, + visitMemberExpression(path) { this.traverse(path); const node = path.node; @@ -360,6 +530,24 @@ export const PrototypeSanitizer: ASTAfterHook = (ast, dataNode) => { } }, + visitSpreadElement(path) { + this.traverse(path); + const { argument } = path.node; + if (argument.type === 'Identifier' && BLOCKED_SPREAD_GLOBALS.has(argument.name)) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any + (path.node as any).argument = buildSafeSpreadArg(argument.name, dataNode); + } + }, + + visitSpreadProperty(path) { + this.traverse(path); + const { argument } = path.node; + if (argument.type === 'Identifier' && BLOCKED_SPREAD_GLOBALS.has(argument.name)) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any + (path.node as any).argument = buildSafeSpreadArg(argument.name, dataNode); + } + }, + visitWithStatement() { throw new ExpressionWithStatementError(); },
packages/workflow/src/expression.ts+10 −0 modified@@ -190,8 +190,18 @@ export class Expression { data.uneval = {}; data.setTimeout = {}; data.setInterval = {}; + data.setImmediate = {}; + data.clearImmediate = {}; + data.queueMicrotask = {}; data.Function = {}; + // Prevent Node.js module access + data.require = {}; + data.module = {}; + data.Buffer = {}; + data.__dirname = {}; + data.__filename = {}; + // Prevent requests data.fetch = {}; data.XMLHttpRequest = {};
packages/workflow/src/extensions/expression-extension.ts+22 −11 modified@@ -17,6 +17,7 @@ import { objectExtensions } from './object-extensions'; import { stringExtensions } from './string-extensions'; import { checkIfValueDefinedOrThrow } from './utils'; import { ExpressionExtensionError } from '../errors/expression-extension.error'; +import { isSafeObjectProperty } from '../utils'; const EXPRESSION_EXTENDER = 'extend'; const EXPRESSION_EXTENDER_OPTIONAL = 'extendOptional'; @@ -454,26 +455,36 @@ interface FoundFunction { } function findExtendedFunction(input: unknown, functionName: string): FoundFunction | undefined { + // Coerce to string early so the name is stable for the property check below + const name = typeof functionName === 'string' ? functionName : String(functionName); + + // Ensure the property name is in the allowed set before looking it up + if (!isSafeObjectProperty(name)) { + throw new ExpressionExtensionError( + `Cannot access "${name}" via expression extension due to security concerns`, + ); + } + // eslint-disable-next-line @typescript-eslint/no-restricted-types let foundFunction: Function | undefined; if (Array.isArray(input)) { - foundFunction = arrayExtensions.functions[functionName]; - } else if (isDate(input) && functionName !== 'toDate' && functionName !== 'toDateTime') { + foundFunction = arrayExtensions.functions[name]; + } else if (isDate(input) && name !== 'toDate' && name !== 'toDateTime') { // If it's a string date (from $json), convert it to a Date object, // unless that function is `toDate`, since `toDate` does something // very different on date objects input = new Date(input as string); - foundFunction = dateExtensions.functions[functionName]; + foundFunction = dateExtensions.functions[name]; } else if (typeof input === 'string') { - foundFunction = stringExtensions.functions[functionName]; + foundFunction = stringExtensions.functions[name]; } else if (typeof input === 'number') { - foundFunction = numberExtensions.functions[functionName]; + foundFunction = numberExtensions.functions[name]; } else if (input && (DateTime.isDateTime(input) || input instanceof Date)) { - foundFunction = dateExtensions.functions[functionName]; + foundFunction = dateExtensions.functions[name]; } else if (input !== null && typeof input === 'object') { - foundFunction = objectExtensions.functions[functionName]; + foundFunction = objectExtensions.functions[name]; } else if (typeof input === 'boolean') { - foundFunction = booleanExtensions.functions[functionName]; + foundFunction = booleanExtensions.functions[name]; } // Look for generic or builtin @@ -482,13 +493,13 @@ function findExtendedFunction(input: unknown, functionName: string): FoundFuncti const inputAny: any = input; // This is likely a builtin we're implementing for another type // (e.g. toLocaleString). We'll return that instead - if (inputAny && functionName && typeof inputAny[functionName] === 'function') { + if (inputAny && name && typeof inputAny[name] === 'function') { // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - return { type: 'native', function: inputAny[functionName] }; + return { type: 'native', function: inputAny[name] }; } // Use a generic version if available - foundFunction = genericExtensions[functionName]; + foundFunction = genericExtensions[name]; } if (!foundFunction) {
packages/workflow/src/interfaces.ts+2 −0 modified@@ -1279,6 +1279,7 @@ export interface IWebhookFunctions extends FunctionsBaseWithRequiredKeys<'getMod getRequestObject(): express.Request; getResponseObject(): express.Response; getWebhookName(): string; + validateCookieAuth(cookieValue: string): Promise<void>; nodeHelpers: NodeHelperFunctions; helpers: RequestHelperFunctions & BaseHelperFunctions & BinaryHelperFunctions; } @@ -2922,6 +2923,7 @@ export interface IWorkflowExecuteAdditionalData { executeData?: IExecuteData, ): Promise<Result<T, E>>; getRunnerStatus?(taskType: string): { available: true } | { available: false; reason?: string }; + validateCookieAuth?: (cookieValue: string) => Promise<void>; } export type WorkflowActivateMode =
packages/workflow/src/node-helpers.ts+10 −0 modified@@ -7,6 +7,7 @@ import get from 'lodash/get'; import isEqual from 'lodash/isEqual'; import { EXECUTE_WORKFLOW_NODE_TYPE, WORKFLOW_TOOL_LANGCHAIN_NODE_TYPE } from './constants'; +import { isExpression } from './expressions/expression-helpers'; import { NodeConnectionTypes } from './interfaces'; import type { FieldType, @@ -790,6 +791,15 @@ export function getNodeParameters( nodeParametersFull[nodeProperties.name] = nodeParameters[nodeProperties.name]; continue; } + + // Strip expression prefix if noDataExpression is true + if (nodeProperties.noDataExpression && nodeParameters[nodeProperties.name] !== undefined) { + const value = nodeParameters[nodeProperties.name]; + if (isExpression(value)) { + nodeParameters[nodeProperties.name] = value.slice(1); + nodeParametersFull[nodeProperties.name] = nodeParameters[nodeProperties.name]; + } + } } if (onlySimpleTypes) {
packages/workflow/src/utils.ts+7 −0 modified@@ -372,12 +372,19 @@ const unsafeObjectProperties = new Set([ 'getPrototypeOf', 'mainModule', 'binding', + '_linkedBinding', '_load', 'prepareStackTrace', '__lookupGetter__', '__lookupSetter__', '__defineGetter__', '__defineSetter__', + 'caller', + 'arguments', + 'getBuiltinModule', + 'dlopen', + 'execve', + 'loadEnvFile', ]); /**
packages/workflow/test/expression-sandboxing.test.ts+248 −2 modified@@ -11,6 +11,7 @@ import { ExpressionClassExtensionError, ExpressionComputedDestructuringError, ExpressionDestructuringError, + ExpressionError, ExpressionWithStatementError, } from '../src/errors'; @@ -100,6 +101,36 @@ describe('PrototypeSanitizer', () => { }).toThrowError(errorRegex); }); + it.each([ + ['dot notation', '{{ (()=>{}).caller }}'], + ['bracket notation', '{{ (()=>{})["caller"] }}'], + ])('should not allow access to caller via %s', (_, expression) => { + expect(() => { + tournament.execute(expression, { __sanitize: sanitizer }); + }).toThrowError(errorRegex); + }); + + it.each([ + ['dot notation', '{{ (()=>{}).arguments }}'], + ['bracket notation', '{{ (()=>{})["arguments"] }}'], + ])('should not allow access to arguments via %s', (_, expression) => { + expect(() => { + tournament.execute(expression, { __sanitize: sanitizer }); + }).toThrowError(errorRegex); + }); + + it.each([ + ['getBuiltinModule', '{{ ({}).getBuiltinModule }}'], + ['_linkedBinding', '{{ ({})._linkedBinding }}'], + ['dlopen', '{{ ({}).dlopen }}'], + ['execve', '{{ ({}).execve }}'], + ['loadEnvFile', '{{ ({}).loadEnvFile }}'], + ])('should not allow access to %s', (_, expression) => { + expect(() => { + tournament.execute(expression, { __sanitize: sanitizer }); + }).toThrowError(errorRegex); + }); + describe('Dollar sign identifier handling', () => { it('should not allow bare $ identifier', () => { expect(() => { @@ -266,6 +297,22 @@ describe('PrototypeSanitizer', () => { }).toThrowError(errorRegex); }); + it('should not allow access to caller via concatenation', () => { + expect(() => { + tournament.execute('{{ (()=>{})["cal" + "ler"] }}', { + __sanitize: sanitizer, + }); + }).toThrowError(errorRegex); + }); + + it('should not allow access to arguments via concatenation', () => { + expect(() => { + tournament.execute('{{ (()=>{})["arg" + "uments"] }}', { + __sanitize: sanitizer, + }); + }).toThrowError(errorRegex); + }); + describe('Array-based property access bypass attempts', () => { it('should not allow access to __proto__ via array', () => { expect(() => { @@ -381,6 +428,51 @@ describe('PrototypeSanitizer', () => { }); }).not.toThrow(); }); + + it('should not allow class extending via CallExpression bypass', () => { + expect(() => { + tournament.execute( + '{{ (() => { class Z extends (() => Function)() {} return new Z("return 1")(); })() }}', + { __sanitize: sanitizer, Function }, + ); + }).toThrowError(ExpressionError); + }); + + it('should not allow class expression extending via CallExpression bypass', () => { + expect(() => { + tournament.execute( + '{{ (() => { const Z = class extends (() => Function)() {}; return new Z("return 1")(); })() }}', + { __sanitize: sanitizer, Function }, + ); + }).toThrowError(ExpressionError); + }); + + it('should not allow class extending via ConditionalExpression bypass', () => { + expect(() => { + tournament.execute( + '{{ (() => { class Z extends (true ? Function : Object) {} return new Z("return 1")(); })() }}', + { __sanitize: sanitizer, Function, Object }, + ); + }).toThrowError(ExpressionError); + }); + + it('should not allow class extending via SequenceExpression bypass', () => { + expect(() => { + tournament.execute( + '{{ (() => { class Z extends (0, Function) {} return new Z("return 1")(); })() }}', + { __sanitize: sanitizer, Function }, + ); + }).toThrowError(ExpressionError); + }); + + it('should not allow class extending via LogicalExpression bypass', () => { + expect(() => { + tournament.execute( + '{{ (() => { class Z extends (Function || Object) {} return new Z("return 1")(); })() }}', + { __sanitize: sanitizer, Function, Object }, + ); + }).toThrowError(ExpressionError); + }); }); describe('Destructuring patterns', () => { @@ -442,6 +534,22 @@ describe('PrototypeSanitizer', () => { }).toThrowError(ExpressionDestructuringError); }); + it('should not allow destructuring caller', () => { + expect(() => { + tournament.execute('{{ (() => { const {caller} = ()=>{}; return caller; })() }}', { + __sanitize: sanitizer, + }); + }).toThrowError(ExpressionDestructuringError); + }); + + it('should not allow destructuring arguments', () => { + expect(() => { + tournament.execute('{{ (() => { const {arguments: a} = function(){}; return a; })() }}', { + __sanitize: sanitizer, + }); + }).toThrowError(ExpressionDestructuringError); + }); + it('should allow destructuring safe properties', () => { const result = tournament.execute( '{{ (() => { const {name, value} = {name: "test", value: 42}; return name + value; })() }}', @@ -474,6 +582,144 @@ describe('PrototypeSanitizer', () => { }); }); + describe('Spread-based global access', () => { + it('should not allow spreading process', () => { + expect(() => { + tournament.execute('{{ ((g) => g.getBuiltinModule)(({...process})) }}', { + __sanitize: sanitizer, + }); + }).toThrowError(/due to security concerns/); + }); + + it('should not allow spreading process in object literal', () => { + expect(() => { + tournament.execute('{{ ({...process}) }}', { __sanitize: sanitizer }); + }).toThrowError(/Cannot spread "process" due to security concerns/); + }); + + it('should not allow spreading process in array', () => { + expect(() => { + tournament.execute('{{ [...process] }}', { __sanitize: sanitizer }); + }).toThrowError(/Cannot spread "process" due to security concerns/); + }); + + it('should not allow spreading global', () => { + expect(() => { + tournament.execute('{{ ({...global}) }}', { __sanitize: sanitizer }); + }).toThrowError(/Cannot spread "global" due to security concerns/); + }); + + it('should not allow spreading Buffer', () => { + expect(() => { + tournament.execute('{{ ({...Buffer}) }}', { __sanitize: sanitizer }); + }).toThrowError(/Cannot spread "Buffer" due to security concerns/); + }); + + it('should not allow the exact RCE PoC payload', () => { + expect(() => { + tournament.execute( + "{{ ((g) => g.getBuiltinModule('child_process').execSync('id').toString())({...process}) }}", + { __sanitize: sanitizer }, + ); + }).toThrowError(/due to security concerns/); + }); + + it('should not allow spreading process in function call arguments', () => { + expect(() => { + tournament.execute('{{ ((a, b) => a)(...process) }}', { __sanitize: sanitizer }); + }).toThrowError(/Cannot spread "process" due to security concerns/); + }); + + it('should not allow spreading process inside arrow function', () => { + expect(() => { + tournament.execute('{{ (() => ({...process}))() }}', { __sanitize: sanitizer }); + }).toThrowError(/Cannot spread "process" due to security concerns/); + }); + + it('should not allow spreading process in nested spread', () => { + expect(() => { + tournament.execute('{{ ({...({...process})}) }}', { __sanitize: sanitizer }); + }).toThrowError(/Cannot spread "process" due to security concerns/); + }); + + it('should not allow spreading process in template expression', () => { + expect(() => { + // eslint-disable-next-line n8n-local-rules/no-interpolation-in-regular-string + tournament.execute('{{ `${JSON.stringify({...process})}` }}', { + __sanitize: sanitizer, + }); + }).toThrow(); + }); + + it('should not allow spreading process among other spreads', () => { + expect(() => { + tournament.execute('{{ ({...{a:1}, ...process}) }}', { __sanitize: sanitizer }); + }).toThrowError(/Cannot spread "process" due to security concerns/); + }); + + it('should resolve spread from data context process, not the real one', () => { + const result = tournament.execute('{{ ({...process}).safe }}', { + __sanitize: sanitizer, + process: { safe: true }, + }); + expect(result).toBe(true); + }); + + it('should not expose real process.version via spread', () => { + const result = tournament.execute('{{ typeof ({...process}).version }}', { + __sanitize: sanitizer, + process: {}, + }); + expect(result).toBe('undefined'); + }); + + it('should use data context pid via spread, not real pid', () => { + const result = tournament.execute('{{ ({...process}).pid }}', { + __sanitize: sanitizer, + process: { pid: -1 }, + }); + expect(result).toBe(-1); + }); + + it('should use data context when spread is wrapped in arrow function', () => { + const result = tournament.execute('{{ ((g) => g.pid)({...process}) }}', { + __sanitize: sanitizer, + process: { pid: -1 }, + }); + expect(result).toBe(-1); + }); + + it('should not give access to real process.exit via spread', () => { + const result = tournament.execute('{{ typeof ({...process}).exit }}', { + __sanitize: sanitizer, + process: {}, + }); + expect(result).not.toBe('function'); + }); + + it('should not give access to real process.env via spread', () => { + const result = tournament.execute('{{ typeof ({...process}).env }}', { + __sanitize: sanitizer, + process: {}, + }); + expect(result).not.toBe('object'); + }); + + it('should not give access to getBuiltinModule via spread', () => { + let result: unknown; + try { + result = tournament.execute('{{ typeof ({...process}).getBuiltinModule }}', { + __sanitize: sanitizer, + process: {}, + }); + } catch { + // Blocked by PrototypeSanitizer — also a valid outcome + return; + } + expect(result).not.toBe('function'); + }); + }); + describe('`with` statement', () => { it('should not allow `with` statements', () => { expect(() => { @@ -713,7 +959,7 @@ describe('ThisSanitizer', () => { const result = tournament.execute('{{ (() => this)() }}', { __sanitize: sanitizer, }); - expect(result).toEqual({ process: {} }); + expect(result).toEqual({ process: {}, require: {}, module: {}, Buffer: {} }); }); it('should block process.env access via this in arrow functions', () => { @@ -728,7 +974,7 @@ describe('ThisSanitizer', () => { const result = tournament.execute('{{ (() => (() => this)())() }}', { __sanitize: sanitizer, }); - expect(result).toEqual({ process: {} }); + expect(result).toEqual({ process: {}, require: {}, module: {}, Buffer: {} }); }); it('should block this?.process?.env access pattern', () => {
packages/workflow/test/expression.test.ts+237 −20 modified@@ -366,9 +366,8 @@ describe('Expression', () => { return new Error().stack; })()}}`; - // Attack is blocked - calling undefined() throws, result is undefined - const result = evaluate(payload); - expect(result).toBeUndefined(); + // Attack is blocked - make sure it throws + expect(() => evaluate(payload)).toThrowError(/due to security concerns/); }); it('should block __defineGetter__ bypass attack', () => { @@ -507,30 +506,248 @@ describe('Expression', () => { expect(() => evaluate(payload)).toThrow(); }); - it('should block `___n8n_data` shadowing attempt', () => { - const payload = `={{(() => { - const ___n8n_data = {__sanitize: a => a}; - return ({})['const'+'ructor']['const'+'ructor']('return 1')(); - })()}}`; + const reservedVariablePayloads: Array<[string, string]> = [ + [ + '`___n8n_data` declaration', + `={{(() => { + const ___n8n_data = {__sanitize: a => a}; + return 1; + })()}}`, + ], + [ + '`__sanitize` declaration', + `={{(() => { + const __sanitize = a => a; + return 1; + })()}}`, + ], + [ + 'array destructuring declaration', + `={{(() => { + const [___n8n_data] = [{ __sanitize: (v) => v }]; + return 1; + })()}}`, + ], + [ + 'object destructuring declaration', + `={{(() => { + const {a: ___n8n_data} = { a: { __sanitize: (v) => v } }; + return 1; + })()}}`, + ], + [ + 'function parameter identifier', + `={{((___n8n_data) => { + return ___n8n_data; + })({})}}`, + ], + [ + 'function parameter object pattern', + `={{(({a: ___n8n_data}) => { + return ___n8n_data; + })({ a: { __sanitize: (v) => v } })}}`, + ], + [ + 'function parameter array pattern', + `={{(([___n8n_data]) => { + return ___n8n_data; + })([{ __sanitize: (v) => v }])}}`, + ], + [ + 'function parameter default value', + `={{((___n8n_data = { __sanitize: (v) => v }) => { + return ___n8n_data; + })()}}`, + ], + [ + 'function parameter rest element', + `={{((...___n8n_data) => { + return ___n8n_data; + })(1)}}`, + ], + [ + 'function declaration name', + `={{(() => { + function ___n8n_data() {} + return 1; + })()}}`, + ], + [ + 'class declaration name', + `={{(() => { + class ___n8n_data {} + return 1; + })()}}`, + ], + [ + 'catch object pattern parameter', + `={{(() => { + try { + throw { a: { __sanitize: (v) => v } }; + } catch ({ a: ___n8n_data }) { + return ___n8n_data; + } + })()}}`, + ], + [ + 'catch array pattern parameter', + `={{(() => { + try { + throw [{ __sanitize: (v) => v }]; + } catch ([___n8n_data]) { + return ___n8n_data; + } + })()}}`, + ], + [ + 'for-of object pattern declaration', + `={{(() => { + for (const { a: ___n8n_data } of [{ a: { __sanitize: (v) => v } }]) { + return ___n8n_data; + } + })()}}`, + ], + [ + 'for-of assignment pattern target', + `={{(() => { + for ([___n8n_data] of [[{ __sanitize: (v) => v }]]) { + return ___n8n_data; + } + })()}}`, + ], + [ + 'destructuring assignment target', + `={{(() => { + [___n8n_data] = [{ __sanitize: (v) => v }]; + return ___n8n_data; + })()}}`, + ], + ]; + + for (const [name, payload] of reservedVariablePayloads) { + it(`should block reserved variable shadowing via ${name}`, () => { + expect(() => evaluate(payload)).toThrow(ExpressionReservedVariableError); + }); + } - expect(() => evaluate(payload)).toThrow(ExpressionReservedVariableError); + it('should block extend() constructor access on arrow functions', () => { + expect(() => evaluate('={{ extend((() => {}), "constructor", ["return 1"])() }}')).toThrow( + /due to security concerns/, + ); }); - it('should block `__sanitize` variable declaration', () => { - const payload = `={{(() => { - const __sanitize = a => a; - return 1; - })()}}`; + it('should block extendOptional() constructor access on arrow functions', () => { + expect(() => + evaluate('={{ extendOptional((() => {}), "constructor")("return 1")() }}'), + ).toThrow(/due to security concerns/); + }); + + it('should block extend() constructor access on extend itself', () => { + expect(() => evaluate('={{ extend(extend, "constructor", ["return 1"])() }}')).toThrow( + /due to security concerns/, + ); + }); + + it('should block extend() constructor access on extendOptional', () => { + expect(() => + evaluate('={{ extend(extendOptional, "constructor", ["return 1"])() }}'), + ).toThrow(/due to security concerns/); + }); + + it('should block extend() constructor access on isNaN', () => { + expect(() => evaluate('={{ extend(isNaN, "constructor", ["return 1"])() }}')).toThrow( + /due to security concerns/, + ); + }); - expect(() => evaluate(payload)).toThrow(ExpressionReservedVariableError); + it('should block extend() constructor access on parseFloat', () => { + expect(() => evaluate('={{ extend(parseFloat, "constructor", ["return 1"])() }}')).toThrow( + /due to security concerns/, + ); + }); + + it('should block extend() __proto__ access', () => { + expect(() => evaluate('={{ extend({}, "__proto__", []) }}')).toThrow( + /due to security concerns/, + ); + }); + + it('should block extend() prototype access', () => { + expect(() => evaluate('={{ extend({}, "prototype", []) }}')).toThrow( + /due to security concerns/, + ); + }); + + it('should block extend() with custom toString() returning constructor', () => { + expect(() => + evaluate('={{ extend((() => {}), {toString: () => "constructor"}, ["return 1"])() }}'), + ).toThrow(/due to security concerns/); + }); + + it('should block extend() with custom toString() returning __proto__', () => { + expect(() => evaluate('={{ extend({}, {toString: () => "__proto__"}, []) }}')).toThrow( + /due to security concerns/, + ); + }); + + it('should block extend() constructor access on arrow functions', () => { + expect(() => evaluate('={{ extend((() => {}), "constructor", ["return 1"])() }}')).toThrow( + /due to security concerns/, + ); + }); + + it('should block extendOptional() constructor access on arrow functions', () => { + expect(() => + evaluate('={{ extendOptional((() => {}), "constructor")("return 1")() }}'), + ).toThrow(/due to security concerns/); + }); + + it('should block extend() constructor access on extend itself', () => { + expect(() => evaluate('={{ extend(extend, "constructor", ["return 1"])() }}')).toThrow( + /due to security concerns/, + ); + }); + + it('should block extend() constructor access on extendOptional', () => { + expect(() => + evaluate('={{ extend(extendOptional, "constructor", ["return 1"])() }}'), + ).toThrow(/due to security concerns/); + }); + + it('should block extend() constructor access on isNaN', () => { + expect(() => evaluate('={{ extend(isNaN, "constructor", ["return 1"])() }}')).toThrow( + /due to security concerns/, + ); + }); + + it('should block extend() constructor access on parseFloat', () => { + expect(() => evaluate('={{ extend(parseFloat, "constructor", ["return 1"])() }}')).toThrow( + /due to security concerns/, + ); + }); + + it('should block extend() __proto__ access', () => { + expect(() => evaluate('={{ extend({}, "__proto__", []) }}')).toThrow( + /due to security concerns/, + ); + }); + + it('should block extend() prototype access', () => { + expect(() => evaluate('={{ extend({}, "prototype", []) }}')).toThrow( + /due to security concerns/, + ); }); - it('should block `___n8n_data` as function parameter', () => { - const payload = `={{((___n8n_data) => { - return 1; - })({})}}`; + it('should block extend() with custom toString() returning constructor', () => { + expect(() => + evaluate('={{ extend((() => {}), {toString: () => "constructor"}, ["return 1"])() }}'), + ).toThrow(/due to security concerns/); + }); - expect(() => evaluate(payload)).toThrow(ExpressionReservedVariableError); + it('should block extend() with custom toString() returning __proto__', () => { + expect(() => evaluate('={{ extend({}, {toString: () => "__proto__"}, []) }}')).toThrow( + /due to security concerns/, + ); }); }); });
packages/workflow/test/node-helpers.test.ts+209 −0 modified@@ -7,6 +7,7 @@ import { type INodeParameters, type INodeProperties, type INodeTypeDescription, + type NodeParameterValueType, } from '../src/interfaces'; import { getNodeParameters, @@ -6316,4 +6317,212 @@ describe('NodeHelpers', () => { expect(isHitlToolType('CustomNode')).toBe(false); }); }); + + describe('getNodeParameters - noDataExpression handling', () => { + it('should strip expression prefix when noDataExpression is true and value is an expression', () => { + const nodePropertiesArray: INodeProperties[] = [ + { + name: 'resource', + displayName: 'Resource', + type: 'string', + default: '', + noDataExpression: true, + }, + ]; + + const nodeValues: Record<string, string> = { + resource: '=users', + }; + + const node: INode = { + id: 'test-123', + name: 'Test', + type: 'n8n-nodes-base.test', + typeVersion: 1, + position: [0, 0], + parameters: nodeValues, + credentials: {}, + }; + const description: INodeTypeDescription = { + name: 'Test', + displayName: 'Test', + group: [], + version: 1, + description: 'Test', + defaults: {}, + inputs: [], + outputs: [], + properties: nodePropertiesArray, + }; + + const nodeType: INodeType = { + description, + }; + + const result = getNodeParameters( + nodeType.description.properties, + nodeValues, + true, + false, + node, + nodeType.description, + ); + + expect(result?.resource).toBe('users'); + }); + + it('should not strip expression prefix when noDataExpression is false', () => { + const nodePropertiesArray: INodeProperties[] = [ + { + name: 'resource', + displayName: 'Resource', + type: 'string', + default: '', + noDataExpression: false, + }, + ]; + + const nodeValues: Record<string, string> = { + resource: '=users', + }; + + const node: INode = { + id: 'test-123', + name: 'Test', + type: 'n8n-nodes-base.test', + typeVersion: 1, + position: [0, 0], + parameters: nodeValues, + credentials: {}, + }; + + const nodeType: INodeType = { + description: { + displayName: 'Test', + name: 'Test', + group: [], + version: 1, + description: 'Test', + defaults: {}, + inputs: [], + outputs: [], + properties: nodePropertiesArray, + }, + }; + + const result = getNodeParameters( + nodeType.description.properties, + nodeValues, + true, + false, + node, + nodeType.description, + ); + + expect(result?.resource).toBe('=users'); + }); + + it('should not modify non-expression values when noDataExpression is true', () => { + const nodePropertiesArray: INodeProperties[] = [ + { + name: 'resource', + displayName: 'Resource', + type: 'string', + default: '', + noDataExpression: true, + }, + ]; + + const nodeValues: Record<string, string> = { + resource: 'users', + }; + + const node: INode = { + id: 'test-123', + name: 'Test', + type: 'n8n-nodes-base.test', + typeVersion: 1, + position: [0, 0], + parameters: nodeValues, + credentials: {}, + }; + + const nodeType: INodeType = { + description: { + displayName: 'Test', + name: 'Test', + group: [], + version: 1, + description: 'Test', + defaults: {}, + inputs: [], + outputs: [], + properties: nodePropertiesArray, + }, + }; + + const result = getNodeParameters( + nodeType.description.properties, + nodeValues, + true, + false, + node, + nodeType.description, + ); + + expect(result?.resource).toBe('users'); + }); + + it('should handle undefined values when noDataExpression is true', () => { + const nodePropertiesArray: INodeProperties[] = [ + { + name: 'resource', + displayName: 'Resource', + type: 'string', + default: '', + noDataExpression: true, + }, + ]; + + const nodeValues: NodeParameterValueType = { + resource: undefined, + }; + + const node: INode = { + id: 'test-123', + name: 'Test', + type: 'n8n-nodes-base.test', + typeVersion: 1, + position: [0, 0], + parameters: nodeValues, + credentials: {}, + }; + + const nodeType: INodeType = { + description: { + displayName: 'Test', + name: 'Test', + group: [], + version: 1, + description: 'Test', + defaults: {}, + inputs: [], + outputs: [], + properties: nodePropertiesArray, + }, + }; + + const result = getNodeParameters( + nodeType.description.properties, + nodeValues, + true, + false, + node, + nodeType.description, + ); + + // When undefined, the default value (empty string) is used + expect(result?.resource).toBe(''); + }); + }); });
Vulnerability mechanics
Generated by null/stub on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
8- github.com/advisories/GHSA-75g8-rv7v-32f7ghsaADVISORY
- nvd.nist.gov/vuln/detail/CVE-2026-27493ghsaADVISORY
- github.com/n8n-io/n8n/commit/562d867483e871b0f1e31776252e23bd721df75bghsax_refsource_MISCWEB
- github.com/n8n-io/n8n/issues/19ghsax_refsource_MISCWEB
- github.com/n8n-io/n8n/releases/tag/n8n@1.123.22ghsax_refsource_MISCWEB
- github.com/n8n-io/n8n/releases/tag/n8n@2.10.1ghsax_refsource_MISCWEB
- github.com/n8n-io/n8n/releases/tag/n8n@2.9.3ghsax_refsource_MISCWEB
- github.com/n8n-io/n8n/security/advisories/GHSA-75g8-rv7v-32f7ghsax_refsource_CONFIRMWEB
News mentions
0No linked articles in our index yet.