From 1573ae63522ccf72ea63d988ed01eaf82e85376b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Fri, 20 Jun 2025 16:26:08 +0200 Subject: [PATCH] fix(core): Handle dynamic webhook edge cases (#16554) --- .../__tests__/webhook.service.test.ts | 112 ++++++++++++++++++ packages/cli/src/webhooks/webhook.service.ts | 14 ++- 2 files changed, 125 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/webhooks/__tests__/webhook.service.test.ts b/packages/cli/src/webhooks/__tests__/webhook.service.test.ts index 90518ee5109..f4c1c7ac605 100644 --- a/packages/cli/src/webhooks/__tests__/webhook.service.test.ts +++ b/packages/cli/src/webhooks/__tests__/webhook.service.test.ts @@ -386,4 +386,116 @@ describe('WebhookService', () => { expect(webhookRepository.findBy).toHaveBeenCalledTimes(2); }); }); + + describe('isDynamicPath', () => { + test.each(['a', 'a/b'])('should treat static path (%s) as static', (path) => { + const workflow = new Workflow({ + id: 'test-workflow', + nodes: [], + connections: {}, + active: true, + nodeTypes, + }); + + const node = mock({ + name: 'Webhook', + type: 'n8n-nodes-base.webhook', + }); + + const nodeType = mock({ + description: { + webhooks: [ + { + name: 'default', + httpMethod: 'GET', + path, + isFullPath: false, + restartWebhook: false, + }, + ], + }, + }); + + nodeTypes.getByNameAndVersion.mockReturnValue(nodeType); + + const webhooks = webhookService.getNodeWebhooks(workflow, node, additionalData); + + expect(webhooks).toHaveLength(1); + expect(webhooks[0].webhookId).toBeUndefined(); + }); + + test.each([':', '/:'])('should treat literal colon path (%s) as static', (path) => { + const workflow = new Workflow({ + id: 'test-workflow', + nodes: [], + connections: {}, + active: true, + nodeTypes, + }); + + const nodeWithWebhookId = mock({ + name: 'Webhook', + type: 'n8n-nodes-base.webhook', + }); + + const nodeType = mock({ + description: { + webhooks: [ + { + name: 'default', + httpMethod: 'GET', + path, + isFullPath: false, + restartWebhook: false, + }, + ], + }, + }); + + nodeTypes.getByNameAndVersion.mockReturnValue(nodeType); + + const webhooks = webhookService.getNodeWebhooks(workflow, nodeWithWebhookId, additionalData); + + expect(webhooks).toHaveLength(1); + expect(webhooks[0].webhookId).toBeUndefined(); + }); + + test('should treat dynamic path (user/:id) as dynamic', () => { + const workflow = new Workflow({ + id: 'test-workflow', + nodes: [], + connections: {}, + active: true, + nodeTypes, + }); + + const nodeWithWebhookId = mock({ + name: 'Webhook', + type: 'n8n-nodes-base.webhook', + disabled: false, + webhookId: 'test-webhook-id', + }); + + const nodeType = mock({ + description: { + webhooks: [ + { + name: 'default', + httpMethod: 'GET', + path: 'user/:id', + isFullPath: false, + restartWebhook: false, + }, + ], + }, + }); + + nodeTypes.getByNameAndVersion.mockReturnValue(nodeType); + + const webhooks = webhookService.getNodeWebhooks(workflow, nodeWithWebhookId, additionalData); + + expect(webhooks).toHaveLength(1); + expect(webhooks[0].webhookId).toBe('test-webhook-id'); + }); + }); }); diff --git a/packages/cli/src/webhooks/webhook.service.ts b/packages/cli/src/webhooks/webhook.service.ts index eb5e1733078..e9571a67a53 100644 --- a/packages/cli/src/webhooks/webhook.service.ts +++ b/packages/cli/src/webhooks/webhook.service.ts @@ -139,6 +139,17 @@ export class WebhookService { .then((rows) => rows.map((r) => r.method)); } + private isDynamicPath(rawPath: string) { + const firstSlashIndex = rawPath.indexOf('/'); + const path = firstSlashIndex !== -1 ? rawPath.substring(firstSlashIndex + 1) : rawPath; + + // if dynamic, first segment is webhook ID so disregard it + + if (path === '' || path === ':' || path === '/:') return false; + + return path.startsWith(':') || path.includes('/:'); + } + /** * Returns all the webhooks which should be created for the give node */ @@ -232,7 +243,8 @@ export class WebhookService { } let webhookId: string | undefined; - if ((path.startsWith(':') || path.includes('/:')) && node.webhookId) { + + if (this.isDynamicPath(path) && node.webhookId) { webhookId = node.webhookId; }