fix: avoid child paths in repositories (#111573)

* fix: avoid child paths in repositories

* add another unit test; fix linter

* Update pkg/registry/apis/provisioning/register.go

* skip itself

* fix: failing tests

---------

Co-authored-by: Stephanie Hingtgen <stephanie.hingtgen@grafana.com>

apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go

@@ -159,6 +159,66 @@ func (r *Repository) Branch() string {
 	return ""
 }
 
+// URL returns the URL for git-based repositories
+// or an empty string for local repositories
+func (r *Repository) URL() string {
+	if !r.Spec.Type.IsGit() {
+		return ""
+	}
+
+	switch r.Spec.Type {
+	case GitHubRepositoryType:
+		if r.Spec.GitHub != nil {
+			return r.Spec.GitHub.URL
+		}
+	case GitRepositoryType:
+		if r.Spec.Git != nil {
+			return r.Spec.Git.URL
+		}
+	case BitbucketRepositoryType:
+		if r.Spec.Bitbucket != nil {
+			return r.Spec.Bitbucket.URL
+		}
+	case GitLabRepositoryType:
+		if r.Spec.GitLab != nil {
+			return r.Spec.GitLab.URL
+		}
+	default:
+		return ""
+	}
+
+	return ""
+}
+
+func (r *Repository) Path() string {
+	switch r.Spec.Type {
+	case GitHubRepositoryType:
+		if r.Spec.GitHub != nil {
+			return r.Spec.GitHub.Path
+		}
+	case GitRepositoryType:
+		if r.Spec.Git != nil {
+			return r.Spec.Git.Path
+		}
+	case BitbucketRepositoryType:
+		if r.Spec.Bitbucket != nil {
+			return r.Spec.Bitbucket.Path
+		}
+	case GitLabRepositoryType:
+		if r.Spec.GitLab != nil {
+			return r.Spec.GitLab.Path
+		}
+	case LocalRepositoryType:
+		if r.Spec.Local != nil {
+			return r.Spec.Local.Path
+		}
+	default:
+		return ""
+	}
+
+	return ""
+}
+
 type RepositorySpec struct {
 	// The repository display name (shown in the UI)
 	Title string `json:"title"`

pkg/registry/apis/provisioning/register.go

@@ -2,9 +2,11 @@ package provisioning
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
+	"path/filepath"
 	"slices"
 	"strings"
 	"time"
@@ -75,6 +77,11 @@ var (
 	_ builder.OpenAPIPostProcessor          = (*APIBuilder)(nil)
 )
 
+var (
+	ErrRepositoryParentFolderConflict = errors.New("parent folder conflict")
+	ErrRepositoryDuplicatePath        = errors.New("duplicate repository path")
+)
+
 // JobHistoryConfig holds configuration for job history backends
 type JobHistoryConfig struct {
 	Loki *loki.Config `json:"loki,omitempty"`
@@ -626,18 +633,32 @@ func invalidRepositoryError(name string, list field.ErrorList) error {
 }
 
 func (b *APIBuilder) getRepositoriesInNamespace(ctx context.Context) ([]provisioning.Repository, error) {
-	obj, err := b.store.List(ctx, &internalversion.ListOptions{
-		Limit: 100,
-	})
-	if err != nil {
-		return nil, err
+	var allRepositories []provisioning.Repository
+	continueToken := ""
+
+	for {
+		obj, err := b.store.List(ctx, &internalversion.ListOptions{
+			Limit:    100,
+			Continue: continueToken,
+		})
+		if err != nil {
+			return nil, err
+		}
+
+		repositoryList, ok := obj.(*provisioning.RepositoryList)
+		if !ok {
+			return nil, fmt.Errorf("expected repository list")
+		}
+
+		allRepositories = append(allRepositories, repositoryList.Items...)
+
+		continueToken = repositoryList.GetContinue()
+		if continueToken == "" {
+			break
+		}
 	}
 
-	all, ok := obj.(*provisioning.RepositoryList)
-	if !ok {
-		return nil, fmt.Errorf("expected repository list")
-	}
-	return all.Items, nil
+	return allRepositories, nil
 }
 
 // TODO: move this to a more appropriate place. Probably controller/validation.go
@@ -670,6 +691,33 @@ func (b *APIBuilder) verifyAgainstExistingRepositories(cfg *provisioning.Reposit
 		}
 	}
 
+	// If repo is git, ensure no other repository is defined with a child path
+	if cfg.Spec.Type.IsGit() {
+		for _, v := range all {
+			// skip itself
+			if cfg.Name == v.Name {
+				continue
+			}
+			if v.URL() == cfg.URL() {
+				if v.Path() == cfg.Path() {
+					return field.Forbidden(field.NewPath("spec", string(cfg.Spec.Type), "path"),
+						fmt.Sprintf("%s: %s", ErrRepositoryDuplicatePath.Error(), v.Name))
+				}
+
+				relPath, err := filepath.Rel(v.Path(), cfg.Path())
+				if err != nil {
+					return field.Forbidden(field.NewPath("spec", string(cfg.Spec.Type), "path"), "failed to evaluate path: "+err.Error())
+				}
+				// https://pkg.go.dev/path/filepath#Rel
+				// Rel will return "../" if the relative paths are not related
+				if !strings.HasPrefix(relPath, "../") {
+					return field.Forbidden(field.NewPath("spec", string(cfg.Spec.Type), "path"),
+						fmt.Sprintf("%s: %s", ErrRepositoryParentFolderConflict.Error(), v.Name))
+				}
+			}
+		}
+	}
+
 	// Count repositories excluding the current one being created/updated
 	count := 0
 	for _, v := range all {

pkg/tests/apis/provisioning/repository_test.go

@@ -3,12 +3,14 @@ package provisioning
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"strings"
 	"testing"
 	"time"
 
+	provisioningAPIServer "github.com/grafana/grafana/pkg/registry/apis/provisioning"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -213,6 +215,71 @@ func TestIntegrationProvisioning_RepositoryValidation(t *testing.T) {
 			}
 		})
 	}
+
+	// Test Git repository path validation - ensure child paths are rejected
+	t.Run("Git repository path validation", func(t *testing.T) {
+		baseURL := "https://github.com/grafana/test-repo-path-validation"
+
+		pathTests := []struct {
+			name        string
+			path        string
+			expectError error
+		}{
+			{
+				name:        "first repo with path 'demo/nested' should succeed",
+				path:        "demo/nested",
+				expectError: nil,
+			},
+			{
+				name:        "second repo with child path 'demo/nested/again' should fail",
+				path:        "demo/nested/again",
+				expectError: provisioningAPIServer.ErrRepositoryParentFolderConflict,
+			},
+			{
+				name:        "third repo with parent path 'demo' should fail",
+				path:        "demo",
+				expectError: provisioningAPIServer.ErrRepositoryParentFolderConflict,
+			},
+			{
+				name:        "fourth repo with nested child path 'demo/nested/nested-second' should fail",
+				path:        "demo/nested/again/two",
+				expectError: provisioningAPIServer.ErrRepositoryParentFolderConflict,
+			},
+			{
+				name:        "fifth repo with duplicate path 'demo/nested' should fail",
+				path:        "demo/nested",
+				expectError: provisioningAPIServer.ErrRepositoryDuplicatePath,
+			},
+		}
+
+		for i, test := range pathTests {
+			t.Run(test.name, func(t *testing.T) {
+				repoName := fmt.Sprintf("git-path-test-%d", i+1)
+				gitRepo := helper.RenderObject(t, "testdata/github-readonly.json.tmpl", map[string]any{
+					"Name":        repoName,
+					"URL":         baseURL,
+					"Path":        test.path,
+					"SyncEnabled": false, // Disable sync to avoid external dependencies
+					"SyncTarget":  "folder",
+				})
+
+				_, err := helper.Repositories.Resource.Create(ctx, gitRepo, metav1.CreateOptions{FieldValidation: "Strict"})
+
+				if test.expectError != nil {
+					require.Error(t, err, "Expected error for repository with path: %s", test.path)
+
+					require.ErrorContains(t, err, test.expectError.Error(), "Error should contain expected message for path: %s", test.path)
+					var statusError *apierrors.StatusError
+					if errors.As(err, &statusError) {
+						require.Equal(t, metav1.StatusReasonInvalid, statusError.ErrStatus.Reason, "Should be a validation error")
+						require.Equal(t, http.StatusUnprocessableEntity, int(statusError.ErrStatus.Code), "Should return 422 status code")
+					}
+				} else {
+					require.NoError(t, err, "Expected success for repository with path: %s", test.path)
+				}
+			})
+		}
+	})
 }
 
 func TestIntegrationProvisioning_FailInvalidSchema(t *testing.T) {
@@ -364,7 +431,8 @@ func TestIntegrationProvisioning_CreatingGitHubRepository(t *testing.T) {
 					"Name":        test.name,
 					"URL":         test.input,
 					"SyncTarget":  "folder",
-					"SyncEnabled": false, // Disable sync since we're just testing URL cleanup
+					"SyncEnabled": false, // Disable sync since we're just testing URL cleanup,
+					"Path":        fmt.Sprintf("grafana-%s/", test.name),
 				})
 
 				_, err := helper.Repositories.Resource.Create(ctx, input, metav1.CreateOptions{})