Repository: aws-cloudformation/rain (go)

Issue:
Rain incorrectly strips quotes from string values that resemble YAML booleans, altering data types in CloudFormation templates
Rain currently strips quotes from string values even when doing so changes the data type in CloudFormation. YAML treats certain unquoted words (`ON`, `OFF`, `YES`, `NO`, etc.) as booleans, but CloudFormation expects them as strings in certain parameters (e.g., `require_secure_transport` in `AWS::RDS::DBClusterParameterGroup`). This behavior leads to unintended type mismatches and deployment failures.  

#### **Reproduction Steps**  
Given the following CloudFormation template:  
```yaml
Resources:
  DbClusterParameterGroupBlue:
    Type: AWS::RDS::DBClusterParameterGroup
    Properties:
      Description: "Aurora PG Cluster Parameter Group"
      Family: "aurora-mysql8.0"
      Parameters:
        require_secure_transport: "ON"
```
Running `rain fmt` or deploying via Rain strips the quotes:  
```yaml
Parameters:
  require_secure_transport: ON
```
This causes CloudFormation to interpret `ON` as a boolean (`true`) instead of a string (`"ON"`), resulting in deployment issues.  

#### **Why This is a Bug**  
1. **Stripping Quotes Here Alters the Meaning of the Data**  
   - Unlike normal YAML formatting, this is not just a stylistic change; it transforms a string into a boolean.  

2. **Unexpected and Subtle Behavior**  
   - This issue is difficult to debug because it happens at formatting time, and CloudFormation does not clearly indicate that the type changed due to YAML parsing.  

3. **Existing Workarounds are Insufficient**  
   - `--node-style original` is not a viable fix because:  
     - It affects the entire template, not just the problematic values.  
     - It prevents Rain from formatting other parts of the template where quotes should be stripped.  
     - It is an opt-in fix rather than a default behavior correction.  

#### **Suggested Fix**  
Rain should **preserve quotes around string values that match YAML’s boolean-like keywords** (`ON`, `OFF`, `YES`, `NO`, `TRUE`, `FALSE`, etc.) to prevent unintentional type changes. This would ensure that templates maintain their intended behavior without requiring a global formatting override.  

Related issue #480 

Pull request description:
Fix #653: incorrectly strips quotes from string values that resemble YAML booleans
*Issue #, if available:*
 #653

*Description of changes:*
Ensure ambiguous YAML scalars (`ON`, `OFF`, `YES`, `NO`, `TRUE`, `FALSE`) remain quoted to prevent unintended type conversion in CloudFormation templates. Includes tests to verify correct handling in `Parameters` and `Properties`.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.


Patch:
diff --git a/cft/format/transform.go b/cft/format/transform.go
index 79e30a90..ddbaea57 100644
--- a/cft/format/transform.go
+++ b/cft/format/transform.go
@@ -120,11 +120,25 @@ func formatNode(n *yaml.Node) *yaml.Node {
 			n.Style = yaml.DoubleQuotedStyle
 		}
 	case "":
-		// Default style for consistent formatting
-		n.Style = 0
+		// Default style for consistent formatting:
+		// Force double quotes on ambiguous scalar strings.
+		if n.Kind == yaml.ScalarNode && n.Tag == "!!str" && isAmbiguousScalar(n.Value) {
+			n.Style = yaml.DoubleQuotedStyle
+		} else {
+			n.Style = 0
+		}
 	default:
 		panic("invalid --node-style: " + NodeStyle)
 	}
 
 	return n
 }
+
+func isAmbiguousScalar(val string) bool {
+	switch strings.ToLower(val) {
+	case "y", "yes", "n", "no", "true", "false", "on", "off":
+		return true
+	default:
+		return false
+	}
+}


Test patch:
diff --git a/cft/format/format_test.go b/cft/format/format_test.go
index 9b23f391..4a09fc09 100644
--- a/cft/format/format_test.go
+++ b/cft/format/format_test.go
@@ -800,3 +800,108 @@ Outputs {
 		t.Fatalf("Got:\n[%s]\nExpected:\n[%s]\n", output, expected)
 	}
 }
+
+func TestAmbiguousScalarOn(t *testing.T) {
+	input := `
+Resources:
+  MyResource:
+    Type: AWS::RDS::DBClusterParameterGroup
+    Properties:
+      Parameters:
+        require_secure_transport: "ON"
+`
+	// We expect the ambiguous value "ON" to remain quoted in the output.
+	expected := `Resources:
+  MyResource:
+    Type: AWS::RDS::DBClusterParameterGroup
+    Properties:
+      Parameters:
+        require_secure_transport: "ON"
+`
+	template, err := parse.String(input)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	actual := format.String(template, format.Options{Unsorted: true})
+	if d := cmp.Diff(strings.TrimSpace(expected), strings.TrimSpace(actual)); d != "" {
+		t.Fatalf("Diff: %s", d)
+	}
+}
+
+func TestAmbiguousScalarsInParameters(t *testing.T) {
+	input := `
+Parameters:
+  Param1:
+    Default: "ON"
+  Param2:
+    Default: "OFF"
+  Param3:
+    Default: "Yes"
+  Param4:
+    Default: "No"
+  Param5:
+    Default: "True"
+  Param6:
+    Default: "False"
+  Param7:
+    Default: "Maybe"
+`
+	// The ambiguous values (Param1 through Param6) should be rendered with quotes,
+	// while a non-ambiguous value (Param7) may be unquoted.
+	expected := `Parameters:
+  Param1:
+    Default: "ON"
+
+  Param2:
+    Default: "OFF"
+
+  Param3:
+    Default: "Yes"
+
+  Param4:
+    Default: "No"
+
+  Param5:
+    Default: "True"
+
+  Param6:
+    Default: "False"
+
+  Param7:
+    Default: Maybe
+`
+	template, err := parse.String(input)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	actual := format.String(template, format.Options{Unsorted: true})
+	if d := cmp.Diff(strings.TrimSpace(expected), strings.TrimSpace(actual)); d != "" {
+		t.Fatalf("Diff: %s", d)
+	}
+}
+
+func TestNonAmbiguousScalar(t *testing.T) {
+	input := `
+Resources:
+  MyResource:
+    Properties:
+      example_value: "OnX"
+`
+	// Since "OnX" is not an ambiguous token, it can be rendered without quotes.
+	expected := `Resources:
+  MyResource:
+    Properties:
+      example_value: OnX
+`
+	template, err := parse.String(input)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	actual := format.String(template, format.Options{Unsorted: true})
+	if d := cmp.Diff(strings.TrimSpace(expected), strings.TrimSpace(actual)); d != "" {
+		t.Fatalf("Diff: %s", d)
+	}
+}