fix: replace YAML-based DeepCopy with recursive deep copy to fix #973 (#2675)

Environment.DeepCopy() used a YAML marshal/unmarshal round-trip to copy
values. When SOPS/KMS-encrypted secret files contained values with special
characters (colons, quotes, braces such as ~masked:ab#7i7!;{'".), the
YAML round-trip could mangle or silently drop adjacent keys, producing
the "map has no entry for key" error reported in #973.

Replace the YAML-based DeepCopy with maputil.DeepCopyMap(), a proper
recursive deep copy that:
- Preserves original Go types (string "true" stays string, not bool)
- Never loses data due to special characters in values
- Normalises map[any]any keys to strings (matching CastKeysToStrings)

Signed-off-by: yxxhero <aiopsclub@163.com>

pkg/environment/environment.go

@@ -2,7 +2,6 @@ package environment
 
 import (
 	"github.com/helmfile/helmfile/pkg/maputil"
-	"github.com/helmfile/helmfile/pkg/yaml"
 )
 
 type Environment struct {
@@ -32,52 +31,18 @@ func New(name string) *Environment {
 	}
 }
 
+// DeepCopy returns a deep copy of the environment.
+// It uses maputil.DeepCopyMap rather than a YAML marshal/unmarshal round-trip
+// so that secret values containing special characters (colons, quotes, braces,
+// etc.) are never mangled or lost. This fixes issue #973 where large SOPS-encrypted
+// secret files caused environment values like "myValue" to silently disappear.
 func (e Environment) DeepCopy() Environment {
-	valuesBytes, err := yaml.Marshal(e.Values)
-	if err != nil {
-		panic(err)
-	}
-	var values map[string]any
-	if err := yaml.Unmarshal(valuesBytes, &values); err != nil {
-		panic(err)
-	}
-	values, err = maputil.CastKeysToStrings(values)
-	if err != nil {
-		panic(err)
-	}
-
-	defaultsBytes, err := yaml.Marshal(e.Defaults)
-	if err != nil {
-		panic(err)
-	}
-	var defaults map[string]any
-	if err := yaml.Unmarshal(defaultsBytes, &defaults); err != nil {
-		panic(err)
-	}
-	defaults, err = maputil.CastKeysToStrings(defaults)
-	if err != nil {
-		panic(err)
-	}
-
-	cliOverridesBytes, err := yaml.Marshal(e.CLIOverrides)
-	if err != nil {
-		panic(err)
-	}
-	var cliOverrides map[string]any
-	if err := yaml.Unmarshal(cliOverridesBytes, &cliOverrides); err != nil {
-		panic(err)
-	}
-	cliOverrides, err = maputil.CastKeysToStrings(cliOverrides)
-	if err != nil {
-		panic(err)
-	}
-
 	return Environment{
 		Name:         e.Name,
 		KubeContext:  e.KubeContext,
-		Values:       values,
-		Defaults:     defaults,
-		CLIOverrides: cliOverrides,
+		Values:       maputil.DeepCopyMap(e.Values),
+		Defaults:     maputil.DeepCopyMap(e.Defaults),
+		CLIOverrides: maputil.DeepCopyMap(e.CLIOverrides),
 	}
 }
 

pkg/environment/environment_test.go

@@ -237,3 +237,71 @@ func TestEnvironment_GetMergedValues_Issue2527_ValuesOverrideDefaults(t *testing
 	assert.Equal(t, true, hd["wait"])
 	assert.Equal(t, 300, hd["timeout"])
 }
+
+// TestEnvironment_DeepCopy_Issue973_SecretSpecialChars verifies that DeepCopy
+// preserves all keys when values contain special characters (colons, quotes,
+// braces, etc.) typical of SOPS/KMS-encrypted secrets.
+// Regression test for https://github.com/helmfile/helmfile/issues/973.
+func TestEnvironment_DeepCopy_Issue973_SecretSpecialChars(t *testing.T) {
+	env := &Environment{
+		Name: "myEnv",
+		Values: map[string]any{
+			"masked1": map[string]any{
+				"masked2": "xxxxxxxxxx",
+				"masked3": "xxxxxxxxxx",
+			},
+			"masked85": map[string]any{
+				"masked86": map[string]any{"masked87": "xxxxxxxxxx"},
+				"masked88": map[string]any{"masked89": "~masked:ab#7i7!;{'\"."},
+			},
+			// myValue must survive the deep copy alongside the secret values.
+			"myValue": "valueOfMyValue",
+		},
+		Defaults: map[string]any{
+			"aDependentValue": "{{.Values.myValue}}",
+		},
+	}
+
+	copied := env.DeepCopy()
+
+	// myValue must be preserved.
+	assert.Equal(t, "valueOfMyValue", copied.Values["myValue"], "myValue must survive DeepCopy")
+	// The secret value must be preserved exactly, not mangled.
+	masked85 := copied.Values["masked85"].(map[string]any)
+	masked88 := masked85["masked88"].(map[string]any)
+	assert.Equal(t, "~masked:ab#7i7!;{'\".", masked88["masked89"],
+		"secret value with special chars must survive DeepCopy unchanged")
+	// Defaults must be preserved too.
+	assert.Equal(t, "{{.Values.myValue}}", copied.Defaults["aDependentValue"])
+
+	// Mutating the copy must not affect the original.
+	copied.Values["myValue"] = "changed"
+	assert.Equal(t, "valueOfMyValue", env.Values["myValue"], "DeepCopy must be independent")
+}
+
+// TestEnvironment_Merge_Issue973_NoDataLoss verifies that Merge (which calls
+// DeepCopy internally) does not drop any keys when secret values with special
+// characters are present.
+func TestEnvironment_Merge_Issue973_NoDataLoss(t *testing.T) {
+	base := &Environment{
+		Name:   "myEnv",
+		Values: map[string]any{},
+	}
+
+	loaded := &Environment{
+		Name: "myEnv",
+		Values: map[string]any{
+			"masked1":  map[string]any{"masked2": "xxxxxxxxxx"},
+			"masked89": "~masked:ab#7i7!;{'\".",
+			"myValue":  "valueOfMyValue",
+		},
+	}
+
+	merged, err := base.Merge(loaded)
+	require.NoError(t, err)
+
+	assert.Equal(t, "valueOfMyValue", merged.Values["myValue"],
+		"myValue must survive Merge with secret values present")
+	assert.Equal(t, "~masked:ab#7i7!;{'\".", merged.Values["masked89"],
+		"secret value must survive Merge unchanged")
+}

pkg/maputil/maputil.go

@@ -39,6 +39,44 @@ func CastKeysToStrings(s any) (map[string]any, error) {
 	return new, nil
 }
 
+// DeepCopyMap creates a deep copy of a map[string]any, recursively copying all
+// nested maps and slices. Unlike YAML marshal/unmarshal round-trips, it preserves
+// the original Go types of all values (e.g. the string "true" stays a string,
+// not a bool) and never loses data due to special characters in values.
+// It also normalises map[any]any keys to strings, matching CastKeysToStrings.
+func DeepCopyMap(m map[string]any) map[string]any {
+	if m == nil {
+		return nil
+	}
+	result := make(map[string]any, len(m))
+	for k, v := range m {
+		result[k] = deepCopyValue(v)
+	}
+	return result
+}
+
+func deepCopyValue(v any) any {
+	switch val := v.(type) {
+	case map[string]any:
+		return DeepCopyMap(val)
+	case map[any]any:
+		copied := make(map[string]any, len(val))
+		for k, item := range val {
+			copied[fmt.Sprintf("%v", k)] = deepCopyValue(item)
+		}
+		return copied
+	case []any:
+		copied := make([]any, len(val))
+		for i, item := range val {
+			copied[i] = deepCopyValue(item)
+		}
+		return copied
+	default:
+		// Primitives (string, int, bool, nil, float, etc.) are copied by value.
+		return val
+	}
+}
+
 func RecursivelyStringifyMapKey(v any) (any, error) {
 	var castedV any
 	switch typedV := v.(type) {

pkg/maputil/maputil_test.go

@@ -893,3 +893,114 @@ func TestMergeMaps_ArrayStrategies(t *testing.T) {
 		}
 	})
 }
+
+// TestDeepCopyMap_Issue973 verifies that DeepCopyMap preserves all values
+// exactly, including strings with special characters that would break a
+// YAML marshal/unmarshal round-trip.
+// Regression test for https://github.com/helmfile/helmfile/issues/973.
+func TestDeepCopyMap_Issue973(t *testing.T) {
+	tests := []struct {
+		name  string
+		input map[string]any
+	}{
+		{
+			name: "special_chars_from_sops_secret",
+			input: map[string]any{
+				"masked85": map[string]any{
+					"masked88": map[string]any{
+						"masked89": "~masked:ab#7i7!;{'\".",
+					},
+				},
+				"myValue": "valueOfMyValue",
+			},
+		},
+		{
+			name: "yaml_ambiguous_strings_stay_strings",
+			input: map[string]any{
+				"yes_val":   "yes",
+				"null_val":  "null",
+				"num_str":   "007",
+				"colon_val": "key: value",
+				"brace_val": "{flow}",
+				"pipe_val":  "|block",
+			},
+		},
+		{
+			name: "deeply_nested_structures",
+			input: map[string]any{
+				"l1": map[string]any{
+					"l2": map[string]any{
+						"l3": map[string]any{
+							"l4": map[string]any{
+								"l5": "deep_value",
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "arrays_with_maps",
+			input: map[string]any{
+				"items": []any{
+					map[string]any{"name": "a", "value": "1"},
+					map[string]any{"name": "b", "value": "2"},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			copied := DeepCopyMap(tt.input)
+			if !reflect.DeepEqual(tt.input, copied) {
+				t.Errorf("DeepCopyMap changed data.\nExpected: %v\nGot: %v", tt.input, copied)
+			}
+		})
+	}
+}
+
+// TestDeepCopyMap_Independence verifies that mutating the copy does not affect
+// the original (i.e. it is a true deep copy, not a shallow one).
+func TestDeepCopyMap_Independence(t *testing.T) {
+	original := map[string]any{
+		"nested": map[string]any{"key": "value"},
+		"array":  []any{map[string]any{"k": "v"}},
+	}
+
+	copied := DeepCopyMap(original)
+
+	// Mutate nested map
+	copied["nested"].(map[string]any)["key"] = "changed"
+	if original["nested"].(map[string]any)["key"] != "value" {
+		t.Error("DeepCopyMap: mutating nested map in copy affected original")
+	}
+
+	// Mutate array element map
+	copied["array"].([]any)[0].(map[string]any)["k"] = "changed2"
+	if original["array"].([]any)[0].(map[string]any)["k"] != "v" {
+		t.Error("DeepCopyMap: mutating array element in copy affected original")
+	}
+}
+
+// TestDeepCopyMap_AnyKeysNormalises verifies that map[any]any keys are
+// normalised to strings during the deep copy.
+func TestDeepCopyMap_AnyKeysNormalises(t *testing.T) {
+	// Simulate what yaml v2 produces: map[any]any with int keys.
+	input := map[string]any{
+		"mixed": map[any]any{
+			1:     "int_key",
+			"str": "str_key",
+		},
+	}
+
+	copied := DeepCopyMap(input)
+	mixed := copied["mixed"].(map[string]any)
+
+	if _, ok := mixed["1"]; !ok {
+		t.Errorf("int key 1 should have been normalised to string key \"1\"; got: %v", mixed)
+	}
+	if mixed["str"] != "str_key" {
+		t.Errorf("string key should be preserved; got: %v", mixed)
+	}
+}