Add option to fail due to FailBuild flag on violations for repo scan (#1047)

Author: attiasas

scanrepository/scanrepository.go

@@ -15,6 +15,7 @@ import (
 	"github.com/jfrog/froggit-go/vcsclient"
 	"github.com/jfrog/froggit-go/vcsutils"
 	"github.com/jfrog/gofrog/version"
+	"github.com/jfrog/jfrog-cli-security/policy"
 	"github.com/jfrog/jfrog-cli-security/utils/formats"
 	"github.com/jfrog/jfrog-cli-security/utils/jasutils"
 	"github.com/jfrog/jfrog-cli-security/utils/results"
@@ -73,18 +74,25 @@ func (cfp *ScanRepositoryCmd) scanAndFixRepository(repository *utils.Repository,
 		return
 	}
 	log.Debug(fmt.Sprintf("Detected branches for scan: %s", strings.Join(repository.Branches, ", ")))
+	anyHasFailBuild := false
 	for _, branch := range repository.Branches {
 		log.Debug(fmt.Sprintf("Scanning '%s' branch...", branch))
 		cfp.scanDetails.SetBaseBranch(branch)
 		cfp.scanDetails.SetXscGitInfoContext(branch, repository.Project, client)
-		if err = cfp.scanAndFixBranch(repository); err != nil {
-			return
+		hadFailBuild, e := cfp.scanAndFixBranch(repository)
+		if e != nil {
+			return e
 		}
+		anyHasFailBuild = anyHasFailBuild || hadFailBuild
+	}
+	// Check if we need to output error due to fail the build flag on violations if repository configuration requires it
+	if repository.FailBuildOnViolations != nil && *repository.FailBuildOnViolations && anyHasFailBuild {
+		return policy.NewFailBuildError()
 	}
 	return
 }
 
-func (cfp *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (err error) {
+func (cfp *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (hasFailBuild bool, err error) {
 	repoDir, restoreBaseDir, err := cfp.cloneRepositoryOrUseLocalAndCheckoutToBranch()
 	if err != nil {
 		return
@@ -115,10 +123,11 @@ func (cfp *ScanRepositoryCmd) scanAndFixBranch(repository *utils.Repository) (er
 	for i := range repository.Projects {
 		cfp.scanDetails.Project = &repository.Projects[i]
 		cfp.projectTech = []techutils.Technology{}
-		if findings, e := cfp.scanAndFixProject(repository); e != nil {
-			return e
+		if hasFailBuildInProject, findings, e := cfp.scanAndFixProject(repository); e != nil {
+			return false, e
 		} else {
 			totalFindings += findings
+			hasFailBuild = hasFailBuild || hasFailBuildInProject
 		}
 	}
 
@@ -161,8 +170,9 @@ func (cfp *ScanRepositoryCmd) setCommandPrerequisites(repository *utils.Reposito
 	return
 }
 
-func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) (int, error) {
+func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) (bool, int, error) {
 	var isFixNeeded bool
+	shouldFailBuild := false
 	totalFindings := 0
 	// A map that contains the full project paths as a keys
 	// The value is a map of vulnerable package names -> the scanDetails of the vulnerable packages.
@@ -173,20 +183,12 @@ func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) (i
 		scanResults, err := cfp.scan(fullPathWd)
 		if err != nil {
 			if err = utils.CreateErrorIfPartialResultsDisabled(cfp.scanDetails.AllowPartialResults(), fmt.Sprintf("An error occurred during Audit execution for '%s' working directory. Fixes will be skipped for this working directory", fullPathWd), err); err != nil {
-				return totalFindings, err
+				return shouldFailBuild, totalFindings, err
 			}
 			continue
 		}
-		if summary, err := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{IncludeVulnerabilities: scanResults.IncludesVulnerabilities(), HasViolationContext: scanResults.HasViolationContext()}).ConvertToSummary(scanResults); err != nil {
-			return totalFindings, err
-		} else {
-			findingCount := summary.GetTotalViolations()
-			if findingCount == 0 {
-				findingCount = summary.GetTotalVulnerabilities()
-			}
-			totalFindings += findingCount
-		}
-
+		totalFindings += getTotalFindingsFromScanResults(scanResults)
+		shouldFailBuild = shouldFailBuild || (scanResults.Violations != nil && scanResults.Violations.ShouldFailBuild())
 		if repository.GitProvider.String() == vcsutils.GitHub.String() {
 			// Uploads Sarif results to GitHub in order to view the scan in the code scanning UI
 			// Currently available on GitHub only
@@ -203,42 +205,56 @@ func (cfp *ScanRepositoryCmd) scanAndFixProject(repository *utils.Repository) (i
 		if repository.DetectionOnly {
 			continue
 		}
-
 		for _, target := range scanResults.Targets {
-			targetPath := target.Target
-			convertor := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{
-				IncludeVulnerabilities: scanResults.IncludesVulnerabilities(),
-				HasViolationContext:    scanResults.HasViolationContext(),
-				IncludeTargets:         []string{targetPath},
-			})
-			simpleJsonResult, err := convertor.ConvertToSimpleJson(scanResults)
-			if err != nil {
-				if err = utils.CreateErrorIfPartialResultsDisabled(cfp.scanDetails.AllowPartialResults(), fmt.Sprintf("An error occurred while preparing the vulnerabilities map for '%s' working directory. Fixes will be skipped for this working directory", targetPath), err); err != nil {
-					return totalFindings, err
-				}
-				continue
-			}
-
-			currPathVulnerabilities, err := cfp.createVulnerabilitiesMapFromSimpleJson(simpleJsonResult)
+			currPathVulnerabilities, err := cfp.populateTargetResultsInVulnMap(target, scanResults)
 			if err != nil {
-				if err = utils.CreateErrorIfPartialResultsDisabled(cfp.scanDetails.AllowPartialResults(), fmt.Sprintf("An error occurred while preparing the vulnerabilities map for '%s' working directory. Fixes will be skipped for this working directory", targetPath), err); err != nil {
-					return totalFindings, err
+				if err = utils.CreateErrorIfPartialResultsDisabled(cfp.scanDetails.AllowPartialResults(), fmt.Sprintf("An error occurred while preparing the vulnerabilities map for '%s' working directory. Fixes will be skipped for this working directory", target.Target), err); err != nil {
+					return shouldFailBuild, totalFindings, err
 				}
 				continue
 			}
 			if len(currPathVulnerabilities) > 0 {
 				isFixNeeded = true
-				log.Debug(fmt.Sprintf("Found %d fixable vulnerabilities in '%s': %s", len(currPathVulnerabilities), targetPath, strings.Join(maps.Keys(currPathVulnerabilities), ", ")))
+				log.Debug(fmt.Sprintf("Found %d fixable vulnerabilities in '%s': %s", len(currPathVulnerabilities), target.Target, strings.Join(maps.Keys(currPathVulnerabilities), ", ")))
 			}
-			vulnerabilitiesByPathMap[targetPath] = currPathVulnerabilities
+			vulnerabilitiesByPathMap[target.Target] = currPathVulnerabilities
 		}
 	}
 	if repository.DetectionOnly {
 		log.Info(fmt.Sprintf("This command is running in detection mode only. To enable automatic fixing of issues, set the '%s' environment variable to 'false'.", utils.DetectionOnlyEnv))
 	} else if isFixNeeded {
-		return totalFindings, cfp.fixVulnerablePackages(repository, vulnerabilitiesByPathMap)
+		return shouldFailBuild, totalFindings, cfp.fixVulnerablePackages(repository, vulnerabilitiesByPathMap)
+	}
+	return shouldFailBuild, totalFindings, nil
+}
+
+func getTotalFindingsFromScanResults(scanResults *results.SecurityCommandResults) int {
+	summary, err := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{IncludeVulnerabilities: scanResults.IncludesVulnerabilities(), HasViolationContext: scanResults.HasViolationContext()}).ConvertToSummary(scanResults)
+	if err != nil {
+		return 0
+	}
+	findingCount := summary.GetTotalViolations()
+	if findingCount == 0 {
+		findingCount = summary.GetTotalVulnerabilities()
+	}
+	return findingCount
+}
+
+func (cfp *ScanRepositoryCmd) populateTargetResultsInVulnMap(target *results.TargetResults, scanResults *results.SecurityCommandResults) (map[string]*utils.VulnerabilityDetails, error) {
+	convertor := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{
+		IncludeVulnerabilities: scanResults.IncludesVulnerabilities(),
+		HasViolationContext:    scanResults.HasViolationContext(),
+		IncludeTargets:         []string{target.Target},
+	})
+	simpleJsonResult, err := convertor.ConvertToSimpleJson(scanResults)
+	if err != nil {
+		return nil, fmt.Errorf("an error occurred while converting scan results to simple json for target '%s': %w", target.Target, err)
+	}
+	currPathVulnerabilities, err := cfp.createVulnerabilitiesMapFromSimpleJson(simpleJsonResult)
+	if err != nil {
+		return nil, fmt.Errorf("an error occurred while creating vulnerabilities map from simple json for target '%s': %w", target.Target, err)
 	}
-	return totalFindings, nil
+	return currPathVulnerabilities, nil
 }
 
 // Audit the dependencies of the current commit.

scanrepository/scanrepository_test.go

@@ -699,7 +699,7 @@ func TestCreateVulnerabilitiesMap(t *testing.T) {
 
 func TestMultiTargetVulnerabilitiesMap(t *testing.T) {
 	cfp := &ScanRepositoryCmd{}
-	
+
 	scanResults := &results.SecurityCommandResults{
 		ResultsMetaData: results.ResultsMetaData{ResultContext: results.ResultContext{IncludeVulnerabilities: true}},
 		Targets: []*results.TargetResults{
@@ -739,7 +739,7 @@ func TestMultiTargetVulnerabilitiesMap(t *testing.T) {
 			},
 		},
 	}
-	
+
 	convertor1 := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{
 		IncludeVulnerabilities: true,
 		IncludeTargets:         []string{"project1"},
@@ -751,7 +751,7 @@ func TestMultiTargetVulnerabilitiesMap(t *testing.T) {
 	assert.Len(t, vulnsMap1, 1)
 	assert.Contains(t, vulnsMap1, "pkg1")
 	assert.NotContains(t, vulnsMap1, "pkg2")
-	
+
 	convertor2 := conversion.NewCommandResultsConvertor(conversion.ResultConvertParams{
 		IncludeVulnerabilities: true,
 		IncludeTargets:         []string{"project2"},

utils/consts.go

@@ -71,6 +71,7 @@ const (
 	AvoidPreviousPrCommentsDeletionEnv = "JF_AVOID_PREVIOUS_PR_COMMENTS_DELETION"
 	AddPrCommentOnSuccessEnv           = "JF_PR_ADD_SUCCESS_COMMENT"
 	FailOnSecurityIssuesEnv            = "JF_FAIL"
+	FailBuildOnViolationsEnv           = "JF_REPO_SCAN_FAIL_BUILD"
 	UseWrapperEnv                      = "JF_USE_WRAPPER"
 	DepsRepoEnv                        = "JF_DEPS_REPO"
 	MinSeverityEnv                     = "JF_MIN_SEVERITY"

utils/params.go

@@ -165,6 +165,7 @@ type Scan struct {
 	FixableOnly                     bool      `yaml:"fixableOnly,omitempty"`
 	DetectionOnly                   bool      `yaml:"skipAutoFix,omitempty"`
 	FailOnSecurityIssues            *bool     `yaml:"failOnSecurityIssues,omitempty"`
+	FailBuildOnViolations           *bool     `yaml:"failBuildOnViolations,omitempty"`
 	AvoidPreviousPrCommentsDeletion bool      `yaml:"avoidPreviousPrCommentsDeletion,omitempty"`
 	MinSeverity                     string    `yaml:"minSeverity,omitempty"`
 	DisableJas                      bool      `yaml:"disableJas,omitempty"`
@@ -251,6 +252,14 @@ func (s *Scan) setDefaultsIfNeeded() (err error) {
 		}
 		s.FailOnSecurityIssues = &failOnSecurityIssues
 	}
+	if s.FailBuildOnViolations == nil {
+		var failBuildOnViolations bool
+		// Default is false for backward compatibility
+		if failBuildOnViolations, err = getBoolEnv(FailBuildOnViolationsEnv, false); err != nil {
+			return
+		}
+		s.FailBuildOnViolations = &failBuildOnViolations
+	}
 	if s.MinSeverity == "" {
 		if err = readParamFromEnv(MinSeverityEnv, &s.MinSeverity); err != nil && !e.IsMissingEnvErr(err) {
 			return