Filter by min severity & no fix version vulnerabilities (#322)

Author: omerzi

commands/createfixpullrequests.go

@@ -54,26 +54,24 @@ func (cfp *CreateFixPullRequestsCmd) scanAndFixRepository(repository *utils.Frog
 	if err != nil {
 		return err
 	}
-	cfp.details = &utils.ScanDetails{
-		XrayGraphScanParams:      createXrayScanParams(repository.Watches, repository.JFrogProjectKey),
-		ServerDetails:            &repository.Server,
-		Git:                      &repository.Git,
-		Client:                   client,
-		FailOnInstallationErrors: *repository.FailOnSecurityIssues,
-		Branch:                   branch,
-		ReleasesRepo:             repository.JfrogReleasesRepo,
-	}
-	for _, project := range repository.Projects {
-		cfp.details.Project = project
-		cfp.aggregateFixes = repository.Git.AggregateFixes
-		projectFullPathWorkingDirs := getFullPathWorkingDirs(project.WorkingDirs, baseWd)
+	cfp.details = utils.NewScanDetails(client, &repository.Server, &repository.Git).
+		SetXrayGraphScanParams(repository.Watches, repository.JFrogProjectKey).
+		SetFailOnInstallationErrors(*repository.FailOnSecurityIssues).
+		SetBranch(branch).
+		SetReleasesRepo(repository.JfrogReleasesRepo).
+		SetFixableOnly(repository.FixableOnly).
+		SetMinSeverity(repository.MinSeverity)
+	cfp.aggregateFixes = repository.Git.AggregateFixes
+	for i := range repository.Projects {
+		cfp.details.Project = &repository.Projects[i]
+		projectFullPathWorkingDirs := getFullPathWorkingDirs(cfp.details.Project.WorkingDirs, baseWd)
 		for _, fullPathWd := range projectFullPathWorkingDirs {
 			scanResults, isMultipleRoots, err := cfp.scan(cfp.details, fullPathWd)
 			if err != nil {
 				return err
 			}
 
-			err = utils.UploadScanToGitProvider(scanResults, repository, cfp.details.Branch, cfp.details.Client, isMultipleRoots)
+			err = utils.UploadScanToGitProvider(scanResults, repository, cfp.details.Branch(), cfp.details.Client(), isMultipleRoots)
 			if err != nil {
 				log.Warn(err)
 			}
@@ -151,16 +149,15 @@ func (cfp *CreateFixPullRequestsCmd) fixIssuesSeparatePRs(fixVersionsMap map[str
 			log.Warn(err)
 		}
 		// After finishing to work on the current vulnerability, we go back to the base branch to start the next vulnerability fix
-		log.Info("Running git checkout to base branch:", cfp.details.Branch)
-		if err = cfp.gitManager.Checkout(cfp.details.Branch); err != nil {
+		log.Info("Running git checkout to base branch:", cfp.details.Branch())
+		if err = cfp.gitManager.Checkout(cfp.details.Branch()); err != nil {
 			return
 		}
 	}
 	return
 }
 
 func (cfp *CreateFixPullRequestsCmd) fixIssuesSinglePR(fixVersionsMap map[string]*utils.FixVersionInfo) (err error) {
-	successfullyFixedPackages := make(map[string]*utils.FixVersionInfo)
 	log.Info("-----------------------------------------------------------------")
 	log.Info("Start aggregated packages fix")
 	aggregatedFixBranchName, err := cfp.gitManager.GenerateAggregatedFixBranchName(fixVersionsMap)
@@ -174,13 +171,10 @@ func (cfp *CreateFixPullRequestsCmd) fixIssuesSinglePR(fixVersionsMap map[string
 	for impactedPackage, fixVersionInfo := range fixVersionsMap {
 		if err = cfp.updatePackageToFixedVersion(impactedPackage, fixVersionInfo); err != nil {
 			log.Error("Could not fix impacted package", impactedPackage, "as part of the PR. Skipping it. Cause:", err.Error())
-		} else {
-			log.Info("Successfully fixed", impactedPackage)
-			successfullyFixedPackages[impactedPackage] = fixVersionInfo
 		}
 	}
 
-	if err = cfp.openAggregatedPullRequest(aggregatedFixBranchName, successfullyFixedPackages); err != nil {
+	if err = cfp.openAggregatedPullRequest(aggregatedFixBranchName); err != nil {
 		return fmt.Errorf("failed while creating aggreagted pull request. Error: \n%s", err.Error())
 	}
 	return
@@ -189,7 +183,7 @@ func (cfp *CreateFixPullRequestsCmd) fixIssuesSinglePR(fixVersionsMap map[string
 func (cfp *CreateFixPullRequestsCmd) fixSinglePackageAndCreatePR(impactedPackage string, fixVersionInfo *utils.FixVersionInfo) (err error) {
 	log.Info("-----------------------------------------------------------------")
 	log.Info("Start fixing", impactedPackage, "with", fixVersionInfo.FixVersion)
-	fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.details.Branch, impactedPackage, fixVersionInfo.FixVersion)
+	fixBranchName, err := cfp.gitManager.GenerateFixBranchName(cfp.details.Branch(), impactedPackage, fixVersionInfo.FixVersion)
 	if err != nil {
 		return
 	}
@@ -231,14 +225,14 @@ func (cfp *CreateFixPullRequestsCmd) openFixingPullRequest(impactedPackage, fixB
 	}
 
 	pullRequestTitle := cfp.gitManager.GeneratePullRequestTitle(impactedPackage, fixVersionInfo.FixVersion)
-	log.Info("Creating Pull Request form:", fixBranchName, " to:", cfp.details.Branch)
+	log.Info("Creating Pull Request form:", fixBranchName, " to:", cfp.details.Branch())
 	prBody := commitMessage + "\n\n" + utils.WhatIsFrogbotMd
-	return cfp.details.Client.CreatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, fixBranchName, cfp.details.Branch, pullRequestTitle, prBody)
+	return cfp.details.Client().CreatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, fixBranchName, cfp.details.Branch(), pullRequestTitle, prBody)
 }
 
 // When aggregate mode is active, there can be only one updated pull request to contain all the available fixes.
 // In case of an already opened pull request, Frogbot will only update the branch.
-func (cfp *CreateFixPullRequestsCmd) openAggregatedPullRequest(fixBranchName string, versionsMap map[string]*utils.FixVersionInfo) (err error) {
+func (cfp *CreateFixPullRequestsCmd) openAggregatedPullRequest(fixBranchName string) (err error) {
 	log.Info("Checking if there are changes to commit")
 	isClean, err := cfp.gitManager.IsClean()
 	if err != nil {
@@ -263,7 +257,7 @@ func (cfp *CreateFixPullRequestsCmd) openAggregatedPullRequest(fixBranchName str
 	if !exists {
 		log.Info("Creating Pull Request form:", fixBranchName, " to:", cfp.details.Branch)
 		prBody := commitMessage + "\n\n" + utils.WhatIsFrogbotMd
-		return cfp.details.Client.CreatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, fixBranchName, cfp.details.Branch, utils.AggregatedPullRequestTitleTemplate, prBody)
+		return cfp.details.Client().CreatePullRequest(context.Background(), cfp.details.RepoOwner, cfp.details.RepoName, fixBranchName, cfp.details.Branch(), utils.AggregatedPullRequestTitleTemplate, prBody)
 	}
 	log.Info("Pull Request branch:", fixBranchName, "has been updated")
 	return
@@ -290,8 +284,7 @@ func (cfp *CreateFixPullRequestsCmd) cloneRepository() (tempWd string, restoreDi
 	log.Debug("Created temp working directory:", tempWd)
 
 	// Clone the content of the repo to the new working directory
-	err = cfp.gitManager.Clone(tempWd, cfp.details.Branch)
-	if err != nil {
+	if err = cfp.gitManager.Clone(tempWd, cfp.details.Branch()); err != nil {
 		return
 	}
 

commands/createfixpullrequests_test.go

@@ -112,7 +112,7 @@ func getGenericFixPackageVersionFunc() FixPackagesTestFunc {
 	return func(test packageFixTest) CreateFixPullRequestsCmd {
 		return CreateFixPullRequestsCmd{
 			details: &utils.ScanDetails{
-				Project: utils.Project{
+				Project: &utils.Project{
 					PipRequirementsFile: test.packageDescriptor,
 					WorkingDirs:         []string{test.testPath},
 				},
@@ -267,8 +267,8 @@ func TestPackageTypeFromScan(t *testing.T) {
 			frogbotParams.Projects[0].InstallCommandName = pkg.commandName
 			frogbotParams.Projects[0].InstallCommandArgs = pkg.commandArgs
 			scanSetup := utils.ScanDetails{
-				XrayGraphScanParams: services.XrayGraphScanParams{},
-				Project:             frogbotParams.Projects[0],
+				XrayGraphScanParams: &services.XrayGraphScanParams{},
+				Project:             &frogbotParams.Projects[0],
 				ServerDetails:       &frogbotParams.Server,
 			}
 			scanResponse, _, err := testScan.scan(&scanSetup, tmpDir)

commands/scanandfixrepos.go

@@ -86,7 +86,7 @@ func (saf *ScanAndFixRepositories) downloadAndRunScanAndFix(repository *utils.Fr
 	return cfp.scanAndFixRepository(repository, branch, client)
 }
 
-func (saf ScanAndFixRepositories) setCommitBuildStatus(client vcsclient.VcsClient, repoConfig *utils.FrogbotRepoConfig, state vcsclient.CommitStatus, commitHash, description string) error {
+func (saf *ScanAndFixRepositories) setCommitBuildStatus(client vcsclient.VcsClient, repoConfig *utils.FrogbotRepoConfig, state vcsclient.CommitStatus, commitHash, description string) error {
 	if err := client.SetCommitStatus(context.Background(), state, repoConfig.RepoOwner, repoConfig.RepoName, commitHash, utils.FrogbotCreatorName, description, utils.CommitStatusDetailsUrl); err != nil {
 		return fmt.Errorf("failed to mark last commit build status due to: %s", err.Error())
 	}
@@ -96,7 +96,7 @@ func (saf ScanAndFixRepositories) setCommitBuildStatus(client vcsclient.VcsClien
 
 // Returns true if the latest commit hasn't been scanned
 // or the time passed from the last scan exceeded the configured value.
-func (saf ScanAndFixRepositories) shouldScanLatestCommit(ctx context.Context, repoConfig *utils.FrogbotRepoConfig, client vcsclient.VcsClient, branch string) (shouldScan bool, commitHash string, err error) {
+func (saf *ScanAndFixRepositories) shouldScanLatestCommit(ctx context.Context, repoConfig *utils.FrogbotRepoConfig, client vcsclient.VcsClient, branch string) (shouldScan bool, commitHash string, err error) {
 	owner := repoConfig.RepoOwner
 	repo := repoConfig.RepoName
 	latestCommit, err := client.GetLatestCommit(ctx, owner, repo, branch)

commands/scanpullrequest.go

@@ -74,20 +74,16 @@ func scanPullRequest(repoConfig *utils.FrogbotRepoConfig, client vcsclient.VcsCl
 }
 
 func auditPullRequest(repoConfig *utils.FrogbotRepoConfig, client vcsclient.VcsClient) ([]formats.VulnerabilityOrViolationRow, error) {
-	xrayScanParams := createXrayScanParams(repoConfig.Watches, repoConfig.JFrogProjectKey)
 	var vulnerabilitiesRows []formats.VulnerabilityOrViolationRow
-	for _, project := range repoConfig.Projects {
-		scanSetup := &utils.ScanDetails{
-			XrayGraphScanParams:      xrayScanParams,
-			Project:                  project,
-			Client:                   client,
-			ServerDetails:            &repoConfig.Server,
-			Git:                      &repoConfig.Git,
-			FailOnInstallationErrors: false,
-			Branch:                   repoConfig.Branches[0],
-			ReleasesRepo:             repoConfig.JfrogReleasesRepo,
-		}
-		currentScan, isMultipleRoot, err := auditSource(scanSetup)
+	for i := range repoConfig.Projects {
+		scanDetails := utils.NewScanDetails(client, &repoConfig.Server, &repoConfig.Git).
+			SetProject(&repoConfig.Projects[i]).
+			SetBranch(repoConfig.Branches[0]).
+			SetReleasesRepo(repoConfig.JfrogReleasesRepo).
+			SetXrayGraphScanParams(repoConfig.Watches, repoConfig.JFrogProjectKey).
+			SetMinSeverity(repoConfig.MinSeverity).
+			SetFixableOnly(repoConfig.FixableOnly)
+		currentScan, isMultipleRoot, err := auditSource(scanDetails)
 		if err != nil {
 			return nil, err
 		}
@@ -101,8 +97,8 @@ func auditPullRequest(repoConfig *utils.FrogbotRepoConfig, client vcsclient.VcsC
 			continue
 		}
 		// Audit target code
-		scanSetup.FailOnInstallationErrors = *repoConfig.FailOnSecurityIssues
-		previousScan, isMultipleRoot, err := auditTarget(scanSetup)
+		scanDetails.SetFailOnInstallationErrors(*repoConfig.FailOnSecurityIssues)
+		previousScan, isMultipleRoot, err := auditTarget(scanDetails)
 		if err != nil {
 			return nil, err
 		}
@@ -200,22 +196,6 @@ func createAllIssuesRows(currentScan []services.ScanResponse, isMultipleRoot boo
 	return getScanVulnerabilitiesRows(violations, vulnerabilities, isMultipleRoot)
 }
 
-func createXrayScanParams(watches []string, project string) (params services.XrayGraphScanParams) {
-	params.ScanType = services.Dependency
-	params.IncludeLicenses = false
-	if len(watches) > 0 {
-		params.Watches = watches
-		return
-	}
-	if project != "" {
-		params.ProjectKey = project
-		return
-	}
-	// No context was supplied, request from Xray to return all known vulnerabilities.
-	params.IncludeVulnerabilities = true
-	return
-}
-
 func auditSource(scanSetup *utils.ScanDetails) ([]services.ScanResponse, bool, error) {
 	wd, err := os.Getwd()
 	if err != nil {
@@ -243,8 +223,8 @@ func getFullPathWorkingDirs(workingDirs []string, baseWd string) []string {
 
 func auditTarget(scanSetup *utils.ScanDetails) (res []services.ScanResponse, isMultipleRoot bool, err error) {
 	// First download the target repo to temp dir
-	log.Info("Auditing ", scanSetup.Git.RepoName, scanSetup.Branch)
-	wd, cleanup, err := utils.DownloadRepoToTempDir(scanSetup.Client, scanSetup.Branch, scanSetup.Git)
+	log.Info("Auditing the", scanSetup.Git.RepoName, "repository on the", scanSetup.Branch(), "branch")
+	wd, cleanup, err := utils.DownloadRepoToTempDir(scanSetup.Client(), scanSetup.Branch(), scanSetup.Git)
 	if err != nil {
 		return
 	}
@@ -272,8 +252,10 @@ func runInstallAndAudit(scanSetup *utils.ScanDetails, workDirs ...string) (resul
 		SetRequirementsFile(scanSetup.PipRequirementsFile).
 		SetWorkingDirs(workDirs).
 		SetDepsRepo(scanSetup.Repository).
-		SetReleasesRepo(scanSetup.ReleasesRepo).
-		SetIgnoreConfigFile(true)
+		SetReleasesRepo(scanSetup.ReleasesRepo()).
+		SetIgnoreConfigFile(true).
+		SetMinSeverityFilter(scanSetup.MinSeverityFilter()).
+		SetFixableOnly(scanSetup.FixableOnly())
 	results, isMultipleRoot, err = audit.GenericAudit(auditParams)
 	if err != nil {
 		return nil, false, err
@@ -294,7 +276,7 @@ func runInstallIfNeeded(scanSetup *utils.ScanDetails, workDir string) (err error
 	}()
 	log.Info(fmt.Sprintf("Executing '%s %s' at %s", scanSetup.InstallCommandName, scanSetup.InstallCommandArgs, workDir))
 	output, err := runInstallCommand(scanSetup)
-	if err != nil && !scanSetup.FailOnInstallationErrors {
+	if err != nil && !scanSetup.FailOnInstallationErrors() {
 		log.Info(installationCmdFailedErr, err.Error(), "\n", string(output))
 		// failOnInstallationErrors set to 'false'
 		err = nil

commands/scanpullrequest_test.go

@@ -35,29 +35,6 @@ const (
 	testProjConfigPathNoFail         = "testdata/config/frogbot-config-test-proj-no-fail.yml"
 )
 
-func TestCreateXrayScanParams(t *testing.T) {
-	// Project
-	params := createXrayScanParams(nil, "")
-	assert.Empty(t, params.Watches)
-	assert.Equal(t, "", params.ProjectKey)
-	assert.True(t, params.IncludeVulnerabilities)
-	assert.False(t, params.IncludeLicenses)
-
-	// Watches
-	params = createXrayScanParams([]string{"watch-1", "watch-2"}, "")
-	assert.Equal(t, []string{"watch-1", "watch-2"}, params.Watches)
-	assert.Equal(t, "", params.ProjectKey)
-	assert.False(t, params.IncludeVulnerabilities)
-	assert.False(t, params.IncludeLicenses)
-
-	// Project
-	params = createXrayScanParams(nil, "project")
-	assert.Empty(t, params.Watches)
-	assert.Equal(t, "project", params.ProjectKey)
-	assert.False(t, params.IncludeVulnerabilities)
-	assert.False(t, params.IncludeLicenses)
-}
-
 func TestCreateVulnerabilitiesRows(t *testing.T) {
 	// Previous scan with only one violation - XRAY-1
 	previousScan := services.ScanResponse{
@@ -412,30 +389,30 @@ func TestCreatePullRequestMessage(t *testing.T) {
 
 func TestRunInstallIfNeeded(t *testing.T) {
 	scanSetup := utils.ScanDetails{
-		Project:                  utils.Project{},
-		FailOnInstallationErrors: true,
+		Project: &utils.Project{},
 	}
+	scanSetup.SetFailOnInstallationErrors(true)
 	assert.NoError(t, runInstallIfNeeded(&scanSetup, ""))
 	tmpDir, err := fileutils.CreateTempDir()
 	assert.NoError(t, err)
 	params := &utils.Project{
 		InstallCommandName: "echo",
 		InstallCommandArgs: []string{"Hello"},
 	}
-	scanSetup.Project = *params
+	scanSetup.Project = params
 	assert.NoError(t, runInstallIfNeeded(&scanSetup, tmpDir))
 
 	scanSetup.InstallCommandName = "not-exist"
 	scanSetup.InstallCommandArgs = []string{"1", "2"}
-	scanSetup.FailOnInstallationErrors = false
+	scanSetup.SetFailOnInstallationErrors(false)
 	assert.NoError(t, runInstallIfNeeded(&scanSetup, tmpDir))
 
 	params = &utils.Project{
 		InstallCommandName: "not-existed",
 		InstallCommandArgs: []string{"1", "2"},
 	}
-	scanSetup.Project = *params
-	scanSetup.FailOnInstallationErrors = true
+	scanSetup.Project = params
+	scanSetup.SetFailOnInstallationErrors(true)
 	assert.Error(t, runInstallIfNeeded(&scanSetup, tmpDir))
 }
 

commands/testdata/config/frogbot-config-test-params.yml

@@ -10,6 +10,8 @@
     scan:
       includeAllVulnerabilities: true
       failOnSecurityIssues: true
+      minSeverity: high
+      fixableOnly: true
       projects:
         - installCommand: nuget restore
           workingDirs:

commands/utils/consts.go

@@ -58,6 +58,8 @@ const (
 	FailOnSecurityIssuesEnv      = "JF_FAIL"
 	UseWrapperEnv                = "JF_USE_WRAPPER"
 	DepsRepoEnv                  = "JF_DEPS_REPO"
+	MinSeverityEnv               = "JF_MIN_SEVERITY"
+	FixableOnlyEnv               = "JF_FIXABLE_ONLY"
 	WatchesDelimiter             = ","
 
 	//#nosec G101 -- False positive - no hardcoded credentials.

commands/utils/depsutil_test.go

@@ -97,7 +97,7 @@ func TestResolveDependencies(t *testing.T) {
 			tech: "npm",
 			scanSetup: &ScanDetails{
 				ServerDetails: &params,
-				Project: Project{
+				Project: &Project{
 					InstallCommandName: "npm",
 					InstallCommandArgs: []string{"install"},
 				}},
@@ -108,7 +108,7 @@ func TestResolveDependencies(t *testing.T) {
 			tech: "yarn",
 			scanSetup: &ScanDetails{
 				ServerDetails: &params,
-				Project: Project{
+				Project: &Project{
 					InstallCommandName: "yarn",
 					InstallCommandArgs: []string{"install"},
 				}},
@@ -119,7 +119,7 @@ func TestResolveDependencies(t *testing.T) {
 			tech: "dotnet",
 			scanSetup: &ScanDetails{
 				ServerDetails: &params,
-				Project: Project{
+				Project: &Project{
 					Repository:         "frogbot-nuget-remote-tests",
 					InstallCommandName: "dotnet",
 					InstallCommandArgs: []string{"restore"},