fix(core): Fix waiting webhooks validation when n8n is behind proxy (#18767)

packages/cli/src/webhooks/__tests__/waiting-forms.test.ts

@@ -9,7 +9,7 @@ import type { WaitingWebhookRequest } from '../webhook.types';
 
 describe('WaitingForms', () => {
 	const executionRepository = mock<ExecutionRepository>();
-	const waitingForms = new WaitingForms(mock(), mock(), executionRepository, mock());
+	const waitingForms = new WaitingForms(mock(), mock(), executionRepository, mock(), mock());
 
 	beforeEach(() => {
 		jest.restoreAllMocks();

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<ExecutionRepository>();
-	const waitingWebhooks = new WaitingWebhooks(mock(), mock(), executionRepository, mock());
+	const mockInstanceSettings = mock<InstanceSettings>({
+		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<express.Request>({
+				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<express.Request>({
+				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<express.Request>({
+				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<express.Request>({
+				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);
+		});
+	});
 });

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 };
 			}