mirror of
https://github.com/n8n-io/n8n.git
synced 2024-11-13 16:14:07 -08:00
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:
parent
53c0b49d15
commit
c5e6ba8cdd
|
@ -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})`,
|
||||||
);
|
);
|
||||||
|
|
|
@ -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);
|
||||||
|
|
|
@ -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,
|
||||||
|
|
|
@ -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;
|
||||||
|
|
Loading…
Reference in a new issue