From 1ca95cda4a6dd04007e51ddd50be24686e38d777 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jean-Philippe=20Qu=C3=A9m=C3=A9ner?=
 <JohnnyQQQQ@users.noreply.github.com>
Date: Fri, 7 Nov 2025 15:19:55 +0100
Subject: [PATCH] fix(folders): prevent circular dependencies (#113595)

---
 pkg/registry/apis/folders/validate.go      |  8 +++
 pkg/registry/apis/folders/validate_test.go | 65 ++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/pkg/registry/apis/folders/validate.go b/pkg/registry/apis/folders/validate.go
index db8c3b7e0e8..4f8ccd2250d 100644
--- a/pkg/registry/apis/folders/validate.go
+++ b/pkg/registry/apis/folders/validate.go
@@ -120,6 +120,14 @@ func validateOnUpdate(ctx context.Context,
 		return err
 	}
 
+	// Check that the folder being moved is not an ancestor of the target parent.
+	// This prevents circular references (e.g., moving A under B when B is already under A).
+	for _, ancestor := range info.Items {
+		if ancestor.Name == obj.Name {
+			return fmt.Errorf("cannot move folder under its own descendant, this would create a circular reference")
+		}
+	}
+
 	// if by moving a folder we exceed the max depth, return an error
 	if len(info.Items) > maxDepth+1 {
 		return folder.ErrMaximumDepthReached.Errorf("maximum folder depth reached")
diff --git a/pkg/registry/apis/folders/validate_test.go b/pkg/registry/apis/folders/validate_test.go
index 33fda7ea30f..1f67c0da9d8 100644
--- a/pkg/registry/apis/folders/validate_test.go
+++ b/pkg/registry/apis/folders/validate_test.go
@@ -264,6 +264,71 @@ func TestValidateUpdate(t *testing.T) {
 			maxDepth:    folder.MaxNestedFolderDepth,
 			expectedErr: "[folder.maximum-depth-reached]",
 		},
+		{
+			name: "error when moving folder under its own descendant (direct child)",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "parent",
+					Annotations: map[string]string{
+						utils.AnnoKeyFolder: "child",
+					},
+				},
+				Spec: folders.FolderSpec{
+					Title: "parent folder",
+				},
+			},
+			old: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "parent",
+				},
+				Spec: folders.FolderSpec{
+					Title: "parent folder",
+				},
+			},
+			// When querying parents of "child", we get the chain: child -> parent -> root
+			// This means "parent" is an ancestor of "child", so we can't move "parent" under "child"
+			parents: &folders.FolderInfoList{
+				Items: []folders.FolderInfo{
+					{Name: "child", Parent: "parent"},
+					{Name: "parent", Parent: folder.GeneralFolderUID},
+					{Name: folder.GeneralFolderUID},
+				},
+			},
+			expectedErr: "cannot move folder under its own descendant",
+		},
+		{
+			name: "error when moving folder under its grandchild",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "grandparent",
+					Annotations: map[string]string{
+						utils.AnnoKeyFolder: "grandchild",
+					},
+				},
+				Spec: folders.FolderSpec{
+					Title: "grandparent folder",
+				},
+			},
+			old: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "grandparent",
+				},
+				Spec: folders.FolderSpec{
+					Title: "grandparent folder",
+				},
+			},
+			// When querying parents of "grandchild", we get: grandchild -> child -> grandparent -> root
+			// This means "grandparent" is in the ancestry, so we can't move it under "grandchild"
+			parents: &folders.FolderInfoList{
+				Items: []folders.FolderInfo{
+					{Name: "grandchild", Parent: "child"},
+					{Name: "child", Parent: "grandparent"},
+					{Name: "grandparent", Parent: folder.GeneralFolderUID},
+					{Name: folder.GeneralFolderUID},
+				},
+			},
+			expectedErr: "cannot move folder under its own descendant",
+		},
 	}
 
 	for _, tt := range tests {