From 965f5784c16721609bdcdcadfda5ccc42a1efc56 Mon Sep 17 00:00:00 2001 From: Ian Lewis Date: Sat, 1 Jul 2023 09:03:52 +0900 Subject: [PATCH] refactor: Add more git utils (#645) Adds the functions `NormalizeGitURI`, `ParseGitURIAndRef`, and `ValidateGitRef`. `ParseGitRef` was updated to be permissive of the ref type whereas `ValidateGitRef` validates that the type is of a given type. Code extracted from #641 Signed-off-by: Ian Lewis --- verifiers/internal/gha/provenance.go | 87 +++---- verifiers/internal/gha/provenance_test.go | 7 + verifiers/utils/git.go | 55 ++++- verifiers/utils/git_test.go | 269 ++++++++++++++++++++++ 4 files changed, 349 insertions(+), 69 deletions(-) create mode 100644 verifiers/utils/git_test.go diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index 731fb8f..164d79a 100644 --- a/verifiers/internal/gha/provenance.go +++ b/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() diff --git a/verifiers/internal/gha/provenance_test.go b/verifiers/internal/gha/provenance_test.go index 4415578..8be35ff 100644 --- a/verifiers/internal/gha/provenance_test.go +++ b/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://notgithub.com/some/repo@v1.2.3", + provMaterialsURI: "git+https://notgithub.com/some/repo@v1.2.3", + expectedSourceURI: "git+https://notgithub.com/some/repo", + err: serrors.ErrorMalformedURI, + }, { name: "match source", provTriggerURI: "git+https://github.com/some/repo@v1.2.3", diff --git a/verifiers/utils/git.go b/verifiers/utils/git.go index 1e330ec..12f586e 100644 --- a/verifiers/utils/git.go +++ b/verifiers/utils/git.go @@ -7,16 +7,51 @@ 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'", 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) } - name := strings.TrimPrefix(ref, refPrefix) - if strings.TrimSpace(name) == "" { - return "", fmt.Errorf("%w: %s: not of the form '%s'", serrors.ErrorInvalidRef, ref, refPrefix) + r := strings.SplitN(uri, "@", 2) + if len(r) < 2 { + return r[0], "", nil + } + + 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 @@ -24,10 +59,10 @@ func ParseGitRef(refType, ref string) (string, error) { // 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) } diff --git a/verifiers/utils/git_test.go b/verifiers/utils/git_test.go new file mode 100644 index 0000000..d8d3a67 --- /dev/null +++ b/verifiers/utils/git_test.go @@ -0,0 +1,269 @@ +package utils + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + serrors "github.com/slsa-framework/slsa-verifier/v2/errors" +) + +func Test_NormalizeGitURI(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + uri string + expected string + }{ + { + name: "empty uri", + uri: "", + expected: "git+https://", + }, + { + name: "https scheme", + uri: "https://github.com/kubernetes/kubernetes@refs/tags/v1.0.0", + expected: "git+https://github.com/kubernetes/kubernetes@refs/tags/v1.0.0", + }, + { + name: "http scheme", + uri: "http://github.com/kubernetes/kubernetes@refs/tags/v1.0.0", + expected: "git+http://github.com/kubernetes/kubernetes@refs/tags/v1.0.0", + }, + { + name: "git+https scheme", + uri: "git+https://github.com/kubernetes/kubernetes@refs/tags/v1.0.0", + expected: "git+https://github.com/kubernetes/kubernetes@refs/tags/v1.0.0", + }, + { + name: "no scheme", + uri: "github.com/kubernetes/kubernetes@refs/tags/v1.0.0", + expected: "git+https://github.com/kubernetes/kubernetes@refs/tags/v1.0.0", + }, + { + name: "git+ scheme", + uri: "git+github.com/kubernetes/kubernetes@refs/tags/v1.0.0", + expected: "git+github.com/kubernetes/kubernetes@refs/tags/v1.0.0", + }, + { + name: "https scheme no ref", + uri: "https://github.com/kubernetes/kubernetes", + expected: "git+https://github.com/kubernetes/kubernetes", + }, + { + name: "http scheme no ref", + uri: "http://github.com/kubernetes/kubernetes", + expected: "git+http://github.com/kubernetes/kubernetes", + }, + { + name: "git+https scheme no ref", + uri: "git+https://github.com/kubernetes/kubernetes", + expected: "git+https://github.com/kubernetes/kubernetes", + }, + { + name: "no scheme no ref", + uri: "github.com/kubernetes/kubernetes", + expected: "git+https://github.com/kubernetes/kubernetes", + }, + { + name: "git+ scheme no ref", + uri: "git+github.com/kubernetes/kubernetes", + expected: "git+github.com/kubernetes/kubernetes", + }, + } + + for i := range testCases { + tt := testCases[i] + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + if got, want := NormalizeGitURI(tt.uri), tt.expected; got != want { + t.Errorf("unexpected value, got: %q, want: %q", got, want) + } + }) + } +} + +func Test_ParseGitURIAndRef(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + uri string + expectedURI string + expectedRef string + err error + }{ + { + name: "empty uri", + uri: "", + err: serrors.ErrorMalformedURI, + }, + { + name: "no scheme with ref", + uri: "github.com/kubernetes/kubernetes@v1.0.0", + err: serrors.ErrorMalformedURI, + }, + { + name: "https scheme with ref", + uri: "https://github.com/kubernetes/kubernetes@v1.0.0", + err: serrors.ErrorMalformedURI, + }, + { + name: "git+https scheme with ref", + uri: "git+https://github.com/kubernetes/kubernetes@v1.0.0", + expectedURI: "git+https://github.com/kubernetes/kubernetes", + expectedRef: "v1.0.0", + }, + } + + for i := range testCases { + tt := testCases[i] + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + uri, ref, err := ParseGitURIAndRef(tt.uri) + if diff := cmp.Diff(tt.err, err, cmpopts.EquateErrors()); diff != "" { + t.Fatalf("unexpected error: %v", err) + } + + if want, got := tt.expectedURI, uri; got != want { + t.Fatalf("unexpected uri, got: %q, want: %q", got, want) + } + + if want, got := tt.expectedRef, ref; got != want { + t.Fatalf("unexpected ref, got: %q, want: %q", got, want) + } + }) + } +} + +func Test_ParseGitRef(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + ref string + expectedType string + expectedRef string + }{ + { + name: "empty ref", + ref: "", + expectedType: "", + expectedRef: "", + }, + { + name: "no type ", + ref: "v1.0.0", + expectedType: "", + expectedRef: "v1.0.0", + }, + { + name: "no type with slash", + ref: "tags/v1.0.0", + expectedType: "", + expectedRef: "tags/v1.0.0", + }, + { + name: "type without slash", + ref: "refs/mytype/v1.0.0", + expectedType: "mytype", + expectedRef: "v1.0.0", + }, + { + name: "type with slash", + ref: "refs/mytype/feat/v1.0.0", + expectedType: "mytype", + expectedRef: "feat/v1.0.0", + }, + } + + for i := range testCases { + tt := testCases[i] + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + typ, ref := ParseGitRef(tt.ref) + if want, got := tt.expectedType, typ; got != want { + t.Fatalf("unexpected type, got: %q, want: %q", got, want) + } + + if want, got := tt.expectedRef, ref; got != want { + t.Fatalf("unexpected ref, got: %q, want: %q", got, want) + } + }) + } +} + +func Test_ValidateGitRef(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + ref string + typ string + expectedRef string + err error + }{ + { + name: "empty ref", + ref: "", + typ: "", + err: serrors.ErrorInvalidRef, + }, + { + name: "no type ", + ref: "v1.0.0", + typ: "", + expectedRef: "v1.0.0", + }, + { + name: "no type with slash", + ref: "tags/v1.0.0", + typ: "", + expectedRef: "tags/v1.0.0", + }, + { + name: "type without slash", + ref: "refs/mytype/v1.0.0", + typ: "mytype", + expectedRef: "v1.0.0", + }, + { + name: "mismatch type", + ref: "refs/mytype/v1.0.0", + typ: "tags", + err: serrors.ErrorInvalidRef, + }, + { + name: "type with slash", + ref: "refs/mytype/feat/v1.0.0", + typ: "mytype", + expectedRef: "feat/v1.0.0", + }, + { + name: "mismatch type with slash", + ref: "refs/mytype/feat/v1.0.0", + typ: "tags", + err: serrors.ErrorInvalidRef, + }, + } + + for i := range testCases { + tt := testCases[i] + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ref, err := ValidateGitRef(tt.typ, tt.ref) + if diff := cmp.Diff(tt.err, err, cmpopts.EquateErrors()); diff != "" { + t.Fatalf("unexpected error: %v", err) + } + + if want, got := tt.expectedRef, ref; got != want { + t.Fatalf("unexpected ref, got: %q, want: %q", got, want) + } + }) + } +}