[cherry-pick] Fix scan log mismatch issue (#17241)

Fix scan log mismatch issue

1, add check in label
2, check robot account update
3, validate scan log

Signed-off-by: Wang Yan <wangyan@vmware.com>

Co-authored-by: stonezdj(Daojun Zhang) <stonezdj@gmail.com>

src/common/rbac/project/rbac_role.go

@@ -80,6 +80,7 @@ var (
 
 			{Resource: rbac.ResourceHelmChartVersionLabel, Action: rbac.ActionCreate},
 			{Resource: rbac.ResourceHelmChartVersionLabel, Action: rbac.ActionDelete},
+			{Resource: rbac.ResourceHelmChartVersionLabel, Action: rbac.ActionList},
 
 			{Resource: rbac.ResourceConfiguration, Action: rbac.ActionRead},
 			{Resource: rbac.ResourceConfiguration, Action: rbac.ActionUpdate},
@@ -176,6 +177,7 @@ var (
 
 			{Resource: rbac.ResourceHelmChartVersionLabel, Action: rbac.ActionCreate},
 			{Resource: rbac.ResourceHelmChartVersionLabel, Action: rbac.ActionDelete},
+			{Resource: rbac.ResourceHelmChartVersionLabel, Action: rbac.ActionList},
 
 			{Resource: rbac.ResourceConfiguration, Action: rbac.ActionRead},
 
@@ -234,6 +236,7 @@ var (
 
 			{Resource: rbac.ResourceHelmChartVersionLabel, Action: rbac.ActionCreate},
 			{Resource: rbac.ResourceHelmChartVersionLabel, Action: rbac.ActionDelete},
+			{Resource: rbac.ResourceHelmChartVersionLabel, Action: rbac.ActionList},
 
 			{Resource: rbac.ResourceConfiguration, Action: rbac.ActionRead},
 

src/controller/scan/base_controller.go

@@ -619,11 +619,23 @@ func (bc *basicController) GetSummary(ctx context.Context, artifact *ar.Artifact
 }
 
 // GetScanLog ...
-func (bc *basicController) GetScanLog(ctx context.Context, uuid string) ([]byte, error) {
+func (bc *basicController) GetScanLog(ctx context.Context, artifact *ar.Artifact, uuid string) ([]byte, error) {
 	if len(uuid) == 0 {
 		return nil, errors.New("empty uuid to get scan log")
 	}
+	r, err := bc.sc.GetRegistrationByProject(ctx, artifact.ProjectID)
+	if err != nil {
+		return nil, err
+	}
 
+	artifacts, _, err := bc.collectScanningArtifacts(ctx, r, artifact)
+	if err != nil {
+		return nil, err
+	}
+	artifactMap := map[int64]interface{}{}
+	for _, a := range artifacts {
+		artifactMap[a.ID] = struct{}{}
+	}
 	reportUUIDs := vuln.ParseReportIDs(uuid)
 	tasks, err := bc.listScanTasks(ctx, reportUUIDs)
 	if err != nil {
@@ -635,9 +647,12 @@ func (bc *basicController) GetScanLog(ctx context.Context, uuid string) ([]byte,
 	}
 
 	reportUUIDToTasks := map[string]*task.Task{}
-	for _, task := range tasks {
+	for _, t := range tasks {
-		for _, reportUUID := range getReportUUIDs(task.ExtraAttrs) {
+		if !scanTaskForArtifacts(t, artifactMap) {
-			reportUUIDToTasks[reportUUID] = task
+			return nil, errors.NotFoundError(nil).WithMessage("scan log with uuid: %s not found", uuid)
+		}
+		for _, reportUUID := range getReportUUIDs(t.ExtraAttrs) {
+			reportUUIDToTasks[reportUUID] = t
 		}
 	}
 
@@ -705,6 +720,18 @@ func (bc *basicController) GetScanLog(ctx context.Context, uuid string) ([]byte,
 	return b.Bytes(), nil
 }
 
+func scanTaskForArtifacts(task *task.Task, artifactMap map[int64]interface{}) bool {
+	if task == nil {
+		return false
+	}
+	artifactID := int64(task.GetNumFromExtraAttrs(artifactIDKey))
+	if artifactID == 0 {
+		return false
+	}
+	_, exist := artifactMap[artifactID]
+	return exist
+}
+
 // DeleteReports ...
 func (bc *basicController) DeleteReports(ctx context.Context, digests ...string) error {
 	if err := bc.manager.DeleteByDigests(ctx, digests...); err != nil {

src/controller/scan/base_controller_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/goharbor/harbor/src/lib/config"
 	"github.com/goharbor/harbor/src/lib/orm"
 	"github.com/goharbor/harbor/src/lib/q"
+	art "github.com/goharbor/harbor/src/pkg/artifact"
 	_ "github.com/goharbor/harbor/src/pkg/config/db"
 	_ "github.com/goharbor/harbor/src/pkg/config/inmemory"
 	"github.com/goharbor/harbor/src/pkg/permission/types"
@@ -85,7 +86,7 @@ func (suite *ControllerTestSuite) SetupSuite() {
 	suite.artifactCtl = &artifacttesting.Controller{}
 	artifact.Ctl = suite.artifactCtl
 
-	suite.artifact = &artifact.Artifact{}
+	suite.artifact = &artifact.Artifact{Artifact: art.Artifact{ID: 1}}
 	suite.artifact.Type = "IMAGE"
 	suite.artifact.ProjectID = 1
 	suite.artifact.RepositoryName = "library/photon"
@@ -314,7 +315,7 @@ func (suite *ControllerTestSuite) TestScanControllerScan() {
 		}).Once()
 
 		mock.OnAnything(suite.taskMgr, "List").Return([]*task.Task{
-			{ExtraAttrs: suite.makeExtraAttrs("rp-uuid-001"), Status: "Success"},
+			{ExtraAttrs: suite.makeExtraAttrs(int64(1), "rp-uuid-001"), Status: "Success"},
 		}, nil).Once()
 
 		mock.OnAnything(suite.reportMgr, "Delete").Return(nil).Once()
@@ -335,7 +336,7 @@ func (suite *ControllerTestSuite) TestScanControllerScan() {
 		}).Once()
 
 		mock.OnAnything(suite.taskMgr, "List").Return([]*task.Task{
-			{ExtraAttrs: suite.makeExtraAttrs("rp-uuid-001"), Status: "Success"},
+			{ExtraAttrs: suite.makeExtraAttrs(int64(1), "rp-uuid-001"), Status: "Success"},
 		}, nil).Once()
 
 		mock.OnAnything(suite.reportMgr, "Delete").Return(fmt.Errorf("delete failed")).Once()
@@ -351,7 +352,7 @@ func (suite *ControllerTestSuite) TestScanControllerScan() {
 		}).Once()
 
 		mock.OnAnything(suite.taskMgr, "List").Return([]*task.Task{
-			{ExtraAttrs: suite.makeExtraAttrs("rp-uuid-001"), Status: "Running"},
+			{ExtraAttrs: suite.makeExtraAttrs(int64(1), "rp-uuid-001"), Status: "Running"},
 		}, nil).Once()
 
 		suite.Require().Error(suite.c.Scan(context.TODO(), suite.artifact))
@@ -368,7 +369,7 @@ func (suite *ControllerTestSuite) TestScanControllerStop() {
 	{
 		// success
 		mock.OnAnything(suite.execMgr, "List").Return([]*task.Execution{
-			{ExtraAttrs: suite.makeExtraAttrs("rp-uuid-001"), Status: "Running"},
+			{ExtraAttrs: suite.makeExtraAttrs(int64(1), "rp-uuid-001"), Status: "Running"},
 		}, nil).Once()
 		mock.OnAnything(suite.execMgr, "Stop").Return(nil).Once()
 
@@ -405,9 +406,8 @@ func (suite *ControllerTestSuite) TestScanControllerGetReport() {
 	}).Once()
 
 	mock.OnAnything(suite.taskMgr, "List").Return([]*task.Task{
-		{ExtraAttrs: suite.makeExtraAttrs("rp-uuid-001")},
+		{ExtraAttrs: suite.makeExtraAttrs(int64(1), "rp-uuid-001")},
 	}, nil).Once()
-
 	rep, err := suite.c.GetReport(context.TODO(), suite.artifact, []string{v1.MimeTypeNativeReport})
 	require.NoError(suite.T(), err)
 	assert.Equal(suite.T(), 1, len(rep))
@@ -431,13 +431,13 @@ func (suite *ControllerTestSuite) TestScanControllerGetScanLog() {
 	mock.OnAnything(suite.taskMgr, "List").Return([]*task.Task{
 		{
 			ID:         1,
-			ExtraAttrs: suite.makeExtraAttrs("rp-uuid-001"),
+			ExtraAttrs: suite.makeExtraAttrs(int64(1), "rp-uuid-001"),
 		},
 	}, nil).Once()
 
 	mock.OnAnything(suite.taskMgr, "GetLog").Return([]byte("log"), nil).Once()
 
-	bytes, err := suite.c.GetScanLog(context.TODO(), "rp-uuid-001")
+	bytes, err := suite.c.GetScanLog(context.TODO(), &artifact.Artifact{Artifact: art.Artifact{ID: 1, ProjectID: 1}}, "rp-uuid-001")
 	require.NoError(suite.T(), err)
 	assert.Condition(suite.T(), func() (success bool) {
 		success = len(bytes) > 0
@@ -450,23 +450,25 @@ func (suite *ControllerTestSuite) TestScanControllerGetMultiScanLog() {
 	suite.taskMgr.On("List", context.TODO(), q.New(kw1)).Return([]*task.Task{
 		{
 			ID:         1,
-			ExtraAttrs: suite.makeExtraAttrs("rp-uuid-001"),
+			ExtraAttrs: suite.makeExtraAttrs(int64(1), "rp-uuid-001"),
 		},
 	}, nil).Times(4)
-
+	mock.OnAnything(suite.ar, "Walk").Return(nil).Run(func(args mock.Arguments) {
+		walkFn := args.Get(2).(func(*artifact.Artifact) error)
+		walkFn(suite.artifact)
+	})
 	kw2 := q.KeyWords{"extra_attrs.report:rp-uuid-002": "1"}
 	suite.taskMgr.On("List", context.TODO(), q.New(kw2)).Return([]*task.Task{
 		{
 			ID:         2,
-			ExtraAttrs: suite.makeExtraAttrs("rp-uuid-002"),
+			ExtraAttrs: suite.makeExtraAttrs(int64(1), "rp-uuid-002"),
 		},
 	}, nil).Times(4)
-
 	{
 		// Both success
 		mock.OnAnything(suite.taskMgr, "GetLog").Return([]byte("log"), nil).Twice()
 
-		bytes, err := suite.c.GetScanLog(context.TODO(), base64.StdEncoding.EncodeToString([]byte("rp-uuid-001|rp-uuid-002")))
+		bytes, err := suite.c.GetScanLog(context.TODO(), &artifact.Artifact{Artifact: art.Artifact{ID: 1, ProjectID: 1}}, base64.StdEncoding.EncodeToString([]byte("rp-uuid-001|rp-uuid-002")))
 		suite.Nil(err)
 		suite.NotEmpty(bytes)
 		suite.Contains(string(bytes), "Logs of report rp-uuid-001")
@@ -478,7 +480,7 @@ func (suite *ControllerTestSuite) TestScanControllerGetMultiScanLog() {
 		suite.taskMgr.On("GetLog", context.TODO(), int64(1)).Return([]byte("log"), nil).Once()
 		suite.taskMgr.On("GetLog", context.TODO(), int64(2)).Return(nil, fmt.Errorf("failed")).Once()
 
-		bytes, err := suite.c.GetScanLog(context.TODO(), base64.StdEncoding.EncodeToString([]byte("rp-uuid-001|rp-uuid-002")))
+		bytes, err := suite.c.GetScanLog(context.TODO(), &artifact.Artifact{Artifact: art.Artifact{ID: 1, ProjectID: 1}}, base64.StdEncoding.EncodeToString([]byte("rp-uuid-001|rp-uuid-002")))
 		suite.Nil(err)
 		suite.NotEmpty(bytes)
 		suite.NotContains(string(bytes), "Logs of report rp-uuid-001")
@@ -488,7 +490,7 @@ func (suite *ControllerTestSuite) TestScanControllerGetMultiScanLog() {
 		// Both failed
 		mock.OnAnything(suite.taskMgr, "GetLog").Return(nil, fmt.Errorf("failed")).Twice()
 
-		bytes, err := suite.c.GetScanLog(context.TODO(), base64.StdEncoding.EncodeToString([]byte("rp-uuid-001|rp-uuid-002")))
+		bytes, err := suite.c.GetScanLog(context.TODO(), &artifact.Artifact{Artifact: art.Artifact{ID: 1, ProjectID: 1}}, base64.StdEncoding.EncodeToString([]byte("rp-uuid-001|rp-uuid-002")))
 		suite.Error(err)
 		suite.Empty(bytes)
 	}
@@ -497,7 +499,7 @@ func (suite *ControllerTestSuite) TestScanControllerGetMultiScanLog() {
 		// Both empty
 		mock.OnAnything(suite.taskMgr, "GetLog").Return(nil, nil).Twice()
 
-		bytes, err := suite.c.GetScanLog(context.TODO(), base64.StdEncoding.EncodeToString([]byte("rp-uuid-001|rp-uuid-002")))
+		bytes, err := suite.c.GetScanLog(context.TODO(), &artifact.Artifact{Artifact: art.Artifact{ID: 1, ProjectID: 1}}, base64.StdEncoding.EncodeToString([]byte("rp-uuid-001|rp-uuid-002")))
 		suite.Nil(err)
 		suite.Empty(bytes)
 	}
@@ -565,11 +567,12 @@ func (suite *ControllerTestSuite) TestDeleteReports() {
 	suite.Error(suite.c.DeleteReports(context.TODO(), "digest"))
 }
 
-func (suite *ControllerTestSuite) makeExtraAttrs(reportUUIDs ...string) map[string]interface{} {
+func (suite *ControllerTestSuite) makeExtraAttrs(artifactID int64, reportUUIDs ...string) map[string]interface{} {
 	b, _ := json.Marshal(map[string]interface{}{reportUUIDsKey: reportUUIDs})
 
 	extraAttrs := map[string]interface{}{}
 	json.Unmarshal(b, &extraAttrs)
+	extraAttrs[artifactIDKey] = float64(artifactID)
 
 	return extraAttrs
 }

src/controller/scan/controller.go

@@ -93,7 +93,7 @@ type Controller interface {
 	//   Returns:
 	//     []byte : the log text stream
 	//     error  : non nil error if any errors occurred
-	GetScanLog(ctx context.Context, uuid string) ([]byte, error)
+	GetScanLog(ctx context.Context, art *artifact.Artifact, uuid string) ([]byte, error)
 
 	// Delete the reports related with the specified digests
 	//

src/core/api/chart_label.go

@@ -107,5 +107,8 @@ func (cla *ChartLabelAPI) RemoveLabel() {
 
 // GetLabels gets labels for the specified chart version.
 func (cla *ChartLabelAPI) GetLabels() {
+	if !cla.requireAccess(rbac.ActionList) {
+		return
+	}
 	cla.getLabelsOfResource(common.ResourceTypeChart, cla.chartFullName)
 }

src/server/v2.0/handler/artifact.go

@@ -25,6 +25,7 @@ import (
 	"github.com/docker/distribution/reference"
 	"github.com/go-openapi/runtime"
 	"github.com/go-openapi/runtime/middleware"
+	"github.com/goharbor/harbor/src/common"
 	"github.com/goharbor/harbor/src/common/rbac"
 	"github.com/goharbor/harbor/src/common/utils"
 	"github.com/goharbor/harbor/src/controller/artifact"
@@ -35,6 +36,7 @@ import (
 	"github.com/goharbor/harbor/src/controller/tag"
 	"github.com/goharbor/harbor/src/lib"
 	"github.com/goharbor/harbor/src/lib/errors"
+	"github.com/goharbor/harbor/src/pkg/label"
 	"github.com/goharbor/harbor/src/pkg/notification"
 	"github.com/goharbor/harbor/src/pkg/scan/report"
 	"github.com/goharbor/harbor/src/server/v2.0/handler/assembler"
@@ -46,21 +48,23 @@ import (
 
 func newArtifactAPI() *artifactAPI {
 	return &artifactAPI{
-		artCtl:  artifact.Ctl,
+		artCtl:   artifact.Ctl,
-		proCtl:  project.Ctl,
+		proCtl:   project.Ctl,
-		repoCtl: repository.Ctl,
+		repoCtl:  repository.Ctl,
-		scanCtl: scan.DefaultController,
+		scanCtl:  scan.DefaultController,
-		tagCtl:  tag.Ctl,
+		tagCtl:   tag.Ctl,
+		labelMgr: label.Mgr,
 	}
 }
 
 type artifactAPI struct {
 	BaseAPI
-	artCtl  artifact.Controller
+	artCtl   artifact.Controller
-	proCtl  project.Controller
+	proCtl   project.Controller
-	repoCtl repository.Controller
+	repoCtl  repository.Controller
-	scanCtl scan.Controller
+	scanCtl  scan.Controller
-	tagCtl  tag.Controller
+	tagCtl   tag.Controller
+	labelMgr label.Manager
 }
 
 func (a *artifactAPI) Prepare(ctx context.Context, operation string, params interface{}) middleware.Responder {
@@ -397,7 +401,14 @@ func (a *artifactAPI) GetAddition(ctx context.Context, params operation.GetAddit
 }
 
 func (a *artifactAPI) AddLabel(ctx context.Context, params operation.AddLabelParams) middleware.Responder {
-	if err := a.RequireProjectAccess(ctx, params.ProjectName, rbac.ActionCreate, rbac.ResourceArtifactLabel); err != nil {
+	projectID, err := getProjectID(ctx, params.ProjectName)
+	if err != nil {
+		return a.SendError(ctx, err)
+	}
+	if err := a.RequireProjectAccess(ctx, projectID, rbac.ActionCreate, rbac.ResourceArtifactLabel); err != nil {
+		return a.SendError(ctx, err)
+	}
+	if err := a.RequireLabelInProject(ctx, projectID, params.Label.ID); err != nil {
 		return a.SendError(ctx, err)
 	}
 	art, err := a.artCtl.GetByReference(ctx, fmt.Sprintf("%s/%s", params.ProjectName, params.RepositoryName), params.Reference, nil)
@@ -424,6 +435,17 @@ func (a *artifactAPI) RemoveLabel(ctx context.Context, params operation.RemoveLa
 	return operation.NewRemoveLabelOK()
 }
 
+func (a *artifactAPI) RequireLabelInProject(ctx context.Context, projectID, labelID int64) error {
+	l, err := a.labelMgr.Get(ctx, labelID)
+	if err != nil {
+		return err
+	}
+	if l.Scope == common.LabelScopeProject && l.ProjectID != projectID {
+		return errors.NotFoundError(nil).WithMessage("project id %d, label %d not found", projectID, labelID)
+	}
+	return nil
+}
+
 func option(withTag, withImmutableStatus, withLabel, withSignature *bool) *artifact.Option {
 	option := &artifact.Option{
 		WithTag:   true, // return the tag by default

src/server/v2.0/handler/robot.go

@@ -1,3 +1,17 @@
+// 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 handler
 
 import (
@@ -92,7 +106,7 @@ func (rAPI *robotAPI) DeleteRobot(ctx context.Context, params operation.DeleteRo
 	}
 
 	if err := rAPI.robotCtl.Delete(ctx, params.RobotID); err != nil {
-		// for the version 1 robot account, has to ignore the no permissions error.
+		// for the version 1 robot account, has to ignore the no permission error.
 		if !r.Editable && errors.IsNotFoundErr(err) {
 			return operation.NewDeleteRobotOK()
 		}
@@ -284,7 +298,14 @@ func (rAPI *robotAPI) updateV2Robot(ctx context.Context, params operation.Update
 		return err
 	}
 
-	if err := rAPI.requireAccess(ctx, params.Robot.Level, params.Robot.Permissions[0].Namespace, rbac.ActionUpdate); err != nil {
+	projectID, err := getProjectID(ctx, params.Robot.Permissions[0].Namespace)
+	if err != nil {
+		return err
+	}
+	if r.Level != robot.LEVELSYSTEM && r.ProjectID != projectID {
+		return errors.BadRequestError(nil).WithMessage("cannot update the project id of robot")
+	}
+	if err := rAPI.requireAccess(ctx, params.Robot.Level, projectID, rbac.ActionUpdate); err != nil {
 		return err
 	}
 
@@ -329,7 +350,7 @@ func isValidDuration(d int64) bool {
 func isValidSec(sec string) bool {
 	hasLower := regexp.MustCompile(`[a-z]`)
 	hasUpper := regexp.MustCompile(`[A-Z]`)
-	hasNumber := regexp.MustCompile(`[0-9]`)
+	hasNumber := regexp.MustCompile(`\d`)
 	if len(sec) >= 8 && hasLower.MatchString(sec) && hasUpper.MatchString(sec) && hasNumber.MatchString(sec) {
 		return true
 	}

src/server/v2.0/handler/scan.go

@@ -93,14 +93,13 @@ func (s *scanAPI) GetReportLog(ctx context.Context, params operation.GetReportLo
 	if err := s.RequireProjectAccess(ctx, params.ProjectName, rbac.ActionRead, rbac.ResourceScan); err != nil {
 		return s.SendError(ctx, err)
 	}
-
 	repository := fmt.Sprintf("%s/%s", params.ProjectName, params.RepositoryName)
-	_, err := s.artCtl.GetByReference(ctx, repository, params.Reference, nil)
+	a, err := s.artCtl.GetByReference(ctx, repository, params.Reference, nil)
 	if err != nil {
 		return s.SendError(ctx, err)
 	}
 
-	bytes, err := s.scanCtl.GetScanLog(ctx, params.ReportID)
+	bytes, err := s.scanCtl.GetScanLog(ctx, a, params.ReportID)
 	if err != nil {
 		return s.SendError(ctx, err)
 	}

src/testing/controller/scan/controller.go

@@ -65,13 +65,13 @@ func (_m *Controller) GetReport(ctx context.Context, _a1 *artifact.Artifact, mim
 	return r0, r1
 }
 
-// GetScanLog provides a mock function with given fields: ctx, uuid
+// GetScanLog provides a mock function with given fields: ctx, art, uuid
-func (_m *Controller) GetScanLog(ctx context.Context, uuid string) ([]byte, error) {
+func (_m *Controller) GetScanLog(ctx context.Context, art *artifact.Artifact, uuid string) ([]byte, error) {
-	ret := _m.Called(ctx, uuid)
+	ret := _m.Called(ctx, art, uuid)
 
 	var r0 []byte
-	if rf, ok := ret.Get(0).(func(context.Context, string) []byte); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, *artifact.Artifact, string) []byte); ok {
-		r0 = rf(ctx, uuid)
+		r0 = rf(ctx, art, uuid)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).([]byte)
@@ -79,8 +79,8 @@ func (_m *Controller) GetScanLog(ctx context.Context, uuid string) ([]byte, erro
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(context.Context, string) error); ok {
+	if rf, ok := ret.Get(1).(func(context.Context, *artifact.Artifact, string) error); ok {
-		r1 = rf(ctx, uuid)
+		r1 = rf(ctx, art, uuid)
 	} else {
 		r1 = ret.Error(1)
 	}