diff --git a/go.mod b/go.mod index 927285e..f9c38dd 100644 --- a/go.mod +++ b/go.mod @@ -220,3 +220,5 @@ require ( ) replace oras.land/oras-go => oras.land/oras-go v1.2.4-0.20230801060855-932dd06d38af + +replace github.com/aquasecurity/defsec => github.com/nikpivkin/defsec v0.0.0-20240124115050-1ac1c9fb3624 diff --git a/go.sum b/go.sum index c4d1468..2d6fc46 100644 --- a/go.sum +++ b/go.sum @@ -236,8 +236,6 @@ 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.94.1 h1:lk44bfUltm0f0Dw4DbO3Ka9d/bf3N8cWclSdHXMyKF4= -github.com/aquasecurity/defsec v0.94.1/go.mod h1:wiX9BX0SOG0ZWjVIPYGPl46fyO3Gu8lJnk4rmhFR7IA= github.com/aquasecurity/trivy-policies v0.8.0 h1:LvmIdw/DfTF72Lc8L+CKLYzfb5BFYzLBGFFR95PKC74= github.com/aquasecurity/trivy-policies v0.8.0/go.mod h1:qF/t59pgK/0JTV6tXaeA3Iw3opzoMgzGCDcTDBmqb30= github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0 h1:jfIu9sQUG6Ig+0+Ap1h4unLjW6YQJpKZVmUzxsD4E/Q= @@ -669,6 +667,8 @@ github.com/morikuni/aec v1.0.0/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7P github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= +github.com/nikpivkin/defsec v0.0.0-20240124115050-1ac1c9fb3624 h1:qke77MLT+eMxhVq3BEm0xz+DDJhot97dYKGGmqWm63U= +github.com/nikpivkin/defsec v0.0.0-20240124115050-1ac1c9fb3624/go.mod h1:wiX9BX0SOG0ZWjVIPYGPl46fyO3Gu8lJnk4rmhFR7IA= github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec= github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY= github.com/onsi/ginkgo/v2 v2.9.4 h1:xR7vG4IXt5RWx6FfIjyAtsoMAtnc3C/rFXBBd2AjZwE= diff --git a/pkg/scanners/terraform/parser/evaluator.go b/pkg/scanners/terraform/parser/evaluator.go index cc3b245..6958655 100644 --- a/pkg/scanners/terraform/parser/evaluator.go +++ b/pkg/scanners/terraform/parser/evaluator.go @@ -3,7 +3,6 @@ package parser import ( "context" "errors" - "fmt" "io/fs" "reflect" "time" @@ -148,7 +147,8 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str } } - // expand out resources and modules via count (not a typo, we do this twice so every order is processed) + // expand out resources and modules via count, for-each and dynamic + // (not a typo, we do this twice so every order is processed) e.blocks = e.expandBlocks(e.blocks) e.blocks = e.expandBlocks(e.blocks) @@ -219,6 +219,9 @@ func (e *evaluator) expandDynamicBlock(b *terraform.Block) { e.expandDynamicBlock(sub) } for _, sub := range b.AllBlocks().OfType("dynamic") { + if sub.IsCountExpanded() { + continue + } blockName := sub.TypeLabel() expanded := e.expandBlockForEaches(terraform.Blocks{sub}) for _, ex := range expanded { @@ -227,45 +230,10 @@ func (e *evaluator) expandDynamicBlock(b *terraform.Block) { b.InjectBlock(content, blockName) } } + sub.MarkCountExpanded() } } -func validateForEachArg(arg cty.Value) error { - if arg.IsNull() { - return errors.New("arg is null") - } - - ty := arg.Type() - - if !arg.IsKnown() || ty.Equals(cty.DynamicPseudoType) || arg.LengthInt() == 0 { - return nil - } - - if !(ty.IsSetType() || ty.IsObjectType() || ty.IsMapType()) { - return fmt.Errorf("%s type is not supported: arg is not set or map", ty.FriendlyName()) - } - - if ty.IsSetType() { - if !ty.ElementType().Equals(cty.String) { - return errors.New("arg is not set of strings") - } - - it := arg.ElementIterator() - for it.Next() { - key, _ := it.Element() - if key.IsNull() { - return errors.New("arg is set of strings, but contains null") - } - - if !key.IsKnown() { - return errors.New("arg is set of strings, but contains unknown value") - } - } - } - - return nil -} - func isBlockSupportsForEachMetaArgument(block *terraform.Block) bool { return slices.Contains([]string{"module", "resource", "data", "dynamic"}, block.Type()) } @@ -284,15 +252,15 @@ func (e *evaluator) expandBlockForEaches(blocks terraform.Blocks) terraform.Bloc forEachVal := forEachAttr.Value() - if err := validateForEachArg(forEachVal); err != nil { - e.debug.Log(`"for_each" argument is invalid: %s`, err.Error()) + if forEachVal.IsNull() || !forEachVal.IsKnown() || !forEachAttr.IsIterable() { continue } clones := make(map[string]cty.Value) _ = forEachAttr.Each(func(key cty.Value, val cty.Value) { - if !key.Type().Equals(cty.String) { + idx, err := convert.Convert(key, cty.String) + if err != nil { e.debug.Log( `Invalid "for-each" argument: map key (or set value) is not a string, but %s`, key.Type().FriendlyName(), @@ -315,7 +283,7 @@ func (e *evaluator) expandBlockForEaches(blocks terraform.Blocks) terraform.Bloc forEachFiltered = append(forEachFiltered, clone) values := clone.Values() - clones[key.AsString()] = values + clones[idx.AsString()] = values e.ctx.SetByDot(values, clone.GetMetadata().Reference()) }) diff --git a/pkg/scanners/terraform/parser/evaluator_test.go b/pkg/scanners/terraform/parser/evaluator_test.go deleted file mode 100644 index 8d3ef7b..0000000 --- a/pkg/scanners/terraform/parser/evaluator_test.go +++ /dev/null @@ -1,94 +0,0 @@ -package parser - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/zclconf/go-cty/cty" -) - -func TestValidateForEachArg(t *testing.T) { - tests := []struct { - name string - arg cty.Value - expectedError string - }{ - { - name: "empty set", - arg: cty.SetValEmpty(cty.String), - }, - { - name: "set of strings", - arg: cty.SetVal([]cty.Value{cty.StringVal("val1"), cty.StringVal("val2")}), - }, - { - name: "set of non-strings", - arg: cty.SetVal([]cty.Value{cty.NumberIntVal(1), cty.NumberIntVal(2)}), - expectedError: "is not set of strings", - }, - { - name: "set with null", - arg: cty.SetVal([]cty.Value{cty.StringVal("val1"), cty.NullVal(cty.String)}), - expectedError: "arg is set of strings, but contains null", - }, - { - name: "set with unknown", - arg: cty.SetVal([]cty.Value{cty.StringVal("val1"), cty.UnknownVal(cty.String)}), - expectedError: "arg is set of strings, but contains unknown", - }, - { - name: "set with unknown", - arg: cty.SetVal([]cty.Value{cty.StringVal("val1"), cty.UnknownVal(cty.String)}), - expectedError: "arg is set of strings, but contains unknown", - }, - { - name: "non empty map", - arg: cty.MapVal(map[string]cty.Value{ - "val1": cty.StringVal("..."), - "val2": cty.StringVal("..."), - }), - }, - { - name: "map with unknown", - arg: cty.MapVal(map[string]cty.Value{ - "val1": cty.UnknownVal(cty.String), - "val2": cty.StringVal("..."), - }), - }, - { - name: "empty obj", - arg: cty.EmptyObjectVal, - }, - { - name: "obj with strings", - arg: cty.ObjectVal(map[string]cty.Value{ - "val1": cty.StringVal("..."), - "val2": cty.StringVal("..."), - }), - }, - { - name: "null", - arg: cty.NullVal(cty.Set(cty.String)), - expectedError: "arg is null", - }, - { - name: "unknown", - arg: cty.UnknownVal(cty.Set(cty.String)), - }, - { - name: "dynamic", - arg: cty.DynamicVal, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := validateForEachArg(tt.arg) - if tt.expectedError != "" && err != nil { - assert.ErrorContains(t, err, tt.expectedError) - return - } - assert.NoError(t, err) - }) - } -} diff --git a/pkg/scanners/terraform/parser/parser_test.go b/pkg/scanners/terraform/parser/parser_test.go index 21ee4ff..e1a7bd4 100644 --- a/pkg/scanners/terraform/parser/parser_test.go +++ b/pkg/scanners/terraform/parser/parser_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/aquasecurity/defsec/pkg/scanners/options" + "github.com/aquasecurity/defsec/pkg/terraform" "github.com/aquasecurity/defsec/test/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -918,8 +919,22 @@ func TestForEach(t *testing.T) { resource "aws_s3_bucket" "this" { for_each = local.buckets - bucket = each.key -}`, + bucket = each.value +} +`, + expectedCount: 2, + }, + { + name: "arg is empty list", + source: `locals { + buckets = [] +} + +resource "aws_s3_bucket" "this" { + for_each = loca.buckets + bucket = each.value +} +`, expectedCount: 0, }, { @@ -930,8 +945,9 @@ resource "aws_s3_bucket" "this" { resource "aws_s3_bucket" "this" { for_each = loca.buckets - bucket = each.key -}`, + bucket = each.value +} +`, expectedCount: 0, }, { @@ -942,8 +958,9 @@ resource "aws_s3_bucket" "this" { resource "aws_s3_bucket" "this" { for_each = toset(local.buckets) - bucket = each.key -}`, + bucket = each.value +} +`, expectedCount: 2, }, { @@ -957,10 +974,24 @@ resource "aws_s3_bucket" "this" { resource "aws_s3_bucket" "this" { for_each = local.buckets - bucket = each.key -}`, + bucket = each.value +} +`, expectedCount: 2, }, + { + name: "arg is empty map", + source: `locals { + buckets = {} +} + +resource "aws_s3_bucket" "this" { + for_each = local.buckets + bucket = each.value +} +`, + expectedCount: 0, + }, } for _, tt := range tests { @@ -1139,3 +1170,156 @@ func TestForEachWithObjectsOfDifferentTypes(t *testing.T) { assert.NoError(t, err) assert.Len(t, modules, 1) } + +func TestDynamicBlocks(t *testing.T) { + + tests := []struct { + name string + source string + count int + }{} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fs := testutil.CreateFS(t, map[string]string{ + "main.tf": tt.source, + }) + parser := New(fs, "", OptionStopOnHCLError(true)) + require.NoError(t, parser.ParseFS(context.TODO(), ".")) + }) + } + + t.Run("arg is list of int", func(t *testing.T) { + fs := testutil.CreateFS(t, map[string]string{ + "main.tf": ` +resource "aws_security_group" "sg-webserver" { + vpc_id = "1111" + dynamic "ingress" { + for_each = [80, 443] + content { + from_port = ingress.value + to_port = ingress.value + protocol = "tcp" + cidr_blocks = ["0.0.0.0/0"] + } + } +} +`, + }) + parser := New(fs, "", OptionStopOnHCLError(true)) + require.NoError(t, parser.ParseFS(context.TODO(), ".")) + + modules, _, err := parser.EvaluateAll(context.TODO()) + require.NoError(t, err) + require.Len(t, modules, 1) + + secGroups := modules.GetResourcesByType("aws_security_group") + assert.Len(t, secGroups, 1) + ingressBlocks := secGroups[0].GetBlocks("ingress") + assert.Len(t, ingressBlocks, 2) + + var inboundPorts []int + for _, ingress := range ingressBlocks { + fromPort := ingress.GetAttribute("from_port").AsIntValueOrDefault(-1, ingress).Value() + inboundPorts = append(inboundPorts, fromPort) + } + + assert.True(t, compareSets([]int{80, 443}, inboundPorts)) + }) + + t.Run("empty for-each", func(t *testing.T) { + fs := testutil.CreateFS(t, map[string]string{ + "main.tf": ` +resource "aws_lambda_function" "analyzer" { + dynamic "vpc_config" { + for_each = [] + content {} + } +} +`, + }) + parser := New(fs, "", OptionStopOnHCLError(true)) + require.NoError(t, parser.ParseFS(context.TODO(), ".")) + modules, _, err := parser.EvaluateAll(context.TODO()) + require.NoError(t, err) + require.Len(t, modules, 1) + + secGroups := modules.GetResourcesByType("aws_lambda_function") + assert.Len(t, secGroups, 1) + vpcConfigs := secGroups[0].GetBlocks("vpc_config") + assert.Empty(t, vpcConfigs) + }) + + t.Run("arg is list of bool", func(t *testing.T) { + fs := testutil.CreateFS(t, map[string]string{ + "main.tf": ` +resource "aws_lambda_function" "analyzer" { + dynamic "vpc_config" { + for_each = [true] + content {} + } +} +`, + }) + parser := New(fs, "", OptionStopOnHCLError(true)) + require.NoError(t, parser.ParseFS(context.TODO(), ".")) + modules, _, err := parser.EvaluateAll(context.TODO()) + require.NoError(t, err) + require.Len(t, modules, 1) + + secGroups := modules.GetResourcesByType("aws_lambda_function") + assert.Len(t, secGroups, 1) + vpcConfigs := secGroups[0].GetBlocks("vpc_config") + assert.Len(t, vpcConfigs, 1) + }) + + t.Run("nested dynamic", func(t *testing.T) { + fs := testutil.CreateFS(t, map[string]string{ + "main.tf": ` +resource "test_block" "this" { + name = "name" + location = "loc" + dynamic "env" { + for_each = ["1", "2"] + content { + dynamic "value_source" { + for_each = [true, true] + content {} + } + } + } +}`, + }) + parser := New(fs, "", OptionStopOnHCLError(true)) + require.NoError(t, parser.ParseFS(context.TODO(), ".")) + modules, _, err := parser.EvaluateAll(context.TODO()) + require.NoError(t, err) + require.Len(t, modules, 1) + + secGroups := modules.GetResourcesByType("test_block") + assert.Len(t, secGroups, 1) + envs := secGroups[0].GetBlocks("env") + assert.Len(t, envs, 2) + + var sources []*terraform.Block + for _, env := range envs { + sources = append(sources, env.GetBlocks("value_source")...) + } + assert.Len(t, sources, 4) + }) +} + +func compareSets(a []int, b []int) bool { + m := make(map[int]bool) + for _, el := range a { + m[el] = true + } + + for _, el := range b { + if !m[el] { + return false + } + } + + return true +}