fix: report passed not being calculated correctly (#1187)

* fix: report passed not being calculated correctly

* test: update to work with new exit codes and refactor

e2e/rules/helper.go

@@ -15,6 +15,7 @@ func buildRulesTestCase(testName, path, ruleID string) testhelper.TestCase {
 		"--only-rule=" + ruleID,
 		"--format=yaml",
 		"--disable-default-rules",
+		"--exit-code=0",
 		"--external-rule-dir=" + filepath.Join("e2e", "rules", "testdata", "rules"),
 	}
 

e2e/rules/rules_test.go

@@ -18,6 +18,7 @@ func TestSecrets(t *testing.T) {
 				"--only-rule=gitleaks",
 				"--format=yaml",
 				"--disable-default-rules",
+				"--exit-code=0",
 			},
 			testhelper.TestCaseOptions{},
 		),

new/detector/composition/testhelper/testhelper.go

@@ -143,7 +143,7 @@ func (runner *Runner) scanSingleFile(t *testing.T, testDataPath string, fileRela
 	}
 
 	runner.config.Scan.Target = testDataPath
-	detections, _, _ := output.GetOutput(
+	detections, _, _, _ := output.GetOutput(
 		types.Report{
 			Path: detectorsReportPath,
 		},

pkg/commands/artifact/run.go

@@ -193,7 +193,8 @@ func (r *runner) Scan(ctx context.Context, opts flag.Options) ([]files.File, *ba
 		}
 
 		report := types.Report{Path: r.reportPath + ".base", Inputgocloc: r.goclocResult}
-		detections, _, err := reportoutput.GetOutput(report, r.scanSettings, fileList.BaseFiles, nil)
+
+		detections, _, _, err := reportoutput.GetOutput(report, r.scanSettings, fileList.BaseFiles, nil)
 		if err != nil {
 			return err
 		}
@@ -330,7 +331,7 @@ func (r *runner) Report(
 		outputhandler.StdErrLog("Using cached data")
 	}
 
-	detections, dataflow, err := reportoutput.GetOutput(report, r.scanSettings, files, baseBranchFindings)
+	detections, dataflow, reportPassed, err := reportoutput.GetOutput(report, r.scanSettings, files, baseBranchFindings)
 	if err != nil {
 		return false, err
 	}
@@ -359,8 +360,7 @@ func (r *runner) Report(
 		if r.scanSettings.Report.Report == flag.ReportSecurity {
 			// for security report, default report format is Table
 			detectionReport := detections.(*security.Results)
-			var reportStr *strings.Builder
-			reportStr, reportPassed = security.BuildReportString(r.scanSettings, detectionReport, report.Inputgocloc, dataflow)
+			reportStr := security.BuildReportString(r.scanSettings, detectionReport, report.Inputgocloc, dataflow, reportPassed)
 
 			logger(reportStr.String())
 		} else if r.scanSettings.Report.Report == flag.ReportPrivacy {

pkg/report/output/output.go

@@ -27,28 +27,33 @@ func GetOutput(
 	config settings.Config,
 	files []files.File,
 	baseBranchFindings *basebranchfindings.Findings,
-) (any, *dataflow.DataFlow, error) {
+) (data any, flow *dataflow.DataFlow, reportPassed bool, err error) {
+	reportPassed = true
+
 	switch config.Report.Report {
 	case flag.ReportDetectors:
-		return detectors.GetOutput(report, config)
+		data, flow, err = detectors.GetOutput(report, config)
 	case flag.ReportDataFlow:
-		return GetDataflow(report, config, false)
+		data, flow, err = GetDataflow(report, config, false)
 	case flag.ReportSecurity:
-		return reportSecurity(report, config, files, baseBranchFindings)
+		data, flow, reportPassed, err = reportSecurity(report, config, files, baseBranchFindings)
 	case flag.ReportSaaS:
-		securityResults, dataflow, err := reportSecurity(report, config, files, baseBranchFindings)
-		if err != nil {
-			return nil, nil, err
+		securityResults, dataflow, _, secErr := reportSecurity(report, config, files, baseBranchFindings)
+
+		if secErr != nil {
+			err = secErr
+			return
 		}
 
-		return saas.GetReport(config, securityResults, dataflow, files)
+		data, flow, err = saas.GetReport(config, securityResults, dataflow, files)
 	case flag.ReportPrivacy:
-		return getPrivacyReportOutput(report, config)
+		data, flow, err = getPrivacyReportOutput(report, config)
 	case flag.ReportStats:
-		return reportStats(report, config)
+		data, flow, err = reportStats(report, config)
+	default:
+		err = fmt.Errorf(`--report flag "%s" is not supported`, config.Report.Report)
 	}
-
-	return nil, nil, fmt.Errorf(`--report flag "%s" is not supported`, config.Report.Report)
+	return
 }
 
 func GetPrivacyReportCSVOutput(report types.Report, dataflow *dataflow.DataFlow, config settings.Config) (*string, error) {
@@ -101,6 +106,7 @@ func reportSecurity(
 ) (
 	securityResults *security.Results,
 	dataflow *dataflow.DataFlow,
+	reportPassed bool,
 	err error,
 ) {
 	dataflow, _, err = GetDataflow(report, config, true)
@@ -109,7 +115,7 @@ func reportSecurity(
 		return
 	}
 
-	securityResults, err = security.GetOutput(dataflow, config, baseBranchFindings)
+	securityResults, reportPassed, err = security.GetOutput(dataflow, config, baseBranchFindings)
 	if err != nil {
 		log.Debug().Msgf("error in security %s", err)
 		return

pkg/report/output/security/security.go

@@ -129,19 +129,19 @@ func GetOutput(
 	dataflow *dataflow.DataFlow,
 	config settings.Config,
 	baseBranchFindings *basebranchfindings.Findings,
-) (*Results, error) {
+) (*Results, bool, error) {
 	summaryResults := make(Results)
 	if !config.Scan.Quiet {
 		output.StdErrLog("Evaluating rules")
 	}
 
 	builtInFingerprints, err := evaluateRules(summaryResults, config.BuiltInRules, config, dataflow, baseBranchFindings, true)
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 	fingerprints, err := evaluateRules(summaryResults, config.Rules, config, dataflow, baseBranchFindings, false)
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 
 	if !config.Scan.Quiet {
@@ -156,8 +156,16 @@ func GetOutput(
 			output.StdErrLog("\n=====================================")
 		}
 	}
+	reportPassed := true
+
+	for severityLevel, results := range summaryResults {
+		if severityLevel != types.LevelWarning && len(results) != 0 {
+			// fail the report if we have failures above the severity threshold
+			reportPassed = false
+		}
+	}
 
-	return &summaryResults, nil
+	return &summaryResults, reportPassed, nil
 }
 
 func evaluateRules(
@@ -310,8 +318,7 @@ func getExtract(rawCodeExtract []file.Line) string {
 	return strings.Join(parts, "\n")
 }
 
-func BuildReportString(config settings.Config, results *Results, lineOfCodeOutput *gocloc.Result, dataflow *dataflow.DataFlow) (*strings.Builder, bool) {
-	severityForFailure := config.Report.Severity
+func BuildReportString(config settings.Config, results *Results, lineOfCodeOutput *gocloc.Result, dataflow *dataflow.DataFlow, reportPassed bool) *strings.Builder {
 	reportStr := &strings.Builder{}
 
 	reportStr.WriteString("\n\nSecurity Report\n")
@@ -331,7 +338,7 @@ func BuildReportString(config settings.Config, results *Results, lineOfCodeOutpu
 	)
 
 	if rulesAvailableCount == 0 {
-		return reportStr, false
+		return reportStr
 	}
 
 	failures := map[string]map[string]bool{
@@ -342,16 +349,7 @@ func BuildReportString(config settings.Config, results *Results, lineOfCodeOutpu
 		types.LevelWarning:  make(map[string]bool),
 	}
 
-	reportPassed := true
 	for _, severityLevel := range orderedSeverityLevels {
-		if !severityForFailure[severityLevel] {
-			continue
-		}
-		if severityLevel != types.LevelWarning && len((*results)[severityLevel]) != 0 {
-			// fail the report if we have failures above the severity threshold
-			reportPassed = false
-		}
-
 		for _, failure := range (*results)[severityLevel] {
 			for i := 0; i < len(failure.CWEIDs); i++ {
 				failures[severityLevel]["CWE-"+failure.CWEIDs[i]] = true
@@ -364,7 +362,7 @@ func BuildReportString(config settings.Config, results *Results, lineOfCodeOutpu
 		reportStr.WriteString("\nNeed to add your own custom rule? Check out the guide: https://docs.bearer.com/guides/custom-rule\n")
 	}
 
-	noFailureSummary := checkAndWriteFailureSummaryToString(reportStr, *results, rulesAvailableCount, failures, severityForFailure)
+	noFailureSummary := checkAndWriteFailureSummaryToString(reportStr, *results, rulesAvailableCount, failures, config.Report.Severity)
 
 	if noFailureSummary {
 		writeSuccessToString(rulesAvailableCount, reportStr)
@@ -381,7 +379,7 @@ func BuildReportString(config settings.Config, results *Results, lineOfCodeOutpu
 
 	color.NoColor = initialColorSetting
 
-	return reportStr, reportPassed
+	return reportStr
 }
 
 func CalculateSeverity(groups []string, severity string, hasLocalDataTypes bool) string {
@@ -602,9 +600,6 @@ func checkAndWriteFailureSummaryToString(
 	failureCount := 0
 	warningCount := 0
 	for _, severityLevel := range maps.Keys(severityForFailure) {
-		if !severityForFailure[severityLevel] {
-			continue
-		}
 		if severityLevel == types.LevelWarning {
 			warningCount += len(results[severityLevel])
 			continue
@@ -617,16 +612,13 @@ func checkAndWriteFailureSummaryToString(
 	}
 
 	reportStr.WriteString("\n\n")
-	reportStr.WriteString(color.RedString(fmt.Sprint(ruleCount) + " checks, " + fmt.Sprint(failureCount+warningCount) + " findings\n\n"))
+	reportStr.WriteString(color.RedString(fmt.Sprint(ruleCount) + " checks, " + fmt.Sprint(failureCount+warningCount) + " findings\n"))
 
-	for i, severityLevel := range orderedSeverityLevels {
+	for _, severityLevel := range orderedSeverityLevels {
 		if !severityForFailure[severityLevel] {
 			continue
 		}
-		if i > 0 {
-			reportStr.WriteString("\n")
-		}
-		reportStr.WriteString(formatSeverity(severityLevel) + fmt.Sprint(len(results[severityLevel])))
+		reportStr.WriteString("\n" + formatSeverity(severityLevel) + fmt.Sprint(len(results[severityLevel])))
 		if len(failures[severityLevel]) > 0 {
 			ruleIds := maps.Keys(failures[severityLevel])
 			sort.Strings(ruleIds)

pkg/report/output/security/security_test.go

@@ -51,7 +51,7 @@ func TestBuildReportString(t *testing.T) {
 
 	dataflow := dummyDataflow()
 
-	results, err := security.GetOutput(&dataflow, config, nil)
+	results, reportPassed, err := security.GetOutput(&dataflow, config, nil)
 	if err != nil {
 		t.Fatalf("failed to generate security output err:%s", err)
 	}
@@ -66,7 +66,7 @@ func TestBuildReportString(t *testing.T) {
 		MaxPathLength: 0,
 	}
 
-	stringBuilder, _ := security.BuildReportString(config, results, &dummyGoclocResult, &dataflow)
+	stringBuilder := security.BuildReportString(config, results, &dummyGoclocResult, &dataflow, reportPassed)
 	cupaloy.SnapshotT(t, stringBuilder.String())
 }
 
@@ -91,7 +91,7 @@ func TestNoRulesBuildReportString(t *testing.T) {
 
 	dataflow := dummyDataflow()
 
-	results, err := security.GetOutput(&dataflow, config, nil)
+	results, reportPassed, err := security.GetOutput(&dataflow, config, nil)
 	if err != nil {
 		t.Fatalf("failed to generate security output err:%s", err)
 	}
@@ -106,7 +106,7 @@ func TestNoRulesBuildReportString(t *testing.T) {
 		MaxPathLength: 0,
 	}
 
-	stringBuilder, _ := security.BuildReportString(config, results, &dummyGoclocResult, &dataflow)
+	stringBuilder := security.BuildReportString(config, results, &dummyGoclocResult, &dataflow, reportPassed)
 	cupaloy.SnapshotT(t, stringBuilder.String())
 }
 
@@ -128,7 +128,7 @@ func TestGetOutput(t *testing.T) {
 
 	dataflow := dummyDataflow()
 
-	res, err := security.GetOutput(&dataflow, config, nil)
+	res, _, err := security.GetOutput(&dataflow, config, nil)
 	if err != nil {
 		t.Fatalf("failed to generate security output err:%s", err)
 	}
@@ -154,7 +154,7 @@ func TestTestGetOutputWithSeverity(t *testing.T) {
 
 	dataflow := dummyDataflow()
 
-	res, err := security.GetOutput(&dataflow, config, nil)
+	res, _, err := security.GetOutput(&dataflow, config, nil)
 	if err != nil {
 		t.Fatalf("failed to generate security output err:%s", err)
 	}