Merge pull request #37 from cultureamp/fix-merge

fix: result merging for multi-plat images needs to take PackageName and PackageVersion into account

.envrc

@@ -0,0 +1,18 @@
+#!/bin/sh
+
+#
+# Sensible default environment values for use with direnv.
+#
+# BUILDKITE_PLUGIN_ECR_SCAN_RESULTS_IMAGE_NAME is entirely environment dependent though, and is required:
+# `cp .envrc.example .envrc.private` to get a set of values that you can override.
+#
+
+# BUILDKITE_PLUGIN_ECR_SCAN_RESULTS_IMAGE_NAME is only specified in .envc.private
+
+export BUILDKITE_PLUGIN_ECR_SCAN_RESULTS_MAX_CRITICALS=0
+export BUILDKITE_PLUGIN_ECR_SCAN_RESULTS_MAX_HIGHS=0
+
+export ECRSCANRESULTS_DISABLE_AGENT=true
+
+# load local values if specified
+source_env_if_exists .envrc.private

.envrc.example

@@ -0,0 +1,22 @@
+#!/bin/sh
+
+#
+# REQUIRED: set the image that is to be inspected.
+#
+
+# export BUILDKITE_PLUGIN_ECR_SCAN_RESULTS_IMAGE_NAME="[account-id].dkr.ecr.[region-id].amazonaws.com/[repo-name]:[label]"
+
+
+#
+# set thresholds as desired
+#
+
+# export BUILDKITE_PLUGIN_ECR_SCAN_RESULTS_MAX_CRITICALS=0
+# export BUILDKITE_PLUGIN_ECR_SCAN_RESULTS_MAX_HIGHS=0
+
+
+#
+# set display name if desired: this variable is basically obsolete though.
+#
+
+# export BUILDKITE_PLUGIN_ECR_SCAN_RESULTS_IMAGE_LABEL=latest

.gitignore

@@ -1,5 +1,7 @@
-.envrc
+.envrc.private
+
 # VIM swap files
 *.swp
+
 src/ecrscanresults
 src/*.html

src/finding/summary.go

@@ -24,6 +24,8 @@ const (
 	StatusAllPlatformsFailed
 )
 
+// Detail represents a vulnerability finding detail. Details are unique
+// according to their Name, PackageName and PackageVersion attributes.
 type Detail struct {
 	// The name associated with the finding, usually a CVE number.
 	Name        string
@@ -236,7 +238,7 @@ func mergeSingle(merged, other Summary) Summary {
 
 func mergeDetails(summary Summary, merged, other []Detail) []Detail {
 	for _, d := range other {
-		insertIdx, found := slices.BinarySearchFunc(merged, d, findingByID)
+		insertIdx, found := slices.BinarySearchFunc(merged, d, findingByKey)
 
 		if found {
 			// already exists, update the platform set for the current finding
@@ -399,6 +401,14 @@ func convertScore(s string) *decimal.Decimal {
 	return &d
 }
 
-func findingByID(a Detail, b Detail) int {
-	return strings.Compare(a.Name, b.Name)
+func findingByKey(a Detail, b Detail) int {
+	if n := strings.Compare(a.Name, b.Name); n != 0 {
+		return n
+	}
+
+	if n := strings.Compare(a.PackageName, b.PackageName); n != 0 {
+		return n
+	}
+
+	return strings.Compare(a.PackageVersion, b.PackageVersion)
 }

src/finding/summary_test.go

@@ -109,6 +109,7 @@ func TestSummarize(t *testing.T) {
 }
 
 func TestMergeSummary(t *testing.T) {
+	// details match on ID, PackageName and PackageValue
 	others := []finding.Summary{
 		{
 			Platforms: p("base"),
@@ -122,9 +123,11 @@ func TestMergeSummary(t *testing.T) {
 					Platforms: p("base"),
 				},
 				{
-					Name:      "CVE-a",
-					Severity:  "HIGH",
-					Platforms: p("base"),
+					Name:           "CVE-a",
+					Severity:       "HIGH",
+					Platforms:      p("base"),
+					PackageName:    "cvea-pkg",
+					PackageVersion: "1.0.0",
 				},
 			},
 		},
@@ -139,10 +142,35 @@ func TestMergeSummary(t *testing.T) {
 					Severity:  "HIGH",
 					Platforms: p("other1"),
 				},
+				{
+					Name:           "CVE-a",
+					Severity:       "HIGH",
+					Platforms:      p("other1"),
+					PackageName:    "cvea-pkg",
+					PackageVersion: "1.0.0",
+				},
 				{
 					Name:      "CVE-a",
 					Severity:  "HIGH",
 					Platforms: p("other1"),
+					// varying by package name is a separate finding
+					PackageName:    "cvea-pkg-2",
+					PackageVersion: "1.0.0",
+				},
+				{
+					Name:           "CVE-a",
+					Severity:       "HIGH",
+					Platforms:      p("other1"),
+					PackageName:    "cvea-pkg-3",
+					PackageVersion: "1.0.0",
+				},
+				{
+					Name:        "CVE-a",
+					Severity:    "HIGH",
+					Platforms:   p("other1"),
+					PackageName: "cvea-pkg-3",
+					// varying by version is a separate finding
+					PackageVersion: "1.0.1",
 				},
 				{
 					Name:      "CVE-b",

src/finding/testdata/TestMergeSummary.golden

@@ -1,16 +1,39 @@
 finding.Summary{
 	Counts: map[types.FindingSeverity]finding.SeverityCount{
-		types.FindingSeverity("HIGH"): {Included: 4},
+		types.FindingSeverity("HIGH"): {Included: 7},
 	},
 	Details: []finding.Detail{
 		{
-			Name:     "CVE-a",
-			Severity: types.FindingSeverity("HIGH"),
+			Name:           "CVE-a",
+			Severity:       types.FindingSeverity("HIGH"),
+			PackageName:    "cvea-pkg",
+			PackageVersion: "1.0.0",
 			Platforms: []v1.Platform{
 				{OS: "base"},
 				{OS: "other1"},
 			},
 		},
+		{
+			Name:           "CVE-a",
+			Severity:       types.FindingSeverity("HIGH"),
+			PackageName:    "cvea-pkg-2",
+			PackageVersion: "1.0.0",
+			Platforms:      []v1.Platform{{OS: "other1"}},
+		},
+		{
+			Name:           "CVE-a",
+			Severity:       types.FindingSeverity("HIGH"),
+			PackageName:    "cvea-pkg-3",
+			PackageVersion: "1.0.0",
+			Platforms:      []v1.Platform{{OS: "other1"}},
+		},
+		{
+			Name:           "CVE-a",
+			Severity:       types.FindingSeverity("HIGH"),
+			PackageName:    "cvea-pkg-3",
+			PackageVersion: "1.0.1",
+			Platforms:      []v1.Platform{{OS: "other1"}},
+		},
 		{
 			Name:      "CVE-b",
 			Severity:  types.FindingSeverity("HIGH"),

src/report/annotation.go

@@ -120,7 +120,16 @@ func sortFindings(findings []finding.Detail) []finding.Detail {
 
 		// descending order of CVE, in general this means that newer CVEs will be at
 		// the top
-		return strings.Compare(b.Name, a.Name)
+		if name := strings.Compare(a.Name, b.Name); name != 0 {
+			return name * -1
+		}
+
+		// package name and version in ascending lexical order
+		if pname := strings.Compare(a.PackageName, b.PackageName); pname != 0 {
+			return pname
+		}
+
+		return strings.Compare(a.PackageVersion, b.PackageVersion)
 	})
 
 	return sorted

src/report/annotation_test.go

@@ -211,9 +211,7 @@ func TestReports(t *testing.T) {
 				ImageLabel: "label of image",
 				FindingSummary: finding.Summary{
 					Counts: map[types.FindingSeverity]finding.SeverityCount{
-						"HIGH":     {Included: 1},
-						"CRITICAL": {Included: 1, Ignored: 1},
-						"LOW":      {Included: 0, Ignored: 1},
+						"HIGH": {Included: 10},
 					},
 					Details: []finding.Detail{
 						{
@@ -231,7 +229,12 @@ func TestReports(t *testing.T) {
 							Severity: "HIGH",
 						},
 						{
-							Name:     "CVE-d",
+							Name:     "CVE-d1",
+							Severity: "HIGH",
+							CVSS2:    finding.NewCVSS2Score("6.0", ""),
+						},
+						{
+							Name:     "CVE-d2",
 							Severity: "HIGH",
 							CVSS2:    finding.NewCVSS2Score("6.0", ""),
 						},
@@ -242,9 +245,22 @@ func TestReports(t *testing.T) {
 							CVSS2:    finding.NewCVSS2Score("4.0", ""),
 						},
 						{
-							Name:     "CVE-g",
-							Severity: "HIGH",
-							CVSS2:    finding.NewCVSS3Score("8.0", ""),
+							Name:        "CVE-g",
+							Severity:    "HIGH",
+							CVSS2:       finding.NewCVSS3Score("8.0", ""),
+							PackageName: "g-3",
+						},
+						{
+							Name:        "CVE-g",
+							Severity:    "HIGH",
+							CVSS2:       finding.NewCVSS3Score("8.0", ""),
+							PackageName: "g-1",
+						},
+						{
+							Name:        "CVE-g",
+							Severity:    "HIGH",
+							CVSS2:       finding.NewCVSS3Score("8.0", ""),
+							PackageName: "g-2",
 						},
 						{
 							Name:     "CVE-h",

src/report/testdata/TestReports/sorted_findings.golden

@@ -12,28 +12,10 @@
 
 
 
-<div class="m1 p1 mr3">
-<dt>Critical</dt>
-<dd><h1 class="m0 red">1</h1>
-<em>+ 1 ignored</em>
-</dd>
-</div>
-
-
-
 <div class="m1 p1 mr3">
 <dt>High</dt>
-<dd><h1 class="m0 red">1</h1>
-
-</dd>
-</div>
-
+<dd><h1 class="m0 red">10</h1>
 
-
-<div class="m1 p1 mr3">
-<dt>Low</dt>
-<dd><h1 class="m0">0</h1>
-<em>+ 1 ignored</em>
 </dd>
 </div>
 
@@ -98,15 +80,45 @@
 <tr>
 <td>CVE-g</td>
 <td>High</td>
-<td>&nbsp; &nbsp;</td>
+<td>g-1 &nbsp;</td>
+
+<td>8.0 <em>(*CVSS2)</em></td><td>&nbsp;</td>
+
+
+</tr>
+
+<tr>
+<td>CVE-g</td>
+<td>High</td>
+<td>g-2 &nbsp;</td>
+
+<td>8.0 <em>(*CVSS2)</em></td><td>&nbsp;</td>
+
+
+</tr>
+
+<tr>
+<td>CVE-g</td>
+<td>High</td>
+<td>g-3 &nbsp;</td>
 
 <td>8.0 <em>(*CVSS2)</em></td><td>&nbsp;</td>
 
 
 </tr>
 
 <tr>
-<td>CVE-d</td>
+<td>CVE-d2</td>
+<td>High</td>
+<td>&nbsp; &nbsp;</td>
+
+<td>6.0 <em>(*CVSS2)</em></td><td>&nbsp;</td>
+
+
+</tr>
+
+<tr>
+<td>CVE-d1</td>
 <td>High</td>
 <td>&nbsp; &nbsp;</td>