Skip scan in-toto sbom artifact (#20415)

fixes #20337

Signed-off-by: stonezdj <stone.zhang@broadcom.com>
Co-authored-by: Wang Yan <wangyan@vmware.com>

src/controller/artifact/controller.go

@@ -59,6 +59,9 @@ import (
 var (
 	// Ctl is a global artifact controller instance
 	Ctl                 = NewController()
+	skippedContentTypes = map[string]struct{}{
+		"application/vnd.in-toto+json": {},
+	}
 )
 
 var (
@@ -113,6 +116,8 @@ type Controller interface {
 	RemoveLabel(ctx context.Context, artifactID int64, labelID int64) (err error)
 	// Walk walks the artifact tree rooted at root, calling walkFn for each artifact in the tree, including root.
 	Walk(ctx context.Context, root *Artifact, walkFn func(*Artifact) error, option *Option) error
+	// HasUnscannableLayer check artifact with digest if has unscannable layer
+	HasUnscannableLayer(ctx context.Context, dgst string) (bool, error)
 }
 
 // NewController creates an instance of the default artifact controller
@@ -759,3 +764,21 @@ func (c *controller) populateAccessories(ctx context.Context, art *Artifact) {
 	}
 	art.Accessories = accs
 }
+
+// HasUnscannableLayer check if it is a in-toto sbom, if it contains any blob with a content_type is application/vnd.in-toto+json, then consider as in-toto sbom
+func (c *controller) HasUnscannableLayer(ctx context.Context, dgst string) (bool, error) {
+	if len(dgst) == 0 {
+		return false, nil
+	}
+	blobs, err := c.blobMgr.GetByArt(ctx, dgst)
+	if err != nil {
+		return false, err
+	}
+	for _, b := range blobs {
+		if _, exist := skippedContentTypes[b.ContentType]; exist {
+			log.Debugf("the artifact with digest %v is unscannable, because it contains content type: %v", dgst, b.ContentType)
+			return true, nil
+		}
+	}
+	return false, nil
+}

src/controller/artifact/controller_test.go

@@ -35,6 +35,7 @@ import (
 	accessorymodel "github.com/goharbor/harbor/src/pkg/accessory/model"
 	basemodel "github.com/goharbor/harbor/src/pkg/accessory/model/base"
 	"github.com/goharbor/harbor/src/pkg/artifact"
+	"github.com/goharbor/harbor/src/pkg/blob/models"
 	"github.com/goharbor/harbor/src/pkg/label/model"
 	repomodel "github.com/goharbor/harbor/src/pkg/repository/model"
 	model_tag "github.com/goharbor/harbor/src/pkg/tag/model/tag"
@@ -678,6 +679,29 @@ func (c *controllerTestSuite) TestWalk() {
 	}
 }
 
+func (c *controllerTestSuite) TestIsIntoto() {
+	blobs := []*models.Blob{
+		{Digest: "sha256:00000", ContentType: "application/vnd.oci.image.manifest.v1+json"},
+		{Digest: "sha256:22222", ContentType: "application/vnd.oci.image.config.v1+json"},
+		{Digest: "sha256:11111", ContentType: "application/vnd.in-toto+json"},
+	}
+	c.blobMgr.On("GetByArt", mock.Anything, mock.Anything).Return(blobs, nil).Once()
+	isIntoto, err := c.ctl.HasUnscannableLayer(context.Background(), "sha256: 77777")
+	c.Nil(err)
+	c.True(isIntoto)
+
+	blobs2 := []*models.Blob{
+		{Digest: "sha256:00000", ContentType: "application/vnd.oci.image.manifest.v1+json"},
+		{Digest: "sha256:22222", ContentType: "application/vnd.oci.image.config.v1+json"},
+		{Digest: "sha256:11111", ContentType: "application/vnd.oci.image.layer.v1.tar+gzip"},
+	}
+
+	c.blobMgr.On("GetByArt", mock.Anything, mock.Anything).Return(blobs2, nil).Once()
+	isIntoto2, err := c.ctl.HasUnscannableLayer(context.Background(), "sha256: 8888")
+	c.Nil(err)
+	c.False(isIntoto2)
+}
+
 func TestControllerTestSuite(t *testing.T) {
 	suite.Run(t, &controllerTestSuite{})
 }

src/controller/scan/base_controller.go

@@ -52,7 +52,6 @@ import (
 	sbomModel "github.com/goharbor/harbor/src/pkg/scan/sbom/model"
 	"github.com/goharbor/harbor/src/pkg/scan/vuln"
 	"github.com/goharbor/harbor/src/pkg/task"
-	"github.com/goharbor/harbor/src/testing/controller/artifact"
 )
 
 var (
@@ -111,8 +110,6 @@ type basicController struct {
 	rc robot.Controller
 	// Tag controller
 	tagCtl tag.Controller
-	// Artifact controller
-	artCtl artifact.Controller
 	// UUID generator
 	uuid uuidGenerator
 	// Configuration getter func
@@ -199,6 +196,18 @@ func (bc *basicController) collectScanningArtifacts(ctx context.Context, r *scan
 			return nil
 		}
 
+		// because there are lots of in-toto sbom artifacts in dockerhub and replicated to Harbor, they are considered as image type
+		// when scanning these type of sbom artifact, the scanner might assume it is image layer with tgz format, and if scanner read the layer with a stream of tgz,
+		// it fail and close the stream abruptly and cause the pannic in the harbor core log
+		// to avoid pannic, skip scan the in-toto sbom artifact sbom artifact
+		unscannable, err := bc.ar.HasUnscannableLayer(ctx, a.Digest)
+		if err != nil {
+			return err
+		}
+		if unscannable {
+			return nil
+		}
+
 		supported := hasCapability(r, a)
 
 		if !supported && a.IsImageIndex() {

src/controller/scan/base_controller_test.go

@@ -350,6 +350,7 @@ func (suite *ControllerTestSuite) TestScanControllerScan() {
 	{
 		// artifact not provieded
 		suite.Require().Error(suite.c.Scan(context.TODO(), nil))
+		mock.OnAnything(suite.ar, "HasUnscannableLayer").Return(false, nil).Times(3)
 	}
 
 	{
@@ -448,6 +449,7 @@ func (suite *ControllerTestSuite) TestScanControllerStop() {
 
 // TestScanControllerGetReport ...
 func (suite *ControllerTestSuite) TestScanControllerGetReport() {
+	mock.OnAnything(suite.ar, "HasUnscannableLayer").Return(false, nil).Once()
 	ctx := orm.NewContext(nil, &ormtesting.FakeOrmer{})
 	mock.OnAnything(suite.ar, "Walk").Return(nil).Run(func(args mock.Arguments) {
 		walkFn := args.Get(2).(func(*artifact.Artifact) error)
@@ -466,13 +468,13 @@ func (suite *ControllerTestSuite) TestScanControllerGetReport() {
 // TestScanControllerGetSummary ...
 func (suite *ControllerTestSuite) TestScanControllerGetSummary() {
 	ctx := orm.NewContext(nil, &ormtesting.FakeOrmer{})
+	mock.OnAnything(suite.ar, "HasUnscannableLayer").Return(false, nil).Once()
 	mock.OnAnything(suite.accessoryMgr, "List").Return([]accessoryModel.Accessory{}, nil).Once()
 	mock.OnAnything(suite.ar, "Walk").Return(nil).Run(func(args mock.Arguments) {
 		walkFn := args.Get(2).(func(*artifact.Artifact) error)
 		walkFn(suite.artifact)
 	}).Once()
 	mock.OnAnything(suite.taskMgr, "ListScanTasksByReportUUID").Return(nil, nil).Once()
-
 	sum, err := suite.c.GetSummary(ctx, suite.artifact, []string{v1.MimeTypeNativeReport})
 	require.NoError(suite.T(), err)
 	assert.Equal(suite.T(), 1, len(sum))
@@ -480,6 +482,7 @@ func (suite *ControllerTestSuite) TestScanControllerGetSummary() {
 
 // TestScanControllerGetScanLog ...
 func (suite *ControllerTestSuite) TestScanControllerGetScanLog() {
+	mock.OnAnything(suite.ar, "HasUnscannableLayer").Return(false, nil).Once()
 	ctx := orm.NewContext(nil, &ormtesting.FakeOrmer{})
 	mock.OnAnything(suite.taskMgr, "ListScanTasksByReportUUID").Return([]*task.Task{
 		{
@@ -500,6 +503,7 @@ func (suite *ControllerTestSuite) TestScanControllerGetScanLog() {
 
 func (suite *ControllerTestSuite) TestScanControllerGetMultiScanLog() {
 	ctx := orm.NewContext(nil, &ormtesting.FakeOrmer{})
+	mock.OnAnything(suite.ar, "HasUnscannableLayer").Return(false, nil).Times(4)
 	suite.taskMgr.On("ListScanTasksByReportUUID", ctx, "rp-uuid-001").Return([]*task.Task{
 		{
 			ID:         1,
@@ -562,7 +566,7 @@ func (suite *ControllerTestSuite) TestScanAll() {
 	{
 		// no artifacts found when scan all
 		executionID := int64(1)
-
+		mock.OnAnything(suite.ar, "HasUnscannableLayer").Return(false, nil).Once()
 		suite.execMgr.On(
 			"Create", mock.Anything, "SCAN_ALL", int64(0), "SCHEDULE",
 			mock.Anything).Return(executionID, nil).Once()

src/controller/scan/checker.go

@@ -86,6 +86,18 @@ func (c *checker) IsScannable(ctx context.Context, art *artifact.Artifact) (bool
 			return artifact.ErrBreak
 		}
 
+		// because there are lots of in-toto sbom artifacts in dockerhub and replicated to Harbor, they are considered as image type
+		// when scanning these type of sbom artifact, the scanner might assume it is image layer with tgz format, and if scanner read the layer with a stream of tgz,
+		// it fail and close the stream abruptly and cause the pannic in the harbor core log
+		// to avoid pannic, skip scan the in-toto sbom artifact sbom artifact
+		unscannable, err := c.artifactCtl.HasUnscannableLayer(ctx, a.Digest)
+		if err != nil {
+			return err
+		}
+		if unscannable {
+			return nil
+		}
+
 		return nil
 	}
 

src/controller/scan/checker_test.go

@@ -81,7 +81,7 @@ func (suite *CheckerTestSuite) TestIsScannable() {
 			walkFn := args.Get(2).(func(*artifact.Artifact) error)
 			walkFn(art)
 		})
-
+		mock.OnAnything(c.artifactCtl, "HasUnscannableLayer").Return(false, nil).Once()
 		isScannable, err := c.IsScannable(context.TODO(), art)
 		suite.Nil(err)
 		suite.False(isScannable)
@@ -97,6 +97,7 @@ func (suite *CheckerTestSuite) TestIsScannable() {
 			walkFn := args.Get(2).(func(*artifact.Artifact) error)
 			walkFn(art)
 		})
+		mock.OnAnything(c.artifactCtl, "HasUnscannableLayer").Return(false, nil).Once()
 
 		isScannable, err := c.IsScannable(context.TODO(), art)
 		suite.Nil(err)

src/testing/controller/artifact/controller.go

@@ -238,6 +238,34 @@ func (_m *Controller) GetByReference(ctx context.Context, repository string, ref
 	return r0, r1
 }
 
+// HasUnscannableLayer provides a mock function with given fields: ctx, dgst
+func (_m *Controller) HasUnscannableLayer(ctx context.Context, dgst string) (bool, error) {
+	ret := _m.Called(ctx, dgst)
+
+	if len(ret) == 0 {
+		panic("no return value specified for HasUnscannableLayer")
+	}
+
+	var r0 bool
+	var r1 error
+	if rf, ok := ret.Get(0).(func(context.Context, string) (bool, error)); ok {
+		return rf(ctx, dgst)
+	}
+	if rf, ok := ret.Get(0).(func(context.Context, string) bool); ok {
+		r0 = rf(ctx, dgst)
+	} else {
+		r0 = ret.Get(0).(bool)
+	}
+
+	if rf, ok := ret.Get(1).(func(context.Context, string) error); ok {
+		r1 = rf(ctx, dgst)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // List provides a mock function with given fields: ctx, query, option
 func (_m *Controller) List(ctx context.Context, query *q.Query, option *artifact.Option) ([]*artifact.Artifact, error) {
 	ret := _m.Called(ctx, query, option)