fix: Prevent empty path in webhooks (#16864)

Co-authored-by: Roman Davydchuk <roman.davydchuk@n8n.io>

cypress/e2e/5-ndv.cy.ts

@@ -346,7 +346,7 @@ describe('NDV', () => {
 		});
 	});
 
-	it('should flag issues as soon as params are set', () => {
+	it('webhook shoul fallback to webhookId if path is empty', () => {
 		workflowPage.actions.addInitialNodeToCanvas('Webhook');
 		workflowPage.getters.canvasNodes().first().dblclick();
 
@@ -356,13 +356,31 @@ describe('NDV', () => {
 
 		ndv.getters.parameterInput('path').clear();
 
-		ndv.getters.nodeExecuteButton().should('be.disabled');
-		ndv.getters.triggerPanelExecuteButton().should('not.exist');
+		const urrWithUUID =
+			/https?:\/\/[^\/]+\/webhook-test\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/i;
+
+		cy.contains('Webhook URLs')
+			.parent()
+			.invoke('text')
+			.then((text) => {
+				const match = text.match(urrWithUUID);
+				expect(match, 'Should contain dynamic URL with UUID').to.not.be.null;
+			});
+
 		ndv.actions.close();
-		workflowPage.getters.nodeIssuesByName('Webhook').should('exist');
 
 		workflowPage.getters.canvasNodes().first().dblclick();
-		ndv.getters.parameterInput('path').type('t');
+		ndv.getters.parameterInput('path').type('test-path');
+
+		const urlWithCustomPath = /https?:\/\/[^\/]+\/webhook-test\/test-path/i;
+
+		cy.contains('Webhook URLs')
+			.parent()
+			.invoke('text')
+			.then((text) => {
+				const match = text.match(urlWithCustomPath);
+				expect(match, 'Should contain URL with custom path').to.not.be.null;
+			});
 
 		ndv.getters.nodeExecuteButton().should('not.be.disabled');
 		ndv.getters.triggerPanelExecuteButton().should('exist');

packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts

@@ -448,6 +448,56 @@ describe('useWorkflowHelpers', () => {
 			});
 			expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual(null);
 		});
+
+		it('should return trigger.parameters.path when both trigger.parameters.path and trigger.webhookId are strings', async () => {
+			const workflowHelpers = useWorkflowHelpers();
+			uiStore.stateIsDirty = false;
+			vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({
+				nodes: [
+					{
+						type: WEBHOOK_NODE_TYPE,
+						parameters: {
+							method: 'GET',
+							path: 'test-path',
+						},
+						webhookId: 'test-webhook-id',
+					},
+				],
+			} as unknown as IWorkflowDb);
+			const findWebhookSpy = vi.spyOn(apiWebhooks, 'findWebhook');
+			findWebhookSpy.mockResolvedValue(null);
+			expect(await workflowHelpers.checkConflictingWebhooks('12345')).toEqual(null);
+			expect(findWebhookSpy).toBeCalledWith(expect.anything(), {
+				method: 'GET',
+				path: 'test-path',
+			});
+			findWebhookSpy.mockRestore();
+		});
+
+		it('should return trigger.webhookId when trigger.parameters.path is an empty string', async () => {
+			const workflowHelpers = useWorkflowHelpers();
+			uiStore.stateIsDirty = false;
+			vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({
+				nodes: [
+					{
+						type: WEBHOOK_NODE_TYPE,
+						parameters: {
+							method: 'GET',
+							path: '',
+						},
+						webhookId: 'test-webhook-id',
+					},
+				],
+			} as unknown as IWorkflowDb);
+			const findWebhookSpy = vi.spyOn(apiWebhooks, 'findWebhook');
+			findWebhookSpy.mockResolvedValue(null);
+			expect(await workflowHelpers.checkConflictingWebhooks('12345')).toEqual(null);
+			expect(findWebhookSpy).toBeCalledWith(expect.anything(), {
+				method: 'GET',
+				path: 'test-webhook-id',
+			});
+			findWebhookSpy.mockRestore();
+		});
 	});
 
 	describe('executeData', () => {

packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts

@@ -922,7 +922,7 @@ export function useWorkflowHelpers() {
 
 	function getWebhookPath(trigger: INode) {
 		if (trigger.type === WEBHOOK_NODE_TYPE) {
-			return trigger.parameters.path as string;
+			return (trigger.parameters.path as string) || (trigger.webhookId as string);
 		}
 		if (trigger.type === FORM_TRIGGER_NODE_TYPE) {
 			return ((trigger.parameters.options as { path: string }) || {}).path ?? trigger.webhookId;

packages/nodes-base/nodes/Webhook/Webhook.node.ts

@@ -133,7 +133,6 @@ export class Webhook extends Node {
 				type: 'string',
 				default: '',
 				placeholder: 'webhook',
-				required: true,
 				description:
 					"The path to listen to, dynamic values could be specified by using ':', e.g. 'your-path/:dynamic-value'. If dynamic values are set 'webhookId' would be prepended to path.",
 			},

packages/workflow/src/node-helpers.ts

@@ -956,17 +956,22 @@ export function getNodeWebhookPath(
 	path: string,
 	isFullPath?: boolean,
 	restartWebhook?: boolean,
-): string {
+) {
 	let webhookPath = '';
+
 	if (restartWebhook === true) {
 		return path;
 	}
+
 	if (node.webhookId === undefined) {
-		webhookPath = `${workflowId}/${encodeURIComponent(node.name.toLowerCase())}/${path}`;
+		const nodeName = encodeURIComponent(node.name.toLowerCase());
+
+		webhookPath = `${workflowId}/${nodeName}/${path}`;
 	} else {
 		if (isFullPath === true) {
-			return path;
+			return path || node.webhookId;
 		}
+
 		webhookPath = `${node.webhookId}/${path}`;
 	}
 	return webhookPath;

packages/workflow/test/node-helpers.test.ts

@@ -21,8 +21,10 @@ import {
 	isDefaultNodeName,
 	makeNodeName,
 	isTool,
+	getNodeWebhookPath,
 } from '../src/node-helpers';
 import type { Workflow } from '../src/workflow';
+import { mock } from 'vitest-mock-extended';
 
 describe('NodeHelpers', () => {
 	describe('getNodeParameters', () => {
@@ -4382,6 +4384,7 @@ describe('NodeHelpers', () => {
 			test(testData.description, () => {
 				// If this test has a custom mock return value, configure it
 				if (testData.mockReturnValue) {
+					// eslint-disable-next-line @typescript-eslint/unbound-method
 					vi.mocked(workflowMock.expression.getSimpleParameterValue).mockReturnValueOnce(
 						testData.mockReturnValue,
 					);
@@ -5611,4 +5614,48 @@ describe('NodeHelpers', () => {
 			expect(result).toBe(false);
 		});
 	});
+	describe('getNodeWebhookPath', () => {
+		const mockWorkflowId = 'workflow-123';
+		const mockPath = 'test-path';
+
+		it('should return path when restartWebhook is true', () => {
+			const node = mock<INode>({ name: 'TestNode' });
+
+			const result = getNodeWebhookPath(mockWorkflowId, node, mockPath, false, true);
+
+			expect(result).toBe(mockPath);
+		});
+
+		it('should return path when node has webhookId and isFullPath is true', () => {
+			const node = mock<INode>({ name: 'TestNode', webhookId: 'webhook-456' });
+
+			const result = getNodeWebhookPath(mockWorkflowId, node, mockPath, true, false);
+
+			expect(result).toBe(mockPath);
+		});
+
+		it('should return webhookId when node has webhookId, isFullPath is true, and path is empty', () => {
+			const node = mock<INode>({ name: 'TestNode', webhookId: 'webhook-456' });
+
+			const result = getNodeWebhookPath(mockWorkflowId, node, '', true, false);
+
+			expect(result).toBe('webhook-456');
+		});
+
+		it('should return webhookId/path when node has webhookId and isFullPath is false', () => {
+			const node = mock<INode>({ name: 'TestNode', webhookId: 'webhook-456' });
+
+			const result = getNodeWebhookPath(mockWorkflowId, node, mockPath, false, false);
+
+			expect(result).toBe('webhook-456/test-path');
+		});
+
+		it('should return workflowId/nodename/path when node has no webhookId', () => {
+			const node = mock<INode>({ name: 'TestNode', webhookId: undefined });
+
+			const result = getNodeWebhookPath(mockWorkflowId, node, mockPath, false, false);
+
+			expect(result).toBe('workflow-123/testnode/test-path');
+		});
+	});
 });