fix(engine): harden YAML scalar edge cases

Signed-off-by: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com>

pkg/engine/funcs.go

@@ -19,6 +19,7 @@ package engine
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"math"
 	"reflect"
 	"strings"
@@ -30,7 +31,7 @@ import (
 	goYaml "sigs.k8s.io/yaml/goyaml.v3"
 )
 
-const maxSafeYAMLInteger = 1 << 53
+const maxSafeYAMLInteger = (1 << 53) - 1
 
 // funcMap returns a mapping of all of the functions that Engine has.
 //
@@ -99,12 +100,23 @@ func toYAMLPretty(v interface{}) string {
 	var data bytes.Buffer
 	encoder := goYaml.NewEncoder(&data)
 	encoder.SetIndent(2)
+	closeEncoder := func() error {
+		if encoder == nil {
+			return nil
+		}
+		err := encoder.Close()
+		encoder = nil
+		return err
+	}
+	defer func() {
+		_ = closeEncoder()
+	}()
 
 	if err := encoder.Encode(normalizeYAMLScalars(v)); err != nil {
 		// Swallow errors inside of a template.
 		return ""
 	}
-	if err := encoder.Close(); err != nil {
+	if err := closeEncoder(); err != nil {
 		// Swallow errors inside of a template.
 		return ""
 	}
@@ -148,7 +160,10 @@ func normalizeYAMLMapKey(key any) any {
 	if reflect.TypeOf(normalized).Comparable() {
 		return normalized
 	}
-	return key
+	if reflect.TypeOf(key).Comparable() {
+		return key
+	}
+	return fmt.Sprint(normalized)
 }
 
 // fromYAML converts a YAML document into a map[string]interface{}.

pkg/engine/funcs_test.go

@@ -160,11 +160,26 @@ func TestNormalizeYAMLScalars(t *testing.T) {
 			input:  map[string]any{"value": 1.0},
 			expect: map[string]any{"value": int64(1)},
 		},
+		{
+			name:   "max safe integer float becomes integer",
+			input:  map[string]any{"value": float64(maxSafeYAMLInteger)},
+			expect: map[string]any{"value": int64(maxSafeYAMLInteger)},
+		},
 		{
 			name:   "unsafe integer floats stay floats",
 			input:  map[string]any{"value": aboveSafeInteger},
 			expect: map[string]any{"value": aboveSafeInteger},
 		},
+		{
+			name:   "safe negative integer floats become integers",
+			input:  map[string]any{"value": -float64(maxSafeYAMLInteger)},
+			expect: map[string]any{"value": -int64(maxSafeYAMLInteger)},
+		},
+		{
+			name:   "unsafe negative 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{
@@ -189,6 +204,11 @@ func TestNormalizeYAMLScalars(t *testing.T) {
 	}
 }
 
+func TestNormalizeYAMLMapKey(t *testing.T) {
+	assert.Equal(t, int64(1), normalizeYAMLMapKey(float64(1)))
+	assert.Equal(t, "[1 key]", normalizeYAMLMapKey([]any{float64(1), "key"}))
+}
+
 // 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