Merge pull request #1204 from jhrozek/osv_summary

PR vulnerability evaluation: Display summary of vulnerabilities found

examples/github/rule-types/pr_vulnerability_check.yaml

@@ -27,6 +27,8 @@ def:
           - comment
           # the evaluator engine will merely pass on an error, marking the profile as failed if a vulnerability is found
           - profile_only
+          # the evaluator engine will add a single summary comment with a table listing the vulnerabilities found
+          - summary
         default: review
       ecosystem_config:
         type: array

internal/engine/eval/vulncheck/actions.go

@@ -29,6 +29,7 @@ type prStatusHandler interface {
 	trackVulnerableDep(
 		ctx context.Context,
 		dep *pb.PrDependencies_ContextualDependency,
+		vulnResp *VulnerabilityResponse,
 		patch patchLocatorFormatter,
 	) error
 	submit(ctx context.Context) error
@@ -49,6 +50,8 @@ func newPrStatusHandler(
 		return newReviewPrHandler(ctx, pr, client, withVulnsFoundReviewStatus(github.String("COMMENT")))
 	case actionProfileOnly:
 		return newProfileOnlyPrHandler(), nil
+	case actionSummary:
+		return newSummaryPrHandler(ctx, pr, client)
 	default:
 		return nil, fmt.Errorf("unknown action: %s", action)
 	}

internal/engine/eval/vulncheck/config.go

@@ -39,6 +39,7 @@ const (
 	actionComment      action = "comment"
 	actionCommitStatus action = "commit_status"
 	actionProfileOnly  action = "profile_only"
+	actionSummary      action = "summary"
 )
 
 type packageRepository struct {

internal/engine/eval/vulncheck/review.go

@@ -215,6 +215,7 @@ func newReviewPrHandler(
 func (ra *reviewPrHandler) trackVulnerableDep(
 	ctx context.Context,
 	dep *pb.PrDependencies_ContextualDependency,
+	_ *VulnerabilityResponse,
 	patch patchLocatorFormatter,
 ) error {
 	location, err := locateDepInPr(ctx, ra.cli, dep, patch)
@@ -414,13 +415,152 @@ func (csh *commitStatusPrHandler) setCommitStatus(
 	return err
 }
 
+// summaryPrHandler is a prStatusHandler that adds a summary text to the PR as a comment.
+type summaryPrHandler struct {
+	cli provifv1.GitHub
+	pr  *pb.PullRequest
+
+	logger      zerolog.Logger
+	trackedDeps []dependencyVulnerabilities
+	headerTmpl  *template.Template
+	rowsTmpl    *template.Template
+}
+
+const (
+	tableVulnerabilitiesHeaderName = "vulnerabilitiesTableHeader"
+	tableVulnerabilitiesHeader     = `### Summary of vulnerabilities found
+Mediator found the following vulnerabilities in this PR:
+<table>
+  <tr>
+    <th>Ecosystem</th>
+    <th>Name</th>
+    <th>Version</th>
+    <th>Vulnerability ID</th>
+    <th>Summary</th>
+    <th>Introduced</th>
+    <th>Fixed</th>
+  </tr>
+`
+	tableVulnerabilitiesRowsName = "vulnerabilitiesTableRow"
+	tableVulnerabilitiesRows     = `
+  {{ range .Vulnerabilities }}
+  <tr>
+    <td>{{ $.DependencyEcosystem }}</td>
+    <td>{{ $.DependencyName }}</td>
+    <td>{{ $.DependencyVersion }}</td>
+    <td>{{ .ID }}</td>
+    <td>{{ .Summary }}</td>
+    <td>{{ .Introduced }}</td>
+    <td>{{ .Fixed }}</td>
+  </tr>
+  {{ end }}
+`
+	tableVulnerabilitiesFooter = "</table>"
+)
+
+type dependencyVulnerabilities struct {
+	Dependency      *pb.Dependency
+	Vulnerabilities []Vulnerability
+}
+
+func (sph *summaryPrHandler) trackVulnerableDep(
+	_ context.Context,
+	dep *pb.PrDependencies_ContextualDependency,
+	vulnResp *VulnerabilityResponse,
+	_ patchLocatorFormatter,
+) error {
+	sph.trackedDeps = append(sph.trackedDeps, dependencyVulnerabilities{
+		Dependency:      dep.Dep,
+		Vulnerabilities: vulnResp.Vulns,
+	})
+	return nil
+}
+
+func (sph *summaryPrHandler) submit(ctx context.Context) error {
+	summary, err := sph.generateSummary()
+	if err != nil {
+		return fmt.Errorf("could not generate summary: %w", err)
+	}
+
+	err = sph.cli.CreateComment(ctx, sph.pr.GetRepoOwner(), sph.pr.GetRepoName(), int(sph.pr.GetNumber()), summary)
+	if err != nil {
+		return fmt.Errorf("could not create comment: %w", err)
+	}
+
+	return nil
+}
+
+func (sph *summaryPrHandler) generateSummary() (string, error) {
+	var summary strings.Builder
+
+	var headerBuf bytes.Buffer
+	if err := sph.headerTmpl.Execute(&headerBuf, nil); err != nil {
+		return "", fmt.Errorf("could not execute template: %w", err)
+	}
+	summary.WriteString(headerBuf.String())
+
+	for i := range sph.trackedDeps {
+		var rowBuf bytes.Buffer
+
+		if err := sph.rowsTmpl.Execute(&rowBuf, struct {
+			DependencyEcosystem string
+			DependencyName      string
+			DependencyVersion   string
+			Vulnerabilities     []Vulnerability
+		}{
+			DependencyEcosystem: pbEcosystemAsString(sph.trackedDeps[i].Dependency.Ecosystem),
+			DependencyName:      sph.trackedDeps[i].Dependency.Name,
+			DependencyVersion:   sph.trackedDeps[i].Dependency.Version,
+			Vulnerabilities:     sph.trackedDeps[i].Vulnerabilities,
+		}); err != nil {
+			return "", fmt.Errorf("could not execute template: %w", err)
+		}
+		summary.WriteString(rowBuf.String())
+	}
+	summary.WriteString(tableVulnerabilitiesFooter)
+
+	return summary.String(), nil
+}
+
+func newSummaryPrHandler(
+	ctx context.Context,
+	pr *pb.PullRequest,
+	cli provifv1.GitHub,
+) (prStatusHandler, error) {
+	logger := zerolog.Ctx(ctx).With().
+		Int32("pull-number", pr.Number).
+		Str("repo-owner", pr.RepoOwner).
+		Str("repo-name", pr.RepoName).
+		Logger()
+
+	headerTmpl, err := template.New(tableVulnerabilitiesHeaderName).Parse(tableVulnerabilitiesHeader)
+	if err != nil {
+		return nil, fmt.Errorf("could not parse dependency template: %w", err)
+	}
+	rowsTmpl, err := template.New(tableVulnerabilitiesRowsName).Parse(tableVulnerabilitiesRows)
+	if err != nil {
+		return nil, fmt.Errorf("could not parse vulnerability template: %w", err)
+	}
+
+	return &summaryPrHandler{
+		cli:         cli,
+		pr:          pr,
+		logger:      logger,
+		headerTmpl:  headerTmpl,
+		rowsTmpl:    rowsTmpl,
+		trackedDeps: make([]dependencyVulnerabilities, 0),
+	}, nil
+}
+
 // just satisfies the interface but really does nothing. Useful for testing.
 type profileOnlyPrHandler struct{}
 
 func (profileOnlyPrHandler) trackVulnerableDep(
 	_ context.Context,
 	_ *pb.PrDependencies_ContextualDependency,
-	_ patchLocatorFormatter) error {
+	_ *VulnerabilityResponse,
+	_ patchLocatorFormatter,
+) error {
 	return nil
 }
 

internal/engine/eval/vulncheck/review_test.go

@@ -137,7 +137,7 @@ func TestReviewPrHandlerVulnerabilitiesDifferentIdentities(t *testing.T) {
 		NewRequest("GET", server.URL, nil).
 		Return(http.NewRequest("GET", server.URL, nil))
 
-	err = handler.trackVulnerableDep(context.TODO(), dep, patchPackage)
+	err = handler.trackVulnerableDep(context.TODO(), dep, nil, patchPackage)
 	require.NoError(t, err)
 
 	expBody, err := createReviewBody(vulnsFoundText)
@@ -322,7 +322,7 @@ func TestCommitStatusPrHandlerWithVulnerabilities(t *testing.T) {
 		NewRequest("GET", server.URL, nil).
 		Return(http.NewRequest("GET", server.URL, nil))
 
-	err = handler.trackVulnerableDep(context.TODO(), dep, patchPackage)
+	err = handler.trackVulnerableDep(context.TODO(), dep, nil, patchPackage)
 	require.NoError(t, err)
 
 	expBody, err := createReviewBody(vulnsFoundText)

internal/engine/eval/vulncheck/vulncheck.go

@@ -108,7 +108,7 @@ func (e *Evaluator) Eval(ctx context.Context, pol map[string]any, res *engif.Res
 			return fmt.Errorf("failed to send package request: %w", err)
 		}
 
-		if err := prReplyHandler.trackVulnerableDep(ctx, dep, patch); err != nil {
+		if err := prReplyHandler.trackVulnerableDep(ctx, dep, response, patch); err != nil {
 			return fmt.Errorf("failed to add package patch for further processing: %w", err)
 		}
 	}

internal/providers/github/github_rest.go

@@ -682,3 +682,11 @@ func (c *RestClient) ListPullRequests(
 
 	return prs, nil
 }
+
+// CreateComment creates a comment on a pull request or an issue
+func (c *RestClient) CreateComment(ctx context.Context, owner, repo string, number int, comment string) error {
+	_, _, err := c.client.Issues.CreateComment(ctx, owner, repo, number, &github.IssueComment{
+		Body: &comment,
+	})
+	return err
+}

pkg/providers/v1/providers.go

@@ -113,6 +113,7 @@ type GitHub interface {
 	UpdateRef(ctx context.Context, owner, repo, ref, sha string, force bool) (*github.Reference, error)
 	CreatePullRequest(ctx context.Context, owner, repo, title, body, head, base string) (*github.PullRequest, error)
 	ListPullRequests(ctx context.Context, owner, repo string, opt *github.PullRequestListOptions) ([]*github.PullRequest, error)
+	CreateComment(ctx context.Context, owner, repo string, number int, comment string) error
 }
 
 // ParseAndValidate parses the given provider configuration and validates it.