Trusty: Support blocking PRs through reviews

This commit adds to minder the capability to request changes in PRs when minder finds something odd using trusty data. We now also introduce a new setting to disable blocking on malicious deps (all power to the users, but why would you want that?!). Signed-off-by: Adolfo García Veytia (puerco) <[email protected]>
mindersec · May 22, 2024 · ad8d582 · ad8d582
1 parent b4ef6ec
commit ad8d582
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 12 deletions.
diff --git a/internal/engine/eval/trusty/actions.go b/internal/engine/eval/trusty/actions.go
@@ -26,7 +26,10 @@ import (
 	template "text/template"
 	"unicode"
 
+	"github.com/google/go-github/v61/github"
+
 	"github.com/stacklok/minder/internal/constants"
+	"github.com/stacklok/minder/internal/engine/eval/pr_actions"
 	pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
 	provifv1 "github.com/stacklok/minder/pkg/providers/v1"
 )
@@ -166,24 +169,65 @@ func (sph *summaryPrHandler) trackAlternatives(
 	})
 }
 
-func (sph *summaryPrHandler) submit(ctx context.Context) error {
+func (sph *summaryPrHandler) submit(ctx context.Context, ruleConfig *config) error {
 	if len(sph.trackedAlternatives) == 0 {
 		return nil
 	}
+	var reviewAction string = "COMMENT"
+	var blockOnMalicious bool
+	if v, ok := ruleConfig.Block["malicious"]; ok {
+		blockOnMalicious = v
+	}
 
 	summary, err := sph.generateSummary()
 	if err != nil {
 		return fmt.Errorf("could not generate summary: %w", err)
 	}
 
-	_, err = sph.cli.CreateIssueComment(ctx, sph.pr.GetRepoOwner(), sph.pr.GetRepoName(), int(sph.pr.GetNumber()), summary)
-	if err != nil {
-		return fmt.Errorf("could not create comment: %w", err)
-	}
+	action := ruleConfig.Action
 
+	switch action {
+	case pr_actions.ActionReviewPr:
+		// If we don't block on malicious dependencies, switch the review action to
+		// COMMENT only.
+		if sph.hasMaliciousDeps() && blockOnMalicious {
+			reviewAction = "REQUEST_CHANGES"
+		}
+
+		_, err = sph.cli.CreateReview(
+			ctx, sph.pr.GetRepoOwner(), sph.pr.GetRepoName(), int(sph.pr.GetNumber()),
+			&github.PullRequestReviewRequest{
+				NodeID:   new(string),
+				CommitID: &sph.pr.CommitSha,
+				Body:     &summary,
+				Event:    github.String(reviewAction),
+				Comments: []*github.DraftReviewComment{},
+			},
+		)
+		if err != nil {
+			return fmt.Errorf("submitting pr summary: %w", err)
+		}
+	case pr_actions.ActionSummary:
+		_, err = sph.cli.CreateIssueComment(ctx, sph.pr.GetRepoOwner(), sph.pr.GetRepoName(), int(sph.pr.GetNumber()), summary)
+		if err != nil {
+			return fmt.Errorf("could not create comment: %w", err)
+		}
+	case pr_actions.ActionComment, pr_actions.ActionCommitStatus, pr_actions.ActionProfileOnly:
+		return fmt.Errorf("pull request action not supported")
+	}
 	return nil
 }
 
+// hasMaliciousDeps eturns true is a pull request is introducing malicious dependencies
+func (sph *summaryPrHandler) hasMaliciousDeps() bool {
+	for _, alternative := range sph.trackedAlternatives {
+		if slices.Contains(alternative.Reasons, TRUSTY_MALICIOUS_PKG) {
+			return true
+		}
+	}
+	return false
+}
+
 func (sph *summaryPrHandler) generateSummary() (string, error) {
 	var malicious = []maliciousTemplateData{}
 	var lowScorePackages = map[string]templatePackage{}

diff --git a/internal/engine/eval/trusty/config.go b/internal/engine/eval/trusty/config.go
@@ -49,15 +49,19 @@ type ecosystemConfig struct {
 	Activity float64 `json:"activity" mapstructure:"activity"`
 }
 
-// config is the configuration for the vulncheck evaluator
+// config is the configuration for the trusty evaluator
 type config struct {
 	Action          pr_actions.Action `json:"action" mapstructure:"action" validate:"required"`
+	Block           map[string]bool   `json:"block" mapstructure:"block"`
 	EcosystemConfig []ecosystemConfig `json:"ecosystem_config" mapstructure:"ecosystem_config" validate:"required"`
 }
 
 func defaultConfig() *config {
 	return &config{
-		Action: pr_actions.ActionSummary,
+		Action: pr_actions.ActionReviewPr,
+		Block: map[string]bool{
+			"malicious": true,
+		},
 		EcosystemConfig: []ecosystemConfig{
 			{
 				Name:       "npm",

diff --git a/internal/engine/eval/trusty/trusty.go b/internal/engine/eval/trusty/trusty.go
@@ -121,8 +121,8 @@ func (e *Evaluator) Eval(ctx context.Context, pol map[string]any, res *engif.Res
 		return nil
 	}
 
-	if err := submitSummary(ctx, prSummaryHandler); err != nil {
-		logger.Err(err).Msgf("Failed Generating PR Summary: %s", err.Error())
+	if err := submitSummary(ctx, prSummaryHandler, ruleConfig); err != nil {
+		logger.Err(err).Msgf("Failed generating PR summary: %s", err.Error())
 		return fmt.Errorf("submitting pull request summary: %w", err)
 	}
 
@@ -160,7 +160,7 @@ func parseRuleConfig(pol map[string]any) (*config, error) {
 		return nil, fmt.Errorf("failed to parse config: %w", err)
 	}
 
-	if ruleConfig.Action != pr_actions.ActionSummary {
+	if ruleConfig.Action != pr_actions.ActionSummary && ruleConfig.Action != pr_actions.ActionReviewPr {
 		return nil, fmt.Errorf("action %s is not implemented", ruleConfig.Action)
 	}
 
@@ -169,8 +169,8 @@ func parseRuleConfig(pol map[string]any) (*config, error) {
 
 // submitSummary submits the pull request summary. It will return an error if
 // something fails.
-func submitSummary(ctx context.Context, prSummary *summaryPrHandler) error {
-	if err := prSummary.submit(ctx); err != nil {
+func submitSummary(ctx context.Context, prSummary *summaryPrHandler, ruleConfig *config) error {
+	if err := prSummary.submit(ctx, ruleConfig); err != nil {
 		return fmt.Errorf("failed to submit summary: %w", err)
 	}
 	return nil