From faba298ec1b5cabdfd0a9bbaf3d5a9dc9feb67ef Mon Sep 17 00:00:00 2001 From: nirmo Date: Thu, 28 Sep 2023 11:00:12 +0200 Subject: [PATCH] feat: ignore-on-exit flag to control the error code (#162) Fixes #155 Reference: [`ignore-on-exit`](https://docs.kics.io/latest/commands/) in Kics Waits for #188 ## Proposed Changes - Ran manual tests with the confluence plugin: | cmd | exit code | |--------|--------| | `2ms confluence --url --spaces --token --username <>` | 1 | | `2ms confluence --url --spaces --token --username <> --ignore-secrets` | 0 | --------- Co-authored-by: nirmo Co-authored-by: Baruch Odem --- README.md | 27 +++++----- cmd/config.go | 57 +++++++++++++++++++++ cmd/enum_flags.go | 37 ++++++++++++++ cmd/exit_handler.go | 53 ++++++++++++++++++++ cmd/exit_handler_test.go | 73 +++++++++++++++++++++++++++ cmd/main.go | 104 +++++++++++---------------------------- main.go | 6 +-- plugins/filesystem.go | 5 +- reporting/json.go | 8 +-- reporting/report.go | 28 +++++++---- reporting/sarif.go | 7 ++- reporting/yaml.go | 7 ++- secrets/rules/rule.go | 4 +- tests/lint.go | 6 ++- 14 files changed, 304 insertions(+), 118 deletions(-) create mode 100644 cmd/config.go create mode 100644 cmd/enum_flags.go create mode 100644 cmd/exit_handler.go create mode 100644 cmd/exit_handler_test.go diff --git a/README.md b/README.md index ee5f39cf..59ce2ee7 100644 --- a/README.md +++ b/README.md @@ -117,18 +117,21 @@ Additional Commands: rules List all rules Flags: - --add-special-rule strings special (non-default) rules to apply. - This list is not affected by the --rule and --ignore-rule flags. - --config string config file path - -h, --help help for 2ms - --ignore-result strings ignore specific result by id - --ignore-rule strings ignore rules by name or tag - --log-level string log level (trace, debug, info, warn, error, fatal) (default "info") - --regex stringArray custom regexes to apply to the scan, must be valid Go regex - --report-path strings path to generate report files. The output format will be determined by the file extension (.json, .yaml, .sarif) - --rule strings select rules by name or tag to apply to this scan - --stdout-format string stdout output format, available formats are: json, yaml, sarif (default "yaml") - -v, --version version for 2ms + --add-special-rule strings special (non-default) rules to apply. + This list is not affected by the --rule and --ignore-rule flags. + --config string config file path + -h, --help help for 2ms + --ignore-on-exit ignoreOnExit defines which kind of non-zero exits code should be ignored + accepts: all, results, errors, none + example: if 'results' is set, only engine errors will make 2ms exit code different from 0 (default none) + --ignore-result strings ignore specific result by id + --ignore-rule strings ignore rules by name or tag + --log-level string log level (trace, debug, info, warn, error, fatal) (default "info") + --regex stringArray custom regexes to apply to the scan, must be valid Go regex + --report-path strings path to generate report files. The output format will be determined by the file extension (.json, .yaml, .sarif) + --rule strings select rules by name or tag to apply to this scan + --stdout-format string stdout output format, available formats are: json, yaml, sarif (default "yaml") + -v, --version version for 2ms Use "2ms [command] --help" for more information about a command. ``` diff --git a/cmd/config.go b/cmd/config.go new file mode 100644 index 00000000..3c07efd4 --- /dev/null +++ b/cmd/config.go @@ -0,0 +1,57 @@ +package cmd + +import ( + "fmt" + "path/filepath" + "regexp" + "strings" + + "github.com/checkmarx/2ms/lib" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" + "github.com/spf13/cobra" +) + +func initialize() { + configFilePath, err := rootCmd.Flags().GetString(configFileFlag) + if err != nil { + cobra.CheckErr(err) + } + cobra.CheckErr(lib.LoadConfig(vConfig, configFilePath)) + cobra.CheckErr(lib.BindFlags(rootCmd, vConfig, envPrefix)) + + logLevel := zerolog.InfoLevel + switch strings.ToLower(logLevelVar) { + case "trace": + logLevel = zerolog.TraceLevel + case "debug": + logLevel = zerolog.DebugLevel + case "info": + logLevel = zerolog.InfoLevel + case "warn": + logLevel = zerolog.WarnLevel + case "err", "error": + logLevel = zerolog.ErrorLevel + case "fatal": + logLevel = zerolog.FatalLevel + } + zerolog.SetGlobalLevel(logLevel) + log.Logger = log.Logger.Level(logLevel) +} + +func validateFormat(stdout string, reportPath []string) error { + r := regexp.MustCompile(outputFormatRegexpPattern) + if !(r.MatchString(stdout)) { + return fmt.Errorf(`invalid output format: %s, available formats are: json, yaml and sarif`, stdout) + } + + for _, path := range reportPath { + fileExtension := filepath.Ext(path) + format := strings.TrimPrefix(fileExtension, ".") + if !(r.MatchString(format)) { + return fmt.Errorf(`invalid report extension: %s, available extensions are: json, yaml and sarif`, format) + } + } + + return nil +} diff --git a/cmd/enum_flags.go b/cmd/enum_flags.go new file mode 100644 index 00000000..34fad7d8 --- /dev/null +++ b/cmd/enum_flags.go @@ -0,0 +1,37 @@ +package cmd + +import ( + "flag" + "fmt" +) + +type ignoreOnExit string + +const ( + ignoreOnExitNone ignoreOnExit = "none" + ignoreOnExitAll ignoreOnExit = "all" + ignoreOnExitResults ignoreOnExit = "results" + ignoreOnExitErrors ignoreOnExit = "errors" +) + +// verify that ignoreOnExit implements flag.Value interface +// https://github.com/uber-go/guide/blob/master/style.md#verify-interface-compliance +var _ flag.Value = (*ignoreOnExit)(nil) + +func (i *ignoreOnExit) String() string { + return string(*i) +} + +func (i *ignoreOnExit) Set(value string) error { + switch value { + case "none", "all", "results", "errors": + *i = ignoreOnExit(value) + return nil + default: + return fmt.Errorf("invalid value %s", value) + } +} + +func (i *ignoreOnExit) Type() string { + return "ignoreOnExit" +} diff --git a/cmd/exit_handler.go b/cmd/exit_handler.go new file mode 100644 index 00000000..aeed0ca2 --- /dev/null +++ b/cmd/exit_handler.go @@ -0,0 +1,53 @@ +package cmd + +import ( + "os" +) + +const ( + errorCode = 1 + resultsCode = 2 +) + +func isNeedReturnErrorCodeFor(kind ignoreOnExit) bool { + if ignoreOnExitVar == ignoreOnExitNone { + return true + } + + if ignoreOnExitVar == ignoreOnExitAll { + return false + } + + if ignoreOnExitVar != ignoreOnExit(kind) { + return true + } + + return false +} + +func exitCodeIfError(err error) int { + if err != nil && isNeedReturnErrorCodeFor("errors") { + return errorCode + } + + return 0 +} + +func exitCodeIfResults(resultsCount int) int { + if resultsCount > 0 && isNeedReturnErrorCodeFor("results") { + return resultsCode + } + + return 0 +} + +func Exit(resultsCount int, err error) { + os.Exit(exitCodeIfError(err) + exitCodeIfResults(resultsCount)) +} + +func listenForErrors(errors chan error) { + go func() { + err := <-errors + Exit(0, err) + }() +} diff --git a/cmd/exit_handler_test.go b/cmd/exit_handler_test.go new file mode 100644 index 00000000..a3f27d57 --- /dev/null +++ b/cmd/exit_handler_test.go @@ -0,0 +1,73 @@ +package cmd + +import ( + "fmt" + "testing" +) + +func TestExitHandler_IsNeedReturnErrorCode(t *testing.T) { + + var onErrorsTests = []struct { + userInput ignoreOnExit + expectedResult bool + }{ + { + userInput: ignoreOnExitNone, + expectedResult: true, + }, + { + userInput: ignoreOnExitAll, + expectedResult: false, + }, + { + userInput: ignoreOnExitResults, + expectedResult: true, + }, + { + userInput: ignoreOnExitErrors, + expectedResult: false, + }, + } + + for idx, testCase := range onErrorsTests { + t.Run(fmt.Sprintf("Print test case %d", idx), func(t *testing.T) { + ignoreOnExitVar = testCase.userInput + result := isNeedReturnErrorCodeFor("errors") + if result != testCase.expectedResult { + t.Errorf("Expected %v, got %v", testCase.expectedResult, result) + } + }) + } + + var onResultsTests = []struct { + userInput ignoreOnExit + expectedResult bool + }{ + { + userInput: ignoreOnExitNone, + expectedResult: true, + }, + { + userInput: ignoreOnExitAll, + expectedResult: false, + }, + { + userInput: ignoreOnExitResults, + expectedResult: false, + }, + { + userInput: ignoreOnExitErrors, + expectedResult: true, + }, + } + + for idx, testCase := range onResultsTests { + t.Run(fmt.Sprintf("Print test case %d", idx), func(t *testing.T) { + ignoreOnExitVar = testCase.userInput + result := isNeedReturnErrorCodeFor("results") + if result != testCase.expectedResult { + t.Errorf("Expected %v, got %v", testCase.expectedResult, result) + } + }) + } +} diff --git a/cmd/main.go b/cmd/main.go index 0c4a8a70..eb77f3a7 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -2,21 +2,12 @@ package cmd import ( "fmt" - "os" - "path/filepath" - "regexp" - "strings" - - "github.com/checkmarx/2ms/config" - "github.com/checkmarx/2ms/lib" - "sync" + "github.com/checkmarx/2ms/config" "github.com/checkmarx/2ms/plugins" "github.com/checkmarx/2ms/reporting" "github.com/checkmarx/2ms/secrets" - - "github.com/rs/zerolog" "github.com/rs/zerolog/log" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -37,6 +28,7 @@ const ( ignoreRuleFlagName = "ignore-rule" ignoreFlagName = "ignore-result" specialRulesFlagName = "add-special-rule" + ignoreOnExitFlagName = "ignore-on-exit" ) var ( @@ -48,6 +40,7 @@ var ( ignoreRuleVar []string ignoreVar []string specialRulesVar []string + ignoreOnExitVar = ignoreOnExitNone ) var rootCmd = &cobra.Command{ @@ -80,34 +73,7 @@ var channels = plugins.Channels{ var report = reporting.Init() var secretsChan = make(chan reporting.Secret) -func initialize() { - configFilePath, err := rootCmd.Flags().GetString(configFileFlag) - if err != nil { - cobra.CheckErr(err) - } - cobra.CheckErr(lib.LoadConfig(vConfig, configFilePath)) - cobra.CheckErr(lib.BindFlags(rootCmd, vConfig, envPrefix)) - - logLevel := zerolog.InfoLevel - switch strings.ToLower(logLevelVar) { - case "trace": - logLevel = zerolog.TraceLevel - case "debug": - logLevel = zerolog.DebugLevel - case "info": - logLevel = zerolog.InfoLevel - case "warn": - logLevel = zerolog.WarnLevel - case "err", "error": - logLevel = zerolog.ErrorLevel - case "fatal": - logLevel = zerolog.FatalLevel - } - zerolog.SetGlobalLevel(logLevel) - log.Logger = log.Logger.Level(logLevel) -} - -func Execute() { +func Execute() (int, error) { vConfig.SetEnvPrefix(envPrefix) vConfig.AutomaticEnv() @@ -122,6 +88,7 @@ func Execute() { rootCmd.PersistentFlags().StringSliceVar(&ignoreRuleVar, ignoreRuleFlagName, []string{}, "ignore rules by name or tag") rootCmd.PersistentFlags().StringSliceVar(&ignoreVar, ignoreFlagName, []string{}, "ignore specific result by id") rootCmd.PersistentFlags().StringSliceVar(&specialRulesVar, specialRulesFlagName, []string{}, "special (non-default) rules to apply.\nThis list is not affected by the --rule and --ignore-rule flags.") + rootCmd.PersistentFlags().Var(&ignoreOnExitVar, ignoreOnExitFlagName, "defines which kind of non-zero exits code should be ignored\naccepts: all, results, errors, none\nexample: if 'results' is set, only engine errors will make 2ms exit code different from 0") rootCmd.AddCommand(secrets.GetRulesCommand(&ruleVar, &ignoreRuleVar, &specialRulesVar)) @@ -131,43 +98,35 @@ func Execute() { for _, plugin := range allPlugins { subCommand, err := plugin.DefineCommand(channels.Items, channels.Errors) if err != nil { - log.Fatal().Msg(fmt.Sprintf("error while defining command for plugin %s: %s", plugin.GetName(), err.Error())) + return 0, fmt.Errorf("error while defining command for plugin %s: %s", plugin.GetName(), err.Error()) } subCommand.GroupID = group - subCommand.PreRun = preRun - subCommand.PostRun = postRun + subCommand.PreRunE = preRun + subCommand.PostRunE = postRun rootCmd.AddCommand(subCommand) } + listenForErrors(channels.Errors) + if err := rootCmd.Execute(); err != nil { - log.Fatal().Msg(err.Error()) + return 0, err } -} -func validateFormat(stdout string, reportPath []string) { - r := regexp.MustCompile(outputFormatRegexpPattern) - if !(r.MatchString(stdout)) { - log.Fatal().Msgf(`invalid output format: %s, available formats are: json, yaml and sarif`, stdout) - } + return report.TotalSecretsFound, nil +} - for _, path := range reportPath { - fileExtension := filepath.Ext(path) - format := strings.TrimPrefix(fileExtension, ".") - if !(r.MatchString(format)) { - log.Fatal().Msgf(`invalid report extension: %s, available extensions are: json, yaml and sarif`, format) - } +func preRun(cmd *cobra.Command, args []string) error { + if err := validateFormat(stdoutFormatVar, reportPathVar); err != nil { + return err } -} -func preRun(cmd *cobra.Command, args []string) { - validateFormat(stdoutFormatVar, reportPathVar) secrets, err := secrets.Init(ruleVar, ignoreRuleVar, specialRulesVar) if err != nil { - log.Fatal().Msg(err.Error()) + return err } if err := secrets.AddRegexRules(customRegexRuleVar); err != nil { - log.Fatal().Msg(err.Error()) + return err } channels.WaitGroup.Add(1) @@ -190,39 +149,32 @@ func preRun(cmd *cobra.Command, args []string) { for secret := range secretsChan { report.TotalSecretsFound++ report.Results[secret.ID] = append(report.Results[secret.ID], secret) - } - }() - go func() { - for err := range channels.Errors { - log.Fatal().Msg(err.Error()) } }() + + return nil } -func postRun(cmd *cobra.Command, args []string) { +func postRun(cmd *cobra.Command, args []string) error { channels.WaitGroup.Wait() cfg := config.LoadConfig("2ms", Version) - // ------------------------------------- - // Show Report if report.TotalItemsScanned > 0 { - report.ShowReport(stdoutFormatVar, cfg) + if err := report.ShowReport(stdoutFormatVar, cfg); err != nil { + return err + } + if len(reportPathVar) > 0 { err := report.WriteFile(reportPathVar, cfg) if err != nil { - log.Error().Msgf("Failed to create report file with error: %s", err) + return fmt.Errorf("failed to create report file with error: %s", err) } } } else { - log.Error().Msg("Scan completed with empty content") - os.Exit(0) + log.Info().Msg("Scan completed with empty content") } - if report.TotalSecretsFound > 0 { - os.Exit(1) - } else { - os.Exit(0) - } + return nil } diff --git a/main.go b/main.go index 88d8e2ac..bd0dcbca 100644 --- a/main.go +++ b/main.go @@ -6,7 +6,6 @@ import ( "github.com/checkmarx/2ms/cmd" "github.com/checkmarx/2ms/lib" - "github.com/rs/zerolog" "github.com/rs/zerolog/log" ) @@ -20,10 +19,11 @@ func main() { stopChan := make(chan os.Signal, 1) signal.Notify(stopChan, os.Interrupt) go listenForInterrupt(stopChan) - cmd.Execute() + + cmd.Exit(cmd.Execute()) } func listenForInterrupt(stopScan chan os.Signal) { <-stopScan - log.Fatal().Msg("Interrupt signal received. Exiting...") + log.Fatal().Msg("Interrupt signal received. Exiting...") // lint:ignore We want to exit immediately } diff --git a/plugins/filesystem.go b/plugins/filesystem.go index c90e8744..a17b803b 100644 --- a/plugins/filesystem.go +++ b/plugins/filesystem.go @@ -63,7 +63,7 @@ func (p *FileSystemPlugin) getFiles(items chan Item, errs chan error, wg *sync.W fileList := make([]string, 0) err := filepath.Walk(p.Path, func(path string, fInfo os.FileInfo, err error) error { if err != nil { - log.Fatal().Err(err).Msg("error while walking through the directory") + return err } for _, ignoredFolder := range ignoredFolders { if fInfo.Name() == ignoredFolder && fInfo.IsDir() { @@ -92,7 +92,8 @@ func (p *FileSystemPlugin) getFiles(items chan Item, errs chan error, wg *sync.W }) if err != nil { - log.Fatal().Err(err).Msg("error while walking through the directory") + errs <- fmt.Errorf("error while walking through the directory: %w", err) + return } p.getItems(items, errs, wg, fileList) diff --git a/reporting/json.go b/reporting/json.go index 14a8df1a..37aa1821 100644 --- a/reporting/json.go +++ b/reporting/json.go @@ -2,14 +2,14 @@ package reporting import ( "encoding/json" - "log" + "fmt" ) -func writeJson(report Report) string { +func writeJson(report Report) (string, error) { jsonReport, err := json.MarshalIndent(report, "", " ") if err != nil { - log.Fatalf("failed to create Json report with error: %v", err) + return "", fmt.Errorf("failed to create Json report with error: %v", err) } - return string(jsonReport) + return string(jsonReport), nil } diff --git a/reporting/report.go b/reporting/report.go index 2e7cfb14..e2a75ff7 100644 --- a/reporting/report.go +++ b/reporting/report.go @@ -39,9 +39,14 @@ func Init() *Report { } } -func (r *Report) ShowReport(format string, cfg *config.Config) { - output := r.getOutput(format, cfg) - log.Info().Msg(output) +func (r *Report) ShowReport(format string, cfg *config.Config) error { + output, err := r.getOutput(format, cfg) + if err != nil { + return err + } + + log.Info().Msg("\n" + output) + return nil } func (r *Report) WriteFile(reportPath []string, cfg *config.Config) error { @@ -53,7 +58,10 @@ func (r *Report) WriteFile(reportPath []string, cfg *config.Config) error { fileExtension := filepath.Ext(path) format := strings.TrimPrefix(fileExtension, ".") - output := r.getOutput(format, cfg) + output, err := r.getOutput(format, cfg) + if err != nil { + return err + } _, err = file.WriteString(output) if err != nil { @@ -63,15 +71,17 @@ func (r *Report) WriteFile(reportPath []string, cfg *config.Config) error { return nil } -func (r *Report) getOutput(format string, cfg *config.Config) string { +func (r *Report) getOutput(format string, cfg *config.Config) (string, error) { var output string + var err error + switch format { case jsonFormat: - output = writeJson(*r) + output, err = writeJson(*r) case longYamlFormat, shortYamlFormat: - output = writeYaml(*r) + output, err = writeYaml(*r) case sarifFormat: - output = writeSarif(*r, cfg) + output, err = writeSarif(*r, cfg) } - return output + return output, err } diff --git a/reporting/sarif.go b/reporting/sarif.go index 451dae0f..a26c3d60 100644 --- a/reporting/sarif.go +++ b/reporting/sarif.go @@ -3,12 +3,11 @@ package reporting import ( "encoding/json" "fmt" - "log" "github.com/checkmarx/2ms/config" ) -func writeSarif(report Report, cfg *config.Config) string { +func writeSarif(report Report, cfg *config.Config) (string, error) { sarif := Sarif{ Schema: "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", Version: "2.1.0", @@ -17,10 +16,10 @@ func writeSarif(report Report, cfg *config.Config) string { sarifReport, err := json.MarshalIndent(sarif, "", " ") if err != nil { - log.Fatalf("failed to create Sarif report with error: %v", err) + return "", fmt.Errorf("failed to create Sarif report with error: %v", err) } - return string(sarifReport) + return string(sarifReport), nil } func getRuns(report Report, cfg *config.Config) []Runs { diff --git a/reporting/yaml.go b/reporting/yaml.go index f91bf7d1..e9335002 100644 --- a/reporting/yaml.go +++ b/reporting/yaml.go @@ -2,14 +2,13 @@ package reporting import ( "gopkg.in/yaml.v2" - "log" ) -func writeYaml(report Report) string { +func writeYaml(report Report) (string, error) { yamlReport, err := yaml.Marshal(&report) if err != nil { - log.Fatalf("failed to create Yaml report with error: %v", err) + return "", err } - return string(yamlReport) + return string(yamlReport), nil } diff --git a/secrets/rules/rule.go b/secrets/rules/rule.go index 860ae2ab..5de08dd7 100644 --- a/secrets/rules/rule.go +++ b/secrets/rules/rule.go @@ -30,12 +30,12 @@ func validate(r config.Rule, truePositives []string, falsePositives []string) *c }) for _, tp := range truePositives { if len(d.DetectString(tp)) != 1 { - log.Fatal().Msgf("Failed to validate. For rule ID [%s], true positive [%s] was not detected by regexp [%s]", r.RuleID, tp, r.Regex) + log.Fatal().Msgf("Failed to validate. For rule ID [%s], true positive [%s] was not detected by regexp [%s]", r.RuleID, tp, r.Regex) // lint:ignore This Fatal happens in a test } } for _, fp := range falsePositives { if len(d.DetectString(fp)) != 0 { - log.Fatal().Msgf("Failed to validate. For rule ID [%s], false positive [%s] was detected by regexp [%s]", r.RuleID, fp, r.Regex) + log.Fatal().Msgf("Failed to validate. For rule ID [%s], false positive [%s] was detected by regexp [%s]", r.RuleID, fp, r.Regex) // lint:ignore This Fatal happens in a test } } return &r diff --git a/tests/lint.go b/tests/lint.go index 68630da1..65e43ffc 100644 --- a/tests/lint.go +++ b/tests/lint.go @@ -8,6 +8,8 @@ import ( "regexp" ) +var ignoreComment = regexp.MustCompile(`//\s*lint:ignore`) + func walkGoFiles() <-chan string { ignoredDirs := []string{ ".git", @@ -50,7 +52,7 @@ func walkGoFiles() <-chan string { var forbiddenPatterns = []*regexp.Regexp{ regexp.MustCompile(`fmt\.Print`), - // regexp.MustCompile(`log\.Fatal`), + regexp.MustCompile(`log\.Fatal\(\)`), } var ignoreFiles = regexp.MustCompile(`_test\.go$`) @@ -71,7 +73,7 @@ func lintFile(path string) error { for scanner.Scan() { lineText := scanner.Text() for _, forbiddenPattern := range forbiddenPatterns { - if forbiddenPattern.MatchString(lineText) { + if forbiddenPattern.MatchString(lineText) && !ignoreComment.MatchString(lineText) { return fmt.Errorf("%s:%d: forbidden pattern found: %s", path, line, forbiddenPattern.String()) } }