Escape verbatim JSON and defend against jinja in it. Add option to salt role prefix to guard against malicious injection of var names

enroll/jinjaturtle.py

@@ -1,7 +1,9 @@
 from __future__ import annotations
 
 import hashlib
+import os
 import re
+import secrets
 import shutil
 import subprocess  # nosec
 import tempfile
@@ -206,6 +208,61 @@ def jinjify_artifact(
     )
 
 
+def _resolve_var_prefix_salt() -> str:
+    """Return the salt mixed into generated JinjaTurtle variable prefixes.
+
+    Rationale and the security/reproducibility trade-off
+    ----------------------------------------------------
+    The generated variable namespace is ``<role>_jt_<salt>_<src_rel>``. The salt
+    exists as *defence in depth* for one specific attack: a malicious harvested
+    config (e.g. a JSON file) that injects a key referencing an Enroll-declared
+    variable, e.g. ``{{ <role>_jt_<src_rel>_port }}``. To pre-compute that name at
+    harvest-authoring time the attacker must know the salt.
+
+    This is deliberately *not* random-per-run by default. JinjaTurtle already
+    closes the injection directly (it escapes verbatim keys and an independent
+    output-safety gate rejects any Jinja in a JSON key), so the salt is a belt to
+    that suspenders. A random-per-run salt would change every generated variable
+    name on every ``manifest`` run, producing spurious churn in any
+    version-controlled manifest tree and in tools that diff regenerated output.
+    Note this never affected ``enroll diff``, which compares *harvest bundles*
+    (state.json + artifact content hashes) and never reads generated variable
+    names -- but it would have churned a git-tracked Ansible manifest.
+
+    Default behaviour is therefore a fixed, reproducible namespace:
+
+    * ``ENROLL_JINJATURTLE_VAR_SALT=<value>`` -- operator-pinned salt. Use this to
+      make the namespace unpredictable to a config author while staying stable and
+      reproducible across runs/machines (recommended when generating manifests
+      from untrusted harvests into a tracked repo). ``random`` is special-cased to
+      mean "fresh per process".
+    * unset -- a fixed default salt. Output is fully reproducible; security relies
+      on JinjaTurtle's escaping + key gate, which fully close the issue.
+    """
+
+    override = os.environ.get("ENROLL_JINJATURTLE_VAR_SALT")
+    if override is None or override == "":
+        # Stable default: reproducible output, no manifest churn. The vulnerability
+        # is already closed in JinjaTurtle; this fixed namespace is sufficient.
+        return "en"
+    if override == "random":
+        # Opt-in: unpredictable per process (maximally defensive, churns output).
+        return secrets.token_hex(4)
+    return re.sub(r"[^A-Za-z0-9]+", "", override)[:16] or "en"
+
+
+# Cache holder; resolved lazily on first use so an operator (or test) can set
+# ENROLL_JINJATURTLE_VAR_SALT before the first manifest call and have it honoured.
+_VAR_PREFIX_SALT: Optional[str] = None
+
+
+def _var_prefix_salt() -> str:
+    global _VAR_PREFIX_SALT
+    if _VAR_PREFIX_SALT is None:
+        _VAR_PREFIX_SALT = _resolve_var_prefix_salt()
+    return _VAR_PREFIX_SALT
+
+
 def managed_file_var_prefix(role_name: str, src_rel: str) -> str:
     """Return a JinjaTurtle-safe variable prefix for one managed file.
 
@@ -215,9 +272,14 @@ def managed_file_var_prefix(role_name: str, src_rel: str) -> str:
     Always include a ``jt`` namespace and the relative artifact path so harvested
     config keys cannot produce Enroll-owned variables such as
     ``<role>_managed_files``.
+
+    A salt segment is mixed in (see ``_resolve_var_prefix_salt``). It is a fixed,
+    reproducible value by default and can be pinned or randomised via
+    ``ENROLL_JINJATURTLE_VAR_SALT`` as defence in depth against an injected key
+    that references an Enroll-declared variable.
     """
 
-    raw = f"{role_name}_jt_{src_rel}"
+    raw = f"{role_name}_jt_{_var_prefix_salt()}_{src_rel}"
     safe = re.sub(r"[^A-Za-z0-9_]+", "_", raw).strip("_").lower()
     safe = re.sub(r"_+", "_", safe)
     if not safe:

tests/test_jinjaturtle.py

@@ -110,11 +110,16 @@ def test_manifest_uses_jinjaturtle_templates_and_does_not_copy_raw(
         jinjaturtle_mod, "find_jinjaturtle_cmd", lambda: "/usr/bin/jinjaturtle"
     )
 
+    # Pin the per-run var-prefix salt so generated variable names are
+    # deterministic for this test.
+    monkeypatch.setattr(jinjaturtle_mod, "_VAR_PREFIX_SALT", "t")
+    expected_prefix = jinjaturtle_mod.managed_file_var_prefix("foo", "etc/foo.ini")
+
     # Stub jinjaturtle output.
     def fake_run_jinjaturtle(
         jt_exe: str, src_path: str, *, role_name: str, force_format=None
     ):
-        assert role_name == "foo_jt_etc_foo_ini"
+        assert role_name == expected_prefix
         return JinjifyResult(
             template_text=f"[main]\nkey = {{{{ {role_name}_key }}}}\n",
             vars_text=f"{role_name}_key: 1\n",
@@ -134,7 +139,7 @@ def test_manifest_uses_jinjaturtle_templates_and_does_not_copy_raw(
 
     # Defaults should include jinjaturtle vars.
     defaults = (role_dir / "defaults" / "main.yml").read_text(encoding="utf-8")
-    assert "foo_jt_etc_foo_ini_key: 1" in defaults
+    assert f"{expected_prefix}_key: 1" in defaults
 
 
 def test_openssh_paths_are_jinjaturtle_supported_and_forced_to_ssh() -> None:
@@ -171,6 +176,10 @@ def test_jinjify_managed_files_namespaces_multiple_templates(
         )
 
     monkeypatch.setattr(jinjaturtle_mod, "run_jinjaturtle", fake_run_jinjaturtle)
+    monkeypatch.setattr(jinjaturtle_mod, "_VAR_PREFIX_SALT", "t")
+
+    pa = jinjaturtle_mod.managed_file_var_prefix("foo", "etc/foo/a.yaml")
+    pb = jinjaturtle_mod.managed_file_var_prefix("foo", "etc/foo/b.yaml")
 
     templated, vars_text = jinjify_managed_files(
         bundle,
@@ -188,14 +197,14 @@ def test_jinjify_managed_files_namespaces_multiple_templates(
 
     assert templated == {"etc/foo/a.yaml", "etc/foo/b.yaml"}
     assert calls == [
-        ("a.yaml", "foo_jt_etc_foo_a_yaml"),
+        ("a.yaml", pa),
-        ("b.yaml", "foo_jt_etc_foo_b_yaml"),
+        ("b.yaml", pb),
     ]
-    assert "foo_jt_etc_foo_a_yaml_ignore: []" in vars_text
+    assert f"{pa}_ignore: []" in vars_text
-    assert "foo_jt_etc_foo_b_yaml_ignore: []" in vars_text
+    assert f"{pb}_ignore: []" in vars_text
     assert (template_root / "etc" / "foo" / "a.yaml.j2").read_text(
         encoding="utf-8"
-    ) == "ignore: {{ foo_jt_etc_foo_a_yaml_ignore }}\n"
+    ) == f"ignore: {{{{ {pa}_ignore }}}}\n"
 
 
 def test_jinjify_managed_files_rejects_templates_with_missing_defaults(
@@ -254,6 +263,8 @@ def test_jinjify_managed_files_always_namespaces_single_template(
         )
 
     monkeypatch.setattr(jinjaturtle_mod, "run_jinjaturtle", fake_run_jinjaturtle)
+    monkeypatch.setattr(jinjaturtle_mod, "_VAR_PREFIX_SALT", "t")
+    prefix = jinjaturtle_mod.managed_file_var_prefix("foo", "etc/foo.yaml")
 
     templated, vars_text = jinjify_managed_files(
         bundle,
@@ -267,8 +278,8 @@ def test_jinjify_managed_files_always_namespaces_single_template(
     )
 
     assert templated == {"etc/foo.yaml"}
-    assert calls == ["foo_jt_etc_foo_yaml"]
+    assert calls == [prefix]
-    assert "foo_jt_etc_foo_yaml_managed_files: []" in vars_text
+    assert f"{prefix}_managed_files: []" in vars_text
     assert "foo_managed_files:" not in vars_text
 
 
@@ -307,6 +318,50 @@ def test_jinjify_managed_files_rejects_reserved_role_variable_collision(
     assert not (template_root / "etc" / "foo.yaml.j2").exists()
 
 
+def test_malicious_json_key_falls_back_to_raw_copy(monkeypatch, tmp_path: Path):
+    """A harvested JSON config that injects a Jinja construct in an object key
+    must not produce a template. The real JinjaTurtle output-safety gate refuses
+    it (exit 2), so Enroll falls back to copying the raw file verbatim.
+
+    Uses the real jinjaturtle binary when present; skipped otherwise.
+    """
+    import shutil as _shutil
+
+    import pytest
+
+    from enroll.jinjaturtle import jinjify_managed_files
+
+    jt_exe = _shutil.which("jinjaturtle")
+    if not jt_exe:
+        pytest.skip("jinjaturtle binary not on PATH")
+
+    bundle = tmp_path / "bundle"
+    template_root = tmp_path / "templates"
+    artifact = bundle / "artifacts" / "foo" / "etc" / "app.json"
+    artifact.parent.mkdir(parents=True, exist_ok=True)
+    # Injected key references a (predictable) Enroll-declared variable name; the
+    # JSON-key gate must still refuse it regardless of the salt.
+    artifact.write_text(
+        '{ "{{ ansible_hostname }}": "x", "port": 8080 }', encoding="utf-8"
+    )
+
+    templated, vars_text = jinjify_managed_files(
+        bundle,
+        "foo",
+        template_root,
+        [{"path": "/etc/app.json", "src_rel": "etc/app.json"}],
+        jt_exe=jt_exe,
+        jt_enabled=True,
+        overwrite_templates=True,
+        role_name="foo",
+    )
+
+    # No template emitted; the raw file is left to be copied verbatim downstream.
+    assert templated == set()
+    assert vars_text == ""
+    assert not (template_root / "etc" / "app.json.j2").exists()
+
+
 def test_jinjify_artifact_rejects_unsafe_src_rel(monkeypatch, tmp_path: Path):
     from enroll.jinjaturtle import jinjify_artifact