fix: validate nested configuration value graphs - #365
codeforester wants to merge 2 commits into
Conversation
| if exiting: | ||
| active.remove(identity) | ||
| continue | ||
| if not isinstance(current, (Mapping, list, tuple)): |
There was a problem hiding this comment.
Correctness (reproduced): _CONFIG_MAX_NODES in _validate_config_graph only increments visited_nodes for Mapping/list/tuple nodes, so a flat structure made almost entirely of scalar leaves never trips the 100,000-node limit. Reproduced: a single top-level mapping with 5,000,000 scalar leaf keys validated in ~1.8s with zero errors, even though the PR's stated goal is to reject excessive size/traversal cost. This defeats the guard for the most common shape of a bloated config.
| return {} | ||
| if not isinstance(data, dict): | ||
| raise ConfigurationError(f"Config file '{path}' must contain a YAML mapping.") | ||
| _validate_config_graph(data, source=f"Config file '{path}'") |
There was a problem hiding this comment.
Bug (message quality): load_yaml_file builds source=f"Config file '{path}'" and _validate_config_graph wraps that again as f"Configuration source '{source}' ...", producing a doubled-quote error like Configuration source 'Config file '/tmp/x.yaml'' has a non-string key... — reads like a bug in the CLI's own output rather than deliberate nested attribution.
| raise ConfigurationError(f"Unable to read config file '{path}': {exc}") from exc | ||
| try: | ||
| data = yaml.safe_load(contents) | ||
| except RecursionError as exc: |
There was a problem hiding this comment.
Test gap: the new except RecursionError handler in load_yaml_file isn't exercised by any test — the PR's deepest test config (65 levels) is caught earlier by _validate_config_graph's own _CONFIG_MAX_DEPTH check, never reaching PyYAML's parser far enough to raise RecursionError. A future change to the depth limit or parser could silently regress this path with nothing to catch it.
1 participant
Summary
Closes #359
Validation
uv run --extra dev --extra typer --extra quality python -m pytest tests/test_batteries_included_config.py tests/test_explicit_config_validation.py -quv run --extra dev --extra typer --extra quality python -m mypy --strict lib/python/base_cli/config.pygit diff --check