Skip to content

Commit eb8483a

Browse files
authored
fix: isolate aliased configuration mappings (#438)
Fixes #407. Copies nested configuration containers so shared YAML aliases cannot make unrelated effective values mutate together. Tests: tests/test_batteries_included_config.py.
1 parent 9d60142 commit eb8483a

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

‎lib/python/base_cli/config.py‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,22 @@ def _merge_mapping_validated(
176176
for existing_path in tuple(provenance):
177177
if existing_path == path or existing_path.startswith(f"{path}."):
178178
del provenance[existing_path]
179-
target[key] = dict(value) if isinstance(value, Mapping) else value
179+
target[key] = _copy_config_value(value)
180180
provenance.update(_leaf_provenance(value, source, path))
181181

182182

183+
def _copy_config_value(value: Any) -> Any:
184+
"""Copy nested configuration containers without preserving YAML aliases."""
185+
186+
if isinstance(value, Mapping):
187+
return {key: _copy_config_value(child) for key, child in value.items()}
188+
if isinstance(value, list):
189+
return [_copy_config_value(child) for child in value]
190+
if isinstance(value, tuple):
191+
return tuple(_copy_config_value(child) for child in value)
192+
return value
193+
194+
183195
class BatteriesIncludedConfigLoader:
184196
"""Load conventional user, project, environment, and explicit layers.
185197

‎tests/test_batteries_included_config.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ def test_shared_mapping_alias_is_valid_but_recursive_alias_is_rejected_with_path
6161
provenance: dict[str, str] = {}
6262
_merge_mapping(values, provenance, valid, "user")
6363
self.assertEqual(values, {"left": shared, "right": shared})
64+
self.assertIsNot(values["left"], shared)
65+
self.assertIsNot(values["left"], values["right"])
66+
assert isinstance(values["left"], dict)
67+
values["left"]["answer"] = 7
68+
self.assertEqual(values["right"], {"answer": 42})
6469
self.assertEqual(provenance, {"left.answer": "user", "right.answer": "user"})
6570

6671
with tempfile.TemporaryDirectory() as tmpdir:
@@ -69,6 +74,18 @@ def test_shared_mapping_alias_is_valid_but_recursive_alias_is_rejected_with_path
6974
with self.assertRaisesRegex(base_cli.ConfigurationError, "recursive.yaml.*recursive value.*nested.child"):
7075
BatteriesIncludedConfigLoader(user_config_dir=Path(tmpdir) / "user").load(None, path)
7176

77+
def test_nested_mapping_aliases_are_isolated_across_layers(self) -> None:
78+
shared = {"limits": {"retries": 2}}
79+
values: dict[str, object] = {}
80+
provenance: dict[str, str] = {}
81+
82+
_merge_mapping(values, provenance, {"first": shared, "second": shared}, "user")
83+
_merge_mapping(values, provenance, {"first": {"limits": {"timeout": 30}}}, "project")
84+
85+
self.assertEqual(values["first"], {"limits": {"retries": 2, "timeout": 30}})
86+
self.assertEqual(values["second"], {"limits": {"retries": 2}})
87+
self.assertEqual(shared, {"limits": {"retries": 2}})
88+
7289
def test_mapping_depth_is_bounded_before_recursive_merge_or_provenance(self) -> None:
7390
nested: dict[str, object] = {"value": 1}
7491
for index in range(65):

0 commit comments

Comments
 (0)