From d77f523794a3c4a92d38a479c72c9470536b08a9 Mon Sep 17 00:00:00 2001 From: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com> Date: Sat, 23 May 2026 10:47:30 +0530 Subject: [PATCH] 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 | 28 ++++++++++++++++++----- pkg/engine/funcs_test.go | 48 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/pkg/engine/funcs.go b/pkg/engine/funcs.go index 5eef2a2b8..e1a12ea76 100644 --- a/pkg/engine/funcs.go +++ b/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 diff --git a/pkg/engine/funcs_test.go b/pkg/engine/funcs_test.go index c876dd318..8958c2352 100644 --- a/pkg/engine/funcs_test.go +++ b/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