From 22a240e74bc7fcfe99da94deb0f5613b652f11f5 Mon Sep 17 00:00:00 2001 From: Jaakko Husso Date: Mon, 30 Jun 2025 13:44:48 +0300 Subject: [PATCH] fix(core): Prevent accidentally moving folders to their sub folders (#16808) --- packages/cli/src/services/folder.service.ts | 18 ++++ .../folder/folder.controller.test.ts | 87 ++++++++++++++++++- 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/services/folder.service.ts b/packages/cli/src/services/folder.service.ts index 4f2ece0d82..a04370bd1e 100644 --- a/packages/cli/src/services/folder.service.ts +++ b/packages/cli/src/services/folder.service.ts @@ -73,7 +73,16 @@ export class FolderService { if (parentFolderId !== PROJECT_ROOT) { await this.findFolderInProjectOrFail(parentFolderId, projectId); + + // Ensure that the target parentFolder isn't a descendant of the current folder. + const parentFolderPath = await this.getFolderTree(parentFolderId, projectId); + if (this.isDescendant(folderId, parentFolderPath)) { + throw new UserError( + "Cannot set a folder's parent to a folder that is a descendant of the current folder", + ); + } } + await this.folderRepository.update( { id: folderId }, { parentFolder: parentFolderId !== PROJECT_ROOT ? { id: parentFolderId } : null }, @@ -221,6 +230,15 @@ export class FolderService { return rootNode ? [rootNode] : []; } + private isDescendant(folderId: string, tree: SimpleFolderNode[]): boolean { + return tree.some((node) => { + if (node.id === folderId) { + return true; + } + return this.isDescendant(folderId, node.children); + }); + } + async getFolderAndWorkflowCount( folderId: string, projectId: string, diff --git a/packages/cli/test/integration/folder/folder.controller.test.ts b/packages/cli/test/integration/folder/folder.controller.test.ts index b97fb8c99f..c26ed67919 100644 --- a/packages/cli/test/integration/folder/folder.controller.test.ts +++ b/packages/cli/test/integration/folder/folder.controller.test.ts @@ -765,11 +765,13 @@ describe('PATCH /projects/:projectId/folders/:folderId', () => { parentFolderId: folder.id, }; - await authOwnerAgent + const response = await authOwnerAgent .patch(`/projects/${project.id}/folders/${folder.id}`) .send(payload) .expect(400); + expect(response.body.message).toBe('Cannot set a folder as its own parent'); + const folderInDb = await folderRepository.findOne({ where: { id: folder.id }, relations: ['parentFolder'], @@ -778,6 +780,89 @@ describe('PATCH /projects/:projectId/folders/:folderId', () => { expect(folderInDb).toBeDefined(); expect(folderInDb?.parentFolder).toBeNull(); }); + + test("should not allow setting folder's parent to a folder that is a direct child", async () => { + const project = await createTeamProject(undefined, owner); + + // A + // └── B + // └── C + const folderA = await createFolder(project, { name: 'A' }); + const folderB = await createFolder(project, { + name: 'B', + parentFolder: folderA, + }); + const folderC = await createFolder(project, { + name: 'C', + parentFolder: folderB, + }); + + // Attempt to make the parent of B its child C + const payload = { + parentFolderId: folderC.id, + }; + + const response = await authOwnerAgent + .patch(`/projects/${project.id}/folders/${folderB.id}`) + .send(payload) + .expect(400); + + expect(response.body.message).toBe( + "Cannot set a folder's parent to a folder that is a descendant of the current folder", + ); + + const folderBInDb = await folderRepository.findOne({ + where: { id: folderB.id }, + relations: ['parentFolder'], + }); + + expect(folderBInDb).toBeDefined(); + expect(folderBInDb?.parentFolder?.id).toBe(folderA.id); + }); + + test("should not allow setting folder's parent to a folder that is a descendant", async () => { + const project = await createTeamProject(undefined, owner); + + // A + // └── B + // └── C + // └── D + const folderA = await createFolder(project, { name: 'A' }); + const folderB = await createFolder(project, { + name: 'B', + parentFolder: folderA, + }); + const folderC = await createFolder(project, { + name: 'C', + parentFolder: folderB, + }); + const folderD = await createFolder(project, { + name: 'D', + parentFolder: folderC, + }); + + // Attempt to make the parent of A the descendant D + const payload = { + parentFolderId: folderD.id, + }; + + const response = await authOwnerAgent + .patch(`/projects/${project.id}/folders/${folderA.id}`) + .send(payload) + .expect(400); + + expect(response.body.message).toBe( + "Cannot set a folder's parent to a folder that is a descendant of the current folder", + ); + + const folderAInDb = await folderRepository.findOne({ + where: { id: folderA.id }, + relations: ['parentFolder'], + }); + + expect(folderAInDb).toBeDefined(); + expect(folderAInDb?.parentFolder?.id).not.toBeDefined(); + }); }); describe('DELETE /projects/:projectId/folders/:folderId', () => {