feat: remove duplicate CVE in scan report and summary (#13918)

1. Remove the duplicate CVE records in the report/summary for the image
index.
2. Add scanner field in the scan overview for the API.

Closes #13913

Signed-off-by: He Weiwei <hweiwei@vmware.com>

api/v2.0/swagger.yaml

@@ -2800,6 +2800,21 @@ definitions:
         type: string
         format: date-time
         description: The update time of the label
+  Scanner:
+    type: object
+    properties:
+      name:
+        type: string
+        description: Name of the scanner
+        example: "Trivy"
+      vendor:
+        type: string
+        description: Name of the scanner provider
+        example: "Aqua Security"
+      version:
+        type: string
+        description: Version of the scanner adapter
+        example: "v0.9.1"
   ScanOverview:
     type: object
     description: 'The scan overview attached in the metadata of tag'
@@ -2842,6 +2857,8 @@ definitions:
         type: integer
         description: 'The complete percent of the scanning which value is between 0 and 100'
         example: 100
+      scanner:
+        $ref: '#/definitions/Scanner'
   VulnerabilitySummary:
     type: object
     description: |
@@ -2852,11 +2869,13 @@ definitions:
         format: int
         description: 'The total number of the found vulnerabilities'
         example: 500
+        x-omitempty: false
       fixable:
         type: integer
         format: int
         description: 'The number of the fixable vulnerabilities'
         example: 100
+        x-omitempty: false
       summary:
         type: object
         description: 'Numbers of the vulnerabilities with different severity'
@@ -2867,6 +2886,7 @@ definitions:
         example:
           'Critical': 5
           'High': 5
+        x-omitempty: false
   AuditLog:
     type: object
     properties:

src/common/models/cve_allowlist.go

@@ -59,9 +59,26 @@ func (c *CVEAllowlist) IsExpired() bool {
 // CVESet defines the CVE allowlist with a hash set way for easy query.
 type CVESet map[string]struct{}
 
+// Add add cve to the set
+func (cs CVESet) Add(cve string) {
+	cs[cve] = struct{}{}
+}
+
 // Contains checks whether the specified CVE is in the set or not.
 func (cs CVESet) Contains(cve string) bool {
 	_, ok := cs[cve]
 
 	return ok
 }
+
+// NewCVESet returns CVESet from cveSets
+func NewCVESet(cveSets ...CVESet) CVESet {
+	s := CVESet{}
+	for _, cveSet := range cveSets {
+		for cve := range cveSet {
+			s.Add(cve)
+		}
+	}
+
+	return s
+}

src/pkg/scan/postprocessors/report_converters.go

@@ -143,7 +143,7 @@ func (c *nativeToRelationalSchemaConverter) fromSchema(ctx context.Context, repo
 	for _, record := range records {
 		vi := new(vuln.VulnerabilityItem)
 		vi.ID = record.CVEID
-		vi.ArtifactDigest = artifactDigest
+		vi.ArtifactDigests = []string{artifactDigest}
 		vi.CVSSDetails.ScoreV2 = record.CVE2Score
 		vi.CVSSDetails.ScoreV3 = record.CVE3Score
 		vi.CVSSDetails.VectorV2 = record.CVSS2Vector

src/pkg/scan/report/summary.go

@@ -154,49 +154,10 @@ func GenerateNativeSummary(r *scan.Report, options ...Option) (interface{}, erro
 
 	sum.CompleteCount = 1
 	sum.CompletePercent = 100
-
 	sum.Severity = rp.Severity
-	vsum := &vuln.VulnerabilitySummary{
-		Total:   len(rp.Vulnerabilities),
-		Summary: make(vuln.SeveritySummary),
-	}
-
-	overallSev := vuln.None
-	for _, v := range rp.Vulnerabilities {
-		if len(ops.CVEAllowlist) > 0 && ops.CVEAllowlist.Contains(v.ID) {
-			// If allowlist is set, then check if we need to bypass it
-			// Reduce the total
-			vsum.Total--
-			// Append the by passed CVEs specified in the allowlist
-			sum.CVEBypassed = append(sum.CVEBypassed, v.ID)
-
-			continue
-		}
-
-		if num, ok := vsum.Summary[v.Severity]; ok {
-			vsum.Summary[v.Severity] = num + 1
-		} else {
-			vsum.Summary[v.Severity] = 1
-		}
-
-		// Update the overall severity if necessary
-		if v.Severity.Code() > overallSev.Code() {
-			overallSev = v.Severity
-		}
-
-		// If the CVE item has a fixable version
-		if len(v.FixVersion) > 0 {
-			vsum.Fixable++
-		}
-	}
-	sum.Summary = vsum
-
-	// Override the overall severity of the filtered list if needed.
-	if len(ops.CVEAllowlist) > 0 {
-		sum.Severity = overallSev
-	}
-
 	sum.Scanner = rp.Scanner
 
+	sum.UpdateSeveritySummaryAndByPassed(rp.GetVulnerabilityItemList(), ops.CVEAllowlist)
+
 	return sum, nil
 }

src/pkg/scan/vuln/report.go

@@ -16,7 +16,9 @@ package vuln
 
 import (
 	"encoding/json"
+	"fmt"
 
+	"github.com/goharbor/harbor/src/common/models"
 	v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
 )
 
@@ -30,6 +32,21 @@ type Report struct {
 	Severity Severity `json:"severity"`
 	// Vulnerability list
 	Vulnerabilities []*VulnerabilityItem `json:"vulnerabilities"`
+
+	vulnerabilityItemList *VulnerabilityItemList
+}
+
+// GetVulnerabilityItemList returns VulnerabilityItemList from the Vulnerabilities of report
+func (report *Report) GetVulnerabilityItemList() *VulnerabilityItemList {
+	l := report.vulnerabilityItemList
+	if l == nil {
+		l = &VulnerabilityItemList{}
+		l.Add(report.Vulnerabilities...)
+
+		report.vulnerabilityItemList = l
+	}
+
+	return l
 }
 
 // MarshalJSON custom function to dump nil slice of Vulnerabilities as empty slice
@@ -52,23 +69,25 @@ func (report *Report) MarshalJSON() ([]byte, error) {
 
 // Merge ...
 func (report *Report) Merge(another *Report) *Report {
+	scanner := report.Scanner
 	generatedAt := report.GeneratedAt
-	if another.GeneratedAt > generatedAt {
+	if another.GeneratedAt > report.GeneratedAt {
 		generatedAt = another.GeneratedAt
+
+		// choose the scanner from the newer summary
+		// because the generatedAt of the report is from the newer report
+		scanner = another.Scanner
 	}
 
-	vulnerabilities := report.Vulnerabilities
-	if vulnerabilities == nil {
-		vulnerabilities = another.Vulnerabilities
-	} else {
-		vulnerabilities = append(vulnerabilities, another.Vulnerabilities...)
-	}
+	l := report.GetVulnerabilityItemList()
+	l.Add(another.Vulnerabilities...)
 
 	r := &Report{
 		GeneratedAt:           generatedAt,
-		Scanner:         report.Scanner,
+		Scanner:               scanner,
 		Severity:              mergeSeverity(report.Severity, another.Severity),
-		Vulnerabilities: vulnerabilities,
+		Vulnerabilities:       l.Items(),
+		vulnerabilityItemList: l,
 	}
 
 	return r
@@ -77,10 +96,100 @@ func (report *Report) Merge(another *Report) *Report {
 // WithArtifactDigest set artifact digest for the report
 func (report *Report) WithArtifactDigest(artifactDigest string) {
 	for _, vul := range report.Vulnerabilities {
-		vul.ArtifactDigest = artifactDigest
+		vul.ArtifactDigests = []string{artifactDigest}
 	}
 }
 
+// NewVulnerabilityItemList returns VulnerabilityItemList from lists
+func NewVulnerabilityItemList(lists ...*VulnerabilityItemList) *VulnerabilityItemList {
+	var availableLists []*VulnerabilityItemList
+	for _, li := range lists {
+		if li != nil {
+			availableLists = append(availableLists, li)
+		}
+	}
+
+	if len(availableLists) == 0 {
+		return nil
+	}
+
+	l := &VulnerabilityItemList{}
+	for _, li := range availableLists {
+		l.Add(li.Items()...)
+	}
+
+	return l
+}
+
+// VulnerabilityItemList the list can skip the VulnerabilityItem exists in the list when adding
+type VulnerabilityItemList struct {
+	items   []*VulnerabilityItem
+	indexed map[string]*VulnerabilityItem
+}
+
+// Items returns the vulnerabilities in the l
+func (l *VulnerabilityItemList) Items() []*VulnerabilityItem {
+	return l.items
+}
+
+// Add add item to the list when the item not exists in list
+func (l *VulnerabilityItemList) Add(items ...*VulnerabilityItem) {
+	if l.indexed == nil {
+		l.indexed = map[string]*VulnerabilityItem{}
+	}
+
+	for _, item := range items {
+		key := item.Key()
+		if v, ok := l.indexed[key]; ok {
+			v.ArtifactDigests = append(v.ArtifactDigests, item.ArtifactDigests...)
+		} else {
+			l.items = append(l.items, item)
+			l.indexed[key] = item
+		}
+	}
+}
+
+// GetSeveritySummaryAndByPassed returns the Severity Summary and ByPassed by allowlist for the l
+func (l *VulnerabilityItemList) GetSeveritySummaryAndByPassed(allowlist models.CVESet) (Severity, *VulnerabilitySummary, []string) {
+	sum := &VulnerabilitySummary{
+		Total:   len(l.Items()),
+		Summary: make(SeveritySummary),
+	}
+
+	var bypassed []string
+
+	severity := None
+	for _, v := range l.Items() {
+		if len(allowlist) > 0 && allowlist.Contains(v.ID) {
+			// If allowlist is set, then check if we need to bypass it
+			// Reduce the total
+			sum.Total--
+			// Append the by passed CVEs specified in the allowlist
+			bypassed = append(bypassed, v.ID)
+
+			continue
+		}
+
+		if num, ok := sum.Summary[v.Severity]; ok {
+			sum.Summary[v.Severity] = num + 1
+		} else {
+			sum.Summary[v.Severity] = 1
+		}
+
+		// Update the overall severity if necessary
+		if v.Severity.Code() > severity.Code() {
+			severity = v.Severity
+		}
+
+		// If the CVE item has a fixable version
+		if len(v.FixVersion) > 0 {
+			sum.Fixable++
+		}
+	}
+
+	return severity, sum, bypassed
+}
+
 // VulnerabilityItem represents one found vulnerability
 type VulnerabilityItem struct {
 	// The unique identifier of the vulnerability.
@@ -106,9 +215,9 @@ type VulnerabilityItem struct {
 	// Format: URI
 	// e.g: List [ "https://security-tracker.debian.org/tracker/CVE-2017-8283" ]
 	Links []string `json:"links"`
-	// The artifact digest which the vulnerability belonged
+	// The artifact digests which the vulnerability belonged
 	// e.g: sha256@ee1d00c5250b5a886b09be2d5f9506add35dfb557f1ef37a7e4b8f0138f32956
-	ArtifactDigest string `json:"artifact_digest"`
+	ArtifactDigests []string `json:"artifact_digests"`
 	// The CVSS3 and CVSS2 based scores and attack vector for the vulnerability item
 	CVSSDetails CVSS `json:"preferred_cvss"`
 	// A separated list of CWE Ids associated with this vulnerability
@@ -119,6 +228,11 @@ type VulnerabilityItem struct {
 	VendorAttributes map[string]interface{} `json:"vendor_attributes"`
 }
 
+// Key returns the uniq key for the item
+func (item *VulnerabilityItem) Key() string {
+	return fmt.Sprintf("%s-%s-%s", item.ID, item.Package, item.Version)
+}
+
 // CVSS holds the score and attack vector for the vulnerability based on the CVSS3 and CVSS2 standards
 type CVSS struct {
 	// The CVSS-3 score for the vulnerability

src/pkg/scan/vuln/report_test.go

@@ -15,14 +15,16 @@
 package vuln
 
 import (
+	"encoding/json"
 	"reflect"
 	"testing"
 
+	"github.com/goharbor/harbor/src/common/models"
 	v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestReport_Merge(t *testing.T) {
-	emptyVulnerabilities := []*VulnerabilityItem{}
 	a := []*VulnerabilityItem{
 		{ID: "CVE-2017-8283"},
 		{ID: "CVE-2017-8284"},
@@ -46,9 +48,6 @@ func TestReport_Merge(t *testing.T) {
 		want   *Report
 	}{
 		{"GeneratedAt", fields{GeneratedAt: "2020-04-06T18:38:34.791086859Z"}, args{&Report{GeneratedAt: "2020-04-06T18:38:34.791086860Z"}}, &Report{GeneratedAt: "2020-04-06T18:38:34.791086860Z"}},
-		{"Vulnerabilities nil & nil", fields{Vulnerabilities: nil}, args{&Report{Vulnerabilities: nil}}, &Report{Vulnerabilities: nil}},
-		{"Vulnerabilities nil & not nil", fields{Vulnerabilities: nil}, args{&Report{Vulnerabilities: emptyVulnerabilities}}, &Report{Vulnerabilities: emptyVulnerabilities}},
-		{"Vulnerabilities not nil & nil", fields{Vulnerabilities: emptyVulnerabilities}, args{&Report{Vulnerabilities: nil}}, &Report{Vulnerabilities: emptyVulnerabilities}},
 		{"Vulnerabilities nil & a", fields{Vulnerabilities: nil}, args{&Report{Vulnerabilities: a}}, &Report{Vulnerabilities: a}},
 		{"Vulnerabilities a & nil", fields{Vulnerabilities: a}, args{&Report{Vulnerabilities: nil}}, &Report{Vulnerabilities: a}},
 		{"Vulnerabilities a & b", fields{Vulnerabilities: a}, args{&Report{Vulnerabilities: b}}, &Report{Vulnerabilities: append(a, b...)}},
@@ -61,9 +60,76 @@ func TestReport_Merge(t *testing.T) {
 				Severity:        tt.fields.Severity,
 				Vulnerabilities: tt.fields.Vulnerabilities,
 			}
-			if got := report.Merge(tt.args.another); !reflect.DeepEqual(got, tt.want) {
+			got := report.Merge(tt.args.another)
+			got.vulnerabilityItemList = nil
+			if !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("Report.Merge() = %v, want %v", got, tt.want)
 			}
 		})
 	}
 }
+
+func TestReportMarshalJSON(t *testing.T) {
+	assert := assert.New(t)
+
+	report := &Report{
+		GeneratedAt: "GeneratedAt",
+	}
+
+	b, _ := json.Marshal(report)
+	assert.Contains(string(b), "vulnerabilities")
+}
+
+func TestGetSummarySeverityAndByPassed(t *testing.T) {
+	assert := assert.New(t)
+
+	vul1 := &VulnerabilityItem{
+		ID:         "cve1",
+		Severity:   Low,
+		FixVersion: "1.3",
+	}
+
+	vul2 := &VulnerabilityItem{
+		ID:       "cve2",
+		Severity: Low,
+	}
+
+	vul3 := &VulnerabilityItem{
+		ID:       "cve3",
+		Severity: Medium,
+	}
+
+	l := VulnerabilityItemList{}
+	l.Add(vul1, vul2, vul3)
+
+	{
+		s := SeveritySummary{
+			Low:    2,
+			Medium: 1,
+		}
+
+		severity, sum, byPassed := l.GetSeveritySummaryAndByPassed(models.CVESet{})
+		assert.Equal(3, sum.Total)
+		assert.Equal(1, sum.Fixable)
+		assert.Equal(s, sum.Summary)
+		assert.Equal(Medium, severity)
+		assert.Empty(byPassed)
+	}
+
+	{
+		s := SeveritySummary{
+			Low: 2,
+		}
+
+		cveSet := models.CVESet{}
+		cveSet.Add("cve3")
+
+		severity, sum, byPassed := l.GetSeveritySummaryAndByPassed(cveSet)
+		assert.Equal(2, sum.Total)
+		assert.Equal(1, sum.Fixable)
+		assert.Equal(s, sum.Summary)
+		assert.Equal(Low, severity)
+		assert.NotEmpty(byPassed)
+		assert.Equal([]string{"cve3"}, byPassed)
+	}
+}

src/pkg/scan/vuln/summary.go

@@ -17,6 +17,7 @@ package vuln
 import (
 	"time"
 
+	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/jobservice/job"
 	v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
 )
@@ -37,6 +38,26 @@ type NativeReportSummary struct {
 
 	TotalCount            int                    `json:"-"`
 	CompleteCount         int                    `json:"-"`
+	VulnerabilityItemList *VulnerabilityItemList `json:"-"`
+	CVESet                models.CVESet          `json:"-"`
+}
+
+// UpdateSeveritySummaryAndByPassed update the Severity, Summary and CVEBypassed of the sum from l and s
+func (sum *NativeReportSummary) UpdateSeveritySummaryAndByPassed(l *VulnerabilityItemList, s models.CVESet) {
+	sum.VulnerabilityItemList = l
+	sum.CVESet = s
+
+	if l == nil {
+		return
+	}
+
+	var severity Severity
+	severity, sum.Summary, sum.CVEBypassed = l.GetSeveritySummaryAndByPassed(s)
+
+	if len(s) > 0 {
+		// Override the overall severity of the filtered list if needed.
+		sum.Severity = severity
+	}
 }
 
 // IsSuccessStatus returns true when the scan status is success
@@ -51,7 +72,13 @@ func (sum *NativeReportSummary) Merge(another *NativeReportSummary) *NativeRepor
 	r.StartTime = minTime(sum.StartTime, another.StartTime)
 	r.EndTime = maxTime(sum.EndTime, another.EndTime)
 	r.Duration = r.EndTime.Unix() - r.StartTime.Unix()
+	// choose the scanner from the newer summary
+	// because the endtime of the summary is from the newer summary
+	if sum.StartTime.After(another.StartTime) {
 		r.Scanner = sum.Scanner
+	} else {
+		r.Scanner = another.Scanner
+	}
 	r.TotalCount = sum.TotalCount + another.TotalCount
 	r.CompleteCount = sum.CompleteCount + another.CompleteCount
 	r.CompletePercent = r.CompleteCount * 100 / r.TotalCount
@@ -59,13 +86,10 @@ func (sum *NativeReportSummary) Merge(another *NativeReportSummary) *NativeRepor
 	r.Severity = mergeSeverity(sum.Severity, another.Severity)
 	r.ScanStatus = mergeScanStatus(sum.ScanStatus, another.ScanStatus)
 
-	if sum.Summary != nil && another.Summary != nil {
-		r.Summary = sum.Summary.Merge(another.Summary)
-	} else if sum.Summary != nil {
-		r.Summary = sum.Summary
-	} else {
-		r.Summary = another.Summary
-	}
+	r.UpdateSeveritySummaryAndByPassed(
+		NewVulnerabilityItemList(sum.VulnerabilityItemList, another.VulnerabilityItemList),
+		models.NewCVESet(sum.CVESet, another.CVESet),
+	)
 
 	return r
 }
@@ -78,28 +102,5 @@ type VulnerabilitySummary struct {
 	Summary SeveritySummary `json:"summary"`
 }
 
-// Merge ...
-func (v *VulnerabilitySummary) Merge(a *VulnerabilitySummary) *VulnerabilitySummary {
-	r := &VulnerabilitySummary{
-		Total:   v.Total + a.Total,
-		Fixable: v.Fixable + a.Fixable,
-		Summary: SeveritySummary{},
-	}
-
-	for k, v := range v.Summary {
-		r.Summary[k] = v
-	}
-
-	for k, v := range a.Summary {
-		if _, ok := r.Summary[k]; ok {
-			r.Summary[k] += v
-		} else {
-			r.Summary[k] = v
-		}
-	}
-
-	return r
-}
-
 // SeveritySummary ...
 type SeveritySummary map[Severity]int

src/pkg/scan/vuln/summary_test.go

@@ -21,27 +21,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestMergeVulnerabilitySummary(t *testing.T) {
-	assert := assert.New(t)
-	v1 := VulnerabilitySummary{
-		Total:   1,
-		Fixable: 1,
-		Summary: map[Severity]int{Low: 1},
-	}
-
-	r := v1.Merge(&VulnerabilitySummary{
-		Total:   1,
-		Fixable: 1,
-		Summary: map[Severity]int{Low: 1, High: 1},
-	})
-
-	assert.Equal(2, r.Total)
-	assert.Equal(2, r.Fixable)
-	assert.Len(r.Summary, 2)
-	assert.Equal(2, r.Summary[Low])
-	assert.Equal(1, r.Summary[High])
-}
-
 func TestMergeNativeReportSummary(t *testing.T) {
 	assert := assert.New(t)
 	errorStatus := job.ErrorStatus.String()
@@ -53,11 +32,22 @@ func TestMergeNativeReportSummary(t *testing.T) {
 		Summary: map[Severity]int{Low: 1},
 	}
 
+	l := &VulnerabilityItemList{}
+	l.Add(&VulnerabilityItem{
+		ID:         "cve-id",
+		Package:    "openssl-libs",
+		Version:    "1:1.1.1g-11.el8",
+		Severity:   Low,
+		FixVersion: "1:1.1.1g-12.el8_3",
+	})
+
+	{
 		n1 := NativeReportSummary{
 			ScanStatus:            runningStatus,
 			Severity:              Low,
 			TotalCount:            1,
 			Summary:               &v1,
+			VulnerabilityItemList: l,
 		}
 
 		r := n1.Merge(&NativeReportSummary{
@@ -69,4 +59,65 @@ func TestMergeNativeReportSummary(t *testing.T) {
 		assert.Equal(runningStatus, r.ScanStatus)
 		assert.Equal(Low, r.Severity)
 		assert.Equal(v1, *r.Summary)
+	}
+
+	{
+		n1 := NativeReportSummary{
+			ScanStatus: runningStatus,
+			Severity:   Severity(""),
+			TotalCount: 1,
+		}
+
+		r := n1.Merge(&NativeReportSummary{
+			ScanStatus: errorStatus,
+			Severity:   Severity(""),
+			TotalCount: 1,
+		})
+
+		assert.Equal(runningStatus, r.ScanStatus)
+		assert.Equal(Severity(""), r.Severity)
+		assert.Nil(r.Summary)
+	}
+
+	{
+		n1 := &NativeReportSummary{
+			ScanStatus: errorStatus,
+			Severity:   Severity(""),
+			TotalCount: 1,
+		}
+
+		r := n1.Merge(&NativeReportSummary{
+			ScanStatus:            runningStatus,
+			Severity:              Low,
+			TotalCount:            1,
+			Summary:               &v1,
+			VulnerabilityItemList: l,
+		})
+
+		assert.Equal(runningStatus, r.ScanStatus)
+		assert.Equal(Low, r.Severity)
+		assert.Equal(v1, *r.Summary)
+	}
+
+	{
+		n1 := &NativeReportSummary{
+			ScanStatus:            runningStatus,
+			Severity:              Low,
+			TotalCount:            1,
+			Summary:               &v1,
+			VulnerabilityItemList: l,
+		}
+
+		r := n1.Merge(&NativeReportSummary{
+			ScanStatus:            runningStatus,
+			Severity:              Low,
+			TotalCount:            1,
+			Summary:               &v1,
+			VulnerabilityItemList: l,
+		})
+
+		assert.Equal(runningStatus, r.ScanStatus)
+		assert.Equal(Low, r.Severity)
+		assert.Equal(v1, *r.Summary)
+	}
 }