diff --git a/src/jinjaturtle/cli.py b/src/jinjaturtle/cli.py
index bdabc18..65dfa72 100644
--- a/src/jinjaturtle/cli.py
+++ b/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:
diff --git a/src/jinjaturtle/core.py b/src/jinjaturtle/core.py
index d86992b..de34735 100644
--- a/src/jinjaturtle/core.py
+++ b/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)
diff --git a/src/jinjaturtle/handlers/base.py b/src/jinjaturtle/handlers/base.py
index 14aaec7..6be91f0 100644
--- a/src/jinjaturtle/handlers/base.py
+++ b/src/jinjaturtle/handlers/base.py
@@ -1,5 +1,6 @@
from __future__ import annotations
+import re
from pathlib import Path
from typing import Any, Iterable
@@ -57,8 +58,20 @@ class BaseHandler:
role_prefix_section_subsection_key
Sanitises parts to lowercase [a-z0-9_] and strips extras.
+
+ Consecutive separators are collapsed to a single underscore. This is
+ required for correctness, not just aesthetics: a source key such as
+ ``log..level`` or ``cache--size`` would otherwise sanitise to a name
+ containing a double underscore (``log__level``). The output safety gate
+ in ``safety.py`` deliberately rejects *any* ``__`` in a generated
+ identifier because ``__`` is the gateway to every Jinja2 SSTI gadget
+ (``__class__``/``__globals__``/...). Emitting a dunder here would make
+ JinjaTurtle's own gate reject JinjaTurtle's own placeholder, aborting
+ generation on entirely benign config. Collapsing runs keeps every
+ generated name a plain single-underscore-delimited identifier that the
+ gate accepts.
"""
- role_prefix = role_prefix.strip().lower()
+ role_prefix = re.sub(r"_+", "_", role_prefix.strip().lower())
clean_parts: list[str] = []
for part in path:
@@ -70,7 +83,9 @@ class BaseHandler:
cleaned_chars.append(c.lower())
else:
cleaned_chars.append("_")
- cleaned_part = "".join(cleaned_chars).strip("_")
+ # Collapse runs of underscores (from adjacent separators) to a
+ # single "_" so the result can never contain a forbidden "__".
+ cleaned_part = re.sub(r"_+", "_", "".join(cleaned_chars)).strip("_")
if cleaned_part:
clean_parts.append(cleaned_part)
diff --git a/src/jinjaturtle/multi.py b/src/jinjaturtle/multi.py
index c968d61..ce54e1c 100644
--- a/src/jinjaturtle/multi.py
+++ b/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
diff --git a/tests/test_config_parse_errors.py b/tests/test_config_parse_errors.py
new file mode 100644
index 0000000..ab42fe5
--- /dev/null
+++ b/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", "<{{tag}}/>"),
+ ("xml", "bad2.xml", ""),
+ ("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 = (
+ '\n'
+ ' ]>\n'
+ "&xxe;\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("<{{tag}}/>", 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
diff --git a/tests/test_core_utils.py b/tests/test_core_utils.py
index c8e41e1..fda6dd0 100644
--- a/tests/test_core_utils.py
+++ b/tests/test_core_utils.py
@@ -191,3 +191,43 @@ def test_flatten_config_unsupported_format():
flatten_config("bogusfmt", parsed=None)
assert "Unsupported format" in str(exc.value)
+
+
+def test_make_var_name_collapses_adjacent_separators():
+ """Regression: adjacent separators must not produce a forbidden ``__``.
+
+ A source key such as ``log..level`` or ``cache--size`` previously sanitised
+ to a name containing a double underscore (``..._log__level``). The output
+ safety gate in safety.py rejects *any* ``__`` in a generated identifier
+ (it is the gateway to Jinja2 SSTI gadgets), so emitting one made JinjaTurtle
+ reject its own placeholder and abort generation on entirely benign config.
+ make_var_name now collapses runs of underscores to a single ``_``.
+ """
+ for raw_key in ("log..level", "cache--size", "a...b", "x.-.y", "a..b--c"):
+ name = make_var_name("role", ("main", raw_key))
+ assert "__" not in name, f"{raw_key!r} produced {name!r}"
+ # Still a valid Ansible/Jinja identifier.
+ assert name.replace("_", "").isalnum() or name == "role"
+
+ # The double-underscore-free collapse is stable and predictable.
+ assert make_var_name("role", ("main", "log..level")) == "role_main_log_level"
+ assert make_var_name("role", ("main", "cache--size")) == "role_main_cache_size"
+ # A leading-digit-free prefix with its own repeated separators is collapsed too.
+ assert make_var_name("My__Role", ("k",)) == "my_role_k"
+
+
+def test_make_var_name_collapsed_names_pass_output_safety_gate():
+ """The names make_var_name emits must be accepted by the safety gate.
+
+ This binds the two modules together: whatever identifier make_var_name
+ produces for a hostile-looking key must lex as the JinjaTurtle subset, so a
+ real template built from it is not refused.
+ """
+ from jinjaturtle.safety import verify_jinja2_template_safe
+
+ for raw_key in ("log..level", "cache--size", "a...b", "weird..key..name"):
+ var = make_var_name("demo", ("section", raw_key))
+ # Build the kind of expression a handler would emit for this variable.
+ template = "{{ " + var + " }}"
+ # Must not raise TemplateSafetyError.
+ verify_jinja2_template_safe(template)