feat(middleware,vulnerable): add image index checking for vulnerability prevention (#11084)

1. Skip vulnerability prevention checking when artifact is not
scannable.
2. Skip vulnerability prevention checking when artifact is image index
and its type is `IMAGE` or `CNAB`.
3. Skip vulnerability prevention checking when the artifact is pulling
by the scanner.
4. Change `hasCapability` from blacklist to whitelist.

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

src/api/artifact/const.go

@@ -0,0 +1,29 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package artifact
+
+const (
+	// ChartType chart type of artifact
+	ChartType = "CHART"
+
+	// CNABType cnab type of artifact
+	CNABType = "CNAB"
+
+	// ImageType image type of artifact
+	ImageType = "IMAGE"
+
+	// UnknownType unknown type of artifact
+	UnknownType = "UNKNOWN"
+)

src/api/project/controller.go

@@ -18,8 +18,11 @@ import (
 	"context"
 
 	"github.com/goharbor/harbor/src/common/models"
+	"github.com/goharbor/harbor/src/common/utils/log"
+	"github.com/goharbor/harbor/src/core/promgr/metamgr"
 	ierror "github.com/goharbor/harbor/src/internal/error"
 	"github.com/goharbor/harbor/src/pkg/project"
+	"github.com/goharbor/harbor/src/pkg/scan/whitelist"
 )
 
 var (
@@ -30,23 +33,27 @@ var (
 // Controller defines the operations related with blobs
 type Controller interface {
 	// Get get the project by project id
-	Get(ctx context.Context, projectID int64) (*models.Project, error)
+	Get(ctx context.Context, projectID int64, options ...Option) (*models.Project, error)
 	// GetByName get the project by project name
-	GetByName(ctx context.Context, projectName string) (*models.Project, error)
+	GetByName(ctx context.Context, projectName string, options ...Option) (*models.Project, error)
 }
 
 // NewController creates an instance of the default project controller
 func NewController() Controller {
 	return &controller{
 		projectMgr:   project.Mgr,
+		metaMgr:      metamgr.NewDefaultProjectMetadataManager(),
+		whitelistMgr: whitelist.NewDefaultManager(),
 	}
 }
 
 type controller struct {
 	projectMgr   project.Manager
+	metaMgr      metamgr.ProjectMetadataManager
+	whitelistMgr whitelist.Manager
 }
 
-func (c *controller) Get(ctx context.Context, projectID int64) (*models.Project, error) {
+func (c *controller) Get(ctx context.Context, projectID int64, options ...Option) (*models.Project, error) {
 	p, err := c.projectMgr.Get(projectID)
 	if err != nil {
 		return nil, err
@@ -55,10 +62,10 @@ func (c *controller) Get(ctx context.Context, projectID int64) (*models.Project,
 		return nil, ierror.NotFoundError(nil).WithMessage("project %d not found", projectID)
 	}
 
-	return p, nil
+	return c.assembleProject(ctx, p, options...)
 }
 
-func (c *controller) GetByName(ctx context.Context, projectName string) (*models.Project, error) {
+func (c *controller) GetByName(ctx context.Context, projectName string, options ...Option) (*models.Project, error) {
 	if projectName == "" {
 		return nil, ierror.BadRequestError(nil).WithMessage("project name required")
 	}
@@ -71,5 +78,46 @@ func (c *controller) GetByName(ctx context.Context, projectName string) (*models
 		return nil, ierror.NotFoundError(nil).WithMessage("project %s not found", projectName)
 	}
 
+	return c.assembleProject(ctx, p, options...)
+}
+
+func (c *controller) assembleProject(ctx context.Context, p *models.Project, options ...Option) (*models.Project, error) {
+	opts := newOptions(options...)
+
+	if opts.Metadata {
+		meta, err := c.metaMgr.Get(p.ProjectID)
+		if err != nil {
+			return nil, err
+		}
+		if len(p.Metadata) == 0 {
+			p.Metadata = make(map[string]string)
+		}
+
+		for k, v := range meta {
+			p.Metadata[k] = v
+		}
+	}
+
+	if opts.CVEWhitelist {
+		if p.ReuseSysCVEWhitelist() {
+			wl, err := c.whitelistMgr.GetSys()
+			if err != nil {
+				log.Errorf("get system CVE whitelist failed, error: %v", err)
+				return nil, err
+			}
+
+			wl.ProjectID = p.ProjectID
+			p.CVEWhitelist = *wl
+		} else {
+			wl, err := c.whitelistMgr.Get(p.ProjectID)
+			if err != nil {
+				return nil, err
+			}
+
+			p.CVEWhitelist = *wl
+		}
+
+	}
+
 	return p, nil
 }

src/api/project/controller_test.go

@@ -38,21 +38,21 @@ func (suite *ControllerTestSuite) TestGetByName() {
 	c := controller{projectMgr: mgr}
 
 	{
-		p, err := c.GetByName(context.TODO(), "library")
+		p, err := c.GetByName(context.TODO(), "library", Metadata(false))
 		suite.Nil(err)
 		suite.Equal("library", p.Name)
 		suite.Equal(int64(1), p.ProjectID)
 	}
 
 	{
-		p, err := c.GetByName(context.TODO(), "test")
+		p, err := c.GetByName(context.TODO(), "test", Metadata(false))
 		suite.Error(err)
 		suite.True(ierror.IsNotFoundErr(err))
 		suite.Nil(p)
 	}
 
 	{
-		p, err := c.GetByName(context.TODO(), "oops")
+		p, err := c.GetByName(context.TODO(), "oops", Metadata(false))
 		suite.Error(err)
 		suite.False(ierror.IsNotFoundErr(err))
 		suite.Nil(p)

src/api/project/options.go

@@ -0,0 +1,50 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package project
+
+// Option option for `Get` and `Exist` method of `Controller`
+type Option func(*Options)
+
+// Options options used by `Get` method of `Controller`
+type Options struct {
+	CVEWhitelist bool // get project with cve whitelist
+	Metadata     bool // get project with metadata
+}
+
+// CVEWhitelist set CVEWhitelist for the Options
+func CVEWhitelist(whitelist bool) Option {
+	return func(opts *Options) {
+		opts.CVEWhitelist = whitelist
+	}
+}
+
+// Metadata set Metadata for the Options
+func Metadata(metadata bool) Option {
+	return func(opts *Options) {
+		opts.Metadata = metadata
+	}
+}
+
+func newOptions(options ...Option) *Options {
+	opts := &Options{
+		Metadata: true, // default get project with metadata
+	}
+
+	for _, f := range options {
+		f(opts)
+	}
+
+	return opts
+}

src/api/scan/base_controller.go

@@ -131,16 +131,16 @@ func (bc *basicController) collectScanningArtifacts(ctx context.Context, r *scan
 	)
 
 	walkFn := func(a *ar.Artifact) error {
-		hasCapability := HasCapability(r, a)
+		supported := hasCapability(r, a)
 
-		if !hasCapability && a.HasChildren() {
+		if !supported && a.IsImageIndex() {
 			// image index not supported by the scanner, so continue to walk its children
 			return nil
 		}
 
 		artifacts = append(artifacts, a)
 
-		if hasCapability {
+		if supported {
 			scannable = true
 			return ar.ErrSkip // this artifact supported by the scanner, skip to walk its children
 		}
@@ -263,7 +263,7 @@ func (bc *basicController) makeReportPlaceholder(ctx context.Context, r *scanner
 		return e
 	}
 
-	if HasCapability(r, art) {
+	if hasCapability(r, art) {
 		var producesMimes []string
 
 		for _, pm := range r.GetProducesMimeTypes(art.ManifestMediaType) {

src/api/scan/base_controller_test.go

@@ -66,6 +66,7 @@ func TestController(t *testing.T) {
 // SetupSuite ...
 func (suite *ControllerTestSuite) SetupSuite() {
 	suite.artifact = &artifact.Artifact{}
+	suite.artifact.Type = "IMAGE"
 	suite.artifact.ProjectID = 1
 	suite.artifact.RepositoryName = "library/photon"
 	suite.artifact.Digest = "digest-code"

src/api/scan/checker.go

@@ -68,7 +68,7 @@ func (c *checker) IsScannable(ctx context.Context, art *artifact.Artifact) (bool
 	var scannable bool
 
 	walkFn := func(a *artifact.Artifact) error {
-		if HasCapability(r, a) {
+		if hasCapability(r, a) {
 			scannable = true
 			return artifact.ErrBreak
 		}
@@ -83,12 +83,17 @@ func (c *checker) IsScannable(ctx context.Context, art *artifact.Artifact) (bool
 	return scannable, nil
 }
 
-// HasCapability returns true when scanner has capability for the artifact
+// hasCapability returns true when scanner has capability for the artifact
 // See https://github.com/goharbor/pluggable-scanner-spec/issues/2 to get more info
-func HasCapability(r *models.Registration, a *artifact.Artifact) bool {
+func hasCapability(r *models.Registration, a *artifact.Artifact) bool {
-	if a.Type == "CHART" || a.Type == "UNKNOWN" {
+	// use whitelist here because currently only docker image is supported by the scanner
-		return false
+	// https://github.com/goharbor/pluggable-scanner-spec/issues/2
+	whitelist := []string{artifact.ImageType}
+	for _, t := range whitelist {
+		if a.Type == t {
+			return r.HasCapability(a.ManifestMediaType)
+		}
 	}
 
-	return r.HasCapability(a.ManifestMediaType)
+	return false
 }

src/api/scan/checker_test.go

@@ -82,6 +82,7 @@ func (suite *CheckerTestSuite) TestIsScannable() {
 
 	{
 		art := &artifact.Artifact{}
+		art.Type = "IMAGE"
 		art.ManifestMediaType = supportMimeType
 
 		mock.OnAnything(c.artifactCtl, "Walk").Return(nil).Once().Run(func(args mock.Arguments) {

src/pkg/artifact/manager.go

@@ -151,7 +151,7 @@ func (m *manager) assemble(ctx context.Context, art *dao.Artifact) (*Artifact, e
 	artifact.From(art)
 
 	// populate the references
-	if artifact.HasChildren() {
+	if artifact.IsImageIndex() {
 		references, err := m.ListReferences(ctx, q.New(q.KeyWords{"ParentID": artifact.ID}))
 		if err != nil {
 			return nil, err

src/pkg/artifact/model.go

@@ -16,12 +16,12 @@ package artifact
 
 import (
 	"encoding/json"
-	"github.com/goharbor/harbor/src/server/v2.0/models"
 	"time"
 
 	"github.com/docker/distribution/manifest/manifestlist"
 	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/pkg/artifact/dao"
+	"github.com/goharbor/harbor/src/server/v2.0/models"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
@@ -45,8 +45,8 @@ type Artifact struct {
 	References        []*Reference           `json:"references"` // child artifacts referenced by the parent artifact if the artifact is an index
 }
 
-// HasChildren returns true when artifact has children artifacts, most times that means the artifact is Image Index.
+// IsImageIndex returns true when artifact is image index
-func (a *Artifact) HasChildren() bool {
+func (a *Artifact) IsImageIndex() bool {
 	return a.ManifestMediaType == v1.MediaTypeImageIndex ||
 		a.ManifestMediaType == manifestlist.MediaTypeManifestList
 }

src/pkg/artifact/model_test.go

@@ -96,15 +96,15 @@ func (m *modelTestSuite) TestArtifactTo() {
 	assert.Equal(t, `{"anno1":"value1"}`, dbArt.Annotations)
 }
 
-func (m *modelTestSuite) TestHasChildren() {
+func (m *modelTestSuite) TestIsImageIndex() {
 	art1 := Artifact{ManifestMediaType: v1.MediaTypeImageIndex}
-	m.True(art1.HasChildren())
+	m.True(art1.IsImageIndex())
 
 	art2 := Artifact{ManifestMediaType: manifestlist.MediaTypeManifestList}
-	m.True(art2.HasChildren())
+	m.True(art2.IsImageIndex())
 
 	art3 := Artifact{ManifestMediaType: v1.MediaTypeImageManifest}
-	m.False(art3.HasChildren())
+	m.False(art3.IsImageIndex())
 }
 
 func TestModel(t *testing.T) {

src/server/middleware/util.go

@@ -9,15 +9,11 @@ import (
 
 	"github.com/docker/distribution/reference"
 	"github.com/goharbor/harbor/src/api/artifact"
-	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/core/config"
 	"github.com/goharbor/harbor/src/core/promgr"
 	"github.com/goharbor/harbor/src/internal"
-	"github.com/goharbor/harbor/src/pkg/scan/vuln"
-	"github.com/goharbor/harbor/src/pkg/scan/whitelist"
 	"github.com/opencontainers/go-digest"
-	"github.com/pkg/errors"
 )
 
 const (
@@ -74,8 +70,6 @@ func CopyResp(rec *httptest.ResponseRecorder, rw http.ResponseWriter) {
 type PolicyChecker interface {
 	// contentTrustEnabled returns whether a project has enabled content trust.
 	ContentTrustEnabled(name string) bool
-	// vulnerablePolicy  returns whether a project has enabled vulnerable, and the project's severity.
-	VulnerablePolicy(name string) (bool, vuln.Severity, models.CVEWhitelist)
 }
 
 // PmsPolicyChecker ...
@@ -97,38 +91,6 @@ func (pc PmsPolicyChecker) ContentTrustEnabled(name string) bool {
 	return project.ContentTrustEnabled()
 }
 
-// VulnerablePolicy ...
-func (pc PmsPolicyChecker) VulnerablePolicy(name string) (bool, vuln.Severity, models.CVEWhitelist) {
-	project, err := pc.pm.Get(name)
-	wl := models.CVEWhitelist{}
-	if err != nil {
-		log.Errorf("Unexpected error when getting the project, error: %v", err)
-		return true, vuln.Unknown, wl
-	}
-
-	mgr := whitelist.NewDefaultManager()
-	if project.ReuseSysCVEWhitelist() {
-		w, err := mgr.GetSys()
-		if err != nil {
-			log.Error(errors.Wrap(err, "policy checker: vulnerable policy"))
-		} else {
-			wl = *w
-
-			// Use the real project ID
-			wl.ProjectID = project.ProjectID
-		}
-	} else {
-		w, err := mgr.Get(project.ProjectID)
-		if err != nil {
-			log.Error(errors.Wrap(err, "policy checker: vulnerable policy"))
-		} else {
-			wl = *w
-		}
-	}
-
-	return project.VulPrevented(), vuln.ParseSeverityVersion3(project.Severity()), wl
-}
-
 // NewPMSPolicyChecker returns an instance of an pmsPolicyChecker
 func NewPMSPolicyChecker(pm promgr.ProjectManager) PolicyChecker {
 	return &PmsPolicyChecker{

src/server/middleware/vulnerable/controller.go

@@ -0,0 +1,27 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package vulnerable
+
+import (
+	"github.com/goharbor/harbor/src/api/artifact"
+	"github.com/goharbor/harbor/src/api/project"
+	"github.com/goharbor/harbor/src/api/scan"
+)
+
+var (
+	artifactController = artifact.Ctl
+	projectController  = project.Ctl
+	scanController     = scan.DefaultController
+)

src/server/middleware/vulnerable/vulnerable.go

@@ -1,125 +1,145 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 package vulnerable
 
 import (
+	"fmt"
 	"net/http"
-	"net/http/httptest"
 
 	"github.com/goharbor/harbor/src/api/artifact"
 	"github.com/goharbor/harbor/src/api/project"
-	sc "github.com/goharbor/harbor/src/api/scan"
+	"github.com/goharbor/harbor/src/api/scan"
-	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/rbac"
 	"github.com/goharbor/harbor/src/common/security"
 	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/internal"
-	internal_errors "github.com/goharbor/harbor/src/internal/error"
+	ierror "github.com/goharbor/harbor/src/internal/error"
 	"github.com/goharbor/harbor/src/pkg/scan/report"
 	v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
 	"github.com/goharbor/harbor/src/pkg/scan/vuln"
-	serror "github.com/goharbor/harbor/src/server/error"
 	"github.com/goharbor/harbor/src/server/middleware"
-	"github.com/pkg/errors"
 )
 
-// Middleware handle docker pull vulnerable check
+var (
+	scanChecker = func() scan.Checker {
+		return scan.NewChecker()
+	}
+)
+
+// Middleware middleware which does the vulnerability prevention checking for the artifact in GET /v2/<name>/manifests/<reference> API
 func Middleware() func(http.Handler) http.Handler {
-	return func(next http.Handler) http.Handler {
+	return middleware.BeforeRequest(func(r *http.Request) error {
-		return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		ctx := r.Context()
-			doVulCheck, img, projectVulnerableSeverity, wl := validate(req)
+
-			if !doVulCheck {
+		logger := log.G(ctx).WithFields(log.Fields{"middleware": "vulnerable"})
-				next.ServeHTTP(rw, req)
+
-				return
+		none := internal.ArtifactInfo{}
-			}
+		info := internal.GetArtifactInfo(ctx)
-			rec := httptest.NewRecorder()
+		if info == none {
-			next.ServeHTTP(rec, req)
+			return fmt.Errorf("artifactinfo middleware required before this middleware")
-			// only enable vul policy check the response 200
-			if rec.Result().StatusCode == http.StatusOK {
-				// Invalid project ID
-				if wl.ProjectID == 0 {
-					err := errors.Errorf("project verification error: project %s", img.ProjectName)
-					pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION)
-					serror.SendError(rw, pkgE)
-					return
 		}
 
-				ctx := req.Context()
+		art, err := artifactController.GetByReference(ctx, info.Repository, info.Reference, nil)
-				art, err := artifact.Ctl.GetByReference(ctx, img.Repository, img.Digest, nil)
 		if err != nil {
-					// TODO: error handle
+			if !ierror.IsNotFoundErr(err) {
-					return
+				logger.Errorf("get artifact failed, error %v", err)
+			}
+			return err
 		}
 
-				cve := report.CVESet(wl.CVESet())
+		proj, err := projectController.Get(ctx, art.ProjectID, project.CVEWhitelist(true))
-				summaries, err := sc.DefaultController.GetSummary(ctx, art, []string{v1.MimeTypeNativeReport}, report.WithCVEWhitelist(&cve))
-
 		if err != nil {
-					err = errors.Wrap(err, "middleware: vulnerable handler")
+			logger.Errorf("get the project %d failed, error: %v", art.ProjectID, err)
-					pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION)
+			return err
-					serror.SendError(rw, pkgE)
+		}
-					return
+
+		if !proj.VulPrevented() {
+			// vulnerability prevention disabled, skip the checking
+			logger.Debugf("project %s vulnerability prevention disabled, skip the checking", proj.Name)
+			return nil
+		}
+
+		securityCtx, ok := security.FromContext(ctx)
+		if ok &&
+			securityCtx.Name() == "robot" &&
+			securityCtx.Can(rbac.ActionScannerPull, rbac.NewProjectNamespace(proj.ProjectID).Resource(rbac.ResourceRepository)) {
+			// the artifact is pulling by the scanner, skip the checking
+			logger.Debugf("artifact %s@%s is pulling by the scanner, skip the checking", art.RepositoryName, art.Digest)
+			return nil
+		}
+
+		checker := scanChecker()
+
+		scannable, err := checker.IsScannable(ctx, art)
+		if err != nil {
+			logger.Errorf("check the scannable status of the artifact %s@%s failed, error: %v", art.RepositoryName, art.Digest, err)
+			return err
+		}
+
+		if !scannable {
+			// the artifact is not scannable, skip the checking
+			logger.Debugf("artifact %s@%s is not scannable, skip the checking", art.RepositoryName, art.Digest)
+			return nil
+		}
+
+		whitelist := report.CVESet(proj.CVEWhitelist.CVESet())
+		summaries, err := scanController.GetSummary(ctx, art, []string{v1.MimeTypeNativeReport}, report.WithCVEWhitelist(&whitelist))
+		if err != nil {
+			logger.Errorf("get vulnerability summary of the artifact %s@%s failed, error: %v", art.RepositoryName, art.Digest, err)
+			return err
 		}
 
 		rawSummary, ok := summaries[v1.MimeTypeNativeReport]
-				// No report yet?
 		if !ok {
-					err = errors.Errorf("no scan report existing for the artifact: %s:%s@%s", img.Repository, img.Tag, img.Digest)
+			// No report yet?
-					pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION)
+			msg := "vulnerability prevention enabled, but no scan report existing for the artifact"
-					serror.SendError(rw, pkgE)
+			return ierror.New(nil).WithCode(ierror.PROJECTPOLICYVIOLATION).WithMessage(msg)
-					return
 		}
 
-				summary := rawSummary.(*vuln.NativeReportSummary)
+		summary, ok := rawSummary.(*vuln.NativeReportSummary)
+		if !ok {
+			return fmt.Errorf("report summary is invalid")
+		}
+
+		if art.IsImageIndex() {
+			// artifact is image index, skip the checking when it is in the whitelist
+			skippingWhitelist := []string{artifact.ImageType, artifact.CNABType}
+			for _, t := range skippingWhitelist {
+				if art.Type == t {
+					logger.Debugf("artifact %s@%s is image index and its type is %s in skipping whitelist, "+
+						"skip the vulnerability prevention checking", art.RepositoryName, art.Digest, art.Type)
+					return nil
+				}
+			}
+		}
 
 		// Do judgement
-				if summary.Severity.Code() >= projectVulnerableSeverity.Code() {
+		severity := vuln.ParseSeverityVersion3(proj.Severity())
-					err = errors.Errorf("current image with '%q vulnerable' cannot be pulled due to configured policy in 'Prevent images with vulnerability severity of %q from running.' "+
+		if summary.Severity.Code() >= severity.Code() {
-						"Please contact your project administrator for help'", summary.Severity, projectVulnerableSeverity)
+			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.' "+
-					pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION)
+				"Please contact your project administrator for help'", summary.Severity, severity)
-					serror.SendError(rw, pkgE)
+			return ierror.New(nil).WithCode(ierror.PROJECTPOLICYVIOLATION).WithMessage(msg)
-					return
 		}
 
 		// Print scannerPull CVE list
 		if len(summary.CVEBypassed) > 0 {
 			for _, cve := range summary.CVEBypassed {
-						log.Infof("Vulnerable policy check: scannerPull CVE %s", cve)
+				logger.Infof("Vulnerable policy check: scannerPull CVE %s", cve)
 			}
 		}
-			}
+
-			middleware.CopyResp(rec, rw)
+		return nil
 	})
-	}
-}
-
-func validate(req *http.Request) (bool, internal.ArtifactInfo, vuln.Severity, models.CVEWhitelist) {
-	var vs vuln.Severity
-	var wl models.CVEWhitelist
-	var none internal.ArtifactInfo
-	err := middleware.EnsureArtifactDigest(req.Context())
-	if err != nil {
-		return false, none, vs, wl
-	}
-	af := internal.GetArtifactInfo(req.Context())
-	if af == none {
-		return false, af, vs, wl
-	}
-
-	pro, err := project.Ctl.GetByName(req.Context(), af.ProjectName)
-	if err != nil {
-		return false, af, vs, wl
-	}
-	resource := rbac.NewProjectNamespace(pro.ProjectID).Resource(rbac.ResourceRepository)
-	securityCtx, ok := security.FromContext(req.Context())
-	if !ok {
-		return false, af, vs, wl
-	}
-	if !securityCtx.Can(rbac.ActionScannerPull, resource) {
-		return false, af, vs, wl
-	}
-	// Is vulnerable policy set?
-	projectVulnerableEnabled, projectVulnerableSeverity, wl := middleware.GetPolicyChecker().VulnerablePolicy(af.ProjectName)
-	if !projectVulnerableEnabled {
-		return false, af, vs, wl
-	}
-	return true, af, projectVulnerableSeverity, wl
 }

src/server/middleware/vulnerable/vulnerable_test.go

@@ -1 +1,291 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 package vulnerable
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/docker/distribution/manifest/manifestlist"
+	"github.com/goharbor/harbor/src/api/artifact"
+	"github.com/goharbor/harbor/src/api/project"
+	"github.com/goharbor/harbor/src/api/scan"
+	"github.com/goharbor/harbor/src/common/models"
+	"github.com/goharbor/harbor/src/common/security"
+	"github.com/goharbor/harbor/src/internal"
+	v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
+	"github.com/goharbor/harbor/src/pkg/scan/vuln"
+	artifacttesting "github.com/goharbor/harbor/src/testing/api/artifact"
+	projecttesting "github.com/goharbor/harbor/src/testing/api/project"
+	scantesting "github.com/goharbor/harbor/src/testing/api/scan"
+	securitytesting "github.com/goharbor/harbor/src/testing/common/security"
+	"github.com/goharbor/harbor/src/testing/mock"
+	"github.com/stretchr/testify/suite"
+)
+
+type MiddlewareTestSuite struct {
+	suite.Suite
+
+	originalArtifactController artifact.Controller
+	artifactController         *artifacttesting.Controller
+
+	originalProjectController project.Controller
+	projectController         *projecttesting.Controller
+
+	originalScanController scan.Controller
+	scanController         *scantesting.Controller
+
+	checker     *scantesting.Checker
+	scanChecker func() scan.Checker
+
+	artifact *artifact.Artifact
+	project  *models.Project
+
+	next http.Handler
+}
+
+func (suite *MiddlewareTestSuite) SetupTest() {
+	suite.originalArtifactController = artifactController
+	suite.artifactController = &artifacttesting.Controller{}
+	artifactController = suite.artifactController
+
+	suite.originalProjectController = projectController
+	suite.projectController = &projecttesting.Controller{}
+	projectController = suite.projectController
+
+	suite.originalScanController = scanController
+	suite.scanController = &scantesting.Controller{}
+	scanController = suite.scanController
+
+	suite.checker = &scantesting.Checker{}
+	suite.scanChecker = scanChecker
+
+	scanChecker = func() scan.Checker {
+		return suite.checker
+	}
+
+	suite.artifact = &artifact.Artifact{}
+	suite.artifact.Type = artifact.ImageType
+	suite.artifact.ProjectID = 1
+	suite.artifact.RepositoryName = "library/photon"
+	suite.artifact.Digest = "digest"
+
+	suite.project = &models.Project{
+		ProjectID: suite.artifact.ProjectID,
+		Name:      "library",
+		Metadata: map[string]string{
+			models.ProMetaPreventVul: "true",
+			models.ProMetaSeverity:   vuln.High.String(),
+		},
+	}
+
+	suite.next = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	})
+}
+
+func (suite *MiddlewareTestSuite) TearDownTest() {
+	artifactController = suite.originalArtifactController
+	projectController = suite.originalProjectController
+	scanController = suite.originalScanController
+
+	scanChecker = suite.scanChecker
+}
+
+func (suite *MiddlewareTestSuite) makeRequest() *http.Request {
+	req := httptest.NewRequest("GET", "/v1/library/photon/manifests/2.0", nil)
+
+	info := internal.ArtifactInfo{
+		Repository: "library/photon",
+		Reference:  "2.0",
+		Tag:        "2.0",
+		Digest:     "",
+	}
+
+	return req.WithContext(internal.WithArtifactInfo(req.Context(), info))
+}
+
+func (suite *MiddlewareTestSuite) TestGetArtifactFailed() {
+	mock.OnAnything(suite.artifactController, "GetByReference").Return(nil, fmt.Errorf("error"))
+
+	req := suite.makeRequest()
+	rr := httptest.NewRecorder()
+
+	Middleware()(suite.next).ServeHTTP(rr, req)
+	suite.Equal(rr.Code, http.StatusInternalServerError)
+}
+
+func (suite *MiddlewareTestSuite) TestGetProjectFailed() {
+	mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
+	mock.OnAnything(suite.projectController, "Get").Return(nil, fmt.Errorf("err"))
+
+	req := suite.makeRequest()
+	rr := httptest.NewRecorder()
+
+	Middleware()(suite.next).ServeHTTP(rr, req)
+	suite.Equal(rr.Code, http.StatusInternalServerError)
+}
+
+func (suite *MiddlewareTestSuite) TestPreventionDisabled() {
+	mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
+	suite.project.Metadata[models.ProMetaPreventVul] = "false"
+	mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
+
+	req := suite.makeRequest()
+	rr := httptest.NewRecorder()
+
+	Middleware()(suite.next).ServeHTTP(rr, req)
+	suite.Equal(rr.Code, http.StatusOK)
+}
+
+func (suite *MiddlewareTestSuite) TestNonRobotPulling() {
+	mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
+	mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
+	securityCtx := &securitytesting.Context{}
+	mock.OnAnything(securityCtx, "Name").Return("local")
+	mock.OnAnything(securityCtx, "Can").Return(true, nil)
+	mock.OnAnything(suite.checker, "IsScannable").Return(false, nil)
+
+	req := suite.makeRequest()
+	req = req.WithContext(security.NewContext(req.Context(), securityCtx))
+	rr := httptest.NewRecorder()
+
+	Middleware()(suite.next).ServeHTTP(rr, req)
+	suite.Equal(rr.Code, http.StatusOK)
+}
+
+func (suite *MiddlewareTestSuite) TestScannerPulling() {
+	mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
+	mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
+	securityCtx := &securitytesting.Context{}
+	mock.OnAnything(securityCtx, "Name").Return("robot")
+	mock.OnAnything(securityCtx, "Can").Return(true, nil)
+
+	req := suite.makeRequest()
+	req = req.WithContext(security.NewContext(req.Context(), securityCtx))
+	rr := httptest.NewRecorder()
+
+	Middleware()(suite.next).ServeHTTP(rr, req)
+	suite.Equal(rr.Code, http.StatusOK)
+}
+
+func (suite *MiddlewareTestSuite) TestCheckIsScannableFailed() {
+	mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
+	mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
+	mock.OnAnything(suite.checker, "IsScannable").Return(false, fmt.Errorf("error"))
+
+	req := suite.makeRequest()
+	rr := httptest.NewRecorder()
+
+	Middleware()(suite.next).ServeHTTP(rr, req)
+	suite.Equal(rr.Code, http.StatusInternalServerError)
+}
+
+func (suite *MiddlewareTestSuite) TestArtifactIsNotScannable() {
+	mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
+	mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
+	mock.OnAnything(suite.checker, "IsScannable").Return(false, nil)
+
+	req := suite.makeRequest()
+	rr := httptest.NewRecorder()
+
+	Middleware()(suite.next).ServeHTTP(rr, req)
+	suite.Equal(rr.Code, http.StatusOK)
+}
+
+func (suite *MiddlewareTestSuite) TestArtifactNotScanned() {
+	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(nil, 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)
+	mock.OnAnything(suite.checker, "IsScannable").Return(true, nil)
+	mock.OnAnything(suite.scanController, "GetSummary").Return(nil, fmt.Errorf("error"))
+
+	req := suite.makeRequest()
+	rr := httptest.NewRecorder()
+
+	Middleware()(suite.next).ServeHTTP(rr, req)
+	suite.Equal(rr.Code, http.StatusInternalServerError)
+}
+
+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{
+			Severity:    vuln.Low,
+			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) TestPrevented() {
+	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{
+			Severity: vuln.Critical,
+		},
+	}, nil)
+
+	req := suite.makeRequest()
+	rr := httptest.NewRecorder()
+
+	Middleware()(suite.next).ServeHTTP(rr, req)
+	suite.Equal(rr.Code, http.StatusPreconditionFailed)
+}
+
+func (suite *MiddlewareTestSuite) TestArtifactIsImageIndex() {
+	suite.artifact.ManifestMediaType = manifestlist.MediaTypeManifestList
+	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{
+			Severity: vuln.Critical,
+		},
+	}, nil)
+
+	req := suite.makeRequest()
+	rr := httptest.NewRecorder()
+
+	Middleware()(suite.next).ServeHTTP(rr, req)
+	suite.Equal(rr.Code, http.StatusOK)
+}
+
+func TestMiddlewareTestSuite(t *testing.T) {
+	suite.Run(t, &MiddlewareTestSuite{})
+}

src/testing/api/project/controller.go

@@ -7,6 +7,8 @@ import (
 
 	models "github.com/goharbor/harbor/src/common/models"
 	mock "github.com/stretchr/testify/mock"
+
+	project "github.com/goharbor/harbor/src/api/project"
 )
 
 // Controller is an autogenerated mock type for the Controller type
@@ -14,13 +16,20 @@ type Controller struct {
 	mock.Mock
 }
 
-// Get provides a mock function with given fields: ctx, projectID
+// Get provides a mock function with given fields: ctx, projectID, options
-func (_m *Controller) Get(ctx context.Context, projectID int64) (*models.Project, error) {
+func (_m *Controller) Get(ctx context.Context, projectID int64, options ...project.Option) (*models.Project, error) {
-	ret := _m.Called(ctx, projectID)
+	_va := make([]interface{}, len(options))
+	for _i := range options {
+		_va[_i] = options[_i]
+	}
+	var _ca []interface{}
+	_ca = append(_ca, ctx, projectID)
+	_ca = append(_ca, _va...)
+	ret := _m.Called(_ca...)
 
 	var r0 *models.Project
-	if rf, ok := ret.Get(0).(func(context.Context, int64) *models.Project); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, int64, ...project.Option) *models.Project); ok {
-		r0 = rf(ctx, projectID)
+		r0 = rf(ctx, projectID, options...)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(*models.Project)
@@ -28,8 +37,8 @@ func (_m *Controller) Get(ctx context.Context, projectID int64) (*models.Project
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(context.Context, int64) error); ok {
+	if rf, ok := ret.Get(1).(func(context.Context, int64, ...project.Option) error); ok {
-		r1 = rf(ctx, projectID)
+		r1 = rf(ctx, projectID, options...)
 	} else {
 		r1 = ret.Error(1)
 	}
@@ -37,13 +46,20 @@ func (_m *Controller) Get(ctx context.Context, projectID int64) (*models.Project
 	return r0, r1
 }
 
-// GetByName provides a mock function with given fields: ctx, projectName
+// GetByName provides a mock function with given fields: ctx, projectName, options
-func (_m *Controller) GetByName(ctx context.Context, projectName string) (*models.Project, error) {
+func (_m *Controller) GetByName(ctx context.Context, projectName string, options ...project.Option) (*models.Project, error) {
-	ret := _m.Called(ctx, projectName)
+	_va := make([]interface{}, len(options))
+	for _i := range options {
+		_va[_i] = options[_i]
+	}
+	var _ca []interface{}
+	_ca = append(_ca, ctx, projectName)
+	_ca = append(_ca, _va...)
+	ret := _m.Called(_ca...)
 
 	var r0 *models.Project
-	if rf, ok := ret.Get(0).(func(context.Context, string) *models.Project); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, string, ...project.Option) *models.Project); ok {
-		r0 = rf(ctx, projectName)
+		r0 = rf(ctx, projectName, options...)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(*models.Project)
@@ -51,8 +67,8 @@ func (_m *Controller) GetByName(ctx context.Context, projectName string) (*model
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(context.Context, string) error); ok {
+	if rf, ok := ret.Get(1).(func(context.Context, string, ...project.Option) error); ok {
-		r1 = rf(ctx, projectName)
+		r1 = rf(ctx, projectName, options...)
 	} else {
 		r1 = ret.Error(1)
 	}