Skip to content

Commit

Permalink
fix(terraform): do not scan local modules as root modules (#34)
Browse files Browse the repository at this point in the history
* fix(terraform): do not scan local modules as root modules

* chore: bump defsec

* test: skip integration tests in short mode

---------

Co-authored-by: simar7 <1254783+simar7@users.noreply.github.com>
  • Loading branch information
nikpivkin and simar7 authored Nov 1, 2023
1 parent d1753fd commit b08cb85
Show file tree
Hide file tree
Showing 8 changed files with 340 additions and 49 deletions.
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/Masterminds/semver v1.5.0
github.com/alecthomas/chroma v0.10.0 // indirect
github.com/apparentlymart/go-cidr v1.1.0
github.com/aquasecurity/defsec v0.93.2-0.20231020041402-7ccc46780c09
github.com/aquasecurity/defsec v0.93.2-0.20231024055158-015ab97ce898
github.com/aquasecurity/trivy-policies v0.3.1-0.20231021040354-0572a07131c2
github.com/aws/aws-sdk-go v1.44.245 // indirect
github.com/aws/smithy-go v1.14.2
Expand Down Expand Up @@ -39,6 +39,8 @@ require (

require github.com/mitchellh/mapstructure v1.5.0

require golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea

require (
cloud.google.com/go v0.110.4 // indirect
cloud.google.com/go/compute v1.21.0 // indirect
Expand Down
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,8 @@ github.com/apparentlymart/go-textseg/v13 v13.0.0 h1:Y+KvPE1NYz0xl601PVImeQfFyEy6
github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo=
github.com/apparentlymart/go-textseg/v15 v15.0.0 h1:uYvfpb3DyLSCGWnctWKGj857c6ew1u1fNQOlOtuGxQY=
github.com/apparentlymart/go-textseg/v15 v15.0.0/go.mod h1:K8XmNZdhEBkdlyDdvbmmsvpAG721bKi0joRfFdHIWJ4=
github.com/aquasecurity/defsec v0.93.2-0.20231020041402-7ccc46780c09 h1:dYBDwBnNzDsJr6l+FkrkrvWysAKc6VAO/leOcjvJfaA=
github.com/aquasecurity/defsec v0.93.2-0.20231020041402-7ccc46780c09/go.mod h1:J30VViSgmoW2Ic/6aqVJO2qvuADsmZ3MYuNxPcU6Vt0=
github.com/aquasecurity/defsec v0.93.2-0.20231024055158-015ab97ce898 h1:gu7XQvv2CswgzOdOFHg/AmtR4vBonG35XvGxHHvcIr4=
github.com/aquasecurity/defsec v0.93.2-0.20231024055158-015ab97ce898/go.mod h1:J30VViSgmoW2Ic/6aqVJO2qvuADsmZ3MYuNxPcU6Vt0=
github.com/aquasecurity/trivy-policies v0.3.1-0.20231021040354-0572a07131c2 h1:Xkm2i9Dy98p/DMR0smfog487zaTJ11hLVL+PvIgVWyM=
github.com/aquasecurity/trivy-policies v0.3.1-0.20231021040354-0572a07131c2/go.mod h1:Wqj81EIp4lDQGVzbPalKLNucR7c96YLQbfdA60KpEkQ=
github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0 h1:jfIu9sQUG6Ig+0+Ap1h4unLjW6YQJpKZVmUzxsD4E/Q=
Expand Down Expand Up @@ -836,6 +836,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea h1:vLCWI/yYrdEHyN2JzIzPO3aaQJHQdp89IZBA/+azVC4=
golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w=
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
Expand Down
13 changes: 11 additions & 2 deletions pkg/scanners/terraform/parser/evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str
parseDuration += time.Since(start)

e.debug.Log("Starting submodule evaluation...")
var modules []*terraform.Module
var modules terraform.Modules
for _, definition := range e.loadModules(ctx) {
submodules, outputs, err := definition.Parser.EvaluateAll(ctx)
if err != nil {
Expand Down Expand Up @@ -187,7 +187,16 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str

e.debug.Log("Module evaluation complete.")
parseDuration += time.Since(start)
return append([]*terraform.Module{terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores)}, modules...), fsMap, parseDuration
rootModule := terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores, e.isModuleLocal())
for _, m := range modules {
m.SetParent(rootModule)
}
return append(terraform.Modules{rootModule}, modules...), fsMap, parseDuration
}

func (e *evaluator) isModuleLocal() bool {
// the module source is empty only for local modules
return e.parentParser.moduleSource == ""
}

func (e *evaluator) expandBlocks(blocks terraform.Blocks) terraform.Blocks {
Expand Down
51 changes: 51 additions & 0 deletions pkg/scanners/terraform/parser/parser_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package parser

import (
"context"
"testing"

"github.com/aquasecurity/trivy-iac/test/testutil"
"github.com/stretchr/testify/require"
)

func Test_DefaultRegistry(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
fs := testutil.CreateFS(t, map[string]string{
"code/test.tf": `
module "registry" {
source = "terraform-aws-modules/vpc/aws"
}
`,
})

parser := New(fs, "", OptionStopOnHCLError(true))
if err := parser.ParseFS(context.TODO(), "code"); err != nil {
t.Fatal(err)
}
modules, _, err := parser.EvaluateAll(context.TODO())
require.NoError(t, err)
require.Len(t, modules, 2)
}

func Test_SpecificRegistry(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
fs := testutil.CreateFS(t, map[string]string{
"code/test.tf": `
module "registry" {
source = "registry.terraform.io/terraform-aws-modules/vpc/aws"
}
`,
})

parser := New(fs, "", OptionStopOnHCLError(true))
if err := parser.ParseFS(context.TODO(), "code"); err != nil {
t.Fatal(err)
}
modules, _, err := parser.EvaluateAll(context.TODO())
require.NoError(t, err)
require.Len(t, modules, 2)
}
38 changes: 0 additions & 38 deletions pkg/scanners/terraform/parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -592,44 +592,6 @@ resource "something" "blah" {
assert.Equal(t, true, values[2].GetMetadata().IsResolvable())
}

func Test_DefaultRegistry(t *testing.T) {

fs := testutil.CreateFS(t, map[string]string{
"code/test.tf": `
module "registry" {
source = "terraform-aws-modules/vpc/aws"
}
`,
})

parser := New(fs, "", OptionStopOnHCLError(true))
if err := parser.ParseFS(context.TODO(), "code"); err != nil {
t.Fatal(err)
}
modules, _, err := parser.EvaluateAll(context.TODO())
require.NoError(t, err)
require.Len(t, modules, 2)
}

func Test_SpecificRegistry(t *testing.T) {

fs := testutil.CreateFS(t, map[string]string{
"code/test.tf": `
module "registry" {
source = "registry.terraform.io/terraform-aws-modules/vpc/aws"
}
`,
})

parser := New(fs, "", OptionStopOnHCLError(true))
if err := parser.ParseFS(context.TODO(), "code"); err != nil {
t.Fatal(err)
}
modules, _, err := parser.EvaluateAll(context.TODO())
require.NoError(t, err)
require.Len(t, modules, 2)
}

func Test_NullDefaultValueForVar(t *testing.T) {
fs := testutil.CreateFS(t, map[string]string{
"test.tf": `
Expand Down
49 changes: 43 additions & 6 deletions pkg/scanners/terraform/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import (
"github.com/aquasecurity/defsec/pkg/framework"
"github.com/aquasecurity/defsec/pkg/scan"
"github.com/aquasecurity/defsec/pkg/scanners/options"
"github.com/aquasecurity/defsec/pkg/terraform"
"github.com/aquasecurity/defsec/pkg/types"
"golang.org/x/exp/slices"

"github.com/aquasecurity/trivy-iac/pkg/extrafs"
"github.com/aquasecurity/trivy-iac/pkg/rego"
Expand Down Expand Up @@ -158,6 +160,31 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) (*rego.Scanner, error) {
return regoScanner, nil
}

// terraformRootModule represents the module to be used as the root module for Terraform deployment.
type terraformRootModule struct {
rootPath string
childs terraform.Modules
fsMap map[string]fs.FS
}

func excludeNonRootModules(modules []terraformRootModule) []terraformRootModule {
var result []terraformRootModule
var childPaths []string

for _, module := range modules {
childPaths = append(childPaths, module.childs.ChildModulesPaths()...)
}

for _, module := range modules {
// if the path of the root module matches the path of the child module,
// then we should not scan it
if !slices.Contains(childPaths, module.rootPath) {
result = append(result, module)
}
}
return result
}

func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir string) (scan.Results, Metrics, error) {

var metrics Metrics
Expand Down Expand Up @@ -185,14 +212,12 @@ func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir strin
var allResults scan.Results

// parse all root module directories
var rootModules []terraformRootModule
for _, dir := range rootDirs {

s.debug.Log("Scanning root module '%s'...", dir)

p := parser.New(target, "", s.parserOpt...)
s.execLock.RLock()
e := executor.New(s.executorOpt...)
s.execLock.RUnlock()

if err := p.ParseFS(ctx, dir); err != nil {
return nil, metrics, err
Expand All @@ -210,12 +235,24 @@ func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir strin
metrics.Parser.Timings.DiskIODuration += parserMetrics.Timings.DiskIODuration
metrics.Parser.Timings.ParseDuration += parserMetrics.Timings.ParseDuration

results, execMetrics, err := e.Execute(modules)
rootModules = append(rootModules, terraformRootModule{
rootPath: dir,
childs: modules,
fsMap: p.GetFilesystemMap(),
})
}

rootModules = excludeNonRootModules(rootModules)

for _, module := range rootModules {
s.execLock.RLock()
e := executor.New(s.executorOpt...)
s.execLock.RUnlock()
results, execMetrics, err := e.Execute(module.childs)
if err != nil {
return nil, metrics, err
}

fsMap := p.GetFilesystemMap()
for i, result := range results {
if result.Metadata().Range().GetFS() != nil {
continue
Expand All @@ -224,7 +261,7 @@ func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir strin
if key == "" {
continue
}
if filesystem, ok := fsMap[key]; ok {
if filesystem, ok := module.fsMap[key]; ok {
override := scan.Results{
result,
}
Expand Down
130 changes: 130 additions & 0 deletions pkg/scanners/terraform/scanner_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
package terraform

import (
"bytes"
"context"
"fmt"
"testing"

"github.com/aquasecurity/defsec/pkg/scanners/options"
"github.com/aquasecurity/trivy-iac/test/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func Test_ScanRemoteModule(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
fs := testutil.CreateFS(t, map[string]string{
"main.tf": `
module "s3_bucket" {
source = "terraform-aws-modules/s3-bucket/aws"
bucket = "my-s3-bucket"
}
`,
"/rules/bucket_name.rego": `
# METADATA
# schemas:
# - input: schema.input
# custom:
# avd_id: AVD-AWS-0001
# input:
# selector:
# - type: cloud
# subtypes:
# - service: s3
# provider: aws
package defsec.test.aws1
deny[res] {
bucket := input.aws.s3.buckets[_]
bucket.name.value == ""
res := result.new("The name of the bucket must not be empty", bucket)
}`,
})

debugLog := bytes.NewBuffer([]byte{})

scanner := New(
options.ScannerWithDebug(debugLog),
options.ScannerWithPolicyFilesystem(fs),
options.ScannerWithPolicyDirs("rules"),
options.ScannerWithEmbeddedPolicies(false),
options.ScannerWithEmbeddedLibraries(false),
options.ScannerWithRegoOnly(true),
ScannerWithAllDirectories(true),
)

results, err := scanner.ScanFS(context.TODO(), fs, ".")
require.NoError(t, err)

assert.Len(t, results.GetPassed(), 1)

if t.Failed() {
fmt.Printf("Debug logs:\n%s\n", debugLog.String())
}
}

func Test_ScanChildUseRemoteModule(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
fs := testutil.CreateFS(t, map[string]string{
"main.tf": `
module "this" {
source = "./modules/s3"
bucket = "my-s3-bucket"
}
`,
"modules/s3/main.tf": `
variable "bucket" {
type = string
}
module "s3_bucket" {
source = "github.com/terraform-aws-modules/terraform-aws-s3-bucket?ref=v3.15.1"
bucket = var.bucket
}
`,
"rules/bucket_name.rego": `
# METADATA
# schemas:
# - input: schema.input
# custom:
# avd_id: AVD-AWS-0001
# input:
# selector:
# - type: cloud
# subtypes:
# - service: s3
# provider: aws
package defsec.test.aws1
deny[res] {
bucket := input.aws.s3.buckets[_]
bucket.name.value == ""
res := result.new("The name of the bucket must not be empty", bucket)
}`,
})

debugLog := bytes.NewBuffer([]byte{})

scanner := New(
options.ScannerWithDebug(debugLog),
options.ScannerWithPolicyFilesystem(fs),
options.ScannerWithPolicyDirs("rules"),
options.ScannerWithEmbeddedPolicies(false),
options.ScannerWithEmbeddedLibraries(false),
options.ScannerWithRegoOnly(true),
ScannerWithAllDirectories(true),
)

results, err := scanner.ScanFS(context.TODO(), fs, ".")
require.NoError(t, err)

assert.Len(t, results.GetPassed(), 1)

if t.Failed() {
fmt.Printf("Debug logs:\n%s\n", debugLog.String())
}
}
Loading

0 comments on commit b08cb85

Please sign in to comment.