feat: Prevent webhook url takeover (#14783)

packages/cli/src/server.ts

@@ -69,6 +69,7 @@ import '@/evaluation.ee/test-definitions.controller.ee';
 import '@/evaluation.ee/test-runs.controller.ee';
 import '@/workflows/workflow-history.ee/workflow-history.controller.ee';
 import '@/workflows/workflows.controller';
+import '@/webhooks/webhooks.controller';
 
 @Service()
 export class Server extends AbstractServer {

packages/cli/src/webhooks/webhook.service.ts

@@ -19,7 +19,7 @@ import { WebhookRepository } from '@/databases/repositories/webhook.repository';
 import { NodeTypes } from '@/node-types';
 import { CacheService } from '@/services/cache/cache.service';
 
-type Method = NonNullable<IHttpRequestMethods>;
+import type { Method } from './webhook.types';
 
 @Service()
 export class WebhookService {

packages/cli/src/webhooks/webhook.types.ts

@@ -35,3 +35,5 @@ export interface IWebhookResponseCallbackData {
 	noWebhookResponse?: boolean;
 	responseCode?: number;
 }
+
+export type Method = NonNullable<IHttpRequestMethods>;

packages/cli/src/webhooks/webhooks.controller.ts

@@ -0,0 +1,23 @@
+import { Post, RestController } from '@n8n/decorators';
+import { Request } from 'express';
+import get from 'lodash/get';
+
+import { WebhookService } from './webhook.service';
+import type { Method } from './webhook.types';
+
+@RestController('/webhooks')
+export class WebhooksController {
+	constructor(private readonly webhookService: WebhookService) {}
+
+	@Post('/find')
+	async findWebhook(req: Request) {
+		const body = get(req, 'body', {}) as { path: string; method: Method };
+
+		try {
+			const webhook = await this.webhookService.findWebhook(body.method, body.path);
+			return webhook;
+		} catch (error) {
+			return null;
+		}
+	}
+}

packages/frontend/editor-ui/src/api/webhooks.ts

@@ -0,0 +1,17 @@
+import { makeRestApiRequest } from '@/utils/apiUtils';
+import type { IRestApiContext } from '@/Interface';
+import type { IHttpRequestMethods } from 'n8n-workflow';
+
+type WebhookData = {
+	workflowId: string;
+	webhookPath: string;
+	method: IHttpRequestMethods;
+	node: string;
+};
+
+export const findWebhook = async (
+	context: IRestApiContext,
+	data: { path: string; method: string },
+): Promise<WebhookData | null> => {
+	return await makeRestApiRequest(context, 'POST', '/webhooks/find', data);
+};

packages/frontend/editor-ui/src/components/Modals.vue

@@ -35,6 +35,7 @@ import {
 	COMMUNITY_PLUS_ENROLLMENT_MODAL,
 	DELETE_FOLDER_MODAL_KEY,
 	MOVE_FOLDER_MODAL_KEY,
+	WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY,
 } from '@/constants';
 
 import AboutModal from '@/components/AboutModal.vue';
@@ -71,6 +72,7 @@ import ProjectMoveResourceModal from '@/components/Projects/ProjectMoveResourceM
 import NewAssistantSessionModal from '@/components/AskAssistant/NewAssistantSessionModal.vue';
 import PromptMfaCodeModal from './PromptMfaCodeModal/PromptMfaCodeModal.vue';
 import CommunityPlusEnrollmentModal from '@/components/CommunityPlusEnrollmentModal.vue';
+import WorkflowActivationConflictingWebhookModal from '@/components/WorkflowActivationConflictingWebhookModal.vue';
 import type { EventBus } from '@n8n/utils/event-bus';
 </script>
 
@@ -294,5 +296,11 @@ import type { EventBus } from '@n8n/utils/event-bus';
 				<MoveToFolderModal :modal-name="modalName" :active-id="activeId" :data="data" />
 			</template>
 		</ModalRoot>
+
+		<ModalRoot :name="WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY">
+			<template #default="{ modalName, data }">
+				<WorkflowActivationConflictingWebhookModal :data="data" :modal-name="modalName" />
+			</template>
+		</ModalRoot>
 	</div>
 </template>

packages/frontend/editor-ui/src/components/WorkflowActivationConflictingWebhookModal.test.ts

@@ -0,0 +1,65 @@
+import { createTestingPinia } from '@pinia/testing';
+import { createComponentRenderer } from '@/__tests__/render';
+import WorkflowActivationConflictingWebhookModal from '@/components/WorkflowActivationConflictingWebhookModal.vue';
+import { WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY } from '@/constants';
+
+import { waitFor } from '@testing-library/vue';
+
+vi.mock('@/stores/ui.store', () => {
+	return {
+		useUIStore: vi.fn(() => ({
+			closeModal: vi.fn(),
+		})),
+	};
+});
+vi.mock('@/stores/root.store', () => {
+	return {
+		useRootStore: vi.fn(() => ({
+			webhookUrl: 'http://webhook-base',
+			urlBaseEditor: 'http://editor-base',
+		})),
+	};
+});
+
+const renderComponent = createComponentRenderer(WorkflowActivationConflictingWebhookModal, {
+	global: {
+		stubs: {
+			Modal: {
+				template:
+					'<div role="dialog"><slot name="header" /><slot name="content" /><slot name="footer" /></div>',
+			},
+		},
+	},
+});
+
+describe('WorkflowActivationConflictingWebhookModal', () => {
+	beforeEach(() => {
+		createTestingPinia();
+	});
+
+	it('should render modal', async () => {
+		const props = {
+			modalName: WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY,
+			data: {
+				triggerName: 'Trigger in this workflow',
+				workflowName: 'Test Workflow',
+				workflowId: '123',
+				webhookPath: 'webhook-path',
+				method: 'GET',
+				node: 'Node in workflow',
+			},
+		};
+
+		const wrapper = renderComponent({ props });
+		await waitFor(() => {
+			expect(wrapper.queryByTestId('conflicting-webhook-callout')).toBeInTheDocument();
+		});
+
+		expect(wrapper.getByTestId('conflicting-webhook-callout')).toHaveTextContent(
+			"A webhook trigger 'Node in workflow' in the workflow 'Test Workflow' uses a conflicting URL path, so this workflow cannot be activated",
+		);
+		expect(wrapper.getByTestId('conflicting-webhook-path')).toHaveTextContent(
+			'http://webhook-base/webhook-path',
+		);
+	});
+});

packages/frontend/editor-ui/src/components/WorkflowActivationConflictingWebhookModal.vue

@@ -0,0 +1,84 @@
+<script setup lang="ts">
+import { createEventBus } from '@n8n/utils/event-bus';
+import Modal from '@/components/Modal.vue';
+import { WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY } from '@/constants';
+import { useUIStore } from '@/stores/ui.store';
+
+import { useRootStore } from '@/stores/root.store';
+import { computed } from 'vue';
+
+const modalBus = createEventBus();
+const uiStore = useUIStore();
+const rootStore = useRootStore();
+
+const props = defineProps<{
+	data: {
+		workflowName: string;
+		workflowId: string;
+		webhookPath: string;
+		node: string;
+	};
+}>();
+
+const { data } = props;
+
+const webhookUrl = computed(() => {
+	return rootStore.webhookUrl;
+});
+
+const workflowUrl = computed(() => {
+	return rootStore.urlBaseEditor + 'workflow/' + data.workflowId;
+});
+
+const onClick = async () => {
+	uiStore.closeModal(WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY);
+};
+</script>
+
+<template>
+	<Modal
+		width="540px"
+		:name="WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY"
+		title="Conflicting Webhook Path"
+		:event-bus="modalBus"
+		:center="true"
+	>
+		<template #content>
+			<n8n-callout theme="danger" data-test-id="conflicting-webhook-callout">
+				A webhook trigger '{{ data.node }}' in the workflow '{{ data.workflowName }}' uses a
+				conflicting URL path, so this workflow cannot be activated
+			</n8n-callout>
+			<div :class="$style.container">
+				<div>
+					<n8n-text color="text-base"> You can deactivate </n8n-text>
+					<n8n-link :to="workflowUrl" underline="true"> '{{ data.workflowName }}' </n8n-link>
+					<n8n-text color="text-base">
+						and activate this one, or adjust the following URL path in either workflow:
+					</n8n-text>
+				</div>
+			</div>
+			<div data-test-id="conflicting-webhook-path">
+				<n8n-text color="text-light"> {{ webhookUrl }}/</n8n-text>
+				<n8n-text color="text-dark" bold>
+					{{ data.webhookPath }}
+				</n8n-text>
+			</div>
+		</template>
+		<template #footer>
+			<n8n-button
+				label="Done"
+				size="medium"
+				float="right"
+				data-test-id="close-button"
+				@click="onClick"
+			/>
+		</template>
+	</Modal>
+</template>
+
+<style module lang="scss">
+.container {
+	margin-top: var(--spacing-m);
+	margin-bottom: var(--spacing-s);
+}
+</style>

packages/frontend/editor-ui/src/components/WorkflowActivator.vue

@@ -7,11 +7,19 @@ import type { VNode } from 'vue';
 import { computed, h, watch } from 'vue';
 import { useI18n } from '@/composables/useI18n';
 import type { PermissionsRecord } from '@/permissions';
-import { EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE, PLACEHOLDER_EMPTY_WORKFLOW_ID } from '@/constants';
+import {
+	WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY,
+	EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE,
+	PLACEHOLDER_EMPTY_WORKFLOW_ID,
+} from '@/constants';
 import WorkflowActivationErrorMessage from './WorkflowActivationErrorMessage.vue';
 import { useCredentialsStore } from '@/stores/credentials.store';
 import type { INodeUi, IUsedCredential } from '@/Interface';
 import { OPEN_AI_API_CREDENTIAL_TYPE } from 'n8n-workflow';
+import { useUIStore } from '@/stores/ui.store';
+
+import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers';
+import { useRouter } from 'vue-router';
 
 const props = defineProps<{
 	workflowActive: boolean;
@@ -26,6 +34,11 @@ const emit = defineEmits<{
 const { showMessage } = useToast();
 const workflowActivate = useWorkflowActivate();
 
+const uiStore = useUIStore();
+
+const router = useRouter();
+const workflowHelpers = useWorkflowHelpers({ router });
+
 const i18n = useI18n();
 const workflowsStore = useWorkflowsStore();
 const credentialsStore = useCredentialsStore();
@@ -47,9 +60,12 @@ const isCurrentWorkflow = computed((): boolean => {
 	return workflowsStore.workflowId === props.workflowId;
 });
 
+const foundTriggers = computed(() =>
+	getActivatableTriggerNodes(workflowsStore.workflowTriggerNodes),
+);
+
 const containsTrigger = computed((): boolean => {
-	const foundTriggers = getActivatableTriggerNodes(workflowsStore.workflowTriggerNodes);
+	return foundTriggers.value.length > 0;
-	return foundTriggers.length > 0;
 });
 
 const containsOnlyExecuteWorkflowTrigger = computed((): boolean => {
@@ -114,10 +130,31 @@ const shouldShowFreeAiCreditsWarning = computed((): boolean => {
 });
 
 async function activeChanged(newActiveState: boolean) {
+	if (!isWorkflowActive.value) {
+		const conflictData = await workflowHelpers.checkConflictingWebhooks(props.workflowId);
+
+		if (conflictData) {
+			const { trigger, conflict } = conflictData;
+			const conflictingWorkflow = await workflowsStore.fetchWorkflow(conflict.workflowId);
+
+			uiStore.openModalWithData({
+				name: WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY,
+				data: {
+					triggerName: trigger.name,
+					workflowName: conflictingWorkflow.name,
+					...conflict,
+				},
+			});
+
+			return;
+		}
+	}
+
 	const newState = await workflowActivate.updateWorkflowActivation(
 		props.workflowId,
 		newActiveState,
 	);
+
 	emit('update:workflowActive', { id: props.workflowId, active: newState });
 }
 

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

@@ -1,4 +1,4 @@
-import type { IWorkflowDataUpdate } from '@/Interface';
+import type { IWorkflowData, IWorkflowDataUpdate, IWorkflowDb } from '@/Interface';
 import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers';
 import router from '@/router';
 import { createTestingPinia } from '@pinia/testing';
@@ -6,8 +6,10 @@ import { setActivePinia } from 'pinia';
 import { useWorkflowsStore } from '@/stores/workflows.store';
 import { useWorkflowsEEStore } from '@/stores/workflows.ee.store';
 import { useTagsStore } from '@/stores/tags.store';
+import { useUIStore } from '@/stores/ui.store';
 import { createTestWorkflow } from '@/__tests__/mocks';
-import type { AssignmentCollectionValue } from 'n8n-workflow';
+import { WEBHOOK_NODE_TYPE, type AssignmentCollectionValue } from 'n8n-workflow';
+import * as apiWebhooks from '../api/webhooks';
 
 const getDuplicateTestWorkflow = (): IWorkflowDataUpdate => ({
 	name: 'Duplicate webhook test',
@@ -59,12 +61,14 @@ describe('useWorkflowHelpers', () => {
 	let workflowsStore: ReturnType<typeof useWorkflowsStore>;
 	let workflowsEEStore: ReturnType<typeof useWorkflowsEEStore>;
 	let tagsStore: ReturnType<typeof useTagsStore>;
+	let uiStore: ReturnType<typeof useUIStore>;
 
 	beforeAll(() => {
 		setActivePinia(createTestingPinia());
 		workflowsStore = useWorkflowsStore();
 		workflowsEEStore = useWorkflowsEEStore();
 		tagsStore = useTagsStore();
+		uiStore = useUIStore();
 	});
 
 	afterEach(() => {
@@ -370,4 +374,84 @@ describe('useWorkflowHelpers', () => {
 			expect(upsertTagsSpy).toHaveBeenCalledWith([]);
 		});
 	});
+
+	describe('checkConflictingWebhooks', () => {
+		it('should return null if no conflicts', async () => {
+			const workflowHelpers = useWorkflowHelpers({ router });
+			uiStore.stateIsDirty = false;
+			vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({
+				nodes: [],
+			} as unknown as IWorkflowDb);
+			expect(await workflowHelpers.checkConflictingWebhooks('12345')).toEqual(null);
+		});
+
+		it('should return conflicting webhook data and workflow id is different', async () => {
+			const workflowHelpers = useWorkflowHelpers({ router });
+			uiStore.stateIsDirty = false;
+			vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({
+				nodes: [
+					{
+						type: WEBHOOK_NODE_TYPE,
+						parameters: {
+							method: 'GET',
+							path: 'test-path',
+						},
+					},
+				],
+			} as unknown as IWorkflowDb);
+			vi.spyOn(apiWebhooks, 'findWebhook').mockResolvedValue({
+				method: 'GET',
+				webhookPath: 'test-path',
+				node: 'Webhook 1',
+				workflowId: '456',
+			});
+			expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual({
+				conflict: {
+					method: 'GET',
+					node: 'Webhook 1',
+					webhookPath: 'test-path',
+					workflowId: '456',
+				},
+				trigger: {
+					parameters: {
+						method: 'GET',
+						path: 'test-path',
+					},
+					type: 'n8n-nodes-base.webhook',
+				},
+			});
+		});
+
+		it('should return null if webhook already exist but workflow id is the same', async () => {
+			const workflowHelpers = useWorkflowHelpers({ router });
+			uiStore.stateIsDirty = false;
+			vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({
+				nodes: [
+					{
+						type: WEBHOOK_NODE_TYPE,
+						parameters: {
+							method: 'GET',
+							path: 'test-path',
+						},
+					},
+				],
+			} as unknown as IWorkflowDb);
+			vi.spyOn(apiWebhooks, 'findWebhook').mockResolvedValue({
+				method: 'GET',
+				webhookPath: 'test-path',
+				node: 'Webhook 1',
+				workflowId: '123',
+			});
+			expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual(null);
+		});
+
+		it('should call getWorkflowDataToSave if state is dirty', async () => {
+			const workflowHelpers = useWorkflowHelpers({ router });
+			uiStore.stateIsDirty = true;
+			vi.spyOn(workflowHelpers, 'getWorkflowDataToSave').mockResolvedValue({
+				nodes: [],
+			} as unknown as IWorkflowData);
+			expect(await workflowHelpers.checkConflictingWebhooks('12345')).toEqual(null);
+		});
+	});
 });

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

@@ -26,7 +26,12 @@ import type {
 	NodeParameterValue,
 	Workflow,
 } from 'n8n-workflow';
-import { NodeConnectionTypes, ExpressionEvaluatorProxy, NodeHelpers } from 'n8n-workflow';
+import {
+	NodeConnectionTypes,
+	ExpressionEvaluatorProxy,
+	NodeHelpers,
+	WEBHOOK_NODE_TYPE,
+} from 'n8n-workflow';
 
 import type {
 	ICredentialsResponse,
@@ -71,6 +76,7 @@ import { useProjectsStore } from '@/stores/projects.store';
 import { useTagsStore } from '@/stores/tags.store';
 import { useWorkflowsEEStore } from '@/stores/workflows.ee.store';
 import { useNpsSurveyStore } from '@/stores/npsSurvey.store';
+import { findWebhook } from '../api/webhooks';
 
 export type ResolveParameterOptions = {
 	targetItem?: TargetItem;
@@ -851,6 +857,22 @@ export function useWorkflowHelpers(options: { router: ReturnType<typeof useRoute
 
 			workflowDataRequest.versionId = workflowsStore.workflowVersionId;
 
+			// workflow should not be active if there is live webhook with the same path
+			const conflictData = await checkConflictingWebhooks(currentWorkflow);
+			if (conflictData) {
+				workflowDataRequest.active = false;
+
+				if (workflowsStore.isWorkflowActive) {
+					toast.showMessage({
+						title: 'Conflicting Webhook Path',
+						message: `Workflow set to inactive: Live webhook in another workflow uses same path as node '${conflictData.trigger.name}'.`,
+						type: 'error',
+					});
+
+					workflowsStore.setWorkflowInactive(currentWorkflow);
+				}
+			}
+
 			const workflowData = await workflowsStore.updateWorkflow(
 				currentWorkflow,
 				workflowDataRequest,
@@ -993,6 +1015,20 @@ export function useWorkflowHelpers(options: { router: ReturnType<typeof useRoute
 				return true;
 			}
 
+			// workflow should not be active if there is live webhook with the same path
+			if (workflowData.active) {
+				const conflict = await checkConflictingWebhooks(workflowData.id);
+				if (conflict) {
+					workflowData.active = false;
+
+					toast.showMessage({
+						title: 'Conflicting Webhook Path',
+						message: `Workflow set to inactive: Live webhook in another workflow uses same path as node '${conflict.trigger.name}'.`,
+						type: 'error',
+					});
+				}
+			}
+
 			workflowsStore.setActive(workflowData.active || false);
 			workflowsStore.setWorkflowId(workflowData.id);
 			workflowsStore.setWorkflowVersionId(workflowData.versionId);
@@ -1221,6 +1257,36 @@ export function useWorkflowHelpers(options: { router: ReturnType<typeof useRoute
 		return workflow.nodes.some((node) => node.type.startsWith(packageName));
 	};
 
+	async function checkConflictingWebhooks(workflowId: string) {
+		let data;
+		if (uiStore.stateIsDirty) {
+			data = await getWorkflowDataToSave();
+		} else {
+			data = await workflowsStore.fetchWorkflow(workflowId);
+		}
+
+		const webhookTriggers = data.nodes.filter(
+			(node) => node.disabled !== true && node.type === WEBHOOK_NODE_TYPE,
+		);
+
+		for (const trigger of webhookTriggers) {
+			const method = (trigger.parameters.method as string) ?? 'GET';
+
+			const path = trigger.parameters.path as string;
+
+			const conflict = await findWebhook(rootStore.restApiContext, {
+				path,
+				method,
+			});
+
+			if (conflict && conflict.workflowId !== workflowId) {
+				return { trigger, conflict };
+			}
+		}
+
+		return null;
+	}
+
 	return {
 		setDocumentTitle,
 		resolveParameter,
@@ -1248,5 +1314,6 @@ export function useWorkflowHelpers(options: { router: ReturnType<typeof useRoute
 		initState,
 		getNodeParametersWithResolvedExpressions,
 		containsNodeFromPackage,
+		checkConflictingWebhooks,
 	};
 }

packages/frontend/editor-ui/src/constants.ts

@@ -78,6 +78,8 @@ export const EXTERNAL_SECRETS_PROVIDER_MODAL_KEY = 'externalSecretsProvider';
 export const COMMUNITY_PLUS_ENROLLMENT_MODAL = 'communityPlusEnrollment';
 export const DELETE_FOLDER_MODAL_KEY = 'deleteFolder';
 export const MOVE_FOLDER_MODAL_KEY = 'moveFolder';
+export const WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY =
+	'workflowActivationConflictingWebhook';
 
 export const COMMUNITY_PACKAGE_MANAGE_ACTIONS = {
 	UNINSTALL: 'uninstall',

packages/frontend/editor-ui/src/stores/ui.store.ts

@@ -39,6 +39,7 @@ import {
 	API_KEY_CREATE_OR_EDIT_MODAL_KEY,
 	DELETE_FOLDER_MODAL_KEY,
 	MOVE_FOLDER_MODAL_KEY,
+	WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY,
 } from '@/constants';
 import type {
 	INodeUi,
@@ -182,6 +183,15 @@ export const useUIStore = defineStore(STORES.UI, () => {
 				customHeading: undefined,
 			},
 		},
+		[WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY]: {
+			open: false,
+			data: {
+				workflowName: '',
+				workflowId: '',
+				webhookPath: '',
+				node: '',
+			},
+		},
 	});
 
 	const modalStack = ref<string[]>([]);