Fix panic on successful policy validation where err == nil. (#488)

Also adds a test.

go.mod

@@ -13,6 +13,7 @@ require (
 	github.com/cloudevents/sdk-go/v2 v2.15.2
 	github.com/coreos/go-oidc/v3 v3.11.0
 	github.com/golang-jwt/jwt/v4 v4.5.0
+	github.com/google/go-cmp v0.6.0
 	github.com/google/go-github/v61 v61.0.0
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hashicorp/golang-lru/v2 v2.0.7

pkg/webhook/testdata/app/installations/1111/access_tokens

@@ -0,0 +1,71 @@
+{
+  "id": 1,
+  "url": "https://api.github.com/authorizations/1",
+  "scopes": [],
+  "token": "ghu_16C7e42F292c6912E7710c838347Ae178B4a",
+  "token_last_eight": "Ae178B4a",
+  "hashed_token": "25f94a2a5c7fbaf499c665bc73d67c1c87e496da8985131633ee0a95819db2e8",
+  "app": {
+    "url": "http://my-github-app.com",
+    "name": "my github app",
+    "client_id": "Iv1.8a61f9b3a7aba766"
+  },
+  "note": "optional note",
+  "note_url": "http://optional/note/url",
+  "updated_at": "2011-09-06T20:39:23Z",
+  "created_at": "2011-09-06T17:26:27Z",
+  "fingerprint": "jklmnop12345678",
+  "expires_at": "2011-09-08T17:26:27Z",
+  "user": {
+    "login": "octocat",
+    "id": 1,
+    "node_id": "MDQ6VXNlcjE=",
+    "avatar_url": "https://github.com/images/error/octocat_happy.gif",
+    "gravatar_id": "",
+    "url": "https://api.github.com/users/octocat",
+    "html_url": "https://github.com/octocat",
+    "followers_url": "https://api.github.com/users/octocat/followers",
+    "following_url": "https://api.github.com/users/octocat/following{/other_user}",
+    "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
+    "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
+    "subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
+    "organizations_url": "https://api.github.com/users/octocat/orgs",
+    "repos_url": "https://api.github.com/users/octocat/repos",
+    "events_url": "https://api.github.com/users/octocat/events{/privacy}",
+    "received_events_url": "https://api.github.com/users/octocat/received_events",
+    "type": "User",
+    "site_admin": false
+  },
+  "installation": {
+    "permissions": {
+      "metadata": "read",
+      "issues": "write",
+      "contents": "read"
+    },
+    "repository_selection": "selected",
+    "single_file_name": ".github/workflow.yml",
+    "repositories_url": "https://api.github.com/user/repos",
+    "account": {
+      "login": "octocat",
+      "id": 1,
+      "node_id": "MDQ6VXNlcjE=",
+      "avatar_url": "https://github.com/images/error/octocat_happy.gif",
+      "gravatar_id": "",
+      "url": "https://api.github.com/users/octocat",
+      "html_url": "https://github.com/octocat",
+      "followers_url": "https://api.github.com/users/octocat/followers",
+      "following_url": "https://api.github.com/users/octocat/following{/other_user}",
+      "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
+      "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
+      "subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
+      "organizations_url": "https://api.github.com/users/octocat/orgs",
+      "repos_url": "https://api.github.com/users/octocat/repos",
+      "events_url": "https://api.github.com/users/octocat/events{/privacy}",
+      "received_events_url": "https://api.github.com/users/octocat/received_events",
+      "type": "User",
+      "site_admin": false
+    },
+    "has_multiple_single_files": false,
+    "single_file_paths": []
+  }
+}

pkg/webhook/webhook.go

@@ -136,13 +136,14 @@ func (e *Validator) handleSHA(ctx context.Context, client *github.Client, owner,
 	// Whether or not the commit is verified, we still create a CheckRun.
 	// The only difference is whether it shows up to the user as success or
 	// failure.
-	var conclusion, title string
+	var conclusion, title, summary string
 	if err == nil {
 		conclusion = "success"
 		title = "Valid trust policy."
 	} else {
 		conclusion = "failure"
 		title = "Invalid trust policy."
+		summary = "Failed to validate trust policy.\n\n" + err.Error()
 	}
 
 	opts := github.CreateCheckRunOptions{
@@ -155,7 +156,7 @@ func (e *Validator) handleSHA(ctx context.Context, client *github.Client, owner,
 		CompletedAt: &github.Timestamp{Time: time.Now()},
 		Output: &github.CheckRunOutput{
 			Title:   github.String(title),
-			Summary: github.String(err.Error()),
+			Summary: github.String(summary),
 		},
 	}
 
@@ -229,13 +230,21 @@ func (e *Validator) handlePush(ctx context.Context, event *github.PushEvent) (*g
 	client := github.NewClient(&http.Client{
 		Transport: ghinstallation.NewFromAppsTransport(e.Transport, installationID),
 	})
+	if e.Transport.BaseURL != "" {
+		var err error
+		client, err = client.WithEnterpriseURLs(e.Transport.BaseURL, e.Transport.BaseURL)
+		if err != nil {
+			return nil, err
+		}
+	}
 
 	// Check diff
 	// TODO: Pagination?
 	resp, _, err := client.Repositories.CompareCommits(ctx, owner, repo, event.GetBefore(), sha, &github.ListOptions{})
 	if err != nil {
 		return nil, err
 	}
+	log.Infof("%+v\n%+v", resp, resp.Files)
 	var files []string
 	for _, file := range resp.Files {
 		if ok, err := filepath.Match(".github/chainguard/*.sts.yaml", file.GetFilename()); err == nil && ok {
@@ -274,6 +283,13 @@ func (e *Validator) handlePullRequest(ctx context.Context, pr *github.PullReques
 	client := github.NewClient(&http.Client{
 		Transport: ghinstallation.NewFromAppsTransport(e.Transport, installationID),
 	})
+	if e.Transport.BaseURL != "" {
+		var err error
+		client, err = client.WithEnterpriseURLs(e.Transport.BaseURL, e.Transport.BaseURL)
+		if err != nil {
+			return nil, err
+		}
+	}
 
 	// Check diff
 	var files []string
@@ -327,6 +343,13 @@ func (e *Validator) handleCheckSuite(ctx context.Context, cs checkSuite) (*githu
 	client := github.NewClient(&http.Client{
 		Transport: ghinstallation.NewFromAppsTransport(e.Transport, installationID),
 	})
+	if e.Transport.BaseURL != "" {
+		var err error
+		client, err = client.WithEnterpriseURLs(e.Transport.BaseURL, e.Transport.BaseURL)
+		if err != nil {
+			return nil, err
+		}
+	}
 
 	var files []string
 	if cs.GetCheckSuite().GetBeforeSHA() == zeroHash {

pkg/webhook/webhook_test.go

@@ -21,7 +21,9 @@ import (
 	"testing"
 
 	"github.com/bradleyfalzon/ghinstallation/v2"
+	"github.com/chainguard-dev/clog"
 	"github.com/chainguard-dev/clog/slogtest"
+	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-github/v61/github"
 )
 
@@ -47,7 +49,7 @@ func TestValidatePolicy(t *testing.T) {
 		t.Fatal(err)
 	}
 	ctx := slogtest.TestContextWithLogger(t)
-	if err := validatePolicies(ctx, gh, "foo", "bar", "deadbeef", []string{"policy.json"}); err != nil {
+	if err := validatePolicies(ctx, gh, "foo", "bar", "deadbeef", []string{".github/chainguard/test.sts.yaml"}); err != nil {
 		t.Fatal(err)
 	}
 }
@@ -74,7 +76,6 @@ func TestOrgFilter(t *testing.T) {
 		WebhookSecret: [][]byte{secret},
 		Organizations: []string{"foo"},
 	}
-
 	srv := httptest.NewServer(v)
 	defer srv.Close()
 
@@ -127,3 +128,109 @@ func signature(secret, body []byte) string {
 
 	return fmt.Sprintf("sha256=%s", hex.EncodeToString(b))
 }
+
+func TestWebhookOK(t *testing.T) {
+	// CheckRuns will be collected here.
+	got := []*github.CreateCheckRunOptions{}
+
+	mux := http.NewServeMux()
+	mux.HandleFunc("POST /api/v3/repos/foo/bar/check-runs", func(w http.ResponseWriter, r *http.Request) {
+		opt := new(github.CreateCheckRunOptions)
+		if err := json.NewDecoder(r.Body).Decode(opt); err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+		got = append(got, opt)
+	})
+	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		path := filepath.Join("testdata", r.URL.Path)
+		f, err := os.Open(path)
+		if err != nil {
+			clog.FromContext(r.Context()).Errorf("%s not found", path)
+			http.Error(w, err.Error(), http.StatusNotFound)
+			return
+		}
+		defer f.Close()
+		if _, err := io.Copy(w, f); err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	})
+	gh := httptest.NewServer(mux)
+	defer gh.Close()
+
+	key, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		t.Fatal(err)
+	}
+	tr := ghinstallation.NewAppsTransportFromPrivateKey(gh.Client().Transport, 1234, key)
+	if err != nil {
+		t.Fatal(err)
+	}
+	tr.BaseURL = gh.URL
+
+	secret := []byte("hunter2")
+	v := &Validator{
+		Transport:     tr,
+		WebhookSecret: [][]byte{secret},
+	}
+	srv := httptest.NewServer(v)
+	defer srv.Close()
+
+	body, err := json.Marshal(github.PushEvent{
+		Installation: &github.Installation{
+			ID: github.Int64(1111),
+		},
+		Organization: &github.Organization{
+			Login: github.String("foo"),
+		},
+		Repo: &github.PushEventRepository{
+			Owner: &github.User{
+				Login: github.String("foo"),
+			},
+			Name: github.String("bar"),
+		},
+		Before: github.String("1234"),
+		After:  github.String("5678"),
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	req, err := http.NewRequest(http.MethodPost, srv.URL, bytes.NewBuffer(body))
+	if err != nil {
+		t.Fatal(err)
+	}
+	req.Header.Set("X-Hub-Signature", signature(secret, body))
+	req.Header.Set("X-GitHub-Event", "push")
+	req.Header.Set("Content-Type", "application/json")
+	resp, err := srv.Client().Do(req.WithContext(slogtest.TestContextWithLogger(t)))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp.StatusCode != 200 {
+		out, _ := httputil.DumpResponse(resp, true)
+		t.Fatalf("expected %d, got\n%s", 200, string(out))
+	}
+
+	if len(got) != 1 {
+		t.Fatalf("expected 1 check run, got %d", len(got))
+	}
+
+	want := []*github.CreateCheckRunOptions{{
+		Name:       "Trust Policy Validation",
+		HeadSHA:    "5678",
+		ExternalID: github.String("5678"),
+		Status:     github.String("completed"),
+		Conclusion: github.String("success"),
+		// Use time from the response to ignore it.
+		StartedAt:   &github.Timestamp{Time: got[0].StartedAt.Time},
+		CompletedAt: &github.Timestamp{Time: got[0].CompletedAt.Time},
+		Output: &github.CheckRunOutput{
+			Title:   github.String("Valid trust policy."),
+			Summary: github.String(""),
+		},
+	}}
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Fatalf("unexpected check run (-want +got):\n%s", diff)
+	}
+}