diff --git a/src/jinjaturtle/core.py b/src/jinjaturtle/core.py index 458dbdc..de7b552 100644 --- a/src/jinjaturtle/core.py +++ b/src/jinjaturtle/core.py @@ -375,6 +375,10 @@ def parse_config(path: Path, fmt: str | None = None) -> tuple[str, Any]: raise ValueError(f"Unsupported config format: {fmt}") try: parsed = handler.parse(path) + # Make sure datetime objects are treated as strings (TOML, YAML). This + # walks the parsed object recursively, so keep it inside the try where a + # RecursionError from a pathological structure is normalised below. + parsed = _stringify_timestamps(parsed) except ConfigParseError: raise except _MALFORMED_CONFIG_ERRORS as exc: @@ -391,8 +395,16 @@ def parse_config(path: Path, fmt: str | None = None) -> tuple[str, Any]: # attempted XXE/entity-expansion attack and must propagate unchanged so # callers (and tests) can distinguish "malformed" from "malicious". raise ConfigParseError(f"could not parse {path} as {fmt}: {exc}") from exc - # Make sure datetime objects are treated as strings (TOML, YAML) - parsed = _stringify_timestamps(parsed) + except RecursionError as exc: + # A deeply-nested or self-referential structure (e.g. a recursive YAML + # anchor) can exhaust the Python stack while walking the parsed object. + # The YAML handler already rejects reference cycles up front; this is a + # format-agnostic backstop so any such input fails closed with a clean + # message instead of a stack-overflow traceback. + raise ConfigParseError( + f"could not parse {path} as {fmt}: input is too deeply nested " + "or self-referential" + ) from exc return fmt, parsed diff --git a/src/jinjaturtle/handlers/yaml.py b/src/jinjaturtle/handlers/yaml.py index a1447bb..9a868a6 100644 --- a/src/jinjaturtle/handlers/yaml.py +++ b/src/jinjaturtle/handlers/yaml.py @@ -11,6 +11,36 @@ from ..loop_analyzer import is_safe_loop_field_key from ..loop_analyzer import LoopCandidate +def _reject_recursive_structure(obj: Any) -> None: + """Raise ``yaml.YAMLError`` if *obj* contains a reference cycle. + + A recursive YAML anchor (``a: &a [*a]``) produces a container that contains + itself. Every consumer in JinjaTurtle walks the parsed object depth-first, + so a cycle would raise ``RecursionError`` deep in unrelated code. Detect it + up front by tracking the ``id()`` of containers on the current descent path; + a repeat means a cycle. ``yaml.YAMLError`` is raised so ``parse_config`` + normalises it into a clean ``ConfigParseError`` like any other malformed + input, rather than surfacing a stack-overflow traceback. + """ + + on_path: set[int] = set() + + def walk(node: Any) -> None: + if isinstance(node, (dict, list)): + marker = id(node) + if marker in on_path: + raise yaml.YAMLError( + "recursive/self-referential YAML structure is not supported" + ) + on_path.add(marker) + children = node.values() if isinstance(node, dict) else node + for child in children: + walk(child) + on_path.discard(marker) + + walk(obj) + + class YamlHandler(DictLikeHandler): """ YAML handler that can generate both scalar templates and loop-based templates. @@ -21,7 +51,16 @@ class YamlHandler(DictLikeHandler): def parse(self, path: Path) -> Any: text = path.read_text(encoding="utf-8") - return yaml.safe_load(text) or {} + parsed = yaml.safe_load(text) or {} + # PyYAML's safe_load happily builds *recursive* structures from an anchor + # that references itself (e.g. ``a: &a [*a]``). Downstream flattening, + # timestamp-stringifying and template generation all walk the parsed + # object recursively and would blow the Python stack (RecursionError) on + # such input. JinjaTurtle is regularly pointed at harvested, + # attacker-influenceable config, so reject a self-referential document + # cleanly here rather than crashing later. + _reject_recursive_structure(parsed) + return parsed def generate_jinja2_template( self, diff --git a/tests/test_config_parse_errors.py b/tests/test_config_parse_errors.py index 01ba1ef..7f4c422 100644 --- a/tests/test_config_parse_errors.py +++ b/tests/test_config_parse_errors.py @@ -162,3 +162,44 @@ def test_folder_mode_skips_unparseable_file(tmp_path: Path) -> None: assert "localhost" in defaults_yaml assert "good.json" in defaults_yaml assert outputs + + +@pytest.mark.parametrize( + "content", + [ + "a: &a [*a]\n", # self-referential sequence alias + "root: &r\n child: *r\n", # self-referential mapping alias + ], +) +def test_recursive_yaml_is_rejected_cleanly(tmp_path: Path, content: str) -> None: + """A recursive YAML anchor must fail as a clean ConfigParseError. + + PyYAML's safe_load builds a self-referential object from a recursive anchor; + JinjaTurtle's downstream walks (flatten, timestamp-stringify, template gen) + would otherwise blow the stack with a RecursionError traceback. Harvested + config is attacker-influenceable, so this must fail closed. + """ + src = tmp_path / "cyclic.yaml" + src.write_text(content, encoding="utf-8") + with pytest.raises(ConfigParseError): + parse_config(src, "yaml") + + +def test_recursive_yaml_cli_exits_nonzero_without_traceback(tmp_path: Path) -> None: + src = tmp_path / "cyclic.yaml" + src.write_text("a: &a [*a]\n", encoding="utf-8") + res = _run_cli([str(src), "-f", "yaml", "-r", "role"]) + assert res.returncode != 0 + assert "Traceback" not in res.stderr + assert "RecursionError" not in res.stderr + + +def test_shared_noncyclic_yaml_aliases_still_work(tmp_path: Path) -> None: + """A shared (acyclic) anchor referenced multiple times is a DAG, not a + cycle, and must still parse and template normally.""" + src = tmp_path / "shared.yaml" + src.write_text("base: &b\n x: 1\na: *b\nc: *b\n", encoding="utf-8") + fmt, parsed = parse_config(src, "yaml") + assert fmt == "yaml" + assert parsed["a"] == {"x": 1} + assert parsed["c"] == {"x": 1}