From 44e468603d44b077cc4606327fb3e7d7ca435e05 Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Thu, 29 Aug 2024 05:26:02 +0600 Subject: [PATCH] feat(misconf): support for ignore by nested attributes (#7205) Signed-off-by: nikpivkin --- docs/docs/scanner/misconfiguration/index.md | 7 +- .../scanners/terraform/executor/executor.go | 17 +- pkg/iac/scanners/terraform/ignore_test.go | 163 ++++++++++++++++++ pkg/iac/terraform/block.go | 115 ++++++++++++ 4 files changed, 287 insertions(+), 15 deletions(-) diff --git a/docs/docs/scanner/misconfiguration/index.md b/docs/docs/scanner/misconfiguration/index.md index 7615f3342265..9824936b8e53 100644 --- a/docs/docs/scanner/misconfiguration/index.md +++ b/docs/docs/scanner/misconfiguration/index.md @@ -534,7 +534,7 @@ If you want to ignore multiple resources on different attributes, you can specif #trivy:ignore:aws-ec2-no-public-ingress-sgr[from_port=5432] ``` -You can also ignore a resource on multiple attributes: +You can also ignore a resource on multiple attributes in the same rule: ```tf locals { rules = { @@ -563,10 +563,7 @@ resource "aws_security_group_rule" "example" { } ``` -Checks can also be ignored by nested attributes, but certain restrictions apply: - -- You cannot access an individual block using indexes, for example when working with dynamic blocks. -- Special variables like [each](https://developer.hashicorp.com/terraform/language/meta-arguments/for_each#the-each-object) and [count](https://developer.hashicorp.com/terraform/language/meta-arguments/count#the-count-object) cannot be accessed. +Checks can also be ignored by nested attributes: ```tf #trivy:ignore:*[logging_config.prefix=myprefix] diff --git a/pkg/iac/scanners/terraform/executor/executor.go b/pkg/iac/scanners/terraform/executor/executor.go index e7561997305d..452e47dcc8de 100644 --- a/pkg/iac/scanners/terraform/executor/executor.go +++ b/pkg/iac/scanners/terraform/executor/executor.go @@ -135,26 +135,23 @@ func ignoreByParams(params map[string]string, modules terraform.Modules, m *type if block == nil { return true } - for key, val := range params { - attr, _ := block.GetNestedAttribute(key) - if attr.IsNil() || !attr.Value().IsKnown() { - return false - } - switch attr.Type() { + for key, param := range params { + val := block.GetValueByPath(key) + switch val.Type() { case cty.String: - if !attr.Equals(val) { + if val.AsString() != param { return false } case cty.Number: - bf := attr.Value().AsBigFloat() + bf := val.AsBigFloat() f64, _ := bf.Float64() comparableInt := fmt.Sprintf("%d", int(f64)) comparableFloat := fmt.Sprintf("%f", f64) - if val != comparableInt && val != comparableFloat { + if param != comparableInt && param != comparableFloat { return false } case cty.Bool: - if fmt.Sprintf("%t", attr.IsTrue()) != val { + if fmt.Sprintf("%t", val.True()) != param { return false } default: diff --git a/pkg/iac/scanners/terraform/ignore_test.go b/pkg/iac/scanners/terraform/ignore_test.go index 6183469d5296..4a2cbce14a86 100644 --- a/pkg/iac/scanners/terraform/ignore_test.go +++ b/pkg/iac/scanners/terraform/ignore_test.go @@ -442,6 +442,21 @@ resource "bad" "my-rule" { } } } +`, + assertLength: 0, + }, + { + name: "ignore by indexed dynamic block value", + inputOptions: ` +// trivy:ignore:*[secure_settings.0.enabled=false] +resource "bad" "my-rule" { + dynamic "secure_settings" { + for_each = ["false", "true"] + content { + enabled = secure_settings.value + } + } +} `, assertLength: 0, }, @@ -604,6 +619,154 @@ data "aws_iam_policy_document" "test_policy" { resources = ["*"] # trivy:ignore:aws-iam-enforce-mfa } } +`, + assertLength: 0, + }, + { + name: "ignore by each.value", + inputOptions: ` +// trivy:ignore:*[each.value=false] +resource "bad" "my-rule" { + for_each = toset(["false", "true", "false"]) + secure = each.value +} +`, + assertLength: 0, + }, + { + name: "ignore by nested each.value", + inputOptions: ` +locals { + vms = [ + { + ip_address = "10.0.0.1" + name = "vm-1" + }, + { + ip_address = "10.0.0.2" + name = "vm-2" + } + ] +} +// trivy:ignore:*[each.value.name=vm-2] +resource "bad" "my-rule" { + secure = false + for_each = { for vm in local.vms : vm.name => vm } + ip_address = each.value.ip_address +} +`, + assertLength: 1, + }, + { + name: "ignore resource with `count` meta-argument", + inputOptions: ` +// trivy:ignore:*[count.index=1] +resource "bad" "my-rule" { + count = 2 + secure = false +} +`, + assertLength: 1, + }, + { + name: "invalid index when accessing blocks", + inputOptions: ` +// trivy:ignore:*[ingress.99.port=9090] +// trivy:ignore:*[ingress.-10.port=9090] +resource "bad" "my-rule" { + secure = false + dynamic "ingress" { + for_each = [8080, 9090] + content { + port = ingress.value + } + } +} +`, + assertLength: 1, + }, + { + name: "ignore by list value", + inputOptions: ` +#trivy:ignore:*[someattr.1.Environment=dev] +resource "bad" "my-rule" { + secure = false + someattr = [ + { + Environment = "prod" + }, + { + Environment = "dev" + } + ] +} +`, + assertLength: 0, + }, + { + name: "ignore by list value with invalid index", + inputOptions: ` +#trivy:ignore:*[someattr.-2.Environment=dev] +resource "bad" "my-rule" { + secure = false + someattr = [ + { + Environment = "prod" + }, + { + Environment = "dev" + } + ] +} +`, + assertLength: 1, + }, + { + name: "ignore by object value", + inputOptions: ` +#trivy:ignore:*[tags.Environment=dev] +resource "bad" "my-rule" { + secure = false + tags = { + Environment = "dev" + } +} +`, + assertLength: 0, + }, + { + name: "ignore by object value in block", + inputOptions: ` +#trivy:ignore:*[someblock.tags.Environment=dev] +resource "bad" "my-rule" { + secure = false + someblock { + tags = { + Environment = "dev" + } + } +} +`, + assertLength: 0, + }, + { + name: "ignore by list value in map", + inputOptions: ` +variable "testvar" { + type = map(list(string)) + default = { + server1 = ["web", "dev"] + server2 = ["prod"] + } +} + +#trivy:ignore:*[someblock.someattr.server1.1=dev] +resource "bad" "my-rule" { + secure = false + someblock { + someattr = var.testvar + } +} `, assertLength: 0, }, diff --git a/pkg/iac/terraform/block.go b/pkg/iac/terraform/block.go index 9245ad6c38a8..dbf7352959d0 100644 --- a/pkg/iac/terraform/block.go +++ b/pkg/iac/terraform/block.go @@ -3,6 +3,7 @@ package terraform import ( "fmt" "io/fs" + "strconv" "strings" "github.com/google/uuid" @@ -298,6 +299,120 @@ func (b *Block) GetAttribute(name string) *Attribute { return nil } +// GetValueByPath returns the value of the attribute located at the given path. +// Supports special paths like "count.index," "each.key," and "each.value." +// The path may contain indices, keys and dots (used as separators). +func (b *Block) GetValueByPath(path string) cty.Value { + + if path == "count.index" || path == "each.key" || path == "each.value" { + return b.Context().GetByDot(path) + } + + if restPath, ok := strings.CutPrefix(path, "each.value."); ok { + if restPath == "" { + return cty.NilVal + } + + val := b.Context().GetByDot("each.value") + res, err := getValueByPath(val, strings.Split(restPath, ".")) + if err != nil { + return cty.NilVal + } + return res + } + + attr, restPath := b.getAttributeByPath(path) + + if attr == nil { + return cty.NilVal + } + + if !attr.IsIterable() || len(restPath) == 0 { + return attr.Value() + } + + res, err := getValueByPath(attr.Value(), restPath) + if err != nil { + return cty.NilVal + } + return res +} + +func (b *Block) getAttributeByPath(path string) (*Attribute, []string) { + steps := strings.Split(path, ".") + + if len(steps) == 1 { + return b.GetAttribute(steps[0]), nil + } + + var ( + attribute *Attribute + stepIndex int + ) + + for currentBlock := b; currentBlock != nil && stepIndex < len(steps); { + blocks := currentBlock.GetBlocks(steps[stepIndex]) + var nextBlock *Block + if !hasIndex(steps, stepIndex+1) && len(blocks) > 0 { + // if index is not provided then return the first block for backwards compatibility + nextBlock = blocks[0] + } else if len(blocks) > 1 && stepIndex < len(steps)-2 { + // handling the case when there are multiple blocks with the same name, + // e.g. when using a `dynamic` block + indexVal, err := strconv.Atoi(steps[stepIndex+1]) + if err == nil && indexVal >= 0 && indexVal < len(blocks) { + nextBlock = blocks[indexVal] + stepIndex++ + } + } + + if nextBlock == nil { + attribute = currentBlock.GetAttribute(steps[stepIndex]) + } + + currentBlock = nextBlock + stepIndex++ + } + + return attribute, steps[stepIndex:] +} + +func hasIndex(steps []string, idx int) bool { + if idx < 0 || idx >= len(steps) { + return false + } + _, err := strconv.Atoi(steps[idx]) + return err == nil +} + +func getValueByPath(val cty.Value, path []string) (cty.Value, error) { + var err error + for _, step := range path { + switch valType := val.Type(); { + case valType.IsMapType(): + val, err = cty.IndexStringPath(step).Apply(val) + case valType.IsObjectType(): + val, err = cty.GetAttrPath(step).Apply(val) + case valType.IsListType() || valType.IsTupleType(): + var idx int + idx, err = strconv.Atoi(step) + if err != nil { + return cty.NilVal, fmt.Errorf("index %q is not a number", step) + } + val, err = cty.IndexIntPath(idx).Apply(val) + default: + return cty.NilVal, fmt.Errorf( + "unexpected value type %s for path step %q", + valType.FriendlyName(), step, + ) + } + if err != nil { + return cty.NilVal, err + } + } + return val, nil +} + func (b *Block) GetNestedAttribute(name string) (*Attribute, *Block) { parts := strings.Split(name, ".")