fix(core): Prevent accidentally moving folders to their sub folders (#16808)

This commit is contained in:
Jaakko Husso
2025-06-30 13:44:48 +03:00
committed by GitHub
parent c8a7156254
commit 22a240e74b
2 changed files with 104 additions and 1 deletions

View File

@@ -73,7 +73,16 @@ export class FolderService {
if (parentFolderId !== PROJECT_ROOT) { if (parentFolderId !== PROJECT_ROOT) {
await this.findFolderInProjectOrFail(parentFolderId, projectId); 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( await this.folderRepository.update(
{ id: folderId }, { id: folderId },
{ parentFolder: parentFolderId !== PROJECT_ROOT ? { id: parentFolderId } : null }, { parentFolder: parentFolderId !== PROJECT_ROOT ? { id: parentFolderId } : null },
@@ -221,6 +230,15 @@ export class FolderService {
return rootNode ? [rootNode] : []; 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( async getFolderAndWorkflowCount(
folderId: string, folderId: string,
projectId: string, projectId: string,

View File

@@ -765,11 +765,13 @@ describe('PATCH /projects/:projectId/folders/:folderId', () => {
parentFolderId: folder.id, parentFolderId: folder.id,
}; };
await authOwnerAgent const response = await authOwnerAgent
.patch(`/projects/${project.id}/folders/${folder.id}`) .patch(`/projects/${project.id}/folders/${folder.id}`)
.send(payload) .send(payload)
.expect(400); .expect(400);
expect(response.body.message).toBe('Cannot set a folder as its own parent');
const folderInDb = await folderRepository.findOne({ const folderInDb = await folderRepository.findOne({
where: { id: folder.id }, where: { id: folder.id },
relations: ['parentFolder'], relations: ['parentFolder'],
@@ -778,6 +780,89 @@ describe('PATCH /projects/:projectId/folders/:folderId', () => {
expect(folderInDb).toBeDefined(); expect(folderInDb).toBeDefined();
expect(folderInDb?.parentFolder).toBeNull(); 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', () => { describe('DELETE /projects/:projectId/folders/:folderId', () => {