Skip to content

Commit c488b3c

Browse files
committed
return defensive deepcopy from GetDeploymentConfig to prevent mutation
1 parent dacc042 commit c488b3c

File tree

2 files changed

+66
-3
lines changed

2 files changed

+66
-3
lines changed

internal/operator-controller/config/config.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ func (c *Config) GetWatchNamespace() *string {
102102
// Returns nil if deploymentConfig is not set or is explicitly set to null.
103103
// The returned value is a generic map[string]any that can be marshaled to JSON
104104
// for validation or conversion to specific types (like v1alpha1.SubscriptionConfig).
105+
//
106+
// Returns a defensive deep copy so callers can't mutate the internal Config state.
105107
func (c *Config) GetDeploymentConfig() map[string]any {
106108
if c == nil || *c == nil {
107109
return nil
@@ -115,10 +117,26 @@ func (c *Config) GetDeploymentConfig() map[string]any {
115117
return nil
116118
}
117119
// Schema validation ensures this is an object (map)
118-
if dcMap, ok := val.(map[string]any); ok {
119-
return dcMap
120+
dcMap, ok := val.(map[string]any)
121+
if !ok {
122+
return nil
120123
}
121-
return nil
124+
125+
// Return a defensive deep copy so callers can't mutate the internal Config state.
126+
// We use JSON marshal/unmarshal because the data is already JSON-compatible and
127+
// this handles nested structures correctly.
128+
data, err := json.Marshal(dcMap)
129+
if err != nil {
130+
// This should never happen since the map came from validated JSON/YAML,
131+
// but return nil as a safe fallback
132+
return nil
133+
}
134+
var copied map[string]any
135+
if err := json.Unmarshal(data, &copied); err != nil {
136+
// This should never happen for valid JSON
137+
return nil
138+
}
139+
return copied
122140
}
123141

124142
// UnmarshalConfig takes user configuration, validates it, and creates a Config object.

internal/operator-controller/config/config_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,4 +651,49 @@ func Test_GetDeploymentConfig(t *testing.T) {
651651
result := cfg.GetDeploymentConfig()
652652
require.Nil(t, result)
653653
})
654+
655+
// Test that returned map is a defensive copy (mutations don't affect original)
656+
t.Run("returned map is defensive copy - mutations don't affect original", func(t *testing.T) {
657+
rawConfig := []byte(`{
658+
"deploymentConfig": {
659+
"nodeSelector": {
660+
"kubernetes.io/os": "linux"
661+
}
662+
}
663+
}`)
664+
665+
schema, err := bundle.GetConfigSchema()
666+
require.NoError(t, err)
667+
668+
cfg, err := config.UnmarshalConfig(rawConfig, schema, "")
669+
require.NoError(t, err)
670+
671+
// Get the deploymentConfig
672+
result1 := cfg.GetDeploymentConfig()
673+
require.NotNil(t, result1)
674+
675+
// Mutate the returned map
676+
result1["nodeSelector"] = map[string]any{
677+
"mutated": "value",
678+
}
679+
result1["newField"] = "added"
680+
681+
// Get deploymentConfig again - should be unaffected by mutations
682+
result2 := cfg.GetDeploymentConfig()
683+
require.NotNil(t, result2)
684+
685+
// Original values should be intact
686+
require.Equal(t, map[string]any{
687+
"nodeSelector": map[string]any{
688+
"kubernetes.io/os": "linux",
689+
},
690+
}, result2)
691+
692+
// New field should not exist
693+
_, exists := result2["newField"]
694+
require.False(t, exists)
695+
696+
// result1 should have the mutations
697+
require.Equal(t, "added", result1["newField"])
698+
})
654699
}

0 commit comments

Comments
 (0)