Sandbox template rendering; cover the non-raising failure (#104)

examples/supported/05_reported_failure.yaml

@@ -0,0 +1,56 @@
+# Supported example 05: a step that fails without raising.
+#
+# There are two ways a step fails, and they are not the same shape.
+#
+# 06 covers the first: the step raises, the task ends FAILED, and the error is
+# an exception. This example covers the second, which is easier to miss. A
+# tool can decline its work and return `{"success": false, "error": ...}`
+# without raising anything. The task then ends **completed** -- it did run to
+# the end -- while the step did **not** succeed.
+#
+# Reading `status` alone reports this run as fine. Only the step's `success`
+# flag says otherwise, and the pipeline must take the failure from there: this
+# run exits 1 and reports success=false even though no task is FAILED and
+# nothing raised.
+#
+# `read_missing` asks for a file that is not there. The filesystem tool
+# reports that as a failed result rather than an exception.
+id: reported_failure
+name: Reported Failure
+description: A tool that reports failure without raising, and a run that honours it
+version: "1.0.0"
+
+parameters:
+  out_dir:
+    type: string
+    default: "./output"
+  missing_path:
+    type: string
+    default: "./output/does-not-exist.txt"
+
+steps:
+  - id: before
+    tool: filesystem
+    action: write
+    parameters:
+      path: "{{ out_dir }}/before.txt"
+      content: "this step succeeds"
+
+  # Completes without raising, and reports that it did not succeed.
+  - id: read_missing
+    tool: filesystem
+    action: read
+    on_failure: continue
+    parameters:
+      path: "{{ missing_path }}"
+    dependencies:
+      - before
+
+  - id: after
+    tool: filesystem
+    action: write
+    parameters:
+      path: "{{ out_dir }}/after.txt"
+      content: "the run continued"
+    dependencies:
+      - read_missing

src/orchestrator/_api.py

@@ -417,7 +417,7 @@ async def _resolve_outputs(self, inputs):
         outputs_def = self._extract_outputs()
 
         if outputs_def:
-            from jinja2 import Template
+            from .core.template_sandbox import sandboxed_template
 
             for name, value in outputs_def.items():
                 if isinstance(value, str):
@@ -438,7 +438,7 @@ async def _resolve_outputs(self, inputs):
                     else:
                         # Regular template - render with current context
                         try:
-                            template = Template(value)
+                            template = sandboxed_template(value)
                             outputs[name] = template.render(
                                 inputs=inputs, outputs=outputs
                             )
@@ -475,12 +475,12 @@ async def _resolve_runtime_templates(
 
     async def _resolve_task_templates(self, obj, context):
         """Recursively resolve templates in task parameters."""
-        from jinja2 import Template
+        from .core.template_sandbox import sandboxed_template
 
         if isinstance(obj, str):
             if "{{" in obj and "}}" in obj:
                 try:
-                    template = Template(obj)
+                    template = sandboxed_template(obj)
                     return template.render(**context)
                 except Exception:
                     # If template resolution fails, return original

src/orchestrator/compiler/yaml_compiler.py

@@ -8,7 +8,9 @@
 from typing import Any, Dict, List, Optional
 
 import yaml
-from jinja2 import Environment, StrictUndefined
+from jinja2 import StrictUndefined
+
+from ..core.template_sandbox import create_sandboxed_environment
 
 from ..core.actions import canonical_action
 from ..core.pipeline import Pipeline
@@ -147,7 +149,7 @@ def __init__(
             self.ambiguity_resolver = None
             logger.info("No model registry provided - AUTO tags will be preserved")
 
-        self.template_engine = Environment(undefined=StrictUndefined)
+        self.template_engine = create_sandboxed_environment()
 
         # Add custom filters to Jinja2 environment
         self._register_custom_filters()

src/orchestrator/core/output_tracker.py

@@ -271,9 +271,9 @@ def get_template_variables(self) -> Dict[str, Any]:
 
     def resolve_template_string(self, template: str) -> str:
         """Resolve template string with tracked output values."""
-        from jinja2 import Environment, StrictUndefined
+        from .template_sandbox import create_sandboxed_environment
 
-        env = Environment(undefined=StrictUndefined)
+        env = create_sandboxed_environment()
         template_obj = env.from_string(template)
 
         try:

src/orchestrator/core/template_manager.py

@@ -3,7 +3,11 @@
 import asyncio
 import logging
 from typing import Any, Dict, List, Union, Optional
-from jinja2 import Environment, StrictUndefined, Template, TemplateSyntaxError, UndefinedError
+from jinja2 import StrictUndefined, Template, TemplateSyntaxError, UndefinedError
+
+from jinja2.sandbox import SecurityError
+
+from .template_sandbox import create_sandboxed_environment
 from jinja2.filters import FILTERS
 from datetime import datetime
 import json
@@ -100,7 +104,7 @@ def __init__(self, debug_mode: bool = False, file_inclusion_processor: Optional[
         self.loop_context_manager = loop_context_manager or GlobalLoopContextManager()
 
         # Set up Jinja2 environment with custom filters and undefined handling
-        self.env = Environment(
+        self.env = create_sandboxed_environment(
             undefined=StrictUndefined,
             trim_blocks=True,
             lstrip_blocks=True,
@@ -538,6 +542,12 @@ def render(self, template_string: str, additional_context: Optional[Dict[str, An
                     logger.warning(f"Variable '{undefined_var}' not found in context")
 
             return template_string
+        except SecurityError:
+            # A sandbox violation is never recoverable. Every other render
+            # failure below falls back to returning the template unrendered,
+            # which for this one would hand the caller back the payload it was
+            # refused -- and leave the refusal visible only in a log line.
+            raise
         except Exception as e:
             logger.error(f"Error rendering template: {e}")
             logger.error(f"Error type: {type(e).__name__}")

src/orchestrator/core/template_sandbox.py

@@ -0,0 +1,45 @@
+"""One place that builds Jinja environments, and it builds sandboxed ones.
+
+A pipeline's `{{ }}` expressions are authored with the pipeline. The values
+substituted into them are not: they arrive from `-i name=value`, from an
+inputs file, or from an upstream step's output. Those values are themselves
+rendered, which is a deliberate feature -- `-i out_dir='{{ base }}/reports'`
+is useful -- but it means a parameter value is executed as a template.
+
+Jinja's stock `Environment` executes it with Python's object graph in reach.
+`orchestrator run 01_hello_filesystem.yaml -i greeting='{{ "".__class__ }}'`
+was enough to walk from a string literal to `__subclasses__()`, which is the
+first hop of the standard Jinja sandbox escape. `SandboxedEnvironment` refuses
+that traversal while leaving ordinary expressions -- arithmetic, filters,
+`{{ step.result.content }}` -- working exactly as before.
+
+Every environment on the execution path is built here so a plain
+`Environment(...)` cannot quietly reappear in one of them; `test_template_sandbox.py`
+asserts that none does.
+"""
+
+from __future__ import annotations
+
+from typing import Any
+
+from jinja2 import StrictUndefined, Template
+from jinja2.sandbox import SandboxedEnvironment
+
+
+def create_sandboxed_environment(**kwargs: Any) -> SandboxedEnvironment:
+    """A Jinja environment that will not hand out Python internals.
+
+    `undefined=StrictUndefined` unless the caller overrides it: an unresolved
+    name must fail the render rather than silently become an empty string.
+    """
+    kwargs.setdefault("undefined", StrictUndefined)
+    return SandboxedEnvironment(**kwargs)
+
+
+def sandboxed_template(source: str, **kwargs: Any) -> Template:
+    """A single template, compiled under the sandbox.
+
+    For the call sites that want one template rather than a whole environment.
+    `Template(source)` there would build its own unsandboxed environment.
+    """
+    return create_sandboxed_environment(**kwargs).from_string(source)

src/orchestrator/orchestrator.py

@@ -2339,13 +2339,15 @@ def _extract_outputs(
         output_defs = pipeline.metadata.get("outputs", {})
 
         # Use Jinja2 for template rendering to support filters
-        from jinja2 import Template, TemplateError
+        from jinja2 import TemplateError
+
+        from .core.template_sandbox import sandboxed_template
 
         for output_name, output_expr in output_defs.items():
             try:
                 if isinstance(output_expr, str) and "{{" in output_expr:
                     # Render template with results context
-                    template = Template(output_expr)
+                    template = sandboxed_template(output_expr)
                     # Create a context that includes all step results
                     # Also create objects with .result attribute for backward compatibility
                     context = {}

src/orchestrator/runtime/dependency_resolver.py

@@ -11,7 +11,9 @@
 import re
 from typing import Any, Dict, List, Optional, Set, Tuple, Union
 from dataclasses import dataclass
-from jinja2 import Environment, StrictUndefined, TemplateSyntaxError, UndefinedError, meta
+from jinja2 import StrictUndefined, TemplateSyntaxError, UndefinedError, meta
+
+from ..core.template_sandbox import create_sandboxed_environment
 import ast
 
 from .execution_state import PipelineExecutionState, UnresolvedItem, ItemStatus
@@ -59,7 +61,7 @@ def __init__(self, execution_state: PipelineExecutionState, max_iterations: int
         self.max_iterations = max_iterations
 
         # Initialize Jinja2 environment for template parsing
-        self.jinja_env = Environment(undefined=StrictUndefined)
+        self.jinja_env = create_sandboxed_environment()
 
         # Regex patterns for different types of references
         self.patterns = {

src/orchestrator/validation/data_flow_validator.py

@@ -14,7 +14,9 @@
 import re
 from typing import Any, Dict, List, Optional, Set, Tuple, Union
 from dataclasses import dataclass, field
-from jinja2 import Environment, TemplateSyntaxError, meta
+from jinja2 import TemplateSyntaxError, Undefined, meta
+
+from ..core.template_sandbox import create_sandboxed_environment
 
 logger = logging.getLogger(__name__)
 
@@ -128,7 +130,7 @@ def __init__(self,
         self.tool_validator = tool_validator
 
         # Jinja2 environment for template analysis
-        self.jinja_env = Environment()
+        self.jinja_env = create_sandboxed_environment(undefined=Undefined)
 
         # Pattern for extracting template variables
         self.template_var_pattern = re.compile(r'\{\{\s*([^}]+)\s*\}\}')

tests/test_supported_examples.py

@@ -85,9 +85,16 @@ class Case:
     artifacts: Dict[str, str]
     #: The complete declared `outputs` document, compared exactly.
     outputs: Dict[str, Any]
+    #: Steps that ran to the end AND succeeded.
     completed: Tuple[str, ...]
     skipped: Tuple[str, ...] = ()
+    #: Steps whose task ended FAILED, i.e. something raised.
     failed: Tuple[str, ...] = ()
+    #: Steps that ran to the end and reported `success: false` without raising.
+    #: Their status is "completed" like `completed` above, so status alone
+    #: cannot tell the two apart -- which is the entire point of listing them
+    #: separately.
+    reported_failure: Tuple[str, ...] = ()
     #: Pipeline parameters, passed as `-i k=v` to the CLI and as `context=` to
     #: the API -- the two surfaces must reach the same branch the same way.
     inputs: Dict[str, Any] = field(default_factory=dict)
@@ -178,6 +185,31 @@ def id(self) -> str:
             ("summarise",),
         ),
     ),
+    # The sibling of 06. There, the step raises and the task ends FAILED; here
+    # the tool declines without raising and the task ends *completed*, so the
+    # run is failing while no task is. Nothing else in this suite covers it:
+    # deleting the `reported_failure` branch of StepResult.from_task left the
+    # whole suite green before this case existed.
+    Case(
+        example="05_reported_failure.yaml",
+        variant="default",
+        exit_code=1,
+        artifacts={
+            "output/before.txt": "this step succeeds",
+            "output/after.txt": "the run continued",
+        },
+        outputs={},
+        completed=("before", "after"),
+        reported_failure=("read_missing",),
+        levels=(("before",), ("read_missing",), ("after",)),
+        step_fields={
+            "read_missing": {
+                "status": "completed",
+                "success": False,
+                "error": "File not found: ./output/does-not-exist.txt",
+            },
+        },
+    ),
     Case(
         example="06_failure_policy.yaml",
         variant="default",
@@ -286,9 +318,12 @@ def _assert_case_holds(case: Case, payload: Dict[str, Any], run_dir: Path):
     for step_id, step in steps.items():
         by_status.setdefault(step["status"], []).append(step_id)
 
-    assert sorted(by_status.get("completed", [])) == sorted(case.completed), (
+    # Both `completed` and `reported_failure` steps carry status "completed";
+    # only the success flag separates them.
+    ran_to_the_end = sorted(case.completed) + sorted(case.reported_failure)
+    assert sorted(by_status.get("completed", [])) == sorted(ran_to_the_end), (
         f"{case.id}: completed steps were {sorted(by_status.get('completed', []))}, "
-        f"expected {sorted(case.completed)}"
+        f"expected {sorted(ran_to_the_end)}"
     )
     assert sorted(by_status.get("skipped", [])) == sorted(case.skipped), (
         f"{case.id}: skipped steps were {sorted(by_status.get('skipped', []))}, "
@@ -310,6 +345,17 @@ def _assert_case_holds(case: Case, payload: Dict[str, Any], run_dir: Path):
         assert steps[step_id]["success"] is False, (
             f"{case.id}: step {step_id} was expected to fail"
         )
+    # The combination that status alone cannot express, and that nothing else
+    # in this suite exercises: ran to the end, and did not succeed.
+    for step_id in case.reported_failure:
+        assert steps[step_id]["status"] == "completed", (
+            f"{case.id}: step {step_id} was expected to report failure without "
+            f"raising, but its status was {steps[step_id]['status']!r}"
+        )
+        assert steps[step_id]["success"] is False, (
+            f"{case.id}: step {step_id} completed and was reported successful, "
+            f"but it returned a failing result"
+        )
 
     assert payload["success"] is (case.exit_code == 0), (
         f"{case.id}: success={payload['success']} disagrees with the expected "