Hand catalog inputs to the team's playbook as data, never as templates (#181)

changelog.d/181-catalog-inputs-as-data.md

@@ -0,0 +1,3 @@
+### Security
+
+- A catalog operation's inputs are never run as an Ansible template: `cloudfall operations propose`, `cloudfall decisions approve` and `cloudfall agent investigate` hand them to the team's playbook in `--extra-vars` with every string marked unsafe (`{"__ansible_unsafe": ...}`), so an input holding `{{ ... }}` reaches the playbook as written, and integers and booleans keep their types. Before, an input such as `{{ lookup('pipe', '...') }}` ran on the management host with the operator's rights when the playbook started, and an honest input holding `{{` failed with a template syntax error. A team playbook that relied on an input being evaluated as Jinja now gets the literal text. The decision record's `inputs` are unchanged, and recordings made before this change replay as before (#181)

sdk/src/cloudfall/decision.py

@@ -30,6 +30,7 @@
 from jsonschema.exceptions import ValidationError
 
 from cloudfall.ansible_api import callback_plugin_path
+from cloudfall.ansible_data import as_ansible_data
 from cloudfall.catalog import RiskLevel, TargetScope
 from cloudfall.domain import ResourceId
 
@@ -870,9 +871,10 @@ def run_command(
     if invocation.pattern is not None:
         argv.extend(("--limit", invocation.pattern))
     if invocation.inputs:
-        argv.extend(
-            ("--extra-vars", json.dumps(dict(invocation.inputs), sort_keys=True))
-        )
+        # Ansible templates extra vars: an input is marked as data so its
+        # text reaches the playbook as written, never run as Jinja (#181).
+        inputs = as_ansible_data(dict(invocation.inputs))
+        argv.extend(("--extra-vars", json.dumps(inputs, sort_keys=True)))
     argv.append(str(invocation.repository / invocation.playbook))
     configuration = invocation.repository / _ANSIBLE_CONFIG_FILE
     team = configuration if configuration.is_file() else None

sdk/src/cloudfall/recording.py

@@ -18,6 +18,7 @@
 from pathlib import Path
 from typing import TYPE_CHECKING, cast
 
+from cloudfall.ansible_data import unmarked
 from cloudfall.decision import HOST_STATS_VARIABLE
 
 if TYPE_CHECKING:
@@ -138,7 +139,13 @@ def _invocation(
     arguments = list(argv)
     pattern = _value(arguments, "--limit")
     extra = _value(arguments, "--extra-vars")
-    inputs = cast("Mapping[str, object]", json.loads(extra)) if extra else {}
+    # The inputs are marked as Ansible data on the argv; a recording keeps
+    # them as the decision record does, unmarked, so they are read back so.
+    inputs = (
+        cast("Mapping[str, object]", json.loads(extra, object_hook=unmarked))
+        if extra
+        else {}
+    )
     playbook = arguments[-1]
     root = f"{repository}/"
     return playbook.removeprefix(root), pattern, inputs

sdk/tests/test_decision.py

@@ -10,6 +10,7 @@
 from typing import TYPE_CHECKING
 
 import pytest
+from cloudfall.ansible_data import unmarked
 from cloudfall.app import app
 from cloudfall.catalog import RiskLevel, TargetScope, load_catalog
 from cloudfall.cli import main
@@ -57,6 +58,33 @@
   playbook: playbooks/health.yml
 """
 
+ECHO = """\
+id: echo
+playbook: playbooks/echo.yml
+risk: read
+targets: fleet
+inputs:
+  message: string
+  count: integer
+  force: boolean
+"""
+
+# Writes the inputs as the play sees them, even in check mode, and their
+# types with them: a number or a boolean marked as text would arrive quoted.
+ECHO_PLAYBOOK = """\
+---
+- hosts: localhost
+  connection: local
+  gather_facts: false
+  tasks:
+    - name: Keep the inputs
+      check_mode: false
+      ansible.builtin.copy:
+        content: "{{{{ dict(message=message, count=count, force=force) | to_json }}}}"
+        dest: {seen}
+        mode: "0644"
+"""
+
 LATER = datetime(2026, 9, 21, 14, 45, 0, tzinfo=UTC)
 LATEST = datetime(2026, 9, 21, 15, 0, 0, tzinfo=UTC)
 
@@ -225,10 +253,48 @@ def test_the_check_run_is_check_mode_with_a_diff(tmp_path: Path) -> None:
     assert "--check" in argv
     assert "--diff" in argv
     assert argv[argv.index("--limit") + 1] == "web-1"
-    assert json.loads(argv[argv.index("--extra-vars") + 1]) == {"version": "1.4.0"}
+    extra_vars = argv[argv.index("--extra-vars") + 1]
+    assert json.loads(extra_vars, object_hook=unmarked) == {"version": "1.4.0"}
     assert argv[-1].endswith("playbooks/deploy.yml")
 
 
+def test_inputs_reach_the_playbook_as_data_never_as_templates(
+    tmp_path: Path,
+) -> None:
+    """A string input holding Jinja is text to the team's playbook (#181).
+
+    Ansible templates extra vars, so an unmarked ``{{ lookup('pipe', ...) }}``
+    would run on the management host when the playbook starts.
+    """
+    marker = tmp_path / "pwned"
+    payload = "{{ lookup('pipe', 'touch " + str(marker) + "') }}"
+    seen = tmp_path / "inputs.json"
+    repository = _repository(tmp_path, echo=ECHO)
+    (repository / "playbooks" / "echo.yml").write_text(
+        ECHO_PLAYBOOK.format(seen=seen), encoding="utf-8"
+    )
+
+    decision = propose(
+        _proposal(
+            repository,
+            "echo",
+            target=None,
+            inputs={"message": payload, "count": 3, "force": True},
+        ),
+        _store(repository),
+        lambda: MOMENT,
+    )
+
+    assert decision.check.exit_code == 0
+    assert not marker.exists()
+    assert json.loads(seen.read_text(encoding="utf-8")) == {
+        "message": payload,
+        "count": 3,
+        "force": True,
+    }
+    assert decision.inputs == {"message": payload, "count": 3, "force": True}
+
+
 def test_the_team_configuration_runs_their_playbook(tmp_path: Path) -> None:
     """Their playbook needs their roles and their connection settings."""
     repository = _repository(tmp_path)

sdk/tests/test_host_stats.py

@@ -184,6 +184,30 @@ def test_a_replay_leaves_what_the_callback_would_have_written(
     assert recorded.check.unchanged == replayed.check.unchanged == ("web-2",)
 
 
+def test_a_replay_matches_inputs_holding_jinja_as_recorded(tmp_path: Path) -> None:
+    """The argv marks inputs as data; the recording keeps them as written (#181)."""
+    repository = _repository(tmp_path)
+    inputs: dict[str, object] = {"version": "{{ 1.4.0 }}", "force": True}
+    recorded = propose(
+        _proposal(repository, inputs=inputs),
+        _store(repository),
+        lambda: MOMENT,
+        _changed_web_1,
+    )
+    replayed_repository = _repository(tmp_path / "replay")
+
+    replayed = propose(
+        _proposal(replayed_repository, inputs=inputs),
+        _store(replayed_repository),
+        lambda: MOMENT,
+        Recording.load(repository / "decisions").runner(replayed_repository),
+    )
+
+    assert recorded.inputs == replayed.inputs == inputs
+    assert replayed.check.exit_code == 0
+    assert replayed.check.changed == ("web-1",)
+
+
 def _changed_web_1(
     _: Sequence[str], environment: Mapping[str, str], diff: Path
 ) -> int: