Extend PR vulnerability checks with a configurable action to set commit status

Makefile

@@ -144,3 +144,4 @@ dbschema:	## generate database schema with schema spy, monitor file until doc is
 
 mock:
 	mockgen -package mockdb -destination database/mock/store.go github.com/stacklok/mediator/internal/db Store
+	mockgen -package mockgh -destination internal/providers/github/mock/github.go -source internal/providers/github/github.go RestAPI

examples/github/policies/policy.yaml

@@ -67,7 +67,7 @@ artifact:
 pull_request:
   - type: pr_vulnerability_check
     def:
-      action: reject_pr
+      action: review
       ecosystem_config:
         - name: npm
           vulnerability_database_type: osv

examples/github/rule-types/pr_vulnerability_check.yaml

@@ -17,15 +17,18 @@ def:
     properties:
       action:
         type: string
-        description: "The action to take if a vulnerability is found. Can be `fail` or `comment`."
+        description: "The action to take if a vulnerability is found."
         enum:
           # mediator will review the PR, suggest changes and mark the PR as changes requested if a vulnerability is found
-          - reject_pr
-          # mediator will comment on the PR if a vulnerability is found
+          - review
+          # mediator will comment and suggest changes on the PR if a vulnerability is found. Additionally, mediator
+          # will set the commit_status of the PR HEAD to failed to prevent the commit from being merged
+          - commit_status
+          # mediator will comment and suggest changes on the PR if a vulnerability is found, but not request changes
           - comment
           # the evaluator engine will merely pass on an error, marking the policy as failed if a vulnerability is found
           - policy_only
-        default: fail
+        default: review
       ecosystem_config:
         type: array
         description: "The configuration for the ecosystems to check."

internal/engine/eval/vulncheck/actions.go

@@ -19,6 +19,8 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/google/go-github/v53/github"
+
 	ghclient "github.com/stacklok/mediator/internal/providers/github"
 	pb "github.com/stacklok/mediator/pkg/generated/protobuf/go/mediator/v1"
 )
@@ -39,10 +41,14 @@ func newPrStatusHandler(
 	client ghclient.RestAPI,
 ) (prStatusHandler, error) {
 	switch action {
-	case actionRejectPr:
+	case actionReviewPr:
 		return newReviewPrHandler(ctx, pr, client)
-	case actionComment, actionPolicyOnly:
-		return nil, fmt.Errorf("action %s not implemented", action)
+	case actionCommitStatus:
+		return newCommitStatusPrHandler(ctx, pr, client)
+	case actionComment:
+		return newReviewPrHandler(ctx, pr, client, withVulnsFoundReviewStatus(github.String("COMMENT")))
+	case actionPolicyOnly:
+		return newPolicyOnlyPrHandler(), nil
 	default:
 		return nil, fmt.Errorf("unknown action: %s", action)
 	}

internal/engine/eval/vulncheck/config.go

@@ -34,9 +34,10 @@ const (
 type action string
 
 const (
-	actionRejectPr   action = "reject_pr"
-	actionComment    action = "comment"
-	actionPolicyOnly action = "policy_only"
+	actionReviewPr     action = "reject_pr"
+	actionComment      action = "comment"
+	actionCommitStatus action = "commit_status"
+	actionPolicyOnly   action = "policy_only"
 )
 
 type packageRepository struct {

internal/engine/eval/vulncheck/review.go

@@ -32,13 +32,17 @@ import (
 )
 
 const (
-	reviewBodyMagicComment              = "<!-- mediator: pr-review-body -->"
-	reviewBodyRequestChangesCommentText = `
+	reviewBodyMagicComment = "<!-- mediator: pr-review-body -->"
+	commitStatusContext    = "mediator.stacklok.dev/pr-vulncheck"
+	vulnsFoundText         = `
 Mediator found vulnerable dependencies in this PR. Either push an updated
 version or accept the proposed changes. Note that accepting the changes will
 include mediator as a co-author of this PR.
 `
-	reviewBodyCommentText = `
+	vulnsFoundTextShort = `
+Vulnerable dependencies found.
+`
+	noVulsFoundText = `
 Mediator analyzed this PR and found no vulnerable dependencies.
 `
 	reviewBodyDismissCommentText = `
@@ -56,7 +60,7 @@ type reviewTemplateData struct {
 	ReviewText   string
 }
 
-func createReviewBody(magicComment, reviewText string) (string, error) {
+func createReviewBody(reviewText string) (string, error) {
 	// Create and parse the template
 	tmpl, err := template.New(reviewTemplateName).Parse(reviewTmplStr)
 	if err != nil {
@@ -65,7 +69,7 @@ func createReviewBody(magicComment, reviewText string) (string, error) {
 
 	// Define the data for the template
 	data := reviewTemplateData{
-		MagicComment: magicComment,
+		MagicComment: reviewBodyMagicComment,
 		ReviewText:   reviewText,
 	}
 
@@ -105,7 +109,7 @@ func locateDepInPr(
 	}
 	// TODO:(jakub) I couldn't make this work with the GH client
 	netClient := &http.Client{}
-	resp, _ := netClient.Do(req)
+	resp, err := netClient.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("could not send request: %w", err)
 	}
@@ -129,6 +133,10 @@ func locateDepInPr(
 	return &loc, nil
 }
 
+func reviewBodyWithSuggestion(comment string) string {
+	return fmt.Sprintf("```suggestion\n%s\n```\n", comment)
+}
+
 type reviewPrHandler struct {
 	cli ghclient.RestAPI
 	pr  *pb.PullRequest
@@ -143,11 +151,21 @@ type reviewPrHandler struct {
 	logger zerolog.Logger
 }
 
+type reviewPrHandlerOption func(*reviewPrHandler)
+
+// WithSetReviewStatus is an option to set the vulnsFoundReviewStatus field of reviewPrHandler.
+func withVulnsFoundReviewStatus(status *string) reviewPrHandlerOption {
+	return func(r *reviewPrHandler) {
+		r.failStatus = status
+	}
+}
+
 func newReviewPrHandler(
 	ctx context.Context,
 	pr *pb.PullRequest,
 	cli ghclient.RestAPI,
-) (prStatusHandler, error) {
+	opts ...reviewPrHandlerOption,
+) (*reviewPrHandler, error) {
 	if pr == nil {
 		return nil, fmt.Errorf("pr was nil, can't review")
 	}
@@ -174,13 +192,19 @@ func newReviewPrHandler(
 		logger.Debug().Msg("author is different than the authenticated user, can request changes")
 	}
 
-	return &reviewPrHandler{
+	handler := &reviewPrHandler{
 		cli:        cli,
 		pr:         pr,
 		comments:   []*github.DraftReviewComment{},
 		logger:     logger,
 		failStatus: failStatus,
-	}, nil
+	}
+
+	for _, opt := range opts {
+		opt(handler)
+	}
+
+	return handler, nil
 }
 
 func (ra *reviewPrHandler) trackVulnerableDep(
@@ -194,7 +218,7 @@ func (ra *reviewPrHandler) trackVulnerableDep(
 	}
 
 	comment := patch.IndentedString(location.leadingWhitespace)
-	body := fmt.Sprintf("```suggestion\n"+"%s\n"+"```\n", comment)
+	body := reviewBodyWithSuggestion(comment)
 
 	reviewComment := &github.DraftReviewComment{
 		Path:      github.String(dep.File.Name),
@@ -205,6 +229,10 @@ func (ra *reviewPrHandler) trackVulnerableDep(
 	}
 	ra.comments = append(ra.comments, reviewComment)
 
+	ra.logger.Debug().
+		Str("dep-name", dep.Dep.Name).
+		Msg("vulnerable dependency found")
+
 	return nil
 }
 
@@ -237,13 +265,17 @@ func (ra *reviewPrHandler) submit(ctx context.Context) error {
 func (ra *reviewPrHandler) setStatus() {
 	if len(ra.comments) > 0 {
 		// if this pass produced comments, request changes
+		ra.text = github.String(vulnsFoundText)
 		ra.status = ra.failStatus
-		ra.text = github.String(reviewBodyRequestChangesCommentText)
+		ra.logger.Debug().Msg("vulnerabilities found")
 	} else {
 		// if this pass produced no comments, resolve the mediator review
 		ra.status = github.String("COMMENT")
-		ra.text = github.String(reviewBodyCommentText)
+		ra.text = github.String(noVulsFoundText)
+		ra.logger.Debug().Msg("no vulnerabilities found")
 	}
+
+	ra.logger.Debug().Str("status", *ra.status).Msg("will set review status")
 }
 
 func (ra *reviewPrHandler) findPreviousReview(ctx context.Context) error {
@@ -264,7 +296,7 @@ func (ra *reviewPrHandler) findPreviousReview(ctx context.Context) error {
 }
 
 func (ra *reviewPrHandler) submitReview(ctx context.Context) error {
-	body, err := createReviewBody(reviewBodyMagicComment, *ra.text)
+	body, err := createReviewBody(*ra.text)
 	if err != nil {
 		return fmt.Errorf("could not create review body: %w", err)
 	}
@@ -311,3 +343,85 @@ func (ra *reviewPrHandler) dismissReview(ctx context.Context) error {
 	}
 	return nil
 }
+
+type commitStatusPrHandler struct {
+	// embed the reviewPrHandler to automatically satisfy the prStatusHandler interface
+	reviewPrHandler
+}
+
+func newCommitStatusPrHandler(
+	ctx context.Context,
+	pr *pb.PullRequest,
+	client ghclient.RestAPI,
+) (prStatusHandler, error) {
+	// create a reviewPrHandler and embed it in the commitStatusPrHandler
+	rph, err := newReviewPrHandler(
+		ctx,
+		pr,
+		client,
+		withVulnsFoundReviewStatus(github.String("COMMENT")),
+	)
+	if err != nil {
+		return nil, fmt.Errorf("could not create review handler: %w", err)
+	}
+
+	return &commitStatusPrHandler{
+		reviewPrHandler: *rph,
+	}, nil
+}
+
+func (csh *commitStatusPrHandler) submit(ctx context.Context) error {
+	// first submit the review, we force the status to be COMMENT to not block
+	if err := csh.reviewPrHandler.submit(ctx); err != nil {
+		return fmt.Errorf("could not submit review: %w", err)
+	}
+
+	// next either pass or fail the commit status to eventually block the PR
+	if err := csh.setCommitStatus(ctx); err != nil {
+		return fmt.Errorf("could not set commit status: %w", err)
+	}
+
+	return nil
+}
+
+func (csh *commitStatusPrHandler) setCommitStatus(
+	ctx context.Context,
+) error {
+	commitStatus := &github.RepoStatus{
+		Context: github.String(commitStatusContext),
+	}
+
+	if len(csh.comments) > 0 {
+		commitStatus.State = github.String("failure")
+		commitStatus.Description = github.String(vulnsFoundTextShort)
+	} else {
+		commitStatus.State = github.String("success")
+		commitStatus.Description = github.String(noVulsFoundText)
+	}
+
+	csh.logger.Debug().
+		Str("commit-status", commitStatus.String()).
+		Str("commit-sha", csh.pr.CommitSha).
+		Msg("setting commit status")
+
+	_, err := csh.cli.SetCommitStatus(ctx, csh.pr.RepoOwner, csh.pr.RepoName, csh.pr.CommitSha, commitStatus)
+	return err
+}
+
+// just satisfies the interface but really does nothing. Useful for testing.
+type policyOnlyPrHandler struct{}
+
+func (policyOnlyPrHandler) trackVulnerableDep(
+	_ context.Context,
+	_ *pb.PrDependencies_ContextualDependency,
+	_ patchFormatter) error {
+	return nil
+}
+
+func (policyOnlyPrHandler) submit(_ context.Context) error {
+	return nil
+}
+
+func newPolicyOnlyPrHandler() prStatusHandler {
+	return &policyOnlyPrHandler{}
+}