From f2eca28e6479143dce01f48700144714d74f4ed4 Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Wed, 27 Aug 2025 09:29:04 +0300 Subject: [PATCH] fix(core): Fix waiting webhooks validation when n8n is behind proxy (#18767) --- .../webhooks/__tests__/waiting-forms.test.ts | 2 +- .../__tests__/waiting-webhooks.test.ts | 121 +++++++++++++++++- packages/cli/src/webhooks/waiting-webhooks.ts | 36 +++--- 3 files changed, 140 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts b/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts index 41d8d2bc348..522a6cb292e 100644 --- a/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts +++ b/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts @@ -9,7 +9,7 @@ import type { WaitingWebhookRequest } from '../webhook.types'; describe('WaitingForms', () => { const executionRepository = mock(); - const waitingForms = new WaitingForms(mock(), mock(), executionRepository, mock()); + const waitingForms = new WaitingForms(mock(), mock(), executionRepository, mock(), mock()); beforeEach(() => { jest.restoreAllMocks(); diff --git a/packages/cli/src/webhooks/__tests__/waiting-webhooks.test.ts b/packages/cli/src/webhooks/__tests__/waiting-webhooks.test.ts index 68b4c432b52..e8f3c711515 100644 --- a/packages/cli/src/webhooks/__tests__/waiting-webhooks.test.ts +++ b/packages/cli/src/webhooks/__tests__/waiting-webhooks.test.ts @@ -1,6 +1,8 @@ import type { IExecutionResponse, ExecutionRepository } from '@n8n/db'; import type express from 'express'; import { mock } from 'jest-mock-extended'; +import type { InstanceSettings } from 'n8n-core'; +import { generateUrlSignature, prepareUrlForSigning, WAITING_TOKEN_QUERY_PARAM } from 'n8n-core'; import { ConflictError } from '@/errors/response-errors/conflict.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; @@ -8,8 +10,18 @@ import { WaitingWebhooks } from '@/webhooks/waiting-webhooks'; import type { WaitingWebhookRequest } from '@/webhooks/webhook.types'; describe('WaitingWebhooks', () => { + const SIGNING_SECRET = 'test-secret'; const executionRepository = mock(); - const waitingWebhooks = new WaitingWebhooks(mock(), mock(), executionRepository, mock()); + const mockInstanceSettings = mock({ + hmacSignatureSecret: SIGNING_SECRET, + }); + const waitingWebhooks = new WaitingWebhooks( + mock(), + mock(), + executionRepository, + mock(), + mockInstanceSettings, + ); beforeEach(() => { jest.restoreAllMocks(); @@ -78,4 +90,111 @@ describe('WaitingWebhooks', () => { */ await expect(promise).rejects.toThrowError(ConflictError); }); + + describe('validateSignatureInRequest', () => { + const EXAMPLE_HOST = 'example.com'; + const generateValidSignature = (host = EXAMPLE_HOST) => + generateUrlSignature( + prepareUrlForSigning(new URL('/webhook/test', `http://${host}`)), + SIGNING_SECRET, + ); + + const createMockRequest = (opts: { host?: string; signature: string }) => + mock({ + url: `/webhook/test?${WAITING_TOKEN_QUERY_PARAM}=` + opts.signature, + host: opts.host ?? EXAMPLE_HOST, + query: { [WAITING_TOKEN_QUERY_PARAM]: opts.signature }, + headers: { host: opts.host ?? EXAMPLE_HOST }, + }); + + it('should validate signature correctly', () => { + /* Arrange */ + const signature = generateValidSignature(); + const mockReq = createMockRequest({ signature }); + + /* Act */ + const result = waitingWebhooks.validateSignatureInRequest(mockReq); + + /* Assert */ + expect(result).toBe(true); + }); + + it('should validate signature correctly when host contains a port', () => { + /* Arrange */ + const signature = generateValidSignature('example.com:8080'); + const mockReq = createMockRequest({ + signature, + host: 'example.com:8080', + }); + + /* Act */ + const result = waitingWebhooks.validateSignatureInRequest(mockReq); + + /* Assert */ + expect(result).toBe(true); + }); + + it('should validate signature correctly when n8n is behind a reverse proxy', () => { + /* Arrange */ + const signature = generateValidSignature('proxy.example.com'); + const mockReq = mock({ + url: `/webhook/test?${WAITING_TOKEN_QUERY_PARAM}=` + signature, + host: 'proxy.example.com', + query: { [WAITING_TOKEN_QUERY_PARAM]: signature }, + headers: { + host: 'localhost', + // eslint-disable-next-line @typescript-eslint/naming-convention + 'x-forwarded-host': 'proxy.example.com', + }, + }); + + /* Act */ + const result = waitingWebhooks.validateSignatureInRequest(mockReq); + + /* Assert */ + expect(result).toBe(true); + }); + + it('should return false when signature is missing', () => { + /* Arrange */ + const mockReq = mock({ + url: '/webhook/test', + hostname: 'example.com', + query: {}, + }); + + /* Act */ + const result = waitingWebhooks.validateSignatureInRequest(mockReq); + + /* Assert */ + expect(result).toBe(false); + }); + + it('should return false when signature is empty', () => { + /* Arrange */ + const mockReq = mock({ + url: `/webhook/test?${WAITING_TOKEN_QUERY_PARAM}=`, + hostname: 'example.com', + query: { [WAITING_TOKEN_QUERY_PARAM]: '' }, + }); + + /* Act */ + const result = waitingWebhooks.validateSignatureInRequest(mockReq); + + /* Assert */ + expect(result).toBe(false); + }); + + it('should return false when signatures do not match', () => { + /* Arrange */ + const wrongSignature = 'wrong-signature'; + const mockReq = createMockRequest({ signature: wrongSignature }); + + /* Act */ + const result = waitingWebhooks.validateSignatureInRequest(mockReq); + + /* Assert */ + expect(result).toBe(false); + }); + }); }); diff --git a/packages/cli/src/webhooks/waiting-webhooks.ts b/packages/cli/src/webhooks/waiting-webhooks.ts index 9766d130c06..ec49f0dcf88 100644 --- a/packages/cli/src/webhooks/waiting-webhooks.ts +++ b/packages/cli/src/webhooks/waiting-webhooks.ts @@ -1,8 +1,15 @@ import { Logger } from '@n8n/backend-common'; import type { IExecutionResponse } from '@n8n/db'; import { ExecutionRepository } from '@n8n/db'; -import { Container, Service } from '@n8n/di'; +import { Service } from '@n8n/di'; +import crypto from 'crypto'; import type express from 'express'; +import { + InstanceSettings, + WAITING_TOKEN_QUERY_PARAM, + prepareUrlForSigning, + generateUrlSignature, +} from 'n8n-core'; import { FORM_NODE_TYPE, type INodes, @@ -25,13 +32,6 @@ import type { IWebhookManager, WaitingWebhookRequest, } from './webhook.types'; -import { - InstanceSettings, - WAITING_TOKEN_QUERY_PARAM, - prepareUrlForSigning, - generateUrlSignature, -} from 'n8n-core'; -import crypto from 'crypto'; /** * Service for handling the execution of webhooks of Wait nodes that use the @@ -47,6 +47,7 @@ export class WaitingWebhooks implements IWebhookManager { protected readonly nodeTypes: NodeTypes, private readonly executionRepository: ExecutionRepository, private readonly webhookService: WebhookService, + private readonly instanceSettings: InstanceSettings, ) {} // TODO: implement `getWebhookMethods` for CORS support @@ -89,22 +90,23 @@ export class WaitingWebhooks implements IWebhookManager { }); } - private getHmacSecret() { - return Container.get(InstanceSettings).hmacSignatureSecret; - } - - private validateSignatureInRequest(req: express.Request, secret: string) { + validateSignatureInRequest(req: express.Request) { try { const actualToken = req.query[WAITING_TOKEN_QUERY_PARAM]; if (typeof actualToken !== 'string') return false; - const parsedUrl = new URL(req.url, `http://${req.headers.host}`); + // req.host is set correctly even when n8n is behind a reverse proxy + // as long as N8N_PROXY_HOPS is set correctly + const parsedUrl = new URL(req.url, `http://${req.host}`); parsedUrl.searchParams.delete(WAITING_TOKEN_QUERY_PARAM); const urlForSigning = prepareUrlForSigning(parsedUrl); - const expectedToken = generateUrlSignature(urlForSigning, secret); + const expectedToken = generateUrlSignature( + urlForSigning, + this.instanceSettings.hmacSignatureSecret, + ); const valid = crypto.timingSafeEqual(Buffer.from(actualToken), Buffer.from(expectedToken)); return valid; @@ -128,12 +130,12 @@ export class WaitingWebhooks implements IWebhookManager { const execution = await this.getExecution(executionId); - if (execution && execution.data.validateSignature) { + if (execution?.data.validateSignature) { const lastNodeExecuted = execution.data.resultData.lastNodeExecuted as string; const lastNode = execution.workflowData.nodes.find((node) => node.name === lastNodeExecuted); const shouldValidate = lastNode?.parameters.operation === SEND_AND_WAIT_OPERATION; - if (shouldValidate && !this.validateSignatureInRequest(req, this.getHmacSecret())) { + if (shouldValidate && !this.validateSignatureInRequest(req)) { res.status(401).json({ error: 'Invalid token' }); return { noWebhookResponse: true }; }