refactor: Add more git utils

verifiers/internal/gha/provenance.go

@@ -76,100 +76,69 @@ func verifyBuilderIDLooseMatch(prov iface.Provenance, expectedBuilderID string)
 	return nil
 }
 
-func asURI(s string) string {
-	source := s
-	if !strings.HasPrefix(source, "https://") &&
-		!strings.HasPrefix(source, "git+") {
-		source = "git+https://" + source
-	}
-	if !strings.HasPrefix(source, "git+") {
-		source = "git+" + source
-	}
-
-	return source
-}
-
 // Verify source URI in provenance statement.
 func verifySourceURI(prov iface.Provenance, expectedSourceURI string, allowNoMaterialRef bool) error {
-	source := asURI(expectedSourceURI)
+	source := utils.NormalizeGitURI(expectedSourceURI)
 
 	// We expect github.com URIs only.
 	if !strings.HasPrefix(source, "git+https://github.com/") {
-		return fmt.Errorf("%w: expected source github.com repository '%s'", serrors.ErrorMalformedURI,
+		return fmt.Errorf("%w: expected source github.com repository %q", serrors.ErrorMalformedURI,
 			source)
 	}
 
 	// Verify source in the trigger
-	fullConfigURI, err := prov.TriggerURI()
+	fullTriggerURI, err := prov.TriggerURI()
 	if err != nil {
 		return err
 	}
 
-	configURI, err := sourceFromURI(fullConfigURI, false)
+	triggerURI, triggerRef, err := utils.ParseGitURIAndRef(fullTriggerURI)
 	if err != nil {
 		return err
 	}
-	if configURI != source {
-		return fmt.Errorf("%w: expected source '%s' in configSource.uri, got '%s'", serrors.ErrorMismatchSource,
-			source, fullConfigURI)
+	if triggerURI != source {
+		return fmt.Errorf("%w: expected source '%s' in configSource.uri, got %q", serrors.ErrorMismatchSource,
+			source, fullTriggerURI)
+	}
+	// We expect the trigger URI to always have a ref.
+	if triggerRef == "" {
+		return fmt.Errorf("%w: missing ref: %q", serrors.ErrorMalformedURI, fullTriggerURI)
 	}
 
 	// Verify source from material section.
-	materialSourceURI, err := prov.SourceURI()
+	fullSourceURI, err := prov.SourceURI()
 	if err != nil {
 		return err
 	}
 
-	materialURI, err := sourceFromURI(materialSourceURI, allowNoMaterialRef)
+	sourceURI, sourceRef, err := utils.ParseGitURIAndRef(fullSourceURI)
 	if err != nil {
 		return err
 	}
-	if materialURI != source {
-		return fmt.Errorf("%w: expected source '%s' in material section, got '%s'", serrors.ErrorMismatchSource,
-			source, materialSourceURI)
+	if sourceURI != source {
+		return fmt.Errorf("%w: expected source '%s' in material section, got %q", serrors.ErrorMismatchSource,
+			source, fullSourceURI)
 	}
 
-	// Last, verify that both fields match.
-	// We use the full URI to match on the tag as well.
-	if allowNoMaterialRef && len(strings.Split(materialSourceURI, "@")) == 1 {
-		// NOTE: this is an exception for npm packages built before GA,
-		// see https://github.com/slsa-framework/slsa-verifier/issues/492.
-		// We don't need to compare the ref since materialSourceURI does not contain it.
-		return nil
+	if sourceRef == "" {
+		if allowNoMaterialRef {
+			// NOTE: this is an exception for npm packages built before GA,
+			// see https://github.com/slsa-framework/slsa-verifier/issues/492.
+			// We don't need to compare the ref since materialSourceURI does not contain it.
+			return nil
+		}
+		return fmt.Errorf("%w: missing ref: %q", serrors.ErrorMalformedURI, fullSourceURI)
 	}
-	if fullConfigURI != materialSourceURI {
-		return fmt.Errorf("%w: material and config URIs do not match: '%s' != '%s'",
+
+	if fullTriggerURI != fullSourceURI {
+		return fmt.Errorf("%w: material and config URIs do not match: %q != %q",
 			serrors.ErrorInvalidDssePayload,
-			fullConfigURI, materialSourceURI)
+			fullTriggerURI, fullSourceURI)
 	}
 
 	return nil
 }
 
-// sourceFromURI retrieves the source repository given a repository URI with ref.
-//
-// NOTE: `allowNoRef` is to allow for verification of npm packages
-// generated before GA. Their provenance did not have a ref,
-// see https://github.com/slsa-framework/slsa-verifier/issues/492.
-// `allowNoRef` should be set to `false` for all other cases.
-func sourceFromURI(uri string, allowNoRef bool) (string, error) {
-	if uri == "" {
-		return "", fmt.Errorf("%w: empty uri", serrors.ErrorMalformedURI)
-	}
-
-	r := strings.Split(uri, "@")
-	if len(r) < 2 && !allowNoRef {
-		return "", fmt.Errorf("%w: %s", serrors.ErrorMalformedURI,
-			uri)
-	}
-	if len(r) < 1 {
-		return "", fmt.Errorf("%w: %s", serrors.ErrorMalformedURI,
-			uri)
-	}
-
-	return r[0], nil
-}
-
 // Verify Subject Digest from the provenance statement.
 func verifyDigest(prov iface.Provenance, expectedHash string) error {
 	subjects, err := prov.Subjects()

verifiers/internal/gha/provenance_test.go

@@ -263,6 +263,13 @@ func Test_verifySourceURI(t *testing.T) {
 			expectedSourceURI: "git+https://github.com/some/repo",
 			err:               serrors.ErrorMalformedURI,
 		},
+		{
+			name:              "not github repo",
+			provTriggerURI:    "git+https://notgit.luolix.top/some/repo@v1.2.3",
+			provMaterialsURI:  "git+https://notgit.luolix.top/some/repo@v1.2.3",
+			expectedSourceURI: "git+https://notgit.luolix.top/some/repo",
+			err:               serrors.ErrorMalformedURI,
+		},
 		{
 			name:              "match source",
 			provTriggerURI:    "git+https://github.com/some/repo@v1.2.3",

verifiers/utils/git.go

@@ -7,27 +7,62 @@ import (
 	serrors "github.com/slsa-framework/slsa-verifier/v2/errors"
 )
 
-// ParseGitRef validates that the given git ref is a valid ref of the given type and returns its name.
-func ParseGitRef(refType, ref string) (string, error) {
-	refPrefix := fmt.Sprintf("refs/%s/", refType)
-	if !strings.HasPrefix(ref, refPrefix) {
-		return "", fmt.Errorf("%w: %s: not of the form '%s<name>'", serrors.ErrorInvalidRef, ref, refPrefix)
+// NormalizeGitURI normalizes a git URI to include a git+https:// prefix.
+func NormalizeGitURI(s string) string {
+	if !strings.HasPrefix(s, "git+") {
+		if !strings.Contains(s, "://") {
+			return "git+https://" + s
+		}
+		return "git+" + s
+	}
+	return s
+}
+
+// ParseGitURIAndRef retrieves the URI and ref from the given URI.
+func ParseGitURIAndRef(uri string) (string, string, error) {
+	if uri == "" {
+		return "", "", fmt.Errorf("%w: empty uri", serrors.ErrorMalformedURI)
+	}
+	if !strings.HasPrefix(uri, "git+") {
+		return "", "", fmt.Errorf("%w: not a git URI: %q", serrors.ErrorMalformedURI, uri)
+	}
+
+	r := strings.SplitN(uri, "@", 2)
+	if len(r) < 2 {
+		return r[0], "", nil
 	}
 
-	name := strings.TrimPrefix(ref, refPrefix)
-	if strings.TrimSpace(name) == "" {
-		return "", fmt.Errorf("%w: %s: not of the form '%s<name>'", serrors.ErrorInvalidRef, ref, refPrefix)
+	return r[0], r[1], nil
+}
+
+// ParseGitRef parses the git ref and returns its type and name.
+func ParseGitRef(ref string) (string, string) {
+	parts := strings.SplitN(ref, "/", 3)
+	if len(parts) < 3 || parts[0] != "refs" {
+		return "", ref
+	}
+	return parts[1], parts[2]
+}
+
+// ValidateGitRef validates that the given git ref is a valid ref of the given type and returns its name.
+func ValidateGitRef(refType, ref string) (string, error) {
+	typ, name := ParseGitRef(ref)
+	if typ != refType {
+		return "", fmt.Errorf("%w: %q: unexpected ref type: %q", serrors.ErrorInvalidRef, ref, typ)
+	}
+	if name == "" {
+		return "", fmt.Errorf("%w: %q: empty ref name", serrors.ErrorInvalidRef, ref)
 	}
 
 	return name, nil
 }
 
 // TagFromGitRef returns the tagname from a tag ref.
 func TagFromGitRef(ref string) (string, error) {
-	return ParseGitRef("tags", ref)
+	return ValidateGitRef("tags", ref)
 }
 
 // BranchFromGitRef returns the tagname from a tag ref.
 func BranchFromGitRef(ref string) (string, error) {
-	return ParseGitRef("heads", ref)
+	return ValidateGitRef("heads", ref)
 }