diff --git a/src/pkg/scan/vuln/summary.go b/src/pkg/scan/vuln/summary.go index e629100b6..ac03e138b 100644 --- a/src/pkg/scan/vuln/summary.go +++ b/src/pkg/scan/vuln/summary.go @@ -45,6 +45,11 @@ type NativeReportSummary struct { CompleteCount int `json:"-"` } +// IsSuccessStatus returns true when the scan status is success +func (sum *NativeReportSummary) IsSuccessStatus() bool { + return sum.ScanStatus == job.SuccessStatus.String() +} + // Merge ... func (sum *NativeReportSummary) Merge(another *NativeReportSummary) *NativeReportSummary { r := &NativeReportSummary{} diff --git a/src/server/middleware/vulnerable/vulnerable.go b/src/server/middleware/vulnerable/vulnerable.go index 399fdc288..dce45bfe2 100644 --- a/src/server/middleware/vulnerable/vulnerable.go +++ b/src/server/middleware/vulnerable/vulnerable.go @@ -101,10 +101,13 @@ func Middleware() func(http.Handler) http.Handler { return err } + projectSeverity := vuln.ParseSeverityVersion3(proj.Severity()) + rawSummary, ok := summaries[v1.MimeTypeNativeReport] if !ok { // No report yet? - msg := "vulnerability prevention enabled, but no scan report existing for the artifact" + msg := fmt.Sprintf(`current image without vulnerability scanning cannot be pulled due to configured policy in 'Prevent images with vulnerability severity of "%s" or higher from running.' `+ + `To continue with pull, please contact your project administrator for help.`, projectSeverity) return errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage(msg) } @@ -125,18 +128,30 @@ func Middleware() func(http.Handler) http.Handler { } } + if !summary.IsSuccessStatus() { + msg := fmt.Sprintf(`current image with "%s" status of vulnerability scanning cannot be pulled due to configured policy in 'Prevent images with vulnerability severity of "%s" or higher from running.' `+ + `To continue with pull, please contact your project administrator for help.`, summary.ScanStatus, projectSeverity) + return errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage(msg) + } + + if summary.Summary == nil || summary.Summary.Total == 0 { + // No vulnerabilities found in the artifact, skip the checking + // See https://github.com/goharbor/harbor/issues/11210 to get more details + logger.Debugf("no vulnerabilities found in artifact %s@%s, skip the vulnerability prevention checking", art.RepositoryName, art.Digest) + return nil + } + // Do judgement - severity := vuln.ParseSeverityVersion3(proj.Severity()) - if summary.Severity.Code() >= severity.Code() { - msg := fmt.Sprintf("current image with '%q vulnerable' cannot be pulled due to configured policy in 'Prevent images with vulnerability severity of %q from running.' "+ - "Please contact your project administrator for help'", summary.Severity, severity) + if summary.Severity.Code() >= projectSeverity.Code() { + msg := fmt.Sprintf(`current image with %d vulnerabilities cannot be pulled due to configured policy in 'Prevent images with vulnerability severity of "%s" or higher from running.' `+ + `To continue with pull, please contact your project administrator to exempt matched vulnerabilities through configuring the CVE whitelist.`, summary.TotalCount, projectSeverity) return errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage(msg) } // Print scannerPull CVE list if len(summary.CVEBypassed) > 0 { for _, cve := range summary.CVEBypassed { - logger.Infof("Vulnerable policy check: scannerPull CVE %s", cve) + logger.Infof("Vulnerable policy check: bypassed CVE %s", cve) } } diff --git a/src/server/middleware/vulnerable/vulnerable_test.go b/src/server/middleware/vulnerable/vulnerable_test.go index b3d996cea..914e6e44d 100644 --- a/src/server/middleware/vulnerable/vulnerable_test.go +++ b/src/server/middleware/vulnerable/vulnerable_test.go @@ -119,6 +119,16 @@ func (suite *MiddlewareTestSuite) makeRequest() *http.Request { return req.WithContext(lib.WithArtifactInfo(req.Context(), info)) } +func (suite *MiddlewareTestSuite) TestNoArtifactInfo() { + mock.OnAnything(suite.artifactController, "GetByReference").Return(nil, fmt.Errorf("error")) + + req := httptest.NewRequest("GET", "/v1/library/photon/manifests/2.0", nil) + rr := httptest.NewRecorder() + + Middleware()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusInternalServerError) +} + func (suite *MiddlewareTestSuite) TestGetArtifactFailed() { mock.OnAnything(suite.artifactController, "GetByReference").Return(nil, fmt.Errorf("error")) @@ -220,6 +230,24 @@ func (suite *MiddlewareTestSuite) TestArtifactNotScanned() { suite.Equal(rr.Code, http.StatusPreconditionFailed) } +func (suite *MiddlewareTestSuite) TestArtifactScanFailed() { + mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) + mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil) + mock.OnAnything(suite.checker, "IsScannable").Return(true, nil) + mock.OnAnything(suite.scanController, "GetSummary").Return(map[string]interface{}{ + v1.MimeTypeNativeReport: &vuln.NativeReportSummary{ + ScanStatus: "Error", + CVEBypassed: []string{"cve-2020"}, + }, + }, nil) + + req := suite.makeRequest() + rr := httptest.NewRecorder() + + Middleware()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusPreconditionFailed) +} + func (suite *MiddlewareTestSuite) TestGetSummaryFailed() { mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil) @@ -233,13 +261,69 @@ func (suite *MiddlewareTestSuite) TestGetSummaryFailed() { suite.Equal(rr.Code, http.StatusInternalServerError) } +func (suite *MiddlewareTestSuite) TestBadSummary() { + mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) + mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil) + mock.OnAnything(suite.checker, "IsScannable").Return(true, nil) + mock.OnAnything(suite.scanController, "GetSummary").Return(map[string]interface{}{ + v1.MimeTypeNativeReport: "bad report", + }, nil) + + req := suite.makeRequest() + rr := httptest.NewRecorder() + + Middleware()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusInternalServerError) +} + +func (suite *MiddlewareTestSuite) TestNoVulnerabilities() { + mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) + mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil) + mock.OnAnything(suite.checker, "IsScannable").Return(true, nil) + mock.OnAnything(suite.scanController, "GetSummary").Return(map[string]interface{}{ + v1.MimeTypeNativeReport: &vuln.NativeReportSummary{ + ScanStatus: "Success", + Severity: vuln.Unknown, + CVEBypassed: []string{"cve-2020"}, + }, + }, nil) + + req := suite.makeRequest() + rr := httptest.NewRecorder() + + Middleware()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusOK) +} + +func (suite *MiddlewareTestSuite) TestTotalVulnerabilitiesIsZero() { + mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) + mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil) + mock.OnAnything(suite.checker, "IsScannable").Return(true, nil) + mock.OnAnything(suite.scanController, "GetSummary").Return(map[string]interface{}{ + v1.MimeTypeNativeReport: &vuln.NativeReportSummary{ + ScanStatus: "Success", + Severity: vuln.Unknown, + Summary: &vuln.VulnerabilitySummary{Total: 0}, + CVEBypassed: []string{"cve-2020"}, + }, + }, nil) + + req := suite.makeRequest() + rr := httptest.NewRecorder() + + Middleware()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusOK) +} + func (suite *MiddlewareTestSuite) TestAllowed() { mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil) mock.OnAnything(suite.checker, "IsScannable").Return(true, nil) mock.OnAnything(suite.scanController, "GetSummary").Return(map[string]interface{}{ v1.MimeTypeNativeReport: &vuln.NativeReportSummary{ + ScanStatus: "Success", Severity: vuln.Low, + Summary: &vuln.VulnerabilitySummary{Total: 1}, CVEBypassed: []string{"cve-2020"}, }, }, nil) @@ -257,7 +341,9 @@ func (suite *MiddlewareTestSuite) TestPrevented() { mock.OnAnything(suite.checker, "IsScannable").Return(true, nil) mock.OnAnything(suite.scanController, "GetSummary").Return(map[string]interface{}{ v1.MimeTypeNativeReport: &vuln.NativeReportSummary{ - Severity: vuln.Critical, + ScanStatus: "Success", + Severity: vuln.Critical, + Summary: &vuln.VulnerabilitySummary{Total: 1}, }, }, nil)