Fix retention and schema checks for dynamic resource properties

src/CUSTOM_RULES.md

@@ -300,7 +300,7 @@ condition assignment and should be emitted with `make_diag_conditional`.
 | `is_satisfiable`          | `({condition_name: bool}) -> bool`               | Whether the condition assignment is satisfiable; an empty assignment returns `true`.                                                |
 | `unreachable_if_branches` | `(resource_id) -> [{resourceId, path, message}]` | Finds immediate `Fn::If` branches unreachable under surrounding conditions. `__output__<name>` addresses an output.                 |
 | `has_property`            | `(resource_id, property_name) -> bool`           | Whether the resource authors the named property, whatever its value: a `Ref`, `Fn::GetAtt`, or `Fn::ImportValue` counts as present. Use `"BucketName"`, not `"Properties.BucketName"`. |
-| `property_can_be_absent`  | `(resource_id, path) -> bool`                    | Whether the property is missing or can resolve to null/`AWS::NoValue` in a scenario.                                                |
+| `property_can_be_absent`  | `(resource_id, path) -> bool`                    | Whether a top-level property is absent in a reachable deployment scenario. Macro-owned property sets are unknown rather than absent. |
 
 ### Parameters and Template Metadata
 

src/cel-engine/src/rules/best_practices.rs

@@ -638,22 +638,9 @@ fn eval_retention_period_rules(ctx: &EvalContext) -> Vec<Diagnostic> {
             // anchored on the properties object, so report only the first missing
             // one - emitting one per missing property would over-report a single
             // underlying concern.
-            let first_missing = required_props.iter().find(|prop| {
-                let key_present = m
-                    .resources
-                    .get(resource_name.as_str())
-                    .map(|r| r.properties.contains_key(prop.as_str()))
-                    .unwrap_or(false);
-                // The property is only "set" if it resolves to a real value in
-                // every satisfiable scenario. A value supplied through
-                // `Fn::If [cond, X, AWS::NoValue]` is absent in the NoValue
-                // branch, so the retention period can still lapse - the reference
-                // linter flags that, and treating mere key presence as "set" would
-                // miss it.
-                let path = format!("Properties.{}", prop);
-                let scenarios = m.resolve_scenarios_json(resource_name.as_str(), &path);
-                !key_present || scenarios.is_empty() || scenarios.iter().any(|(val, _)| val.is_null())
-            });
+            let first_missing = required_props
+                .iter()
+                .find(|property| !m.property_absence_scenarios(resource_name, property).is_empty());
             if let Some(prop) = first_missing {
                 out.push(make_resource_diagnostic("I3013",
                     &format!("'{}' is a required property (The default retention period will delete the data after a pre-defined time. Set an explicit values to avoid data loss on resource)", prop),

src/cfn-validate/tests/resource_rule_parity.rs

@@ -797,6 +797,185 @@ Resources:
     assert!(rego_findings.iter().any(|finding| finding.contains("'UpdateReplacePolicy'")));
 }
 
+#[test]
+fn dynamic_properties_do_not_imply_missing_retention_period() {
+    let template = r#"
+Parameters:
+  Stage:
+    Type: String
+  RetentionDays:
+    Type: Number
+Conditions:
+  IsProduction: !Equals [!Ref Stage, production]
+  IsNotProduction: !Not [!Condition IsProduction]
+Resources:
+  DynamicPropertiesQueue:
+    Type: AWS::SQS::Queue
+    Properties:
+      Fn::Transform:
+        Name: AWS::Include
+        Parameters:
+          Location: s3://example-bucket/queue-properties.yaml
+  SiblingTransformQueue:
+    Type: AWS::SQS::Queue
+    Properties:
+      QueueName: example-queue
+      Fn::Transform:
+        Name: AWS::Include
+        Parameters:
+          Location: s3://example-bucket/queue-properties.yaml
+  ResourceTransformQueue:
+    Type: AWS::SQS::Queue
+    Fn::Transform:
+      Name: AWS::Include
+      Parameters:
+        Location: s3://example-bucket/queue-resource.yaml
+  NestedTransformQueue:
+    Type: AWS::SQS::Queue
+    Properties:
+      Tags:
+        Fn::Transform:
+          Name: AWS::Include
+          Parameters:
+            Location: s3://example-bucket/queue-tags.yaml
+  DynamicRetentionQueue:
+    Type: AWS::SQS::Queue
+    Properties:
+      MessageRetentionPeriod: !Ref RetentionDays
+  ConditionalSetQueue:
+    Type: AWS::SQS::Queue
+    Properties: !If
+      - IsProduction
+      - MessageRetentionPeriod: 1209600
+      - MessageRetentionPeriod: 60
+  ConditionalMacroOrSetQueue:
+    Type: AWS::SQS::Queue
+    Properties: !If
+      - IsProduction
+      - Fn::Transform:
+          Name: AWS::Include
+          Parameters:
+            Location: s3://example-bucket/production-queue-properties.yaml
+      - MessageRetentionPeriod: 60
+  ConditionalMacroOnlyQueue:
+    Type: AWS::SQS::Queue
+    Properties: !If
+      - IsProduction
+      - Fn::Transform:
+          Name: AWS::Include
+          Parameters:
+            Location: s3://example-bucket/production-queue-properties.yaml
+      - Fn::Transform:
+          Name: AWS::Include
+          Parameters:
+            Location: s3://example-bucket/development-queue-properties.yaml
+  ConditionalRetentionQueue:
+    Type: AWS::SQS::Queue
+    Properties:
+      MessageRetentionPeriod: !If [IsProduction, 1209600, !Ref AWS::NoValue]
+  ResourceConditionAllowsAbsenceQueue:
+    Type: AWS::SQS::Queue
+    Condition: IsProduction
+    Properties:
+      MessageRetentionPeriod: !If [IsProduction, !Ref AWS::NoValue, 60]
+  ResourceConditionExcludesAbsenceQueue:
+    Type: AWS::SQS::Queue
+    Condition: IsProduction
+    Properties:
+      MessageRetentionPeriod: !If [IsProduction, 60, !Ref AWS::NoValue]
+  NegatedResourceConditionExcludesAbsenceQueue:
+    Type: AWS::SQS::Queue
+    Condition: IsNotProduction
+    Properties:
+      MessageRetentionPeriod: !If [IsProduction, !Ref AWS::NoValue, 60]
+  CorrelatedConditionsExcludeAbsenceQueue:
+    Type: AWS::SQS::Queue
+    Properties:
+      MessageRetentionPeriod: !If
+        - IsProduction
+        - 1209600
+        - !If [IsNotProduction, 60, !Ref AWS::NoValue]
+  ConditionalTransformQueue:
+    Type: AWS::SQS::Queue
+    Properties: !If
+      - IsProduction
+      - Fn::Transform:
+          Name: AWS::Include
+          Parameters:
+            Location: s3://example-bucket/queue-properties.yaml
+      - VisibilityTimeout: 900
+  ConcreteQueue:
+    Type: AWS::SQS::Queue
+    Properties:
+      QueueName: concrete-queue
+"#;
+    let (rego, cel) = engines();
+    let rego_findings = selected_findings(&rego, template, &["I3013"]);
+    let cel_findings = selected_findings(&cel, template, &["I3013"]);
+
+    assert_eq!(rego_findings, cel_findings);
+    assert_eq!(
+        rego_findings.len(),
+        5,
+        "only known reachable missing retention periods are reported: {rego_findings:?}"
+    );
+    for resource in [
+        "ConcreteQueue",
+        "ConditionalRetentionQueue",
+        "ConditionalTransformQueue",
+        "NestedTransformQueue",
+        "ResourceConditionAllowsAbsenceQueue",
+    ] {
+        assert!(rego_findings.iter().any(|finding| finding.contains(&format!("|{resource}|Properties|"))));
+    }
+    for resource in [
+        "ConditionalMacroOnlyQueue",
+        "ConditionalMacroOrSetQueue",
+        "ConditionalSetQueue",
+        "CorrelatedConditionsExcludeAbsenceQueue",
+        "DynamicPropertiesQueue",
+        "DynamicRetentionQueue",
+        "NegatedResourceConditionExcludesAbsenceQueue",
+        "ResourceConditionExcludesAbsenceQueue",
+        "ResourceTransformQueue",
+        "SiblingTransformQueue",
+    ] {
+        assert!(rego_findings.iter().all(|finding| !finding.contains(&format!("|{resource}|"))));
+    }
+}
+
+#[test]
+fn whole_properties_no_value_is_a_schema_type_error() {
+    let template = r#"
+Resources:
+  Queue:
+    Type: AWS::SQS::Queue
+    Properties: !Ref AWS::NoValue
+  MacroOwnedQueue:
+    Type: AWS::SQS::Queue
+    Properties: !Ref AWS::NoValue
+    Fn::Transform:
+      Name: AWS::Include
+      Parameters:
+        Location: s3://example-bucket/queue-resource.yaml
+"#;
+    let (rego, cel) = engines();
+    let rego_findings = selected_findings(&rego, template, &["F3012", "I3013"]);
+    let cel_findings = selected_findings(&cel, template, &["F3012", "I3013"]);
+
+    assert_eq!(rego_findings, cel_findings);
+    assert_eq!(rego_findings.len(), 1, "the non-object Properties value must produce one finding: {rego_findings:?}");
+    assert!(
+        rego_findings[0].contains("F3012|Fatal|Queue|Properties|{'Ref': 'AWS::NoValue'} is not of type 'object'"),
+        "the whole Properties value must be rejected without a missing-retention cascade: {rego_findings:?}"
+    );
+
+    let rego_anchors = selected_finding_anchors(&rego, template, &["F3012"]);
+    let cel_anchors = selected_finding_anchors(&cel, template, &["F3012"]);
+    assert_eq!(rego_anchors, cel_anchors);
+    assert_eq!(rego_anchors, ["Queue|Properties|5:5"]);
+}
+
 #[test]
 fn omitted_dynamodb_billing_mode_requires_provisioned_throughput() {
     let template = r#"

src/rego-engine/src/builtins.rs

@@ -1784,15 +1784,11 @@ fn register_property_can_be_absent(rego: &mut regorus::Engine) {
             };
             let rid = params[0].as_string()?;
             let path = params[1].as_string()?;
-            let prop = path.strip_prefix("Properties.").unwrap_or(path.as_ref());
-            let key_present =
-                model.resources.get(rid.as_ref()).map(|r| r.properties.contains_key(prop)).unwrap_or(false);
-            if !key_present {
+            let property = path.strip_prefix("Properties.").unwrap_or(path.as_ref());
+            if !model.resources.contains_key(rid.as_ref()) {
                 return Ok(Value::from(true));
             }
-            let scenarios = model.resolve_scenarios_json_shared(rid.as_ref(), path.as_ref());
-            let absent = scenarios.is_empty() || scenarios.iter().any(|(v, _)| v.is_null());
-            Ok(Value::from(absent))
+            Ok(Value::from(!model.property_absence_scenarios(rid.as_ref(), property).is_empty()))
         }),
     );
 }

src/rego-engine/src/engine.rs

@@ -990,6 +990,7 @@ Resources:
     Type: AWS::SQS::Queue
     Properties:
       QueueName: my-queue
+      MessageRetentionPeriod: !Ref Port
 Transform: AWS::Serverless-2016-10-31
 "#;
 
@@ -1230,6 +1231,25 @@ violation contains v if {
         assert_eq!(diags.len(), 1, "BucketName is static");
     }
 
+    #[test]
+    fn builtin_property_can_be_absent_uses_known_absence() {
+        let diags = eval_builtin_policy(
+            r#"
+package builtin_test
+import rego.v1
+violation contains v if {
+    property_can_be_absent("MyQueue", "Properties.RedrivePolicy")
+    not property_can_be_absent("MyBucket", "Properties.BucketName")
+    not property_can_be_absent("MyQueue", "Properties.MessageRetentionPeriod")
+    property_can_be_absent("MissingQueue", "Properties.MessageRetentionPeriod")
+    v := {"rule_id": "B_PROPERTY_ABSENCE", "severity": "error", "message": "ok", "resource_id": "MyQueue"}
+}
+"#,
+            "B_PROPERTY_ABSENCE",
+        );
+        assert_eq!(diags.len(), 1, "only known property absence should satisfy the builtin");
+    }
+
     #[test]
     fn builtin_resources_of_type() {
         let diags = eval_builtin_policy(

src/resources/expected/validation_reports5.json

@@ -28891,7 +28891,7 @@
         "fatal": 0,
         "errors": 0,
         "warnings": 0,
-        "informational": 55,
+        "informational": 54,
         "debug": 0
       },
       "suppressed": 0,
@@ -29187,25 +29187,6 @@
         "ruleDescription": "Check stateful resources have a set UpdateReplacePolicy/DeletionPolicy",
         "phase": "LINT"
       },
-      {
-        "ruleId": "I3013",
-        "severity": "INFO",
-        "message": "'MessageRetentionPeriod' is a required property (The default retention period will delete the data after a pre-defined time. Set an explicit values to avoid data loss on resource)",
-        "source": "CFN_LINT",
-        "entity": {
-          "logicalId": "DynamicQueue",
-          "entityType": "Resource",
-          "resourceType": "AWS::SQS::Queue"
-        },
-        "propertyPath": "Properties",
-        "category": "Best Practice",
-        "startLine": 17,
-        "startColumn": 5,
-        "endLine": 17,
-        "endColumn": 15,
-        "ruleDescription": "Check resources with auto expiring content have explicit retention period",
-        "phase": "LINT"
-      },
       {
         "ruleId": "I3013",
         "severity": "INFO",

src/resources/expected/validation_reports7.json

@@ -14985,7 +14985,7 @@
     "metadata": {
       "resourcesScanned": 5,
       "counts": {
-        "fatal": 4,
+        "fatal": 6,
         "errors": 3,
         "warnings": 0,
         "informational": 2,
@@ -15086,6 +15086,57 @@
         "ruleDescription": "Required property missing",
         "phase": "SCHEMA"
       },
+      {
+        "ruleId": "F3012",
+        "severity": "FATAL",
+        "message": "{'Ref': 'AWS::NoValue'} is not of type 'object'",
+        "source": "SCHEMA",
+        "entity": {
+          "logicalId": "IamRole2",
+          "entityType": "Resource",
+          "resourceType": "AWS::IAM::Role"
+        },
+        "propertyPath": "Properties",
+        "category": "Schema",
+        "startLine": 26,
+        "startColumn": 5,
+        "endLine": 26,
+        "endColumn": 15,
+        "documentationUrl": "https://github.com/aws-cloudformation/aws-cloudformation-resource-providers-iam.git",
+        "ruleDescription": "Property type mismatch",
+        "phase": "SCHEMA",
+        "context": {
+          "actualValue": {
+            "Ref": "AWS::NoValue"
+          },
+          "expectedConstraint": "object"
+        }
+      },
+      {
+        "ruleId": "F3012",
+        "severity": "FATAL",
+        "message": "{'Ref': 'AWS::NoValue'} is not of type 'object'",
+        "source": "SCHEMA",
+        "entity": {
+          "logicalId": "CloudFront1",
+          "entityType": "Resource",
+          "resourceType": "AWS::CloudFront::Distribution"
+        },
+        "propertyPath": "Properties",
+        "category": "Schema",
+        "startLine": 39,
+        "startColumn": 5,
+        "endLine": 39,
+        "endColumn": 15,
+        "ruleDescription": "Property type mismatch",
+        "phase": "SCHEMA",
+        "context": {
+          "actualValue": {
+            "Ref": "AWS::NoValue"
+          },
+          "expectedConstraint": "object"
+        }
+      },
       {
         "ruleId": "E3024",
         "severity": "ERROR",