Merge pull request #950 from aymanbagabas/validate-ref

git: validate reference names (#929)

Author: pjbgf

config/branch.go

@@ -54,7 +54,7 @@ func (b *Branch) Validate() error {
 		return errBranchInvalidRebase
 	}
 
-	return nil
+	return plumbing.NewBranchReferenceName(b.Name).Validate()
 }
 
 func (b *Branch) marshal() *format.Subsection {

config/config.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/go-git/go-billy/v5/osfs"
 	"github.com/go-git/go-git/v5/internal/url"
+	"github.com/go-git/go-git/v5/plumbing"
 	format "github.com/go-git/go-git/v5/plumbing/format/config"
 )
 
@@ -614,7 +615,7 @@ func (c *RemoteConfig) Validate() error {
 		c.Fetch = []RefSpec{RefSpec(fmt.Sprintf(DefaultFetchRefSpec, c.Name))}
 	}
 
-	return nil
+	return plumbing.NewRemoteHEADReferenceName(c.Name).Validate()
 }
 
 func (c *RemoteConfig) unmarshal(s *format.Subsection) error {

plumbing/reference.go

@@ -3,6 +3,7 @@ package plumbing
 import (
 	"errors"
 	"fmt"
+	"regexp"
 	"strings"
 )
 
@@ -29,6 +30,9 @@ var RefRevParseRules = []string{
 
 var (
 	ErrReferenceNotFound = errors.New("reference not found")
+
+	// ErrInvalidReferenceName is returned when a reference name is invalid.
+	ErrInvalidReferenceName = errors.New("invalid reference name")
 )
 
 // ReferenceType reference type's
@@ -124,6 +128,91 @@ func (r ReferenceName) Short() string {
 	return res
 }
 
+var (
+	ctrlSeqs = regexp.MustCompile(`[\000-\037\177]`)
+)
+
+// Validate validates a reference name.
+// This follows the git-check-ref-format rules.
+// See https://git-scm.com/docs/git-check-ref-format
+//
+// It is important to note that this function does not check if the reference
+// exists in the repository.
+// It only checks if the reference name is valid.
+// This functions does not support the --refspec-pattern, --normalize, and
+// --allow-onelevel options.
+//
+// Git imposes the following rules on how references are named:
+//
+//  1. They can include slash / for hierarchical (directory) grouping, but no
+//     slash-separated component can begin with a dot . or end with the
+//     sequence .lock.
+//  2. They must contain at least one /. This enforces the presence of a
+//     category like heads/, tags/ etc. but the actual names are not
+//     restricted. If the --allow-onelevel option is used, this rule is
+//     waived.
+//  3. They cannot have two consecutive dots .. anywhere.
+//  4. They cannot have ASCII control characters (i.e. bytes whose values are
+//     lower than \040, or \177 DEL), space, tilde ~, caret ^, or colon :
+//     anywhere.
+//  5. They cannot have question-mark ?, asterisk *, or open bracket [
+//     anywhere. See the --refspec-pattern option below for an exception to this
+//     rule.
+//  6. They cannot begin or end with a slash / or contain multiple consecutive
+//     slashes (see the --normalize option below for an exception to this rule).
+//  7. They cannot end with a dot ..
+//  8. They cannot contain a sequence @{.
+//  9. They cannot be the single character @.
+//  10. They cannot contain a \.
+func (r ReferenceName) Validate() error {
+	s := string(r)
+	if len(s) == 0 {
+		return ErrInvalidReferenceName
+	}
+
+	// HEAD is a special case
+	if r == HEAD {
+		return nil
+	}
+
+	// rule 7
+	if strings.HasSuffix(s, ".") {
+		return ErrInvalidReferenceName
+	}
+
+	// rule 2
+	parts := strings.Split(s, "/")
+	if len(parts) < 2 {
+		return ErrInvalidReferenceName
+	}
+
+	isBranch := r.IsBranch()
+	isTag := r.IsTag()
+	for _, part := range parts {
+		// rule 6
+		if len(part) == 0 {
+			return ErrInvalidReferenceName
+		}
+
+		if strings.HasPrefix(part, ".") || // rule 1
+			strings.Contains(part, "..") || // rule 3
+			ctrlSeqs.MatchString(part) || // rule 4
+			strings.ContainsAny(part, "~^:?*[ \t\n") || // rule 4 & 5
+			strings.Contains(part, "@{") || // rule 8
+			part == "@" || // rule 9
+			strings.Contains(part, "\\") || // rule 10
+			strings.HasSuffix(part, ".lock") { // rule 1
+			return ErrInvalidReferenceName
+		}
+
+		if (isBranch || isTag) && strings.HasPrefix(part, "-") { // branches & tags can't start with -
+			return ErrInvalidReferenceName
+		}
+	}
+
+	return nil
+}
+
 const (
 	HEAD   ReferenceName = "HEAD"
 	Master ReferenceName = "refs/heads/master"

plumbing/reference_test.go

@@ -103,6 +103,65 @@ func (s *ReferenceSuite) TestIsTag(c *C) {
 	c.Assert(r.IsTag(), Equals, true)
 }
 
+func (s *ReferenceSuite) TestValidReferenceNames(c *C) {
+	valid := []ReferenceName{
+		"refs/heads/master",
+		"refs/notes/commits",
+		"refs/remotes/origin/master",
+		"HEAD",
+		"refs/tags/v3.1.1",
+		"refs/pulls/1/head",
+		"refs/pulls/1/merge",
+		"refs/pulls/1/abc.123",
+		"refs/pulls",
+		"refs/-", // should this be allowed?
+	}
+	for _, v := range valid {
+		c.Assert(v.Validate(), IsNil)
+	}
+
+	invalid := []ReferenceName{
+		"refs",
+		"refs/",
+		"refs//",
+		"refs/heads/\\",
+		"refs/heads/\\foo",
+		"refs/heads/\\foo/bar",
+		"abc",
+		"",
+		"refs/heads/ ",
+		"refs/heads/ /",
+		"refs/heads/ /foo",
+		"refs/heads/.",
+		"refs/heads/..",
+		"refs/heads/foo..",
+		"refs/heads/foo.lock",
+		"refs/heads/foo@{bar}",
+		"refs/heads/foo[",
+		"refs/heads/foo~",
+		"refs/heads/foo^",
+		"refs/heads/foo:",
+		"refs/heads/foo?",
+		"refs/heads/foo*",
+		"refs/heads/foo[bar",
+		"refs/heads/foo\t",
+		"refs/heads/@",
+		"refs/heads/@{bar}",
+		"refs/heads/\n",
+		"refs/heads/-foo",
+		"refs/heads/foo..bar",
+		"refs/heads/-",
+		"refs/tags/-",
+		"refs/tags/-foo",
+	}
+
+	for i, v := range invalid {
+		comment := Commentf("invalid reference name case %d: %s", i, v)
+		c.Assert(v.Validate(), NotNil, comment)
+		c.Assert(v.Validate(), ErrorMatches, "invalid reference name", comment)
+	}
+}
+
 func benchMarkReferenceString(r *Reference, b *testing.B) {
 	for n := 0; n < b.N; n++ {
 		_ = r.String()

repository.go

@@ -98,6 +98,10 @@ func InitWithOptions(s storage.Storer, worktree billy.Filesystem, options InitOp
 		options.DefaultBranch = plumbing.Master
 	}
 
+	if err := options.DefaultBranch.Validate(); err != nil {
+		return nil, err
+	}
+
 	r := newRepository(s, worktree)
 	_, err := r.Reference(plumbing.HEAD, false)
 	switch err {
@@ -724,7 +728,10 @@ func (r *Repository) DeleteBranch(name string) error {
 // CreateTag creates a tag. If opts is included, the tag is an annotated tag,
 // otherwise a lightweight tag is created.
 func (r *Repository) CreateTag(name string, hash plumbing.Hash, opts *CreateTagOptions) (*plumbing.Reference, error) {
-	rname := plumbing.ReferenceName(path.Join("refs", "tags", name))
+	rname := plumbing.NewTagReferenceName(name)
+	if err := rname.Validate(); err != nil {
+		return nil, err
+	}
 
 	_, err := r.Storer.Reference(rname)
 	switch err {

repository_test.go

@@ -75,6 +75,13 @@ func (s *RepositorySuite) TestInitWithOptions(c *C) {
 
 }
 
+func (s *RepositorySuite) TestInitWithInvalidDefaultBranch(c *C) {
+	_, err := InitWithOptions(memory.NewStorage(), memfs.New(), InitOptions{
+		DefaultBranch: "foo",
+	})
+	c.Assert(err, NotNil)
+}
+
 func createCommit(c *C, r *Repository) {
 	// Create a commit so there is a HEAD to check
 	wt, err := r.Worktree()
@@ -391,6 +398,22 @@ func (s *RepositorySuite) TestDeleteRemote(c *C) {
 	c.Assert(alt, IsNil)
 }
 
+func (s *RepositorySuite) TestEmptyCreateBranch(c *C) {
+	r, _ := Init(memory.NewStorage(), nil)
+	err := r.CreateBranch(&config.Branch{})
+
+	c.Assert(err, NotNil)
+}
+
+func (s *RepositorySuite) TestInvalidCreateBranch(c *C) {
+	r, _ := Init(memory.NewStorage(), nil)
+	err := r.CreateBranch(&config.Branch{
+		Name: "-foo",
+	})
+
+	c.Assert(err, NotNil)
+}
+
 func (s *RepositorySuite) TestCreateBranchAndBranch(c *C) {
 	r, _ := Init(memory.NewStorage(), nil)
 	testBranch := &config.Branch{
@@ -2797,6 +2820,20 @@ func (s *RepositorySuite) TestDeleteTagAnnotatedUnpacked(c *C) {
 	c.Assert(err, Equals, plumbing.ErrObjectNotFound)
 }
 
+func (s *RepositorySuite) TestInvalidTagName(c *C) {
+	r, err := Init(memory.NewStorage(), nil)
+	c.Assert(err, IsNil)
+	for i, name := range []string{
+		"",
+		"foo bar",
+		"foo\tbar",
+		"foo\nbar",
+	} {
+		_, err = r.CreateTag(name, plumbing.ZeroHash, nil)
+		c.Assert(err, NotNil, Commentf("case %d %q", i, name))
+	}
+}
+
 func (s *RepositorySuite) TestBranches(c *C) {
 	f := fixtures.ByURL("https://github.com/git-fixtures/root-references.git").One()
 	sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault())

worktree.go

@@ -189,6 +189,10 @@ func (w *Worktree) Checkout(opts *CheckoutOptions) error {
 	return w.Reset(ro)
 }
 func (w *Worktree) createBranch(opts *CheckoutOptions) error {
+	if err := opts.Branch.Validate(); err != nil {
+		return err
+	}
+
 	_, err := w.r.Storer.Reference(opts.Branch)
 	if err == nil {
 		return fmt.Errorf("a branch named %q already exists", opts.Branch)

worktree_test.go

@@ -785,6 +785,30 @@ func (s *WorktreeSuite) TestCheckoutCreateMissingBranch(c *C) {
 	c.Assert(err, Equals, ErrCreateRequiresBranch)
 }
 
+func (s *WorktreeSuite) TestCheckoutCreateInvalidBranch(c *C) {
+	w := &Worktree{
+		r:          s.Repository,
+		Filesystem: memfs.New(),
+	}
+
+	for _, name := range []plumbing.ReferenceName{
+		"foo",
+		"-",
+		"-foo",
+		"refs/heads//",
+		"refs/heads/..",
+		"refs/heads/a..b",
+		"refs/heads/.",
+	} {
+		err := w.Checkout(&CheckoutOptions{
+			Create: true,
+			Branch: name,
+		})
+
+		c.Assert(err, Equals, plumbing.ErrInvalidReferenceName)
+	}
+}
+
 func (s *WorktreeSuite) TestCheckoutTag(c *C) {
 	f := fixtures.ByTag("tags").One()
 	r := s.NewRepositoryWithEmptyWorktree(f)