diff --git a/e2e/fixtures/E2E_CLI_104_PAYLOAD.json b/e2e/fixtures/E2E_CLI_104_PAYLOAD.json new file mode 100644 index 00000000000..af2aeea1194 --- /dev/null +++ b/e2e/fixtures/E2E_CLI_104_PAYLOAD.json @@ -0,0 +1,57 @@ +{ + "document": [ + { + "file": "file", + "id": "0", + "locals": { + "environment": "production", + "resource_name": "my-app-service" + }, + "resource": { + "aws_s3_bucket": { + "example": { + "bucket": "my-app-service", + "tags": { + "Environment": "production", + "Name": "my-app-service" + } + } + }, + "kubernetes_service_v1": { + "example": { + "metadata": { + "labels": { + "app": "my-app-service" + }, + "name": "my-service", + "namespace": "default" + }, + "spec": { + "port": { + "port": 80, + "target_port": 8080 + }, + "selector": { + "app": "my-app-service" + } + } + } + } + } + }, + { + "file": "file", + "id": "0", + "variable": { + "name": { + "default": "service", + "type": "${string}" + }, + "resource_prefix": { + "default": "my-app-", + "type": "${string}" + } + } + } + ] +} diff --git a/e2e/fixtures/E2E_CLI_104_RESULT.json b/e2e/fixtures/E2E_CLI_104_RESULT.json new file mode 100644 index 00000000000..b001ff32d26 --- /dev/null +++ b/e2e/fixtures/E2E_CLI_104_RESULT.json @@ -0,0 +1,159 @@ +{ + "kics_version": "development", + "files_scanned": 2, + "lines_scanned": 47, + "files_parsed": 2, + "lines_parsed": 47, + "lines_ignored": 0, + "files_failed_to_scan": 0, + "queries_total": 1101, + "queries_failed_to_execute": 0, + "queries_failed_to_compute_similarity_id": 0, + "scan_id": "console", + "severity_counters": { + "CRITICAL": 0, + "HIGH": 0, + "INFO": 2, + "LOW": 1, + "MEDIUM": 2, + "TRACE": 0 + }, + "total_counter": 5, + "total_bom_resources": 0, + "start": "2026-01-14T21:29:02.4901976Z", + "end": "2026-01-14T21:29:16.8821957Z", + "paths": [ + "/path/e2e/fixtures/samples/terraform-locals" + ], + "queries": [ + { + "query_name": "S3 Bucket Logging Disabled", + "query_id": "f861041c-8c9f-4156-acfc-5e6e524f5884", + "query_url": "https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket", + "severity": "MEDIUM", + "platform": "Terraform", + "cwe": "778", + "risk_score": "5.1", + "cloud_provider": "AWS", + "category": "Observability", + "experimental": false, + "description": "Server Access Logging should be enabled on S3 Buckets so that all changes are logged and trackable", + "description_id": "fa5c7c72", + "files": [ + { + "file_name": "/path/e2e/fixtures/samples/terraform-locals/main.tf", + "similarity_id": "e0510199dceea096d9c476c2b1a5e181b6d3050159e1019049ef068ad0d0e3c9", + "line": 27, + "resource_type": "aws_s3_bucket", + "resource_name": "my-app-service", + "issue_type": "MissingAttribute", + "search_key": "aws_s3_bucket[example]", + "search_line": 27, + "search_value": "", + "expected_value": "'logging' should be defined and not null", + "actual_value": "'logging' is undefined or null" + } + ] + }, + { + "query_name": "S3 Bucket Without Versioning", + "query_id": "568a4d22-3517-44a6-a7ad-6a7eed88722c", + "query_url": "https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket#versioning", + "severity": "MEDIUM", + "platform": "Terraform", + "cwe": "710", + "risk_score": "5.7", + "cloud_provider": "AWS", + "category": "Backup", + "experimental": false, + "description": "S3 bucket should have versioning enabled", + "description_id": "7614ce3b", + "files": [ + { + "file_name": "/path/e2e/fixtures/samples/terraform-locals/main.tf", + "similarity_id": "7152d8bab096599974a6e046e2b7ae3855c5ba4bf5aa1e7b47de1d3834dddc1a", + "line": 27, + "resource_type": "aws_s3_bucket", + "resource_name": "my-app-service", + "issue_type": "MissingAttribute", + "search_key": "aws_s3_bucket[example]", + "search_line": 27, + "search_value": "", + "expected_value": "'versioning' should be true", + "actual_value": "'versioning' is undefined or null" + } + ] + }, + { + "query_name": "IAM Access Analyzer Not Enabled", + "query_id": "e592a0c5-5bdb-414c-9066-5dba7cdea370", + "query_url": "https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/accessanalyzer_analyzer", + "severity": "LOW", + "platform": "Terraform", + "cwe": "710", + "risk_score": "3.5", + "cloud_provider": "AWS", + "category": "Best Practices", + "experimental": false, + "description": "IAM Access Analyzer should be enabled and configured to continuously monitor resource permissions", + "description_id": "d03e85ae", + "files": [ + { + "file_name": "/path/e2e/fixtures/samples/terraform-locals/main.tf", + "similarity_id": "0a7d0464de505a54720d6002d14a22a32e6b20bb8189d444ab621d8ab838304f", + "line": 6, + "resource_type": "n/a", + "resource_name": "n/a", + "issue_type": "MissingAttribute", + "search_key": "resource", + "search_line": -1, + "search_value": "", + "expected_value": "'aws_accessanalyzer_analyzer' should be set", + "actual_value": "'aws_accessanalyzer_analyzer' is undefined" + } + ] + }, + { + "query_name": "Variable Without Description", + "query_id": "2a153952-2544-4687-bcc9-cc8fea814a9b", + "query_url": "https://www.terraform.io/docs/language/values/variables.html#input-variable-documentation", + "severity": "INFO", + "platform": "Terraform", + "cwe": "710", + "risk_score": "0.0", + "cloud_provider": "COMMON", + "category": "Best Practices", + "experimental": false, + "description": "All variables should contain a valid description.", + "description_id": "b44986be", + "files": [ + { + "file_name": "/path/e2e/fixtures/samples/terraform-locals/variables.tf", + "similarity_id": "71c203d56572e4a0245f96d579cf681005b1cd368cbe27273e237b286eeb1867", + "line": 6, + "resource_type": "n/a", + "resource_name": "n/a", + "issue_type": "MissingAttribute", + "search_key": "variable.{{name}}", + "search_line": -1, + "search_value": "", + "expected_value": "'description' should be defined and not null", + "actual_value": "'description' is undefined or null" + }, + { + "file_name": "/path/e2e/fixtures/samples/terraform-locals/variables.tf", + "similarity_id": "77b2c29716b6deec350157e0176aba10a75eefb94ebc3c0e19bd2d20ff19eb3b", + "line": 1, + "resource_type": "n/a", + "resource_name": "n/a", + "issue_type": "MissingAttribute", + "search_key": "variable.{{resource_prefix}}", + "search_line": -1, + "search_value": "", + "expected_value": "'description' should be defined and not null", + "actual_value": "'description' is undefined or null" + } + ] + } + ] +} diff --git a/e2e/fixtures/samples/terraform-locals/main.tf b/e2e/fixtures/samples/terraform-locals/main.tf new file mode 100644 index 00000000000..7a426696820 --- /dev/null +++ b/e2e/fixtures/samples/terraform-locals/main.tf @@ -0,0 +1,35 @@ +locals { + resource_name = "${var.resource_prefix}${var.name}" + environment = "production" +} + +resource "kubernetes_service_v1" "example" { + metadata { + name = "my-service" + namespace = "default" + labels = { + app = local.resource_name + } + } + + spec { + selector = { + app = local.resource_name + } + + port { + port = 80 + target_port = 8080 + } + } +} + +resource "aws_s3_bucket" "example" { + bucket = local.resource_name + + tags = { + Name = local.resource_name + Environment = local.environment + } +} + diff --git a/e2e/fixtures/samples/terraform-locals/variables.tf b/e2e/fixtures/samples/terraform-locals/variables.tf new file mode 100644 index 00000000000..f686a1c5174 --- /dev/null +++ b/e2e/fixtures/samples/terraform-locals/variables.tf @@ -0,0 +1,10 @@ +variable "resource_prefix" { + type = string + default = "my-app-" +} + +variable "name" { + type = string + default = "service" +} + diff --git a/e2e/testcases/e2e-cli-104_tf_locals_support.go b/e2e/testcases/e2e-cli-104_tf_locals_support.go new file mode 100644 index 00000000000..3c79cbedfc6 --- /dev/null +++ b/e2e/testcases/e2e-cli-104_tf_locals_support.go @@ -0,0 +1,29 @@ +// Package testcases provides end-to-end (E2E) testing functionality for the application. +package testcases + +// E2E-CLI-104 - KICS scan should parse and evaluate terraform locals and find vulnerabilities +// should perform the scan successfully, find issues, and return exit code 50 +func init() { //nolint + testSample := TestCase{ + Name: "should perform a valid scan, evaluate terraform locals, and find vulnerabilities [E2E-CLI-104]", + Args: args{ + Args: []cmdArgs{ + []string{"scan", "-o", "/path/e2e/output", + "--output-name", "E2E_CLI_104_RESULT", + "-p", "\"/path/e2e/fixtures/samples/terraform-locals\"", + "--payload-path", "/path/e2e/output/E2E_CLI_104_PAYLOAD.json"}, + }, + ExpectedPayload: []string{ + "E2E_CLI_104_PAYLOAD.json", + }, + ExpectedResult: []ResultsValidation{ + { + ResultsFile: "E2E_CLI_104_RESULT", + }, + }, + }, + WantStatus: []int{40}, + } + + Tests = append(Tests, testSample) +} diff --git a/pkg/parser/terraform/locals.go b/pkg/parser/terraform/locals.go new file mode 100644 index 00000000000..af4c4a70ae8 --- /dev/null +++ b/pkg/parser/terraform/locals.go @@ -0,0 +1,234 @@ +package terraform + +import ( + "fmt" + "maps" + "path/filepath" + "sync" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/rs/zerolog/log" + "github.com/zclconf/go-cty/cty" + + "github.com/Checkmarx/kics/v2/pkg/parser/terraform/converter" + "github.com/Checkmarx/kics/v2/pkg/parser/terraform/functions" +) + +// Cache for directory-level locals resolution +var ( + localsCache = make(map[string]converter.VariableMap) + localsCacheMutex sync.RWMutex +) + +// extractLocalsFromFile extracts all locals blocks from a single .tf file +func extractLocalsFromFile(filename string) (map[string]*hclsyntax.Attribute, error) { + localsAttrs := make(map[string]*hclsyntax.Attribute) + + parsedFile, err := parseFile(filename, false) + if err != nil || parsedFile == nil { + return nil, err + } + + body, ok := parsedFile.Body.(*hclsyntax.Body) + if !ok { + return localsAttrs, nil + } + + // Extract all locals blocks from this file + for _, block := range body.Blocks { + if block.Type == "locals" { + maps.Copy(localsAttrs, block.Body.Attributes) + } + } + + return localsAttrs, nil +} + +func extractLocalDependencies(expr hclsyntax.Expression) []string { + var deps []string + + // nolint:errcheck + hclsyntax.VisitAll(expr, func(node hclsyntax.Node) hcl.Diagnostics { + if traversal, ok := node.(*hclsyntax.ScopeTraversalExpr); ok { + if len(traversal.Traversal) > 0 { + if root, ok := traversal.Traversal[0].(hcl.TraverseRoot); ok { + if root.Name == "local" && len(traversal.Traversal) > 1 { + if attr, ok := traversal.Traversal[1].(hcl.TraverseAttr); ok { + deps = append(deps, attr.Name) + } + } + } + } + } + return nil + }) + + return deps +} + +func topologicalSort(graph map[string][]string) ([]string, error) { + visited := make(map[string]bool) + recStack := make(map[string]bool) + var result []string + + var visit func(string) error + visit = func(node string) error { + // Check if a node is currently in the recursion stack + if recStack[node] { + return fmt.Errorf("cycle detected in locals: local.%s", node) + } + + // Check if a node has already been visited + // If not in the recursion stack, we already visited it and skip + if visited[node] { + return nil + } + + // Currently visiting a node + recStack[node] = true + for _, dep := range graph[node] { + if _, exists := graph[dep]; exists { + if err := visit(dep); err != nil { + return err + } + } + } + + // Visited the node and dependencies are resolved + // Remove from recursion stack and mark as visited + recStack[node] = false + visited[node] = true + result = append(result, node) + return nil + } + + for node := range graph { + if !visited[node] { + if err := visit(node); err != nil { + return nil, err + } + } + } + + return result, nil +} + +func evaluateLocal(attr *hclsyntax.Attribute, localsMap converter.VariableMap) (cty.Value, bool) { + evalCtx := &hcl.EvalContext{ + Variables: make(map[string]cty.Value), + Functions: functions.TerraformFuncs, + } + + maps.Copy(evalCtx.Variables, inputVariableMap) + + if len(localsMap) > 0 { + evalCtx.Variables["local"] = cty.ObjectVal(localsMap) + } + + value, diags := attr.Expr.Value(evalCtx) + if diags.HasErrors() { + return cty.NilVal, false + } + + return value, true +} + +// buildLocalsForDirectory scans all .tf files in a directory once and builds the complete locals map +func buildLocalsForDirectory(currentPath string) (converter.VariableMap, error) { + localsMap := make(converter.VariableMap) + + // Get all .tf files in the directory + tfFiles, err := filepath.Glob(filepath.Join(currentPath, "*.tf")) + if err != nil { + log.Error().Msg("Error getting .tf files") + return localsMap, err + } + + if len(tfFiles) == 0 { + return localsMap, nil + } + + // Collect all locals attributes with duplicate detection + allLocalsAttrs := make(map[string]*hclsyntax.Attribute) + + for _, tfFile := range tfFiles { + fileLocals, errExtract := extractLocalsFromFile(tfFile) + if errExtract != nil { + log.Error().Msgf("Error extracting locals from %s", tfFile) + log.Err(errExtract) + continue + } + + // Check for duplicate local values + for name, attr := range fileLocals { + if existing, exists := allLocalsAttrs[name]; exists { + log.Error().Msgf("Duplicate local value definition: "+ + "A local value named '%s' was already defined at %s. "+ + "Local value names must be unique within a module.", + name, existing.NameRange.Filename) + return localsMap, fmt.Errorf("duplicate local value definition: %s", name) + } + allLocalsAttrs[name] = attr + } + } + + if len(allLocalsAttrs) == 0 { + return localsMap, nil + } + + // Build dependency graph + depGraph := make(map[string][]string) + for name, attr := range allLocalsAttrs { + depGraph[name] = extractLocalDependencies(attr.Expr) + } + + // Topological sort with cycle detection + evalOrder, err := topologicalSort(depGraph) + if err != nil { + log.Error().Msgf("Cycle in locals at %s: %v", currentPath, err) + return localsMap, err + } + + // Evaluate in dependency order + for _, name := range evalOrder { + attr := allLocalsAttrs[name] + value, success := evaluateLocal(attr, localsMap) + if !success { + log.Warn().Msgf("Could not evaluate local.%s (missing references or evaluation error)", name) + localsMap[name] = cty.StringVal("${local." + name + "}") + } else { + localsMap[name] = value + } + } + + return localsMap, nil +} + +// getLocals extracts locals from all .tf files in the directory and caches the result +func getLocals(currentPath string) { + localsCacheMutex.RLock() + if cachedLocals, exists := localsCache[currentPath]; exists { + localsCacheMutex.RUnlock() + if len(cachedLocals) > 0 { + inputVariableMap["local"] = cty.ObjectVal(cachedLocals) + } + return + } + localsCacheMutex.RUnlock() + + // Cache miss - build locals for this directory + localsMap, err := buildLocalsForDirectory(currentPath) + if err != nil { + log.Error().Msgf("Error building locals for directory %s: %v", currentPath, err) + return + } + + localsCacheMutex.Lock() + localsCache[currentPath] = localsMap + localsCacheMutex.Unlock() + + if len(localsMap) > 0 { + inputVariableMap["local"] = cty.ObjectVal(localsMap) + } +} diff --git a/pkg/parser/terraform/locals_test.go b/pkg/parser/terraform/locals_test.go new file mode 100644 index 00000000000..19a088d24bf --- /dev/null +++ b/pkg/parser/terraform/locals_test.go @@ -0,0 +1,488 @@ +package terraform + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "github.com/zclconf/go-cty/cty" + + "github.com/Checkmarx/kics/v2/pkg/parser/terraform/converter" +) + +type extractLocalsTest struct { + name string + filename string + wantKeys []string + wantErr bool +} + +type buildLocalsTest struct { + name string + currentPath string + wantKeys []string + wantValues map[string]cty.Value + wantErr bool +} + +func TestExtractLocalsFromFile(t *testing.T) { + tests := []extractLocalsTest{ + { + name: "Should extract simple locals from file", + filename: filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "simple", "simple_locals.tf"), + wantKeys: []string{"simple_string", "simple_number", "simple_bool"}, + wantErr: false, + }, + { + name: "Should extract locals referencing variables", + filename: filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "with_vars", "locals_with_vars.tf"), + wantKeys: []string{"resource_prefix", "tag_name"}, + wantErr: false, + }, + { + name: "Should extract multiple locals blocks from single file", + filename: filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "multi_blocks", "multiple_blocks.tf"), + wantKeys: []string{"first_local", "second_local", "third_local", "combined"}, + wantErr: false, + }, + { + name: "Should return empty map for file without locals", + filename: filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "no_locals", "no_locals.tf"), + wantKeys: []string{}, + wantErr: false, + }, + { + name: "Should return error for non-existent file", + filename: filepath.FromSlash("not_found.tf"), + wantKeys: nil, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + localsAttrs, err := extractLocalsFromFile(tt.filename) + if tt.wantErr { + require.NotNil(t, err) + require.Nil(t, localsAttrs) + } else { + require.NoError(t, err) + require.Equal(t, len(tt.wantKeys), len(localsAttrs)) + for _, key := range tt.wantKeys { + _, exists := localsAttrs[key] + require.True(t, exists, "Expected local '%s' not found", key) + } + } + }) + } +} + +func TestBuildLocalsForDirectory(t *testing.T) { + tests := []buildLocalsTest{ + { + name: "Should build locals from directory with simple locals", + currentPath: filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "isolated"), + wantKeys: []string{"isolated_value"}, + wantValues: map[string]cty.Value{ + "isolated_value": cty.StringVal("isolated"), + }, + wantErr: false, + }, + { + name: "Should return empty map for directory with no locals", + currentPath: filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_variables"), + wantKeys: []string{}, + wantValues: map[string]cty.Value{}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + localsMap, err := buildLocalsForDirectory(tt.currentPath) + if tt.wantErr { + require.NotNil(t, err) + } else { + require.NoError(t, err) + require.Equal(t, len(tt.wantKeys), len(localsMap)) + for key, expectedValue := range tt.wantValues { + actualValue, exists := localsMap[key] + require.True(t, exists, "Expected local '%s' not found", key) + require.True(t, expectedValue.RawEquals(actualValue), "Value mismatch for local '%s'", key) + } + } + }) + } + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestBuildLocalsForDirectory_CrossFileReferences(t *testing.T) { + t.Run("Should handle locals referencing other locals from different files", func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + currentPath := filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "cross_file") + + localsMap, err := buildLocalsForDirectory(currentPath) + require.NoError(t, err) + + // Check that base_name from cross_file_locals_a.tf exists + baseName, exists := localsMap["base_name"] + require.True(t, exists, "base_name should exist") + require.Equal(t, "myapp", baseName.AsString()) + + // Check that full_name from cross_file_locals_b.tf references base_name + fullName, exists := localsMap["full_name"] + require.True(t, exists, "full_name should exist") + require.Equal(t, "myapp-service", fullName.AsString()) + + // Check that base_port is referenced correctly + basePort, exists := localsMap["base_port"] + require.True(t, exists, "base_port should exist") + + fullPort, exists := localsMap["full_port"] + require.True(t, exists, "full_port should exist") + require.True(t, basePort.RawEquals(fullPort), "full_port should equal base_port") + }) + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestBuildLocalsForDirectory_ForwardReferences(t *testing.T) { + t.Run("Should handle locals referencing locals defined later in same file", func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + currentPath := filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "forward_ref") + + localsMap, err := buildLocalsForDirectory(currentPath) + require.NoError(t, err) + + // Check backend_name (defined after full_backend) + backendName, exists := localsMap["backend_name"] + require.True(t, exists, "backend_name should exist") + require.Equal(t, "api", backendName.AsString()) + + // Check full_backend references backend_name + fullBackend, exists := localsMap["full_backend"] + require.True(t, exists, "full_backend should exist") + require.Equal(t, "api-production", fullBackend.AsString()) + + // Check db_port (defined after connection_string) + _, exists = localsMap["db_port"] + require.True(t, exists, "db_port should exist") + + // Check connection_string references db_port + connectionString, exists := localsMap["connection_string"] + require.True(t, exists, "connection_string should exist") + require.Equal(t, "localhost:5432", connectionString.AsString()) + }) + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestBuildLocalsForDirectory_DuplicateLocals(t *testing.T) { + t.Run("Should error when duplicate locals are defined across files", func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + currentPath := filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "duplicates") + + localsMap, err := buildLocalsForDirectory(currentPath) + require.Error(t, err, "Should error on duplicate locals") + require.Contains(t, err.Error(), "duplicate local value definition", "Error message should mention duplicate local value definition") + require.Empty(t, localsMap, "localsMap should be empty when error occurs") + }) + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestBuildLocalsForDirectory_OverrideLocals(t *testing.T) { + t.Run("Should NOT override - duplicate detection should catch this", func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + currentPath := filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "override") + + localsMap, err := buildLocalsForDirectory(currentPath) + require.Error(t, err, "Duplicate locals should cause error") + require.Contains(t, err.Error(), "duplicate local value definition", "Error should mention duplicate local value definition") + require.Empty(t, localsMap, "localsMap should be empty on error") + }) + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestBuildLocalsForDirectory_MultipleBlocks(t *testing.T) { + t.Run("Should handle multiple locals blocks in same file", func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + currentPath := filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "multi_blocks") + + localsMap, err := buildLocalsForDirectory(currentPath) + require.NoError(t, err) + + // Check all locals from multiple blocks + firstLocal, exists := localsMap["first_local"] + require.True(t, exists, "first_local should exist") + require.Equal(t, "first", firstLocal.AsString()) + + secondLocal, exists := localsMap["second_local"] + require.True(t, exists, "second_local should exist") + require.Equal(t, "second", secondLocal.AsString()) + + thirdLocal, exists := localsMap["third_local"] + require.True(t, exists, "third_local should exist") + require.Equal(t, "third", thirdLocal.AsString()) + + // Check combined local that references first and second + combined, exists := localsMap["combined"] + require.True(t, exists, "combined should exist") + require.Equal(t, "first-second", combined.AsString()) + }) + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestBuildLocalsForDirectory_CircularReference(t *testing.T) { + t.Run("Should error on circular references", func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + currentPath := filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "circular") + + localsMap, err := buildLocalsForDirectory(currentPath) + require.Error(t, err, "Circular references should cause error") + require.Contains(t, err.Error(), "cycle", "Error should mention cycle") + require.Empty(t, localsMap, "localsMap should be empty on error") + }) + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestBuildLocalsForDirectory_SubdirectoryIsolation(t *testing.T) { + t.Run("Should not access locals from parent directory", func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + parentPath := filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "subdir_isolation", "parent") + childPath := filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "subdir_isolation", "parent", "child") + + // Build locals for parent directory + parentLocals, err := buildLocalsForDirectory(parentPath) + require.NoError(t, err) + require.Contains(t, parentLocals, "parent_value", "Parent should have parent_value") + require.Contains(t, parentLocals, "shared_name", "Parent should have shared_name") + require.Equal(t, 2, len(parentLocals), "Parent should have exactly 2 locals") + + // Build locals for child directory (separate module) + childLocals, err := buildLocalsForDirectory(childPath) + require.NoError(t, err) + require.Contains(t, childLocals, "child_value", "Child should have child_value") + require.Contains(t, childLocals, "shared_name", "Child should have shared_name") + require.NotContains(t, childLocals, "parent_value", "Child should NOT have parent_value") + require.Equal(t, 2, len(childLocals), "Child should have exactly 2 locals") + + // Verify the shared_name values are different (proving isolation) + parentSharedName := parentLocals["shared_name"].AsString() + childSharedName := childLocals["shared_name"].AsString() + require.Equal(t, "from_parent", parentSharedName, "Parent's shared_name should be 'from_parent'") + require.Equal(t, "from_child", childSharedName, "Child's shared_name should be 'from_child'") + require.NotEqual(t, parentSharedName, childSharedName, "shared_name should be different in parent and child") + }) + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestBuildLocalsForDirectory_MissingVariableReference(t *testing.T) { + t.Run("Should use placeholder for locals with missing variable references", func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + currentPath := filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "missing_var") + + localsMap, err := buildLocalsForDirectory(currentPath) + require.NoError(t, err, "Missing var should not cause error") + + // Local referencing missing var should have placeholder + localWithMissingVar, exists := localsMap["with_missing_var"] + require.True(t, exists, "Local with missing var should exist") + require.Equal(t, cty.String, localWithMissingVar.Type(), "Should be string placeholder") + require.Contains(t, localWithMissingVar.AsString(), "${local.", "Should contain placeholder pattern") + }) + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestBuildLocalsForDirectory_WithVariables(t *testing.T) { + t.Run("Should evaluate locals that reference variables", func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + + inputVariableMap["var"] = cty.ObjectVal(map[string]cty.Value{ + "environment": cty.StringVal("production"), + "region": cty.StringVal("us-east-1"), + "prefix": cty.StringVal("prod"), + }) + + currentPath := filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "with_vars") + + localsMap, err := buildLocalsForDirectory(currentPath) + require.NoError(t, err) + + // Check locals that reference variables + resourcePrefix, exists := localsMap["resource_prefix"] + require.True(t, exists, "resource_prefix should exist") + require.Equal(t, "production-us-east-1", resourcePrefix.AsString()) + + tagName, exists := localsMap["tag_name"] + require.True(t, exists, "tag_name should exist") + require.Equal(t, "production", tagName.AsString()) + }) + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestGetLocals(t *testing.T) { + tests := []struct { + name string + currentPath string + wantKeys []string + }{ + { + name: "Should load locals and populate inputVariableMap", + currentPath: filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "isolated"), + wantKeys: []string{"isolated_value"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + + getLocals(tt.currentPath) + + localObj, exists := inputVariableMap["local"] + require.True(t, exists, "inputVariableMap should contain 'local' key") + + localMap := localObj.AsValueMap() + require.Equal(t, len(tt.wantKeys), len(localMap)) + + for _, key := range tt.wantKeys { + _, exists := localMap[key] + require.True(t, exists, "Expected local '%s' not found", key) + } + }) + } + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestGetLocals_Caching(t *testing.T) { + tests := []struct { + name string + firstCallPath string + secondCallPath string + shouldUseCache bool + expectedKeyCount int + }{ + { + name: "Should use cache for same directory", + firstCallPath: filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "isolated"), + secondCallPath: filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "isolated"), + shouldUseCache: true, + expectedKeyCount: 1, + }, + { + name: "Should not use cache for different directory", + firstCallPath: filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "isolated"), + secondCallPath: filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_variables"), + shouldUseCache: false, + expectedKeyCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + localsCache = make(map[string]converter.VariableMap) + inputVariableMap = make(converter.VariableMap) + + getLocals(tt.firstCallPath) + + cacheSize := len(localsCache) + require.Equal(t, 1, cacheSize, "cache should have one entry after first call") + + getLocals(tt.secondCallPath) + + if tt.shouldUseCache { + require.Equal(t, cacheSize, len(localsCache), "cache size should not change when reusing cache") + } else { + require.Equal(t, cacheSize+1, len(localsCache), "cache should grow for new directory") + } + + if tt.expectedKeyCount > 0 { + localObj, ok := inputVariableMap["local"] + require.True(t, ok, "inputVariableMap should contain 'local' key") + require.Equal(t, tt.expectedKeyCount, len(localObj.AsValueMap()), "wrong number of locals") + } + }) + } + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + }) +} + +func TestGetLocals_Integration(t *testing.T) { + t.Run("Should work with getInputVariables for complete parsing", func(t *testing.T) { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + variableCache = make(map[string]converter.VariableMap) + + currentPath := filepath.Join("..", "..", "..", "test", "fixtures", "test_terraform_locals", "with_vars") + fileContent, err := os.ReadFile(filepath.Join(currentPath, "locals_with_vars.tf")) + require.NoError(t, err) + + getInputVariables(currentPath, string(fileContent), "") + getLocals(currentPath) + + // Check that both var and local are in inputVariableMap + _, varExists := inputVariableMap["var"] + require.True(t, varExists, "var should exist in inputVariableMap") + + localObj, localExists := inputVariableMap["local"] + require.True(t, localExists, "local should exist in inputVariableMap") + + // Verify locals can use variables + localMap := localObj.AsValueMap() + require.NotEmpty(t, localMap, "locals map should not be empty") + }) + + t.Cleanup(func() { + inputVariableMap = make(converter.VariableMap) + localsCache = make(map[string]converter.VariableMap) + variableCache = make(map[string]converter.VariableMap) + }) +} diff --git a/pkg/parser/terraform/terraform.go b/pkg/parser/terraform/terraform.go index 6354f6bfae5..b5a0a5cdc86 100644 --- a/pkg/parser/terraform/terraform.go +++ b/pkg/parser/terraform/terraform.go @@ -5,16 +5,17 @@ import ( "path/filepath" "regexp" - "github.com/Checkmarx/kics/v2/pkg/model" - "github.com/Checkmarx/kics/v2/pkg/parser/terraform/comment" - "github.com/Checkmarx/kics/v2/pkg/parser/terraform/converter" - "github.com/Checkmarx/kics/v2/pkg/parser/utils" - masterUtils "github.com/Checkmarx/kics/v2/pkg/utils" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/pkg/errors" "github.com/rs/zerolog/log" ctyjson "github.com/zclconf/go-cty/cty/json" + + "github.com/Checkmarx/kics/v2/pkg/model" + "github.com/Checkmarx/kics/v2/pkg/parser/terraform/comment" + "github.com/Checkmarx/kics/v2/pkg/parser/terraform/converter" + "github.com/Checkmarx/kics/v2/pkg/parser/utils" + masterUtils "github.com/Checkmarx/kics/v2/pkg/utils" ) // RetriesDefaultValue is default number of times a parser will retry to execute @@ -56,6 +57,7 @@ func (p *Parser) Resolve(fileContent []byte, filename string, _ bool, _ int) ([] }() getInputVariables(filepath.Dir(filename), string(fileContent), p.terraformVarsPath) getDataSourcePolicy(filepath.Dir(filename)) + getLocals(filepath.Dir(filename)) return fileContent, nil } diff --git a/test/fixtures/test_terraform_locals/circular/circular_reference.tf b/test/fixtures/test_terraform_locals/circular/circular_reference.tf new file mode 100644 index 00000000000..4676e9728c8 --- /dev/null +++ b/test/fixtures/test_terraform_locals/circular/circular_reference.tf @@ -0,0 +1,10 @@ +# This creates a circular dependency that should be handled gracefully +locals { + circular_a = local.circular_b + circular_b = local.circular_a +} + +resource "terraform_data" "circular" { + value_a = local.circular_a + value_b = local.circular_b +} diff --git a/test/fixtures/test_terraform_locals/cross_file/cross_file_locals_a.tf b/test/fixtures/test_terraform_locals/cross_file/cross_file_locals_a.tf new file mode 100644 index 00000000000..70486d9219d --- /dev/null +++ b/test/fixtures/test_terraform_locals/cross_file/cross_file_locals_a.tf @@ -0,0 +1,10 @@ +locals { + base_name = "myapp" + base_port = 8080 +} + +resource "terraform_data" "a" { + name = local.base_name + port = local.base_port +} + diff --git a/test/fixtures/test_terraform_locals/cross_file/cross_file_locals_b.tf b/test/fixtures/test_terraform_locals/cross_file/cross_file_locals_b.tf new file mode 100644 index 00000000000..222e6752013 --- /dev/null +++ b/test/fixtures/test_terraform_locals/cross_file/cross_file_locals_b.tf @@ -0,0 +1,10 @@ +locals { + full_name = "${local.base_name}-service" + full_port = local.base_port +} + +resource "terraform_data" "b" { + name = local.full_name + port = local.full_port +} + diff --git a/test/fixtures/test_terraform_locals/duplicates/file1.tf b/test/fixtures/test_terraform_locals/duplicates/file1.tf new file mode 100644 index 00000000000..57bf4250bb4 --- /dev/null +++ b/test/fixtures/test_terraform_locals/duplicates/file1.tf @@ -0,0 +1,9 @@ +locals { + duplicate_name = "from_file1" + unique_to_file1 = "value1" +} + +resource "terraform_data" "from_file1" { + input = local.duplicate_name +} + diff --git a/test/fixtures/test_terraform_locals/duplicates/file2.tf b/test/fixtures/test_terraform_locals/duplicates/file2.tf new file mode 100644 index 00000000000..1e0633ad323 --- /dev/null +++ b/test/fixtures/test_terraform_locals/duplicates/file2.tf @@ -0,0 +1,10 @@ +# This creates a duplicate local which should cause an error +locals { + duplicate_name = "from_file2" + unique_to_file2 = "value2" +} + +resource "terraform_data" "from_file2" { + input = local.duplicate_name +} + diff --git a/test/fixtures/test_terraform_locals/forward_ref/forward_reference.tf b/test/fixtures/test_terraform_locals/forward_ref/forward_reference.tf new file mode 100644 index 00000000000..4d6430e9c90 --- /dev/null +++ b/test/fixtures/test_terraform_locals/forward_ref/forward_reference.tf @@ -0,0 +1,15 @@ +locals { + # This references 'backend_name' which is defined later in the same block + full_backend = "${local.backend_name}-production" + backend_name = "api" + + # This references 'db_port' which is defined later + connection_string = "localhost:${local.db_port}" + db_port = 5432 +} + +resource "terraform_data" "forward_ref" { + backend = local.full_backend + connection = local.connection_string +} + diff --git a/test/fixtures/test_terraform_locals/isolated/isolated_locals.tf b/test/fixtures/test_terraform_locals/isolated/isolated_locals.tf new file mode 100644 index 00000000000..bf4280e2ac1 --- /dev/null +++ b/test/fixtures/test_terraform_locals/isolated/isolated_locals.tf @@ -0,0 +1,8 @@ +locals { + isolated_value = "isolated" +} + +resource "terraform_data" "isolated" { + value = local.isolated_value +} + diff --git a/test/fixtures/test_terraform_locals/missing_var/locals_with_missing_var.tf b/test/fixtures/test_terraform_locals/missing_var/locals_with_missing_var.tf new file mode 100644 index 00000000000..84ae481cdf4 --- /dev/null +++ b/test/fixtures/test_terraform_locals/missing_var/locals_with_missing_var.tf @@ -0,0 +1,14 @@ +# This local references a variable that doesn't exist +# Should result in a placeholder value with warning +locals { + with_missing_var = var.nonexistent_variable + valid_local = "this_works" +} + +resource "terraform_data" "example" { + input = { + value = local.valid_local + missing = local.with_missing_var + } +} + diff --git a/test/fixtures/test_terraform_locals/multi_blocks/multiple_blocks.tf b/test/fixtures/test_terraform_locals/multi_blocks/multiple_blocks.tf new file mode 100644 index 00000000000..29820718d54 --- /dev/null +++ b/test/fixtures/test_terraform_locals/multi_blocks/multiple_blocks.tf @@ -0,0 +1,22 @@ +# Multiple locals blocks in the same file +locals { + first_local = "first" +} + +locals { + second_local = "second" +} + +locals { + third_local = "third" + # Reference to earlier local + combined = "${local.first_local}-${local.second_local}" +} + +resource "terraform_data" "multi_blocks" { + first = local.first_local + second = local.second_local + third = local.third_local + combo = local.combined +} + diff --git a/test/fixtures/test_terraform_locals/no_locals/no_locals.tf b/test/fixtures/test_terraform_locals/no_locals/no_locals.tf new file mode 100644 index 00000000000..74308408e44 --- /dev/null +++ b/test/fixtures/test_terraform_locals/no_locals/no_locals.tf @@ -0,0 +1,9 @@ +variable "test_var" { + type = string + default = "test" +} + +resource "terraform_data" "no_locals" { + name = var.test_var +} + diff --git a/test/fixtures/test_terraform_locals/override/override_locals_a.tf b/test/fixtures/test_terraform_locals/override/override_locals_a.tf new file mode 100644 index 00000000000..e022ca1b9c7 --- /dev/null +++ b/test/fixtures/test_terraform_locals/override/override_locals_a.tf @@ -0,0 +1,10 @@ +locals { + app_name = "first_name" + app_version = "1.0.0" +} + +resource "terraform_data" "override_a" { + name = local.app_name + version = local.app_version +} + diff --git a/test/fixtures/test_terraform_locals/override/override_locals_b.tf b/test/fixtures/test_terraform_locals/override/override_locals_b.tf new file mode 100644 index 00000000000..405ad880b9a --- /dev/null +++ b/test/fixtures/test_terraform_locals/override/override_locals_b.tf @@ -0,0 +1,9 @@ +# This file overrides app_name from override_locals_a.tf +locals { + app_name = "overridden_name" +} + +resource "terraform_data" "override_b" { + input = local.app_name +} + diff --git a/test/fixtures/test_terraform_locals/simple/simple_locals.tf b/test/fixtures/test_terraform_locals/simple/simple_locals.tf new file mode 100644 index 00000000000..944640d5446 --- /dev/null +++ b/test/fixtures/test_terraform_locals/simple/simple_locals.tf @@ -0,0 +1,17 @@ +variable "prefix" { + type = string + default = "prod" +} + +locals { + simple_string = "hello" + simple_number = 42 + simple_bool = true +} + +resource "terraform_data" "example" { + name = local.simple_string + count = local.simple_number + active = local.simple_bool +} + diff --git a/test/fixtures/test_terraform_locals/subdir_isolation/parent/child/child.tf b/test/fixtures/test_terraform_locals/subdir_isolation/parent/child/child.tf new file mode 100644 index 00000000000..5aaa590058e --- /dev/null +++ b/test/fixtures/test_terraform_locals/subdir_isolation/parent/child/child.tf @@ -0,0 +1,13 @@ +# This is a separate module (subdirectory) +# It should NOT have access to parent's locals +locals { + child_value = "child" + shared_name = "from_child" +} + +resource "terraform_data" "child" { + input = local.child_value + # This would fail if parent_value is not accessible (which is correct) + # parent_ref = local.parent_value +} + diff --git a/test/fixtures/test_terraform_locals/subdir_isolation/parent/parent.tf b/test/fixtures/test_terraform_locals/subdir_isolation/parent/parent.tf new file mode 100644 index 00000000000..dd690325fcf --- /dev/null +++ b/test/fixtures/test_terraform_locals/subdir_isolation/parent/parent.tf @@ -0,0 +1,9 @@ +locals { + parent_value = "parent" + shared_name = "from_parent" +} + +resource "terraform_data" "parent" { + input = local.parent_value +} + diff --git a/test/fixtures/test_terraform_locals/with_vars/locals_with_vars.tf b/test/fixtures/test_terraform_locals/with_vars/locals_with_vars.tf new file mode 100644 index 00000000000..913bc1db033 --- /dev/null +++ b/test/fixtures/test_terraform_locals/with_vars/locals_with_vars.tf @@ -0,0 +1,20 @@ +variable "environment" { + type = string + default = "production" +} + +variable "region" { + type = string + default = "us-east-1" +} + +locals { + resource_prefix = "${var.environment}-${var.region}" + tag_name = var.environment +} + +resource "terraform_data" "with_vars" { + prefix = local.resource_prefix + tag = local.tag_name +} +