✨ Add missing AWS runtime and CloudFormation check variants#2734
Conversation
|
Addressed the review feedback in 2aa6c25:
|
|
/review |
2aa6c25 to
796189e
Compare
796189e to
a271bfd
Compare
This comment has been minimized.
This comment has been minimized.
a271bfd to
af3f4cf
Compare
af3f4cf to
2f7cee1
Compare
2f7cee1 to
61074d3
Compare
There was a problem hiding this comment.
Well-structured addition of CloudFormation check variants and AWS runtime checks with minor correctness concerns
Additional findings (file/line not in diff):
- 🔵
content/mondoo-aws-security.mql.yaml:48549— The CloudFormation DataSync task check only verifiesCloudWatchLogGroupArn != emptybut the check title says "logging encryption". Unlike the Terraform variant which can cross-reference the log group's KMS key, this CloudFormation variant doesn't verify the referenced log group actually uses a CMK. Consider adding a comment noting this limitation, or cross-checkingAWS::Logs::LogGroupresources forKmsKeyId.
0616914 to
f3eca24
Compare
There was a problem hiding this comment.
Well-structured addition of CloudFormation check variants and AWS runtime checks with minor issues
Additional findings (file/line not in diff):
- 🔵
content/mondoo-aws-security.mql.yaml:67700— The Route 53 Resolver runtime check uses a string-join trick ("," + ... + ",") to avoid substring false positives. This is clever but fragile — if VPC IDs ever contain commas or if the API returns unexpected whitespace, it could break. Consider adding a brief inline comment explaining this assumption (that VPC IDs are alwaysvpc-xxxxformat with no commas/spaces). - 🔵
content/mondoo-aws-security.mql.yaml:48977— The EKS nodegroup encrypted volumes CloudFormation check flags allAWS::EC2::LaunchTemplateresources, not just those referenced by EKS node groups. The comment explains this intentional design, which is good. Just noting that this is a broader scope than the check title implies — users may get unexpected findings for non-EKS launch templates.
f3eca24 to
61d521e
Compare
f632f96 to
fa63f36
Compare
58d5925 to
9cc03e2
Compare
Fill in missing variants across the AWS security policy so checks run against the live AWS runtime and CloudFormation templates, not just Terraform assets. New variants: - route53-resolver-query-logging-enabled: aws runtime + cloudformation - datasync-task-cloudwatch-logging-encryption: cloudformation - ecr-no-public-access: cloudformation - eks-cluster-private-controlplane: cloudformation - cloudfront-sni-only: cloudformation - rds-cluster-parameter-group-ssl: cloudformation - eks-nodegroup-encrypted-volumes: cloudformation Also: - Remove a stale comment claiming the provider doesn't expose Route 53 Resolver query log configs (it does, via aws.route53.resolver). - Fix two CloudFormation remediation property-name bugs: AWS::EKS::Cluster uses ResourcesVpcConfig (not VpcConfig); AWS::CloudFront::Distribution ViewerCertificate uses SslSupportMethod (not SSLSupportMethod). - Reword "cnquery" -> "mql" in all No-runtime-variant comments and drop a duplicate Kinesis Video comment. Kinesis Video Streams and DataSync have no aws runtime variant because the mql aws provider exposes neither resource; both are documented in comments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- ecr-no-public-access CloudFormation variant: also pass when RepositoryPolicyText has no Statement, so .none() never runs against a missing/null Statement key. - eks-nodegroup-encrypted-volumes CloudFormation variant: document that it inspects every launch template (matching the existing Terraform variants), not only EKS-referenced ones. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrite the Route 53 resolver query-logging runtime check to use a proper list-membership test (`id.in(loggedVpcIds)`) instead of a comma-delimited string with `contains`, eliminating the false-match risk when an association resourceId is empty. Allow the CloudFront SNI CloudFormation check to pass distributions that use the default CloudFront certificate, where `SslSupportMethod` is absent because SNI does not apply. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
9cc03e2 to
5ac05ff
Compare
| @@ -52475,7 +52544,7 @@ queries: | |||
| DistributionConfig: | |||
There was a problem hiding this comment.
🟡 warning — The remediation example changed SSLSupportMethod to SslSupportMethod, and the new check at line 52583 uses SslSupportMethod. The actual CloudFormation property name is SslSupportMethod, so this fix is correct — just flagging that this is a breaking change to the remediation docs for anyone who copy-pasted the old example.
| properties['RepositoryPolicyText'] == empty || | ||
| properties['RepositoryPolicyText']['Statement'] == empty || | ||
| properties['RepositoryPolicyText']['Statement'].none( |
There was a problem hiding this comment.
🔵 suggestion — The ECR public-access check passes when RepositoryPolicyText is empty OR Statement is empty. This means a repository with no policy at all is considered compliant, which is the right default. However, the comment notes that string-form policies "pass through unevaluated" — this is a silent false-negative. Consider adding a guard that fails if RepositoryPolicyText is a non-empty value but Statement is absent (i.e., it's likely a string), so users get a warning rather than a silent pass.
| mql: | | ||
| cloudformation.template.resources.where(type == "AWS::EC2::LaunchTemplate").all( | ||
| properties['LaunchTemplateData']['BlockDeviceMappings'] != empty && | ||
| properties['LaunchTemplateData']['BlockDeviceMappings'].all( | ||
| _['Ebs'] != empty && | ||
| _['Ebs']['Encrypted'] == true | ||
| ) |
There was a problem hiding this comment.
🔵 suggestion — The EKS nodegroup encrypted volumes CloudFormation check requires BlockDeviceMappings != empty, meaning a launch template that omits BlockDeviceMappings entirely will fail. Some launch templates legitimately rely on AMI-default block device mappings. If the intent is to only flag templates that have explicit unencrypted mappings, consider making BlockDeviceMappings != empty part of a filter rather than a failure condition, or document that omission is intentionally treated as non-compliant.
| - uid: mondoo-aws-security-route53-resolver-query-logging-enabled-aws | ||
| tags: | ||
| mondoo.com/filter-title: AWS Account | ||
| mondoo.com/filter-icon: aws |
There was a problem hiding this comment.
🔵 suggestion — The new Route 53 runtime check aws.vpcs.all(id.in(loggedVpcIds)) requires every VPC to have an active query log association. This is a strict posture — default VPCs or utility VPCs that carry no workloads will cause failures. This may be intentional, but worth documenting or noting in the check description so users understand the scope.
Summary
Fills in missing variants across
mondoo-aws-security.mql.yamlso checks run against the live AWS runtime and CloudFormation templates, not only Terraform assets. Started from the three checks flagged manually, then swept the policy withinspect-policies.pyfor other genuinely-addable variants.New variants
route53-resolver-query-logging-enabledawsruntime +cloudformationdatasync-task-cloudwatch-logging-encryptioncloudformationecr-no-public-accesscloudformationeks-cluster-private-controlplanecloudformationcloudfront-sni-onlycloudformationrds-cluster-parameter-group-sslcloudformationeks-nodegroup-encrypted-volumescloudformationEach new
cloudformationvariant lines up with an existingid: cloudformationremediation entry;route53-resolveranddatasyncalso gained the matching CloudFormation remediation block.Fixes pulled in along the way
aws.route53.resolver.queryLogConfigAssociations), so the runtime variant is now real.AWS::EKS::Cluster→ResourcesVpcConfig(wasVpcConfig)AWS::CloudFront::DistributionViewerCertificate→SslSupportMethod(wasSSLSupportMethod)cnquery→mqlin the# No runtime variant:comments, and removed a duplicate Kinesis Video comment.Intentionally not added
These were flagged by the script but are genuinely not addable, and stay documented in-line:
kinesis-video-stream-cmk-encryption(aws) — themqlaws.kinesisresource exposes only Data Streams/Firehose, not Kinesis Video.datasync-task-...(aws) — no DataSync resource in the provider.ssm-document-not-public(CFN) — document sharing isn't a CloudFormation property.ec2-snapshot-not-public(CFN) — no CloudFormation snapshot-sharing resource.ec2-launch-template-no-secrets(CFN) —UserDatais base64-encoded in CloudFormation, so the secret-detection regexes can't match.cloudfront-trust-store-not-empty(CFN) — no corresponding CloudFormation resource type.Many other "MISSING aws" rows from the script are false positives: the runtime variant exists under a resource-specific suffix (
-distribution,-cluster) the script doesn't recognize.Test plan
cnspec policy lint content/mondoo-aws-security.mql.yaml→ valid policy bundle (only pre-existingquery-deprecated-symbolwarnings remain).🤖 Generated with Claude Code