From 7c35d741ba665235c15cda5b0de44a2525706ba1 Mon Sep 17 00:00:00 2001
From: Stephanie Hingtgen <stephanie.hingtgen@grafana.com>
Date: Tue, 4 Mar 2025 12:56:21 -0700
Subject: [PATCH] Folders: Add validation that folder is not a parent of itself
 (#101569)

---
 pkg/registry/apis/folders/register.go         |  4 ++
 pkg/registry/apis/folders/register_test.go    | 38 ++++++++++++++++---
 pkg/services/folder/folderimpl/sqlstore.go    |  4 ++
 .../folder/folderimpl/sqlstore_test.go        | 12 ++++++
 pkg/services/folder/model.go                  |  2 +
 5 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/pkg/registry/apis/folders/register.go b/pkg/registry/apis/folders/register.go
index 4e5e144d50e..6d4d64f1d9a 100644
--- a/pkg/registry/apis/folders/register.go
+++ b/pkg/registry/apis/folders/register.go
@@ -297,6 +297,10 @@ func (b *FolderAPIBuilder) validateOnCreate(ctx context.Context, id string, obj
 		return dashboards.ErrFolderTitleEmpty
 	}
 
+	if f.Name == getParent(obj) {
+		return folder.ErrFolderCannotBeParentOfItself
+	}
+
 	_, err := b.checkFolderMaxDepth(ctx, obj)
 	if err != nil {
 		return err
diff --git a/pkg/registry/apis/folders/register_test.go b/pkg/registry/apis/folders/register_test.go
index 5d80a2c2369..17ba1f542e1 100644
--- a/pkg/registry/apis/folders/register_test.go
+++ b/pkg/registry/apis/folders/register_test.go
@@ -9,6 +9,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apiserver/pkg/admission"
 
+	"github.com/grafana/grafana/pkg/apimachinery/utils"
 	"github.com/grafana/grafana/pkg/apis/folder/v0alpha1"
 	grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
@@ -26,13 +27,22 @@ func TestFolderAPIBuilder_Validate_Create(t *testing.T) {
 		name        string
 	}
 
-	circularObj := &v0alpha1.Folder{
+	initialMaxDepth := folderValidationRules.maxDepth
+	folderValidationRules.maxDepth = 2
+	defer func() { folderValidationRules.maxDepth = initialMaxDepth }()
+	deepFolder := &v0alpha1.Folder{
 		Spec: v0alpha1.Spec{
 			Title: "foo",
 		},
 	}
-	circularObj.Name = "valid-name"
-	circularObj.Annotations = map[string]string{"grafana.app/folder": "valid-name"}
+	deepFolder.Name = "valid-parent"
+	deepFolder.Annotations = map[string]string{"grafana.app/folder": "valid-grandparent"}
+	parentFolder := &v0alpha1.Folder{
+		Spec: v0alpha1.Spec{
+			Title: "foo-grandparent",
+		},
+	}
+	deepFolder.Name = "valid-grandparent"
 
 	tests := []struct {
 		name    string
@@ -71,12 +81,15 @@ func TestFolderAPIBuilder_Validate_Create(t *testing.T) {
 						Title: "foo",
 					},
 				},
-				annotations: map[string]string{"grafana.app/folder": "valid-name"},
+				annotations: map[string]string{"grafana.app/folder": "valid-parent"},
 				name:        "valid-name",
 			},
 			setupFn: func(m *mock.Mock) {
-				m.On("Get", mock.Anything, "valid-name", mock.Anything).Return(
-					circularObj,
+				m.On("Get", mock.Anything, "valid-parent", mock.Anything).Return(
+					deepFolder,
+					nil)
+				m.On("Get", mock.Anything, "valid-grandparent", mock.Anything).Return(
+					parentFolder,
 					nil)
 			},
 			err: folder.ErrMaximumDepthReached,
@@ -93,6 +106,19 @@ func TestFolderAPIBuilder_Validate_Create(t *testing.T) {
 			},
 			err: dashboards.ErrFolderTitleEmpty,
 		},
+		{
+			name: "should return error if folder is a parent of itself",
+			input: input{
+				annotations: map[string]string{utils.AnnoKeyFolder: "myself"},
+				obj: &v0alpha1.Folder{
+					Spec: v0alpha1.Spec{
+						Title: "title",
+					},
+				},
+				name: "myself",
+			},
+			err: folder.ErrFolderCannotBeParentOfItself,
+		},
 	}
 
 	s := (grafanarest.Storage)(nil)
diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go
index e257c7d8761..13a9943b7b2 100644
--- a/pkg/services/folder/folderimpl/sqlstore.go
+++ b/pkg/services/folder/folderimpl/sqlstore.go
@@ -39,6 +39,10 @@ func (ss *FolderStoreImpl) Create(ctx context.Context, cmd folder.CreateFolderCo
 		return nil, folder.ErrBadRequest.Errorf("missing UID")
 	}
 
+	if cmd.UID == cmd.ParentUID {
+		return nil, folder.ErrFolderCannotBeParentOfItself
+	}
+
 	var foldr *folder.Folder
 	/*
 		version := 1
diff --git a/pkg/services/folder/folderimpl/sqlstore_test.go b/pkg/services/folder/folderimpl/sqlstore_test.go
index e76607cb401..aff18e4e3d7 100644
--- a/pkg/services/folder/folderimpl/sqlstore_test.go
+++ b/pkg/services/folder/folderimpl/sqlstore_test.go
@@ -60,6 +60,18 @@ func TestIntegrationCreate(t *testing.T) {
 		require.Error(t, err)
 	})
 
+	t.Run("creating a folder with itself as a parent should fail", func(t *testing.T) {
+		uid := util.GenerateShortUID()
+		_, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{
+			Title:       folderTitle,
+			OrgID:       orgID,
+			ParentUID:   uid,
+			Description: folderDsc,
+			UID:         uid,
+		})
+		require.ErrorIs(t, err, folder.ErrFolderCannotBeParentOfItself)
+	})
+
 	t.Run("creating a folder without providing a parent should default to the empty parent folder", func(t *testing.T) {
 		uid := util.GenerateShortUID()
 		f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{
diff --git a/pkg/services/folder/model.go b/pkg/services/folder/model.go
index 769ee259f3f..19e6c43abe6 100644
--- a/pkg/services/folder/model.go
+++ b/pkg/services/folder/model.go
@@ -1,6 +1,7 @@
 package folder
 
 import (
+	"errors"
 	"fmt"
 	"time"
 
@@ -20,6 +21,7 @@ var ErrInternal = errutil.Internal("folder.internal")
 var ErrCircularReference = errutil.BadRequest("folder.circular-reference", errutil.WithPublicMessage("Circular reference detected"))
 var ErrTargetRegistrySrvConflict = errutil.Internal("folder.target-registry-srv-conflict")
 var ErrFolderNotEmpty = errutil.BadRequest("folder.not-empty", errutil.WithPublicMessage("Folder cannot be deleted: folder is not empty"))
+var ErrFolderCannotBeParentOfItself = errors.New("folder cannot be parent of itself")
 
 const (
 	GeneralFolderUID      = "general"