fix: validate export cve request params (#17341)

1. Validate export cve request params in the API handler
2. Trim space for request in the scan export job

Closes: #17326

Signed-off-by: chlins <chenyuzh@vmware.com>
This commit is contained in:
Chenyu Zhang 2022-08-08 11:07:05 +08:00 committed by GitHub
parent 49d73fa57d
commit 1e13999fff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 114 additions and 20 deletions

View File

@ -285,7 +285,20 @@ func (sde *ScanDataExport) extractCriteria(params job.Parameters) (*export.Reque
if err != nil { if err != nil {
return nil, err return nil, err
} }
return criteria, nil
// sterilize trim spaces for some fields.
sterilize := func(c *export.Request) *export.Request {
if c != nil {
space, empty := " ", ""
c.Repositories = strings.ReplaceAll(c.Repositories, space, empty)
c.Tags = strings.ReplaceAll(c.Tags, space, empty)
c.CVEIds = strings.ReplaceAll(c.CVEIds, space, empty)
}
return c
}
return sterilize(criteria), nil
} }
func (sde *ScanDataExport) calculateFileHash(fileName string) (digest.Digest, error) { func (sde *ScanDataExport) calculateFileHash(fileName string) (digest.Digest, error) {

View File

@ -144,6 +144,25 @@ func (suite *ScanDataExportJobTestSuite) TestRunAttributeUpdateError() {
} }
func (suite *ScanDataExportJobTestSuite) TestExtractCriteria() {
// empty request should return error
_, err := suite.job.extractCriteria(job.Parameters{})
suite.Error(err)
// invalid request should return error
_, err = suite.job.extractCriteria(job.Parameters{"Request": ""})
suite.Error(err)
// valid request should not return error and trim space
c, err := suite.job.extractCriteria(job.Parameters{"Request": map[string]interface{}{
"CVEIds": "CVE-123, CVE-456 ",
"Repositories": " test-repo1 ",
"Tags": "test-tag1, test-tag2",
}})
suite.NoError(err)
suite.Equal("CVE-123,CVE-456", c.CVEIds)
suite.Equal("test-repo1", c.Repositories)
suite.Equal("test-tag1,test-tag2", c.Tags)
}
func (suite *ScanDataExportJobTestSuite) TestRunWithCriteria() { func (suite *ScanDataExportJobTestSuite) TestRunWithCriteria() {
{ {
data := suite.createDataRecords(3, 1) data := suite.createDataRecords(3, 1)

View File

@ -46,19 +46,12 @@ func (se *scanDataExportAPI) Prepare(ctx context.Context, operation string, para
} }
func (se *scanDataExportAPI) ExportScanData(ctx context.Context, params operation.ExportScanDataParams) middleware.Responder { func (se *scanDataExportAPI) ExportScanData(ctx context.Context, params operation.ExportScanDataParams) middleware.Responder {
// validate project id, currently we only support single project // validate the request params
criteria := params.Criteria if err := validateScanExportParams(params); err != nil {
if criteria == nil {
err := errors.New(errors.Errorf("criteria is invalid: %v", criteria)).WithCode(errors.BadRequestCode)
return se.SendError(ctx, err) return se.SendError(ctx, err)
} }
if len(criteria.Projects) != 1 { if err := se.RequireProjectAccess(ctx, params.Criteria.Projects[0], rbac.ActionCreate, rbac.ResourceExportCVE); err != nil {
err := errors.New(errors.Errorf("only support export single project, invalid value: %v", criteria.Projects)).WithCode(errors.BadRequestCode)
return se.SendError(ctx, err)
}
if err := se.RequireProjectAccess(ctx, criteria.Projects[0], rbac.ActionCreate, rbac.ResourceExportCVE); err != nil {
return se.SendError(ctx, err) return se.SendError(ctx, err)
} }
@ -312,3 +305,32 @@ func (se *scanDataExportAPI) requireProjectsAccess(ctx context.Context, pids []i
return nil return nil
} }
// validateScanExportParams validates scan data export request parameters by
// following policies.
// rules:
// 1. the criteria should not be empty
// 2. currently only the export of single project is open
// 3. do not allow to input space in the repo/tag/cve_id (space will lead to misjudge for doublestar filter)
func validateScanExportParams(params operation.ExportScanDataParams) error {
criteria := params.Criteria
if criteria == nil {
return errors.BadRequestError(errors.Errorf("criteria is invalid: %v", criteria))
}
// validate project id, currently we only support single project
if len(criteria.Projects) != 1 {
return errors.BadRequestError(errors.Errorf("only support export single project, invalid value: %v", criteria.Projects))
}
// check spaces
space := " "
inspectList := []string{criteria.Repositories, criteria.Tags, criteria.CVEIds}
for _, s := range inspectList {
if strings.Contains(s, space) {
return errors.BadRequestError(errors.Errorf("invalid criteria value, please remove additional spaces for input: %s", s))
}
}
return nil
}

View File

@ -3,7 +3,6 @@ package handler
import ( import (
"bytes" "bytes"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"io" "io"
"net/http" "net/http"
@ -13,6 +12,7 @@ import (
"time" "time"
beegoorm "github.com/beego/beego/orm" beegoorm "github.com/beego/beego/orm"
"github.com/goharbor/harbor/src/lib/errors"
testifymock "github.com/stretchr/testify/mock" testifymock "github.com/stretchr/testify/mock"
"github.com/stretchr/testify/suite" "github.com/stretchr/testify/suite"
@ -21,6 +21,7 @@ import (
v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
"github.com/goharbor/harbor/src/server/v2.0/models" "github.com/goharbor/harbor/src/server/v2.0/models"
"github.com/goharbor/harbor/src/server/v2.0/restapi" "github.com/goharbor/harbor/src/server/v2.0/restapi"
operation "github.com/goharbor/harbor/src/server/v2.0/restapi/operations/scan_data_export"
"github.com/goharbor/harbor/src/testing/controller/scandataexport" "github.com/goharbor/harbor/src/testing/controller/scandataexport"
"github.com/goharbor/harbor/src/testing/mock" "github.com/goharbor/harbor/src/testing/mock"
systemartifacttesting "github.com/goharbor/harbor/src/testing/pkg/systemartifact" systemartifacttesting "github.com/goharbor/harbor/src/testing/pkg/systemartifact"
@ -61,7 +62,7 @@ func (suite *ScanExportTestSuite) TestAuthorization() {
Labels: []int64{100}, Labels: []int64{100},
Projects: []int64{200}, Projects: []int64{200},
Repositories: "test-repo", Repositories: "test-repo",
Tags: "{test-tag1, test-tag2}", Tags: "{test-tag1,test-tag2}",
} }
reqs := []struct { reqs := []struct {
@ -92,6 +93,45 @@ func (suite *ScanExportTestSuite) TestAuthorization() {
} }
} }
} }
func (suite *ScanExportTestSuite) TestValidateScanExportParams() {
// empty criteria should return error
err := validateScanExportParams(operation.ExportScanDataParams{})
suite.Error(err)
suite.True(errors.IsErr(err, errors.BadRequestCode))
// multiple projects in input should return error
criteria := models.ScanDataExportRequest{
Projects: []int64{200, 300},
}
err = validateScanExportParams(operation.ExportScanDataParams{Criteria: &criteria})
suite.Error(err)
suite.True(errors.IsErr(err, errors.BadRequestCode))
// spaces in input should return error
criteria = models.ScanDataExportRequest{
CVEIds: "CVE-123, CVE-456",
Labels: []int64{100},
Projects: []int64{200},
Repositories: "test-repo1, test-repo2",
Tags: "{test-tag1, test-tag2}",
}
err = validateScanExportParams(operation.ExportScanDataParams{Criteria: &criteria})
suite.Error(err)
suite.True(errors.IsErr(err, errors.BadRequestCode))
// valid params should pass validator
criteria = models.ScanDataExportRequest{
CVEIds: "CVE-123,CVE-456",
Labels: []int64{100},
Projects: []int64{200},
Repositories: "test-repo1,test-repo2",
Tags: "{test-tag1,test-tag2}",
}
err = validateScanExportParams(operation.ExportScanDataParams{Criteria: &criteria})
suite.NoError(err)
}
func (suite *ScanExportTestSuite) TestExportScanData() { func (suite *ScanExportTestSuite) TestExportScanData() {
suite.Security.On("GetUsername").Return("test-user") suite.Security.On("GetUsername").Return("test-user")
suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Once() suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Once()
@ -107,7 +147,7 @@ func (suite *ScanExportTestSuite) TestExportScanData() {
Labels: []int64{100}, Labels: []int64{100},
Projects: []int64{200}, Projects: []int64{200},
Repositories: "test-repo", Repositories: "test-repo",
Tags: "{test-tag1, test-tag2}", Tags: "{test-tag1,test-tag2}",
} }
data, err := json.Marshal(criteria) data, err := json.Marshal(criteria)
@ -128,7 +168,7 @@ func (suite *ScanExportTestSuite) TestExportScanData() {
// validate job name and user name set in the request for job execution // validate job name and user name set in the request for job execution
jobRequestMatcher := testifymock.MatchedBy(func(req export.Request) bool { jobRequestMatcher := testifymock.MatchedBy(func(req export.Request) bool {
return req.UserName == "test-user" && req.JobName == "test-job" && req.Tags == "{test-tag1, test-tag2}" && req.UserID == 1000 return req.UserName == "test-user" && req.JobName == "test-job" && req.Tags == "{test-tag1,test-tag2}" && req.UserID == 1000
}) })
suite.scanExportCtl.AssertCalled(suite.T(), "Start", mock.Anything, jobRequestMatcher) suite.scanExportCtl.AssertCalled(suite.T(), "Start", mock.Anything, jobRequestMatcher)
} }
@ -144,7 +184,7 @@ func (suite *ScanExportTestSuite) TestExportScanData() {
Labels: []int64{100}, Labels: []int64{100},
Projects: []int64{200}, Projects: []int64{200},
Repositories: "test-repo", Repositories: "test-repo",
Tags: "{test-tag1, test-tag2}", Tags: "{test-tag1,test-tag2}",
} }
data, err := json.Marshal(criteria) data, err := json.Marshal(criteria)
@ -170,7 +210,7 @@ func (suite *ScanExportTestSuite) TestExportScanData() {
Labels: []int64{100}, Labels: []int64{100},
Projects: []int64{200, 300}, Projects: []int64{200, 300},
Repositories: "test-repo", Repositories: "test-repo",
Tags: "{test-tag1, test-tag2}", Tags: "{test-tag1,test-tag2}",
} }
data, err := json.Marshal(criteria) data, err := json.Marshal(criteria)
@ -201,7 +241,7 @@ func (suite *ScanExportTestSuite) TestExportScanDataGetUserIdError() {
Labels: []int64{100}, Labels: []int64{100},
Projects: []int64{200}, Projects: []int64{200},
Repositories: "test-repo", Repositories: "test-repo",
Tags: "{test-tag1, test-tag2}", Tags: "{test-tag1,test-tag2}",
} }
data, err := json.Marshal(criteria) data, err := json.Marshal(criteria)
@ -234,7 +274,7 @@ func (suite *ScanExportTestSuite) TestExportScanDataGetUserIdNotFound() {
Labels: []int64{100}, Labels: []int64{100},
Projects: []int64{200}, Projects: []int64{200},
Repositories: "test-repo", Repositories: "test-repo",
Tags: "{test-tag1, test-tag2}", Tags: "{test-tag1,test-tag2}",
} }
data, err := json.Marshal(criteria) data, err := json.Marshal(criteria)
@ -264,7 +304,7 @@ func (suite *ScanExportTestSuite) TestExportScanDataNoPrivileges() {
Labels: []int64{100}, Labels: []int64{100},
Projects: []int64{200}, Projects: []int64{200},
Repositories: "test-repo", Repositories: "test-repo",
Tags: "{test-tag1, test-tag2}", Tags: "{test-tag1,test-tag2}",
} }
data, err := json.Marshal(criteria) data, err := json.Marshal(criteria)