🧹 terraform: collapse HCL variants onto block.values to match plan/state bodies#2758
🧹 terraform: collapse HCL variants onto block.values to match plan/state bodies#2758tas50 wants to merge 15 commits into
Conversation
There was a problem hiding this comment.
Bulk migration from blocks/arguments to values API is consistent overall, but has several semantic regressions and a few logic bugs.
Additional findings (file/line not in diff):
- 🟡
content/mondoo-gcp-security.mql.yaml:6177— Same weakening pattern for logging bucket CMEK: old check verifiedkms_key_name != empty, new check only verifiesvalues['cmek_settings'] != empty. Restore thekms_key_namevalidation.
There was a problem hiding this comment.
Consistent mechanical migration from blocks/arguments to values API — looks correct with a few semantic concerns
Additional findings (file/line not in diff):
- 🔵
content/mondoo-aws-security.mql.yaml:8144— The overly-permissive ingress firewall check adds a new condition_['ports'].length > 0that wasn't in the original (arguments.ports != empty). Same addition appears in the egress check at line 22601. While this may be more correct, it's a behavioral change beyond the mechanical migration.
0ed3bbb to
328bb03
Compare
328bb03 to
22446d6
Compare
This comment has been minimized.
This comment has been minimized.
22446d6 to
0dd8cfa
Compare
0dd8cfa to
c79be4f
Compare
There was a problem hiding this comment.
Consistent mechanical migration from blocks/arguments to values-based access pattern, with a few semantic concerns worth noting.
Additional findings (file/line not in diff):
- 🔵
content/mondoo-aws-security.mql.yaml:8151— The overly-permissive ingress check added_['ports'].length > 0in addition to_['ports'] != empty. If!= emptyalready ensures the list is non-empty, the.length > 0is redundant. Not a bug, just unnecessary.
There was a problem hiding this comment.
Consistent mechanical migration from blocks/arguments to values with a few semantic changes and minor inconsistencies worth verifying
Additional findings (file/line not in diff):
- 🔵
content/mondoo-aws-security.mql.yaml:33339— Uses== nullhere (values['vulnerability_scanning_config'] == null) while the vast majority of this PR uses== empty. MQL treats these differently —nullchecks for JSON null,emptychecks for null/empty/missing. For consistency with the rest of the PR (and to catch missing keys), consider using== empty. - 🔵
content/mondoo-aws-security.mql.yaml:6643— The old query had two separate MQL statements: first asserting the filtered set was non-empty, then checking.all(...). The new query drops the non-empty assertion, so if there are zero public DNS zones,.all()is vacuously true. This is likely fine (no public zones = nothing to enforce), but worth confirming that's the intended behavior.
28f7e00 to
beaaea1
Compare
beaaea1 to
c486dbd
Compare
There was a problem hiding this comment.
Consistent mechanical migration from blocks/arguments to values-based access for Terraform HCL queries — looks correct overall with a few logic concerns.
Additional findings (file/line not in diff):
- 🟡
content/mondoo-digitalocean-security.mql.yaml:524— Operator precedence concern:values["inbound_rule"] == empty || values["inbound_rule"].none(...) && values["inbound_rule"].none(...)— if&&binds tighter than||, this reads asempty || (none1 && none2)which is correct. But for clarity and safety, consider wrapping the&&expressions in parentheses:... == empty || (none1 && none2). This same pattern appears in the RDP, DB ports, and all-ports firewall checks. - 🔵
content/mondoo-aws-security.mql.yaml:38603— Pub/Sub push endpoint HTTPS check: the new version adds_['push_endpoint'] == empty ||which allows push configs with no endpoint to pass. The old version required all push configs to have an HTTPS endpoint. This relaxation seems reasonable (no endpoint = no insecure endpoint) but is a logic change worth noting.
e183592 to
d8fa98b
Compare
| ).where( | ||
| arguments.source_ranges.any(_ == '0.0.0.0/0' || _ == '::/0') | ||
| values.source_ranges.any(_ == '0.0.0.0/0' || _ == '::/0') |
There was a problem hiding this comment.
🔵 suggestion — Added _['ports'].length > 0 check: The overly-permissive ingress check now requires _['ports'] != empty && _['ports'].length > 0, which is stricter than the old arguments.ports != empty. The length > 0 is redundant with != empty in most MQL implementations — consider if both are needed, or if this is intentional to guard against edge cases like [].
| - uid: mondoo-aws-security-secgroup-restricted-ssh-terraform-hcl | ||
| filters: asset.platform == "terraform-hcl" && terraform.resources.contains( nameLabel == "aws_security_group") | ||
| mql: terraform.resources("aws_security_group").where(blocks.where( type == "ingress").contains( arguments.to_port == 22 )).none( blocks.where( type == "ingress" ) { arguments.cidr_blocks.contains("0.0.0.0/0") || arguments.cidr_blocks.contains("::/0") }) | ||
| mql: terraform.resources("aws_security_group").where( values.ingress.contains( from_port <= 22 && to_port >= 22 ) ).none( values.ingress.where(from_port <= 22 && to_port >= 22).any(cidr_blocks.contains("0.0.0.0/0") || cidr_blocks.contains("::/0"))) |
There was a problem hiding this comment.
🟡 warning — SSH security group check was significantly rewritten: Old logic used blocks.where(type == "ingress").contains(arguments.to_port == 22) which only matched exact port 22. New logic uses values.ingress.contains(from_port <= 22 && to_port >= 22) which correctly catches port ranges that include 22. This is a correctness improvement, not just a migration. The same fix was applied to plan and state variants.
| values.viewer_certificate == empty || | ||
| values.viewer_certificate.all(_['minimum_protocol_version'] == /TLSv1\.2/) | ||
| ) | ||
| - uid: mondoo-aws-security-cloudfront-minimum-tls-version-terraform-plan |
There was a problem hiding this comment.
🔵 suggestion — CloudFront TLS check relaxed: Old logic required viewer_certificate block to exist. New logic allows values.viewer_certificate == empty to pass. If no viewer certificate is configured, CloudFront uses the default certificate which may not enforce TLS 1.2. Verify this relaxation is intentional.
| values.volume.where(_['efs_volume_configuration'] != empty).all( | ||
| _['efs_volume_configuration']['transit_encryption'] == "ENABLED" | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🔵 suggestion — ECS EFS volume check simplified: Old logic checked nested blocks.where(type == "efs_volume_configuration") inside all volumes. New logic filters volumes that have efs_volume_configuration first, then checks transit encryption. This is cleaner and avoids vacuous truth on volumes without EFS configs — good improvement.
| @@ -7386,8 +7370,8 @@ queries: | |||
| terraform.state.resources.where(type == 'google_dns_managed_zone').where(values['visibility'] == 'public' || values['visibility'] == empty).all( | |||
| values.dnssec_config != empty && | |||
| values.dnssec_config.all( | |||
| state == "on" && | |||
| non_existence == "nsec3" | |||
| _['state'] == "on" && | |||
| _['non_existence'] == "nsec3" | |||
| ) | |||
There was a problem hiding this comment.
🔵 suggestion — Plan/state variants also updated: The DNSSEC NSEC3 check updated plan and state variants to use _['state'] and _['non_existence'] bracket notation. This is good — it ensures consistency across all three query variants.
|
/review |
| values.volume.where(_['efs_volume_configuration'] != empty).all( | ||
| _['efs_volume_configuration']['transit_encryption'] == "ENABLED" | ||
| ) |
There was a problem hiding this comment.
🟡 warning — If efs_volume_configuration is represented as an array in the values model (as sub-blocks typically are), then _['efs_volume_configuration']['transit_encryption'] performs a bracket access on an array, which likely won't behave as expected.
Consider:
values.volume.where(_['efs_volume_configuration'] != empty).all(
_['efs_volume_configuration'].all(_['transit_encryption'] == "ENABLED")
)There was a problem hiding this comment.
Confirmed and fixed in all three Terraform variants (hcl/plan/state). Verified against the cnquery terraform provider: hclBodyToValuesDict (providers/terraform/resources/hcl.go) builds every nested sub-block as an array via dict[block.Type] = append(existing, child), and in plan/state JSON nested blocks are likewise serialized as arrays. So efs_volume_configuration is an array and the chained bracket index was wrong. Applied your suggested form: _['efs_volume_configuration'].all(_['transit_encryption'] == "ENABLED"). This is the same array pattern already used elsewhere in this PR (e.g. the launch-template _['ebs'].all(...) and codebuild _['environment_variable'] checks). Pushed in 0902cdf.
| _['gce_cluster_config'] != empty && | ||
| _['gce_cluster_config'].all( | ||
| _['service_account'] != empty && | ||
| _['service_account'] != /^\d+-compute@developer\.gserviceaccount\.com$/ && | ||
| _['service_account'] != /^[a-z0-9\-]+@appspot\.gserviceaccount\.com$/ | ||
| ) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-no-default-service-account-terraform-plan |
There was a problem hiding this comment.
🟡 warning — This query now rejects default service accounts (*-compute@developer.*, *@appspot.*) in addition to checking != empty, but the original HCL query only checked service_account != empty. The plan/state variants were also updated to match. If this is an intentional policy tightening, it should be called out in the PR description; otherwise it's a silent behavior change that could cause unexpected failures for existing users.
There was a problem hiding this comment.
This is an intentional tightening for parity, not a silent change. The native gcp-dataproc-cluster check already rejects the default compute service account (serviceAccountEmail != /^\d+-compute@developer\.gserviceaccount\.com$/), so the Terraform variants were aligned to the same behavior (default compute + App Engine SAs) across hcl/plan/state. I've documented this under a new "Behavior changes beyond the mechanical migration" section in the PR description. No revert.
| _['encryption_config'] != empty && | ||
| _['encryption_config'].all(_['kms_key_name'] != empty) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-encryption-configured-terraform-plan |
There was a problem hiding this comment.
🔵 suggestion — Logic changed from "require encryption_config inside cluster_config" to "allow empty cluster_config OR require encryption_config". This means a Dataproc cluster with no cluster_config block at all now passes the check, whereas before it would have failed. Verify this is the intended behavior for the security policy.
There was a problem hiding this comment.
Intended. An absent HCL key is null (not []), and calling .all() on null errors rather than passing vacuously, so the cluster_config == empty || guard is necessary to avoid a runtime error. Semantically, a Dataproc cluster that declares no cluster_config block has nothing on which to assert an encryption_config, so it passes. The guard is applied consistently across all three Terraform variants (hcl/plan/state), and the native check is unaffected. Documented in the PR description's behavior-change note. No change.
| values['replication'].all( | ||
| _['user_managed'].length == 1 && | ||
| _['user_managed'].all( | ||
| _['replicas'] == empty || | ||
| _['replicas'].all( | ||
| _['customer_managed_encryption'].length == 1 | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🔵 suggestion — Added _['replicas'] == empty || which allows secrets with no replicas to pass the CMEK check. The original required every replica to have customer_managed_encryption. If a user_managed replication config with zero replicas is valid Terraform, this is fine; otherwise it silently weakens the check.
There was a problem hiding this comment.
The _['replicas'] == empty || guard is correct. replicas is a nested block, so an absent replicas is null, and .all() on null errors rather than passing vacuously; the guard prevents that. Semantically it covers the case where a user_managed replication declares no per-replica entries to assert CMEK on. While reviewing this I also found and fixed a pre-existing array-access bug in the plan/state variants of this same check: replication, user_managed, and replicas are all nested blocks (arrays in the values/JSON model), but the plan/state bodies used object-style bracket chains (change.after['replication']['user_managed']['replicas']). I rewrote them with array semantics to match the HCL variant, including this same guard. Pushed in 0902cdf and noted in the PR description.
| @@ -7386,8 +7370,8 @@ queries: | |||
| terraform.state.resources.where(type == 'google_dns_managed_zone').where(values['visibility'] == 'public' || values['visibility'] == empty).all( | |||
| values.dnssec_config != empty && | |||
| values.dnssec_config.all( | |||
| state == "on" && | |||
| non_existence == "nsec3" | |||
| _['state'] == "on" && | |||
There was a problem hiding this comment.
🔵 suggestion — Good catch fixing the plan and state variants to use _['state'] and _['non_existence'] bracket notation, matching the HCL query update. This looks like a pre-existing bug fix bundled into the migration.
There was a problem hiding this comment.
Thanks. Yes, the plan and state variants were updated to use _['state'] and _['non_existence'] bracket notation matching the corrected HCL query, bundled in with this migration. No further change needed here.
| email == /^(default|\d{12}-compute@developer\.gserviceaccount\.com)$/ && | ||
| scopes.contains(/(https?:\/\/www\.googleapis\.com\/auth\/)?cloud-platform/) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-instances-not-configured-with-default-service-account-full-access-cloud-api-terraform-plan |
There was a problem hiding this comment.
🟡 warning — Logic change: the old query used .all() to check that matching service accounts have no cloud-platform scope, but the new query uses .none() with &&, which changes the semantics. Old: "all service accounts with default email must have no cloud-platform scope". New: "no service account has both default email AND cloud-platform scope". These are logically equivalent only if scopes is a flat field — verify that scopes.contains(...) on values behaves the same as the old arguments.scopes.none(...) on blocks.
| filters: | | ||
| asset.platform == 'terraform-hcl' && terraform.resources.contains(nameLabel == 'google_dataproc_cluster') | ||
| mql: | | ||
| terraform.resources('google_dataproc_cluster').all( | ||
| blocks.where(type == 'cluster_config').all( | ||
| blocks.where(type == 'gce_cluster_config').all(arguments.service_account != empty) | ||
| terraform.resources("google_dataproc_cluster").all( | ||
| values.cluster_config != empty && | ||
| values.cluster_config.all( | ||
| _['gce_cluster_config'] != empty && | ||
| _['gce_cluster_config'].all( | ||
| _['service_account'] != empty && | ||
| _['service_account'] != /^\d+-compute@developer\.gserviceaccount\.com$/ && | ||
| _['service_account'] != /^[a-z0-9\-]+@appspot\.gserviceaccount\.com$/ | ||
| ) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-no-default-service-account-terraform-plan |
There was a problem hiding this comment.
🟡 warning — The HCL variant for dataproc-cluster-no-default-service-account now adds two extra regex checks (rejecting default compute and appspot service accounts) that were not in the original HCL query. This is a behavioral change beyond the mechanical migration — the plan/state variants are also updated to match, which is good, but this should be called out as an intentional policy tightening rather than a pure refactor.
| asset.platform == 'terraform-plan' && terraform.plan.resourceChanges.contains(type == 'google_dataproc_cluster') | ||
| mql: | | ||
| terraform.plan.resourceChanges.where(type == 'google_dataproc_cluster').all( | ||
| change.after.cluster_config != empty && | ||
| change.after.cluster_config.all(_['encryption_config'] != empty) | ||
| change.after.cluster_config == empty || | ||
| change.after.cluster_config.all( | ||
| _['encryption_config'] != empty && | ||
| _['encryption_config'].all(_['kms_key_name'] != empty) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-encryption-configured-terraform-state | ||
| filters: | | ||
| asset.platform == 'terraform-state' && terraform.state.resources.contains(type == 'google_dataproc_cluster') | ||
| mql: | | ||
| terraform.state.resources.where(type == 'google_dataproc_cluster').all( | ||
| values.cluster_config != empty && | ||
| values.cluster_config.all(_['encryption_config'] != empty) | ||
| values.cluster_config == empty || | ||
| values.cluster_config.all( | ||
| _['encryption_config'] != empty && | ||
| _['encryption_config'].all(_['kms_key_name'] != empty) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-internal-ip-only | ||
| title: Ensure Dataproc clusters use internal IP addresses only |
There was a problem hiding this comment.
🟡 warning — The dataproc encryption check changed from requiring cluster_config != empty (old HCL had .all(blocks...) which implicitly required it) to cluster_config == empty ||, and the plan/state variants were also updated to match this relaxed logic. This means a cluster with no cluster_config block now passes, which may be intentional but is a semantic change that weakens the security check.
| @@ -9802,13 +9800,13 @@ queries: | |||
| ) | |||
There was a problem hiding this comment.
🟡 warning — SSH security group check changed from to_port == 22 to from_port <= 22 && to_port >= 22 (port range matching). This is a meaningful improvement that catches ranges containing port 22, but it's a behavioral change beyond the blocks→values migration. The same pattern is applied to the terraform-plan and terraform-state variants, which is good for consistency.
| filters: asset.platform == "terraform-hcl" && terraform.resources.contains(nameLabel == "aws_ecs_task_definition") | ||
| mql: | | ||
| terraform.resources("aws_ecs_task_definition").all( | ||
| blocks.where(type == "volume").all( | ||
| blocks.where(type == "efs_volume_configuration").all( | ||
| attributes.transit_encryption.value == "ENABLED" | ||
| ) | ||
| values.volume.where(_['efs_volume_configuration'] != empty).all( | ||
| _['efs_volume_configuration'].all(_['transit_encryption'] == "ENABLED") | ||
| ) | ||
| ) | ||
| - uid: mondoo-aws-security-ecs-efs-volume-transit-encryption-terraform-plan | ||
| filters: asset.platform == "terraform-plan" && terraform.plan.resourceChanges.contains(type == "aws_ecs_task_definition") | ||
| mql: | | ||
| terraform.plan.resourceChanges.where(type == "aws_ecs_task_definition").all( | ||
| change.after.volume.where(_['efs_volume_configuration'] != empty).all( | ||
| _['efs_volume_configuration']['transit_encryption'] == "ENABLED" | ||
| _['efs_volume_configuration'].all(_['transit_encryption'] == "ENABLED") | ||
| ) | ||
| ) | ||
| - uid: mondoo-aws-security-ecs-efs-volume-transit-encryption-terraform-state | ||
| filters: asset.platform == "terraform-state" && terraform.state.resources.contains(type == "aws_ecs_task_definition") | ||
| mql: | |
There was a problem hiding this comment.
🔵 suggestion — ECS EFS volume transit encryption: the plan and state variants changed from _['efs_volume_configuration']['transit_encryption'] (direct map access) to _['efs_volume_configuration'].all(_['transit_encryption'] == ...) (array iteration). This is a structural change — verify that efs_volume_configuration is indeed an array in plan/state bodies, not a single map, otherwise .all() may behave unexpectedly.
| @@ -38250,9 +38239,7 @@ queries: | |||
| filters: asset.platform == "terraform-hcl" && terraform.resources.contains(nameLabel == "aws_sagemaker_code_repository") | |||
There was a problem hiding this comment.
🔵 suggestion — SageMaker code repository check lost its != empty existence guard: old code had blocks.where(...).any(...) which returns false on empty, but values["git_config"].any(...) could behave differently if git_config is null/empty. Consider adding values["git_config"] != empty && as a guard.
| filters: asset.platform == "terraform-hcl" && terraform.resources.contains(nameLabel == "aws_launch_configuration") | ||
| mql: | |
There was a problem hiding this comment.
🔵 suggestion — Launch configuration encrypted volumes: the old code had existence guards (blocks.where(...) != empty) for both ebs_block_device and root_block_device that are now removed. If either values['ebs_block_device'] or values['root_block_device'] is null/empty, .all() could be vacuously true, potentially allowing unencrypted volumes to pass. Consider adding != empty guards.
|
/review |
There was a problem hiding this comment.
Consistent mechanical migration from blocks/arguments to values-based access; a few logic changes worth verifying
Additional findings (file/line not in diff):
- 🟡
content/mondoo-aws-security.mql.yaml:38727— Pub/Sub push endpoint HTTPS check added_['push_endpoint'] == empty ||, so apush_configblock with an empty/missingpush_endpointnow passes. Previously this would have failed the regex match. Confirm that an empty push endpoint is a valid safe state and not an accidental misconfiguration.
| values.cluster_config == empty || | ||
| values.cluster_config.all( | ||
| _['encryption_config'] != empty && | ||
| _['encryption_config'].all(_['kms_key_name'] != empty) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-encryption-configured-terraform-plan | ||
| filters: | | ||
| asset.platform == 'terraform-plan' && terraform.plan.resourceChanges.contains(type == 'google_dataproc_cluster') | ||
| mql: | |
There was a problem hiding this comment.
🟡 warning — Dataproc encryption check changed from requiring cluster_config to exist (blocks.where(type == 'cluster_config').all(...)) to allowing it to be absent (values.cluster_config == empty ||). This relaxes the policy — a cluster with no cluster_config will now pass. The same change was applied to the plan and state variants (lines ~13726, ~13735), so it appears intentional, but please confirm this is the desired behavior.
There was a problem hiding this comment.
Intentional and documented in the PR description. For google_dataproc_cluster, cluster_config is an optional block; when absent the values model returns null, and calling .all() on null errors rather than passing vacuously. The values.cluster_config == empty || guard short-circuits that case. A cluster with no cluster_config block declares no clusters/disks to encrypt, so there is nothing for the encryption assertion to evaluate. No change.
| terraform.resources("google_dataproc_cluster").all( | ||
| values.cluster_config != empty && | ||
| values.cluster_config.all( | ||
| _['gce_cluster_config'] != empty && |
There was a problem hiding this comment.
🔵 suggestion — The Dataproc service-account check was strengthened to also reject default compute and appspot service accounts (matching the Cloud Run checks). Good improvement — just flagging it as a non-mechanical behavioral change that should be covered by tests.
There was a problem hiding this comment.
Thanks. This is documented in the PR description. These are MQL policy checks rather than Go code, so they are validated by cnspec policy lint and the policy/remediation tooling rather than a unit-test framework. The strengthened service-account assertion (rejecting the default Compute SA and the App Engine default SA, and requiring a non-empty custom SA) lints clean across the hcl/plan/state variants. No code change.
| blocks.where(type == 'user_managed').all( | ||
| blocks.where(type == 'replicas').all( | ||
| blocks.one(type == 'customer_managed_encryption') | ||
| terraform.resources("google_secret_manager_secret").all( |
There was a problem hiding this comment.
🟡 warning — Secret Manager CMEK check: _['replicas'] == empty || _['replicas'].all(...) now allows secrets with no replicas in user_managed replication to pass. The old code required replicas to exist and each to have customer_managed_encryption. If a user_managed block without any replicas is invalid Terraform anyway this is fine, but if it can occur in practice this weakens the CMEK guarantee.
There was a problem hiding this comment.
The guard is correct and does not weaken the CMEK guarantee. In the Terraform google provider, a user_managed replication block requires at least one replicas entry (MinItems 1), so a user_managed block with zero replicas is invalid Terraform and cannot exist. The _['replicas'] == empty || guard primarily prevents a null-.all() error for shapes where the key is absent; for any real user_managed config the .all(_['customer_managed_encryption'].length == 1) branch runs and enforces CMEK on every replica. No change.
| @@ -9802,13 +9800,13 @@ queries: | |||
| ) | |||
There was a problem hiding this comment.
🔵 suggestion — SSH security-group check improved from to_port == 22 to from_port <= 22 && to_port >= 22, correctly catching port ranges that include 22. Nice fix — the same treatment was applied to the state variant but verify the plan variant (line ~9807) also matches.
There was a problem hiding this comment.
Verified. All three Terraform variants of this check use the same port-range form from_port <= 22 && to_port >= 22: the hcl variant (mondoo-aws-security-secgroup-restricted-ssh-terraform-hcl), the plan variant (...-terraform-plan, which evaluates change.after.ingress with the identical predicate), and the state variant (...-terraform-state). The plan variant was not missed. No change needed.
| blocks.where(type == "logging_configuration").any( | ||
| arguments.level != "OFF" && arguments.level != null | ||
| values["logging_configuration"] != null && values["logging_configuration"].any( | ||
| _["level"] != "OFF" && _["level"] != null |
There was a problem hiding this comment.
🔵 suggestion — Step Functions logging/encryption checks use != null for existence, while most other AWS checks in this file use != empty. Verify null vs empty semantics are equivalent in MQL for these resources, or standardize on one form for consistency.
There was a problem hiding this comment.
Fixed. These lines are part of this PR's diff (the blocks.where(...) to values[...] migration), so I switched the outer collection guards on the Step Functions logging and encryption checks from != null to != empty across the hcl/plan/state variants for consistency with the prevailing idiom. Behavior is unchanged: an absent HCL block is null and a present-but-empty block list both fail the guard identically, and either way .any() over an empty list would also yield false. Pushed in 6018ac7. (Inner scalar guards like _['level'] != null / _['type'] != null are left as != null since those are scalar fields, not collections.)
| values.origin == null || | ||
| values.origin.all( | ||
| _['s3_origin_config'] == null || | ||
| _['s3_origin_config'] == [] || |
There was a problem hiding this comment.
🔵 suggestion — CloudFront S3 origin OAC check: _['s3_origin_config'] == null || _['s3_origin_config'] == [] mixes null and [] comparisons. Consider using == empty for consistency with the rest of the file, unless there's a semantic difference in MQL between null, empty array, and empty.
There was a problem hiding this comment.
Simplified. This line is part of this PR's diff, so I collapsed _['s3_origin_config'] == null || _['s3_origin_config'] == [] to _['s3_origin_config'] == empty across the hcl/plan/state variants. In MQL == empty is true for both null and empty collections, so it cleanly replaces the two-clause check with identical semantics. Lints clean. Pushed in 6018ac7.
bb6cd42 to
4d82983
Compare
| email == /^(default|\d{12}-compute@developer\.gserviceaccount\.com)$/ && | ||
| scopes.contains(/(https?:\/\/www\.googleapis\.com\/auth\/)?cloud-platform/) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-instances-not-configured-with-default-service-account-full-access-cloud-api-terraform-plan |
There was a problem hiding this comment.
🟡 warning — Possible logic change in service_account full-access check. The old query found service accounts matching the default email pattern and then checked that those specific accounts had no cloud-platform scope. The new query uses .none(email == ... && scopes.contains(...)), which asserts no single entry has both the default email and the scope. This is subtly different: the old version would fail if any default SA existed with the scope, while the new version using .none() with && would pass if a default SA exists without the scope and a non-default SA has the scope. Verify this is the intended semantic.
| values['access'].none(_['domain'] != empty) | ||
| ) | ||
| - uid: mondoo-gcp-security-bigquery-dataset-no-domain-wide-access-terraform-plan |
There was a problem hiding this comment.
🟡 warning — Added existence guard changes semantics for BigQuery dataset domain access. Old: blocks.where(type == 'access').none(arguments.domain != empty) — if no access blocks exist, .none() is vacuously true (pass). New: values['access'] == empty || values['access'].none(...) — also passes when empty, so this is fine. Just confirming the || short-circuit is intentional (datasets with no explicit access blocks should pass).
| asset.platform == 'terraform-plan' && terraform.plan.resourceChanges.contains(type == 'google_dataproc_cluster') | ||
| mql: | | ||
| terraform.plan.resourceChanges.where(type == 'google_dataproc_cluster').all( | ||
| change.after.cluster_config != empty && | ||
| change.after.cluster_config.all(_['encryption_config'] != empty) | ||
| change.after.cluster_config == empty || | ||
| change.after.cluster_config.all( | ||
| _['encryption_config'] != empty && | ||
| _['encryption_config'].all(_['kms_key_name'] != empty) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-encryption-configured-terraform-state |
There was a problem hiding this comment.
🟡 warning — Dataproc encryption check changed from != empty to == empty ||. Old HCL query required encryption_config to exist within cluster_config. New query passes if cluster_config is empty (no cluster config = pass). The plan/state variants were also updated to match. This is a deliberate relaxation — verify that a missing cluster_config should indeed be treated as compliant rather than non-compliant.
| filters: | | ||
| asset.platform == 'terraform-hcl' && terraform.resources.contains(nameLabel == 'google_dataproc_cluster') | ||
| mql: | | ||
| terraform.resources('google_dataproc_cluster').all( | ||
| blocks.where(type == 'cluster_config').all( | ||
| blocks.where(type == 'gce_cluster_config').all(arguments.service_account != empty) | ||
| terraform.resources("google_dataproc_cluster").all( | ||
| values.cluster_config != empty && | ||
| values.cluster_config.all( | ||
| _['gce_cluster_config'] != empty && | ||
| _['gce_cluster_config'].all( | ||
| _['service_account'] != empty && | ||
| _['service_account'] != /^\d+-compute@developer\.gserviceaccount\.com$/ && | ||
| _['service_account'] != /^[a-z0-9\-]+@appspot\.gserviceaccount\.com$/ | ||
| ) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-no-default-service-account-terraform-plan |
There was a problem hiding this comment.
🔵 suggestion — Dataproc service account check now also rejects default service accounts. The old HCL query only checked service_account != empty, but the new one adds regex checks for default compute and appspot SAs. The plan/state variants got the same addition. This is a security improvement beyond the mechanical migration — good change, but worth noting it's a behavior change, not just a syntax migration.
| _['user_managed'].length == 1 && | ||
| _['user_managed'].all( | ||
| _['replicas'] == empty || | ||
| _['replicas'].all( | ||
| _['customer_managed_encryption'].length == 1 | ||
| ) |
There was a problem hiding this comment.
🟡 warning — Secret Manager CMEK: blocks.one() replaced with .length == 1. The old query used blocks.one(type == 'replication') (exactly one matching block) and the new uses values['replication'].length == 1. Also, _['replicas'] == empty || was added, allowing secrets with no replicas to pass. Verify that a user_managed replication with zero replicas is a valid and compliant configuration.
| values['retention_policy'].all( | ||
| _['enabled'] == true && _['days'] >= 365 || _['days'] == 0 | ||
| ) |
There was a problem hiding this comment.
🟡 warning — Operator precedence concern with || and &&. values['retention_policy'].all(_['enabled'] == true && _['days'] >= 365 || _['days'] == 0) — due to && binding tighter than ||, this reads as (enabled == true && days >= 365) || (days == 0). If days == 0 the check passes even if enabled == false. This matches the old query's behavior, but consider adding explicit parentheses for clarity.
| terraform.resources("digitalocean_firewall").all( | ||
| blocks.where(type == "inbound_rule").none( | ||
| arguments["protocol"] == "tcp" && | ||
| arguments["port_range"] == /^(3306|5432|27017|6379|9092|9200)$/ && | ||
| arguments["source_addresses"].contains("0.0.0.0/0") | ||
| ) && | ||
| blocks.where(type == "inbound_rule").none( | ||
| arguments["protocol"] == "tcp" && | ||
| arguments["port_range"] == /^(3306|5432|27017|6379|9092|9200)$/ && | ||
| arguments["source_addresses"].contains("::/0") | ||
| ) | ||
| values["inbound_rule"] == empty || | ||
| values["inbound_rule"].none(_["protocol"] == "tcp" && _["port_range"] == /^(3306|5432|27017|6379|9092|9200)$/ && _["source_addresses"].contains("0.0.0.0/0")) && |
There was a problem hiding this comment.
🟡 warning — Operator precedence concern with || and &&. values["inbound_rule"] == empty || values["inbound_rule"].none(...) && values["inbound_rule"].none(...) — because && binds tighter than ||, this reads as empty || (none1 && none2). This is likely correct, but the SSH and RDP checks above have the same pattern. Consider adding parentheses to make the grouping explicit and prevent future misreading.
| values.protocol != "http" || | ||
| values.http != empty && | ||
| values.http.all(_['redirect_http'] == true) | ||
| ) |
There was a problem hiding this comment.
🟡 warning — Operator precedence: || vs && in LB redirect check. values.protocol != "http" || values.http != empty && values.http.all(...) — && binds tighter, so this is != http || (http != empty && all(...)). If protocol is http and values.http is empty, the whole expression is false. The plan and state variants at lines 1538-1541 and 1543-1545 have the same pattern. This seems correct but fragile — parentheses would help readability.
| terraform.plan.resourceChanges.where(type == 'google_dns_managed_zone').where(change.after['visibility'] == 'public' || change.after['visibility'] == empty).all( | ||
| change.after.dnssec_config != empty && | ||
| change.after.dnssec_config.all( | ||
| state == "on" && | ||
| non_existence == "nsec3" | ||
| _['state'] == "on" && |
There was a problem hiding this comment.
🔵 suggestion — Plan/state variants also updated for DNSSEC NSEC3 check. The plan and state queries gained _['state'] and _['non_existence'] bracket notation, which is a good consistency fix aligning them with the HCL variant's style.
| filters: asset.platform == "terraform-state" && terraform.state.resources.contains(type == "azurerm_private_endpoint") | ||
| mql: | | ||
| terraform.state.resources.where(type == "azurerm_private_endpoint").any( | ||
| values['private_service_connection'] != empty && |
There was a problem hiding this comment.
🔵 suggestion — Good: null guards added to plan/state variants for private_service_connection. Several Azure private endpoint checks (ACR, managed HSM, disk access, recovery vault) now include change.after['private_service_connection'] != empty && / values['private_service_connection'] != empty && guards in plan/state variants that previously lacked them. This is a correctness improvement.
4d82983 to
6407574
Compare
| email == /^(default|\d{12}-compute@developer\.gserviceaccount\.com)$/ && | ||
| scopes.contains(/(https?:\/\/www\.googleapis\.com\/auth\/)?cloud-platform/) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-instances-not-configured-with-default-service-account-full-access-cloud-api-terraform-plan |
There was a problem hiding this comment.
🟡 warning — Logic change: the old query checked that instances with default service account emails had scopes that did NOT contain cloud-platform (using .all(.scopes.none(...))). The new query uses .none(email == ... && scopes.contains(...)) which inverts the logic — it now passes if any matching instance lacks the scope, rather than requiring all matching instances to lack it. Double-check this is the intended semantic change.
| terraform.resources("google_secret_manager_secret").all( | ||
| values['replication'].length == 1 && | ||
| values['replication'].all( | ||
| _['user_managed'].length == 1 && | ||
| _['user_managed'].all( | ||
| _['replicas'] == empty || | ||
| _['replicas'].all( | ||
| _['customer_managed_encryption'].length == 1 | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🟡 warning — Using .length == 1 is a stricter check than the old blocks.one(...) pattern for replication and user_managed. Also, the new query adds _['replicas'] == empty || which allows secrets with zero replicas to pass — the old query required replicas to each have customer_managed_encryption. Verify this relaxation is intentional.
| filters: | | ||
| asset.platform == 'terraform-hcl' && terraform.resources.contains(nameLabel == 'google_dataproc_cluster') | ||
| mql: | | ||
| terraform.resources('google_dataproc_cluster').all( | ||
| blocks.where(type == 'cluster_config').all( | ||
| blocks.where(type == 'gce_cluster_config').all(arguments.service_account != empty) | ||
| terraform.resources("google_dataproc_cluster").all( | ||
| values.cluster_config != empty && | ||
| values.cluster_config.all( | ||
| _['gce_cluster_config'] != empty && | ||
| _['gce_cluster_config'].all( | ||
| _['service_account'] != empty && | ||
| _['service_account'] != /^\d+-compute@developer\.gserviceaccount\.com$/ && | ||
| _['service_account'] != /^[a-z0-9\-]+@appspot\.gserviceaccount\.com$/ | ||
| ) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-no-default-service-account-terraform-plan |
There was a problem hiding this comment.
🔵 suggestion — The Dataproc custom service account check was strengthened: the HCL variant now also rejects default compute and appspot service accounts (matching the plan/state variants). This is a good improvement but is a behavioral change beyond the mechanical migration — worth calling out in the PR description.
| asset.platform == 'terraform-plan' && terraform.plan.resourceChanges.contains(type == 'google_dataproc_cluster') | ||
| mql: | | ||
| terraform.plan.resourceChanges.where(type == 'google_dataproc_cluster').all( | ||
| change.after.cluster_config != empty && | ||
| change.after.cluster_config.all(_['encryption_config'] != empty) | ||
| change.after.cluster_config == empty || | ||
| change.after.cluster_config.all( | ||
| _['encryption_config'] != empty && | ||
| _['encryption_config'].all(_['kms_key_name'] != empty) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-encryption-configured-terraform-state | ||
| filters: | | ||
| asset.platform == 'terraform-state' && terraform.state.resources.contains(type == 'google_dataproc_cluster') | ||
| mql: | | ||
| terraform.state.resources.where(type == 'google_dataproc_cluster').all( | ||
| values.cluster_config != empty && | ||
| values.cluster_config.all(_['encryption_config'] != empty) | ||
| values.cluster_config == empty || | ||
| values.cluster_config.all( | ||
| _['encryption_config'] != empty && | ||
| _['encryption_config'].all(_['kms_key_name'] != empty) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-internal-ip-only | ||
| title: Ensure Dataproc clusters use internal IP addresses only |
There was a problem hiding this comment.
🟡 warning — The Dataproc encryption plan+state variants changed from requiring cluster_config != empty to cluster_config == empty ||, making the check pass when cluster_config is absent. This is a logic relaxation. Ensure this is intentional — it means a Dataproc cluster with no cluster_config block now passes the encryption check.
| change.after.http.all(_['redirect_http'] == true) | ||
| ) | ||
| - uid: mondoo-hetzner-security-lb-http-redirected-to-https-terraform-state | ||
| filters: asset.platform == "terraform-state" && terraform.state.resources.contains(type == "hcloud_load_balancer_service") | ||
| mql: | | ||
| terraform.state.resources.where(type == "hcloud_load_balancer_service").all( | ||
| values.protocol != "http" || |
There was a problem hiding this comment.
🟡 warning — Operator precedence concern: values.protocol != "http" || values.http != empty && values.http.all(...) — due to && binding tighter than ||, this reads as (protocol != http) || (http != empty && http.all(...)). The old code had the same structure, but the addition of values.http != empty && guard means a missing http block when protocol == "http" now evaluates the && short-circuit to false, correctly failing. This is fine, just noting the precedence is subtle.
| mql: | | ||
| terraform.resources("digitalocean_firewall").all( | ||
| blocks.where(type == "inbound_rule").none( | ||
| arguments["protocol"] == "tcp" && | ||
| arguments["port_range"] == /^(3306|5432|27017|6379|9092|9200)$/ && | ||
| arguments["source_addresses"].contains("0.0.0.0/0") | ||
| ) && | ||
| blocks.where(type == "inbound_rule").none( | ||
| arguments["protocol"] == "tcp" && | ||
| arguments["port_range"] == /^(3306|5432|27017|6379|9092|9200)$/ && | ||
| arguments["source_addresses"].contains("::/0") | ||
| ) | ||
| values["inbound_rule"] == empty || |
There was a problem hiding this comment.
🟡 warning — Operator precedence: values["inbound_rule"] == empty || values["inbound_rule"].none(...) && values["inbound_rule"].none(...) — since && binds tighter than ||, this is empty || (none1 && none2) which is correct. But the SSH and RDP variants above have the same pattern. Consider adding parentheses for clarity to avoid future maintenance mistakes.
| values['retention_policy'] != empty && | ||
| values['retention_policy'].all( | ||
| _['enabled'] == true && _['days'] >= 365 || _['days'] == 0 |
There was a problem hiding this comment.
🟡 warning — Operator precedence in retention policy check: _['enabled'] == true && _['days'] >= 365 || _['days'] == 0. This evaluates as (enabled == true && days >= 365) || (days == 0), meaning days == 0 passes regardless of enabled. The old code had the same ambiguity. Consider adding parentheses to clarify intent.
6407574 to
7ca234b
Compare
7ca234b to
68529f1
Compare
| email == /^(default|\d{12}-compute@developer\.gserviceaccount\.com)$/ && | ||
| scopes.contains(/(https?:\/\/www\.googleapis\.com\/auth\/)?cloud-platform/) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-instances-not-configured-with-default-service-account-full-access-cloud-api-terraform-plan |
There was a problem hiding this comment.
🟡 warning — Logic change: the old query checked that instances with default service account emails that also have cloud-platform scope fail (using .all(.none(scopes...)) on matching SAs). The new query uses .none(email == ... && scopes.contains(...)) which changes semantics — it now passes if any SA lacks the matching email, even if another SA on the same instance has a default email with cloud-platform scope. Verify this is the intended behavior and not a regression in the security check.
| _['user_managed'].length == 1 && | ||
| _['user_managed'].all( | ||
| _['replicas'] == empty || | ||
| _['replicas'].all( | ||
| _['customer_managed_encryption'].length == 1 | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🔵 suggestion — Using .length == 1 instead of != empty is a stricter check that requires exactly one replication/user_managed block. This is a semantic change from the old blocks.one(...) pattern. It's likely correct for this resource, but worth confirming that Terraform can't produce >1 replication blocks.
| filters: | | ||
| asset.platform == 'terraform-hcl' && terraform.resources.contains(nameLabel == 'google_dataproc_cluster') | ||
| mql: | | ||
| terraform.resources('google_dataproc_cluster').all( | ||
| blocks.where(type == 'cluster_config').all( | ||
| blocks.where(type == 'gce_cluster_config').all(arguments.service_account != empty) | ||
| terraform.resources("google_dataproc_cluster").all( | ||
| values.cluster_config != empty && | ||
| values.cluster_config.all( | ||
| _['gce_cluster_config'] != empty && | ||
| _['gce_cluster_config'].all( | ||
| _['service_account'] != empty && | ||
| _['service_account'] != /^\d+-compute@developer\.gserviceaccount\.com$/ && | ||
| _['service_account'] != /^[a-z0-9\-]+@appspot\.gserviceaccount\.com$/ | ||
| ) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-no-default-service-account-terraform-plan |
There was a problem hiding this comment.
🟡 warning — The Dataproc service account check was strengthened to also reject default compute and appspot service accounts (added regex patterns). This is a security improvement but is a behavioral change, not just a syntax migration. The plan/state variants were also updated to match. Make sure this intentional tightening is called out in the PR description.
| asset.platform == 'terraform-plan' && terraform.plan.resourceChanges.contains(type == 'google_dataproc_cluster') | ||
| mql: | | ||
| terraform.plan.resourceChanges.where(type == 'google_dataproc_cluster').all( | ||
| change.after.cluster_config != empty && | ||
| change.after.cluster_config.all(_['encryption_config'] != empty) | ||
| change.after.cluster_config == empty || | ||
| change.after.cluster_config.all( | ||
| _['encryption_config'] != empty && | ||
| _['encryption_config'].all(_['kms_key_name'] != empty) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-encryption-configured-terraform-state | ||
| filters: | | ||
| asset.platform == 'terraform-state' && terraform.state.resources.contains(type == 'google_dataproc_cluster') | ||
| mql: | | ||
| terraform.state.resources.where(type == 'google_dataproc_cluster').all( | ||
| values.cluster_config != empty && | ||
| values.cluster_config.all(_['encryption_config'] != empty) | ||
| values.cluster_config == empty || | ||
| values.cluster_config.all( | ||
| _['encryption_config'] != empty && | ||
| _['encryption_config'].all(_['kms_key_name'] != empty) | ||
| ) | ||
| ) | ||
| - uid: mondoo-gcp-security-dataproc-cluster-internal-ip-only | ||
| title: Ensure Dataproc clusters use internal IP addresses only |
There was a problem hiding this comment.
🟡 warning — The Dataproc encryption check changed from cluster_config != empty && ... encryption_config != empty to cluster_config == empty || ... encryption_config != empty && kms_key_name != empty. This flips the logic: clusters without cluster_config now pass instead of fail. The plan/state variants were also updated. Verify this is the intended semantic — a cluster with no config block passing an encryption check seems permissive.
| ) | ||
| - uid: mondoo-hetzner-security-lb-http-redirected-to-https-terraform-plan |
There was a problem hiding this comment.
🟡 warning — Operator precedence concern: values.protocol != "http" || values.http != empty && values.http.all(...) — the && binds tighter than ||, so this reads as (protocol != http) || (http != empty && http.all(...)). If protocol is http and values.http is empty, the check passes vacuously (no redirect configured). The old code had the same issue with blocks, but worth confirming this is acceptable. Same pattern appears in the certificate check below and in the plan/state variants.
| terraform.resources("digitalocean_firewall").all( | ||
| blocks.where(type == "inbound_rule").none( | ||
| arguments["protocol"] == "tcp" && arguments["port_range"] == "22" && arguments["source_addresses"].contains("0.0.0.0/0") | ||
| ) && | ||
| blocks.where(type == "inbound_rule").none( | ||
| arguments["protocol"] == "tcp" && arguments["port_range"] == "22" && arguments["source_addresses"].contains("::/0") | ||
| ) | ||
| values["inbound_rule"] == empty || | ||
| values["inbound_rule"].none(_["protocol"] == "tcp" && _["port_range"] == "22" && _["source_addresses"].contains("0.0.0.0/0")) && | ||
| values["inbound_rule"].none(_["protocol"] == "tcp" && _["port_range"] == "22" && _["source_addresses"].contains("::/0")) | ||
| ) | ||
| - uid: mondoo-digitalocean-security-firewall-no-unrestricted-ssh-terraform-plan | ||
| filters: asset.platform == "terraform-plan" && terraform.plan.resourceChanges.contains(type == "digitalocean_firewall") |
There was a problem hiding this comment.
🟡 warning — Operator precedence issue: values["inbound_rule"] == empty || values["inbound_rule"].none(...) && values["inbound_rule"].none(...) — since && binds tighter than ||, this evaluates as empty || (none1 && none2). This works correctly by coincidence, but wrapping the && clause in parentheses would make the intent clearer and prevent future maintenance bugs. Same pattern in RDP, DB ports, and all-ports checks.
| ) | ||
| ) | ||
| - uid: mondoo-azure-security-activity-log-retention-365-days-terraform-plan |
There was a problem hiding this comment.
🔵 suggestion — Operator precedence: _['enabled'] == true && _['days'] >= 365 || _['days'] == 0 reads as (enabled && days>=365) || (days==0). If days == 0 and enabled == false, the check still passes. This matches the old behavior, but adding parentheses would improve clarity: _['enabled'] == true && (_['days'] >= 365 || _['days'] == 0).
…ate bodies terraform provider 13.2.0 added a `values()` dict field to terraform.block (mondoohq/mql#8289) that folds child blocks into the arguments dict as lists-of-maps keyed by block type, recursively — the same shape Terraform plan (change.after) and state (values) already expose. This rewrites 369 `*-terraform-hcl` variant bodies that used the bespoke nested-block traversal dialect (blocks.where(type == ...) / blocks { ... } / nested arguments.X) so they mirror their plan/state sibling: `.values` as the root accessor and `_['key']` for nested map access. Each HCL body is now the state sibling's body verbatim with the root selector swapped from terraform.state.resources.where(type == "X") to terraform.resources("X"). Only `mql:` bodies of HCL variants changed; filters, docs, remediation, compliance tags, and query counts are untouched. All eight policy files lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r checks) in values rewrite The block.values rewrite transplanted HCL bodies from their weaker -terraform-state siblings, silently dropping existence guards, inner field checks, and "exactly one" semantics, and substituting .any() for .all(). Restore semantic equivalence to the original origin/main HCL bodies expressed via the values field across AWS, Azure, GCP, and OCI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…) style Mirror the original HCL body's `.all(email != …)` form instead of the equivalent `.none(email == …)`, matching the sibling variants and the rest of the policy. No behavioral change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Address mondoo-code-review feedback on PR #2758: - secgroup-restricted-ssh (aws): terraform hcl/plan/state variants only matched ingress rules with exactly to_port == 22, missing port RANGES that span 22. Align with the native aws-security-group variant by using from_port <= 22 && to_port >= 22. - dataproc-cluster-no-default-service-account (gcp): the terraform-plan variant lacked the default compute/appspot service-account regex exclusions present in the hcl and state variants. Add them for parity. - dataproc-cluster-encryption-configured (gcp): align terraform-plan and terraform-state with the hcl variant — use the cluster_config == empty || null guard and assert kms_key_name is set inside encryption_config. - cloud-dns-dnssec-nsec3-enabled (gcp): switch bare field access to _['state'] / _['non_existence'] across all three terraform variants for consistency with the rest of the PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Nested HCL sub-blocks are always represented as arrays in the terraform block.values model (and as JSON arrays in plan/state). Accessing a nested sub-block with a chained bracket index treats the array as a map and does not behave as intended. - ECS EFS transit-encryption (aws): replace _['efs_volume_configuration']['transit_encryption'] with _['efs_volume_configuration'].all(_['transit_encryption'] == "ENABLED") across the hcl/plan/state variants. - Secret Manager CMEK (gcp): the plan/state variants used object-style bracket chains on replication/user_managed/replicas (all nested blocks, i.e. arrays). Rewrite them with array semantics to match the corrected HCL variant, including the _['replicas'] == empty guard for parity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address re-review feedback on PR #2758: - Step Functions logging/encryption: use `!= empty` instead of `!= null` for the outer collection guards, matching the prevailing idiom across the file and this PR. Behavior is unchanged (an absent HCL block is null and an empty block list both fail the guard identically). - CloudFront S3-origin OAC: collapse `_['s3_origin_config'] == null || _['s3_origin_config'] == []` to `_['s3_origin_config'] == empty`, which covers both null and empty-collection cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review: an aws_sagemaker_code_repository with no git_config block would make .any() error on null rather than fail. Add a values["git_config"] != empty guard across the hcl/plan/state variants so the check fails closed, matching the != empty guard pattern used by the dnssec/dataproc checks in this PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review: the SSH terraform variants detect port ranges that span port 22 (from_port <= 22 && to_port >= 22), and the native RDP variant is already range-aware, but the RDP terraform hcl/plan/state variants still matched only the exact to_port == 3389. A rule opening a range that includes 3389 (e.g. 3380-3400) would slip past. Align them with the SSH and native RDP form. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ecks - azure kv-logging hcl: re-assert at least one enabled_log/log block is present (the values migration had dropped this to target_resource_id only) - azure batch private-endpoint hcl: guard private_service_connection before .any() to avoid erroring on absent block, matching sibling patterns - github webhook-ssl hcl: guard configuration before .all(); preserve the original vacuous-pass when the block is absent Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review: the acr-private-endpoints terraform hcl/plan/state variants called .any() on values[private_service_connection] without the != empty guard that the six sibling private-endpoint checks already use. Add the guard so an azurerm_private_endpoint without the block does not error at runtime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…checks Address review: the managed HSM, compute disk access, and Recovery Vault private-endpoint checks called .any() on values[private_service_connection] without the != empty guard used by the sibling private-endpoint checks. Add the guard across all three terraform variants of each so an azurerm_private_endpoint without the block does not error at runtime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…k (hetzner LB) azure diagnostic-settings essential-categories: restore the legacy `log` block fallback alongside `enabled_log` across all 4 categories in the hcl/plan/state variants, matching the KV-logging sibling, so older azurerm provider `log` blocks are still detected. hetzner LB http-redirect: add a `http == empty ||` guard to the terraform hcl/plan/state variants so a missing `http` block passes vacuously (as the old blocks.where form did) instead of erroring on `.all()`-over-null. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…http block Address review: the iteration-10 guard used http == empty || which let an http-protocol service with no http block pass the redirect check. The native hetzner-loadbalancer variant fails such a service (redirect_http not true), so the terraform variants now use http != empty && to match: an http service must have an http block with redirect_http = true. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ant gcp ports guards hetzner: add `http != empty &&` guard to the https-certificate terraform variants so an https service with no http/cert block fails (matching the native variant) instead of erroring on `.any()` over a null http block. gcp: simplify the firewall ports guards from `_['ports'] != empty && _['ports'].length > 0` to `_['ports'] != empty` across the no-overly-permissive-ingress and no-egress-all-ports check families. `x != empty` is already false for null and for [], so the `.length > 0` was redundant; this also makes all four variants of each family consistent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
68529f1 to
1f04c62
Compare
What
terraform provider 13.2.0 added a
values()dict field to theterraform.blockresource (mondoohq/mql#8289). It folds child blocks into the arguments dict as lists-of-maps keyed by block type, recursively — byte-for-byte the same shape Terraform plan (change.after) and state (values) already expose.Until now, a cloud check that runs against Terraform was written three times because the backends exposed different shapes:
terraform.blocks, walked withblocks.where(type == "x")+arguments.foo— its own traversal dialect.change.after.x+_['key'].values.x.With
block.values, the HCL check body becomes identical to its plan/state sibling except for the selector prefix and root accessor. This PR rewrites the HCL bodies accordingly.Change
369
*-terraform-hclvariantmql:bodies across 8 policy files were rewritten to mirror their state sibling: the state body verbatim, with the root selector swapped fromterraform.state.resources.where(type == "X")toterraform.resources("X")(.valuesroot,_['key']for nested map access).content/mondoo-aws-security.mql.yamlcontent/mondoo-azure-security.mql.yamlcontent/mondoo-digitalocean-security.mql.yamlcontent/mondoo-gcp-security.mql.yamlcontent/mondoo-github-security.mql.yamlcontent/mondoo-hetzner-security.mql.yamlcontent/mondoo-m365-security.mql.yamlcontent/mondoo-oci-security.mql.yamlOnly the
mql:bodies of HCL variants changed. Filters, docs, remediation, compliance tags, and all other fields are untouched, and query counts are unchanged in every file.Before / after (EKS CMK check)
Before — HCL's own
blocks.where/argumentsdialect:After — the state sibling's body,
terraform.state.resources.where(type == "...")→terraform.resources("..."):Deliberately left unchanged (41)
These have a plan/state sibling but were intentionally not rewritten because the translation would not be a faithful, mechanical transplant:
secgroup-restricted-*family (aws_security_group+aws_vpc_security_group_ingress_rule), the Azureapp-service-*family (azurerm_linux_web_app+azurerm_windows_web_app),gcp-security-iam-default-service-accounts-disabled,gcp-security-vertex-ai-job-cmek-encryption.aws-security-s3-bucket-versioning-enabled(cross-resource join via a helper variable),gcp-security-bigquery-dataset-not-publicly-accessible(HCL guards three resource types withifblocks; state checks one), and the OCI/Route53/S3-ownership checks whose HCL traversal has no equivalent single-resource state body.Validation
cnspec policy lint ./content/<file>.mql.yaml→ valid policy bundle(s) for all 8 modified files (terraform provider 13.2.0+ installed, which providesblock.values).-terraform-hclmql:bodies differ frommain.Behavior changes beyond the mechanical migration
A few checks were tightened or corrected during the migration so the Terraform variants stay in parity with each other and with the native cloud check:
mondoo-gcp-security-dataproc-cluster-no-default-service-account-*): all Terraform variants now reject the GCP default compute and App Engine service accounts (*-compute@developer.gserviceaccount.com,*@appspot.gserviceaccount.com) in addition to the previousservice_account != emptycheck. This brings the Terraform variants into parity with the nativegcp-dataproc-clustercheck, which already rejected the default compute SA.mondoo-gcp-security-dataproc-cluster-encryption-configured-*): the guardcluster_config == empty ||allows a cluster that declares nocluster_configblock to pass. An absent HCL key isnull, and calling.all()onnullerrors rather than passing vacuously, so the guard is required; a cluster with nocluster_confighas nothing to assert encryption on. Applied consistently across all Terraform variants.mondoo-gcp-security-secretmanager-cmek-encryption-terraform-plan/state): corrected a pre-existing array-access bug.replication,user_managed, andreplicasare nested blocks (arrays in the values/JSON model), so the previous object-style bracket chains were rewritten with array semantics to match the HCL variant, including the_['replicas'] == empty ||guard (the automatic-replication / zero-replica case carries no per-replica CMEK and is handled by the native check).mondoo-aws-security-ecs-efs-volume-transit-encryption-terraform-*): corrected an array-access bug.efs_volume_configurationis a nested block (an array), so per-config assertion now uses_['efs_volume_configuration'].all(_['transit_encryption'] == "ENABLED")instead of a bracket index on the array.🤖 Generated with Claude Code