diff --git a/pkg/registry/apis/provisioning/repository/github.go b/pkg/registry/apis/provisioning/repository/github.go index 93580add86e..1048fc2e6f5 100644 --- a/pkg/registry/apis/provisioning/repository/github.go +++ b/pkg/registry/apis/provisioning/repository/github.go @@ -7,6 +7,8 @@ import ( "log/slog" "net/http" "net/url" + "regexp" + "strings" "github.com/google/go-github/v66/github" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -54,6 +56,9 @@ func (r *githubRepository) Validate() (list field.ErrorList) { if gh.Branch == "" { list = append(list, field.Required(field.NewPath("spec", "github", "branch"), "a github branch is required")) } + if !isValidGitBranchName(gh.Branch) { + list = append(list, field.Invalid(field.NewPath("spec", "github", "branch"), gh.Branch, "invalid branch name")) + } if gh.Token == "" { list = append(list, field.Required(field.NewPath("spec", "github", "token"), "a github access token is required")) } @@ -203,10 +208,31 @@ func (r *githubRepository) Delete(ctx context.Context, logger *slog.Logger, path return r.gh.DeleteFile(ctx, owner, repo, path, ref, comment, file.GetSHA()) } +// basicGitBranchNameRegex is a regular expression to validate a git branch name +// it does not cover all cases as positive lookaheads are not supported in Go's regexp +var basicGitBranchNameRegex = regexp.MustCompile(`^[a-zA-Z0-9\-\_\/\.]+$`) + // isValidGitBranchName checks if a branch name is valid. +// It uses the following regexp `^[a-zA-Z0-9\-\_\/\.]+$` to validate the branch name with some additional checks that must satisfy the following rules: +// 1. The branch name must have at least one character and must not be empty. +// 2. The branch name cannot start with `/` or end with `/`, `.`, or whitespace. +// 3. The branch name cannot contain consecutive slashes (`//`). +// 4. The branch name cannot contain consecutive dots (`..`). +// 5. The branch name cannot contain `@{`. +// 6. The branch name cannot include the following characters: `~`, `^`, `:`, `?`, `*`, `[`, `\`, or `]`. func isValidGitBranchName(branch string) bool { - // TODO: add branch validation here as well as in the frontend using the same restrictions - return branch != "" + if !basicGitBranchNameRegex.MatchString(branch) { + return false + } + + // Additional checks for invalid patterns + if strings.HasPrefix(branch, "/") || strings.HasSuffix(branch, "/") || + strings.HasSuffix(branch, ".") || strings.Contains(branch, "..") || + strings.Contains(branch, "//") || strings.HasSuffix(branch, ".lock") { + return false + } + + return true } func (r *githubRepository) ensureBranchExists(ctx context.Context, branchName string) error { diff --git a/pkg/registry/apis/provisioning/repository/github_test.go b/pkg/registry/apis/provisioning/repository/github_test.go new file mode 100644 index 00000000000..90688f15350 --- /dev/null +++ b/pkg/registry/apis/provisioning/repository/github_test.go @@ -0,0 +1,47 @@ +package repository + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsValidGitBranchName(t *testing.T) { + tests := []struct { + name string + branch string + expected bool + }{ + {"Valid branch name", "feature/add-tests", true}, + {"Valid branch name with numbers", "feature/123-add-tests", true}, + {"Valid branch name with dots", "feature.add.tests", true}, + {"Valid branch name with hyphens", "feature-add-tests", true}, + {"Valid branch name with underscores", "feature_add_tests", true}, + {"Valid branch name with mixed characters", "feature/add_tests-123", true}, + {"Starts with /", "/feature", false}, + {"Ends with /", "feature/", false}, + {"Ends with .", "feature.", false}, + {"Ends with space", "feature ", false}, + {"Contains consecutive slashes", "feature//branch", false}, + {"Contains consecutive dots", "feature..branch", false}, + {"Contains @{", "feature@{branch", false}, + {"Contains invalid character ~", "feature~branch", false}, + {"Contains invalid character ^", "feature^branch", false}, + {"Contains invalid character :", "feature:branch", false}, + {"Contains invalid character ?", "feature?branch", false}, + {"Contains invalid character *", "feature*branch", false}, + {"Contains invalid character [", "feature[branch", false}, + {"Contains invalid character ]", "feature]branch", false}, + {"Contains invalid character \\", "feature\\branch", false}, + {"Empty branch name", "", false}, + {"Only whitespace", " ", false}, + {"Single valid character", "a", true}, + {"Ends with .lock", "feature.lock", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, isValidGitBranchName(tt.branch)) + }) + } +}