Catch malformed config errors gracefully

src/jinjaturtle/cli.py

@@ -12,6 +12,7 @@ from .core import (
     flatten_config,
     generate_ansible_yaml,
     generate_jinja2_template,
+    ConfigParseError,
 )
 
 from .multi import process_directory
@@ -76,6 +77,12 @@ def _main(argv: list[str] | None = None) -> int:
     except OutputPathError as exc:
         print(f"jinjaturtle: refusing unsafe output path: {exc}", file=sys.stderr)
         return 2
+    except ConfigParseError as exc:
+        # The source file could not be parsed as its (detected or forced)
+        # format. This is expected for malformed/attacker-influenced input;
+        # fail cleanly with a non-zero exit code instead of a traceback.
+        print(f"jinjaturtle: {exc}", file=sys.stderr)
+        return 1
 
 
 def _run(argv: list[str] | None = None) -> int:

src/jinjaturtle/core.py

@@ -305,6 +305,66 @@ def detect_format(path: Path, explicit: str | None = None) -> str:
     return "ini"
 
 
+class ConfigParseError(Exception):
+    """Raised when a source config file cannot be parsed as its format.
+
+    Each underlying parser (json, tomllib, PyYAML, defusedxml/ElementTree,
+    configparser) raises its own exception type on malformed input. Without a
+    single normalised error, a malformed file -- which is entirely expected when
+    JinjaTurtle is pointed at harvested, attacker-influenceable config -- would
+    escape as an unhandled traceback (e.g. ``xml.etree.ElementTree.ParseError``
+    on an XML file whose element name is not well-formed). ``parse_config``
+    converts every such failure into this one type so the CLI can fail closed
+    with a clean message and a non-zero exit code, and so library callers (such
+    as Enroll, which falls back to copying the raw file) have a single, stable
+    exception to catch.
+
+    Note: defusedxml's *security* exceptions (``EntitiesForbidden``,
+    ``DTDForbidden``, ...) are intentionally NOT folded into this type. They
+    signal an attempted XXE/entity-expansion attack rather than a benign
+    malformed file, and must propagate unchanged so callers can tell the two
+    apart.
+    """
+
+
+def _build_malformed_config_errors() -> tuple[type[BaseException], ...]:
+    """Return the concrete "this file is malformed" exception types to catch.
+
+    Deliberately specific. In particular we must avoid catching plain
+    ``ValueError``: defusedxml's ``EntitiesForbidden``/``DTDForbidden`` subclass
+    ``ValueError``, and those are security signals that must NOT be swallowed.
+    """
+
+    import configparser
+    import json
+    from xml.etree.ElementTree import ParseError as _XMLParseError  # nosec
+
+    import yaml as _yaml
+
+    errs: list[type[BaseException]] = [
+        _XMLParseError,
+        json.JSONDecodeError,
+        configparser.Error,
+        _yaml.YAMLError,
+        UnicodeDecodeError,
+    ]
+    try:
+        import tomllib
+
+        errs.append(tomllib.TOMLDecodeError)
+    except ModuleNotFoundError:  # pragma: no cover - Python < 3.11 fallback
+        try:
+            import tomli  # type: ignore
+
+            errs.append(tomli.TOMLDecodeError)
+        except ModuleNotFoundError:
+            pass
+    return tuple(errs)
+
+
+_MALFORMED_CONFIG_ERRORS = _build_malformed_config_errors()
+
+
 def parse_config(path: Path, fmt: str | None = None) -> tuple[str, Any]:
     """
     Parse config file into a Python object.
@@ -313,7 +373,24 @@ def parse_config(path: Path, fmt: str | None = None) -> tuple[str, Any]:
     handler = _HANDLERS.get(fmt)
     if handler is None:
         raise ValueError(f"Unsupported config format: {fmt}")
-    parsed = handler.parse(path)
+    try:
+        parsed = handler.parse(path)
+    except ConfigParseError:
+        raise
+    except _MALFORMED_CONFIG_ERRORS as exc:
+        # Normalise the per-parser "this file is malformed" errors into one
+        # type: json.JSONDecodeError / tomllib.TOMLDecodeError (ValueError
+        # subclasses), PyYAML's YAMLError, configparser.Error, and
+        # xml.etree.ElementTree.ParseError (raised by defusedxml on XML whose
+        # structure/element name is not well-formed). A bad input file is
+        # expected when parsing harvested config, so fail closed with a clean
+        # error instead of an unhandled traceback.
+        #
+        # IMPORTANT: this deliberately does NOT catch defusedxml's security
+        # exceptions (EntitiesForbidden, DTDForbidden, ...). Those signal an
+        # 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)
 

src/jinjaturtle/multi.py

@@ -25,13 +25,20 @@ from copy import deepcopy
 import os
 import configparser
 import stat
+import sys
 from dataclasses import dataclass
 from pathlib import Path
 from typing import Any, Iterable
 import xml.etree.ElementTree as ET  # nosec
 
 from . import j2
-from .core import dump_yaml, flatten_config, make_var_name, parse_config
+from .core import (
+    dump_yaml,
+    flatten_config,
+    make_var_name,
+    parse_config,
+    ConfigParseError,
+)
 from .handlers.xml import XmlHandler
 from .safety import verify_jinja2_template_safe, verify_no_live_jinja_in_json_keys
 from .escape import escape_jinja_literal
@@ -631,7 +638,13 @@ def process_directory(
     # Parse and group by format
     grouped: dict[str, list[tuple[Path, Any]]] = defaultdict(list)
     for p in files:
-        fmt, parsed = parse_config(p, None)
+        try:
+            fmt, parsed = parse_config(p, None)
+        except ConfigParseError as exc:
+            # One malformed file should not abort processing of an entire
+            # directory. Skip it with a warning; the rest still generate.
+            print(f"jinjaturtle: skipping {p}: {exc}", file=sys.stderr)
+            continue
         if fmt not in FOLDER_SUPPORTED_FORMATS:
             # Directory mode only supports a subset of formats for now.
             continue

tests/test_config_parse_errors.py

@@ -0,0 +1,123 @@
+"""Regression tests for malformed-input handling (``ConfigParseError``).
+
+Bug: every parse-layer handler (xml/json/toml/yaml/ini) used to let its
+underlying parser's exception escape as an unhandled traceback when given a
+malformed file. Pointing JinjaTurtle (or Enroll, which calls it as a library)
+at harvested, attacker-influenceable config makes malformed input an entirely
+expected condition, so it must fail cleanly instead of crashing.
+
+These tests pin down that:
+
+  * ``parse_config`` raises the normalised ``ConfigParseError`` for malformed
+    XML/JSON/TOML/INI;
+  * defusedxml's *security* exceptions (XXE / ``EntitiesForbidden``) are NOT
+    swallowed by that normalisation -- they must still propagate so a caller can
+    distinguish "malformed" from "malicious";
+  * an unrelated error (e.g. the TOML "tomllib missing" ``RuntimeError``) is not
+    captured by the normalisation either;
+  * the CLI exits non-zero with a clean message (no traceback) on malformed
+    input; and
+  * folder mode skips an unparseable file instead of aborting the whole run.
+"""
+
+from __future__ import annotations
+
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+from jinjaturtle.core import ConfigParseError, parse_config
+
+
+def _run_cli(args: list[str]) -> subprocess.CompletedProcess[str]:
+    return subprocess.run(
+        [sys.executable, "-m", "jinjaturtle.cli", *args],
+        capture_output=True,
+        text=True,
+    )
+
+
+@pytest.mark.parametrize(
+    "fmt,filename,content",
+    [
+        # Element name is not well-formed XML -> expat ParseError historically
+        # escaped as an uncaught traceback.
+        ("xml", "bad.xml", "<root><{{tag}}/></root>"),
+        ("xml", "bad2.xml", "<root><unclosed></root>"),
+        ("json", "bad.json", "{not valid json"),
+        ("toml", "bad.toml", "x = = ="),
+        # A bare key with no value is invalid INI for configparser.
+        ("ini", "bad.ini", "[s]\nthis line has no equals and no colon\n"),
+    ],
+)
+def test_parse_config_raises_configparseerror_on_malformed(
+    tmp_path: Path, fmt: str, filename: str, content: str
+) -> None:
+    src = tmp_path / filename
+    src.write_text(content, encoding="utf-8")
+    with pytest.raises(ConfigParseError):
+        parse_config(src, fmt=fmt)
+
+
+def test_parse_config_does_not_swallow_xxe_security_exception(tmp_path: Path) -> None:
+    """defusedxml's EntitiesForbidden must propagate, not become ConfigParseError.
+
+    EntitiesForbidden subclasses ValueError, so a naive ``except ValueError`` /
+    ``except Exception`` in parse_config would mask an XXE attempt as a benign
+    "malformed file". The normalisation is deliberately scoped to exclude it.
+    """
+    from defusedxml.common import EntitiesForbidden
+
+    xxe = (
+        '<?xml version="1.0"?>\n'
+        '<!DOCTYPE foo [ <!ENTITY xxe SYSTEM "file:///etc/passwd"> ]>\n'
+        "<foo>&xxe;</foo>\n"
+    )
+    src = tmp_path / "xxe.xml"
+    src.write_text(xxe, encoding="utf-8")
+    with pytest.raises(EntitiesForbidden):
+        parse_config(src, fmt="xml")
+
+
+def test_parse_config_does_not_swallow_unrelated_runtimeerror(
+    tmp_path: Path, monkeypatch
+) -> None:
+    """A non-malformation error (tomllib missing) must propagate unchanged."""
+    import jinjaturtle.handlers.toml as toml_module
+
+    monkeypatch.setattr(toml_module, "tomllib", None)
+    src = tmp_path / "x.toml"
+    src.write_text('a = "b"\n', encoding="utf-8")
+    with pytest.raises(RuntimeError) as exc:
+        parse_config(src, fmt="toml")
+    assert "tomllib/tomli is required" in str(exc.value)
+
+
+def test_cli_fails_cleanly_on_malformed_xml(tmp_path: Path) -> None:
+    """The CLI must exit non-zero with a clean message, not a traceback."""
+    src = tmp_path / "bad.xml"
+    src.write_text("<root><{{tag}}/></root>", encoding="utf-8")
+    res = _run_cli([str(src), "-f", "xml", "-r", "demo"])
+    assert res.returncode != 0
+    # Clean, user-facing message -- not a Python traceback.
+    assert "could not parse" in res.stderr
+    assert "Traceback (most recent call last)" not in res.stderr
+
+
+def test_folder_mode_skips_unparseable_file(tmp_path: Path) -> None:
+    """One malformed file should not abort processing of the whole directory."""
+    from jinjaturtle.multi import process_directory
+
+    good = tmp_path / "good.json"
+    good.write_text('{ "host": "localhost" }', encoding="utf-8")
+    bad = tmp_path / "bad.json"
+    bad.write_text("{not valid json", encoding="utf-8")
+
+    defaults_yaml, outputs = process_directory(tmp_path, False, "demo")
+    # The good file still produced output despite the bad sibling: its content
+    # and source id appear, and a template was generated.
+    assert "localhost" in defaults_yaml
+    assert "good.json" in defaults_yaml
+    assert outputs