From bec74aeb4fda198853b3ea82ed135a1db3ba4988 Mon Sep 17 00:00:00 2001 From: Daria Date: Wed, 6 May 2026 14:42:12 +0300 Subject: [PATCH] fix(core): Add workflow structure validation (#29699) --- packages/cli/src/services/import.service.ts | 3 +- packages/cli/src/workflow-helpers.ts | 24 +- .../workflow-creation.service.test.ts | 19 ++ .../__tests__/workflow.service.test.ts | 21 ++ .../workflows/workflow-creation.service.ts | 1 + .../cli/src/workflows/workflow.service.ts | 4 + .../workflows/workflow.service.test.ts | 40 ++- .../workflows/workflows.controller.test.ts | 1 + .../frontend/@n8n/i18n/src/locales/en.json | 2 + .../composables/useCanvasOperations.test.ts | 98 ++++++- .../app/composables/useCanvasOperations.ts | 42 ++- .../composables/useWorkflowInitialization.ts | 63 +++- .../src/app/utils/workflowUtils.test.ts | 188 +++++++++++- .../editor-ui/src/app/utils/workflowUtils.ts | 74 ++++- packages/workflow/src/index.ts | 1 + .../src/workflow-structure-validation.ts | 198 +++++++++++++ .../workflow-structure-validation.test.ts | 268 ++++++++++++++++++ 17 files changed, 1021 insertions(+), 26 deletions(-) create mode 100644 packages/workflow/src/workflow-structure-validation.ts create mode 100644 packages/workflow/test/workflow-structure-validation.test.ts diff --git a/packages/cli/src/services/import.service.ts b/packages/cli/src/services/import.service.ts index 34f8e9d67fa..3e9afd97378 100644 --- a/packages/cli/src/services/import.service.ts +++ b/packages/cli/src/services/import.service.ts @@ -24,7 +24,7 @@ import { import { v4 as uuid } from 'uuid'; import { readdir, readFile } from 'fs/promises'; -import { replaceInvalidCredentials } from '@/workflow-helpers'; +import { replaceInvalidCredentials, validateWorkflowStructure } from '@/workflow-helpers'; import { validateDbTypeForImportEntities } from '@/utils/validate-database-type'; import { Cipher } from 'n8n-core'; import { decompressFolder } from '@/utils/compression.util'; @@ -114,6 +114,7 @@ export class ImportService { const hasInvalidCreds = workflow.nodes.some((node) => !node.credentials?.id); if (hasInvalidCreds) await this.replaceInvalidCreds(workflow, projectId); + validateWorkflowStructure(workflow); // Remove workflows from ActiveWorkflowManager BEFORE transaction to prevent orphaned trigger listeners // Only remove if the workflow already exists in the database and is active diff --git a/packages/cli/src/workflow-helpers.ts b/packages/cli/src/workflow-helpers.ts index 0cbb1329270..77c07dd9a6a 100644 --- a/packages/cli/src/workflow-helpers.ts +++ b/packages/cli/src/workflow-helpers.ts @@ -12,7 +12,11 @@ import type { IWorkflowSettings, RelatedExecution, } from 'n8n-workflow'; -import { resolveNodeWebhookId } from 'n8n-workflow'; +import { + formatWorkflowStructureIssuePath, + resolveNodeWebhookId, + safeParseWorkflowStructure, +} from 'n8n-workflow'; import { v4 as uuid } from 'uuid'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; @@ -121,6 +125,24 @@ export function resolveNodeWebhookIds(workflow: IWorkflowBase, nodeTypes: INodeT } } +export function validateWorkflowStructure(workflow: Pick) { + const result = safeParseWorkflowStructure(workflow); + + if (result.success) return; + + const details = result.issues + .map(({ path, message, code }) => { + const formattedPath = Array.isArray(path) + ? formatWorkflowStructureIssuePath(path) + : 'workflow'; + + return `${formattedPath} (${code}): ${message}`; + }) + .join('; '); + + throw new BadRequestError(`Workflow structure is invalid. ${details}`); +} + /** * Removes default values from workflow settings to avoid storing them in the database. * Returns a new settings object without mutating the original. diff --git a/packages/cli/src/workflows/__tests__/workflow-creation.service.test.ts b/packages/cli/src/workflows/__tests__/workflow-creation.service.test.ts index 83cc2c2cbdd..ec9899389ec 100644 --- a/packages/cli/src/workflows/__tests__/workflow-creation.service.test.ts +++ b/packages/cli/src/workflows/__tests__/workflow-creation.service.test.ts @@ -10,6 +10,7 @@ import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { userHasScopes } from '@/permissions.ee/check-access'; import type { ProjectService } from '@/services/project.service.ee'; +import * as WorkflowHelpers from '@/workflow-helpers'; import { WorkflowCreationService } from '@/workflows/workflow-creation.service'; import type { NodeTypes } from '@/node-types'; import type { EnterpriseWorkflowService } from '@/workflows/workflow.service.ee'; @@ -85,6 +86,24 @@ describe('WorkflowCreationService', () => { } describe('createWorkflow()', () => { + it('should throw BadRequestError for invalid workflow structure', async () => { + projectServiceMock.getProjectWithScope.mockResolvedValue({ id: 'project-1' } as never); + licenseStateMock.isSharingLicensed.mockReturnValue(false); + jest.mocked(WorkflowHelpers.validateWorkflowStructure).mockImplementationOnce(() => { + throw new BadRequestError('Workflow structure is invalid. nodes[0].type: Required'); + }); + + const user = mock(); + const newWorkflow = new WorkflowEntity(); + newWorkflow.name = 'Test'; + newWorkflow.nodes = [{ name: 'Start', position: [0, 0], parameters: {} }] as never; + newWorkflow.connections = {}; + + await expect( + workflowCreationService.createWorkflow(user, newWorkflow, { projectId: 'project-1' }), + ).rejects.toThrow('Workflow structure is invalid.'); + }); + describe('credential retrieval', () => { it('should include global credentials when checking credential permissions', async () => { /** diff --git a/packages/cli/src/workflows/__tests__/workflow.service.test.ts b/packages/cli/src/workflows/__tests__/workflow.service.test.ts index 33ef0b287f9..e95b951d5d9 100644 --- a/packages/cli/src/workflows/__tests__/workflow.service.test.ts +++ b/packages/cli/src/workflows/__tests__/workflow.service.test.ts @@ -4,6 +4,7 @@ import type { Scope } from '@n8n/permissions'; import type { MockProxy } from 'jest-mock-extended'; import { mock } from 'jest-mock-extended'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { userHasScopes } from '@/permissions.ee/check-access'; import type { OwnershipService } from '@/services/ownership.service'; import type { RoleService } from '@/services/role.service'; @@ -236,6 +237,26 @@ describe('WorkflowService', () => { return { settings } as unknown as WorkflowEntity; } + test('should throw BadRequestError for invalid workflow structure', async () => { + setupExistingWorkflow(); + jest.mocked(WorkflowHelpers.validateWorkflowStructure).mockImplementationOnce(() => { + throw new BadRequestError('Workflow structure is invalid. nodes[0].position: Required'); + }); + + const user = mock(); + + await expect( + workflowService.update( + user, + { + nodes: [{ name: 'Start', type: 'n8n-nodes-base.manualTrigger', parameters: {} }], + } as unknown as WorkflowEntity, + 'workflow-1', + { forceSave: true }, + ), + ).rejects.toThrow('Workflow structure is invalid.'); + }); + test('should strip redactionPolicy when user lacks scope and value is changing', async () => { setupExistingWorkflow({ redactionPolicy: 'none' }); userHasScopesMock.mockResolvedValue(false); diff --git a/packages/cli/src/workflows/workflow-creation.service.ts b/packages/cli/src/workflows/workflow-creation.service.ts index a322f774ff0..d9b152105c6 100644 --- a/packages/cli/src/workflows/workflow-creation.service.ts +++ b/packages/cli/src/workflows/workflow-creation.service.ts @@ -109,6 +109,7 @@ export class WorkflowCreationService { WorkflowHelpers.addNodeIds(newWorkflow); WorkflowHelpers.resolveNodeWebhookIds(newWorkflow, this.nodeTypes); + WorkflowHelpers.validateWorkflowStructure(newWorkflow); if ('pinData' in newWorkflow) { WorkflowHelpers.validatePinDataSize(newWorkflow); diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index e52560eac3d..46aabc4f3aa 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -374,6 +374,10 @@ export class WorkflowService { WorkflowHelpers.addNodeIds(workflowUpdateData); WorkflowHelpers.resolveNodeWebhookIds(workflowUpdateData, this.nodeTypes); + WorkflowHelpers.validateWorkflowStructure({ + nodes: workflowUpdateData.nodes ?? workflow.nodes, + connections: workflowUpdateData.connections ?? workflow.connections, + }); // Strip redactionPolicy if instance lacks data-redaction license if ( diff --git a/packages/cli/test/integration/workflows/workflow.service.test.ts b/packages/cli/test/integration/workflows/workflow.service.test.ts index 26b96bf678d..4eb3a0c056d 100644 --- a/packages/cli/test/integration/workflows/workflow.service.test.ts +++ b/packages/cli/test/integration/workflows/workflow.service.test.ts @@ -148,7 +148,29 @@ describe('update()', () => { test('should save workflow history version with backfilled data when connection change', async () => { const owner = await createOwner(); - const workflow = await createWorkflowWithHistory({}, owner); + const workflow = await createWorkflowWithHistory( + { + nodes: [ + { + id: 'uuid-1', + name: 'Manual Trigger', + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + position: [240, 300], + parameters: {}, + }, + { + id: 'uuid-2', + name: 'Code Node', + type: 'n8n-nodes-base.code', + typeVersion: 1, + position: [500, 300], + parameters: {}, + }, + ], + }, + owner, + ); const addRecordSpy = jest.spyOn(workflowPublishHistoryRepository, 'addRecord'); const saveVersionSpy = jest.spyOn(workflowHistoryService, 'saveVersion'); @@ -281,7 +303,7 @@ describe('activateWorkflow()', () => { webhookId: 'version1', name: 'test', typeVersion: 0, - type: '', + type: 'n8n-nodes-base.webhook', position: [1, 2], parameters: {}, }, @@ -290,7 +312,7 @@ describe('activateWorkflow()', () => { webhookId: 'version1-2', name: 'test2', typeVersion: 0, - type: '', + type: 'n8n-nodes-base.webhook', position: [1, 2], parameters: {}, }, @@ -308,9 +330,9 @@ describe('activateWorkflow()', () => { { id: '123', webhookId: 'version2', - name: '', + name: 'updatedNode', typeVersion: 0, - type: '', + type: 'n8n-nodes-base.webhook', position: [1, 2], parameters: {}, }, @@ -330,7 +352,7 @@ describe('activateWorkflow()', () => { webhookId: 'version2', name: 'newNode', typeVersion: 0, - type: '', + type: 'n8n-nodes-base.webhook', position: [1, 2], parameters: {}, }, @@ -354,7 +376,7 @@ describe('activateWorkflow()', () => { webhookId: 'version1', name: 'test', typeVersion: 0, - type: '', + type: 'n8n-nodes-base.webhook', position: [1, 2], parameters: {}, }, @@ -363,7 +385,7 @@ describe('activateWorkflow()', () => { webhookId: 'version1-2', name: 'test2', typeVersion: 0, - type: '', + type: 'n8n-nodes-base.webhook', position: [1, 2], parameters: {}, }, @@ -382,7 +404,7 @@ describe('activateWorkflow()', () => { webhookId: 'version2', name: 'newNode', typeVersion: 0, - type: '', + type: 'n8n-nodes-base.webhook', position: [1, 2], parameters: {}, }, diff --git a/packages/cli/test/integration/workflows/workflows.controller.test.ts b/packages/cli/test/integration/workflows/workflows.controller.test.ts index 3c67803ad08..ba09e38788e 100644 --- a/packages/cli/test/integration/workflows/workflows.controller.test.ts +++ b/packages/cli/test/integration/workflows/workflows.controller.test.ts @@ -3147,6 +3147,7 @@ describe('PATCH /workflows/:workflowId', () => { const payload = { nodes: [], + connections: {}, }; const response = await authOwnerAgent.patch(`/workflows/${workflow.id}`).send(payload); diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index c748380a21f..273175065cd 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -2157,6 +2157,7 @@ "nodeView.showError.workflowError": "Workflow execution had an error", "nodeView.showError.getWorkflowDataFromUrl.title": "Problem loading workflow", "nodeView.showError.importWorkflowData.title": "Problem importing workflow", + "nodeView.showError.importWorkflowData.invalidNodes": "Skipped {count} node(s) with missing type", "nodeView.showError.mounted1.message": "There was a problem loading init data", "nodeView.showError.mounted1.title": "Init Problem", "nodeView.showError.mounted2.message": "There was a problem initializing the workflow", @@ -2258,6 +2259,7 @@ "nodeWebhooks.webhookUrls.chatTrigger": "Chat URL", "nodeWebhooks.webhookUrls.mcpTrigger": "MCP URL", "openWorkflow.workflowImportError": "Could not import workflow", + "openWorkflow.workflowDataInvalidError": "Workflow data is invalid", "openWorkflow.workflowNotFoundError": "Could not find workflow", "oauth.consentView.title": "OAuth access consent", "oauth.consentView.heading": "{clientName} wants access to your n8n instance", diff --git a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts index 845a001f48f..4950a10efe4 100644 --- a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts +++ b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts @@ -3752,7 +3752,7 @@ describe('useCanvasOperations', () => { const newWorkflowId = 'new-workflow-id'; const workflow = createTestWorkflow({ id: newWorkflowId, - nodes: [createTestNode()], + nodes: [createTestNode({ name: 'Node 1' }), createTestNode({ name: 'Node 2' })], connections: testConnections, }); @@ -3768,6 +3768,56 @@ describe('useCanvasOperations', () => { expect(workflowDocumentStore.setConnections).toHaveBeenCalledWith(testConnections); }); + it('should default position to [0, 0] for nodes with missing position', async () => { + const { position: _, ...nodeWithoutPosition } = createTestNode({ name: 'Start' }); + + const workflow = createTestWorkflow({ + id: workflowId, + nodes: [nodeWithoutPosition as INodeUi], + connections: {}, + }); + + const setNodesSpy = vi.spyOn(workflowDocumentStoreInstance, 'setNodes'); + const { initializeWorkspace } = useCanvasOperations(); + await initializeWorkspace(workflow); + + expect(setNodesSpy).toHaveBeenCalledWith([ + expect.objectContaining({ name: 'Start', position: [0, 0] }), + ]); + }); + + it('should remove connections for nodes filtered out during initialization', async () => { + const validNode = createTestNode({ name: 'Start' }); + const invalidNode = createTestNode({ name: 'Missing', type: '' }); + const targetNode = createTestNode({ name: 'End' }); + const workflow = createTestWorkflow({ + id: workflowId, + nodes: [validNode, invalidNode, targetNode], + connections: { + Start: { + main: [ + [ + { node: 'End', type: 'main', index: 0 }, + { node: 'Missing', type: 'main', index: 0 }, + ], + ], + }, + Missing: { + main: [[{ node: 'End', type: 'main', index: 0 }]], + }, + }, + }); + + const { initializeWorkspace } = useCanvasOperations(); + await initializeWorkspace(workflow); + + expect(workflowDocumentStoreInstance.setConnections).toHaveBeenCalledWith({ + Start: { + main: [[{ node: 'End', type: 'main', index: 0 }]], + }, + }); + }); + it('should initialize node data from node type description', async () => { const nodeTypesStore = mockedStore(useNodeTypesStore); const type = SET_NODE_TYPE; @@ -3788,14 +3838,18 @@ describe('useCanvasOperations', () => { nodeTypesStore.nodeTypes = { [type]: { [version]: expectedDescription } }; const workflow = createTestWorkflow({ + id: workflowId, nodes: [createTestNode()], connections: {}, }); + const setNodesSpy = vi.spyOn(workflowDocumentStoreInstance, 'setNodes'); const { initializeWorkspace } = useCanvasOperations(); await initializeWorkspace(workflow); - expect(workflow.nodes[0].parameters).toEqual({ value: true }); + expect(setNodesSpy).toHaveBeenCalledWith([ + expect.objectContaining({ parameters: { value: true } }), + ]); }); }); @@ -4616,6 +4670,46 @@ describe('useCanvasOperations', () => { expect(toast.showError).not.toHaveBeenCalled(); }); + it('should remove connections for nodes filtered out during import', async () => { + vi.mocked(workflowDocumentStoreInstance.createWorkflowObject).mockImplementation( + (nodes, connections) => + createTestWorkflowObject({ + nodes: nodes as INodeUi[], + connections, + }), + ); + + const workflowDataToImport = { + nodes: [ + createTestNode({ id: 'start-id', name: 'Start' }), + createTestNode({ id: 'missing-id', name: 'Missing', type: '' }), + createTestNode({ id: 'end-id', name: 'End' }), + ], + connections: { + Start: { + main: [ + [ + { node: 'End', type: 'main' as const, index: 0 }, + { node: 'Missing', type: 'main' as const, index: 0 }, + ], + ], + }, + Missing: { + main: [[{ node: 'End', type: 'main' as const, index: 0 }]], + }, + }, + }; + + const canvasOperations = useCanvasOperations(); + const workflow = await canvasOperations.importWorkflowData(workflowDataToImport, 'paste'); + + expect(workflow.connections).toEqual({ + Start: { + main: [[{ node: 'End', type: 'main', index: 0 }]], + }, + }); + }); + it.each(UPDATE_WEBHOOK_ID_NODE_TYPES)( 'should regenerate webhook ids for node type "%s" on pasting into canvas', async (type) => { diff --git a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts index 9e33f50faca..6897ed18cf7 100644 --- a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts +++ b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts @@ -118,6 +118,7 @@ import { computed, nextTick, ref, type DeepReadonly } from 'vue'; import { useUniqueNodeName } from '@/app/composables/useUniqueNodeName'; import { injectWorkflowState } from '@/app/composables/useWorkflowState'; import { isPresent, tryToParseNumber } from '@/app/utils/typesUtils'; +import { ensureNodePosition, sanitizeConnections } from '@/app/utils/workflowUtils'; import { useProjectsStore } from '@/features/collaboration/projects/projects.store'; import type { CanvasLayoutEvent } from '@/features/workflows/canvas/composables/useCanvasLayout'; import { chatEventBus } from '@n8n/chat/event-buses'; @@ -2334,7 +2335,14 @@ export function useCanvasOperations() { async function initializeWorkspace(data: IWorkflowDb) { const { workflowDocumentStore: initializedDocumentStore } = await workflowHelpers.initState(data); - data.nodes.forEach((node) => { + + // Filter out nodes with missing type to prevent canvas rendering crashes + const validNodes = data.nodes + .filter((node) => !!node.type) + .map((node) => ({ ...node, position: ensureNodePosition(node.position) })); + const validNodeNames = validNodes.map((node) => node.name); + + validNodes.forEach((node) => { const nodeTypeDescription = requireNodeTypeDescription(node.type, node.typeVersion); const isInstalledNode = nodeTypesStore.getIsNodeInstalled(node.type); nodeHelpers.matchCredentials(node); @@ -2345,8 +2353,8 @@ export function useCanvasOperations() { } }); - initializedDocumentStore.setNodes(data.nodes); - initializedDocumentStore.setConnections(data.connections); + initializedDocumentStore.setNodes(validNodes); + initializedDocumentStore.setConnections(sanitizeConnections(data.connections, validNodeNames)); return { workflowDocumentStore: initializedDocumentStore }; } @@ -2620,6 +2628,34 @@ export function useCanvasOperations() { return {}; } + // Filter out nodes with missing type to prevent crashes + if (workflowData.nodes) { + const invalidNodes = workflowData.nodes.filter((node) => !node.type); + if (invalidNodes.length > 0) { + toast.showError( + new Error( + i18n.baseText('nodeView.showError.importWorkflowData.invalidNodes', { + interpolate: { count: String(invalidNodes.length) }, + }), + ), + i18n.baseText('nodeView.showError.importWorkflowData.title'), + ); + workflowData.nodes = workflowData.nodes.filter((node) => !!node.type); + } + } + + if (workflowData.nodes) { + workflowData.nodes = workflowData.nodes.map((node) => ({ + ...node, + position: ensureNodePosition(node.position), + })); + } + + if (workflowData.connections) { + const validNodeNames = workflowData.nodes?.map((node) => node.name); + workflowData.connections = sanitizeConnections(workflowData.connections, validNodeNames); + } + try { const nodeIdMap: { [prev: string]: string } = {}; if (workflowData.nodes) { diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowInitialization.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowInitialization.ts index 1710a163a7f..12779c8fa88 100644 --- a/packages/frontend/editor-ui/src/app/composables/useWorkflowInitialization.ts +++ b/packages/frontend/editor-ui/src/app/composables/useWorkflowInitialization.ts @@ -1,6 +1,7 @@ import { ref, computed, shallowRef } from 'vue'; import { type RouteRecordNameGeneric, useRoute, useRouter } from 'vue-router'; import { useI18n } from '@n8n/i18n'; +import { safeParseWorkflowStructure, WorkflowStructureValidationError } from 'n8n-workflow'; import { useToast } from '@/app/composables/useToast'; import { useDocumentTitle } from '@/app/composables/useDocumentTitle'; import { useExternalHooks } from '@/app/composables/useExternalHooks'; @@ -154,6 +155,20 @@ export function useWorkflowInitialization(workflowState: WorkflowState) { return true; } + const templateValidation = safeParseWorkflowStructure({ + nodes: workflow.nodes, + connections: workflow.connections, + }); + + if (!templateValidation.success) { + toast.showError( + new WorkflowStructureValidationError(templateValidation.issues), + i18n.baseText('nodeView.showError.openWorkflow.title'), + { message: i18n.baseText('openWorkflow.workflowDataInvalidError') }, + ); + return true; + } + await openWorkflowTemplateFromJSON(workflow); } else { await openWorkflowTemplate(templateId.toString()); @@ -228,6 +243,19 @@ export function useWorkflowInitialization(workflowState: WorkflowState) { } async function openWorkflow(data: IWorkflowDb) { + const validationResult = safeParseWorkflowStructure({ + nodes: data.nodes, + connections: data.connections, + }); + + if (!validationResult.success) { + toast.showError( + new WorkflowStructureValidationError(validationResult.issues), + i18n.baseText('nodeView.showError.openWorkflow.title'), + { message: i18n.baseText('openWorkflow.workflowDataInvalidError') }, + ); + } + disposeCurrentWorkflowDocumentStore(); resetWorkspace(); @@ -237,14 +265,33 @@ export function useWorkflowInitialization(workflowState: WorkflowState) { documentTitle.setDocumentTitle(data.name, 'IDLE'); } - const { workflowDocumentStore } = await initializeWorkspace(data); - currentWorkflowDocumentStore.value = workflowDocumentStore; - currentNDVStore.value = useNDVStore( - createWorkflowDocumentId( - workflowDocumentStore.workflowId, - workflowDocumentStore.workflowVersion, - ), - ); + try { + const { workflowDocumentStore } = await initializeWorkspace(data); + currentWorkflowDocumentStore.value = workflowDocumentStore; + currentNDVStore.value = useNDVStore( + createWorkflowDocumentId( + workflowDocumentStore.workflowId, + workflowDocumentStore.workflowVersion, + ), + ); + } catch (error) { + // Using error instead of warn so that unexpected errors are captured by Sentry + console.error('Failed to initialize workspace for workflow', { + workflowId: data.id, + error, + }); + toast.showError(error, i18n.baseText('nodeView.showError.openWorkflow.title')); + + // Set up a minimal document store so the UI stays functional + workflowsStore.setWorkflowId(data.id); + const workflowDocumentId = createWorkflowDocumentId(data.id); + currentWorkflowDocumentStore.value = useWorkflowDocumentStore(workflowDocumentId); + currentWorkflowDocumentStore.value.setName(data.name); + currentWorkflowDocumentStore.value.setHomeProject(data.homeProject ?? null); + currentWorkflowDocumentStore.value.setScopes(data.scopes ?? []); + currentNDVStore.value = useNDVStore(workflowDocumentId); + return; + } void externalHooks.run('workflow.open', { workflowId: data.id, diff --git a/packages/frontend/editor-ui/src/app/utils/workflowUtils.test.ts b/packages/frontend/editor-ui/src/app/utils/workflowUtils.test.ts index 0cfb3228aee..8909b7304c6 100644 --- a/packages/frontend/editor-ui/src/app/utils/workflowUtils.test.ts +++ b/packages/frontend/editor-ui/src/app/utils/workflowUtils.test.ts @@ -2,9 +2,12 @@ import { removeWorkflowExecutionData, convertWorkflowTagsToIds, sortNodesByExecutionOrder, + sanitizeConnections, + ensureNodePosition, } from './workflowUtils'; import type { IWorkflowDb } from '@/Interface'; -import type { INodeIssues } from 'n8n-workflow'; +import type { IConnection, IConnections, INodeIssues } from 'n8n-workflow'; +import { NodeConnectionTypes } from 'n8n-workflow'; describe('workflowUtils', () => { describe('convertWorkflowTagsToIds', () => { @@ -577,4 +580,187 @@ describe('workflowUtils', () => { expect(result.map((n) => n.node.name)).toEqual(['Trigger', 'A']); }); }); + + describe('ensureNodePosition', () => { + it('should return valid position as-is', () => { + expect(ensureNodePosition([100, 200])).toEqual([100, 200]); + }); + + it('should return [0, 0] for undefined', () => { + expect(ensureNodePosition(undefined)).toEqual([0, 0]); + }); + + it('should return [0, 0] for a string', () => { + expect(ensureNodePosition('bad')).toEqual([0, 0]); + }); + + it('should return [0, 0] for an array with fewer than 2 elements', () => { + expect(ensureNodePosition([100])).toEqual([0, 0]); + }); + + it('should return [0, 0] for an array with non-numeric strings', () => { + expect(ensureNodePosition(['a', 'b'])).toEqual([0, 0]); + }); + + it('should coerce numeric strings to numbers', () => { + expect(ensureNodePosition(['100', '200'])).toEqual([100, 200]); + }); + }); + + describe('sanitizeConnections', () => { + it('should return empty object for empty connections', () => { + expect(sanitizeConnections({})).toEqual({}); + }); + + it('should return empty object for non-object input', () => { + expect(sanitizeConnections('broken')).toEqual({}); + }); + + it('should pass through valid connections unchanged', () => { + const connections: IConnections = { + Start: { + [NodeConnectionTypes.Main]: [[{ node: 'End', type: NodeConnectionTypes.Main, index: 0 }]], + }, + }; + expect(sanitizeConnections(connections)).toEqual(connections); + }); + + it('should strip connection type when buckets is a string', () => { + const connections = { + Start: { main: 'not-an-array' }, + } as unknown as IConnections; + + expect(sanitizeConnections(connections)).toEqual({ Start: {} }); + }); + + it('should strip connection type when buckets is a number', () => { + const connections = { + Start: { main: 42 }, + } as unknown as IConnections; + + expect(sanitizeConnections(connections)).toEqual({ Start: {} }); + }); + + it('should skip node entry when value is not an object', () => { + const connections = { + Start: 'broken', + } as unknown as IConnections; + + expect(sanitizeConnections(connections)).toEqual({}); + }); + + it('should skip node entry when value is null', () => { + const connections = { + Start: null, + } as unknown as IConnections; + + expect(sanitizeConnections(connections)).toEqual({}); + }); + + it('should nullify bucket when it is an object instead of an array', () => { + const connections = { + Start: { + main: [{ node: 'Foo', type: 'main', index: 0 }], + }, + } as unknown as IConnections; + + expect(sanitizeConnections(connections)).toEqual({ + Start: { main: [null] }, + }); + }); + + it('should preserve null buckets in sparse connections', () => { + const connections: IConnections = { + Start: { + [NodeConnectionTypes.Main]: [ + null as unknown as IConnection[], + [{ node: 'End', type: NodeConnectionTypes.Main, index: 0 }], + ], + }, + }; + expect(sanitizeConnections(connections)).toEqual(connections); + }); + + it('should keep valid types and strip malformed ones on the same node', () => { + const connections = { + Start: { + [NodeConnectionTypes.Main]: 'not-an-array', + [NodeConnectionTypes.AiAgent]: [ + [{ node: 'Agent', type: NodeConnectionTypes.AiAgent, index: 0 }], + ], + }, + } as unknown as IConnections; + + expect(sanitizeConnections(connections)).toEqual({ + Start: { + [NodeConnectionTypes.AiAgent]: [ + [{ node: 'Agent', type: NodeConnectionTypes.AiAgent, index: 0 }], + ], + }, + }); + }); + + it('should drop connections whose source or target nodes are missing', () => { + const connections: IConnections = { + Start: { + [NodeConnectionTypes.Main]: [ + [ + { node: 'End', type: NodeConnectionTypes.Main, index: 0 }, + { node: 'Missing', type: NodeConnectionTypes.Main, index: 0 }, + ], + ], + }, + Missing: { + [NodeConnectionTypes.Main]: [[{ node: 'End', type: NodeConnectionTypes.Main, index: 0 }]], + }, + }; + + expect(sanitizeConnections(connections, ['Start', 'End'])).toEqual({ + Start: { + [NodeConnectionTypes.Main]: [[{ node: 'End', type: NodeConnectionTypes.Main, index: 0 }]], + }, + }); + }); + + it('should drop malformed entries inside a valid bucket', () => { + const connections = { + Start: { + [NodeConnectionTypes.Main]: [ + [ + { node: 'End', type: NodeConnectionTypes.Main, index: 0 }, + null, + 'broken', + { node: 'Other', type: NodeConnectionTypes.Main }, + ], + ], + }, + } as unknown as IConnections; + + expect(sanitizeConnections(connections)).toEqual({ + Start: { + [NodeConnectionTypes.Main]: [[{ node: 'End', type: NodeConnectionTypes.Main, index: 0 }]], + }, + }); + }); + + it('should drop malformed entries before checking valid node names', () => { + const connections = { + Start: { + [NodeConnectionTypes.Main]: [ + [ + null, + { node: 'End', type: NodeConnectionTypes.Main, index: 0 }, + { node: 'Missing', type: NodeConnectionTypes.Main, index: 0 }, + ], + ], + }, + } as unknown as IConnections; + + expect(sanitizeConnections(connections, ['Start', 'End'])).toEqual({ + Start: { + [NodeConnectionTypes.Main]: [[{ node: 'End', type: NodeConnectionTypes.Main, index: 0 }]], + }, + }); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/app/utils/workflowUtils.ts b/packages/frontend/editor-ui/src/app/utils/workflowUtils.ts index 81f2ef786e4..b191f513241 100644 --- a/packages/frontend/editor-ui/src/app/utils/workflowUtils.ts +++ b/packages/frontend/editor-ui/src/app/utils/workflowUtils.ts @@ -1,7 +1,8 @@ import type { IWorkflowDb, INodeUi } from '@/Interface'; import type { ITag } from '@n8n/rest-api-client/api/tags'; -import type { IConnections } from 'n8n-workflow'; +import type { IConnection, IConnections, INodeConnections } from 'n8n-workflow'; import { SPLIT_IN_BATCHES_NODE_TYPE } from '@/app/constants'; +import { isObject } from '@/app/utils/objectUtils'; /** * Converts workflow tags from ITag[] (API response format) to string[] (store format) @@ -42,6 +43,77 @@ export function removeWorkflowExecutionData( return sanitizedWorkflow; } +/** + * Return a valid position tuple, defaulting to [0, 0] if missing or malformed. + */ +export function ensureNodePosition(position: unknown): [number, number] { + if (Array.isArray(position) && position.length >= 2) { + const x = Number(position[0]); + const y = Number(position[1]); + if (!Number.isNaN(x) && !Number.isNaN(y)) { + return [x, y]; + } + } + return [0, 0]; +} + +function isValidConnectionEntry(connection: unknown): connection is IConnection { + if (!isObject(connection)) return false; + if (!('node' in connection) || !('type' in connection) || !('index' in connection)) { + return false; + } + + return ( + typeof connection.node === 'string' && + typeof connection.type === 'string' && + typeof connection.index === 'number' + ); +} + +/** + * Strip out malformed connection entries that would crash the canvas. + * Keeps only entries where each connection type maps to an array of buckets, + * each bucket is either an array or null, and each bucket entry matches the + * expected connection shape. When validNodeNames is provided, it also removes + * connections whose source or target node is missing. + */ +export function sanitizeConnections( + connections: unknown, + validNodeNames?: Iterable, +): IConnections { + if (!isObject(connections)) return {}; + + const sanitized: IConnections = {}; + + const validNodeNameSet = validNodeNames ? new Set(validNodeNames) : undefined; + + for (const nodeName of Object.keys(connections)) { + if (validNodeNameSet && !validNodeNameSet.has(nodeName)) continue; + + const nodeConnections = connections[nodeName]; + if (!isObject(nodeConnections)) continue; + + const sanitizedNodeConnections: INodeConnections = {}; + for (const type of Object.keys(nodeConnections)) { + const buckets = nodeConnections[type]; + if (!Array.isArray(buckets)) continue; + sanitizedNodeConnections[type] = buckets.map((bucket) => { + if (!Array.isArray(bucket)) return null; + + return bucket.filter( + (connection): connection is IConnection => + isValidConnectionEntry(connection) && + (!validNodeNameSet || validNodeNameSet.has(connection.node)), + ); + }); + } + + sanitized[nodeName] = sanitizedNodeConnections; + } + + return sanitized; +} + interface ExecutionOrderItem { node: { name: string; position: [number, number] }; isTrigger: boolean; diff --git a/packages/workflow/src/index.ts b/packages/workflow/src/index.ts index dd562f16ee9..c404964708b 100644 --- a/packages/workflow/src/index.ts +++ b/packages/workflow/src/index.ts @@ -30,6 +30,7 @@ export * from './workflow-checksum'; export * from './workflow-data-proxy'; export * from './workflow-data-proxy-env-provider'; export * from './workflow-validation'; +export * from './workflow-structure-validation'; export * from './versioned-node-type'; export * from './type-validation'; export * from './result'; diff --git a/packages/workflow/src/workflow-structure-validation.ts b/packages/workflow/src/workflow-structure-validation.ts new file mode 100644 index 00000000000..3fa3e1a3e2e --- /dev/null +++ b/packages/workflow/src/workflow-structure-validation.ts @@ -0,0 +1,198 @@ +import { z } from 'zod'; + +/** + * Workflow Structure Validation + * + * Single source of truth for validating workflow **structure** — the minimum + * shape that the editor and runtime assume is always present and correctly + * formed. Intentionally separate from activation/publish validation + * (WorkflowValidationService) which checks semantic correctness (trigger + * presence, known node types, credential issues, etc.). + * + * Lives in n8n-workflow so it can be shared by: + * - Backend: create/update/import reject malformed payloads (400) + * - Frontend: open path warns but still renders; import path blocks + * + * Validates: + * - Required node fields (name, type, position) + * - Position is a 2-number tuple + * - Connection entries have valid node/type/index + * - No duplicate node names + * - Connection source/target keys reference existing nodes + * + * Does NOT validate: + * - Whether a node type is installed + * - Node parameter correctness + * - Credential validity + * - Activation readiness + * + */ + +const workflowNodeStructureSchema = z + .object({ + name: z.string().min(1), + type: z.string().min(1), + position: z.tuple([z.number(), z.number()]), + parameters: z.record(z.string(), z.unknown()).optional(), + id: z.string().optional(), + typeVersion: z.number().optional(), + disabled: z.boolean().optional(), + }) + .passthrough(); + +const connectionEntrySchema = z + .object({ + node: z.string().min(1), + type: z.string().min(1), + index: z.number().int().min(0), + }) + .passthrough(); + +// Buckets can be null when a multi-output node has unused output slots. +// Matches NodeInputConnections type. +const connectionBucketSchema = z.array(connectionEntrySchema).nullable().optional(); + +const workflowConnectionsStructureSchema = z.record( + z.string(), + z.record(z.string(), z.array(connectionBucketSchema)), +); + +const workflowStructureSchema = z.object({ + nodes: z.array(workflowNodeStructureSchema), + connections: workflowConnectionsStructureSchema, +}); + +type WorkflowStructureData = z.infer; + +type WorkflowStructureGraphIssueCode = + | 'duplicate_node_name' + | 'unknown_connection_source' + | 'unknown_connection_target'; + +type WorkflowStructureGraphIssue = { + code: WorkflowStructureGraphIssueCode; + path: Array; + message: string; +}; + +export type WorkflowStructureIssue = z.ZodIssue | WorkflowStructureGraphIssue; + +type WorkflowStructureValidationSuccess = { + success: true; + data: WorkflowStructureData; +}; + +type WorkflowStructureValidationFailure = { + success: false; + issues: WorkflowStructureIssue[]; +}; + +export type WorkflowStructureValidationResult = + | WorkflowStructureValidationSuccess + | WorkflowStructureValidationFailure; + +export const formatWorkflowStructureIssuePath = (path: Array): string => { + if (path.length === 0) return 'workflow'; + + return path.reduce((acc, segment) => { + if (typeof segment === 'number') return `${acc}[${segment}]`; + return acc ? `${acc}.${segment}` : segment; + }, ''); +}; + +const formatIssuesDescription = (issues: WorkflowStructureIssue[]): string => + issues + .map(({ path, message }) => `${formatWorkflowStructureIssuePath(path)}: ${message}`) + .join('\n'); + +export class WorkflowStructureValidationError extends Error { + override name = 'WorkflowStructureValidationError'; + + readonly description: string; + + constructor(readonly issues: WorkflowStructureIssue[]) { + super('Invalid workflow structure'); + this.description = formatIssuesDescription(issues); + } +} + +export function safeParseWorkflowStructure(input: unknown): WorkflowStructureValidationResult { + const parsed = workflowStructureSchema.safeParse(input); + + if (!parsed.success) { + return { + success: false, + issues: parsed.error.issues, + }; + } + + const { nodes, connections } = parsed.data; + const issues: WorkflowStructureIssue[] = []; + const nodeNames = new Set(); + + for (const [index, node] of nodes.entries()) { + if (nodeNames.has(node.name)) { + issues.push({ + path: ['nodes', index, 'name'], + message: `Duplicate node name "${node.name}"`, + code: 'duplicate_node_name', + }); + continue; + } + + nodeNames.add(node.name); + } + + for (const sourceNodeName of Object.keys(connections)) { + if (!nodeNames.has(sourceNodeName)) { + issues.push({ + path: ['connections', sourceNodeName], + message: `Connection source "${sourceNodeName}" does not reference an existing node`, + code: 'unknown_connection_source', + }); + } + + const connectionTypes = connections[sourceNodeName]; + for (const connectionType of Object.keys(connectionTypes)) { + const buckets = connectionTypes[connectionType]; + + for (const [sourceIndex, bucket] of buckets.entries()) { + for (const [targetIndex, connection] of (bucket ?? []).entries()) { + if (!nodeNames.has(connection.node)) { + issues.push({ + path: [ + 'connections', + sourceNodeName, + connectionType, + sourceIndex, + targetIndex, + 'node', + ], + message: `Connection target "${connection.node}" does not reference an existing node`, + code: 'unknown_connection_target', + }); + } + } + } + } + } + + if (issues.length > 0) { + return { success: false, issues }; + } + + return { + success: true, + data: parsed.data, + }; +} + +export function parseWorkflowStructure(input: unknown): WorkflowStructureData { + const result = safeParseWorkflowStructure(input); + + if (!result.success) { + throw new WorkflowStructureValidationError(result.issues); + } + + return result.data; +} diff --git a/packages/workflow/test/workflow-structure-validation.test.ts b/packages/workflow/test/workflow-structure-validation.test.ts new file mode 100644 index 00000000000..2ac0cd4c8c7 --- /dev/null +++ b/packages/workflow/test/workflow-structure-validation.test.ts @@ -0,0 +1,268 @@ +import { + safeParseWorkflowStructure, + parseWorkflowStructure, + WorkflowStructureValidationError, +} from '../src/workflow-structure-validation'; + +describe('workflow-structure-validation', () => { + const validWorkflow = { + nodes: [ + { + id: 'node-1', + name: 'Start', + type: 'n8n-nodes-base.manualTrigger', + position: [0, 0] as [number, number], + parameters: {}, + }, + { + id: 'node-2', + name: 'Set', + type: 'n8n-nodes-base.set', + position: [200, 0] as [number, number], + parameters: {}, + }, + ], + connections: { + Start: { + main: [[{ node: 'Set', type: 'main', index: 0 }]], + }, + }, + }; + + test('accepts a structurally valid workflow', () => { + expect(safeParseWorkflowStructure(validWorkflow)).toEqual({ + success: true, + data: validWorkflow, + }); + }); + + test('accepts a valid workflow with empty connections', () => { + const result = safeParseWorkflowStructure({ + nodes: [validWorkflow.nodes[0]], + connections: {}, + }); + + expect(result.success).toBe(true); + }); + + test('accepts a valid workflow with empty nodes array', () => { + const result = safeParseWorkflowStructure({ + nodes: [], + connections: {}, + }); + + expect(result.success).toBe(true); + }); + + test('accepts null connection buckets (unused output slots)', () => { + const result = safeParseWorkflowStructure({ + ...validWorkflow, + connections: { + Start: { + main: [null, [{ node: 'Set', type: 'main', index: 0 }]], + }, + }, + }); + + expect(result.success).toBe(true); + }); + + test('rejects nodes missing a required field', () => { + const result = safeParseWorkflowStructure({ + ...validWorkflow, + nodes: [{ ...validWorkflow.nodes[0], type: undefined }], + }); + + expect(result.success).toBe(false); + if (result.success) return; + + expect(result.issues).toContainEqual( + expect.objectContaining({ + code: 'invalid_type', + path: ['nodes', 0, 'type'], + }), + ); + }); + + test('rejects empty string node name', () => { + const result = safeParseWorkflowStructure({ + ...validWorkflow, + nodes: [{ ...validWorkflow.nodes[0], name: '' }], + }); + + expect(result.success).toBe(false); + if (result.success) return; + + expect(result.issues).toContainEqual( + expect.objectContaining({ + code: 'too_small', + path: ['nodes', 0, 'name'], + }), + ); + }); + + test('rejects positions with fewer than two coordinates', () => { + const result = safeParseWorkflowStructure({ + ...validWorkflow, + nodes: [{ ...validWorkflow.nodes[0], position: [0] }], + }); + + expect(result.success).toBe(false); + if (result.success) return; + + expect(result.issues).toContainEqual( + expect.objectContaining({ + code: 'too_small', + path: ['nodes', 0, 'position'], + }), + ); + }); + + test('rejects positions with more than two coordinates', () => { + const result = safeParseWorkflowStructure({ + ...validWorkflow, + nodes: [{ ...validWorkflow.nodes[0], position: [0, 0, 50] }], + }); + + expect(result.success).toBe(false); + if (result.success) return; + + expect(result.issues).toContainEqual( + expect.objectContaining({ + code: 'too_big', + path: ['nodes', 0, 'position'], + }), + ); + }); + + test('rejects connection with negative index', () => { + const result = safeParseWorkflowStructure({ + ...validWorkflow, + connections: { + Start: { + main: [[{ node: 'Set', type: 'main', index: -1 }]], + }, + }, + }); + + expect(result.success).toBe(false); + if (result.success) return; + + expect(result.issues).toContainEqual( + expect.objectContaining({ + code: 'too_small', + path: ['connections', 'Start', 'main', 0, 0, 'index'], + }), + ); + }); + + test('rejects duplicate node names', () => { + const result = safeParseWorkflowStructure({ + ...validWorkflow, + nodes: [ + validWorkflow.nodes[0], + { + ...validWorkflow.nodes[1], + name: 'Start', + }, + ], + }); + + expect(result.success).toBe(false); + if (result.success) return; + + expect(result.issues).toContainEqual( + expect.objectContaining({ + code: 'duplicate_node_name', + path: ['nodes', 1, 'name'], + }), + ); + }); + + test('rejects unknown connection sources', () => { + const result = safeParseWorkflowStructure({ + ...validWorkflow, + connections: { + Missing: { + main: [[{ node: 'Set', type: 'main', index: 0 }]], + }, + }, + }); + + expect(result.success).toBe(false); + if (result.success) return; + + expect(result.issues).toContainEqual( + expect.objectContaining({ + code: 'unknown_connection_source', + path: ['connections', 'Missing'], + }), + ); + }); + + test('rejects unknown connection targets', () => { + const result = safeParseWorkflowStructure({ + ...validWorkflow, + connections: { + Start: { + main: [[{ node: 'Missing', type: 'main', index: 0 }]], + }, + }, + }); + + expect(result.success).toBe(false); + if (result.success) return; + + expect(result.issues).toContainEqual( + expect.objectContaining({ + code: 'unknown_connection_target', + path: ['connections', 'Start', 'main', 0, 0, 'node'], + }), + ); + }); + + test('rejects empty nodes with non-empty connections', () => { + const result = safeParseWorkflowStructure({ + nodes: [], + connections: { + Start: { + main: [[{ node: 'Set', type: 'main', index: 0 }]], + }, + }, + }); + + expect(result.success).toBe(false); + if (result.success) return; + + expect(result.issues).toContainEqual( + expect.objectContaining({ + code: 'unknown_connection_source', + }), + ); + }); + + test('throws a typed error for invalid workflows', () => { + expect(() => + parseWorkflowStructure({ + nodes: [{ name: 'Start', position: [0, 0], parameters: {} }], + connections: {}, + }), + ).toThrow(WorkflowStructureValidationError); + }); + + test('error description formats issue paths', () => { + let thrown: WorkflowStructureValidationError | undefined; + + try { + parseWorkflowStructure({ + nodes: [{ name: 'Start', position: [0, 0], parameters: {} }], + connections: {}, + }); + } catch (error) { + thrown = error as WorkflowStructureValidationError; + } + + expect(thrown).toBeInstanceOf(WorkflowStructureValidationError); + expect(thrown?.description).toContain('nodes[0].type'); + }); +});