From 4721240533b2c26a47db6047ddc51af4c4d59ba6 Mon Sep 17 00:00:00 2001 From: nikpivkin Date: Tue, 12 Nov 2024 17:21:12 +0600 Subject: [PATCH] chore: remove support for custom Terraform checks Signed-off-by: nikpivkin --- .../scanners/cloudformation/scanner_test.go | 5 +- pkg/iac/scanners/dockerfile/scanner_test.go | 8 +- pkg/iac/scanners/generic/scanner_test.go | 13 +- .../terraform/executor/executor_test.go | 132 ----------------- pkg/iac/scanners/terraform/executor/pool.go | 138 +----------------- pkg/iac/scanners/terraform/wildcard_test.go | 92 ------------ 6 files changed, 10 insertions(+), 378 deletions(-) delete mode 100644 pkg/iac/scanners/terraform/executor/executor_test.go delete mode 100644 pkg/iac/scanners/terraform/wildcard_test.go diff --git a/pkg/iac/scanners/cloudformation/scanner_test.go b/pkg/iac/scanners/cloudformation/scanner_test.go index 67ee92cf69c3..3af242712a05 100644 --- a/pkg/iac/scanners/cloudformation/scanner_test.go +++ b/pkg/iac/scanners/cloudformation/scanner_test.go @@ -79,10 +79,7 @@ deny[res] { Severity: "CRITICAL", Terraform: &scan.EngineMetadata{}, CloudFormation: &scan.EngineMetadata{}, - CustomChecks: scan.CustomChecks{ - Terraform: (*scan.TerraformCustomCheck)(nil), - }, - RegoPackage: "data.builtin.dockerfile.DS006", + RegoPackage: "data.builtin.dockerfile.DS006", Frameworks: map[framework.Framework][]string{ framework.Default: {}, }, diff --git a/pkg/iac/scanners/dockerfile/scanner_test.go b/pkg/iac/scanners/dockerfile/scanner_test.go index e8c66b27db08..3182b6d02e6f 100644 --- a/pkg/iac/scanners/dockerfile/scanner_test.go +++ b/pkg/iac/scanners/dockerfile/scanner_test.go @@ -249,9 +249,7 @@ USER root Severity: "CRITICAL", Terraform: &scan.EngineMetadata{}, CloudFormation: &scan.EngineMetadata{}, - CustomChecks: scan.CustomChecks{ - Terraform: (*scan.TerraformCustomCheck)(nil)}, - RegoPackage: "data.builtin.dockerfile.DS006", + RegoPackage: "data.builtin.dockerfile.DS006", Frameworks: map[framework.Framework][]string{ framework.Default: {}, }, @@ -600,9 +598,7 @@ COPY --from=dep /binary /` Severity: "CRITICAL", Terraform: &scan.EngineMetadata{}, CloudFormation: &scan.EngineMetadata{}, - CustomChecks: scan.CustomChecks{ - Terraform: (*scan.TerraformCustomCheck)(nil)}, - RegoPackage: "data.builtin.dockerfile.DS006", + RegoPackage: "data.builtin.dockerfile.DS006", Frameworks: map[framework.Framework][]string{ framework.Default: {}, }, diff --git a/pkg/iac/scanners/generic/scanner_test.go b/pkg/iac/scanners/generic/scanner_test.go index 46d0eef1653a..1bb637ac5a83 100644 --- a/pkg/iac/scanners/generic/scanner_test.go +++ b/pkg/iac/scanners/generic/scanner_test.go @@ -69,10 +69,7 @@ deny[res] { Severity: "CRITICAL", Terraform: &scan.EngineMetadata{}, CloudFormation: &scan.EngineMetadata{}, - CustomChecks: scan.CustomChecks{ - Terraform: (*scan.TerraformCustomCheck)(nil), - }, - RegoPackage: "data.builtin.json.lol", + RegoPackage: "data.builtin.json.lol", Frameworks: map[framework.Framework][]string{ framework.Default: {}, }, @@ -141,9 +138,7 @@ deny[res] { Severity: "CRITICAL", Terraform: &scan.EngineMetadata{}, CloudFormation: &scan.EngineMetadata{}, - CustomChecks: scan.CustomChecks{ - Terraform: (*scan.TerraformCustomCheck)(nil)}, - RegoPackage: "data.builtin.yaml.lol", + RegoPackage: "data.builtin.yaml.lol", Frameworks: map[framework.Framework][]string{ framework.Default: {}, }, @@ -211,9 +206,7 @@ deny[res] { Severity: "CRITICAL", Terraform: &scan.EngineMetadata{}, CloudFormation: &scan.EngineMetadata{}, - CustomChecks: scan.CustomChecks{ - Terraform: (*scan.TerraformCustomCheck)(nil)}, - RegoPackage: "data.builtin.toml.lol", + RegoPackage: "data.builtin.toml.lol", Frameworks: map[framework.Framework][]string{ framework.Default: {}, }, diff --git a/pkg/iac/scanners/terraform/executor/executor_test.go b/pkg/iac/scanners/terraform/executor/executor_test.go deleted file mode 100644 index 838701d163f4..000000000000 --- a/pkg/iac/scanners/terraform/executor/executor_test.go +++ /dev/null @@ -1,132 +0,0 @@ -package executor - -import ( - "context" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/aquasecurity/trivy/internal/testutil" - "github.com/aquasecurity/trivy/pkg/iac/providers" - "github.com/aquasecurity/trivy/pkg/iac/rules" - "github.com/aquasecurity/trivy/pkg/iac/scan" - "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" - "github.com/aquasecurity/trivy/pkg/iac/severity" - "github.com/aquasecurity/trivy/pkg/iac/terraform" -) - -var panicRule = scan.Rule{ - Provider: providers.AWSProvider, - Service: "service", - ShortCode: "abc", - Severity: severity.High, - CustomChecks: scan.CustomChecks{ - Terraform: &scan.TerraformCustomCheck{ - RequiredTypes: []string{"resource"}, - RequiredLabels: []string{"problem"}, - Check: func(resourceBlock *terraform.Block, _ *terraform.Module) (results scan.Results) { - if resourceBlock.GetAttribute("panic").IsTrue() { - panic("This is fine") - } - return - }, - }, - }, -} - -func Test_PanicInCheckNotAllowed(t *testing.T) { - - reg := rules.Register(panicRule) - defer rules.Deregister(reg) - - fs := testutil.CreateFS(t, map[string]string{ - "project/main.tf": ` -resource "problem" "this" { - panic = true -} -`, - }) - - p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) - err := p.ParseFS(context.TODO(), "project") - require.NoError(t, err) - modules, _, err := p.EvaluateAll(context.TODO()) - require.NoError(t, err) - - results, err := New().Execute(modules) - require.Error(t, err) - - assert.Empty(t, results.GetFailed()) -} - -func Test_PanicInCheckAllowed(t *testing.T) { - - reg := rules.Register(panicRule) - defer rules.Deregister(reg) - - fs := testutil.CreateFS(t, map[string]string{ - "project/main.tf": ` -resource "problem" "this" { - panic = true -} -`, - }) - - p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) - err := p.ParseFS(context.TODO(), "project") - require.NoError(t, err) - - modules, _, err := p.EvaluateAll(context.TODO()) - require.NoError(t, err) - _, err = New().Execute(modules) - require.Error(t, err) -} - -func Test_PanicNotInCheckNotIncludePassed(t *testing.T) { - - reg := rules.Register(panicRule) - defer rules.Deregister(reg) - - fs := testutil.CreateFS(t, map[string]string{ - "project/main.tf": ` -resource "problem" "this" { - panic = true -} -`, - }) - - p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) - err := p.ParseFS(context.TODO(), "project") - require.NoError(t, err) - modules, _, err := p.EvaluateAll(context.TODO()) - require.NoError(t, err) - - results, _ := New().Execute(modules) - require.NoError(t, err) - - assert.Empty(t, results.GetFailed()) -} - -func Test_PanicNotInCheckNotIncludePassedStopOnError(t *testing.T) { - - reg := rules.Register(panicRule) - defer rules.Deregister(reg) - - fs := testutil.CreateFS(t, map[string]string{ - "project/main.tf": ` -resource "problem" "this" { - panic = true -} -`, - }) - - p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) - err := p.ParseFS(context.TODO(), "project") - require.NoError(t, err) - modules, _, err := p.EvaluateAll(context.TODO()) - require.NoError(t, err) - - _, err = New().Execute(modules) - require.Error(t, err) -} diff --git a/pkg/iac/scanners/terraform/executor/pool.go b/pkg/iac/scanners/terraform/executor/pool.go index cc2091ff71ed..b822606a1ed6 100644 --- a/pkg/iac/scanners/terraform/executor/pool.go +++ b/pkg/iac/scanners/terraform/executor/pool.go @@ -3,10 +3,7 @@ package executor import ( "context" "fmt" - "os" - "path/filepath" runtimeDebug "runtime/debug" - "strings" "sync" "github.com/aquasecurity/trivy/pkg/iac/rego" @@ -62,21 +59,10 @@ func (p *Pool) Run() (scan.Results, error) { if !p.regoOnly { for _, r := range p.rules { - if r.GetRule().CustomChecks.Terraform != nil && r.GetRule().CustomChecks.Terraform.Check != nil { - // run local hcl rule - for _, module := range p.modules { - mod := *module - outgoing <- &hclModuleRuleJob{ - module: &mod, - rule: r, - } - } - } else { - // run defsec rule - outgoing <- &infraRuleJob{ - state: p.state, - rule: r, - } + // run defsec rule + outgoing <- &infraRuleJob{ + state: p.state, + rule: r, } } } @@ -103,11 +89,6 @@ type infraRuleJob struct { rule types.RegisteredRule } -type hclModuleRuleJob struct { - module *terraform.Module - rule types.RegisteredRule -} - type regoJob struct { state *state.State scanner *rego.Scanner @@ -124,23 +105,6 @@ func (h *infraRuleJob) Run() (_ scan.Results, err error) { return h.rule.Evaluate(h.state), err } -func (h *hclModuleRuleJob) Run() (results scan.Results, err error) { - 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) { - continue - } - results = append(results, customCheck.Check(block, h.module)...) - } - results.SetRule(h.rule.GetRule()) - return -} - func (h *regoJob) Run() (results scan.Results, err error) { regoResults, err := h.scanner.ScanInput(context.TODO(), rego.Input{ Contents: h.state.ToRego(), @@ -152,100 +116,6 @@ func (h *regoJob) Run() (results scan.Results, err error) { return regoResults, nil } -// nolint -func isCustomCheckRequiredForBlock(custom *scan.TerraformCustomCheck, b *terraform.Block) bool { - - var found bool - for _, requiredType := range custom.RequiredTypes { - if b.Type() == requiredType { - found = true - break - } - } - if !found && len(custom.RequiredTypes) > 0 { - return false - } - - found = false - for _, requiredLabel := range custom.RequiredLabels { - if requiredLabel == "*" || (len(b.Labels()) > 0 && wildcardMatch(requiredLabel, b.TypeLabel())) { - found = true - break - } - } - if !found && len(custom.RequiredLabels) > 0 { - return false - } - - found = false - if len(custom.RequiredSources) > 0 && b.Type() == terraform.TypeModule.Name() { - if sourceAttr := b.GetAttribute("source"); sourceAttr.IsNotNil() { - values := sourceAttr.AsStringValues().AsStrings() - if len(values) == 0 { - return false - } - sourcePath := values[0] - - // resolve module source path to path relative to cwd - if strings.HasPrefix(sourcePath, ".") { - sourcePath = cleanPathRelativeToWorkingDir(filepath.Dir(b.GetMetadata().Range().GetFilename()), sourcePath) - } - - for _, requiredSource := range custom.RequiredSources { - if requiredSource == "*" || wildcardMatch(requiredSource, sourcePath) { - found = true - break - } - } - } - return found - } - - return true -} - -func cleanPathRelativeToWorkingDir(dir, path string) string { - absPath := filepath.Clean(filepath.Join(dir, path)) - wDir, err := os.Getwd() - if err != nil { - return absPath - } - relPath, err := filepath.Rel(wDir, absPath) - if err != nil { - return absPath - } - return relPath -} - -func wildcardMatch(pattern, subject string) bool { - if pattern == "" { - return false - } - parts := strings.Split(pattern, "*") - var lastIndex int - for i, part := range parts { - if part == "" { - continue - } - if i == 0 { - if !strings.HasPrefix(subject, part) { - return false - } - } - if i == len(parts)-1 { - if !strings.HasSuffix(subject, part) { - return false - } - } - newIndex := strings.Index(subject, part) - if newIndex < lastIndex { - return false - } - lastIndex = newIndex - } - return true -} - type Worker struct { incoming <-chan Job mu sync.Mutex diff --git a/pkg/iac/scanners/terraform/wildcard_test.go b/pkg/iac/scanners/terraform/wildcard_test.go deleted file mode 100644 index 4e5ec3bacd9f..000000000000 --- a/pkg/iac/scanners/terraform/wildcard_test.go +++ /dev/null @@ -1,92 +0,0 @@ -package terraform - -import ( - "context" - "fmt" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/aquasecurity/trivy/internal/testutil" - "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/terraform" -) - -func Test_WildcardMatchingOnRequiredLabels(t *testing.T) { - - tests := []struct { - input string - pattern string - expectedFailure bool - }{ - { - pattern: "aws_*", - input: `resource "aws_instance" "blah" {}`, - expectedFailure: true, - }, - { - pattern: "gcp_*", - input: `resource "aws_instance" "blah" {}`, - expectedFailure: false, - }, - { - pattern: "x_aws_*", - input: `resource "aws_instance" "blah" {}`, - expectedFailure: false, - }, - { - pattern: "aws_security_group*", - input: `resource "aws_security_group" "blah" {}`, - expectedFailure: true, - }, - { - pattern: "aws_security_group*", - input: `resource "aws_security_group_rule" "blah" {}`, - expectedFailure: true, - }, - } - - for i, test := range tests { - - code := fmt.Sprintf("wild%d", i) - - t.Run(test.pattern, func(t *testing.T) { - - rule := scan.Rule{ - Service: "service", - ShortCode: code, - Summary: "blah", - Provider: "custom", - Severity: severity.High, - CustomChecks: scan.CustomChecks{ - Terraform: &scan.TerraformCustomCheck{ - RequiredTypes: []string{"resource"}, - RequiredLabels: []string{test.pattern}, - Check: func(resourceBlock *terraform.Block, _ *terraform.Module) (results scan.Results) { - results.Add("Custom check failed for resource.", resourceBlock) - return - }, - }, - }, - } - reg := rules.Register(rule) - defer rules.Deregister(reg) - - fsys := testutil.CreateFS(t, map[string]string{ - "main.tf": test.input, - }) - s := New() - results, err := s.ScanFS(context.TODO(), fsys, ".") - require.NoError(t, err) - - if test.expectedFailure { - testutil.AssertRuleFound(t, fmt.Sprintf("custom-service-%s", code), results, "") - } else { - testutil.AssertRuleNotFound(t, fmt.Sprintf("custom-service-%s", code), results, "") - } - }) - } - -}