Skip to content

Commit

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

* test: rename test

* run go mod tidy
  • Loading branch information
nikpivkin authored Oct 7, 2023
1 parent a143d8b commit 78aed65
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 19 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ require (
github.com/docker/docker v24.0.5+incompatible
github.com/mitchellh/mapstructure v1.5.0
github.com/testcontainers/testcontainers-go v0.22.0
golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea
k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5
)

Expand Down Expand Up @@ -238,7 +239,6 @@ require (
go.opentelemetry.io/otel v1.14.0 // indirect
go.opentelemetry.io/otel/trace v1.14.0 // indirect
go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect
golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea // indirect
golang.org/x/mod v0.10.0 // indirect
golang.org/x/net v0.10.0 // indirect
golang.org/x/oauth2 v0.7.0 // indirect
Expand Down
9 changes: 7 additions & 2 deletions pkg/scanners/terraform/parser/evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,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 @@ -190,7 +190,12 @@ 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)
for _, m := range modules {
m.SetParent(rootModule)
}
return append(terraform.Modules{rootModule}, modules...), fsMap, parseDuration
}

func (e *evaluator) expandBlocks(blocks terraform.Blocks) terraform.Blocks {
Expand Down
65 changes: 49 additions & 16 deletions pkg/scanners/terraform/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,20 @@ import (
"sync"
"time"

"github.com/aquasecurity/defsec/pkg/types"

"github.com/aquasecurity/defsec/pkg/framework"
"golang.org/x/exp/slices"

"github.com/aquasecurity/defsec/pkg/debug"

"github.com/aquasecurity/defsec/pkg/scanners/options"

"github.com/aquasecurity/defsec/pkg/extrafs"
"github.com/aquasecurity/defsec/pkg/framework"
"github.com/aquasecurity/defsec/pkg/rego"
"github.com/aquasecurity/defsec/pkg/scan"
"github.com/aquasecurity/defsec/pkg/scanners"
"github.com/aquasecurity/defsec/pkg/scanners/options"
"github.com/aquasecurity/defsec/pkg/scanners/terraform/executor"
"github.com/aquasecurity/defsec/pkg/scanners/terraform/parser"
"github.com/aquasecurity/defsec/pkg/scanners/terraform/parser/resolvers"

"github.com/aquasecurity/defsec/pkg/scan"

"github.com/aquasecurity/defsec/pkg/extrafs"
"github.com/aquasecurity/defsec/pkg/terraform"
"github.com/aquasecurity/defsec/pkg/types"
)

var _ scanners.FSScanner = (*Scanner)(nil)
Expand Down Expand Up @@ -168,6 +165,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 @@ -195,14 +217,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 @@ -220,12 +240,25 @@ 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)
p.GetFilesystemMap()
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 @@ -234,7 +267,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
98 changes: 98 additions & 0 deletions pkg/scanners/terraform/scanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1006,3 +1006,101 @@ deny[res] {
}

}

func Test_DoNotScanNonRootModules(t *testing.T) {
fs := testutil.CreateFS(t, map[string]string{
"/code/app1/main.tf": `
module "s3" {
source = "./modules/s3"
bucket_name = "test"
}
`,
"/code/app1/modules/s3/main.tf": `
variable "bucket_name" {
type = string
}
resource "aws_s3_bucket" "main" {
bucket = var.bucket_name
}
`,
"/code/app1/app2/main.tf": `
module "s3" {
source = "../modules/s3"
bucket_name = "test"
}
module "ec2" {
source = "./modules/ec2"
}
`,
"/code/app1/app2/modules/ec2/main.tf": `
variable "security_group_description" {
type = string
}
resource "aws_security_group" "main" {
description = var.security_group_description
}
`,
"/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)
}
`,
"/rules/sec_group_description.rego": `
# METADATA
# schemas:
# - input: schema.input
# custom:
# avd_id: AVD-AWS-0002
# input:
# selector:
# - type: cloud
# subtypes:
# - service: ec2
# provider: aws
package defsec.test.aws2
deny[res] {
group := input.aws.ec2.securitygroups[_]
group.description.value == ""
res := result.new("The description of the security group must not be empty", group)
}
`,
})

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, "code")
require.NoError(t, err)

assert.Len(t, results.GetPassed(), 2)
require.Len(t, results.GetFailed(), 1)
assert.Equal(t, "AVD-AWS-0002", results.GetFailed()[0].Rule().AVDID)

if t.Failed() {
fmt.Printf("Debug logs:\n%s\n", debugLog.String())
}
}
5 changes: 5 additions & 0 deletions pkg/terraform/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ type Module struct {
rootPath string
modulePath string
ignores Ignores
parent *Module
}

func NewModule(rootPath string, modulePath string, blocks Blocks, ignores Ignores) *Module {
Expand All @@ -32,6 +33,10 @@ func NewModule(rootPath string, modulePath string, blocks Blocks, ignores Ignore
}
}

func (c *Module) SetParent(parent *Module) {
c.parent = parent
}

func (c *Module) RootPath() string {
return c.rootPath
}
Expand Down
10 changes: 10 additions & 0 deletions pkg/terraform/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ import (

type Modules []*Module

func (m Modules) ChildModulesPaths() []string {
var result []string
for _, module := range m {
if module.parent != nil {
result = append(result, module.modulePath)
}
}
return result
}

type ResourceIDResolutions map[string]bool

func (r ResourceIDResolutions) Resolve(id string) {
Expand Down

0 comments on commit 78aed65

Please sign in to comment.