From 5b5289863fcebd0b384770da24315dfb0586eee3 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Tue, 24 Sep 2024 13:26:10 -0700 Subject: [PATCH] move githubVerifier to its own file Signed-off-by: Spencer Schrock --- app/server/github_verifier.go | 92 ++++++++++++++++++++++++++++++ app/server/github_verifier_test.go | 76 ++++++++++++++++++++++++ app/server/verify_workflow.go | 72 ----------------------- app/server/verify_workflow_test.go | 55 ------------------ 4 files changed, 168 insertions(+), 127 deletions(-) create mode 100644 app/server/github_verifier.go create mode 100644 app/server/github_verifier_test.go diff --git a/app/server/github_verifier.go b/app/server/github_verifier.go new file mode 100644 index 00000000..6e6bebeb --- /dev/null +++ b/app/server/github_verifier.go @@ -0,0 +1,92 @@ +// Copyright 2024 OpenSSF Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package server + +import ( + "context" + "fmt" + "net/http" + + "github.com/google/go-github/v65/github" +) + +type githubVerifier struct { + ctx context.Context + client *github.Client +} + +// contains makes two "core" API calls: one for the default branch, and one to compare the target hash to a branch +// if the repo is "github/codeql-action", also check releases/v1 before failing. +func (g *githubVerifier) contains(owner, repo, hash string) (bool, error) { + defaultBranch, err := g.defaultBranch(owner, repo) + if err != nil { + return false, err + } + contains, err := g.branchContains(defaultBranch, owner, repo, hash) + if err != nil { + return false, err + } + if contains { + return true, nil + } + + switch { + // github/codeql-action has commits from their release branches that don't show up in the default branch + // this isn't the best approach for now, but theres no universal "does this commit belong to this repo" call + case owner == "github" && repo == "codeql-action": + releaseBranches := []string{"releases/v3", "releases/v2", "releases/v1"} + for _, branch := range releaseBranches { + contains, err = g.branchContains(branch, owner, repo, hash) + if err != nil { + return false, err + } + if contains { + return true, nil + } + } + + // add fallback lookup for actions/upload-artifact v3/node20 branch + // https://github.com/actions/starter-workflows/pull/2348#discussion_r1536228344 + case owner == "actions" && repo == "upload-artifact": + contains, err = g.branchContains("v3/node20", owner, repo, hash) + } + return contains, err +} + +func (g *githubVerifier) defaultBranch(owner, repo string) (string, error) { + githubRepository, _, err := g.client.Repositories.Get(g.ctx, owner, repo) + if err != nil { + return "", fmt.Errorf("fetching repository info: %w", err) + } + if githubRepository == nil || githubRepository.DefaultBranch == nil { + return "", errNoDefaultBranch + } + return *githubRepository.DefaultBranch, nil +} + +func (g *githubVerifier) branchContains(branch, owner, repo, hash string) (bool, error) { + opts := &github.ListOptions{PerPage: 1} + diff, resp, err := g.client.Repositories.CompareCommits(g.ctx, owner, repo, branch, hash, opts) + if err != nil { + if resp.StatusCode == http.StatusNotFound { + // NotFound can be returned for some divergent cases: "404 No common ancestor between ..." + return false, nil + } + return false, fmt.Errorf("error comparing revisions: %w", err) + } + + // Target should be behind or at the base ref if it is considered contained. + return diff.GetStatus() == "behind" || diff.GetStatus() == "identical", nil +} diff --git a/app/server/github_verifier_test.go b/app/server/github_verifier_test.go new file mode 100644 index 00000000..29a73881 --- /dev/null +++ b/app/server/github_verifier_test.go @@ -0,0 +1,76 @@ +// Copyright 2024 OpenSSF Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package server + +import ( + "context" + "net/http" + "testing" + + "github.com/google/go-github/v65/github" +) + +func Test_githubVerifier_contains_codeql_v1(t *testing.T) { + t.Parallel() + httpClient := http.Client{ + Transport: suffixStubTripper{ + responsePaths: map[string]string{ + "codeql-action": "./testdata/api/github/repository.json", // api call which finds the default branch + "main...somehash": "./testdata/api/github/divergent.json", // doesnt belong to default branch + "v3...somehash": "./testdata/api/github/divergent.json", // doesnt belong to releases/v3 branch + "v2...somehash": "./testdata/api/github/divergent.json", // doesnt belong to releases/v2 branch + "v1...somehash": "./testdata/api/github/containsCommit.json", // belongs to releases/v1 branch + }, + }, + } + client := github.NewClient(&httpClient) + gv := githubVerifier{ + ctx: context.Background(), + client: client, + } + got, err := gv.contains("github", "codeql-action", "somehash") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != true { + t.Errorf("expected to contain hash, but it didnt") + } +} + +func Test_githubVerifier_contains_codeql_v2(t *testing.T) { + t.Parallel() + httpClient := http.Client{ + Transport: suffixStubTripper{ + responsePaths: map[string]string{ + "codeql-action": "./testdata/api/github/repository.json", // api call which finds the default branch + "main...somehash": "./testdata/api/github/divergent.json", // doesnt belong to default branch + "v3...somehash": "./testdata/api/github/divergent.json", // doesnt belong to releases/v3 branch either + "v2...somehash": "./testdata/api/github/containsCommit.json", // belongs to releases/v2 branch + }, + }, + } + client := github.NewClient(&httpClient) + gv := githubVerifier{ + ctx: context.Background(), + client: client, + } + got, err := gv.contains("github", "codeql-action", "somehash") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != true { + t.Errorf("expected to contain hash, but it didnt") + } +} diff --git a/app/server/verify_workflow.go b/app/server/verify_workflow.go index 8041ce1d..0e9155d8 100644 --- a/app/server/verify_workflow.go +++ b/app/server/verify_workflow.go @@ -15,14 +15,11 @@ package server import ( - "context" "errors" "fmt" - "net/http" "regexp" "strings" - "github.com/google/go-github/v65/github" "github.com/rhysd/actionlint" ) @@ -245,75 +242,6 @@ func isCommitHash(s string) bool { return reCommitSHA.MatchString(s) } -type githubVerifier struct { - ctx context.Context - client *github.Client -} - -// contains makes two "core" API calls: one for the default branch, and one to compare the target hash to a branch -// if the repo is "github/codeql-action", also check releases/v1 before failing. -func (g *githubVerifier) contains(owner, repo, hash string) (bool, error) { - defaultBranch, err := g.defaultBranch(owner, repo) - if err != nil { - return false, err - } - contains, err := g.branchContains(defaultBranch, owner, repo, hash) - if err != nil { - return false, err - } - if contains { - return true, nil - } - - switch { - // github/codeql-action has commits from their release branches that don't show up in the default branch - // this isn't the best approach for now, but theres no universal "does this commit belong to this repo" call - case owner == "github" && repo == "codeql-action": - releaseBranches := []string{"releases/v3", "releases/v2", "releases/v1"} - for _, branch := range releaseBranches { - contains, err = g.branchContains(branch, owner, repo, hash) - if err != nil { - return false, err - } - if contains { - return true, nil - } - } - - // add fallback lookup for actions/upload-artifact v3/node20 branch - // https://github.com/actions/starter-workflows/pull/2348#discussion_r1536228344 - case owner == "actions" && repo == "upload-artifact": - contains, err = g.branchContains("v3/node20", owner, repo, hash) - } - return contains, err -} - -func (g *githubVerifier) defaultBranch(owner, repo string) (string, error) { - githubRepository, _, err := g.client.Repositories.Get(g.ctx, owner, repo) - if err != nil { - return "", fmt.Errorf("fetching repository info: %w", err) - } - if githubRepository == nil || githubRepository.DefaultBranch == nil { - return "", errNoDefaultBranch - } - return *githubRepository.DefaultBranch, nil -} - -func (g *githubVerifier) branchContains(branch, owner, repo, hash string) (bool, error) { - opts := &github.ListOptions{PerPage: 1} - diff, resp, err := g.client.Repositories.CompareCommits(g.ctx, owner, repo, branch, hash, opts) - if err != nil { - if resp.StatusCode == http.StatusNotFound { - // NotFound can be returned for some divergent cases: "404 No common ancestor between ..." - return false, nil - } - return false, fmt.Errorf("error comparing revisions: %w", err) - } - - // Target should be behind or at the base ref if it is considered contained. - return diff.GetStatus() == "behind" || diff.GetStatus() == "identical", nil -} - func hasServices(j *actionlint.Job) bool { if j == nil { return false diff --git a/app/server/verify_workflow_test.go b/app/server/verify_workflow_test.go index e4f36dab..c77cb8ca 100644 --- a/app/server/verify_workflow_test.go +++ b/app/server/verify_workflow_test.go @@ -15,14 +15,12 @@ package server import ( - "context" "net/http" "os" "strings" "testing" "unicode/utf8" - "github.com/google/go-github/v65/github" "github.com/stretchr/testify/assert" ) @@ -115,59 +113,6 @@ func (s suffixStubTripper) RoundTrip(r *http.Request) (*http.Response, error) { }, nil } -func Test_githubVerifier_contains_codeql_v1(t *testing.T) { - t.Parallel() - httpClient := http.Client{ - Transport: suffixStubTripper{ - responsePaths: map[string]string{ - "codeql-action": "./testdata/api/github/repository.json", // api call which finds the default branch - "main...somehash": "./testdata/api/github/divergent.json", // doesnt belong to default branch - "v3...somehash": "./testdata/api/github/divergent.json", // doesnt belong to releases/v3 branch - "v2...somehash": "./testdata/api/github/divergent.json", // doesnt belong to releases/v2 branch - "v1...somehash": "./testdata/api/github/containsCommit.json", // belongs to releases/v1 branch - }, - }, - } - client := github.NewClient(&httpClient) - gv := githubVerifier{ - ctx: context.Background(), - client: client, - } - got, err := gv.contains("github", "codeql-action", "somehash") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != true { - t.Errorf("expected to contain hash, but it didnt") - } -} - -func Test_githubVerifier_contains_codeql_v2(t *testing.T) { - t.Parallel() - httpClient := http.Client{ - Transport: suffixStubTripper{ - responsePaths: map[string]string{ - "codeql-action": "./testdata/api/github/repository.json", // api call which finds the default branch - "main...somehash": "./testdata/api/github/divergent.json", // doesnt belong to default branch - "v3...somehash": "./testdata/api/github/divergent.json", // doesnt belong to releases/v3 branch either - "v2...somehash": "./testdata/api/github/containsCommit.json", // belongs to releases/v2 branch - }, - }, - } - client := github.NewClient(&httpClient) - gv := githubVerifier{ - ctx: context.Background(), - client: client, - } - got, err := gv.contains("github", "codeql-action", "somehash") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got != true { - t.Errorf("expected to contain hash, but it didnt") - } -} - func FuzzVerifyWorkflow(f *testing.F) { testfiles := []string{ "testdata/workflow-valid.yml",