🐛 fallback to releases/v1 branch for codeql-action. (#484)

Signed-off-by: Spencer Schrock <[email protected]>

app/server/testdata/api/github/containsCommit.json

@@ -0,0 +1,7 @@
+{
+  "url": "https://api.github.com/repos/github/codeql-action/compare/releases/v1...54d8b0da6bba52eaed1c8042082f6560bc02ecb6",
+  "status": "behind",
+  "ahead_by": 0,
+  "behind_by": 197,
+  "total_commits": 0
+}

app/server/testdata/api/github/divergent.json

@@ -0,0 +1,7 @@
+{
+  "url": "https://api.github.com/repos/github/codeql-action/compare/main...54d8b0da6bba52eaed1c8042082f6560bc02ecb6",
+  "status": "diverged",
+  "ahead_by": 193,
+  "behind_by": 1280,
+  "total_commits": 193
+}

app/server/testdata/api/github/repository.json

@@ -0,0 +1,8 @@
+{
+  "url": "https://api.github.com/repos/github/codeql-action",
+  "id": 259445878,
+  "node_id": "MDEwOlJlcG9zaXRvcnkyNTk0NDU4Nzg=",
+  "name": "codeql-action",
+  "full_name": "github/codeql-action",
+  "default_branch": "main"
+}

app/server/verify_workflow.go

@@ -249,25 +249,26 @@ type githubVerifier struct {
 	client *github.Client
 }
 
-// contains currently makes two "core" API calls: one for the default branch, and one to compare the target hash
-// This could also be done with the search commits API and a query of q=hash:<sha>+repo:<owner>/<repo>.
+// 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
 	}
-	opts := &github.ListOptions{PerPage: 1}
-	diff, resp, err := g.client.Repositories.CompareCommits(g.ctx, owner, repo, defaultBranch, hash, opts)
+	contains, err := g.branchContains(defaultBranch, owner, repo, hash)
 	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)
+		return false, err
 	}
-
-	// Target should be behind or at the base ref if it is considered contained.
-	return diff.GetStatus() == "behind" || diff.GetStatus() == "identical", nil
+	if contains {
+		return true, nil
+	}
+	// github/codeql-action has commits from their v1 release branch 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
+	if owner == "github" && repo == "codeql-action" {
+		contains, err = g.branchContains("releases/v1", owner, repo, hash)
+	}
+	return contains, err
 }
 
 func (g *githubVerifier) defaultBranch(owner, repo string) (string, error) {
@@ -280,3 +281,18 @@ func (g *githubVerifier) defaultBranch(owner, repo string) (string, error) {
 	}
 	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
+}

app/server/verify_workflow_test.go

@@ -15,10 +15,14 @@
 package server
 
 import (
+	"context"
+	"net/http"
 	"os"
+	"strings"
 	"testing"
 	"unicode/utf8"
 
+	"github.com/google/go-github/v42/github"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -89,6 +93,53 @@ func TestVerifyInvalidWorkflows(t *testing.T) {
 	}
 }
 
+// suffix may not be the best term, but maps the final part of a path to a response file.
+// this is helpful when multiple API calls need to be made.
+// e.g. a call to /foo/bar/some/endpoint would have "endpoint" as a suffix.
+type suffixStubTripper struct {
+	// key is suffix, value is response file.
+	responsePaths map[string]string
+}
+
+func (s suffixStubTripper) RoundTrip(r *http.Request) (*http.Response, error) {
+	pathParts := strings.Split(r.URL.Path, "/")
+	suffix := pathParts[len(pathParts)-1]
+	f, err := os.Open(s.responsePaths[suffix])
+	if err != nil {
+		return nil, err
+	}
+	return &http.Response{
+		Status:     "200 OK",
+		StatusCode: http.StatusOK,
+		Body:       f,
+	}, nil
+}
+
+func Test_githubVerifier_contains(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
+				"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 FuzzVerifyWorkflow(f *testing.F) {
 	testfiles := []string{
 		"testdata/workflow-valid.yml",