[Prototyping] validate branch name in backend (#97061)
* Validate branch name * Add more positive cases * Make the logic more readable
This commit is contained in:
1 parent
cd7fd79c96
commit
5327c321f1
2 files changed
+75
-2
No files matched your search
@@ -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 {
|
||||
|
||||
@@ -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))
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user