Skip to content

Commit 8cdd9bd

Browse files
scdekovscdekov
authored
fix(editor): Add checksum validation when archive/unpublish workflow from canvas (#25302)
1 parent 26805b6 commit 8cdd9bd

16 files changed

Lines changed: 241 additions & 28 deletions

File tree

‎packages/@n8n/api-types/src/dto/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ export { UpdateWorkflowDto } from './workflows/update-workflow.dto';
7171
export { ImportWorkflowFromUrlDto } from './workflows/import-workflow-from-url.dto';
7272
export { TransferWorkflowBodyDto } from './workflows/transfer.dto';
7373
export { ActivateWorkflowDto } from './workflows/activate-workflow.dto';
74+
export { DeactivateWorkflowDto } from './workflows/deactivate-workflow.dto';
75+
export { ArchiveWorkflowDto } from './workflows/archive-workflow.dto';
7476

7577
export { CreateOrUpdateTagRequestDto } from './tag/create-or-update-tag-request.dto';
7678
export { RetrieveTagQueryDto } from './tag/retrieve-tag-query.dto';

‎packages/@n8n/api-types/src/dto/workflows/archive-workflow.dto.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { z } from 'zod';
2+
import { Z } from 'zod-class';
3+
4+
export class ArchiveWorkflowDto extends Z.class({
5+
expectedChecksum: z.string().optional(),
6+
}) {}

‎packages/@n8n/api-types/src/dto/workflows/deactivate-workflow.dto.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { z } from 'zod';
2+
import { Z } from 'zod-class';
3+
4+
export class DeactivateWorkflowDto extends Z.class({
5+
expectedChecksum: z.string().optional(),
6+
}) {}

‎packages/cli/src/public-api/v1/handlers/workflows/workflows.handler.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,11 +361,9 @@ export = {
361361
const { id } = req.params;
362362

363363
try {
364-
const workflow = await Container.get(WorkflowService).deactivateWorkflow(
365-
req.user,
366-
id,
367-
true,
368-
);
364+
const workflow = await Container.get(WorkflowService).deactivateWorkflow(req.user, id, {
365+
publicApi: true,
366+
});
369367

370368
return res.json(workflow);
371369
} catch (error) {

‎packages/cli/src/services/folder.service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ export class FolderService {
153153
);
154154

155155
for (const workflowId of workflowIds) {
156-
await this.workflowService.archive(user, workflowId, true);
156+
await this.workflowService.archive(user, workflowId, { skipArchived: true });
157157
}
158158

159159
await this.workflowRepository.moveToFolder(workflowIds, PROJECT_ROOT);

‎packages/cli/src/workflows/workflow.service.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -708,13 +708,13 @@ export class WorkflowService {
708708
* Deactivates a workflow by removing it from the active workflow manager and setting activeVersionId to null.
709709
* @param user - The user deactivating the workflow
710710
* @param workflowId - The ID of the workflow to deactivate
711-
* @param publicApi - Whether this is called from the public API (affects event emission)
711+
* @param options - Optional settings including expectedChecksum for conflict detection and publicApi flag
712712
* @returns The deactivated workflow
713713
*/
714714
async deactivateWorkflow(
715715
user: User,
716716
workflowId: string,
717-
publicApi: boolean = false,
717+
options?: { expectedChecksum?: string; publicApi?: boolean },
718718
): Promise<WorkflowEntity> {
719719
const workflow = await this.workflowFinderService.findWorkflowForUser(workflowId, user, [
720720
'workflow:publish',
@@ -734,6 +734,10 @@ export class WorkflowService {
734734
return workflow;
735735
}
736736

737+
if (options?.expectedChecksum) {
738+
await this._detectConflicts(workflow, options.expectedChecksum);
739+
}
740+
737741
// Remove from active workflow manager
738742
await this.activeWorkflowManager.remove(workflowId);
739743

@@ -760,7 +764,7 @@ export class WorkflowService {
760764
user,
761765
workflowId,
762766
workflow,
763-
publicApi,
767+
publicApi: options?.publicApi ?? false,
764768
});
765769

766770
return workflow;
@@ -814,7 +818,7 @@ export class WorkflowService {
814818
async archive(
815819
user: User,
816820
workflowId: string,
817-
skipArchived: boolean = false,
821+
options?: { skipArchived?: boolean; expectedChecksum?: string },
818822
): Promise<WorkflowEntity | undefined> {
819823
const workflow = await this.workflowFinderService.findWorkflowForUser(workflowId, user, [
820824
'workflow:delete',
@@ -825,13 +829,17 @@ export class WorkflowService {
825829
}
826830

827831
if (workflow.isArchived) {
828-
if (skipArchived) {
832+
if (options?.skipArchived) {
829833
return workflow;
830834
}
831835

832836
throw new BadRequestError('Workflow is already archived.');
833837
}
834838

839+
if (options?.expectedChecksum) {
840+
await this._detectConflicts(workflow, options.expectedChecksum);
841+
}
842+
835843
if (workflow.activeVersionId !== null) {
836844
await this.activeWorkflowManager.remove(workflowId);
837845
await this.workflowPublishHistoryRepository.addRecord({

‎packages/cli/src/workflows/workflows.controller.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {
22
ActivateWorkflowDto,
3+
ArchiveWorkflowDto,
34
CreateWorkflowDto,
5+
DeactivateWorkflowDto,
46
ImportWorkflowFromUrlDto,
57
ROLE,
68
TransferWorkflowBodyDto,
@@ -520,10 +522,15 @@ export class WorkflowsController {
520522
req: AuthenticatedRequest,
521523
_res: Response,
522524
@Param('workflowId') workflowId: string,
525+
@Body body: ArchiveWorkflowDto,
523526
) {
524527
await this.collaborationService.validateWriteLock(req.user.id, workflowId, 'archive');
525528

526-
const workflow = await this.workflowService.archive(req.user, workflowId);
529+
const { expectedChecksum } = body;
530+
531+
const workflow = await this.workflowService.archive(req.user, workflowId, {
532+
expectedChecksum,
533+
});
527534
if (!workflow) {
528535
this.logger.warn('User attempted to archive a workflow without permissions', {
529536
workflowId,
@@ -597,12 +604,19 @@ export class WorkflowsController {
597604

598605
@Post('/:workflowId/deactivate')
599606
@ProjectScope('workflow:publish')
600-
async deactivate(req: WorkflowRequest.Deactivate) {
601-
const { workflowId } = req.params;
602-
607+
async deactivate(
608+
req: WorkflowRequest.Deactivate,
609+
_res: unknown,
610+
@Param('workflowId') workflowId: string,
611+
@Body body: DeactivateWorkflowDto,
612+
) {
603613
await this.collaborationService.validateWriteLock(req.user.id, workflowId, 'deactivate');
604614

605-
const workflow = await this.workflowService.deactivateWorkflow(req.user, workflowId);
615+
const { expectedChecksum } = body;
616+
617+
const workflow = await this.workflowService.deactivateWorkflow(req.user, workflowId, {
618+
expectedChecksum,
619+
});
606620

607621
const scopes = await this.workflowService.getWorkflowScopes(req.user, workflowId);
608622
const checksum = await calculateWorkflowChecksum(workflow);

‎packages/cli/test/integration/workflows/workflows.controller.test.ts‎

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,13 @@ import type { Scope } from '@n8n/permissions';
3232
import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
3333
import { createFolder } from '@test-integration/db/folders';
3434
import { DateTime } from 'luxon';
35-
import { PROJECT_ROOT, type INode, type IPinData, type IWorkflowBase } from 'n8n-workflow';
35+
import {
36+
PROJECT_ROOT,
37+
calculateWorkflowChecksum,
38+
type INode,
39+
type IPinData,
40+
type IWorkflowBase,
41+
} from 'n8n-workflow';
3642
import { v4 as uuid } from 'uuid';
3743

3844
import { saveCredential } from '../shared/db/credentials';
@@ -3953,6 +3959,53 @@ describe('POST /workflows/:workflowId/deactivate', () => {
39533959
expect(updatedWorkflow?.activeVersionId).toBeNull();
39543960
expect(updatedWorkflow?.activeVersion).toBeNull();
39553961
});
3962+
3963+
test('should block deactivation when expectedChecksum does not match', async () => {
3964+
const workflow = await createActiveWorkflow({}, owner);
3965+
await setActiveVersion(workflow.id, workflow.versionId);
3966+
3967+
// Simulate another user updating the workflow
3968+
await authOwnerAgent.patch(`/workflows/${workflow.id}`).send({
3969+
name: 'Updated by another user',
3970+
versionId: workflow.versionId,
3971+
});
3972+
3973+
// Try to deactivate with outdated checksum
3974+
const outdatedChecksum = await calculateWorkflowChecksum(workflow);
3975+
const response = await authOwnerAgent
3976+
.post(`/workflows/${workflow.id}/deactivate`)
3977+
.send({ expectedChecksum: outdatedChecksum });
3978+
3979+
expect(response.statusCode).toBe(400);
3980+
expect(response.body.code).toBe(100);
3981+
});
3982+
3983+
test('should allow deactivation when expectedChecksum matches', async () => {
3984+
const workflow = await createActiveWorkflow({}, owner);
3985+
await setActiveVersion(workflow.id, workflow.versionId);
3986+
3987+
// Get the current workflow to compute correct checksum
3988+
const getResponse = await authOwnerAgent.get(`/workflows/${workflow.id}`);
3989+
const currentChecksum = await calculateWorkflowChecksum(getResponse.body.data);
3990+
3991+
const response = await authOwnerAgent
3992+
.post(`/workflows/${workflow.id}/deactivate`)
3993+
.send({ expectedChecksum: currentChecksum });
3994+
3995+
expect(response.statusCode).toBe(200);
3996+
expect(response.body.data.active).toBe(false);
3997+
expect(response.body.data.activeVersionId).toBeNull();
3998+
});
3999+
4000+
test('should allow deactivation without expectedChecksum (backward compatible)', async () => {
4001+
const workflow = await createActiveWorkflow({}, owner);
4002+
await setActiveVersion(workflow.id, workflow.versionId);
4003+
4004+
const response = await authOwnerAgent.post(`/workflows/${workflow.id}/deactivate`).send({});
4005+
4006+
expect(response.statusCode).toBe(200);
4007+
expect(response.body.data.active).toBe(false);
4008+
});
39564009
});
39574010

39584011
describe('POST /workflows/:workflowId/run', () => {
@@ -4130,6 +4183,49 @@ describe('POST /workflows/:workflowId/archive', () => {
41304183
expect(historyRecord!.nodes).toEqual(workflow.nodes);
41314184
expect(historyRecord!.connections).toEqual(workflow.connections);
41324185
});
4186+
4187+
test('should block archive when expectedChecksum does not match', async () => {
4188+
const workflow = await createWorkflow({}, owner);
4189+
4190+
// Simulate another user updating the workflow
4191+
await authOwnerAgent.patch(`/workflows/${workflow.id}`).send({
4192+
name: 'Updated by another user',
4193+
versionId: workflow.versionId,
4194+
});
4195+
4196+
// Try to archive with outdated checksum
4197+
const outdatedChecksum = await calculateWorkflowChecksum(workflow);
4198+
const response = await authOwnerAgent
4199+
.post(`/workflows/${workflow.id}/archive`)
4200+
.send({ expectedChecksum: outdatedChecksum });
4201+
4202+
expect(response.statusCode).toBe(400);
4203+
expect(response.body.code).toBe(100);
4204+
});
4205+
4206+
test('should allow archive when expectedChecksum matches', async () => {
4207+
const workflow = await createWorkflow({}, owner);
4208+
4209+
// Get the current workflow to compute correct checksum
4210+
const getResponse = await authOwnerAgent.get(`/workflows/${workflow.id}`);
4211+
const currentChecksum = await calculateWorkflowChecksum(getResponse.body.data);
4212+
4213+
const response = await authOwnerAgent
4214+
.post(`/workflows/${workflow.id}/archive`)
4215+
.send({ expectedChecksum: currentChecksum });
4216+
4217+
expect(response.statusCode).toBe(200);
4218+
expect(response.body.data.isArchived).toBe(true);
4219+
});
4220+
4221+
test('should allow archive without expectedChecksum (backward compatible)', async () => {
4222+
const workflow = await createWorkflow({}, owner);
4223+
4224+
const response = await authOwnerAgent.post(`/workflows/${workflow.id}/archive`).send({});
4225+
4226+
expect(response.statusCode).toBe(200);
4227+
expect(response.body.data.isArchived).toBe(true);
4228+
});
41334229
});
41344230

41354231
describe('POST /workflows/:workflowId/unarchive', () => {

‎packages/frontend/editor-ui/src/app/api/workflows.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,12 @@ export async function activateWorkflow(
144144
export async function deactivateWorkflow(
145145
context: IRestApiContext,
146146
workflowId: string,
147+
expectedChecksum?: string,
147148
): Promise<IWorkflowDb> {
148149
return await makeRestApiRequest<IWorkflowDb>(
149150
context,
150151
'POST',
151152
`/workflows/${workflowId}/deactivate`,
153+
{ expectedChecksum },
152154
);
153155
}

‎packages/frontend/editor-ui/src/app/components/MainHeader/WorkflowDetails.test.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ describe('WorkflowDetails', () => {
167167
'123': workflow,
168168
};
169169
workflowsStore.isWorkflowSaved = { '1': true, '123': true };
170+
workflowsStore.workflowId = workflow.id;
171+
workflowsStore.workflowChecksum = 'test-checksum';
170172
projectsStore.currentProject = null;
171173
projectsStore.personalProject = { id: 'personal', name: 'Personal' } as Project;
172174
collaborationStore.shouldBeReadOnly = false;
@@ -474,7 +476,7 @@ describe('WorkflowDetails', () => {
474476
expect(toast.showError).toHaveBeenCalledTimes(0);
475477
expect(toast.showMessage).toHaveBeenCalledTimes(1);
476478
expect(workflowsStore.archiveWorkflow).toHaveBeenCalledTimes(1);
477-
expect(workflowsStore.archiveWorkflow).toHaveBeenCalledWith(workflow.id);
479+
expect(workflowsStore.archiveWorkflow).toHaveBeenCalledWith(workflow.id, 'test-checksum');
478480
expect(router.push).toHaveBeenCalledTimes(1);
479481
expect(router.push).toHaveBeenCalledWith({
480482
name: VIEWS.WORKFLOWS,
@@ -507,7 +509,7 @@ describe('WorkflowDetails', () => {
507509
await userEvent.click(getByTestId('workflow-menu'));
508510
await userEvent.click(getByTestId('workflow-menu-item-archive'));
509511

510-
expect(workflowsStore.archiveWorkflow).toHaveBeenCalledWith(teamWorkflow.id);
512+
expect(workflowsStore.archiveWorkflow).toHaveBeenCalledWith(teamWorkflow.id, 'test-checksum');
511513
expect(router.push).toHaveBeenCalledWith({
512514
name: VIEWS.PROJECTS_WORKFLOWS,
513515
params: { projectId: teamProjectId },
@@ -539,7 +541,10 @@ describe('WorkflowDetails', () => {
539541
await userEvent.click(getByTestId('workflow-menu'));
540542
await userEvent.click(getByTestId('workflow-menu-item-archive'));
541543

542-
expect(workflowsStore.archiveWorkflow).toHaveBeenCalledWith(personalWorkflow.id);
544+
expect(workflowsStore.archiveWorkflow).toHaveBeenCalledWith(
545+
personalWorkflow.id,
546+
'test-checksum',
547+
);
543548
expect(router.push).toHaveBeenCalledWith({
544549
name: VIEWS.WORKFLOWS,
545550
});
@@ -564,7 +569,7 @@ describe('WorkflowDetails', () => {
564569
expect(toast.showError).toHaveBeenCalledTimes(0);
565570
expect(toast.showMessage).toHaveBeenCalledTimes(1);
566571
expect(workflowsStore.archiveWorkflow).toHaveBeenCalledTimes(1);
567-
expect(workflowsStore.archiveWorkflow).toHaveBeenCalledWith(workflow.id);
572+
expect(workflowsStore.archiveWorkflow).toHaveBeenCalledWith(workflow.id, 'test-checksum');
568573
expect(router.push).toHaveBeenCalledTimes(1);
569574
expect(router.push).toHaveBeenCalledWith({
570575
name: VIEWS.WORKFLOWS,

0 commit comments

Comments
 (0)