Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rules): improve rule dx #1516

Merged
merged 1 commit into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
--
Analyzing codebase
Loading rules
Scanning target e2e/flags/testdata/simple
Scanning target e2e/flags/testdata/ok
Running Detectors
Generating dataflow
Evaluating rules
Expand Down
13 changes: 13 additions & 0 deletions e2e/flags/.snapshots/TestNoExternalRuleDir-report-dataflow
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@


Security Report

=====================================

Zero rules found. A security report requires rules to function. Please check configuration.


--
Analyzing codebase
Error: report error: 0 rules found for supported language, default rules could not be downloaded or possibly disabled without using --external-rule-dir

36 changes: 36 additions & 0 deletions e2e/flags/.snapshots/TestSkipRulesFlag-report-dataflow
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@


Security Report

=====================================

Rules:
Language Default Rules Custom Rules Files
Ruby 0 1 1


HIGH: Ruby logger [CWE-42]
To ignore this finding, run: bearer ignore add fa5e03644738e4c17cbbd04a580506b1_0

File: e2e/flags/testdata/simple/main.rb:1

1 logger.info("user info", user.email)
=====================================

1 checks, 1 findings

CRITICAL: 0
HIGH: 1 (CWE-42)
MEDIUM: 0
LOW: 0
WARNING: 0

Need help or want to discuss the output? Join the Community https://discord.gg/eaHZBJUXRF

Retain state and manage your findings directly on Bearer Cloud. Learn more at https://docs.bearer.com/guides/bearer-cloud/


--
Analyzing codebase
Warning: rule IDs bar,foo given to be skipped but were not found

3 changes: 2 additions & 1 deletion e2e/flags/api_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ func TestApiKeyFlags(t *testing.T) {
t.Parallel()
arguments := []string{
"scan",
filepath.Join("e2e", "flags", "testdata", "simple"),
filepath.Join("e2e", "flags", "testdata", "ok"),
"--disable-version-check",
"--disable-default-rules",
"--external-rule-dir", "e2e/testdata/rules",
"--api-key",
"123",
"--format",
Expand Down
20 changes: 20 additions & 0 deletions e2e/flags/report_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,26 @@ func TestReportFlags(t *testing.T) {
testhelper.RunTests(t, tests)
}

func TestNoExternalRuleDir(t *testing.T) {
tests := []testhelper.TestCase{
newScanTest("report-dataflow", []string{"--report=security"}),
}
for i := range tests {
tests[i].ShouldSucceed = false
}
testhelper.RunTestsWithSnapshotSubdirectory(t, tests, ".snapshots")
}

func TestSkipRulesFlag(t *testing.T) {
tests := []testhelper.TestCase{
newScanTest("report-dataflow", []string{"--report=security", "--external-rule-dir=e2e/testdata/rules", "--skip-rule=bar,foo"}),
}
for i := range tests {
tests[i].ShouldSucceed = false
}
testhelper.RunTests(t, tests)
}

func TestReportFlagsShouldFail(t *testing.T) {
t.Parallel()
tests := []testhelper.TestCase{
Expand Down
1 change: 1 addition & 0 deletions e2e/flags/testdata/ok/main.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
logger.info("ok")
5 changes: 5 additions & 0 deletions internal/commands/artifact/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"os"
"slices"
"sort"
"strings"
"time"
Expand Down Expand Up @@ -451,6 +452,10 @@ func (r *runner) Report(
}
}

if len(r.scanSettings.Rules) == 0 && slices.Contains(r.scanSettings.Scan.Scanner, flag.ScannerSAST) && r.scanSettings.Report.Report == flag.ReportSecurity {
return false, fmt.Errorf("%d rules found for supported language, default rules could not be downloaded or possibly disabled without using --external-rule-dir", len(r.scanSettings.Rules))
}

return reportData.ReportFailed, nil
}

Expand Down
9 changes: 6 additions & 3 deletions internal/commands/process/settings/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,18 +325,21 @@ func validateRuleOptionIDs(
invalidRuleIDs = append(invalidRuleIDs, id)
}
}

var invalidSkipRuleIDs []string
for id := range options.SkipRule {
_, existsInDefinition := definitions[id]
_, existsInBuiltInDefinition := builtInDefinitions[id]

if !existsInBuiltInDefinition && !existsInDefinition {
invalidRuleIDs = append(invalidRuleIDs, id)
invalidSkipRuleIDs = append(invalidSkipRuleIDs, id)
}
}

if len(invalidSkipRuleIDs) > 0 {
output.StdErrLog(fmt.Sprintf("Warning: rule IDs %s given to be skipped but were not found", strings.Join(invalidSkipRuleIDs, ",")))
gotbadger marked this conversation as resolved.
Show resolved Hide resolved
}
if len(invalidRuleIDs) > 0 {
return fmt.Errorf("invalid rule IDs in only/skip option: %s", strings.Join(invalidRuleIDs, ","))
return fmt.Errorf("invalid rule IDs in only option: %s", strings.Join(invalidRuleIDs, ","))
}

return nil
Expand Down
Loading