From 8b2aa77290aeb5f2e5ec77bde1f9c34edc5d424b Mon Sep 17 00:00:00 2001 From: Quinten Steenhuis Date: Mon, 21 Sep 2026 12:40:24 -0400 Subject: [PATCH 01/10] Preprocess opted-in Jinja YAML before validation Add a small sandboxed renderer with local includes, explicit rendering diagnostics, and generated-output locations. Keep automatic fixes away from template source. Closes #61 --- README.md | 19 ++++ pyproject.toml | 1 + requirements.txt | 1 + src/dayamlchecker/_jinja.py | 20 ++++ src/dayamlchecker/fixer.py | 4 + src/dayamlchecker/messages.py | 8 ++ src/dayamlchecker/yaml_structure.py | 38 ++++++-- tests/fixtures/jinja/fields.yml | 2 + tests/fixtures/jinja/interview.yml | 13 +++ tests/test_jinja.py | 137 ++++++++++++++++++++++++++++ 10 files changed, 234 insertions(+), 9 deletions(-) create mode 100644 src/dayamlchecker/_jinja.py create mode 100644 tests/fixtures/jinja/fields.yml create mode 100644 tests/fixtures/jinja/interview.yml create mode 100644 tests/test_jinja.py diff --git a/README.md b/README.md index ac2d775..afbb0b6 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,25 @@ cannot safely rewrite is not itself an error, so it does not fail the run. python3 -m dayamlchecker --fix path/to/interview.yml ``` +## Jinja2 preprocessing + +Files beginning with `# use jinja` are rendered before the normal validation +pass, following docassemble's [YAML preprocessing feature](https://docassemble.org/docs/interviews.html#jinja2). +Expressions, loops, conditionals, macros, and local includes are supported. +Include paths are relative to the input file's directory; includes can contain +partial YAML blocks. Ordinary YAML files and Mako expressions are unaffected. + +This is an offline check: server configuration, `jinja data`, docassemble's +special context variables, and package-qualified includes are not supplied. +Missing variables or includes produce `EG105` rather than silently skipping +validation. Rendering uses Jinja's sandbox and disables HTML escaping. + +Findings after preprocessing use a virtual filename ending in `(rendered Jinja)`; +their line numbers and suppression comments refer to the rendered YAML, not the +original template. Only the rendered branches are checked. `--fix` skips these +files because generated line numbers cannot safely identify source edits. +Template-aware formatting is outside this feature's scope. + ## Suppressing checks You can suppress specific errors or warnings by their ID or finding class (`accessibility`, `style`, `translatability`, `general`). diff --git a/pyproject.toml b/pyproject.toml index c816c2b..aeee2fe 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,6 +7,7 @@ requires-python = ">=3.12" dependencies = [ "esprima>=4.0.1", "mako>=1.3.10", + "jinja2>=3.1.6", "black>=24.0.0", "ruamel.yaml>=0.18.0", "docx2python>=3.5.0", diff --git a/requirements.txt b/requirements.txt index 0ab72df..252af92 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,6 @@ esprima>=4.0.1 mako>=1.3.10 +jinja2>=3.1.6 black>=24.0.0 ruamel.yaml>=0.18.0 docx2python>=3.5.0 diff --git a/src/dayamlchecker/_jinja.py b/src/dayamlchecker/_jinja.py new file mode 100644 index 0000000..2997be1 --- /dev/null +++ b/src/dayamlchecker/_jinja.py @@ -0,0 +1,20 @@ +"""The optional docassemble YAML preprocessor, isolated from validation.""" + +from pathlib import Path + +from jinja2 import FileSystemLoader, StrictUndefined +from jinja2.sandbox import SandboxedEnvironment + + +def render_yaml(source: str, input_file: str | None = None) -> str: + """Render local includes and ordinary Jinja without a docassemble server. + + Undefined values fail explicitly: silently selecting a branch based on + missing server configuration could conceal errors in an interview. + """ + env = SandboxedEnvironment( + loader=FileSystemLoader(Path(input_file).parent) if input_file else None, + undefined=StrictUndefined, + autoescape=False, + ) + return env.from_string(source).render() diff --git a/src/dayamlchecker/fixer.py b/src/dayamlchecker/fixer.py index b0c678d..047bec8 100644 --- a/src/dayamlchecker/fixer.py +++ b/src/dayamlchecker/fixer.py @@ -915,6 +915,10 @@ def plan_file(path: Path, options: FixOptions | None = None) -> FilePlan: plan.skipped_reason = f"could not read file: {exc}" return plan + if text.startswith("# use jinja"): + plan.skipped_reason = "Jinja templates cannot be automatically rewritten" + return plan + documents, parse_error = _load_documents(text) if documents is None: plan.skipped_reason = f"YAML parse failed: {parse_error}" diff --git a/src/dayamlchecker/messages.py b/src/dayamlchecker/messages.py index 7230652..da35797 100644 --- a/src/dayamlchecker/messages.py +++ b/src/dayamlchecker/messages.py @@ -22,6 +22,7 @@ class MessageId(StrEnum): YAML_DUPLICATE_KEY = "yaml_duplicate_key" YAML_DUPLICATE_BLOCK_ID = "yaml_duplicate_block_id" YAML_PARSE_ERROR = "yaml_parse_error" + JINJA_RENDER_ERROR = "jinja_render_error" YAML_STRING_REQUIRED = "yaml_string_required" MAKO_SYNTAX_ERROR = "mako_syntax_error" @@ -298,6 +299,13 @@ class MessageDefinition: summary="Duplicate YAML key", template="{error}", ), + MessageId.JINJA_RENDER_ERROR: MessageDefinition( + code="EG105", + severity=Severity.ERROR, + finding_class=FindingClass.GENERAL, + summary="Jinja rendering error", + template="Could not render Jinja YAML: {error}", + ), MessageId.YAML_PARSE_ERROR: MessageDefinition( code="EG102", severity=Severity.ERROR, diff --git a/src/dayamlchecker/yaml_structure.py b/src/dayamlchecker/yaml_structure.py index 042843f..e02719f 100644 --- a/src/dayamlchecker/yaml_structure.py +++ b/src/dayamlchecker/yaml_structure.py @@ -58,8 +58,6 @@ # * is "gathered" a valid attr? # * handle "response" # * labels above fields? -# * if "# use jinja" at top, process whole file with Jinja: -# https://docassemble.org/docs/interviews.html#jinja2 __all__ = [ @@ -2066,6 +2064,34 @@ def find_errors_from_string( input_file: Optional[str] = None, lint_mode: str = DEFAULT_LINT_MODE, runtime_options: Optional[RuntimeOptions] = None, +) -> list[YAMLError]: + """Preprocess opted-in Jinja templates, then run normal YAML validation.""" + if full_content.startswith("# use jinja"): + from dayamlchecker._jinja import render_yaml + + try: + full_content = render_yaml(full_content, input_file) + except Exception as exc: + # Rendering can also raise Python errors (e.g. division by zero). + return [ + make_finding( + MessageId.JINJA_RENDER_ERROR, + file_name=getattr(exc, "filename", None) or input_file, + line_number=getattr(exc, "lineno", None), + error=str(exc), + ) + ] + # Generated lines need not correspond to template lines. A virtual + # filename also prevents CLI suppressions from re-reading raw source. + input_file = f"{input_file or ''} (rendered Jinja)" + return _find_errors_from_yaml(full_content, input_file, lint_mode, runtime_options) + + +def _find_errors_from_yaml( + full_content: str, + input_file: Optional[str] = None, + lint_mode: str = DEFAULT_LINT_MODE, + runtime_options: Optional[RuntimeOptions] = None, ) -> list[YAMLError]: """Return list of findings found in the given full_content string @@ -2519,8 +2545,7 @@ def find_errors( ) -> list[YAMLError]: """Return list of findings found in the given input_file - If the file has Docassemble's optional Jinja2 preprocessor directive at the top, - it is ignored and an empty list is returned. + Files beginning with ``# use jinja`` are rendered before validation. Args: input_file (str): Path to the YAML file to check. @@ -2531,11 +2556,6 @@ def find_errors( with open(input_file, "r") as f: full_content = f.read() - if full_content[:12] == "# use jinja\n": - print() - print(f"Ah Jinja! ignoring {input_file}") - return [] - return find_errors_from_string( full_content, input_file=input_file, diff --git a/tests/fixtures/jinja/fields.yml b/tests/fixtures/jinja/fields.yml new file mode 100644 index 0000000..42f9811 --- /dev/null +++ b/tests/fixtures/jinja/fields.yml @@ -0,0 +1,2 @@ + - Like {{ fruit }}s: likes_{{ fruit }} + datatype: yesno diff --git a/tests/fixtures/jinja/interview.yml b/tests/fixtures/jinja/interview.yml new file mode 100644 index 0000000..f82a2e1 --- /dev/null +++ b/tests/fixtures/jinja/interview.yml @@ -0,0 +1,13 @@ +# use jinja +{% set fruits = ['apple', 'pear'] %} +{% for fruit in fruits %} +--- +id: {{ fruit }} +question: Do you like {{ fruit }}s? +fields: +{% include 'fields.yml' %} +{% endfor %} +--- +mandatory: true +code: | + likes_both = likes_apple and likes_pear diff --git a/tests/test_jinja.py b/tests/test_jinja.py new file mode 100644 index 0000000..a488d37 --- /dev/null +++ b/tests/test_jinja.py @@ -0,0 +1,137 @@ +from pathlib import Path + +import pytest + +from dayamlchecker._jinja import render_yaml +from dayamlchecker.fixer import plan_file +from dayamlchecker.messages import MessageId +from dayamlchecker.yaml_structure import find_errors, find_errors_from_string, main + +FIXTURES = Path(__file__).parent / "fixtures" / "jinja" + + +def test_includes_loops_and_normal_validation(): + path = FIXTURES / "interview.yml" + rendered = render_yaml(path.read_text(), str(path)) + assert "likes_apple" in rendered and "likes_pear" in rendered + assert "{%" not in rendered + assert find_errors(str(path)) == find_errors_from_string( + rendered, input_file=f"{path} (rendered Jinja)" + ) + assert not any(f.severity == "error" for f in find_errors(str(path))) + + +def test_included_invalid_python_is_validated(tmp_path): + (tmp_path / "included.yml").write_text("code: |\n broken =\n") + path = tmp_path / "main.yml" + path.write_text('# use jinja\n{% include "included.yml" %}\n') + findings = find_errors(str(path)) + finding = next(f for f in findings if f.message_id == MessageId.PYTHON_SYNTAX_ERROR) + assert finding.file_name == f"{path} (rendered Jinja)" + assert finding.line_number is not None + + +@pytest.mark.parametrize( + "body, expected", + [ + ("{% if %}", "Expected an expression"), + ('{% include "absent.yml" %}', "absent.yml"), + ("question: {{ missing }}", "missing"), + ("{% if __debug__ %}question: Debug{% endif %}", "__debug__"), + ("question: {{ 1 / 0 }}", "division by zero"), + ("{{ ''.__class__.__mro__ }}", "unsafe"), + ], +) +def test_render_failures_are_findings(tmp_path, body, expected): + findings = find_errors_from_string( + "# use jinja\n" + body, input_file=str(tmp_path / "main.yml") + ) + assert len(findings) == 1 + assert findings[0].message_id == MessageId.JINJA_RENDER_ERROR + assert expected in findings[0].message + + +def test_syntax_error_in_include_has_source_location(tmp_path): + included = tmp_path / "bad.yml" + included.write_text("question: Hello\n{% if %}\n") + findings = find_errors_from_string( + '# use jinja\n{% include "bad.yml" %}', + input_file=str(tmp_path / "main.yml"), + ) + assert findings[0].file_name == str(included) + assert findings[0].line_number == 2 + + +def test_jinja_is_opt_in_and_preserves_mako(): + source = "question: |\n {{ literal }} and ${ answer }\n" + assert render_yaml("# use jinja\n{% raw %}" + source + "{% endraw %}") == ( + "# use jinja\n" + source + ) + assert not any( + f.message_id == MessageId.JINJA_RENDER_ERROR + for f in find_errors_from_string(source) + ) + + +def test_string_input_and_crlf(): + findings = find_errors_from_string( + '# use jinja\r\n{% set value = "broken =" %}\r\ncode: |\r\n {{ value }}\r\n' + ) + assert any(f.message_id == MessageId.PYTHON_SYNTAX_ERROR for f in findings) + + +def test_cli_reports_generated_errors_and_honors_rendered_suppressions( + tmp_path, capsys +): + path = tmp_path / "main.yml" + path.write_text('# use jinja\n{{ "\\n" * 10 }}\n---\ncode: |\n broken =\n') + assert main([str(path), "--no-docx-accessibility"]) == 1 + assert "rendered Jinja" in capsys.readouterr().out + path.write_text( + '# use jinja\n{{ "\\n" * 10 }}\n---\n' + "# no-dayc-block: EG122\ncode: |\n broken =\n# end of block\n" + ) + assert main([str(path), "--no-docx-accessibility"]) == 0 + + +def test_fixer_leaves_even_yaml_parseable_templates_untouched(tmp_path): + path = tmp_path / "main.yml" + source = '# use jinja\nquestion: "{{ 1 + 1 }}"\nyesno: answer\n' + path.write_text(source) + plan = plan_file(path) + assert plan.skipped_reason == "Jinja templates cannot be automatically rewritten" + assert path.read_text() == source + + +def test_conditionals_and_generated_yaml_errors(): + source = ( + "# use jinja\n{% set enabled = true %}\n" + "{% if enabled %}question: [unclosed{% else %}question: Fine{% endif %}\n" + ) + assert any( + f.message_id == MessageId.YAML_PARSE_ERROR + for f in find_errors_from_string(source) + ) + assert not any( + f.severity == "error" + for f in find_errors_from_string( + source.replace("enabled = true", "enabled = false") + ) + ) + + +def test_docassemble_documented_include_pattern(tmp_path): + # Adapted from docassemble's examples/jinjayaml{,-included}.yml. + (tmp_path / "fruit.yml").write_text( + "id: fruit\nquestion: |\n What is your favorite fruit?\n" + "fields:\n - Fruit: favorite_fruit\n---\n" + ) + source = ( + '# use jinja\n{% include "fruit.yml" %}\n' + "id: result\nmandatory: True\nquestion: |\n" + " Your favorite fruit is ${ favorite_fruit }.\n" + ) + assert not any( + f.severity == "error" + for f in find_errors_from_string(source, input_file=str(tmp_path / "main.yml")) + ) From 9a10c6f2133922e471fbffc97f52b35d1bb22dbe Mon Sep 17 00:00:00 2001 From: Quinten Steenhuis Date: Mon, 21 Sep 2026 12:56:03 -0400 Subject: [PATCH 02/10] Validate remaining YAML when Jinja includes are missing Report EG106 at the template include site and require explicit no-dayc suppression to accept missing dependencies. Blank affected documents, retain rendered locations, and test partial validation and suppression behavior. --- README.md | 32 +++++- src/dayamlchecker/_jinja.py | 91 ++++++++++++++-- src/dayamlchecker/messages.py | 11 ++ src/dayamlchecker/yaml_structure.py | 53 ++++++++-- tests/test_jinja.py | 158 +++++++++++++++++++++++++++- 5 files changed, 322 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index afbb0b6..3165f96 100644 --- a/README.md +++ b/README.md @@ -36,8 +36,36 @@ partial YAML blocks. Ordinary YAML files and Mako expressions are unaffected. This is an offline check: server configuration, `jinja data`, docassemble's special context variables, and package-qualified includes are not supplied. -Missing variables or includes produce `EG105` rather than silently skipping -validation. Rendering uses Jinja's sandbox and disables HTML escaping. +Missing variables, imports, and parent templates produce `EG105`. Rendering +uses Jinja's sandbox and disables HTML escaping. + +Missing `{% include %}` files (including unavailable package-qualified paths) +produce error `EG106`: the included Jinja2 document could not be verified and +findings are partial. The checker substitutes a marker, skips each rendered YAML +document containing that marker, and checks the remaining documents. This also +applies to `ignore missing`; include fallback lists try all candidates first. +Repeated execution of the same include site produces one diagnostic. + +Partial validation is best effort. An unavailable include may itself supply YAML +document boundaries or Jinja definitions, so the remaining output may differ from +the real interview. A partial-block include causes its entire containing YAML +document to be skipped. Findings retain rendered line numbers. + +Missing includes fail CI by default. Explicitly accept a known external dependency +with a source-level suppression: + +```yaml +# use jinja +{% include "docassemble.framework:data/questions/base.yml" %} # no-dayc: EG106 +--- +code: | + downstream_value = 1 +``` + +`# no-dayc-block: EG106` also works. These suppressions apply to the original +include location, including includes in local templates; they do not suppress +errors in the remaining YAML. Jinja rendering errors (`EG105`) also honor source +suppressions, but a rendering failure prevents validation of the remaining file. Findings after preprocessing use a virtual filename ending in `(rendered Jinja)`; their line numbers and suppression comments refer to the rendered YAML, not the diff --git a/src/dayamlchecker/_jinja.py b/src/dayamlchecker/_jinja.py index 2997be1..85e2584 100644 --- a/src/dayamlchecker/_jinja.py +++ b/src/dayamlchecker/_jinja.py @@ -1,20 +1,95 @@ """The optional docassemble YAML preprocessor, isolated from validation.""" +from copy import copy +from dataclasses import dataclass from pathlib import Path +import re +from uuid import uuid4 -from jinja2 import FileSystemLoader, StrictUndefined +from jinja2 import ( + DictLoader, + FileSystemLoader, + StrictUndefined, + Template, + TemplateNotFound, + nodes, +) +from jinja2.compiler import CodeGenerator, Frame from jinja2.sandbox import SandboxedEnvironment -def render_yaml(source: str, input_file: str | None = None) -> str: - """Render local includes and ordinary Jinja without a docassemble server. +@dataclass(frozen=True) +class MissingInclude: + description: str + file_name: str | None + line_number: int - Undefined values fail explicitly: silently selecting a branch based on - missing server configuration could conceal errors in an interview. + +class _IncludeCodeGenerator(CodeGenerator): + def visit_Include(self, node: nodes.Include, frame: Frame) -> None: + # Wrap only include lookups. Imports and inheritance must still fail. + node = copy(node) + node.template = nodes.Call( + nodes.EnvironmentAttribute("load_include"), + [ + node.template, + nodes.Const(self.name), + nodes.Const(self.filename), + nodes.Const(node.lineno), + ], + [], + None, + None, + ) + node.template.set_lineno(node.lineno) + super().visit_Include(node, frame) + + +class _PartialEnvironment(SandboxedEnvironment): + code_generator_class = _IncludeCodeGenerator + missing_includes: list[MissingInclude] + missing_marker: str + + def load_include( + self, + name: str | Template | list[str | Template], + parent: str | None, + filename: str | None, + lineno: int, + ) -> Template: + try: + return self.get_or_select_template(name, parent) + except TemplateNotFound as exc: + # Try every candidate in an include list before substituting. + missing = MissingInclude(str(exc), filename, lineno) + if missing not in self.missing_includes: + self.missing_includes.append(missing) + return self.from_string(self.missing_marker) + + +def render_yaml( + source: str, input_file: str | None = None +) -> tuple[str, list[MissingInclude]]: + """Render YAML, blanking documents affected by unavailable includes. + + The result is best effort: missing templates may supply document separators + or definitions. Preserve newlines so unaffected findings keep rendered line + numbers. Missing server variables still fail instead of selecting a branch. """ - env = SandboxedEnvironment( - loader=FileSystemLoader(Path(input_file).parent) if input_file else None, + env = _PartialEnvironment( + loader=( + FileSystemLoader(Path(input_file).parent) if input_file else DictLoader({}) + ), undefined=StrictUndefined, autoescape=False, ) - return env.from_string(source).render() + env.missing_includes = [] + env.missing_marker = f"DAYAMLCHECKER_MISSING_{uuid4().hex}" + rendered = env.from_string(source).render() + # Match the validator's document boundaries, retaining the separators. + parts = re.split(r"(^--- *$)", rendered, flags=re.MULTILINE) + rendered = "".join( + "\n" * part.count("\n") if env.missing_marker in part else part + for part in parts + ) + return rendered, env.missing_includes diff --git a/src/dayamlchecker/messages.py b/src/dayamlchecker/messages.py index da35797..52449de 100644 --- a/src/dayamlchecker/messages.py +++ b/src/dayamlchecker/messages.py @@ -23,6 +23,7 @@ class MessageId(StrEnum): YAML_DUPLICATE_BLOCK_ID = "yaml_duplicate_block_id" YAML_PARSE_ERROR = "yaml_parse_error" JINJA_RENDER_ERROR = "jinja_render_error" + JINJA_MISSING_INCLUDE = "jinja_missing_include" YAML_STRING_REQUIRED = "yaml_string_required" MAKO_SYNTAX_ERROR = "mako_syntax_error" @@ -299,6 +300,16 @@ class MessageDefinition: summary="Duplicate YAML key", template="{error}", ), + MessageId.JINJA_MISSING_INCLUDE: MessageDefinition( + code="EG106", + severity=Severity.ERROR, + finding_class=FindingClass.GENERAL, + summary="Missing Jinja include; validation is partial", + template="Included Jinja2 document could not be verified: {missing}. " + "Validation is partial: rendered YAML documents containing its placeholder " + "were skipped. The missing include may supply document boundaries or " + "definitions, so remaining findings are best effort.", + ), MessageId.JINJA_RENDER_ERROR: MessageDefinition( code="EG105", severity=Severity.ERROR, diff --git a/src/dayamlchecker/yaml_structure.py b/src/dayamlchecker/yaml_structure.py index e02719f..3b9ffbe 100644 --- a/src/dayamlchecker/yaml_structure.py +++ b/src/dayamlchecker/yaml_structure.py @@ -2059,6 +2059,19 @@ def depth(var_name: str) -> int: return max((depth(var) for var in adjacency.keys()), default=0) +def _apply_jinja_suppressions( + findings: list[Finding], source: str, input_file: str | None +) -> list[Finding]: + """Jinja diagnostics refer to template source, not generated YAML.""" + result = [] + for finding in findings: + if finding.file_name == input_file: + result.extend(_apply_dayc_suppressions([finding], source)) + else: + result.extend(_apply_dayc_suppressions_from_files([finding])) + return result + + def find_errors_from_string( full_content: str, input_file: Optional[str] = None, @@ -2066,25 +2079,45 @@ def find_errors_from_string( runtime_options: Optional[RuntimeOptions] = None, ) -> list[YAMLError]: """Preprocess opted-in Jinja templates, then run normal YAML validation.""" + partial_findings: list[YAMLError] = [] if full_content.startswith("# use jinja"): from dayamlchecker._jinja import render_yaml + source_content = full_content try: - full_content = render_yaml(full_content, input_file) + full_content, missing_includes = render_yaml(full_content, input_file) except Exception as exc: # Rendering can also raise Python errors (e.g. division by zero). - return [ - make_finding( - MessageId.JINJA_RENDER_ERROR, - file_name=getattr(exc, "filename", None) or input_file, - line_number=getattr(exc, "lineno", None), - error=str(exc), - ) - ] + return _apply_jinja_suppressions( + [ + make_finding( + MessageId.JINJA_RENDER_ERROR, + file_name=getattr(exc, "filename", None) or input_file, + line_number=getattr(exc, "lineno", None), + error=str(exc), + ) + ], + source_content, + input_file, + ) + partial_findings = [ + make_finding( + MessageId.JINJA_MISSING_INCLUDE, + file_name=missing.file_name or input_file, + line_number=missing.line_number, + missing=missing.description, + ) + for missing in missing_includes + ] + partial_findings = _apply_jinja_suppressions( + partial_findings, source_content, input_file + ) # Generated lines need not correspond to template lines. A virtual # filename also prevents CLI suppressions from re-reading raw source. input_file = f"{input_file or ''} (rendered Jinja)" - return _find_errors_from_yaml(full_content, input_file, lint_mode, runtime_options) + return partial_findings + _find_errors_from_yaml( + full_content, input_file, lint_mode, runtime_options + ) def _find_errors_from_yaml( diff --git a/tests/test_jinja.py b/tests/test_jinja.py index a488d37..b9d1fbb 100644 --- a/tests/test_jinja.py +++ b/tests/test_jinja.py @@ -12,7 +12,8 @@ def test_includes_loops_and_normal_validation(): path = FIXTURES / "interview.yml" - rendered = render_yaml(path.read_text(), str(path)) + rendered, missing = render_yaml(path.read_text(), str(path)) + assert missing == [] assert "likes_apple" in rendered and "likes_pear" in rendered assert "{%" not in rendered assert find_errors(str(path)) == find_errors_from_string( @@ -35,7 +36,6 @@ def test_included_invalid_python_is_validated(tmp_path): "body, expected", [ ("{% if %}", "Expected an expression"), - ('{% include "absent.yml" %}', "absent.yml"), ("question: {{ missing }}", "missing"), ("{% if __debug__ %}question: Debug{% endif %}", "__debug__"), ("question: {{ 1 / 0 }}", "division by zero"), @@ -65,7 +65,8 @@ def test_syntax_error_in_include_has_source_location(tmp_path): def test_jinja_is_opt_in_and_preserves_mako(): source = "question: |\n {{ literal }} and ${ answer }\n" assert render_yaml("# use jinja\n{% raw %}" + source + "{% endraw %}") == ( - "# use jinja\n" + source + "# use jinja\n" + source, + [], ) assert not any( f.message_id == MessageId.JINJA_RENDER_ERROR @@ -135,3 +136,154 @@ def test_docassemble_documented_include_pattern(tmp_path): f.severity == "error" for f in find_errors_from_string(source, input_file=str(tmp_path / "main.yml")) ) + + +@pytest.mark.parametrize( + "include", + [ + '{% include "docassemble.other:data/questions/fields.yml" %}', + '{% include "missing.yml" without context %}', + '{% include "missing.yml" ignore missing %}', + '{% set file = "missing.yml" %}{% include file %}', + ], +) +def test_missing_include_skips_affected_document_only(include): + source = ( + "# use jinja\ncode: |\n before =\n---\n" + "id: incomplete\nquestion: Incomplete\nfields:\n" + + include + + "\n---\ncode: |\n after =\n" + ) + rendered, missing = render_yaml(source) + assert len(missing) == 1 + assert "fields:" not in rendered + assert rendered.count("\n") == source.count("\n") - 1 + findings = find_errors_from_string(source) + missing_findings = [ + f for f in findings if f.message_id == MessageId.JINJA_MISSING_INCLUDE + ] + errors = [f for f in findings if f.message_id != MessageId.JINJA_MISSING_INCLUDE] + assert len(missing_findings) == 1 + assert missing_findings[0].severity == "error" + assert "Validation is partial" in missing_findings[0].message + assert len(errors) == 2 + assert all(f.message_id == MessageId.PYTHON_SYNTAX_ERROR for f in errors) + # Blanking the incomplete document must not shift subsequent locations. + expected = find_errors_from_string(rendered) + assert [f.line_number for f in errors] == [f.line_number for f in expected] + + +def test_missing_standalone_and_nested_includes(tmp_path): + (tmp_path / "local.yml").write_text('{% include "missing.yml" %}') + source = '# use jinja\n{% include "local.yml" %}\n---\ncode: |\n valid = 1\n' + findings = find_errors_from_string(source, input_file=str(tmp_path / "main.yml")) + assert len(findings) == 1 + assert findings[0].message_id == MessageId.JINJA_MISSING_INCLUDE + assert "missing.yml" in findings[0].message + + +def test_include_fallback_list_uses_existing_file(tmp_path): + (tmp_path / "exists.yml").write_text("code: |\n valid = 1\n") + source = '# use jinja\n{% include ["missing.yml", "exists.yml"] %}' + assert find_errors_from_string(source, input_file=str(tmp_path / "main.yml")) == [] + findings = find_errors_from_string(source.replace("exists.yml", "also-missing.yml")) + assert len(findings) == 1 + assert findings[0].message_id == MessageId.JINJA_MISSING_INCLUDE + assert "missing.yml" in findings[0].message + assert "also-missing.yml" in findings[0].message + + +@pytest.mark.parametrize( + "body", + [ + '{% import "missing.yml" as macros %}', + '{% from "missing.yml" import question %}', + '{% extends "missing.yml" %}', + ], +) +def test_missing_imports_and_parents_still_fail(tmp_path, body): + # Also exercise failures inside an otherwise available include. + (tmp_path / "local.yml").write_text(body) + for source in [body, '{% include "local.yml" %}']: + findings = find_errors_from_string( + "# use jinja\n" + source, input_file=str(tmp_path / "main.yml") + ) + assert len(findings) == 1 + assert findings[0].message_id == MessageId.JINJA_RENDER_ERROR + + +def test_missing_includes_report_once_and_ignore_unselected_branches(): + source = ( + "# use jinja\n{% for i in range(3) %}\n" + '{% include "missing.yml" %}\n---\n{% endfor %}\n' + '{% if false %}{% include "unused.yml" %}{% endif %}' + ) + findings = find_errors_from_string(source) + assert len(findings) == 1 + assert "unused.yml" not in findings[0].message + + +def test_partial_validation_cli_exit_status(tmp_path, capsys): + path = tmp_path / "main.yml" + path.write_text( + '# use jinja\n{% include "missing.yml" %}\n---\ncode: |\n valid = 1\n' + ) + args = [str(path), "--no-docx-accessibility"] + assert main(args) == 1 + assert "Validation is partial" in capsys.readouterr().out + path.write_text(path.read_text().replace("%}\n---", "%} # no-dayc: EG106\n---")) + assert main(args + ["--max-warnings", "0"]) == 0 + path.write_text(path.read_text().replace("valid = 1", "broken =")) + assert main(args) == 1 + + +@pytest.mark.parametrize( + "suppression", ["# no-dayc: EG106", "# no-dayc: jinja_missing_include"] +) +def test_inline_suppression_opts_into_partial_validation(suppression): + source = ( + '# use jinja\n{% include "docassemble.framework:data/questions/base.yml" %} ' + + suppression + + "\n---\ncode: |\n broken =\n" + ) + findings = find_errors_from_string(source) + assert [f.message_id for f in findings] == [MessageId.PYTHON_SYNTAX_ERROR] + + +def test_block_suppression_does_not_hide_other_missing_dependencies(): + source = ( + '# use jinja\n# no-dayc-block: EG106\n{% include "known.yml" %}\n' + '---\n{% include "unexpected.yml" %}\n' + ) + findings = find_errors_from_string(source) + assert len(findings) == 1 + assert findings[0].code == "EG106" + assert "unexpected.yml" in findings[0].message + assert findings[0].line_number == 5 + + +def test_nested_include_suppression_uses_its_own_source(tmp_path): + included = tmp_path / "local.yml" + included.write_text('{% include "external.yml" %} # no-dayc: EG106\n') + source = '# use jinja\n{% include "local.yml" %}\n---\ncode: |\n valid = 1\n' + assert find_errors_from_string(source, input_file=str(tmp_path / "main.yml")) == [] + included.write_text('{% include "external.yml" %}\n') + findings = find_errors_from_string(source, input_file=str(tmp_path / "main.yml")) + assert findings[0].file_name == str(included) + assert findings[0].line_number == 1 + + +def test_jinja_syntax_error_suppression_uses_template_source(): + assert find_errors_from_string("# use jinja\n{% if %} # no-dayc: EG105\n") == [] + + +def test_include_suppression_is_per_source_site_not_rendered_line(): + source = ( + '# use jinja\n{{ "\\n" * 10 }}\n' + '{% include "external.yml" %} # no-dayc: EG106\n---\n' + '{% include "external.yml" %}\n' + ) + findings = find_errors_from_string(source) + assert len(findings) == 1 + assert findings[0].code == "EG106" + assert findings[0].line_number == 5 From 31b277c547f79f93690e8bfa1bf4ac12469ecb14 Mon Sep 17 00:00:00 2001 From: Quinten Steenhuis Date: Mon, 21 Sep 2026 13:36:44 -0400 Subject: [PATCH 03/10] Keep missing Jinja includes non-breaking by default Report missing includes as WG106 while preserving errors for other dependency failures unless explicitly suppressed. Test both default exit statuses and no-dayc overrides. --- README.md | 25 ++++++++------- src/dayamlchecker/messages.py | 4 +-- tests/test_jinja.py | 58 +++++++++++++++++++++++++++++------ 3 files changed, 64 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 3165f96..eb014bd 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ Missing variables, imports, and parent templates produce `EG105`. Rendering uses Jinja's sandbox and disables HTML escaping. Missing `{% include %}` files (including unavailable package-qualified paths) -produce error `EG106`: the included Jinja2 document could not be verified and +produce warning `WG106`: the included Jinja2 document could not be verified and findings are partial. The checker substitutes a marker, skips each rendered YAML document containing that marker, and checks the remaining documents. This also applies to `ignore missing`; include fallback lists try all candidates first. @@ -51,21 +51,24 @@ document boundaries or Jinja definitions, so the remaining output may differ fro the real interview. A partial-block include causes its entire containing YAML document to be skipped. Findings retain rendered line numbers. -Missing includes fail CI by default. Explicitly accept a known external dependency -with a source-level suppression: +Missing includes are skipped by default and do not fail CI unless a warning +limit such as `--max-warnings 0` is set. You can suppress the partial-validation +warning with `# no-dayc: WG106` on the include or `# no-dayc-block: WG106` in its +source block. + +Other errors, including missing Jinja variables, imports, and parent templates +(`EG105`), still fail by default. Explicitly suppress a known dependency-related +rendering limitation with a source-level suppression, for example: ```yaml # use jinja -{% include "docassemble.framework:data/questions/base.yml" %} # no-dayc: EG106 ---- -code: | - downstream_value = 1 +# no-dayc-block: EG105 +{% import "external-macros.yml" as framework %} ``` -`# no-dayc-block: EG106` also works. These suppressions apply to the original -include location, including includes in local templates; they do not suppress -errors in the remaining YAML. Jinja rendering errors (`EG105`) also honor source -suppressions, but a rendering failure prevents validation of the remaining file. +Rendering errors also honor source suppressions, but a rendering failure prevents +validation of the remaining file. Missing includes alone allow partial validation; +suppressing their warning does not suppress errors in the remaining YAML. Findings after preprocessing use a virtual filename ending in `(rendered Jinja)`; their line numbers and suppression comments refer to the rendered YAML, not the diff --git a/src/dayamlchecker/messages.py b/src/dayamlchecker/messages.py index 52449de..caa0bb8 100644 --- a/src/dayamlchecker/messages.py +++ b/src/dayamlchecker/messages.py @@ -301,8 +301,8 @@ class MessageDefinition: template="{error}", ), MessageId.JINJA_MISSING_INCLUDE: MessageDefinition( - code="EG106", - severity=Severity.ERROR, + code="WG106", + severity=Severity.WARNING, finding_class=FindingClass.GENERAL, summary="Missing Jinja include; validation is partial", template="Included Jinja2 document could not be verified: {missing}. " diff --git a/tests/test_jinja.py b/tests/test_jinja.py index b9d1fbb..569d940 100644 --- a/tests/test_jinja.py +++ b/tests/test_jinja.py @@ -164,7 +164,7 @@ def test_missing_include_skips_affected_document_only(include): ] errors = [f for f in findings if f.message_id != MessageId.JINJA_MISSING_INCLUDE] assert len(missing_findings) == 1 - assert missing_findings[0].severity == "error" + assert missing_findings[0].severity == "warning" assert "Validation is partial" in missing_findings[0].message assert len(errors) == 2 assert all(f.message_id == MessageId.PYTHON_SYNTAX_ERROR for f in errors) @@ -229,18 +229,19 @@ def test_partial_validation_cli_exit_status(tmp_path, capsys): '# use jinja\n{% include "missing.yml" %}\n---\ncode: |\n valid = 1\n' ) args = [str(path), "--no-docx-accessibility"] - assert main(args) == 1 + assert main(args) == 0 assert "Validation is partial" in capsys.readouterr().out - path.write_text(path.read_text().replace("%}\n---", "%} # no-dayc: EG106\n---")) + assert main(args + ["--max-warnings", "0"]) == 1 + path.write_text(path.read_text().replace("%}\n---", "%} # no-dayc: WG106\n---")) assert main(args + ["--max-warnings", "0"]) == 0 path.write_text(path.read_text().replace("valid = 1", "broken =")) assert main(args) == 1 @pytest.mark.parametrize( - "suppression", ["# no-dayc: EG106", "# no-dayc: jinja_missing_include"] + "suppression", ["# no-dayc: WG106", "# no-dayc: jinja_missing_include"] ) -def test_inline_suppression_opts_into_partial_validation(suppression): +def test_inline_suppression_hides_only_partial_validation_warning(suppression): source = ( '# use jinja\n{% include "docassemble.framework:data/questions/base.yml" %} ' + suppression @@ -252,19 +253,19 @@ def test_inline_suppression_opts_into_partial_validation(suppression): def test_block_suppression_does_not_hide_other_missing_dependencies(): source = ( - '# use jinja\n# no-dayc-block: EG106\n{% include "known.yml" %}\n' + '# use jinja\n# no-dayc-block: WG106\n{% include "known.yml" %}\n' '---\n{% include "unexpected.yml" %}\n' ) findings = find_errors_from_string(source) assert len(findings) == 1 - assert findings[0].code == "EG106" + assert findings[0].code == "WG106" assert "unexpected.yml" in findings[0].message assert findings[0].line_number == 5 def test_nested_include_suppression_uses_its_own_source(tmp_path): included = tmp_path / "local.yml" - included.write_text('{% include "external.yml" %} # no-dayc: EG106\n') + included.write_text('{% include "external.yml" %} # no-dayc: WG106\n') source = '# use jinja\n{% include "local.yml" %}\n---\ncode: |\n valid = 1\n' assert find_errors_from_string(source, input_file=str(tmp_path / "main.yml")) == [] included.write_text('{% include "external.yml" %}\n') @@ -280,10 +281,47 @@ def test_jinja_syntax_error_suppression_uses_template_source(): def test_include_suppression_is_per_source_site_not_rendered_line(): source = ( '# use jinja\n{{ "\\n" * 10 }}\n' - '{% include "external.yml" %} # no-dayc: EG106\n---\n' + '{% include "external.yml" %} # no-dayc: WG106\n---\n' '{% include "external.yml" %}\n' ) findings = find_errors_from_string(source) assert len(findings) == 1 - assert findings[0].code == "EG106" + assert findings[0].code == "WG106" assert findings[0].line_number == 5 + + +@pytest.mark.parametrize( + "body", + [ + '{% import "external.yml" as framework %}', + '{% extends "external.yml" %}', + "question: {{ external_setting }}", + ], +) +def test_other_dependency_errors_require_explicit_suppression(tmp_path, body): + path = tmp_path / "main.yml" + args = [str(path), "--no-docx-accessibility"] + source = "# use jinja\n" + body + "\n" + path.write_text(source) + assert main(args) == 1 + findings = find_errors(str(path)) + assert findings[0].code == "EG105" + assert findings[0].severity == "error" + path.write_text( + source.replace("# use jinja\n", "# use jinja\n# no-dayc-block: EG105\n") + ) + assert main(args) == 0 + assert find_errors(str(path)) == [] + + +def test_missing_include_does_not_make_downstream_errors_nonbreaking(tmp_path): + path = tmp_path / "main.yml" + source = '# use jinja\n{% include "external.yml" %}\n---\ncode: |\n broken =\n' + path.write_text(source) + assert main([str(path), "--no-docx-accessibility"]) == 1 + assert {f.code for f in find_errors(str(path))} == {"WG106", "EG122"} + path.write_text( + source.replace("code: |", "# no-dayc-block: EG122\ncode: |") + "# end\n" + ) + assert main([str(path), "--no-docx-accessibility"]) == 0 + assert [f.code for f in find_errors(str(path))] == ["WG106"] From 1ebb3ee37c4948f3dc3fcec6babcffef3edc5885 Mon Sep 17 00:00:00 2001 From: Quinten Steenhuis Date: Mon, 21 Sep 2026 14:09:12 -0400 Subject: [PATCH 04/10] Bound Jinja rendering and preserve nested error locations Run compilation and rendering in an isolated worker with wall, CPU, memory, and output limits. Share an exact first-line directive matcher between validation and fixing, and carry rewritten Jinja traceback locations across the worker boundary for source suppressions. --- README.md | 13 ++- src/dayamlchecker/_jinja.py | 132 +++++++++++++++++++++++++++- src/dayamlchecker/fixer.py | 3 +- src/dayamlchecker/yaml_structure.py | 3 +- tests/test_jinja.py | 128 +++++++++++++++++++++++++++ 5 files changed, 272 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index eb014bd..9533616 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,8 @@ python3 -m dayamlchecker --fix path/to/interview.yml ## Jinja2 preprocessing -Files beginning with `# use jinja` are rendered before the normal validation +Files whose first line is exactly `# use jinja` (LF, CRLF, or end of file) are +rendered before the normal validation pass, following docassemble's [YAML preprocessing feature](https://docassemble.org/docs/interviews.html#jinja2). Expressions, loops, conditionals, macros, and local includes are supported. Include paths are relative to the input file's directory; includes can contain @@ -37,7 +38,15 @@ partial YAML blocks. Ordinary YAML files and Mako expressions are unaffected. This is an offline check: server configuration, `jinja data`, docassemble's special context variables, and package-qualified includes are not supplied. Missing variables, imports, and parent templates produce `EG105`. Rendering -uses Jinja's sandbox and disables HTML escaping. +uses Jinja's sandbox and disables HTML escaping. Compilation and rendering run in +an isolated worker with a 5-second wall timeout, 2-second CPU limit, and 256 MiB +address-space limit. Source and rendered output are limited to 4 MiB each. +Exceeding a limit produces `EG105`. Bounded rendering requires Unix resource-limit +support; other platforms report `EG105` rather than rendering without limits. +Ordinary YAML checking does not require these limits. + +Jinja syntax and runtime errors identify the original template file and line, +including nested local templates, so their source-level suppressions work. Missing `{% include %}` files (including unavailable package-qualified paths) produce warning `WG106`: the included Jinja2 document could not be verified and diff --git a/src/dayamlchecker/_jinja.py b/src/dayamlchecker/_jinja.py index 85e2584..1dc23d6 100644 --- a/src/dayamlchecker/_jinja.py +++ b/src/dayamlchecker/_jinja.py @@ -1,9 +1,14 @@ """The optional docassemble YAML preprocessor, isolated from validation.""" from copy import copy -from dataclasses import dataclass +from dataclasses import asdict, dataclass +import json from pathlib import Path import re +import subprocess +import sys +from tempfile import TemporaryFile +from typing import Any from uuid import uuid4 from jinja2 import ( @@ -17,6 +22,28 @@ from jinja2.compiler import CodeGenerator, Frame from jinja2.sandbox import SandboxedEnvironment +# Applied before any template is compiled, including constant folding. +_RENDER_TIMEOUT = 5 +_MAX_SOURCE_BYTES = 4 * 1024 * 1024 +_MAX_OUTPUT_BYTES = 4 * 1024 * 1024 +_MAX_RESPONSE_BYTES = 32 * 1024 * 1024 +_MEMORY_BYTES = 256 * 1024 * 1024 +_CPU_SECONDS = 2 + + +def uses_jinja(source: str) -> bool: + """Recognize only the documented first-line directive (LF, CRLF, or EOF).""" + return re.match(r"# use jinja(?:\r?\n|$)", source) is not None + + +class JinjaRenderError(Exception): + def __init__( + self, message: str, filename: str | None = None, lineno: int | None = None + ): + super().__init__(message) + self.filename = filename + self.lineno = lineno + @dataclass(frozen=True) class MissingInclude: @@ -67,7 +94,7 @@ def load_include( return self.from_string(self.missing_marker) -def render_yaml( +def _render_yaml( source: str, input_file: str | None = None ) -> tuple[str, list[MissingInclude]]: """Render YAML, blanking documents affected by unavailable includes. @@ -85,7 +112,14 @@ def render_yaml( ) env.missing_includes = [] env.missing_marker = f"DAYAMLCHECKER_MISSING_{uuid4().hex}" - rendered = env.from_string(source).render() + chunks = [] + size = 0 + for chunk in env.from_string(source).generate(): + size += len(chunk.encode("utf-8")) + if size > _MAX_OUTPUT_BYTES: + raise JinjaRenderError("Jinja rendered output exceeds 4 MiB limit") + chunks.append(chunk) + rendered = "".join(chunks) # Match the validator's document boundaries, retaining the separators. parts = re.split(r"(^--- *$)", rendered, flags=re.MULTILINE) rendered = "".join( @@ -93,3 +127,95 @@ def render_yaml( for part in parts ) return rendered, env.missing_includes + + +def render_yaml( + source: str, input_file: str | None = None +) -> tuple[str, list[MissingInclude]]: + """Compile and render in a disposable, resource-limited worker.""" + if len(source.encode("utf-8")) > _MAX_SOURCE_BYTES: + raise JinjaRenderError("Jinja source exceeds 4 MiB limit") + # A file bounds parent memory even if the worker fails while serializing. + with TemporaryFile() as output: + try: + completed = subprocess.run( + [sys.executable, "-I", str(Path(__file__).resolve())], + input=json.dumps({"source": source, "input_file": input_file}).encode(), + stdout=output, + stderr=subprocess.DEVNULL, + timeout=_RENDER_TIMEOUT, + check=False, + ) + except subprocess.TimeoutExpired as exc: + raise JinjaRenderError( + f"Jinja rendering exceeded {_RENDER_TIMEOUT} second time limit" + ) from exc + if completed.returncode: + raise JinjaRenderError( + "Jinja rendering worker stopped (CPU, memory, or response limit " + f"may have been exceeded; exit {completed.returncode})" + ) + output.seek(0) + response = output.read(_MAX_RESPONSE_BYTES + 1) + if len(response) > _MAX_RESPONSE_BYTES: + raise JinjaRenderError("Jinja worker response exceeded size limit") + result = json.loads(response) + if "error" in result: + raise JinjaRenderError(**result["error"]) + return result["rendered"], [MissingInclude(**item) for item in result["missing"]] + + +def _set_resource_limits() -> None: + # Fail closed on platforms without OS-enforced limits; never fall back to + # unbounded in-process rendering. The parent separately enforces wall time. + try: + import resource + except ImportError as exc: + raise JinjaRenderError( + "Bounded Jinja rendering requires Unix resource limits" + ) from exc + for kind, limit in ( + (resource.RLIMIT_AS, _MEMORY_BYTES), + (resource.RLIMIT_CPU, _CPU_SECONDS), + (resource.RLIMIT_FSIZE, _MAX_RESPONSE_BYTES), + ): + _, hard = resource.getrlimit(kind) + if hard != resource.RLIM_INFINITY: + limit = min(limit, hard) + resource.setrlimit(kind, (limit, limit)) + + +def _worker() -> None: + try: + _set_resource_limits() + request = json.load(sys.stdin) + rendered, missing = _render_yaml(request["source"], request["input_file"]) + result: dict[str, Any] = { + "rendered": rendered, + "missing": [asdict(item) for item in missing], + } + except Exception as exc: + filename = getattr(exc, "filename", None) + lineno = getattr(exc, "lineno", None) + if lineno is None: + # Jinja rewrites runtime tracebacks into template-source frames. + # Keep the innermost such frame, not a later Python library frame. + tb = exc.__traceback__ + while tb is not None: + if "__jinja_exception__" in tb.tb_frame.f_globals: + filename = tb.tb_frame.f_code.co_filename + lineno = tb.tb_lineno + tb = tb.tb_next + if filename == "