Merge pull request #8713 from reasonerjt/fix-8702

Avoid overwriting system CVE whitelist by mistake

src/common/dao/cve_whitelist.go

@@ -21,6 +21,14 @@ import (
 	"github.com/goharbor/harbor/src/common/utils/log"
 )
 
+// CreateCVEWhitelist creates the CVE whitelist
+func CreateCVEWhitelist(l models.CVEWhitelist) (int64, error) {
+	o := GetOrmer()
+	itemsBytes, _ := json.Marshal(l.Items)
+	l.ItemsText = string(itemsBytes)
+	return o.Insert(&l)
+}
+
 // UpdateCVEWhitelist Updates the vulnerability white list to DB
 func UpdateCVEWhitelist(l models.CVEWhitelist) (int64, error) {
 	o := GetOrmer()
@@ -30,23 +38,6 @@ func UpdateCVEWhitelist(l models.CVEWhitelist) (int64, error) {
 	return id, err
 }
 
-// GetSysCVEWhitelist Gets the system level vulnerability white list from DB
-func GetSysCVEWhitelist() (*models.CVEWhitelist, error) {
-	return GetCVEWhitelist(0)
-}
-
-// UpdateSysCVEWhitelist updates the system level CVE whitelist
-/*
-func UpdateSysCVEWhitelist(l models.CVEWhitelist) error {
-	if l.ProjectID != 0 {
-		return fmt.Errorf("system level CVE whitelist cannot set project ID")
-	}
-	l.ProjectID = -1
-	_, err := UpdateCVEWhitelist(l)
-	return err
-}
-*/
-
 // GetCVEWhitelist Gets the CVE whitelist of the project based on the project ID in parameter
 func GetCVEWhitelist(pid int64) (*models.CVEWhitelist, error) {
 	o := GetOrmer()
@@ -58,8 +49,7 @@ func GetCVEWhitelist(pid int64) (*models.CVEWhitelist, error) {
 		return nil, fmt.Errorf("failed to get CVE whitelist for project %d, error: %v", pid, err)
 	}
 	if len(r) == 0 {
-		log.Infof("No CVE whitelist found for project %d, returning empty list.", pid)
-		return &models.CVEWhitelist{ProjectID: pid, Items: []models.CVEWhitelistItem{}}, nil
+		return nil, nil
 	} else if len(r) > 1 {
 		log.Infof("Multiple CVE whitelists found for project %d, length: %d, returning first element.", pid, len(r))
 	}

src/common/dao/cve_whitelist_test.go

@@ -23,12 +23,9 @@ import (
 
 func TestUpdateAndGetCVEWhitelist(t *testing.T) {
 	require.Nil(t, ClearTable("cve_whitelist"))
-	l, err := GetSysCVEWhitelist()
-	assert.Nil(t, err)
-	assert.Equal(t, models.CVEWhitelist{ProjectID: 0, Items: []models.CVEWhitelistItem{}}, *l)
 	l2, err := GetCVEWhitelist(5)
 	assert.Nil(t, err)
-	assert.Equal(t, models.CVEWhitelist{ProjectID: 5, Items: []models.CVEWhitelistItem{}}, *l2)
+	assert.Nil(t, l2)
 
 	longList := []models.CVEWhitelistItem{}
 	for i := 0; i < 50; i++ {
@@ -46,15 +43,6 @@ func TestUpdateAndGetCVEWhitelist(t *testing.T) {
 	assert.Equal(t, longList, out1.Items)
 	assert.Equal(t, e, *out1.ExpiresAt)
 
-	in2 := models.CVEWhitelist{ProjectID: 3, Items: []models.CVEWhitelistItem{}}
-	_, err = UpdateCVEWhitelist(in2)
-	require.Nil(t, err)
-	// assert.Equal(t, int64(1), n2)
-	out2, err := GetCVEWhitelist(3)
-	require.Nil(t, err)
-	assert.Equal(t, int64(3), out2.ProjectID)
-	assert.Equal(t, []models.CVEWhitelistItem{}, out2.Items)
-
 	sysCVEs := []models.CVEWhitelistItem{
 		{CVEID: "CVE-2019-10164"},
 		{CVEID: "CVE-2017-12345"},
@@ -62,11 +50,6 @@ func TestUpdateAndGetCVEWhitelist(t *testing.T) {
 	in3 := models.CVEWhitelist{Items: sysCVEs}
 	_, err = UpdateCVEWhitelist(in3)
 	require.Nil(t, err)
-	// assert.Equal(t, int64(1), n3)
-	sysList, err := GetSysCVEWhitelist()
-	require.Nil(t, err)
-	assert.Equal(t, int64(0), sysList.ProjectID)
-	assert.Equal(t, sysCVEs, sysList.Items)
 
-	// require.Nil(t, ClearTable("cve_whitelist"))
+	require.Nil(t, ClearTable("cve_whitelist"))
 }

src/core/promgr/promgr.go

@@ -94,7 +94,7 @@ func (d *defaultProjectManager) Create(project *models.Project) (int64, error) {
 		return 0, err
 	}
 	if d.metaMgrEnabled {
-		d.whitelistMgr.CreateEmpty(project.ProjectID)
+		d.whitelistMgr.CreateEmpty(id)
 		if len(project.Metadata) > 0 {
 			if err = d.metaMgr.Add(id, project.Metadata); err != nil {
 				log.Errorf("failed to add metadata for project %s: %v", project.Name, err)

src/pkg/scan/whitelist/manager.go

@@ -17,6 +17,7 @@ package whitelist
 import (
 	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/models"
+	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/jobservice/logger"
 )
 
@@ -41,7 +42,7 @@ func (d *defaultManager) CreateEmpty(projectID int64) error {
 	l := models.CVEWhitelist{
 		ProjectID: projectID,
 	}
-	_, err := dao.UpdateCVEWhitelist(l)
+	_, err := dao.CreateCVEWhitelist(l)
 	if err != nil {
 		logger.Errorf("Failed to create empty CVE whitelist for project: %d, error: %v", projectID, err)
 	}
@@ -60,7 +61,12 @@ func (d *defaultManager) Set(projectID int64, list models.CVEWhitelist) error {
 
 // Get gets the whitelist for given project
 func (d *defaultManager) Get(projectID int64) (*models.CVEWhitelist, error) {
-	return dao.GetCVEWhitelist(projectID)
+	wl, err := dao.GetCVEWhitelist(projectID)
+	if wl == nil && err == nil {
+		log.Debugf("No CVE whitelist found for project %d, returning empty list.", projectID)
+		return &models.CVEWhitelist{ProjectID: projectID, Items: []models.CVEWhitelistItem{}}, nil
+	}
+	return wl, err
 }
 
 // SetSys sets the system level whitelist

src/pkg/scan/whitelist/manager_test.go

@@ -0,0 +1,46 @@
+package whitelist
+
+import (
+	"github.com/goharbor/harbor/src/common/dao"
+	"github.com/goharbor/harbor/src/common/utils/log"
+	"github.com/stretchr/testify/assert"
+	"os"
+	"testing"
+)
+
+func TestMain(m *testing.M) {
+
+	// databases := []string{"mysql", "sqlite"}
+	databases := []string{"postgresql"}
+	for _, database := range databases {
+		log.Infof("run test cases for database: %s", database)
+
+		result := 1
+		switch database {
+		case "postgresql":
+			dao.PrepareTestForPostgresSQL()
+		default:
+			log.Fatalf("invalid database: %s", database)
+		}
+
+		result = m.Run()
+
+		if result != 0 {
+			os.Exit(result)
+		}
+	}
+}
+
+func TestDefaultManager_CreateEmpty(t *testing.T) {
+	dm := NewDefaultManager()
+	assert.NoError(t, dm.CreateEmpty(99))
+	assert.Error(t, dm.CreateEmpty(99))
+}
+
+func TestDefaultManager_Get(t *testing.T) {
+	dm := NewDefaultManager()
+	// return empty list
+	l, err := dm.Get(1234)
+	assert.Nil(t, err)
+	assert.Empty(t, l.Items)
+}