From 575c2b79f92aeded7490cf94b31cea5341248b53 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Wed, 1 Jul 2026 12:23:19 +1000 Subject: [PATCH] Fix catching of defusedxml exception --- src/jinjaturtle/cli.py | 15 +++++++++++ tests/test_config_parse_errors.py | 41 +++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/jinjaturtle/cli.py b/src/jinjaturtle/cli.py index 65dfa72..d901606 100644 --- a/src/jinjaturtle/cli.py +++ b/src/jinjaturtle/cli.py @@ -3,6 +3,7 @@ from __future__ import annotations import argparse import sys from defusedxml import defuse_stdlib +from defusedxml.common import DefusedXmlException from pathlib import Path from . import j2 @@ -77,6 +78,20 @@ 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 DefusedXmlException as exc: + # defusedxml rejected the XML because it attempted a DTD, entity + # expansion, or external reference (XXE / billion-laughs class attack). + # This is a deliberately-blocked attack, not a benign malformed file, so + # core.parse_config lets it propagate unchanged rather than folding it + # into ConfigParseError. Report it as a refused unsafe input with a + # non-zero exit code instead of leaking an internal traceback. + print( + "jinjaturtle: refusing unsafe XML input: the document uses a DTD, " + f"entity expansion, or external reference ({exc.__class__.__name__}). " + "This is blocked to prevent XXE / entity-expansion attacks.", + 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; diff --git a/tests/test_config_parse_errors.py b/tests/test_config_parse_errors.py index ab42fe5..01ba1ef 100644 --- a/tests/test_config_parse_errors.py +++ b/tests/test_config_parse_errors.py @@ -106,6 +106,47 @@ def test_cli_fails_cleanly_on_malformed_xml(tmp_path: Path) -> None: assert "Traceback (most recent call last)" not in res.stderr +@pytest.mark.parametrize( + "content", + [ + # Classic XXE: external SYSTEM entity used to read a local file. + ( + '\n' + ']>\n' + "&xxe;\n" + ), + # Billion-laughs style internal entity expansion. + ( + '\n' + ']>\n' + "&lol2;\n" + ), + # External parameter entity (SSRF / out-of-band XXE vector). + ( + '\n' + ' %ext;]>\n' + "x\n" + ), + ], +) +def test_cli_refuses_xxe_cleanly(tmp_path: Path, content: str) -> None: + """defusedxml's security refusal must surface as a clean non-zero exit. + + ``EntitiesForbidden``/``DTDForbidden``/``ExternalReferenceForbidden`` subclass + ``DefusedXmlException`` (and ``ValueError``). ``parse_config`` deliberately + lets them propagate rather than folding them into ``ConfigParseError`` so a + blocked attack stays distinguishable from a benign malformed file. The CLI + must therefore catch ``DefusedXmlException`` itself and exit cleanly instead + of leaking a Python traceback. + """ + src = tmp_path / "xxe.xml" + src.write_text(content, encoding="utf-8") + res = _run_cli([str(src), "-f", "xml", "-r", "demo"]) + assert res.returncode == 2 + assert "refusing unsafe XML input" 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