From c5e6ba8cdd4a8f117ccc2e89e55497117156d8af Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= <ivov.src@gmail.com>
Date: Thu, 14 Dec 2023 18:13:12 +0100
Subject: [PATCH] fix(core): Restore workflow ID during execution creation
 (#8031)

## Summary
Restore workflow ID during execution creation removed by [this
PR](https://github.com/n8n-io/n8n/pull/8002/files#diff-c8cbb62ca9ab2ae45e5f565cd8c63fff6475809a6241ea0b90acc575615224af).
The missing workflow ID, and more generally the fact that `workflow.id`
is optional when it should not be, causes `PermissionChecker.check` to
misreport a credential as inaccessible when it should be accessible.

More generally, start reporting ID-less workflows so we can root them
out and prevent this at type level.

## Related tickets and issues

https://n8nio.slack.com/archives/C035KBDA917/p1702539465555529
---
 packages/cli/src/commands/worker.ts            | 18 ++++++++++++++++--
 .../repositories/execution.repository.ts       |  2 +-
 .../repositories/execution.repository.test.ts  |  1 +
 packages/workflow/src/Workflow.ts              | 10 +++++++++-
 4 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/packages/cli/src/commands/worker.ts b/packages/cli/src/commands/worker.ts
index 97a41988ec..07b4ce98fe 100644
--- a/packages/cli/src/commands/worker.ts
+++ b/packages/cli/src/commands/worker.ts
@@ -13,7 +13,13 @@ import type {
 	INodeTypes,
 	IRun,
 } from 'n8n-workflow';
-import { Workflow, NodeOperationError, sleep, ApplicationError } from 'n8n-workflow';
+import {
+	Workflow,
+	NodeOperationError,
+	sleep,
+	ApplicationError,
+	ErrorReporterProxy as EventReporter,
+} from 'n8n-workflow';
 
 import * as Db from '@/Db';
 import * as ResponseHelper from '@/ResponseHelper';
@@ -130,7 +136,15 @@ export class Worker extends BaseCommand {
 				{ extra: { executionId } },
 			);
 		}
-		const workflowId = fullExecutionData.workflowData.id!;
+		const workflowId = fullExecutionData.workflowData.id!; // @tech_debt Ensure this is not optional
+
+		if (!workflowId) {
+			EventReporter.report('Detected ID-less workflow', {
+				level: 'info',
+				extra: { execution: fullExecutionData },
+			});
+		}
+
 		this.logger.info(
 			`Start job: ${job.id} (Workflow ID: ${workflowId} | Execution: ${executionId})`,
 		);
diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts
index 7598ff788f..0934303267 100644
--- a/packages/cli/src/databases/repositories/execution.repository.ts
+++ b/packages/cli/src/databases/repositories/execution.repository.ts
@@ -220,7 +220,7 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
 		const { connections, nodes, name } = workflowData ?? {};
 		await this.executionDataRepository.insert({
 			executionId,
-			workflowData: { connections, nodes, name },
+			workflowData: { connections, nodes, name, id: workflowData?.id },
 			data: stringify(data),
 		});
 		return String(executionId);
diff --git a/packages/cli/test/integration/database/repositories/execution.repository.test.ts b/packages/cli/test/integration/database/repositories/execution.repository.test.ts
index d16645367f..62cb57d2b8 100644
--- a/packages/cli/test/integration/database/repositories/execution.repository.test.ts
+++ b/packages/cli/test/integration/database/repositories/execution.repository.test.ts
@@ -43,6 +43,7 @@ describe('ExecutionRepository', () => {
 			const executionDataRepo = Container.get(ExecutionDataRepository);
 			const executionData = await executionDataRepo.findOneBy({ executionId });
 			expect(executionData?.workflowData).toEqual({
+				id: workflow.id,
 				connections: workflow.connections,
 				nodes: workflow.nodes,
 				name: workflow.name,
diff --git a/packages/workflow/src/Workflow.ts b/packages/workflow/src/Workflow.ts
index 6ec06db20c..f09256d8ca 100644
--- a/packages/workflow/src/Workflow.ts
+++ b/packages/workflow/src/Workflow.ts
@@ -52,6 +52,7 @@ import { RoutingNode } from './RoutingNode';
 import { Expression } from './Expression';
 import { NODES_WITH_RENAMABLE_CONTENT } from './Constants';
 import { ApplicationError } from './errors/application.error';
+import * as EventReporter from './ErrorReporterProxy';
 
 function dedupe<T>(arr: T[]): T[] {
 	return [...new Set(arr)];
@@ -94,7 +95,14 @@ export class Workflow {
 		settings?: IWorkflowSettings;
 		pinData?: IPinData;
 	}) {
-		this.id = parameters.id as string;
+		if (!parameters.id) {
+			EventReporter.report('Detected ID-less workflow', {
+				level: 'info',
+				extra: { parameters },
+			});
+		}
+
+		this.id = parameters.id as string; // @tech_debt Ensure this is not optional
 		this.name = parameters.name;
 		this.nodeTypes = parameters.nodeTypes;
 		this.pinData = parameters.pinData;