From c23bf79295bcf22080cc02a190ad413afb5ae2f2 Mon Sep 17 00:00:00 2001 From: Satyaki Ghosh Date: Mon, 20 Jul 2026 15:46:54 -0400 Subject: [PATCH 1/4] Only validate AWS:: resource types --- src/cel-engine/src/rules/resources_extra.rs | 21 ++-- .../resources/properties/resource_type.rego | 16 ++- src/rego-engine/tests/integration.rs | 2 +- src/resources/expected/all_templates.json | 104 +++++++----------- .../bad/F3006_invalid_aws_namespaces.yaml | 14 ++- .../good/unknown_resource_types_ignored.yaml | 30 +++++ src/rules/src/registry.rs | 2 +- 7 files changed, 96 insertions(+), 93 deletions(-) create mode 100644 src/resources/templates/good/unknown_resource_types_ignored.yaml diff --git a/src/cel-engine/src/rules/resources_extra.rs b/src/cel-engine/src/rules/resources_extra.rs index 5682f5e..bb95d60 100644 --- a/src/cel-engine/src/rules/resources_extra.rs +++ b/src/cel-engine/src/rules/resources_extra.rs @@ -312,18 +312,15 @@ pub fn eval_extra_resources(ctx: &EvalContext) -> Vec { if !ctx.cached_data.known_types.is_empty() { for (name, res) in &m.resources { - if res.resource_type.is_empty() { - continue; - } - // Only custom resources (`Custom::` prefix) and modules (`::MODULE` - // suffix) are exempt from the known-type set; every other type — - // including all AWS-namespaced types such as `AWS::Serverless::*` and - // `AWS::CloudFormation::*` — must be a recognized type. Valid - // transform/service types already appear in `known_types`, so no - // namespace is exempted wholesale. - if !ctx.cached_data.known_types.contains(&res.resource_type) - && !res.resource_type.starts_with("Custom::") - && !res.resource_type.ends_with("::MODULE") + // An AWS-namespaced type absent from the compiled schema set is a + // typo or nonexistent type — CloudFormation owns the reserved + // `AWS::` namespace, so the embedded catalog is authoritative for + // it. Types in any other namespace (private registry types, + // `Custom::` resources, modules, hook-shaped names) may be + // registered per account/region, so they are skipped entirely + // rather than guessed at. + if res.resource_type.starts_with("AWS::") + && !ctx.cached_data.known_types.contains(&res.resource_type) { out.push(make_resource_diagnostic( "F3006", diff --git a/src/rego-engine/handwritten/rego/resources/properties/resource_type.rego b/src/rego-engine/handwritten/rego/resources/properties/resource_type.rego index 8da9566..d3ac3ca 100644 --- a/src/rego-engine/handwritten/rego/resources/properties/resource_type.rego +++ b/src/rego-engine/handwritten/rego/resources/properties/resource_type.rego @@ -2,19 +2,17 @@ package resources import rego.v1 -# F3006: Resource Type must be known. Only custom resources (`Custom::` prefix) -# and modules (`::MODULE` suffix) are exempt from the known-type set; every other -# type — including all AWS-namespaced types such as `AWS::Serverless::*` and -# `AWS::CloudFormation::*` — must be a recognized type. Valid transform/service -# types already appear in `known_resource_types`, so no namespace is exempted -# wholesale. +# F3006: An AWS-namespaced resource type that is not in the compiled schema +# set is a typo or nonexistent type — CloudFormation owns the reserved `AWS::` +# namespace, so the embedded schema catalog is authoritative for it. Types in +# any other namespace (private registry types, `Custom::` resources, modules, +# hook-shaped names) may be registered per account/region, so they are skipped +# entirely rather than guessed at. violation contains make_diag("F3006", "FATAL", name, sprintf("Unknown resource type '%s'", [rtype])) if { some name, res in input.resources rtype := res.resourceType is_string(rtype) - rtype != "" + startswith(rtype, "AWS::") not rtype in data.known_resource_types - not startswith(rtype, "Custom::") - not endswith(rtype, "::MODULE") } diff --git a/src/rego-engine/tests/integration.rs b/src/rego-engine/tests/integration.rs index 6c75606..6fd5c29 100644 --- a/src/rego-engine/tests/integration.rs +++ b/src/rego-engine/tests/integration.rs @@ -228,7 +228,7 @@ fn e2e_bad_security_issues() { #[test] fn e2e_bad_unknown_properties() { let report = validate_fixture("bad/unknown_properties.yaml"); - assert!(has_rule(&report, "F3006"), "Expected F3006 for unknown type, got: {:?}", report.diagnostics); + assert!(has_rule(&report, "F3006"), "Expected F3006 for unknown AWS type, got: {:?}", report.diagnostics); assert!(has_rule(&report, "F3002"), "Expected F3002 for unknown property, got: {:?}", report.diagnostics); } diff --git a/src/resources/expected/all_templates.json b/src/resources/expected/all_templates.json index 2e2d896..aef4c48 100644 --- a/src/resources/expected/all_templates.json +++ b/src/resources/expected/all_templates.json @@ -822,7 +822,7 @@ "startColumn": 3, "endLine": 8, "endColumn": 10, - "ruleDescription": "Resource type must be recognized", + "ruleDescription": "AWS resource type must be recognized and available in the configured region", "phase": "SCHEMA", "section": "Resources", "context": { @@ -913,7 +913,7 @@ "metadata": { "resourcesScanned": 7, "counts": { - "fatal": 3, + "fatal": 2, "errors": 0, "warnings": 2, "informational": 2, @@ -932,11 +932,11 @@ "resourceId": "BadServerless", "resourceType": "AWS::Serverless::NotARealType", "category": "Schema", - "startLine": 10, + "startLine": 11, "startColumn": 3, - "endLine": 10, + "endLine": 11, "endColumn": 16, - "ruleDescription": "Resource type must be recognized", + "ruleDescription": "AWS resource type must be recognized and available in the configured region", "phase": "SCHEMA", "section": "Resources", "context": { @@ -953,11 +953,11 @@ "resourceId": "BadCloudFormation", "resourceType": "AWS::CloudFormation::NotARealType", "category": "Schema", - "startLine": 15, + "startLine": 16, "startColumn": 3, - "endLine": 15, + "endLine": 16, "endColumn": 20, - "ruleDescription": "Resource type must be recognized", + "ruleDescription": "AWS resource type must be recognized and available in the configured region", "phase": "SCHEMA", "section": "Resources", "context": { @@ -966,27 +966,6 @@ } } }, - { - "ruleId": "F3006", - "severity": "FATAL", - "message": "Unknown resource type 'My::Org::MODULE::Thing'", - "source": "SCHEMA", - "resourceId": "MidStringModule", - "resourceType": "My::Org::MODULE::Thing", - "category": "Schema", - "startLine": 19, - "startColumn": 3, - "endLine": 19, - "endColumn": 18, - "ruleDescription": "Resource type must be recognized", - "phase": "SCHEMA", - "section": "Resources", - "context": { - "extra": { - "resource_type": "My::Org::MODULE::Thing" - } - } - }, { "ruleId": "W9013", "severity": "WARN", @@ -995,9 +974,9 @@ "resourceId": "GoodCustomResource", "resourceType": "AWS::CloudFormation::CustomResource", "category": "Security", - "startLine": 29, + "startLine": 31, "startColumn": 3, - "endLine": 29, + "endLine": 31, "endColumn": 21, "ruleDescription": "Hardcoded account ID in ARN", "phase": "LINT", @@ -1011,9 +990,9 @@ "resourceId": "GoodCustom", "resourceType": "Custom::MyThing", "category": "Security", - "startLine": 36, + "startLine": 38, "startColumn": 3, - "endLine": 36, + "endLine": 38, "endColumn": 13, "ruleDescription": "Hardcoded account ID in ARN", "phase": "LINT", @@ -1028,9 +1007,9 @@ "resourceType": "AWS::CloudFormation::CustomResource", "propertyPath": "Properties.ServiceToken", "category": "Best Practice", - "startLine": 32, + "startLine": 34, "startColumn": 7, - "endLine": 32, + "endLine": 34, "endColumn": 19, "ruleDescription": "Create-only property updated triggers resource replacement", "phase": "SCHEMA", @@ -1049,9 +1028,9 @@ "propertyPath": "Properties.Tags", "suggestedFix": "Add Tags to improve resource organization and cost tracking", "category": "Best Practice", - "startLine": 25, + "startLine": 27, "startColumn": 5, - "endLine": 25, + "endLine": 27, "endColumn": 15, "ruleDescription": "Resource should have Tags", "phase": "LINT", @@ -12317,7 +12296,7 @@ "startColumn": 3, "endLine": 23, "endColumn": 17, - "ruleDescription": "Resource type must be recognized", + "ruleDescription": "AWS resource type must be recognized and available in the configured region", "phase": "SCHEMA", "section": "Resources", "context": { @@ -37016,7 +36995,7 @@ "startColumn": 3, "endLine": 3, "endColumn": 15, - "ruleDescription": "Resource type must be recognized", + "ruleDescription": "AWS resource type must be recognized and available in the configured region", "phase": "SCHEMA", "section": "Resources", "context": { @@ -101829,7 +101808,7 @@ "startColumn": 3, "endLine": 11, "endColumn": 20, - "ruleDescription": "Resource type must be recognized", + "ruleDescription": "AWS resource type must be recognized and available in the configured region", "phase": "SCHEMA", "section": "Resources", "context": { @@ -126842,7 +126821,7 @@ "metadata": { "resourcesScanned": 1, "counts": { - "fatal": 1, + "fatal": 0, "errors": 0, "warnings": 0, "informational": 0, @@ -126852,29 +126831,7 @@ "strict": false, "severityLevel": "DEBUG" }, - "diagnostics": [ - { - "ruleId": "F3006", - "severity": "FATAL", - "message": "Unknown resource type 'Initech::TPS::Report'", - "source": "SCHEMA", - "resourceId": "MyReport", - "resourceType": "Initech::TPS::Report", - "category": "Schema", - "startLine": 2, - "startColumn": 3, - "endLine": 2, - "endColumn": 11, - "ruleDescription": "Resource type must be recognized", - "phase": "SCHEMA", - "section": "Resources", - "context": { - "extra": { - "resource_type": "Initech::TPS::Report" - } - } - } - ] + "diagnostics": [] }, "good/schema_valid_resources.yaml": { "filePath": "good/schema_valid_resources.yaml", @@ -128824,6 +128781,25 @@ } ] }, + "good/unknown_resource_types_ignored.yaml": { + "filePath": "good/unknown_resource_types_ignored.yaml", + "status": "OK", + "version": "1.6.0", + "metadata": { + "resourcesScanned": 5, + "counts": { + "fatal": 0, + "errors": 0, + "warnings": 0, + "informational": 0, + "debug": 0 + }, + "suppressed": 0, + "strict": false, + "severityLevel": "DEBUG" + }, + "diagnostics": [] + }, "good/vpc_subnets.yaml": { "filePath": "good/vpc_subnets.yaml", "status": "OK", diff --git a/src/resources/templates/bad/F3006_invalid_aws_namespaces.yaml b/src/resources/templates/bad/F3006_invalid_aws_namespaces.yaml index c6e87a4..8d23b12 100644 --- a/src/resources/templates/bad/F3006_invalid_aws_namespaces.yaml +++ b/src/resources/templates/bad/F3006_invalid_aws_namespaces.yaml @@ -1,10 +1,11 @@ AWSTemplateFormatVersion: "2010-09-09" Transform: AWS::Serverless-2016-10-31 Description: > - F3006 must fire for AWS-namespaced resource types that are not recognized — - no namespace (AWS::Serverless::, AWS::CloudFormation::) is exempted wholesale, - and ::MODULE is only exempt as a suffix. Both engines must agree, matching - cfn-lint's E3006. Valid types below must stay clean. + F3006 fires only for AWS-namespaced resource types that are not in the + compiled schema set — CloudFormation owns the reserved AWS:: namespace, so + unknown names there are typos. Types in any other namespace (private + registry, Custom::, modules, hook-shaped names) are not validated and must + produce no F3006. Both engines must agree. Valid types below must stay clean. Resources: # Invalid SAM type — not a real AWS::Serverless resource. Must fire. BadServerless: @@ -15,11 +16,12 @@ Resources: BadCloudFormation: Type: AWS::CloudFormation::NotARealType Properties: {} - # ::MODULE appearing mid-string is not the module suffix. Must fire. + # --- Counter-examples that must NOT fire --- + # Non-AWS namespace: may be privately registered, so it is never flagged — + # even though ::MODULE appears mid-string rather than as the module suffix. MidStringModule: Type: My::Org::MODULE::Thing Properties: {} - # --- Counter-examples that must NOT fire --- GoodFunction: Type: AWS::Serverless::Function Properties: diff --git a/src/resources/templates/good/unknown_resource_types_ignored.yaml b/src/resources/templates/good/unknown_resource_types_ignored.yaml new file mode 100644 index 0000000..9440257 --- /dev/null +++ b/src/resources/templates/good/unknown_resource_types_ignored.yaml @@ -0,0 +1,30 @@ +AWSTemplateFormatVersion: "2010-09-09" +Description: > + Resource types outside the reserved AWS:: namespace (private registry + types, third-party namespaces, hook-shaped names, legacy custom resources, + and modules) have no compiled provider schema and may be registered per + account/region. They are not validated and must produce no diagnostics in + either engine. +Resources: + PrivateRegistryType: + Type: Initech::TPS::Report + Properties: + Title: anything-goes-here + UnbundledAmznType: + Type: AMZN::Internal::UnbundledType + Properties: + Foo: bar + HookShapedType: + Type: MyCompany::Testing::MyTestHook + Properties: + Foo: bar + LegacyCustom: + Type: Custom::MyThing + Properties: + ServiceToken: !Sub arn:${AWS::Partition}:sns:${AWS::Region}:${AWS::AccountId}:topic + ArbitraryProperty: + Nested: value + OrgModule: + Type: My::Org::Resource::MODULE + Properties: + Anything: 42 diff --git a/src/rules/src/registry.rs b/src/rules/src/registry.rs index 9a6b9a6..63db558 100644 --- a/src/rules/src/registry.rs +++ b/src/rules/src/registry.rs @@ -788,7 +788,7 @@ pub const RULE_REGISTRY: &[RuleDefinition] = &[ RuleDefinition { id: "F3006", category: Category::Schema, - description: "Resource type must be recognized", + description: "AWS resource type must be recognized and available in the configured region", origin: RuleOrigin::Schema, }, RuleDefinition { From 992376bf498ebe91324af7b42b04a0a3bb29e3d4 Mon Sep 17 00:00:00 2001 From: Satyaki Ghosh Date: Mon, 20 Jul 2026 17:56:20 -0400 Subject: [PATCH 2/4] Fix double/long integer comparison bug --- src/cfn-validate/tests/github_issues.rs | 26 ++ src/resources/expected/all_templates.json | 308 ++++++++++++++++++ .../templates/gh-issues/issue-183.yaml | 35 ++ .../good/number_param_integer_enum.yaml | 30 ++ src/template-model/src/coercion.rs | 36 +- src/template-model/src/resolver.rs | 26 +- 6 files changed, 451 insertions(+), 10 deletions(-) create mode 100644 src/resources/templates/gh-issues/issue-183.yaml create mode 100644 src/resources/templates/good/number_param_integer_enum.yaml diff --git a/src/cfn-validate/tests/github_issues.rs b/src/cfn-validate/tests/github_issues.rs index dd36b20..2f0b516 100644 --- a/src/cfn-validate/tests/github_issues.rs +++ b/src/cfn-validate/tests/github_issues.rs @@ -840,6 +840,32 @@ Resources: assert_count(&diags, "W9009", 1); } +/// Issue #183: W3663 must not fire when a Lambda Permission's literal SourceArn +/// carries a proper 12-digit account id — that ARN pins the calling account by +/// itself. The issue's original ARN had a 10-digit account field, which is not +/// a valid account id: it cannot pin the account, so W3663 fires for that +/// resource only (and the ARN also fails the schema pattern, F3031) — matching +/// cfn-lint, whose extension schema uses the same ':\d{12}:' trigger. +/// https://github.com/aws-cloudformation/cloudformation-validate/issues/183 +#[test] +fn issue_183_w3663_fires_only_for_source_arn_without_valid_account_id() { + let diags = validate_both("issue-183.yaml"); + assert_count(&diags, "W3663", 1); + assert_fires_on_resource(&diags, "W3663", "PermissionInvalidAccountId"); + assert_fires_with_severity(&diags, "W3663", Severity::Warn); + for (engine, d) in &diags { + let on_valid = d + .iter() + .filter(|x| x.rule_id == "W3663") + .any(|x| x.resource.as_ref().and_then(|r| r.id.as_deref()) == Some("PermissionWithAccountId")); + assert!(!on_valid, "[{engine}] W3663 must not fire on the permission whose ARN has a 12-digit account id"); + } + // The invalid 10-digit account field also fails the ARN schema pattern, + // and only there — the valid ARN passes it. + assert_count(&diags, "F3031", 1); + assert_fires_on_resource(&diags, "F3031", "PermissionInvalidAccountId"); +} + // --------------------------------------------------------------------------- // Issue #36 — the IAM-role-ARN checks use a future-proof `arn:aws[a-zA-Z-]*` // partition prefix, so ADC-partition ARNs no longer false-positive and the two diff --git a/src/resources/expected/all_templates.json b/src/resources/expected/all_templates.json index aef4c48..3285aa2 100644 --- a/src/resources/expected/all_templates.json +++ b/src/resources/expected/all_templates.json @@ -105398,6 +105398,295 @@ } ] }, + "gh-issues/issue-183.yaml": { + "filePath": "gh-issues/issue-183.yaml", + "status": "OK", + "version": "1.6.0", + "metadata": { + "resourcesScanned": 3, + "counts": { + "fatal": 1, + "errors": 0, + "warnings": 4, + "informational": 9, + "debug": 0 + }, + "suppressed": 0, + "strict": false, + "severityLevel": "DEBUG" + }, + "diagnostics": [ + { + "ruleId": "F3031", + "severity": "FATAL", + "message": "'arn:aws:sqs:us-east-1:1111111111:MyQueue' does not match pattern '^arn:(aws[a-zA-Z0-9-]*):([a-zA-Z0-9\\-])+:((eusc-)?[a-z]{2}((-gov)|(-iso([a-z]?)))?-[a-z]+-\\d{1})?:(\\d{12})?:(.*)$'", + "source": "SCHEMA", + "resourceId": "PermissionInvalidAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties.SourceArn", + "category": "Schema", + "startLine": 35, + "startColumn": 7, + "endLine": 35, + "endColumn": 16, + "ruleDescription": "Value does not match pattern", + "phase": "SCHEMA", + "section": "Resources", + "context": { + "actualValue": "arn:aws:sqs:us-east-1:1111111111:MyQueue", + "expectedConstraint": "^arn:(aws[a-zA-Z0-9-]*):([a-zA-Z0-9\\-])+:((eusc-)?[a-z]{2}((-gov)|(-iso([a-z]?)))?-[a-z]+-\\d{1})?:(\\d{12})?:(.*)$" + } + }, + { + "ruleId": "W3663", + "severity": "WARN", + "message": "Lambda Permission with a SourceArn that has no account id should also specify SourceAccount", + "source": "CFN_LINT", + "resourceId": "PermissionInvalidAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties", + "suggestedFix": "Add SourceAccount property", + "category": "Security", + "startLine": 31, + "startColumn": 5, + "endLine": 31, + "endColumn": 15, + "ruleDescription": "Validate SourceAccount is required property", + "phase": "LINT", + "section": "Resources" + }, + { + "ruleId": "W9002", + "severity": "WARN", + "message": "Property 'SourceArn' has a hardcoded ARN - use Ref, GetAtt, or a parameter instead", + "source": "ENGINE", + "resourceId": "PermissionWithAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties.SourceArn", + "category": "Best Practice", + "startLine": 25, + "startColumn": 7, + "endLine": 25, + "endColumn": 16, + "ruleDescription": "Hardcoded ARN property", + "phase": "LINT", + "section": "Resources" + }, + { + "ruleId": "W9002", + "severity": "WARN", + "message": "Property 'SourceArn' has a hardcoded ARN - use Ref, GetAtt, or a parameter instead", + "source": "ENGINE", + "resourceId": "PermissionInvalidAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties.SourceArn", + "category": "Best Practice", + "startLine": 35, + "startColumn": 7, + "endLine": 35, + "endColumn": 16, + "ruleDescription": "Hardcoded ARN property", + "phase": "LINT", + "section": "Resources" + }, + { + "ruleId": "W9013", + "severity": "WARN", + "message": "Hardcoded account ID in ARN - use AWS::AccountId pseudo-parameter", + "source": "ENGINE", + "resourceId": "PermissionWithAccountId", + "resourceType": "AWS::Lambda::Permission", + "category": "Security", + "startLine": 19, + "startColumn": 3, + "endLine": 19, + "endColumn": 26, + "ruleDescription": "Hardcoded account ID in ARN", + "phase": "LINT", + "section": "Resources" + }, + { + "ruleId": "I9001", + "severity": "INFO", + "message": "Property 'Action' is create-only; updating it will cause resource replacement", + "source": "ENGINE", + "resourceId": "PermissionWithAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties.Action", + "category": "Best Practice", + "startLine": 22, + "startColumn": 7, + "endLine": 22, + "endColumn": 13, + "ruleDescription": "Create-only property updated triggers resource replacement", + "phase": "SCHEMA", + "section": "Resources", + "context": { + "lifecycle": "create-only" + } + }, + { + "ruleId": "I9001", + "severity": "INFO", + "message": "Property 'FunctionName' is create-only; updating it will cause resource replacement", + "source": "ENGINE", + "resourceId": "PermissionWithAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties.FunctionName", + "category": "Best Practice", + "startLine": 23, + "startColumn": 7, + "endLine": 23, + "endColumn": 19, + "ruleDescription": "Create-only property updated triggers resource replacement", + "phase": "SCHEMA", + "section": "Resources", + "context": { + "lifecycle": "create-only", + "resolutionSource": "GetAtt MyLambda.Arn" + } + }, + { + "ruleId": "I9001", + "severity": "INFO", + "message": "Property 'Principal' is create-only; updating it will cause resource replacement", + "source": "ENGINE", + "resourceId": "PermissionWithAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties.Principal", + "category": "Best Practice", + "startLine": 24, + "startColumn": 7, + "endLine": 24, + "endColumn": 16, + "ruleDescription": "Create-only property updated triggers resource replacement", + "phase": "SCHEMA", + "section": "Resources", + "context": { + "lifecycle": "create-only" + } + }, + { + "ruleId": "I9001", + "severity": "INFO", + "message": "Property 'SourceArn' is create-only; updating it will cause resource replacement", + "source": "ENGINE", + "resourceId": "PermissionWithAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties.SourceArn", + "category": "Best Practice", + "startLine": 25, + "startColumn": 7, + "endLine": 25, + "endColumn": 16, + "ruleDescription": "Create-only property updated triggers resource replacement", + "phase": "SCHEMA", + "section": "Resources", + "context": { + "lifecycle": "create-only" + } + }, + { + "ruleId": "I9001", + "severity": "INFO", + "message": "Property 'Action' is create-only; updating it will cause resource replacement", + "source": "ENGINE", + "resourceId": "PermissionInvalidAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties.Action", + "category": "Best Practice", + "startLine": 32, + "startColumn": 7, + "endLine": 32, + "endColumn": 13, + "ruleDescription": "Create-only property updated triggers resource replacement", + "phase": "SCHEMA", + "section": "Resources", + "context": { + "lifecycle": "create-only" + } + }, + { + "ruleId": "I9001", + "severity": "INFO", + "message": "Property 'FunctionName' is create-only; updating it will cause resource replacement", + "source": "ENGINE", + "resourceId": "PermissionInvalidAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties.FunctionName", + "category": "Best Practice", + "startLine": 33, + "startColumn": 7, + "endLine": 33, + "endColumn": 19, + "ruleDescription": "Create-only property updated triggers resource replacement", + "phase": "SCHEMA", + "section": "Resources", + "context": { + "lifecycle": "create-only", + "resolutionSource": "GetAtt MyLambda.Arn" + } + }, + { + "ruleId": "I9001", + "severity": "INFO", + "message": "Property 'Principal' is create-only; updating it will cause resource replacement", + "source": "ENGINE", + "resourceId": "PermissionInvalidAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties.Principal", + "category": "Best Practice", + "startLine": 34, + "startColumn": 7, + "endLine": 34, + "endColumn": 16, + "ruleDescription": "Create-only property updated triggers resource replacement", + "phase": "SCHEMA", + "section": "Resources", + "context": { + "lifecycle": "create-only" + } + }, + { + "ruleId": "I9001", + "severity": "INFO", + "message": "Property 'SourceArn' is create-only; updating it will cause resource replacement", + "source": "ENGINE", + "resourceId": "PermissionInvalidAccountId", + "resourceType": "AWS::Lambda::Permission", + "propertyPath": "Properties.SourceArn", + "category": "Best Practice", + "startLine": 35, + "startColumn": 7, + "endLine": 35, + "endColumn": 16, + "ruleDescription": "Create-only property updated triggers resource replacement", + "phase": "SCHEMA", + "section": "Resources", + "context": { + "lifecycle": "create-only" + } + }, + { + "ruleId": "I9040", + "severity": "INFO", + "message": "Resource 'MyLambda' of type 'AWS::Lambda::Function' supports Tags but none are configured", + "source": "ENGINE", + "resourceId": "MyLambda", + "resourceType": "AWS::Lambda::Function", + "propertyPath": "Properties.Tags", + "suggestedFix": "Add Tags to improve resource organization and cost tracking", + "category": "Best Practice", + "startLine": 12, + "startColumn": 5, + "endLine": 12, + "endColumn": 15, + "ruleDescription": "Resource should have Tags", + "phase": "LINT", + "section": "Resources" + } + ] + }, "gh-issues/issue-34-w2506-overfire.json": { "filePath": "gh-issues/issue-34-w2506-overfire.json", "status": "OK", @@ -118711,6 +119000,25 @@ } ] }, + "good/number_param_integer_enum.yaml": { + "filePath": "good/number_param_integer_enum.yaml", + "status": "OK", + "version": "1.6.0", + "metadata": { + "resourcesScanned": 2, + "counts": { + "fatal": 0, + "errors": 0, + "warnings": 0, + "informational": 0, + "debug": 0 + }, + "suppressed": 0, + "strict": false, + "severityLevel": "DEBUG" + }, + "diagnostics": [] + }, "good/output_value_string.yaml": { "filePath": "good/output_value_string.yaml", "status": "OK", diff --git a/src/resources/templates/gh-issues/issue-183.yaml b/src/resources/templates/gh-issues/issue-183.yaml new file mode 100644 index 0000000..db17ace --- /dev/null +++ b/src/resources/templates/gh-issues/issue-183.yaml @@ -0,0 +1,35 @@ +AWSTemplateFormatVersion: "2010-09-09" +Description: > + GitHub issue 183: W3663 must not fire when a Lambda Permission's literal + SourceArn carries a proper 12-digit account id — that ARN pins the calling + account by itself. The issue's original ARN had a 10-digit account field, + which is not a valid AWS account id: it fails the ARN pattern (F3031) and + cannot pin the account, so W3663 fires for it — matching cfn-lint, whose + trigger is the same ':\d{12}:' pattern. +Resources: + MyLambda: + Type: AWS::Lambda::Function + Properties: + Code: + ZipFile: "def handler(event, context): pass" + Handler: index.handler + Role: !Sub arn:${AWS::Partition}:iam::${AWS::AccountId}:role/lambda-role + Runtime: python3.12 + # Valid 12-digit account id in the ARN — pins the account, no W3663. + PermissionWithAccountId: + Type: AWS::Lambda::Permission + Properties: + Action: lambda:InvokeFunction + FunctionName: !GetAtt MyLambda.Arn + Principal: my-service + SourceArn: arn:aws:sqs:us-east-1:111111111111:MyQueue + # The ARN exactly as reported in the issue: a 10-digit account field is not + # a valid account id, so the ARN cannot pin the account — W3663 fires (and + # the ARN also fails the schema pattern), matching cfn-lint. + PermissionInvalidAccountId: + Type: AWS::Lambda::Permission + Properties: + Action: lambda:InvokeFunction + FunctionName: !GetAtt MyLambda.Arn + Principal: my-service + SourceArn: arn:aws:sqs:us-east-1:1111111111:MyQueue diff --git a/src/resources/templates/good/number_param_integer_enum.yaml b/src/resources/templates/good/number_param_integer_enum.yaml new file mode 100644 index 0000000..035a853 --- /dev/null +++ b/src/resources/templates/good/number_param_integer_enum.yaml @@ -0,0 +1,30 @@ +AWSTemplateFormatVersion: "2010-09-09" +Description: > + A whole number reaching an integer enum must match regardless of its + written or resolved form: a Number parameter default (which resolves + through the parameter path) and a literal integral float (30.0) are both + the value 30, which is a valid LogGroup RetentionInDays. Neither form may + produce an enum (W3030) or type diagnostic. +Parameters: + RetentionInDays: + Type: Number + Default: 30 +Resources: + LogGroupFromParameter: + Type: AWS::Logs::LogGroup + Properties: + RetentionInDays: !Ref RetentionInDays + Tags: + - Key: purpose + Value: regression-test + UpdateReplacePolicy: Retain + DeletionPolicy: Retain + LogGroupLiteralFloat: + Type: AWS::Logs::LogGroup + Properties: + RetentionInDays: 30.0 + Tags: + - Key: purpose + Value: regression-test + UpdateReplacePolicy: Retain + DeletionPolicy: Retain diff --git a/src/template-model/src/coercion.rs b/src/template-model/src/coercion.rs index be13e93..3ff06e5 100644 --- a/src/template-model/src/coercion.rs +++ b/src/template-model/src/coercion.rs @@ -86,15 +86,22 @@ pub fn coerce_port_to_string(val: &Value) -> Option { /// Whether two values are equal under CloudFormation's loose scalar semantics. /// /// CloudFormation stringifies scalar values before comparison, so `512` and -/// `"512"`, or `true` and `"true"`, are equal. Two values match when they are -/// natively equal (covering arrays, objects, and null) or when both coerce to -/// the same string. Non-scalar values have no string coercion, so distinct -/// arrays/objects/nulls never collapse together — only native equality can make -/// them match. +/// `"512"`, or `true` and `"true"`, are equal. Numeric scalars compare +/// numerically when either operand is a native number, so `30`, `30.0`, and +/// `"30.0"` are all the same value — an integral float must never mismatch its +/// integer form just because they stringify differently. Two string operands +/// keep exact string comparison. Non-scalar values have no coercion, so +/// distinct arrays/objects/nulls never collapse together — only native +/// equality can make them match. pub fn scalar_eq(a: &Value, b: &Value) -> bool { if a == b { return true; } + if (a.is_number() || b.is_number()) + && let (Some(na), Some(nb)) = (coerce_to_number(a), coerce_to_number(b)) + { + return na == nb; + } match (coerce_to_string(a), coerce_to_string(b)) { (Some(sa), Some(sb)) => sa == sb, _ => false, @@ -327,6 +334,25 @@ mod tests { assert!(!scalar_eq(&json!(true), &json!("false"))); } + #[test] + fn scalar_eq_integral_float_equals_integer() { + // A whole number is the same value in every written form — + // 30, 30.0, and "30.0" must all compare equal (issue: a Number + // parameter resolving to 30.0 failed the integer enum [.., 30, ..]). + assert!(scalar_eq(&json!(30.0), &json!(30)), "30.0 must equal 30"); + assert!(scalar_eq(&json!(30), &json!(30.0)), "30 must equal 30.0"); + assert!(scalar_eq(&json!(30.0), &json!("30")), "30.0 must equal '30'"); + assert!(scalar_eq(&json!("30.0"), &json!(30)), "'30.0' must equal 30"); + assert!(!scalar_eq(&json!(30.5), &json!(30)), "30.5 must not equal 30"); + assert!(!scalar_eq(&json!(30.0), &json!(31)), "30.0 must not equal 31"); + } + + #[test] + fn scalar_eq_non_numeric_string_keeps_exact_comparison() { + assert!(!scalar_eq(&json!("30x"), &json!(30)), "'30x' is not a number and must not equal 30"); + assert!(!scalar_eq(&json!("tcp"), &json!("TCP")), "string comparison stays case-sensitive"); + } + #[test] fn scalar_eq_distinct_non_scalars_never_collapse() { // Arrays/objects/null have no string coercion; only native equality can diff --git a/src/template-model/src/resolver.rs b/src/template-model/src/resolver.rs index 460f5d7..c1f9cab 100644 --- a/src/template-model/src/resolver.rs +++ b/src/template-model/src/resolver.rs @@ -1410,12 +1410,18 @@ fn param_string_to_json(value: &str, param_type: &str) -> serde_json::Value { ); } match param_type { + // Parse whole numbers as integers so a Number parameter value of "30" + // resolves to 30, not 30.0 — the float form would fail integer enum + // comparisons and render as '30.0' in diagnostics. PARAM_TYPE_NUMBER => value - .parse::() - .map(|n| { - serde_json::Number::from_f64(n) - .map(serde_json::Value::Number) - .unwrap_or(serde_json::Value::String(value.to_string())) + .parse::() + .map(|i| serde_json::Value::Number(i.into())) + .or_else(|_| { + value.parse::().map(|n| { + serde_json::Number::from_f64(n) + .map(serde_json::Value::Number) + .unwrap_or(serde_json::Value::String(value.to_string())) + }) }) .unwrap_or(serde_json::Value::String(value.to_string())), _ => serde_json::Value::String(value.to_string()), @@ -2110,6 +2116,16 @@ mod tests { use super::*; use crate::parser; + #[test] + fn param_number_whole_value_resolves_to_integer() { + // A whole-number Number parameter must resolve to a JSON integer, not a + // float — 30.0 fails integer enum comparisons and renders as '30.0'. + assert_eq!(param_string_to_json("30", "Number"), serde_json::json!(30)); + assert!(param_string_to_json("30", "Number").is_i64(), "whole number must be an integer"); + assert_eq!(param_string_to_json("1.5", "Number"), serde_json::json!(1.5)); + assert_eq!(param_string_to_json("not-a-number", "Number"), serde_json::json!("not-a-number")); + } + #[test] fn resolve_ref_param_with_allowed_values() { let input = r#"{"Parameters":{"Env":{"Type":"String","AllowedValues":["dev","prod"]}},"Resources":{"R":{"Type":"T","Properties":{"V":{"Ref":"Env"}}}}}"#; From c83ac8b7fce897ebfdbf0e9c5792ba6277f1a2cc Mon Sep 17 00:00:00 2001 From: Satyaki Ghosh Date: Mon, 20 Jul 2026 18:02:51 -0400 Subject: [PATCH 3/4] move templates --- src/cfn-validate/tests/github_issues.rs | 42 +++++++++++++++++++ src/resources/expected/all_templates.json | 38 ++++++++--------- .../issue-185.yaml} | 0 3 files changed, 61 insertions(+), 19 deletions(-) rename src/resources/templates/{good/number_param_integer_enum.yaml => gh-issues/issue-185.yaml} (100%) diff --git a/src/cfn-validate/tests/github_issues.rs b/src/cfn-validate/tests/github_issues.rs index 2f0b516..a029ed2 100644 --- a/src/cfn-validate/tests/github_issues.rs +++ b/src/cfn-validate/tests/github_issues.rs @@ -840,6 +840,48 @@ Resources: assert_count(&diags, "W9009", 1); } +/// Issue #185: a whole-number `Number` parameter default (30) reaching an +/// integer enum (LogGroup RetentionInDays) used to resolve as the float 30.0 +/// and fail the enum comparison ("30.0 is not one of [1, 3, ..., 30, ...]"). +/// Whole numbers now stay integers through the parameter path and numeric +/// scalars compare numerically, so W3030 must not fire on either the +/// parameter-sourced or the literal 30.0 form — cfn-lint is clean on both. +/// Handled in template-model, so both engines agree. +/// https://github.com/aws-cloudformation/cloudformation-validate/issues/185 +#[test] +fn issue_185_no_w3030_on_number_parameter_default() { + let diags = validate_both("issue-185.yaml"); + assert_absent(&diags, "W3030"); + // The float-vs-integer confusion must not surface as a type finding either. + assert_absent(&diags, "F3012"); + assert_absent(&diags, "W9003"); +} + +/// Issue #185 (positive boundary): a genuinely invalid retention value coming +/// through the same Number-parameter path must still fire W3030 — the numeric +/// equality fix must not silence real enum violations. +/// https://github.com/aws-cloudformation/cloudformation-validate/issues/185 +#[test] +fn issue_185_w3030_still_fires_on_invalid_retention_via_parameter() { + const TEMPLATE: &[u8] = br#" +AWSTemplateFormatVersion: "2010-09-09" +Parameters: + RetentionInDays: + Type: Number + Default: 31 +Resources: + LogGroup: + Type: AWS::Logs::LogGroup + Properties: + RetentionInDays: !Ref RetentionInDays + UpdateReplacePolicy: Retain + DeletionPolicy: Retain +"#; + let diags = validate_both_bytes(TEMPLATE); + assert_fires_with_severity(&diags, "W3030", Severity::Warn); + assert_count(&diags, "W3030", 1); +} + /// Issue #183: W3663 must not fire when a Lambda Permission's literal SourceArn /// carries a proper 12-digit account id — that ARN pins the calling account by /// itself. The issue's original ARN had a 10-digit account field, which is not diff --git a/src/resources/expected/all_templates.json b/src/resources/expected/all_templates.json index 3285aa2..dda0db1 100644 --- a/src/resources/expected/all_templates.json +++ b/src/resources/expected/all_templates.json @@ -105687,6 +105687,25 @@ } ] }, + "gh-issues/issue-185.yaml": { + "filePath": "gh-issues/issue-185.yaml", + "status": "OK", + "version": "1.6.0", + "metadata": { + "resourcesScanned": 2, + "counts": { + "fatal": 0, + "errors": 0, + "warnings": 0, + "informational": 0, + "debug": 0 + }, + "suppressed": 0, + "strict": false, + "severityLevel": "DEBUG" + }, + "diagnostics": [] + }, "gh-issues/issue-34-w2506-overfire.json": { "filePath": "gh-issues/issue-34-w2506-overfire.json", "status": "OK", @@ -119000,25 +119019,6 @@ } ] }, - "good/number_param_integer_enum.yaml": { - "filePath": "good/number_param_integer_enum.yaml", - "status": "OK", - "version": "1.6.0", - "metadata": { - "resourcesScanned": 2, - "counts": { - "fatal": 0, - "errors": 0, - "warnings": 0, - "informational": 0, - "debug": 0 - }, - "suppressed": 0, - "strict": false, - "severityLevel": "DEBUG" - }, - "diagnostics": [] - }, "good/output_value_string.yaml": { "filePath": "good/output_value_string.yaml", "status": "OK", diff --git a/src/resources/templates/good/number_param_integer_enum.yaml b/src/resources/templates/gh-issues/issue-185.yaml similarity index 100% rename from src/resources/templates/good/number_param_integer_enum.yaml rename to src/resources/templates/gh-issues/issue-185.yaml From 410fd0a7375476ef743ae3ddf6de8b09c45b3081 Mon Sep 17 00:00:00 2001 From: Satyaki Ghosh Date: Mon, 20 Jul 2026 18:05:11 -0400 Subject: [PATCH 4/4] format --- src/cel-engine/src/rules/resources_extra.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/cel-engine/src/rules/resources_extra.rs b/src/cel-engine/src/rules/resources_extra.rs index bb95d60..fbd0794 100644 --- a/src/cel-engine/src/rules/resources_extra.rs +++ b/src/cel-engine/src/rules/resources_extra.rs @@ -319,9 +319,7 @@ pub fn eval_extra_resources(ctx: &EvalContext) -> Vec { // `Custom::` resources, modules, hook-shaped names) may be // registered per account/region, so they are skipped entirely // rather than guessed at. - if res.resource_type.starts_with("AWS::") - && !ctx.cached_data.known_types.contains(&res.resource_type) - { + if res.resource_type.starts_with("AWS::") && !ctx.cached_data.known_types.contains(&res.resource_type) { out.push(make_resource_diagnostic( "F3006", &format!("Unknown resource type '{}'", res.resource_type),