fix(engine): harden YAML scalar normalization

Signed-off-by: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com>
(cherry picked from commit faddb58b9bca771e011280c40d6b9f9264fe90b9)

pkg/engine/funcs.go

@@ -19,6 +19,8 @@ package engine
 import (
 	"bytes"
 	"encoding/json"
+	"math"
+	"reflect"
 	"strings"
 	"text/template"
 
@@ -28,6 +30,8 @@ import (
 	goYaml "sigs.k8s.io/yaml/goyaml.v3"
 )
 
+const maxSafeYAMLInteger = 1 << 53
+
 // funcMap returns a mapping of all of the functions that Engine has.
 //
 // Because some functions are late-bound (e.g. contain context-sensitive
@@ -95,9 +99,12 @@ func toYAMLPretty(v interface{}) string {
 	var data bytes.Buffer
 	encoder := goYaml.NewEncoder(&data)
 	encoder.SetIndent(2)
-	err := encoder.Encode(normalizeYAMLScalars(v))
 
-	if err != nil {
+	if err := encoder.Encode(normalizeYAMLScalars(v)); err != nil {
+		// Swallow errors inside of a template.
+		return ""
+	}
+	if err := encoder.Close(); err != nil {
 		// Swallow errors inside of a template.
 		return ""
 	}
@@ -115,7 +122,7 @@ func normalizeYAMLScalars(v any) any {
 	case map[any]any:
 		normalized := make(map[any]any, len(typedValue))
 		for key, value := range typedValue {
-			normalized[key] = normalizeYAMLScalars(value)
+			normalized[normalizeYAMLMapKey(key)] = normalizeYAMLScalars(value)
 		}
 		return normalized
 	case []any:
@@ -126,15 +133,24 @@ func normalizeYAMLScalars(v any) any {
 		return normalized
 	case float64:
 		// sigs.k8s.io/yaml may unmarshal integer YAML values as float64.
-		if typedValue == math.Trunc(typedValue) &&
-			typedValue > float64(math.MinInt64) &&
-			typedValue < float64(math.MaxInt64) {
+		if typedValue == math.Trunc(typedValue) && math.Abs(typedValue) <= maxSafeYAMLInteger {
 			return int64(typedValue)
 		}
 	}
 	return v
 }
 
+func normalizeYAMLMapKey(key any) any {
+	normalized := normalizeYAMLScalars(key)
+	if normalized == nil {
+		return normalized
+	}
+	if reflect.TypeOf(normalized).Comparable() {
+		return normalized
+	}
+	return key
+}
+
 // fromYAML converts a YAML document into a map[string]interface{}.
 //
 // This is not a general-purpose YAML parser, and will not parse all valid

pkg/engine/funcs_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package engine
 
 import (
+	"math"
 	"strings"
 	"testing"
 	"text/template"
@@ -141,6 +142,53 @@ keyInElement1 = "valueInElement1"`,
 	}
 }
 
+func TestNormalizeYAMLScalars(t *testing.T) {
+	aboveSafeInteger := math.Nextafter(maxSafeYAMLInteger, math.Inf(1))
+
+	tests := []struct {
+		name   string
+		input  any
+		expect any
+	}{
+		{
+			name:   "non-integer floats stay floats",
+			input:  map[string]any{"value": 1.5},
+			expect: map[string]any{"value": 1.5},
+		},
+		{
+			name:   "safe integer floats become integers",
+			input:  map[string]any{"value": 1.0},
+			expect: map[string]any{"value": int64(1)},
+		},
+		{
+			name:   "unsafe integer floats stay floats",
+			input:  map[string]any{"value": aboveSafeInteger},
+			expect: map[string]any{"value": aboveSafeInteger},
+		},
+		{
+			name: "map keys and nested values are normalized",
+			input: map[any]any{
+				float64(2): float64(3),
+				"nested": map[any]any{
+					float64(4): []any{float64(5)},
+				},
+			},
+			expect: map[any]any{
+				int64(2): int64(3),
+				"nested": map[any]any{
+					int64(4): []any{int64(5)},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.expect, normalizeYAMLScalars(tt.input))
+		})
+	}
+}
+
 // This test to check a function provided by sprig is due to a change in a
 // dependency of sprig. mergo in v0.3.9 changed the way it merges and only does
 // public fields (i.e. those starting with a capital letter). This test, from