From e3f743b5fb82bf54731de64ae099a78e2449d48a Mon Sep 17 00:00:00 2001 From: Satyaki Ghosh Date: Wed, 21 Oct 2026 12:14:26 -0400 Subject: [PATCH] Correct EventBridge pseudo and numeric pattern handling --- .../snapshots/rule_categorization_audit.md | 13 +- src/cel-engine/tests/conformance.rs | 2 +- .../tests/resource_rule_parity.rs | 32 ++- src/rego-engine/tests/integration.rs | 2 +- .../expected/validation_reports1.json | 224 +++++++++++++++++- .../bad/E9014_invalid_event_patterns.yaml | 33 +++ .../good/E9014_valid_event_patterns.yaml | 6 + src/template-model/src/event_pattern.rs | 143 +++++++++-- src/template-model/src/resolver.rs | 55 ++++- 9 files changed, 465 insertions(+), 45 deletions(-) diff --git a/scripts/snapshots/rule_categorization_audit.md b/scripts/snapshots/rule_categorization_audit.md index faa86dc1..f5f715e6 100644 --- a/scripts/snapshots/rule_categorization_audit.md +++ b/scripts/snapshots/rule_categorization_audit.md @@ -5,13 +5,13 @@ severity model documented in `product.md`. ## Summary -- Total rules: **317** -- By severity: Fatal=69, Error=153, Warn=72, Info=23 -- By true origin: CfnLint=218, Engine=29, Engine(collision)=1, Schema=69 -- By category: BestPractice=59, Deprecation=8, Intrinsic=52, Parameter=8, Reference=2, Resource=92, Schema=25, Security=14, Structure=57 +- Total rules: **318** +- By severity: Fatal=69, Error=154, Warn=72, Info=23 +- By true origin: CfnLint=218, Engine=30, Engine(collision)=1, Schema=69 +- By category: BestPractice=59, Deprecation=8, Intrinsic=52, Parameter=8, Reference=2, Resource=93, Schema=25, Security=14, Structure=57 - cfn-lint reference: 329 rule IDs loaded - E→F promoted rules: 51 -- Engine-extra rules: 60 +- Engine-extra rules: 61 ## 1. Origin correctness @@ -301,7 +301,7 @@ with `cargo test -p cfn-validate --test engine_parity` for ground truth. **CEL only (4):** `E3023`, `E3024`, `E3510`, `W2530` -_Scanned 257 CEL sites (197 IDs), 282 Rego sites (193 IDs)._ +_Scanned 258 CEL sites (198 IDs), 283 Rego sites (194 IDs)._ ## Appendix: full rule inventory @@ -458,6 +458,7 @@ _Scanned 257 CEL sites (197 IDs), 282 Rego sites (193 IDs)._ | `E9003` | Error | Intrinsic | CfnLint | CfnLint | GetAtt return type may not match usage context | | `E9004` | Error | Intrinsic | Schema ⚠ | CfnLint | GetAtt attribute must exist on target resource type | | `E9006` | Error | Schema | CfnLint | CfnLint | Property value not valid for conditional extension enum | +| `E9014` | Error | Resource | Engine | Engine | EventBridge rule and archive EventPattern must use valid matchers | | `E9101` | Error | Intrinsic | Schema ⚠ | Engine | Invalid nesting of intrinsic functions | | `E9106` | Error | Structure | Schema ⚠ | Engine | Circular dependency in condition definitions | | `F0000` | Fatal | Structure | Schema | Schema | Duplicate key in template | diff --git a/src/cel-engine/tests/conformance.rs b/src/cel-engine/tests/conformance.rs index 202b7f01..a67cb645 100644 --- a/src/cel-engine/tests/conformance.rs +++ b/src/cel-engine/tests/conformance.rs @@ -108,7 +108,7 @@ Resources: fn invalid_event_patterns_trigger_e9014_once_per_rejected_matcher() { let ids = validate_file("bad/E9014_invalid_event_patterns.yaml"); let event_pattern_findings = ids.iter().filter(|rule_id| rule_id.as_str() == "E9014").count(); - assert_eq!(event_pattern_findings, 29, "Expected one E9014 per rejected pattern shape, got: {ids:?}"); + assert_eq!(event_pattern_findings, 34, "Expected one E9014 per rejected pattern shape, got: {ids:?}"); } #[test] diff --git a/src/cfn-validate/tests/resource_rule_parity.rs b/src/cfn-validate/tests/resource_rule_parity.rs index d9f93f3e..f387dd99 100644 --- a/src/cfn-validate/tests/resource_rule_parity.rs +++ b/src/cfn-validate/tests/resource_rule_parity.rs @@ -2410,7 +2410,11 @@ fn invalid_event_patterns_have_identical_precise_diagnostics() { "Properties.EventPattern.detail.price.0.numeric", "must contain two or four elements", ), - ("EqualNumericBoundsRule", "Properties.EventPattern.detail.price.0.numeric", "cannot match any value"), + ( + "EqualNumericBoundsRule", + "Properties.EventPattern.detail.price.0.numeric", + "must have a lower bound strictly less than its upper bound", + ), ( "AnythingButMixedValuesRule", "Properties.EventPattern.detail.state.0.anything-but", @@ -2437,6 +2441,27 @@ fn invalid_event_patterns_have_identical_precise_diagnostics() { ("EmptyObjectRule", "Properties.EventPattern.detail", "must not be an empty object"), ("NestedMatchArrayRule", "Properties.EventPattern.source.0", "must be a scalar value or a matcher object"), ("ParameterWithInvalidSiblingRule", "Properties.EventPattern.detail.state", "is an empty array"), + ("ScalarPseudoParameterRule", "Properties.EventPattern.source", "must be an array or an object"), + ( + "PseudoNumericOperandRule", + "Properties.EventPattern.detail.account.0.numeric.1", + "must be a finite number whose decimal value is preserved as a 64-bit double", + ), + ( + "NestedNotificationArnsRule", + "Properties.EventPattern.resources.0", + "must be a scalar value or a matcher object", + ), + ( + "NotificationArnsPrefixRule", + "Properties.EventPattern.detail.topic.0.prefix", + "must be a string or an equals-ignore-case object containing one string", + ), + ( + "NonRepresentableNumericRule", + "Properties.EventPattern.detail.price.0.numeric.1", + "must be a finite number whose decimal value is preserved as a 64-bit double", + ), ("NestedScalarArchive", "Properties.EventPattern.detail.a.b.c", "must be an array or an object"), ]; assert_eq!(rego_findings.len(), expected.len(), "one finding per rejected pattern shape: {rego_findings:?}"); @@ -2460,7 +2485,10 @@ fn valid_and_unresolved_event_patterns_are_not_reported() { let rego_findings = selected_findings(®o, template, &["E9014"]); let cel_findings = selected_findings(&cel, template, &["E9014"]); assert_eq!(rego_findings, cel_findings); - assert!(rego_findings.is_empty(), "every matcher here is accepted by EventBridge: {rego_findings:?}"); + assert!( + rego_findings.is_empty(), + "valid and deployment-controlled event patterns must not be reported: {rego_findings:?}" + ); } #[test] diff --git a/src/rego-engine/tests/integration.rs b/src/rego-engine/tests/integration.rs index 94311724..643c71f6 100644 --- a/src/rego-engine/tests/integration.rs +++ b/src/rego-engine/tests/integration.rs @@ -872,7 +872,7 @@ fn e2e_e9014_invalid_event_patterns() { let report = validate_fixture("bad/E9014_invalid_event_patterns.yaml"); let event_pattern_findings = report.diagnostics.iter().filter(|diagnostic| diagnostic.rule_id == "E9014").count(); assert_eq!( - event_pattern_findings, 29, + event_pattern_findings, 34, "Expected one E9014 per rejected pattern shape, got: {:?}", report.diagnostics ); diff --git a/src/resources/expected/validation_reports1.json b/src/resources/expected/validation_reports1.json index 10daf486..038ecbbc 100644 --- a/src/resources/expected/validation_reports1.json +++ b/src/resources/expected/validation_reports1.json @@ -7859,12 +7859,12 @@ "filePath": "bad/E9014_invalid_event_patterns.yaml", "status": "OK", "metadata": { - "resourcesScanned": 30, + "resourcesScanned": 35, "counts": { "fatal": 0, - "errors": 29, + "errors": 34, "warnings": 0, - "informational": 31, + "informational": 36, "debug": 0 }, "suppressed": 0, @@ -8135,7 +8135,7 @@ { "ruleId": "E9014", "severity": "ERROR", - "message": "numeric range in 'EventPattern.detail.price[0]' cannot match any value", + "message": "numeric range in 'EventPattern.detail.price[0]' must have a lower bound strictly less than its upper bound", "source": "ENGINE", "entity": { "logicalId": "EqualNumericBoundsRule", @@ -8432,6 +8432,106 @@ "ruleDescription": "EventBridge rule and archive EventPattern must use valid matchers", "phase": "LINT" }, + { + "ruleId": "E9014", + "severity": "ERROR", + "message": "'EventPattern.source' must be an array or an object", + "source": "ENGINE", + "entity": { + "logicalId": "ScalarPseudoParameterRule", + "entityType": "Resource", + "resourceType": "AWS::Events::Rule" + }, + "propertyPath": "Properties.EventPattern.source", + "suggestedFix": "Wrap the value in an array", + "category": "Resource", + "startLine": 205, + "startColumn": 9, + "endLine": 205, + "endColumn": 15, + "ruleDescription": "EventBridge rule and archive EventPattern must use valid matchers", + "phase": "LINT" + }, + { + "ruleId": "E9014", + "severity": "ERROR", + "message": "numeric comparison value in 'EventPattern.detail.account[0]' must be a finite number whose decimal value is preserved as a 64-bit double", + "source": "ENGINE", + "entity": { + "logicalId": "PseudoNumericOperandRule", + "entityType": "Resource", + "resourceType": "AWS::Events::Rule" + }, + "propertyPath": "Properties.EventPattern.detail.account.0.numeric.1", + "suggestedFix": "Use a finite number whose decimal value is preserved as a 64-bit double", + "category": "Resource", + "startLine": 212, + "startColumn": 42, + "endLine": 212, + "endColumn": 43, + "ruleDescription": "EventBridge rule and archive EventPattern must use valid matchers", + "phase": "LINT" + }, + { + "ruleId": "E9014", + "severity": "ERROR", + "message": "'EventPattern.resources[0]' must be a scalar value or a matcher object", + "source": "ENGINE", + "entity": { + "logicalId": "NestedNotificationArnsRule", + "entityType": "Resource", + "resourceType": "AWS::Events::Rule" + }, + "propertyPath": "Properties.EventPattern.resources.0", + "suggestedFix": "Use a scalar value or an object containing exactly one matcher operator", + "category": "Resource", + "startLine": 218, + "startColumn": 26, + "endLine": 218, + "endColumn": 27, + "ruleDescription": "EventBridge rule and archive EventPattern must use valid matchers", + "phase": "LINT" + }, + { + "ruleId": "E9014", + "severity": "ERROR", + "message": "prefix in 'EventPattern.detail.topic[0]' must be a string or an equals-ignore-case object containing one string", + "source": "ENGINE", + "entity": { + "logicalId": "NotificationArnsPrefixRule", + "entityType": "Resource", + "resourceType": "AWS::Events::Rule" + }, + "propertyPath": "Properties.EventPattern.detail.topic.0.prefix", + "suggestedFix": "Use a string or an object containing only equals-ignore-case with a string value", + "category": "Resource", + "startLine": 225, + "startColumn": 20, + "endLine": 225, + "endColumn": 26, + "ruleDescription": "EventBridge rule and archive EventPattern must use valid matchers", + "phase": "LINT" + }, + { + "ruleId": "E9014", + "severity": "ERROR", + "message": "numeric comparison value in 'EventPattern.detail.price[0]' must be a finite number whose decimal value is preserved as a 64-bit double", + "source": "ENGINE", + "entity": { + "logicalId": "NonRepresentableNumericRule", + "entityType": "Resource", + "resourceType": "AWS::Events::Rule" + }, + "propertyPath": "Properties.EventPattern.detail.price.0.numeric.1", + "suggestedFix": "Use a finite number whose decimal value is preserved as a 64-bit double", + "category": "Resource", + "startLine": 232, + "startColumn": 35, + "endLine": 232, + "endColumn": 36, + "ruleDescription": "EventBridge rule and archive EventPattern must use valid matchers", + "phase": "LINT" + }, { "ruleId": "E9014", "severity": "ERROR", @@ -8445,9 +8545,9 @@ "propertyPath": "Properties.EventPattern.detail.a.b.c", "suggestedFix": "Wrap the value in an array", "category": "Resource", - "startLine": 212, + "startLine": 245, "startColumn": 15, - "endLine": 212, + "endLine": 245, "endColumn": 16, "ruleDescription": "EventBridge rule and archive EventPattern must use valid matchers", "phase": "LINT" @@ -8464,9 +8564,9 @@ }, "propertyPath": "Properties.Name", "category": "Best Practice", - "startLine": 203, + "startLine": 236, "startColumn": 7, - "endLine": 203, + "endLine": 236, "endColumn": 11, "documentationUrl": "https://github.com/aws-cloudformation/aws-cloudformation-resource-providers-events", "ruleDescription": "Create-only property updated triggers resource replacement", @@ -8487,9 +8587,9 @@ }, "propertyPath": "Properties.SourceArn", "category": "Best Practice", - "startLine": 207, + "startLine": 240, "startColumn": 7, - "endLine": 207, + "endLine": 240, "endColumn": 16, "ruleDescription": "Create-only property updated triggers resource replacement", "phase": "SCHEMA", @@ -9058,6 +9158,106 @@ "ruleDescription": "Resource should have Tags", "phase": "LINT" }, + { + "ruleId": "I9040", + "severity": "INFO", + "message": "Resource 'ScalarPseudoParameterRule' of type 'AWS::Events::Rule' supports Tags but none are configured", + "source": "ENGINE", + "entity": { + "logicalId": "ScalarPseudoParameterRule", + "entityType": "Resource", + "resourceType": "AWS::Events::Rule" + }, + "propertyPath": "Properties.Tags", + "suggestedFix": "Add Tags to improve resource organization and cost tracking", + "category": "Best Practice", + "startLine": 202, + "startColumn": 5, + "endLine": 202, + "endColumn": 15, + "ruleDescription": "Resource should have Tags", + "phase": "LINT" + }, + { + "ruleId": "I9040", + "severity": "INFO", + "message": "Resource 'PseudoNumericOperandRule' of type 'AWS::Events::Rule' supports Tags but none are configured", + "source": "ENGINE", + "entity": { + "logicalId": "PseudoNumericOperandRule", + "entityType": "Resource", + "resourceType": "AWS::Events::Rule" + }, + "propertyPath": "Properties.Tags", + "suggestedFix": "Add Tags to improve resource organization and cost tracking", + "category": "Best Practice", + "startLine": 208, + "startColumn": 5, + "endLine": 208, + "endColumn": 15, + "ruleDescription": "Resource should have Tags", + "phase": "LINT" + }, + { + "ruleId": "I9040", + "severity": "INFO", + "message": "Resource 'NestedNotificationArnsRule' of type 'AWS::Events::Rule' supports Tags but none are configured", + "source": "ENGINE", + "entity": { + "logicalId": "NestedNotificationArnsRule", + "entityType": "Resource", + "resourceType": "AWS::Events::Rule" + }, + "propertyPath": "Properties.Tags", + "suggestedFix": "Add Tags to improve resource organization and cost tracking", + "category": "Best Practice", + "startLine": 215, + "startColumn": 5, + "endLine": 215, + "endColumn": 15, + "ruleDescription": "Resource should have Tags", + "phase": "LINT" + }, + { + "ruleId": "I9040", + "severity": "INFO", + "message": "Resource 'NotificationArnsPrefixRule' of type 'AWS::Events::Rule' supports Tags but none are configured", + "source": "ENGINE", + "entity": { + "logicalId": "NotificationArnsPrefixRule", + "entityType": "Resource", + "resourceType": "AWS::Events::Rule" + }, + "propertyPath": "Properties.Tags", + "suggestedFix": "Add Tags to improve resource organization and cost tracking", + "category": "Best Practice", + "startLine": 221, + "startColumn": 5, + "endLine": 221, + "endColumn": 15, + "ruleDescription": "Resource should have Tags", + "phase": "LINT" + }, + { + "ruleId": "I9040", + "severity": "INFO", + "message": "Resource 'NonRepresentableNumericRule' of type 'AWS::Events::Rule' supports Tags but none are configured", + "source": "ENGINE", + "entity": { + "logicalId": "NonRepresentableNumericRule", + "entityType": "Resource", + "resourceType": "AWS::Events::Rule" + }, + "propertyPath": "Properties.Tags", + "suggestedFix": "Add Tags to improve resource organization and cost tracking", + "category": "Best Practice", + "startLine": 228, + "startColumn": 5, + "endLine": 228, + "endColumn": 15, + "ruleDescription": "Resource should have Tags", + "phase": "LINT" + }, { "ruleId": "I9040", "severity": "INFO", @@ -9071,9 +9271,9 @@ "propertyPath": "Properties.Tags", "suggestedFix": "Add Tags to improve resource organization and cost tracking", "category": "Best Practice", - "startLine": 202, + "startLine": 235, "startColumn": 5, - "endLine": 202, + "endLine": 235, "endColumn": 15, "ruleDescription": "Resource should have Tags", "phase": "LINT" diff --git a/src/resources/templates/bad/E9014_invalid_event_patterns.yaml b/src/resources/templates/bad/E9014_invalid_event_patterns.yaml index aace66be..de341024 100644 --- a/src/resources/templates/bad/E9014_invalid_event_patterns.yaml +++ b/src/resources/templates/bad/E9014_invalid_event_patterns.yaml @@ -197,6 +197,39 @@ Resources: source: !Ref EventSources detail: state: [] + ScalarPseudoParameterRule: + Type: AWS::Events::Rule + Properties: + State: DISABLED + EventPattern: + source: !Ref AWS::Region + PseudoNumericOperandRule: + Type: AWS::Events::Rule + Properties: + State: DISABLED + EventPattern: + detail: + account: [{numeric: [">", !Ref AWS::AccountId]}] + NestedNotificationArnsRule: + Type: AWS::Events::Rule + Properties: + State: DISABLED + EventPattern: + resources: [!Ref AWS::NotificationARNs] + NotificationArnsPrefixRule: + Type: AWS::Events::Rule + Properties: + State: DISABLED + EventPattern: + detail: + topic: [{prefix: !Ref AWS::NotificationARNs}] + NonRepresentableNumericRule: + Type: AWS::Events::Rule + Properties: + State: DISABLED + EventPattern: + detail: + price: [{numeric: ["=", 9007199254740993]}] Bus: Type: AWS::Events::EventBus Properties: diff --git a/src/resources/templates/good/E9014_valid_event_patterns.yaml b/src/resources/templates/good/E9014_valid_event_patterns.yaml index 9b4fa092..0d3e3e86 100644 --- a/src/resources/templates/good/E9014_valid_event_patterns.yaml +++ b/src/resources/templates/good/E9014_valid_event_patterns.yaml @@ -100,3 +100,9 @@ Resources: SourceArn: !GetAtt Bus.Arn EventPattern: source: [example.app] + resources: !Ref AWS::NotificationARNs + detail: + representable: + - {numeric: ["=", 9007199254740994]} + - {numeric: ["=", -9007199254740994]} + - {numeric: ["=", 1.0e+16]} diff --git a/src/template-model/src/event_pattern.rs b/src/template-model/src/event_pattern.rs index 325e91cf..2eae25ed 100644 --- a/src/template-model/src/event_pattern.rs +++ b/src/template-model/src/event_pattern.rs @@ -24,7 +24,6 @@ const LOWER_BOUND_COMPARISONS: [&str; 2] = [">", ">="]; const UPPER_BOUND_COMPARISONS: [&str; 2] = ["<", "<="]; const ANYTHING_BUT_NESTED_OPERATORS: [&str; 4] = [PREFIX_OPERATOR, SUFFIX_OPERATOR, WILDCARD_OPERATOR, EQUALS_IGNORE_CASE_OPERATOR]; -const MAX_EXACT_EVENTBRIDGE_INTEGER: u64 = 9_007_199_254_740_992; const MIN_OR_BRANCHES: usize = 2; const OR_BRANCH_RESERVED_FIELD_NAMES: &[&str] = &[ EXACTLY_OPERATOR, @@ -65,6 +64,7 @@ const SCALAR_VALUE_FIX: &str = "Wrap the value in an array"; const MATCH_VALUE_FIX: &str = "Use a scalar value or an object containing exactly one matcher operator"; const PREFIX_SUFFIX_FIX: &str = "Use a string or an object containing only equals-ignore-case with a string value"; const NUMERIC_FIX: &str = "Use [operator, number] or [>, number, <, number] with inclusive variants where allowed"; +const NUMERIC_VALUE_FIX: &str = "Use a finite number whose decimal value is preserved as a 64-bit double"; const ANYTHING_BUT_FIX: &str = "Give anything-but one string, one number, a homogeneous array, or one supported nested matcher"; const EXISTS_FIX: &str = "Set exists to an unquoted true or false"; @@ -466,8 +466,11 @@ impl<'a> EventPatternChecker<'a> { if first_number == NumericOperand::Invalid { self.add_finding( &first_number_location, - format!("numeric comparison value in '{}' must be a representable number", match_location.display_path), - NUMERIC_FIX, + format!( + "numeric comparison value in '{}' must be a finite number whose decimal value is preserved as a 64-bit double", + match_location.display_path + ), + NUMERIC_VALUE_FIX, ); return; } @@ -506,17 +509,23 @@ impl<'a> EventPatternChecker<'a> { if second_number == NumericOperand::Invalid { self.add_finding( &second_number_location, - format!("numeric comparison value in '{}' must be a representable number", match_location.display_path), - NUMERIC_FIX, + format!( + "numeric comparison value in '{}' must be a finite number whose decimal value is preserved as a 64-bit double", + match_location.display_path + ), + NUMERIC_VALUE_FIX, ); return; } if let (NumericOperand::Known(lower), NumericOperand::Known(upper)) = (first_number, second_number) { - let range_is_empty = lower >= upper; - if range_is_empty { + let bounds_are_not_strictly_increasing = lower >= upper; + if bounds_are_not_strictly_increasing { self.add_finding( operand_location, - format!("numeric range in '{}' cannot match any value", match_location.display_path), + format!( + "numeric range in '{}' must have a lower bound strictly less than its upper bound", + match_location.display_path + ), NUMERIC_FIX, ); } @@ -777,7 +786,7 @@ impl<'a> EventPatternChecker<'a> { match self.shape_at(value, location) { PatternShape::Unknown => NumericOperand::Unknown, PatternShape::Scalar(serde_json::Value::Number(number)) => { - numeric_comparison_number(number).map_or(NumericOperand::Invalid, NumericOperand::Known) + comparable_number(number).map_or(NumericOperand::Invalid, NumericOperand::Known) } PatternShape::Object(_) | PatternShape::Array(_) | PatternShape::Scalar(_) => NumericOperand::Invalid, } @@ -848,11 +857,6 @@ fn comparable_number(number: &serde_json::Number) -> Option { (authored == represented).then_some(value) } -fn numeric_comparison_number(number: &serde_json::Number) -> Option { - comparable_number(number) - .filter(|value| value.fract() != 0.0 || value.abs() <= MAX_EXACT_EVENTBRIDGE_INTEGER as f64) -} - fn normalize_decimal(text: &str) -> Option { let (is_negative, unsigned) = text.strip_prefix('-').map_or((false, text), |value| (true, value)); let (mantissa, explicit_exponent) = match unsigned.find(['e', 'E']) { @@ -1022,6 +1026,76 @@ Resources: assert_eq!(findings[0].property_path, "Properties.EventPattern.detail.state"); } + #[test] + fn notification_arns_list_does_not_hide_invalid_concrete_sibling() { + let template = r#" +Resources: + Rule: + Type: AWS::Events::Rule + Properties: + EventPattern: + resources: !Ref AWS::NotificationARNs + detail: + state: [] +"#; + let model = SemanticModel::from_bytes(template.as_bytes()).expect("template parses"); + let findings = event_pattern_violations(&model); + assert_eq!(findings.len(), 1, "the concrete sibling remains invalid: {findings:?}"); + assert_eq!(findings[0].property_path, "Properties.EventPattern.detail.state"); + } + + #[test] + fn scalar_pseudo_parameter_shapes_are_checked() { + let template = r#" +Resources: + Rule: + Type: AWS::Events::Rule + Properties: + EventPattern: + source: !Ref AWS::Region + detail: + account: [{numeric: [">", !Ref AWS::AccountId]}] +"#; + let model = SemanticModel::from_bytes(template.as_bytes()).expect("template parses"); + let findings = event_pattern_violations(&model); + assert_eq!(findings.len(), 2, "both scalar pseudo-parameter shapes are invalid: {findings:?}"); + assert!( + findings.iter().any(|finding| finding.property_path == "Properties.EventPattern.source"), + "source must remain subject to field-selector shape validation: {findings:?}" + ); + assert!( + findings + .iter() + .any(|finding| finding.property_path == "Properties.EventPattern.detail.account.0.numeric.1"), + "AccountId must remain subject to numeric operand validation: {findings:?}" + ); + } + + #[test] + fn notification_arns_list_shape_is_checked_in_matcher_contexts() { + let template = r#" +Resources: + Rule: + Type: AWS::Events::Rule + Properties: + EventPattern: + resources: [!Ref AWS::NotificationARNs] + detail: + topic: [{prefix: !Ref AWS::NotificationARNs}] +"#; + let model = SemanticModel::from_bytes(template.as_bytes()).expect("template parses"); + let findings = event_pattern_violations(&model); + assert_eq!(findings.len(), 2, "list values are invalid in both matcher contexts: {findings:?}"); + assert!( + findings.iter().any(|finding| finding.property_path == "Properties.EventPattern.resources.0"), + "a nested match array must be rejected: {findings:?}" + ); + assert!( + findings.iter().any(|finding| finding.property_path == "Properties.EventPattern.detail.topic.0.prefix"), + "prefix must reject a list operand: {findings:?}" + ); + } + #[test] fn prefix_and_suffix_case_insensitive_forms_require_one_string() { assert!(messages(r#"{"x": [{"prefix": {"equals-ignore-case": "abc"}}, {"suffix": "xyz"}]}"#).is_empty()); @@ -1042,13 +1116,6 @@ Resources: r#"{"x": [{"numeric": [">", true]}]}"#, r#"{"x": [{"numeric": ["<", 1, "<", 2]}]}"#, r#"{"x": [{"numeric": [">", 1, ">=", 2]}]}"#, - r#"{"x": [{"numeric": [">", 2, "<", 1]}]}"#, - r#"{"x": [{"numeric": [">=", 1, "<=", 1]}]}"#, - r#"{"x": [{"numeric": [">=", -0.0, "<=", 0]}]}"#, - r#"{"x": [{"numeric": ["=", 9007199254740993]}]}"#, - r#"'{"x": [{"numeric": ["=", 9007199254740993.0]}]}'"#, - r#"'{"x": [{"numeric": ["=", 0.30000000000000001]}]}'"#, - r#"{"x": [{"numeric": ["=", 1e16]}]}"#, ] { assert_eq!(messages(pattern).len(), 1, "{pattern}"); } @@ -1056,12 +1123,46 @@ Resources: r#"{"x": [{"numeric": ["=", 3.8]}]}"#, r#"'{"x": [{"numeric": ["=", 1E-320]}]}'"#, r#"{"x": [{"numeric": [">=", 0, "<=", 1]}]}"#, + ] { + assert!(messages(pattern).is_empty(), "{pattern}"); + } + } + + #[test] + fn numeric_comparison_values_must_preserve_their_decimal_value() { + for pattern in [ + r#"{"x": [{"numeric": ["=", 9007199254740993]}]}"#, + r#"{"x": [{"numeric": ["=", 9007199254740995]}]}"#, + r#"'{"x": [{"numeric": ["=", 9007199254740993.0]}]}'"#, + r#"'{"x": [{"numeric": ["=", 0.30000000000000001]}]}'"#, + ] { + assert_eq!(messages(pattern).len(), 1, "{pattern}"); + } + for pattern in [ r#"{"x": [{"numeric": ["=", 9007199254740992.0]}]}"#, + r#"{"x": [{"numeric": ["=", 9007199254740994]}]}"#, + r#"{"x": [{"numeric": ["=", -9007199254740994]}]}"#, + r#"{"x": [{"numeric": ["=", 1.0e+16]}]}"#, ] { assert!(messages(pattern).is_empty(), "{pattern}"); } } + #[test] + fn numeric_ranges_require_strictly_increasing_bounds() { + let expected = vec![ + "numeric range in 'EventPattern.x[0]' must have a lower bound strictly less than its upper bound" + .to_string(), + ]; + for pattern in [ + r#"{"x": [{"numeric": [">", 2, "<", 1]}]}"#, + r#"{"x": [{"numeric": [">=", 1, "<=", 1]}]}"#, + r#"{"x": [{"numeric": [">=", -0.0, "<=", 0]}]}"#, + ] { + assert_eq!(messages(pattern), expected, "{pattern}"); + } + } + #[test] fn anything_but_requires_supported_homogeneous_values() { for pattern in [ diff --git a/src/template-model/src/resolver.rs b/src/template-model/src/resolver.rs index 3ec95e46..3ddbbbca 100644 --- a/src/template-model/src/resolver.rs +++ b/src/template-model/src/resolver.rs @@ -935,8 +935,8 @@ impl<'a> Resolver<'a> { if target == PSEUDO_NO_VALUE { return Some(ResolvedValue::Concrete { value: serde_json::Value::Null.into() }); } - if let Some(val) = self.pseudo_parameter_overrides.get(target) { - return Some(ResolvedValue::Concrete { value: serde_json::Value::String(val).into() }); + if let Some(value) = self.pseudo_parameter_overrides.get(target) { + return Some(ResolvedValue::Concrete { value: pseudo_parameter_ref_to_json(target, value).into() }); } return Some(ResolvedValue::Dynamic { reason: format!("pseudo-parameter {}", target) }); } @@ -1583,6 +1583,13 @@ impl<'a> Resolver<'a> { } } +fn pseudo_parameter_ref_to_json(name: &str, value: String) -> serde_json::Value { + if name == PSEUDO_NOTIFICATION_ARNS { + return serde_json::Value::Array(vec![serde_json::Value::String(value)]); + } + serde_json::Value::String(value) +} + fn param_string_to_json(value: &str, param_type: &str) -> serde_json::Value { if param_type == PARAM_TYPE_COMMA_DELIMITED_LIST || param_type.starts_with("List<") { return serde_json::Value::Array( @@ -2310,6 +2317,17 @@ mod tests { use super::*; use crate::parser; + fn concrete_property<'a>( + model: &'a crate::model::SemanticModel, + resource_id: &str, + property_path: &str, + ) -> &'a serde_json::Value { + match model.resolve(resource_id, property_path) { + Some(ResolvedValue::Concrete { value }) => value, + other => panic!("Expected concrete value at {resource_id}.{property_path}, got {other:?}"), + } + } + #[test] fn param_number_whole_value_resolves_to_integer() { // A whole-number Number parameter must resolve to a JSON integer, not a @@ -2369,6 +2387,39 @@ mod tests { } } + #[test] + fn resolve_ref_notification_arns_as_list() { + let input = r#"{"Resources":{"R":{"Type":"T","Properties":{"V":{"Ref":"AWS::NotificationARNs"}}}}}"#; + let model = crate::model::SemanticModel::from_bytes(input.as_bytes()).expect("template parses"); + assert_eq!( + concrete_property(&model, "R", "Properties.V"), + &serde_json::json!(["arn:aws:sns:us-east-1:123456789012:notification"]), + "NotificationARNs must keep its CloudFormation list shape" + ); + } + + #[test] + fn notification_arns_override_preserves_list_shape_through_intrinsics() { + let input = r#"{"Resources":{"R":{"Type":"T","Properties":{ + "Notifications":{"Ref":"AWS::NotificationARNs"}, + "First":{"Fn::Select":[0,{"Ref":"AWS::NotificationARNs"}]}, + "Joined":{"Fn::Join":["|",{"Ref":"AWS::NotificationARNs"}]} + }}}}"#; + let configured_arn = "arn:aws:sns:us-east-1:123456789012:configured"; + let config = crate::model::ParseConfig { + parameters: HashMap::new(), + pseudo_parameters: crate::model::PseudoParameterOverrides { + notification_arns: Some(configured_arn.to_string()), + ..Default::default() + }, + }; + let model = crate::model::SemanticModel::parse(input.as_bytes(), config).expect("template parses").model; + + assert_eq!(concrete_property(&model, "R", "Properties.Notifications"), &serde_json::json!([configured_arn])); + assert_eq!(concrete_property(&model, "R", "Properties.First"), &serde_json::json!(configured_arn)); + assert_eq!(concrete_property(&model, "R", "Properties.Joined"), &serde_json::json!(configured_arn)); + } + #[test] fn resolve_if() { let input = r#"{"Conditions":{"C":{"Fn::Equals":["a","a"]}},"Resources":{"R":{"Type":"T","Properties":{"V":{"Fn::If":["C",100,20]}}}}}"#;