From a6ceff7e83121e2c9e618dcb0584aa62e7077bbf Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Wed, 19 Nov 2025 12:56:10 +0600 Subject: [PATCH] fix(misconf): ensure boolean metadata values are correctly interpreted (#9770) Signed-off-by: nikpivkin --- .../terraform/google/compute/instances.go | 20 ++------ .../google/compute/instances_test.go | 22 +++++++- .../terraform/google/compute/metadata.go | 42 +++++++++++---- .../terraform/google/compute/metadata_test.go | 20 ++++++-- .../adapters/terraform/google/gke/adapt.go | 13 +---- pkg/iac/providers/google/compute/metadata.go | 6 +++ pkg/iac/types/bool.go | 50 ++++++++++++++++++ pkg/iac/types/bool_test.go | 51 +++++++++++++++++++ 8 files changed, 182 insertions(+), 42 deletions(-) diff --git a/pkg/iac/adapters/terraform/google/compute/instances.go b/pkg/iac/adapters/terraform/google/compute/instances.go index 96115db9bd..0f198d06dc 100644 --- a/pkg/iac/adapters/terraform/google/compute/instances.go +++ b/pkg/iac/adapters/terraform/google/compute/instances.go @@ -1,8 +1,6 @@ package compute import ( - "github.com/zclconf/go-cty/cty" - "github.com/aquasecurity/trivy/pkg/iac/providers/google/compute" "github.com/aquasecurity/trivy/pkg/iac/terraform" iacTypes "github.com/aquasecurity/trivy/pkg/iac/types" @@ -31,9 +29,6 @@ func adaptInstances(modules terraform.Modules) (instances []compute.Instance) { OSLoginEnabled: iacTypes.BoolDefault(true, instanceBlock.GetMetadata()), EnableProjectSSHKeyBlocking: iacTypes.BoolDefault(false, instanceBlock.GetMetadata()), EnableSerialPort: iacTypes.BoolDefault(false, instanceBlock.GetMetadata()), - NetworkInterfaces: nil, - BootDisks: nil, - AttachedDisks: nil, } // network interfaces @@ -60,16 +55,11 @@ func adaptInstances(modules terraform.Modules) (instances []compute.Instance) { } // metadata - if metadataAttr := instanceBlock.GetAttribute("metadata"); metadataAttr.IsNotNil() { - if val := metadataAttr.MapValue("enable-oslogin"); val.Type() == cty.Bool { - instance.OSLoginEnabled = iacTypes.BoolExplicit(val.True(), metadataAttr.GetMetadata()) - } - if val := metadataAttr.MapValue("block-project-ssh-keys"); val.Type() == cty.Bool { - instance.EnableProjectSSHKeyBlocking = iacTypes.BoolExplicit(val.True(), metadataAttr.GetMetadata()) - } - if val := metadataAttr.MapValue("serial-port-enable"); val.Type() == cty.Bool { - instance.EnableSerialPort = iacTypes.BoolExplicit(val.True(), metadataAttr.GetMetadata()) - } + if attr := instanceBlock.GetAttribute("metadata"); attr.IsNotNil() { + flags := parseMetadataFlags(attr) + instance.OSLoginEnabled = flags.EnableOSLogin + instance.EnableProjectSSHKeyBlocking = flags.BlockProjectSSHKeys + instance.EnableSerialPort = flags.EnableSerialPort } // disks diff --git a/pkg/iac/adapters/terraform/google/compute/instances_test.go b/pkg/iac/adapters/terraform/google/compute/instances_test.go index e37655b3d0..28acbea463 100644 --- a/pkg/iac/adapters/terraform/google/compute/instances_test.go +++ b/pkg/iac/adapters/terraform/google/compute/instances_test.go @@ -81,10 +81,8 @@ func Test_adaptInstances(t *testing.T) { IsDefault: iacTypes.Bool(false, iacTypes.NewTestMetadata()), }, CanIPForward: iacTypes.Bool(true, iacTypes.NewTestMetadata()), - OSLoginEnabled: iacTypes.Bool(false, iacTypes.NewTestMetadata()), EnableProjectSSHKeyBlocking: iacTypes.Bool(true, iacTypes.NewTestMetadata()), EnableSerialPort: iacTypes.Bool(true, iacTypes.NewTestMetadata()), - BootDisks: []compute.Disk{ { Metadata: iacTypes.NewTestMetadata(), @@ -155,6 +153,26 @@ func Test_adaptInstances(t *testing.T) { }, }, }, + { + name: "handles metadata values in various formats", + terraform: `resource "google_compute_instance" "example" { + name = "test" + + metadata = { + enable-oslogin = "True" + block-project-ssh-keys = 1 + serial-port-enable = "yes" + } +}`, + expected: []compute.Instance{ + { + Name: iacTypes.StringTest("test"), + OSLoginEnabled: iacTypes.BoolTest(true), + EnableSerialPort: iacTypes.BoolTest(true), + EnableProjectSSHKeyBlocking: iacTypes.BoolTest(true), + }, + }, + }, } for _, test := range tests { diff --git a/pkg/iac/adapters/terraform/google/compute/metadata.go b/pkg/iac/adapters/terraform/google/compute/metadata.go index 0a0891d2bb..f9f0df0c33 100644 --- a/pkg/iac/adapters/terraform/google/compute/metadata.go +++ b/pkg/iac/adapters/terraform/google/compute/metadata.go @@ -1,27 +1,49 @@ package compute import ( - "github.com/zclconf/go-cty/cty" - "github.com/aquasecurity/trivy/pkg/iac/providers/google/compute" "github.com/aquasecurity/trivy/pkg/iac/terraform" iacTypes "github.com/aquasecurity/trivy/pkg/iac/types" ) +// TODO: add support for google_compute_project_metadata_item +// https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_project_metadata_item func adaptProjectMetadata(modules terraform.Modules) compute.ProjectMetadata { metadata := compute.ProjectMetadata{ - Metadata: iacTypes.NewUnmanagedMetadata(), - EnableOSLogin: iacTypes.BoolUnresolvable( - iacTypes.NewUnmanagedMetadata(), - ), + Metadata: iacTypes.NewUnmanagedMetadata(), + EnableOSLogin: iacTypes.BoolUnresolvable(iacTypes.NewUnmanagedMetadata()), } + for _, metadataBlock := range modules.GetResourcesByType("google_compute_project_metadata") { metadata.Metadata = metadataBlock.GetMetadata() - if metadataAttr := metadataBlock.GetAttribute("metadata"); metadataAttr.IsNotNil() { - if val := metadataAttr.MapValue("enable-oslogin"); val.Type() == cty.Bool { - metadata.EnableOSLogin = iacTypes.BoolExplicit(val.True(), metadataAttr.GetMetadata()) - } + if attr := metadataBlock.GetAttribute("metadata"); attr.IsNotNil() { + flags := parseMetadataFlags(attr) + metadata.EnableOSLogin = flags.EnableOSLogin } } return metadata } + +func parseMetadataFlags(attr *terraform.Attribute) compute.MetadataFlags { + flags := compute.MetadataFlags{ + EnableOSLogin: iacTypes.BoolDefault(false, attr.GetMetadata()), + BlockProjectSSHKeys: iacTypes.BoolDefault(false, attr.GetMetadata()), + EnableSerialPort: iacTypes.BoolDefault(false, attr.GetMetadata()), + } + + if attr.IsNil() { + return flags + } + + meta := attr.GetMetadata() + if val, ok := iacTypes.BoolFromCtyValue(attr.MapValue("enable-oslogin"), meta); ok { + flags.EnableOSLogin = val + } + if val, ok := iacTypes.BoolFromCtyValue(attr.MapValue("block-project-ssh-keys"), meta); ok { + flags.BlockProjectSSHKeys = val + } + if val, ok := iacTypes.BoolFromCtyValue(attr.MapValue("serial-port-enable"), meta); ok { + flags.EnableSerialPort = val + } + return flags +} diff --git a/pkg/iac/adapters/terraform/google/compute/metadata_test.go b/pkg/iac/adapters/terraform/google/compute/metadata_test.go index 00913b6787..46b8f360b5 100644 --- a/pkg/iac/adapters/terraform/google/compute/metadata_test.go +++ b/pkg/iac/adapters/terraform/google/compute/metadata_test.go @@ -25,8 +25,7 @@ func Test_adaptProjectMetadata(t *testing.T) { } `, expected: compute.ProjectMetadata{ - Metadata: iacTypes.NewTestMetadata(), - EnableOSLogin: iacTypes.Bool(true, iacTypes.NewTestMetadata()), + EnableOSLogin: iacTypes.BoolTest(true), }, }, { @@ -38,8 +37,21 @@ func Test_adaptProjectMetadata(t *testing.T) { } `, expected: compute.ProjectMetadata{ - Metadata: iacTypes.NewTestMetadata(), - EnableOSLogin: iacTypes.Bool(false, iacTypes.NewTestMetadata()), + EnableOSLogin: iacTypes.BoolTest(false), + }, + }, + { + name: "handles metadata values in various formats", + terraform: `resource "google_compute_project_metadata" "example" { + metadata = { + enable-oslogin = "TRUE" + block-project-ssh-keys = 1 + serial-port-enable = "yes" + } +} +`, + expected: compute.ProjectMetadata{ + EnableOSLogin: iacTypes.BoolTest(true), }, }, } diff --git a/pkg/iac/adapters/terraform/google/gke/adapt.go b/pkg/iac/adapters/terraform/google/gke/adapt.go index 26a82e0edc..f4a873faba 100644 --- a/pkg/iac/adapters/terraform/google/gke/adapt.go +++ b/pkg/iac/adapters/terraform/google/gke/adapt.go @@ -2,7 +2,6 @@ package gke import ( "github.com/google/uuid" - "github.com/zclconf/go-cty/cty" "github.com/aquasecurity/trivy/pkg/iac/providers/google/gke" "github.com/aquasecurity/trivy/pkg/iac/terraform" @@ -285,16 +284,8 @@ func adaptNodeConfig(resource *terraform.Block) gke.NodeConfig { if metadata := resource.GetAttribute("metadata"); metadata.IsNotNil() { disableLegacy := metadata.MapValue("disable-legacy-endpoints") - if disableLegacy.IsKnown() { - var enableLegacyEndpoints bool - switch disableLegacy.Type() { - case cty.Bool: - enableLegacyEndpoints = disableLegacy.False() - case cty.String: - enableLegacyEndpoints = disableLegacy.AsString() == "false" - } - - config.EnableLegacyEndpoints = iacTypes.Bool(enableLegacyEndpoints, metadata.GetMetadata()) + if val, ok := iacTypes.BoolFromCtyValue(disableLegacy, metadata.GetMetadata()); ok { + config.EnableLegacyEndpoints = val.Invert() } } diff --git a/pkg/iac/providers/google/compute/metadata.go b/pkg/iac/providers/google/compute/metadata.go index 9083854ba2..65d3b5968f 100755 --- a/pkg/iac/providers/google/compute/metadata.go +++ b/pkg/iac/providers/google/compute/metadata.go @@ -4,6 +4,12 @@ import ( iacTypes "github.com/aquasecurity/trivy/pkg/iac/types" ) +type MetadataFlags struct { + EnableOSLogin iacTypes.BoolValue + BlockProjectSSHKeys iacTypes.BoolValue + EnableSerialPort iacTypes.BoolValue +} + type ProjectMetadata struct { Metadata iacTypes.Metadata EnableOSLogin iacTypes.BoolValue diff --git a/pkg/iac/types/bool.go b/pkg/iac/types/bool.go index 4830e5e94f..b2bc6e4ece 100755 --- a/pkg/iac/types/bool.go +++ b/pkg/iac/types/bool.go @@ -2,6 +2,9 @@ package types import ( "encoding/json" + "strings" + + "github.com/zclconf/go-cty/cty" ) type BoolValue struct { @@ -89,8 +92,55 @@ func (b BoolValue) IsFalse() bool { return !b.Value() } +func (b BoolValue) Invert() BoolValue { + return BoolValue{ + BaseAttribute: b.BaseAttribute, + value: !b.value, + } +} + func (b BoolValue) ToRego() any { m := b.metadata.ToRego().(map[string]any) m["value"] = b.Value() return m } + +// BoolFromCtyValue converts a cty.Value to iacTypes.BoolValue. +// Returns the BoolValue and true if conversion to bool succeeded. +func BoolFromCtyValue(val cty.Value, metadata Metadata) (BoolValue, bool) { + if val.IsNull() || !val.IsKnown() { + return BoolUnresolvable(metadata), false + } + + unmarked, _ := val.Unmark() + v, ok := ctyToBool(unmarked) + if !ok { + return BoolUnresolvable(metadata), false + } + + return BoolExplicit(v, metadata), true +} + +func ctyToBool(val cty.Value) (bool, bool) { + switch val.Type() { + case cty.Bool: + return val.True(), true + case cty.String: + switch strings.ToLower(val.AsString()) { + case "true", "yes", "y", "1": + return true, true + case "false", "no", "n", "0": + return false, true + } + case cty.Number: + v, _ := val.AsBigFloat().Int64() + switch v { + case 1: + return true, true + case 0: + return false, true + } + } + + return false, false +} diff --git a/pkg/iac/types/bool_test.go b/pkg/iac/types/bool_test.go index a63a95eb7a..942fa6511f 100644 --- a/pkg/iac/types/bool_test.go +++ b/pkg/iac/types/bool_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/zclconf/go-cty/cty" ) var fakeMetadata = NewMetadata(NewRange("main.tf", 123, 123, "", nil), "") @@ -43,3 +44,53 @@ func Test_BoolJSON(t *testing.T) { assert.Equal(t, val, restored) } + +func TestGetBoolFromValue(t *testing.T) { + metadata := NewTestMetadata() + + tests := []struct { + name string + ctyVal cty.Value + expected bool + ok bool + }{ + // Bool + {"bool true", cty.BoolVal(true), true, true}, + {"bool false", cty.BoolVal(false), false, true}, + + // Strings (true) + {"string 'true'", cty.StringVal("true"), true, true}, + {"string 'TRUE'", cty.StringVal("TRUE"), true, true}, + {"string 'yes'", cty.StringVal("yes"), true, true}, + {"string '1'", cty.StringVal("1"), true, true}, + + // Strings (false) + {"string 'false'", cty.StringVal("false"), false, true}, + {"string 'NO'", cty.StringVal("NO"), false, true}, + {"string '0'", cty.StringVal("0"), false, true}, + + // Numbers + {"number 1", cty.NumberIntVal(1), true, true}, + {"number 0", cty.NumberIntVal(0), false, true}, + {"number 42 (invalid)", cty.NumberIntVal(42), false, false}, + + // Null / Unknown + {"null", cty.NullVal(cty.Bool), false, false}, + {"unknown", cty.UnknownVal(cty.Bool), false, false}, + + // Invalid string + {"string 'maybe'", cty.StringVal("maybe"), false, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := BoolFromCtyValue(tt.ctyVal, metadata) + assert.Equal(t, tt.ok, ok) + if ok { + assert.Equal(t, tt.expected, got.Value()) + } else { + assert.False(t, got.Value()) + } + }) + } +}