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
This commit is contained in:
Iván Ovejero 2023-12-14 18:13:12 +01:00 committed by GitHub
parent 53c0b49d15
commit c5e6ba8cdd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 27 additions and 4 deletions

View file

@ -13,7 +13,13 @@ import type {
INodeTypes, INodeTypes,
IRun, IRun,
} from 'n8n-workflow'; } 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 Db from '@/Db';
import * as ResponseHelper from '@/ResponseHelper'; import * as ResponseHelper from '@/ResponseHelper';
@ -130,7 +136,15 @@ export class Worker extends BaseCommand {
{ extra: { executionId } }, { 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( this.logger.info(
`Start job: ${job.id} (Workflow ID: ${workflowId} | Execution: ${executionId})`, `Start job: ${job.id} (Workflow ID: ${workflowId} | Execution: ${executionId})`,
); );

View file

@ -220,7 +220,7 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
const { connections, nodes, name } = workflowData ?? {}; const { connections, nodes, name } = workflowData ?? {};
await this.executionDataRepository.insert({ await this.executionDataRepository.insert({
executionId, executionId,
workflowData: { connections, nodes, name }, workflowData: { connections, nodes, name, id: workflowData?.id },
data: stringify(data), data: stringify(data),
}); });
return String(executionId); return String(executionId);

View file

@ -43,6 +43,7 @@ describe('ExecutionRepository', () => {
const executionDataRepo = Container.get(ExecutionDataRepository); const executionDataRepo = Container.get(ExecutionDataRepository);
const executionData = await executionDataRepo.findOneBy({ executionId }); const executionData = await executionDataRepo.findOneBy({ executionId });
expect(executionData?.workflowData).toEqual({ expect(executionData?.workflowData).toEqual({
id: workflow.id,
connections: workflow.connections, connections: workflow.connections,
nodes: workflow.nodes, nodes: workflow.nodes,
name: workflow.name, name: workflow.name,

View file

@ -52,6 +52,7 @@ import { RoutingNode } from './RoutingNode';
import { Expression } from './Expression'; import { Expression } from './Expression';
import { NODES_WITH_RENAMABLE_CONTENT } from './Constants'; import { NODES_WITH_RENAMABLE_CONTENT } from './Constants';
import { ApplicationError } from './errors/application.error'; import { ApplicationError } from './errors/application.error';
import * as EventReporter from './ErrorReporterProxy';
function dedupe<T>(arr: T[]): T[] { function dedupe<T>(arr: T[]): T[] {
return [...new Set(arr)]; return [...new Set(arr)];
@ -94,7 +95,14 @@ export class Workflow {
settings?: IWorkflowSettings; settings?: IWorkflowSettings;
pinData?: IPinData; 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.name = parameters.name;
this.nodeTypes = parameters.nodeTypes; this.nodeTypes = parameters.nodeTypes;
this.pinData = parameters.pinData; this.pinData = parameters.pinData;