From 7e3b96b8fc255d69566f59cecc785ff7bcf86771 Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Thu, 4 Jun 2026 15:05:29 +0900 Subject: [PATCH] fix(query): handle list-form access_logs in ELBv2 LB Access Log Disabled The Terraform query aws/elb_v2_lb_access_log_disabled assumed access_logs was always parsed as a single object and read resource.access_logs.enabled directly. When access_logs is written as a list (access_logs = [{ ... enabled = true }]), the parser stores it as an array, so that path is undefined and the load balancer is flagged even though access logging is enabled. Normalize access_logs to a list of blocks (mirroring the object-or-array handling used by other queries such as efs_volume_with_disabled_transit_encryption) so the enabled check works for both the single-block and list shapes. The true-positive cases (access_logs absent, enabled missing, or enabled = false) are still reported, including when access_logs is a list. Adds negative3.tf (list form with enabled = true, must not flag) and positive7.tf (list form with enabled = false, must still flag). Closes #8036 Signed-off-by: Arpit Jain --- .../elb_v2_lb_access_log_disabled/query.rego | 42 +++++++++++++++---- .../test/negative3.tf | 18 ++++++++ .../test/positive7.tf | 18 ++++++++ .../test/positive_expected_result.json | 6 +++ 4 files changed, 77 insertions(+), 7 deletions(-) create mode 100644 assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/negative3.tf create mode 100644 assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/positive7.tf diff --git a/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/query.rego b/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/query.rego index 01f86520b8b..539e184cf5b 100644 --- a/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/query.rego +++ b/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/query.rego @@ -25,17 +25,18 @@ CxPolicy[result] { load_balancer := get_load_balancer(input.document[i].resource) resource := input.document[i].resource[load_balancer][name] - not common_lib.valid_key(resource.access_logs, "enabled") + log_info := get_access_logs(resource)[_] + not common_lib.valid_key(log_info.block, "enabled") result := { "documentId": input.document[i].id, "resourceType": load_balancer, "resourceName": tf_lib.get_specific_resource_name(resource, load_balancer, name), - "searchKey": sprintf("%s[%s].access_logs", [load_balancer,name]), + "searchKey": get_search_key(load_balancer, name, log_info.index, "access_logs"), "issueType": "MissingAttribute", "keyExpectedValue": "'access_logs.enabled' should be defined and set to true", "keyActualValue": "'access_logs.enabled' is undefined", - "searchLine": common_lib.build_search_line(["resource", load_balancer, name, "access_logs"], []), + "searchLine": get_search_line(load_balancer, name, log_info.index, ["access_logs"]), } } @@ -43,20 +44,48 @@ CxPolicy[result] { load_balancer := get_load_balancer(input.document[i].resource) resource := input.document[i].resource[load_balancer][name] - resource.access_logs.enabled != true + log_info := get_access_logs(resource)[_] + log_info.block.enabled != true result := { "documentId": input.document[i].id, "resourceType": load_balancer, "resourceName": tf_lib.get_specific_resource_name(resource, load_balancer, name), - "searchKey": sprintf("%s[%s].access_logs.enabled", [load_balancer,name]), + "searchKey": get_search_key(load_balancer, name, log_info.index, "access_logs.enabled"), "issueType": "IncorrectValue", "keyExpectedValue": "'access_logs.enabled' should be defined and set to true", "keyActualValue": "'access_logs.enabled' is not set to true", - "searchLine": common_lib.build_search_line(["resource", load_balancer, name, "access_logs", "enabled"], []), + "searchLine": get_search_line(load_balancer, name, log_info.index, ["access_logs", "enabled"]), } } +# access_logs can be parsed as a single block (object) or, when written as a +# list (access_logs = [{ ... }]) or as multiple blocks, as an array. Normalize +# both shapes to a list of {block, index} so the checks above behave the same. +get_access_logs(resource) = logs { + is_array(resource.access_logs) + logs := [{"block": block, "index": idx} | block := resource.access_logs[idx]] +} else = logs { + logs := [{"block": resource.access_logs, "index": null}] +} + +get_search_key(lb, name, index, suffix) = searchKey { + index != null + searchKey := sprintf("%s[%s].access_logs[%d].%s", [lb, name, index, trim_prefix(suffix, "access_logs.")]) +} else = searchKey { + searchKey := sprintf("%s[%s].%s", [lb, name, suffix]) +} + +get_search_line(lb, name, index, path_elements) = searchLine { + index != null + full_path := array.concat(["resource", lb, name], path_elements) + path_with_index := array.concat(array.slice(full_path, 0, 4), array.concat([index], array.slice(full_path, 4, count(full_path)))) + searchLine := common_lib.build_search_line(path_with_index, []) +} else = searchLine { + full_path := array.concat(["resource", lb, name], path_elements) + searchLine := common_lib.build_search_line(full_path, []) +} + get_load_balancer(resource) = lb { common_lib.valid_key(resource,"aws_lb") lb = "aws_lb" @@ -64,4 +93,3 @@ get_load_balancer(resource) = lb { common_lib.valid_key(resource,"aws_alb") lb = "aws_alb" } - diff --git a/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/negative3.tf b/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/negative3.tf new file mode 100644 index 00000000000..a0b1e7dc245 --- /dev/null +++ b/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/negative3.tf @@ -0,0 +1,18 @@ +# access_logs written as a list of objects (= [{ ... }]) instead of a block +resource "aws_lb" "test" { + name = "test-lb-tf" + internal = false + load_balancer_type = "network" + + enable_deletion_protection = true + + access_logs = [{ + bucket = aws_s3_bucket.lb_logs.id + prefix = "test-lb" + enabled = true + }] + + tags = { + Environment = "production" + } +} diff --git a/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/positive7.tf b/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/positive7.tf new file mode 100644 index 00000000000..c61aa9b33fb --- /dev/null +++ b/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/positive7.tf @@ -0,0 +1,18 @@ +# access_logs as a list with logging explicitly disabled - still a finding +resource "aws_lb" "test" { + name = "test-lb-tf" + internal = false + load_balancer_type = "network" + + enable_deletion_protection = true + + access_logs = [{ + bucket = aws_s3_bucket.lb_logs.id + prefix = "test-lb" + enabled = false + }] + + tags = { + Environment = "production" + } +} diff --git a/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/positive_expected_result.json b/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/positive_expected_result.json index 585f9f8c48e..07dfec6e22b 100644 --- a/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/positive_expected_result.json +++ b/assets/queries/terraform/aws/elb_v2_lb_access_log_disabled/test/positive_expected_result.json @@ -34,5 +34,11 @@ "severity": "MEDIUM", "line": 2, "fileName": "positive6.tf" + }, + { + "queryName": "ELBv2 LB Access Log Disabled", + "severity": "MEDIUM", + "line": 12, + "fileName": "positive7.tf" } ]