Improve scenario-aware validation and resolved-value handling

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

@@ -9,9 +9,9 @@ use std::sync::{Arc, LazyLock};
 use template_model::SemanticModel;
 use template_model::coercion::{coerce_port_to_string, coerce_to_integer, coerce_to_string, scalar_eq};
 use template_model::consts::{
-    EDGE_KIND_GET_ATT, EDGE_KIND_REF, EDGE_KIND_SELECT, FIELD_ATTR, FIELD_KIND, FIELD_MAPPINGS, FIELD_OUTGOING_REFS,
-    FIELD_PROPERTIES, FIELD_RESOURCE_TYPE, FIELD_RESOURCES, FIELD_SOURCE_PATH, FIELD_TARGET, FN_IF, FN_REF,
-    KEY_PROPERTIES, PARAM_TYPE_STRING, TRANSFORM_SERVERLESS,
+    EDGE_KIND_GET_ATT, EDGE_KIND_REF, EDGE_KIND_SELECT, FIELD_ATTR, FIELD_CREATION_POLICY, FIELD_KIND, FIELD_MAPPINGS,
+    FIELD_OUTGOING_REFS, FIELD_PROPERTIES, FIELD_RESOURCE_TYPE, FIELD_RESOURCES, FIELD_SOURCE_PATH, FIELD_TARGET,
+    FN_IF, FN_REF, KEY_CREATION_POLICY, KEY_PROPERTIES, KEY_UPDATE_POLICY, PARAM_TYPE_STRING, TRANSFORM_SERVERLESS,
 };
 use template_model::fargate::{CPU_UNIT_LABELS, cpu_is_offered};
 use template_model::iam_policy::validate_identity_policy_scenarios;
@@ -251,12 +251,23 @@ fn push_identity_policy_findings(
     document_path: &str,
 ) {
     for finding in validate_identity_policy_scenarios(model, resource_id, document_path) {
-        let path = if finding.path.is_empty() {
+        let effective_path = if finding.path.is_empty() {
             document_path.to_string()
         } else {
             format!("{}.{}", document_path, finding.path)
         };
-        out.push(make_resource_diagnostic("E3510", &finding.message, model, resource_id, &path, None));
+        // Use the authored source_path (branch-qualified) for span resolution
+        // when available, falling back to the effective path.
+        let source_path = if finding.source_path.is_empty() { effective_path.clone() } else { finding.source_path };
+        out.push(make_resource_diagnostic_at_source(
+            "E3510",
+            &finding.message,
+            model,
+            resource_id,
+            &effective_path,
+            &source_path,
+            None,
+        ));
     }
 }
 
@@ -1255,22 +1266,57 @@ pub fn eval_extra_resources(ctx: &EvalContext) -> Vec<Diagnostic> {
     }
 
     let creation_policy_types = [
+        "AWS::AppStream::Fleet",
         "AWS::AutoScaling::AutoScalingGroup",
-        "AWS::EC2::Instance",
         "AWS::CloudFormation::WaitCondition",
+        "AWS::EC2::Instance",
+    ];
+    let creation_policy_fix =
+        format!("Remove CreationPolicy or change resource type to one of: {}", creation_policy_types.join(", "));
+    let update_policy_types = [
         "AWS::AppStream::Fleet",
+        "AWS::AutoScaling::AutoScalingGroup",
+        "AWS::ElastiCache::ReplicationGroup",
+        "AWS::Elasticsearch::Domain",
+        "AWS::Lambda::Alias",
+        "AWS::OpenSearchService::Domain",
     ];
+    let update_policy_fix =
+        format!("Remove UpdatePolicy or change resource type to one of: {}", update_policy_types.join(", "));
     if let Some(resources) = input.get(FIELD_RESOURCES).and_then(|r| r.as_object()) {
         for (name, res) in resources {
-            if res.get("creation_policy").map(|v| !v.is_null()).unwrap_or(false) {
+            if res.get(FIELD_CREATION_POLICY).map(|v| !v.is_null()).unwrap_or(false) {
                 let rtype = res.get(FIELD_RESOURCE_TYPE).and_then(|t| t.as_str()).unwrap_or("");
                 if !creation_policy_types.contains(&rtype) {
                     out.push(make_resource_diagnostic(
                         "E3055",
-                        &format!("CreationPolicy is not valid on resource type '{}'", rtype),
+                        &format!("CreationPolicy is not supported on resource type '{}'", rtype),
+                        m,
+                        name,
+                        KEY_CREATION_POLICY,
+                        Some(&creation_policy_fix),
+                    ));
+                }
+            }
+            let update_policy_status = m.lifecycle_attribute_status(name, KEY_UPDATE_POLICY);
+            if update_policy_status.may_be_present {
+                let rtype = res.get(FIELD_RESOURCE_TYPE).and_then(|t| t.as_str()).unwrap_or("");
+                if !update_policy_types.contains(&rtype) {
+                    out.push(make_resource_diagnostic(
+                        "E3016",
+                        &format!("UpdatePolicy is not supported on resource type '{}'", rtype),
+                        m,
+                        name,
+                        KEY_UPDATE_POLICY,
+                        Some(&update_policy_fix),
+                    ));
+                } else if let Some(invalid_value) = update_policy_status.invalid_value.as_deref() {
+                    out.push(make_resource_diagnostic(
+                        "E3016",
+                        &format!("{} is not of type 'object'", invalid_value),
                         m,
                         name,
-                        "",
+                        KEY_UPDATE_POLICY,
                         None,
                     ));
                 }

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

@@ -289,12 +289,12 @@ fn eval_structure(ctx: &EvalContext) -> Vec<Diagnostic> {
     }
 
     if let Some(desc) = input.get("template").and_then(|t| t.get("description")).and_then(|v| v.as_str())
-        && desc.len() > 921
-        && desc.len() <= 1024
+        && desc.chars().count() > 921
+        && desc.chars().count() <= 1024
     {
         out.push(make_resource_diagnostic(
             "I1003",
-            &format!("Description length {} is approaching maximum of 1024", desc.len()),
+            &format!("Description length {} is approaching maximum of 1024", desc.chars().count()),
             m,
             "",
             "",
@@ -316,11 +316,11 @@ fn eval_structure(ctx: &EvalContext) -> Vec<Diagnostic> {
     }
 
     if let Some(desc) = input.get("template").and_then(|t| t.get("description")).and_then(|v| v.as_str())
-        && desc.len() > 1024
+        && desc.chars().count() > 1024
     {
         out.push(make_resource_diagnostic(
             "F0011",
-            &format!("Description length {} exceeds maximum 1024", desc.len()),
+            &format!("Description length {} exceeds maximum 1024", desc.chars().count()),
             m,
             "",
             "",
@@ -696,21 +696,41 @@ fn eval_structure(ctx: &EvalContext) -> Vec<Diagnostic> {
     for (name, param) in &m.parameters {
         if let (Some(default), Some(allowed)) = (&param.default, &param.allowed_values)
             && !allowed.is_empty()
-            && !allowed.iter().any(|a| a == default)
         {
-            out.push(make_resource_diagnostic(
-                "F2012",
-                &format!(
-                    "Parameter '{}' Default '{}' is not in AllowedValues {}",
-                    name,
-                    default,
-                    render_str_list(allowed)
-                ),
-                m,
-                "",
-                &format!("{}/{}/Default", SECTION_PARAMETERS, name),
-                None,
-            ));
+            let is_cdl = param.param_type == PARAM_TYPE_COMMA_DELIMITED_LIST || param.param_type.starts_with("List<");
+            if is_cdl {
+                for element in default.split(',').map(|s| s.trim()) {
+                    if !allowed.iter().any(|a| a == element) {
+                        out.push(make_resource_diagnostic(
+                            "F2012",
+                            &format!(
+                                "Parameter '{}' Default '{}' is not in AllowedValues {}",
+                                name,
+                                element,
+                                render_str_list(allowed)
+                            ),
+                            m,
+                            "",
+                            &format!("{}/{}/Default", SECTION_PARAMETERS, name),
+                            None,
+                        ));
+                    }
+                }
+            } else if !allowed.iter().any(|a| a == default) {
+                out.push(make_resource_diagnostic(
+                    "F2012",
+                    &format!(
+                        "Parameter '{}' Default '{}' is not in AllowedValues {}",
+                        name,
+                        default,
+                        render_str_list(allowed)
+                    ),
+                    m,
+                    "",
+                    &format!("{}/{}/Default", SECTION_PARAMETERS, name),
+                    None,
+                ));
+            }
         }
     }
 
@@ -919,23 +939,23 @@ fn eval_structure(ctx: &EvalContext) -> Vec<Diagnostic> {
         }
         // MinLength / MaxLength
         if let Some(min) = info.min_length
-            && (def.len() as u64) < min
+            && (def.chars().count() as u64) < min
         {
             out.push(make_resource_diagnostic(
                 "F2015",
-                &format!("Parameter '{}' Default length {} is less than MinLength {}", pname, def.len(), min),
+                &format!("Parameter '{}' Default length {} is less than MinLength {}", pname, def.chars().count(), min),
                 m,
                 "",
                 &path_str,
                 None,
             ));
         }
         if let Some(max) = info.max_length
-            && (def.len() as u64) > max
+            && (def.chars().count() as u64) > max
         {
             out.push(make_resource_diagnostic(
                 "F2015",
-                &format!("Parameter '{}' Default length {} exceeds MaxLength {}", pname, def.len(), max),
+                &format!("Parameter '{}' Default length {} exceeds MaxLength {}", pname, def.chars().count(), max),
                 m,
                 "",
                 &path_str,

src/cfn-validate/tests/cross_engine.rs

@@ -480,45 +480,75 @@ fn iam_action_resource_findings_target_authored_fields_in_both_engines() {
     }
 }
 
+const GOOD_FIXTURES_WITH_EXPECTED_ERRORS: &[&str] = &[
+    "core/conditions.yaml",
+    "core/config_cfn_lint.json",
+    "core/config_cfn_lint.yaml",
+    "core/config_only_i1002.yaml",
+    "core/config_only_i1003.yaml",
+    "core/config_parameters.yaml",
+    "custom/is-defined.yaml",
+    "custom/numeric-inequalities-large.yaml",
+    "custom/numeric-inequalities-small.yaml",
+    "decode/parsing.json",
+    "functions/relationship_conditions.yaml",
+    "functions/sub.yaml",
+    "functions/sub_needed.yaml",
+    "functions/sub_needed_custom_excludes.yaml",
+    "functions_findinmap_enhanced.yaml",
+    "mappings/name.yaml",
+    "mappings/used.yaml",
+    "parameters/default.yaml",
+    "parameters/not_used_parameters.yaml",
+    "parameters/used_transforms.yaml",
+    "properties_ec2_vpc.yaml",
+    "resources/cloudformation/stack_nested.yaml",
+    "resources/dynamodb/attributes_transform.yaml",
+    "resources/elasticache/cache_cluster_failover.yaml",
+    "resources/iam/policy.yaml",
+    "resources/name.yaml",
+    "resources/properties/az_cdk.yaml",
+    "resources/properties/exclusive.yaml",
+    "resources/properties/list_duplicates.yaml",
+    "some_logs_stream_lambda.yaml",
+    "transform_serverless_globals.yaml",
+    "transform_serverless_ignore_globals.yaml",
+];
+
+/// Fixtures in the exception list have exact Fatal/Error diagnostics protected
+/// by golden tests; this complementary guard covers templates expected clean.
 #[test]
-fn good_templates_produce_no_fatal_or_error_diagnostics() {
-    let new_rule_ids: std::collections::HashSet<&str> = [
-        "E1002", "E1005", "E1015", "E1016", "E1027", "F1030", "F1031", "F1032", "E1033", "E1051", "E1052", "E3011",
-        "E3023", "E3026", "E3027", "E3029", "E3062", "E3617", "E3620", "E3621", "E3647", "E3672", "E3694", "E3640",
-        "E3642", "E3643", "E3644", "E3652", "E3653", "I2003", "W3002", "W3037", "W3660", "W3664", "W3671", "W3688",
-        "W3689", "W3693", "W3694", "W3698",
-    ]
-    .into_iter()
-    .collect();
-
+fn good_templates_without_expected_errors_are_clean() {
     let sv = SchemaValidator::default();
     let root = common::templates_dir().join("good");
     let mut failures = Vec::new();
     for (engine_name, engine) in [("cel", &*CEL as &dyn ValidationEngine), ("rego", &*REGO as &dyn ValidationEngine)] {
         for entry in walkdir(&root) {
             let bytes = std::fs::read(&entry).unwrap();
+            let name = entry.strip_prefix(&root).unwrap_or(&entry);
+            let relative_name = name.to_string_lossy().replace('\\', "/");
+            if GOOD_FIXTURES_WITH_EXPECTED_ERRORS.contains(&relative_name.as_str()) {
+                continue;
+            }
             let report = match validate_bytes(engine, &sv, &bytes, Default::default()) {
                 Ok(r) => r,
-                Err(_) => continue,
+                Err(e) => {
+                    failures.push(format!("[{engine_name}] {}: validation error: {e}", name.display()));
+                    continue;
+                }
             };
             let bad: Vec<_> = report
                 .diagnostics
                 .iter()
-                .filter(|d| new_rule_ids.contains(d.rule_id.as_str()))
                 .filter(|d| matches!(d.severity, Severity::Fatal | Severity::Error))
                 .map(|d| format!("  {} {}: {}", d.rule_id, d.severity, d.message))
                 .collect();
             if !bad.is_empty() {
-                let name = entry.strip_prefix(&root).unwrap_or(&entry);
                 failures.push(format!("[{engine_name}] {}:\n{}", name.display(), bad.join("\n")));
             }
         }
     }
-    assert!(
-        failures.is_empty(),
-        "Good templates produced Fatal/Error diagnostics from new rules:\n{}",
-        failures.join("\n\n")
-    );
+    assert!(failures.is_empty(), "Good templates produced Fatal/Error diagnostics:\n{}", failures.join("\n\n"));
 }
 
 /// Every `good/sam` template must be clean of Fatal/Error diagnostics on both

src/cfn-validate/tests/golden_tests.rs

@@ -136,7 +136,7 @@ fn cel_standard_matches_golden() {
     check_standard("cel", &engine);
 }
 
-const EXPECTED_RULES_EVALUATED: u64 = 301;
+const EXPECTED_RULES_EVALUATED: u64 = 302;
 
 #[test]
 fn rules_evaluated_is_full_rule_count() {