From 5c83e96d18b0f464e2d43147167d605f605e6f18 Mon Sep 17 00:00:00 2001 From: gotbadger Date: Wed, 28 Jun 2023 12:14:43 +0100 Subject: [PATCH 1/3] feat: better handle rule loading and infomation --- pkg/commands/process/settings/ruleLoader.go | 21 ++++++++++ pkg/commands/process/settings/rules.go | 4 +- .../security/.snapshots/TestBuildReportString | 2 +- .../.snapshots/TestNoRulesBuildReportString | 8 ++++ pkg/report/output/security/security.go | 21 +++++++--- pkg/report/output/security/security_test.go | 42 +++++++++++++++++++ 6 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 pkg/report/output/security/.snapshots/TestNoRulesBuildReportString diff --git a/pkg/commands/process/settings/ruleLoader.go b/pkg/commands/process/settings/ruleLoader.go index 01be008bd..48f8f4975 100644 --- a/pkg/commands/process/settings/ruleLoader.go +++ b/pkg/commands/process/settings/ruleLoader.go @@ -10,7 +10,9 @@ import ( "net/http" "os" "path/filepath" + "strconv" "strings" + "time" "gopkg.in/yaml.v3" ) @@ -24,6 +26,21 @@ func LoadRuleDefinitionsFromGitHub(ruleDefinitions map[string]RuleDefinition, fo return tagName, err } defer resp.Body.Close() + headers := resp.Header + + if headers.Get("x-ratelimit-remaining") == "0" { + resetString := headers.Get("x-ratelimit-reset") + unixTime, err := strconv.ParseInt(resetString, 10, 64) + if err != nil { + return tagName, fmt.Errorf("rules download is rate limited please wait until: %s", resetString) + } + tm := time.Unix(unixTime, 0) + return tagName, fmt.Errorf("rules download is rate limited please wait until: %s", tm.Format("2006-01-02 15:04:05")) + } + + if resp.StatusCode != 200 { + return tagName, errors.New("rules download returned non 200 status code - could not download rules") + } // Decode the response JSON to get the URL of the asset we want to download type Asset struct { @@ -40,6 +57,10 @@ func LoadRuleDefinitionsFromGitHub(ruleDefinitions map[string]RuleDefinition, fo return tagName, err } + if release.TagName == "" { + return tagName, errors.New("could not find valid release for rules") + } + bearerRulesDir := bearerRulesDir() if _, err := os.Stat(bearerRulesDir); errors.Is(err, os.ErrNotExist) { err := os.Mkdir(bearerRulesDir, os.ModePerm) diff --git a/pkg/commands/process/settings/rules.go b/pkg/commands/process/settings/rules.go index 57778678b..a77a05104 100644 --- a/pkg/commands/process/settings/rules.go +++ b/pkg/commands/process/settings/rules.go @@ -9,6 +9,7 @@ import ( "github.com/bearer/bearer/pkg/flag" "github.com/bearer/bearer/pkg/report/customdetectors" + "github.com/bearer/bearer/pkg/util/output" "github.com/bearer/bearer/pkg/util/set" "github.com/rs/zerolog/log" "gopkg.in/yaml.v3" @@ -99,7 +100,8 @@ func loadRules( tagVersion, err := LoadRuleDefinitionsFromGitHub(definitions, foundLanguages) if err != nil { - return result, fmt.Errorf("error loading rules: %s", err) + output.Fatal(fmt.Sprintf("Error loading rules: %s", err)) + // sysexit } result.BearerRulesVersion = tagVersion diff --git a/pkg/report/output/security/.snapshots/TestBuildReportString b/pkg/report/output/security/.snapshots/TestBuildReportString index ac54aa999..13889fdd8 100644 --- a/pkg/report/output/security/.snapshots/TestBuildReportString +++ b/pkg/report/output/security/.snapshots/TestBuildReportString @@ -5,7 +5,7 @@ Summary Report ===================================== Rules: - - 2 default rules applied (https://docs.bearer.com/reference/rules) + - 2 default rules applied (https://docs.bearer.com/reference/rules) [TEST] - 1 custom rules applied CRITICAL: Sensitive data sent to Rails loggers detected. [CWE-209, CWE-532] diff --git a/pkg/report/output/security/.snapshots/TestNoRulesBuildReportString b/pkg/report/output/security/.snapshots/TestNoRulesBuildReportString new file mode 100644 index 000000000..1f5758966 --- /dev/null +++ b/pkg/report/output/security/.snapshots/TestNoRulesBuildReportString @@ -0,0 +1,8 @@ + + +Summary Report + +===================================== + +Zero rules found. A security report requires rules to function. Please check configuration. + diff --git a/pkg/report/output/security/security.go b/pkg/report/output/security/security.go index 40737d7ca..deb7f3a4d 100644 --- a/pkg/report/output/security/security.go +++ b/pkg/report/output/security/security.go @@ -265,6 +265,10 @@ func BuildReportString(config settings.Config, results *Results, lineOfCodeOutpu config, ) + if rulesAvailableCount == 0 { + return reportStr, false + } + failures := map[string]map[string]bool{ types.LevelCritical: make(map[string]bool), types.LevelHigh: make(map[string]bool), @@ -385,20 +389,25 @@ func writeRuleListToString( languages map[string]*gocloc.Language, config settings.Config, ) int { - // list rules that were run - reportStr.WriteString("\n\nRules: \n") - defaultRuleCount, customRuleCount := countRules(rules, languages, config, false) builtInCount, _ := countRules(builtInRules, languages, config, true) defaultRuleCount = defaultRuleCount + builtInCount + totalRuleCount := defaultRuleCount + customRuleCount - reportStr.WriteString(fmt.Sprintf(" - %d default rules applied ", defaultRuleCount)) - reportStr.WriteString(color.HiBlackString("(https://docs.bearer.com/reference/rules)\n")) + if totalRuleCount == 0 { + reportStr.WriteString("\n\nZero rules found. A security report requires rules to function. Please check configuration.\n") + return 0 + } + reportStr.WriteString("\n\nRules: \n") + if defaultRuleCount > 0 { + reportStr.WriteString(fmt.Sprintf(" - %d default rules applied ", defaultRuleCount)) + reportStr.WriteString(color.HiBlackString(fmt.Sprintf("(https://docs.bearer.com/reference/rules) [%s]\n", config.BearerRulesVersion))) + } if customRuleCount > 0 { reportStr.WriteString(fmt.Sprintf(" - %d custom rules applied", customRuleCount)) } - return defaultRuleCount + customRuleCount + return totalRuleCount } func writeApiClientResultToString( diff --git a/pkg/report/output/security/security_test.go b/pkg/report/output/security/security_test.go index 2867596c5..fae2dee23 100644 --- a/pkg/report/output/security/security_test.go +++ b/pkg/report/output/security/security_test.go @@ -24,6 +24,8 @@ func TestBuildReportString(t *testing.T) { "warning": true, }, }) + // set rule version + config.BearerRulesVersion = "TEST" // new rules are added customRule := &settings.Rule{ @@ -68,6 +70,46 @@ func TestBuildReportString(t *testing.T) { cupaloy.SnapshotT(t, stringBuilder.String()) } +func TestNoRulesBuildReportString(t *testing.T) { + config, err := generateConfig(flag.ReportOptions{ + Report: "security", + Severity: map[string]bool{ + "critical": true, + "high": true, + "medium": true, + "low": true, + "warning": true, + }, + }) + // set rule version + config.BearerRulesVersion = "TEST" + config.Rules = map[string]*settings.Rule{} + + if err != nil { + t.Fatalf("failed to generate config:%s", err) + } + + dataflow := dummyDataflow() + + results, err := security.GetOutput(&dataflow, config) + if err != nil { + t.Fatalf("failed to generate security output err:%s", err) + } + + dummyGoclocLanguage := gocloc.Language{} + dummyGoclocResult := gocloc.Result{ + Total: &dummyGoclocLanguage, + Files: map[string]*gocloc.ClocFile{}, + Languages: map[string]*gocloc.Language{ + "Ruby": {}, + }, + MaxPathLength: 0, + } + + stringBuilder, _ := security.BuildReportString(config, results, &dummyGoclocResult, &dataflow) + cupaloy.SnapshotT(t, stringBuilder.String()) +} + func TestGetOutput(t *testing.T) { config, err := generateConfig(flag.ReportOptions{ Report: "security", From dc366b18e9ccc0a78e7824dd72f85da72dda22ab Mon Sep 17 00:00:00 2001 From: gotbadger Date: Wed, 28 Jun 2023 15:27:07 +0100 Subject: [PATCH 2/3] fix: remove some old references to summary report --- docs/guides/custom-rule.md | 10 +++++----- .../output/security/.snapshots/TestBuildReportString | 2 +- .../security/.snapshots/TestNoRulesBuildReportString | 2 +- pkg/report/output/security/security.go | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/guides/custom-rule.md b/docs/guides/custom-rule.md index c5cc8218d..dc471978c 100644 --- a/docs/guides/custom-rule.md +++ b/docs/guides/custom-rule.md @@ -30,13 +30,13 @@ the matched code. - `false`: Default. Rule triggers whether or not any data types have been detected in the application. - `true`: Rule only triggers if at least one data type is detected in the application. - `severity`: This sets the lowest severity level of the rule, by default at `low`. The severity level can [automatically increase based on multiple factors](/explanations/severity). A severity level of `warning`, however, will never increase and won’t cause CI to fail.. Bearer CLI groups rule findings by severity, and you can configure the security report to only trigger on specific severity thresholds. -- `metadata`: Rule metadata is used for output to the summary report, and documentation for the internal rules. +- `metadata`: Rule metadata is used for output to the security report, and documentation for the internal rules. - `id`: A unique identifier. Internal rules are named `lang_framework_rule_name`. For rules targeting the language core, `lang` is used instead of a framework name. For example `ruby_lang_logger` and `ruby_rails_logger`. For custom rules, you may consider appending your org name. - `description`: A brief, one-sentence description of the rule. The best practice is to make this an actionable “rule” phrase, such as “Do X” or “Do not do X in Y”. - `cwe_id`: The associated list of [CWE](https://cwe.mitre.org/) identifiers. (Optional) - `associated_recipe`: Links the rule to a [recipe]({{meta.sourcePath}}/tree/main/pkg/classification/db/recipes). Useful for associating a rule with a third party. Example: “Sentry” (Optional) - `remediation_message`: Used for internal rules, this builds the documentation page for a rule. (Optional) - - `documentation_url`: Used to pass custom documentation URL for the summary report. This can be useful for linking to your own internal documentation or policies. By default, all rules in the main repo will automatically generate a link to the rule on [docs.bearer.com](/). (Optional) + - `documentation_url`: Used to pass custom documentation URL for the security report. This can be useful for linking to your own internal documentation or policies. By default, all rules in the main repo will automatically generate a link to the rule on [docs.bearer.com](/). (Optional) - `auxiliary`: Allows you to define helper rules and detectors to make pattern-building more robust. Auxiliary rules contain a unique `id` and their own `patterns` in the same way rules do. You’re unlikely to use this regularly. See the [weak_encryption](https://github.com/Bearer/bearer-rules/blob/main/ruby/lang/weak_encryption.yml) rule for examples. In addition, see our advice on how to avoid [variable joining](#variable-joining) in auxiliary rules. (Optional) - `skip_data_types`: Allows you to prevent the specified data types from triggering this rule. Takes an array of strings matching the data type names. Example: “Passwords”. (Optional) - `only_data_types`: Allows you to limit the specified data types that trigger this rule. Takes an array of strings matching the data type names. Example: “Passwords”. (Optional) @@ -109,7 +109,7 @@ patterns: $.get($<...>$$<...>) ``` -`$`: In some instances, a pattern requires some wider context to match the exact line of code where the rule occurs. For those cases, use this variable type to explicitly mark the line for Bearer CLI to highlight in the summary report. You’ll mostly need this for rules that target configuration files and settings, rather than logic-related code. For example: +`$`: In some instances, a pattern requires some wider context to match the exact line of code where the rule occurs. For those cases, use this variable type to explicitly mark the line for Bearer CLI to highlight in the security report. You’ll mostly need this for rules that target configuration files and settings, rather than logic-related code. For example: ```yaml patterns: @@ -219,11 +219,11 @@ Add the rule to your bearer config file. external-rule-dir: /path/to/rules/ ``` -*Note: Including an external rules directory adds custom rules to the summary report. To only run custom rules, you’ll need to use the `only-rule` flag or configuration setting and pass it the IDs of your custom rule.* +*Note: Including an external rules directory adds custom rules to the security report. To only run custom rules, you’ll need to use the `only-rule` flag or configuration setting and pass it the IDs of your custom rule.* ## Rule best practices -1. Matching patterns in a rule cause *rule findings*. Depending on the severity level, findings can cause CI to exit and will display in the summary report. Keep this in mind when writing patterns so you don’t match a best practice condition and trigger a failed scan. +1. Matching patterns in a rule cause *rule findings*. Depending on the severity level, findings can cause CI to exit and will display in the security report. Keep this in mind when writing patterns so you don’t match a best practice condition and trigger a failed scan. 2. Lean on the built-in resources, like the data type detectors and recipes before creating complex rules. ## Rule starter diff --git a/pkg/report/output/security/.snapshots/TestBuildReportString b/pkg/report/output/security/.snapshots/TestBuildReportString index 13889fdd8..54be0cf9e 100644 --- a/pkg/report/output/security/.snapshots/TestBuildReportString +++ b/pkg/report/output/security/.snapshots/TestBuildReportString @@ -1,6 +1,6 @@ -Summary Report +Security Report ===================================== diff --git a/pkg/report/output/security/.snapshots/TestNoRulesBuildReportString b/pkg/report/output/security/.snapshots/TestNoRulesBuildReportString index 1f5758966..f5c3a5949 100644 --- a/pkg/report/output/security/.snapshots/TestNoRulesBuildReportString +++ b/pkg/report/output/security/.snapshots/TestNoRulesBuildReportString @@ -1,6 +1,6 @@ -Summary Report +Security Report ===================================== diff --git a/pkg/report/output/security/security.go b/pkg/report/output/security/security.go index deb7f3a4d..9a81f4ea5 100644 --- a/pkg/report/output/security/security.go +++ b/pkg/report/output/security/security.go @@ -249,7 +249,7 @@ func BuildReportString(config settings.Config, results *Results, lineOfCodeOutpu severityForFailure := config.Report.Severity reportStr := &strings.Builder{} - reportStr.WriteString("\n\nSummary Report\n") + reportStr.WriteString("\n\nSecurity Report\n") reportStr.WriteString("\n=====================================") initialColorSetting := color.NoColor From d589497049c72183a0b0b0ee4d7de37216546e2b Mon Sep 17 00:00:00 2001 From: gotbadger Date: Wed, 28 Jun 2023 15:46:06 +0100 Subject: [PATCH 3/3] fix: message formatting --- pkg/commands/process/settings/ruleLoader.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/commands/process/settings/ruleLoader.go b/pkg/commands/process/settings/ruleLoader.go index 48f8f4975..bf0ddf932 100644 --- a/pkg/commands/process/settings/ruleLoader.go +++ b/pkg/commands/process/settings/ruleLoader.go @@ -32,10 +32,10 @@ func LoadRuleDefinitionsFromGitHub(ruleDefinitions map[string]RuleDefinition, fo resetString := headers.Get("x-ratelimit-reset") unixTime, err := strconv.ParseInt(resetString, 10, 64) if err != nil { - return tagName, fmt.Errorf("rules download is rate limited please wait until: %s", resetString) + return tagName, fmt.Errorf("rules download is rate limited. Please wait until: %s", resetString) } tm := time.Unix(unixTime, 0) - return tagName, fmt.Errorf("rules download is rate limited please wait until: %s", tm.Format("2006-01-02 15:04:05")) + return tagName, fmt.Errorf("rules download is rate limited. Please wait until: %s", tm.Format("2006-01-02 15:04:05")) } if resp.StatusCode != 200 {