From 6bca7c3c79ec43b7cc1bf3cee3ea027e8b8524f5 Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Thu, 4 Apr 2024 03:29:31 +0300 Subject: [PATCH] refactor(terraform): remove unused options (#6446) --- .../scanners/terraform/executor/executor.go | 111 +++--------- .../terraform/executor/executor_test.go | 5 +- pkg/iac/scanners/terraform/executor/option.go | 43 ----- pkg/iac/scanners/terraform/executor/pool.go | 70 ++++---- pkg/iac/scanners/terraform/options.go | 110 +----------- pkg/iac/scanners/terraform/scanner_test.go | 164 ------------------ 6 files changed, 56 insertions(+), 447 deletions(-) diff --git a/pkg/iac/scanners/terraform/executor/executor.go b/pkg/iac/scanners/terraform/executor/executor.go index 96c4939d756b..88dc1fa9801c 100644 --- a/pkg/iac/scanners/terraform/executor/executor.go +++ b/pkg/iac/scanners/terraform/executor/executor.go @@ -14,35 +14,24 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/rules" "github.com/aquasecurity/trivy/pkg/iac/scan" - "github.com/aquasecurity/trivy/pkg/iac/severity" - "github.com/aquasecurity/trivy/pkg/iac/state" "github.com/aquasecurity/trivy/pkg/iac/terraform" "github.com/aquasecurity/trivy/pkg/iac/types" ) // Executor scans HCL blocks by running all registered rules against them type Executor struct { - enableIgnores bool - excludedRuleIDs []string - includedRuleIDs []string - ignoreCheckErrors bool - workspaceName string - useSingleThread bool - debug debug.Logger - resultsFilters []func(scan.Results) scan.Results - severityOverrides map[string]string - regoScanner *rego.Scanner - regoOnly bool - stateFuncs []func(*state.State) - frameworks []framework.Framework + workspaceName string + debug debug.Logger + resultsFilters []func(scan.Results) scan.Results + regoScanner *rego.Scanner + regoOnly bool + frameworks []framework.Framework } // New creates a new Executor func New(options ...Option) *Executor { s := &Executor{ - ignoreCheckErrors: true, - enableIgnores: true, - regoOnly: false, + regoOnly: false, } for _, option := range options { option(s) @@ -50,16 +39,6 @@ func New(options ...Option) *Executor { return s } -// Find element in list -func checkInList(id string, list []string) bool { - for _, codeIgnored := range list { - if codeIgnored == id { - return true - } - } - return false -} - func (e *Executor) Execute(modules terraform.Modules) (scan.Results, error) { e.debug.Log("Adapting modules...") @@ -70,90 +49,46 @@ func (e *Executor) Execute(modules terraform.Modules) (scan.Results, error) { if threads > 1 { threads-- } - if e.useSingleThread { - threads = 1 - } - e.debug.Log("Using max routines of %d", threads) - e.debug.Log("Applying state modifier functions...") - for _, f := range e.stateFuncs { - f(infra) - } + e.debug.Log("Using max routines of %d", threads) registeredRules := rules.GetRegistered(e.frameworks...) e.debug.Log("Initialized %d rule(s).", len(registeredRules)) - pool := NewPool(threads, registeredRules, modules, infra, e.ignoreCheckErrors, e.regoScanner, e.regoOnly) + pool := NewPool(threads, registeredRules, modules, infra, e.regoScanner, e.regoOnly) e.debug.Log("Created pool with %d worker(s) to apply rules.", threads) + results, err := pool.Run() if err != nil { return nil, err } - e.debug.Log("Finished applying rules.") - if e.enableIgnores { - e.debug.Log("Applying ignores...") - var ignores ignore.Rules - for _, module := range modules { - ignores = append(ignores, module.Ignores()...) - } + e.debug.Log("Finished applying rules.") - ignorers := map[string]ignore.Ignorer{ - "ws": workspaceIgnorer(e.workspaceName), - "ignore": attributeIgnorer(modules), - } + e.debug.Log("Applying ignores...") + var ignores ignore.Rules + for _, module := range modules { + ignores = append(ignores, module.Ignores()...) + } - results.Ignore(ignores, ignorers) + ignorers := map[string]ignore.Ignorer{ + "ws": workspaceIgnorer(e.workspaceName), + "ignore": attributeIgnorer(modules), + } - for _, ignored := range results.GetIgnored() { - e.debug.Log("Ignored '%s' at '%s'.", ignored.Rule().LongID(), ignored.Range()) - } + results.Ignore(ignores, ignorers) - } else { - e.debug.Log("Ignores are disabled.") + for _, ignored := range results.GetIgnored() { + e.debug.Log("Ignored '%s' at '%s'.", ignored.Rule().LongID(), ignored.Range()) } - results = e.updateSeverity(results) results = e.filterResults(results) e.sortResults(results) return results, nil } -func (e *Executor) updateSeverity(results []scan.Result) scan.Results { - if len(e.severityOverrides) == 0 { - return results - } - - var overriddenResults scan.Results - for _, res := range results { - for code, sev := range e.severityOverrides { - if res.Rule().LongID() != code { - continue - } - - overrides := scan.Results([]scan.Result{res}) - override := res.Rule() - override.Severity = severity.Severity(sev) - overrides.SetRule(override) - res = overrides[0] - } - overriddenResults = append(overriddenResults, res) - } - - return overriddenResults -} - func (e *Executor) filterResults(results scan.Results) scan.Results { - includedOnly := len(e.includedRuleIDs) > 0 - for i, result := range results { - id := result.Rule().LongID() - if (includedOnly && !checkInList(id, e.includedRuleIDs)) || checkInList(id, e.excludedRuleIDs) { - e.debug.Log("Excluding '%s' at '%s'.", result.Rule().LongID(), result.Range()) - results[i].OverrideStatus(scan.StatusIgnored) - } - } - if len(e.resultsFilters) > 0 && len(results) > 0 { before := len(results.GetIgnored()) e.debug.Log("Applying %d results filters to %d results...", len(results), before) diff --git a/pkg/iac/scanners/terraform/executor/executor_test.go b/pkg/iac/scanners/terraform/executor/executor_test.go index c8c3e2af60c5..ac663c313c17 100644 --- a/pkg/iac/scanners/terraform/executor/executor_test.go +++ b/pkg/iac/scanners/terraform/executor/executor_test.go @@ -78,8 +78,7 @@ resource "problem" "this" { modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - - _, err = New(OptionStopOnErrors(false)).Execute(modules) + _, err = New().Execute(modules) assert.Error(t, err) } @@ -127,6 +126,6 @@ resource "problem" "this" { modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - _, err = New(OptionStopOnErrors(false)).Execute(modules) + _, err = New().Execute(modules) assert.Error(t, err) } diff --git a/pkg/iac/scanners/terraform/executor/option.go b/pkg/iac/scanners/terraform/executor/option.go index 1e9ab5b9d998..a58d72867b54 100644 --- a/pkg/iac/scanners/terraform/executor/option.go +++ b/pkg/iac/scanners/terraform/executor/option.go @@ -7,7 +7,6 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/scan" - "github.com/aquasecurity/trivy/pkg/iac/state" ) type Option func(s *Executor) @@ -24,66 +23,24 @@ func OptionWithResultsFilter(f func(scan.Results) scan.Results) Option { } } -func OptionWithSeverityOverrides(overrides map[string]string) Option { - return func(s *Executor) { - s.severityOverrides = overrides - } -} - func OptionWithDebugWriter(w io.Writer) Option { return func(s *Executor) { s.debug = debug.New(w, "terraform", "executor") } } -func OptionNoIgnores() Option { - return func(s *Executor) { - s.enableIgnores = false - } -} - -func OptionExcludeRules(ruleIDs []string) Option { - return func(s *Executor) { - s.excludedRuleIDs = ruleIDs - } -} - -func OptionIncludeRules(ruleIDs []string) Option { - return func(s *Executor) { - s.includedRuleIDs = ruleIDs - } -} - -func OptionStopOnErrors(stop bool) Option { - return func(s *Executor) { - s.ignoreCheckErrors = !stop - } -} - func OptionWithWorkspaceName(workspaceName string) Option { return func(s *Executor) { s.workspaceName = workspaceName } } -func OptionWithSingleThread(single bool) Option { - return func(s *Executor) { - s.useSingleThread = single - } -} - func OptionWithRegoScanner(s *rego.Scanner) Option { return func(e *Executor) { e.regoScanner = s } } -func OptionWithStateFunc(f ...func(*state.State)) Option { - return func(e *Executor) { - e.stateFuncs = f - } -} - func OptionWithRegoOnly(regoOnly bool) Option { return func(e *Executor) { e.regoOnly = regoOnly diff --git a/pkg/iac/scanners/terraform/executor/pool.go b/pkg/iac/scanners/terraform/executor/pool.go index a62fbe510de0..69b8405ee3a7 100644 --- a/pkg/iac/scanners/terraform/executor/pool.go +++ b/pkg/iac/scanners/terraform/executor/pool.go @@ -17,24 +17,22 @@ import ( ) type Pool struct { - size int - modules terraform.Modules - state *state.State - rules []types.RegisteredRule - ignoreErrors bool - rs *rego.Scanner - regoOnly bool + size int + modules terraform.Modules + state *state.State + rules []types.RegisteredRule + rs *rego.Scanner + regoOnly bool } -func NewPool(size int, rules []types.RegisteredRule, modules terraform.Modules, st *state.State, ignoreErrors bool, regoScanner *rego.Scanner, regoOnly bool) *Pool { +func NewPool(size int, rules []types.RegisteredRule, modules terraform.Modules, st *state.State, regoScanner *rego.Scanner, regoOnly bool) *Pool { return &Pool{ - size: size, - rules: rules, - state: st, - modules: modules, - ignoreErrors: ignoreErrors, - rs: regoScanner, - regoOnly: regoOnly, + size: size, + rules: rules, + state: st, + modules: modules, + rs: regoScanner, + regoOnly: regoOnly, } } @@ -69,17 +67,15 @@ func (p *Pool) Run() (scan.Results, error) { for _, module := range p.modules { mod := *module outgoing <- &hclModuleRuleJob{ - module: &mod, - rule: r, - ignoreErrors: p.ignoreErrors, + module: &mod, + rule: r, } } } else { // run defsec rule outgoing <- &infraRuleJob{ - state: p.state, - rule: r, - ignoreErrors: p.ignoreErrors, + state: p.state, + rule: r, } } } @@ -105,14 +101,11 @@ type Job interface { type infraRuleJob struct { state *state.State rule types.RegisteredRule - - ignoreErrors bool } type hclModuleRuleJob struct { - module *terraform.Module - rule types.RegisteredRule - ignoreErrors bool + module *terraform.Module + rule types.RegisteredRule } type regoJob struct { @@ -122,24 +115,21 @@ type regoJob struct { } func (h *infraRuleJob) Run() (_ scan.Results, err error) { - if h.ignoreErrors { - defer func() { - if panicErr := recover(); panicErr != nil { - err = fmt.Errorf("%s\n%s", panicErr, string(runtimeDebug.Stack())) - } - }() - } + defer func() { + if panicErr := recover(); panicErr != nil { + err = fmt.Errorf("%s\n%s", panicErr, string(runtimeDebug.Stack())) + } + }() + return h.rule.Evaluate(h.state), err } func (h *hclModuleRuleJob) Run() (results scan.Results, err error) { - if h.ignoreErrors { - defer func() { - if panicErr := recover(); panicErr != nil { - err = fmt.Errorf("%s\n%s", panicErr, string(runtimeDebug.Stack())) - } - }() - } + defer func() { + if panicErr := recover(); panicErr != nil { + err = fmt.Errorf("%s\n%s", panicErr, string(runtimeDebug.Stack())) + } + }() customCheck := h.rule.GetRule().CustomChecks.Terraform for _, block := range h.module.GetBlocks() { if !isCustomCheckRequiredForBlock(customCheck, block) { diff --git a/pkg/iac/scanners/terraform/options.go b/pkg/iac/scanners/terraform/options.go index d78c1f0cf897..f5a0d2223534 100644 --- a/pkg/iac/scanners/terraform/options.go +++ b/pkg/iac/scanners/terraform/options.go @@ -8,8 +8,6 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/executor" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" - "github.com/aquasecurity/trivy/pkg/iac/severity" - "github.com/aquasecurity/trivy/pkg/iac/state" ) type ConfigurableTerraformScanner interface { @@ -27,46 +25,6 @@ func ScannerWithTFVarsPaths(paths ...string) options.ScannerOption { } } -func ScannerWithSeverityOverrides(overrides map[string]string) options.ScannerOption { - return func(s options.ConfigurableScanner) { - if tf, ok := s.(ConfigurableTerraformScanner); ok { - tf.AddExecutorOptions(executor.OptionWithSeverityOverrides(overrides)) - } - } -} - -func ScannerWithNoIgnores() options.ScannerOption { - return func(s options.ConfigurableScanner) { - if tf, ok := s.(ConfigurableTerraformScanner); ok { - tf.AddExecutorOptions(executor.OptionNoIgnores()) - } - } -} - -func ScannerWithExcludedRules(ruleIDs []string) options.ScannerOption { - return func(s options.ConfigurableScanner) { - if tf, ok := s.(ConfigurableTerraformScanner); ok { - tf.AddExecutorOptions(executor.OptionExcludeRules(ruleIDs)) - } - } -} - -func ScannerWithIncludedRules(ruleIDs []string) options.ScannerOption { - return func(s options.ConfigurableScanner) { - if tf, ok := s.(ConfigurableTerraformScanner); ok { - tf.AddExecutorOptions(executor.OptionIncludeRules(ruleIDs)) - } - } -} - -func ScannerWithStopOnRuleErrors(stop bool) options.ScannerOption { - return func(s options.ConfigurableScanner) { - if tf, ok := s.(ConfigurableTerraformScanner); ok { - tf.AddExecutorOptions(executor.OptionStopOnErrors(stop)) - } - } -} - func ScannerWithWorkspaceName(name string) options.ScannerOption { return func(s options.ConfigurableScanner) { if tf, ok := s.(ConfigurableTerraformScanner); ok { @@ -76,14 +34,6 @@ func ScannerWithWorkspaceName(name string) options.ScannerOption { } } -func ScannerWithSingleThread(single bool) options.ScannerOption { - return func(s options.ConfigurableScanner) { - if tf, ok := s.(ConfigurableTerraformScanner); ok { - tf.AddExecutorOptions(executor.OptionWithSingleThread(single)) - } - } -} - func ScannerWithAllDirectories(all bool) options.ScannerOption { return func(s options.ConfigurableScanner) { if tf, ok := s.(ConfigurableTerraformScanner); ok { @@ -92,14 +42,6 @@ func ScannerWithAllDirectories(all bool) options.ScannerOption { } } -func ScannerWithStopOnHCLError(stop bool) options.ScannerOption { - return func(s options.ConfigurableScanner) { - if tf, ok := s.(ConfigurableTerraformScanner); ok { - tf.AddParserOptions(parser.OptionStopOnHCLError(stop)) - } - } -} - func ScannerWithSkipDownloaded(skip bool) options.ScannerOption { return func(s options.ConfigurableScanner) { if !skip { @@ -109,10 +51,7 @@ func ScannerWithSkipDownloaded(skip bool) options.ScannerOption { tf.AddExecutorOptions(executor.OptionWithResultsFilter(func(results scan.Results) scan.Results { for i, result := range results { prefix := result.Range().GetSourcePrefix() - switch { - case prefix == "": - case strings.HasPrefix(prefix, "."): - default: + if prefix != "" && !strings.HasPrefix(prefix, ".") { results[i].OverrideStatus(scan.StatusIgnored) } } @@ -122,53 +61,6 @@ func ScannerWithSkipDownloaded(skip bool) options.ScannerOption { } } -func ScannerWithResultsFilter(f func(scan.Results) scan.Results) options.ScannerOption { - return func(s options.ConfigurableScanner) { - if tf, ok := s.(ConfigurableTerraformScanner); ok { - tf.AddExecutorOptions(executor.OptionWithResultsFilter(f)) - } - } -} - -func ScannerWithMinimumSeverity(minimum severity.Severity) options.ScannerOption { - min := severityAsOrdinal(minimum) - return func(s options.ConfigurableScanner) { - if tf, ok := s.(ConfigurableTerraformScanner); ok { - tf.AddExecutorOptions(executor.OptionWithResultsFilter(func(results scan.Results) scan.Results { - for i, result := range results { - if severityAsOrdinal(result.Severity()) < min { - results[i].OverrideStatus(scan.StatusIgnored) - } - } - return results - })) - } - } -} - -func severityAsOrdinal(sev severity.Severity) int { - switch sev { - case severity.Critical: - return 4 - case severity.High: - return 3 - case severity.Medium: - return 2 - case severity.Low: - return 1 - default: - return 0 - } -} - -func ScannerWithStateFunc(f ...func(*state.State)) options.ScannerOption { - return func(s options.ConfigurableScanner) { - if tf, ok := s.(ConfigurableTerraformScanner); ok { - tf.AddExecutorOptions(executor.OptionWithStateFunc(f...)) - } - } -} - func ScannerWithDownloadsAllowed(allowed bool) options.ScannerOption { return func(s options.ConfigurableScanner) { if tf, ok := s.(ConfigurableTerraformScanner); ok { diff --git a/pkg/iac/scanners/terraform/scanner_test.go b/pkg/iac/scanners/terraform/scanner_test.go index 020954d811db..047ceb972a2a 100644 --- a/pkg/iac/scanners/terraform/scanner_test.go +++ b/pkg/iac/scanners/terraform/scanner_test.go @@ -13,7 +13,6 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scan" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/severity" - "github.com/aquasecurity/trivy/pkg/iac/state" "github.com/aquasecurity/trivy/pkg/iac/terraform" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -68,20 +67,6 @@ func scanWithOptions(t *testing.T, code string, opt ...options.ScannerOption) sc return results } -func Test_OptionWithSeverityOverrides(t *testing.T) { - reg := rules.Register(alwaysFailRule) - defer rules.Deregister(reg) - - options := []options.ScannerOption{ - ScannerWithSeverityOverrides(map[string]string{"aws-service-abc": "LOW"}), - } - results := scanWithOptions(t, ` -resource "something" "else" {} -`, options...) - require.Len(t, results.GetFailed(), 1) - assert.Equal(t, severity.Low, results.GetFailed()[0].Severity()) -} - func Test_OptionWithDebugWriter(t *testing.T) { reg := rules.Register(alwaysFailRule) defer rules.Deregister(reg) @@ -97,67 +82,6 @@ resource "something" "else" {} require.Greater(t, buffer.Len(), 0) } -func Test_OptionNoIgnores(t *testing.T) { - reg := rules.Register(alwaysFailRule) - defer rules.Deregister(reg) - - scannerOpts := []options.ScannerOption{ - ScannerWithNoIgnores(), - } - results := scanWithOptions(t, ` -//tfsec:ignore:aws-service-abc -resource "something" "else" {} -`, scannerOpts...) - require.Len(t, results.GetFailed(), 1) - require.Len(t, results.GetIgnored(), 0) - -} - -func Test_OptionExcludeRules(t *testing.T) { - reg := rules.Register(alwaysFailRule) - defer rules.Deregister(reg) - - options := []options.ScannerOption{ - ScannerWithExcludedRules([]string{"aws-service-abc"}), - } - results := scanWithOptions(t, ` -resource "something" "else" {} -`, options...) - require.Len(t, results.GetFailed(), 0) - require.Len(t, results.GetIgnored(), 1) - -} - -func Test_OptionIncludeRules(t *testing.T) { - reg := rules.Register(alwaysFailRule) - defer rules.Deregister(reg) - - scannerOpts := []options.ScannerOption{ - ScannerWithIncludedRules([]string{"this-only"}), - } - results := scanWithOptions(t, ` -resource "something" "else" {} -`, scannerOpts...) - require.Len(t, results.GetFailed(), 0) - require.Len(t, results.GetIgnored(), 1) - -} - -func Test_OptionWithMinimumSeverity(t *testing.T) { - reg := rules.Register(alwaysFailRule) - defer rules.Deregister(reg) - - scannerOpts := []options.ScannerOption{ - ScannerWithMinimumSeverity(severity.Critical), - } - results := scanWithOptions(t, ` -resource "something" "else" {} -`, scannerOpts...) - require.Len(t, results.GetFailed(), 0) - require.Len(t, results.GetIgnored(), 1) - -} - func Test_OptionWithPolicyDirs(t *testing.T) { fs := testutil.CreateFS(t, map[string]string{ @@ -355,38 +279,6 @@ cause := bucket.name } -func Test_OptionWithStateFunc(t *testing.T) { - - fs := testutil.CreateFS(t, map[string]string{ - "code/main.tf": ` -resource "aws_s3_bucket" "my-bucket" { - bucket = "evil" -} -`, - }) - - var actual state.State - - debugLog := bytes.NewBuffer([]byte{}) - scanner := New( - options.ScannerWithDebug(debugLog), - ScannerWithStateFunc(func(s *state.State) { - require.NotNil(t, s) - actual = *s - }), - ) - - _, err := scanner.ScanFS(context.TODO(), fs, "code") - require.NoError(t, err) - - assert.Equal(t, 1, len(actual.AWS.S3.Buckets)) - - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } - -} - func Test_OptionWithRegoOnly(t *testing.T) { fs := testutil.CreateFS(t, map[string]string{ @@ -794,62 +686,6 @@ resource "aws_s3_bucket_public_access_block" "testB" { } } -func Test_RegoInput(t *testing.T) { - - var regoInput interface{} - - opts := []options.ScannerOption{ - ScannerWithStateFunc(func(s *state.State) { - regoInput = s.ToRego() - }), - } - _ = scanWithOptions(t, ` -resource "aws_security_group" "example_security_group" { - name = "example_security_group" - - description = "Example SG" - - ingress { - description = "Allow SSH" - from_port = 22 - to_port = 22 - protocol = "tcp" - cidr_blocks = ["1.2.3.4", "5.6.7.8"] - } - -} -`, opts...) - - outer, ok := regoInput.(map[string]interface{}) - require.True(t, ok) - aws, ok := outer["aws"].(map[string]interface{}) - require.True(t, ok) - ec2, ok := aws["ec2"].(map[string]interface{}) - require.True(t, ok) - sgs, ok := ec2["securitygroups"].([]interface{}) - require.True(t, ok) - require.Len(t, sgs, 1) - sg0, ok := sgs[0].(map[string]interface{}) - require.True(t, ok) - ingress, ok := sg0["ingressrules"].([]interface{}) - require.True(t, ok) - require.Len(t, ingress, 1) - ingress0, ok := ingress[0].(map[string]interface{}) - require.True(t, ok) - cidrs, ok := ingress0["cidrs"].([]interface{}) - require.True(t, ok) - require.Len(t, cidrs, 2) - - cidr0, ok := cidrs[0].(map[string]interface{}) - require.True(t, ok) - - cidr1, ok := cidrs[1].(map[string]interface{}) - require.True(t, ok) - - assert.Equal(t, "1.2.3.4", cidr0["value"]) - assert.Equal(t, "5.6.7.8", cidr1["value"]) -} - // PoC for replacing Go with Rego: AVD-AWS-0001 func Test_RegoRules(t *testing.T) {