From ba600b65a0f572d9a1167bf6445edff6fd6f9e30 Mon Sep 17 00:00:00 2001 From: uipreliga Date: Tue, 21 Jul 2026 17:21:36 -0700 Subject: [PATCH] feat(criteria): add pass_context to run_command + sandbox PATH hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opt-in pass_context: true on the run_command criterion serializes the in-flight EvaluationResult (task.json schema) to a temp file and points the scoring command at it via $CODER_EVAL_CONTEXT, giving scoring scripts the agent trajectory and resolved config — not just the filesystem. Script authors can develop offline against a task.json from a previous run, then wire the same script in unchanged. Implementation: - New extra_env passthrough on Sandbox.run_command (last-wins), guarded so it cannot clobber the sandbox-isolation floor (PATH / VIRTUAL_ENV / NODE_PATH / NPM_CONFIG_PREFIX). Also corrects the stale Raises: TimeoutExpired docstring (run_command returns (-1,"",msg)). - pass_context threads Orchestrator -> SuccessChecker -> CheckContext (new run_result field) -> RunCommandChecker; _check_impl's signature is unchanged (state rides on CheckContext). _serialize_context clears success_criteria_results via a non-mutating model_copy so scripts can't depend on list position or leak prior-turn verdicts; the temp dir is deleted on every exit path incl. the timeout return. Missing run_result fails closed to score=0.0. - Security: node_modules/.bin (agent-writable) is now APPENDED, not prepended, to PATH so a planted interpreter shim (python/node/sh/uv) cannot hijack scoring commands. Docs (TASK_DEFINITION_GUIDE + CLAUDE.md) updated; criterion count stays 14. Full suite green, 90.91% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 2 +- docs/TASK_DEFINITION_GUIDE.md | 46 +++++++ src/coder_eval/criteria/base.py | 11 +- src/coder_eval/criteria/run_command.py | 45 +++++- src/coder_eval/evaluation/checker.py | 38 +++++- src/coder_eval/models/criteria.py | 18 +++ src/coder_eval/orchestrator.py | 3 + src/coder_eval/sandbox.py | 50 ++++++- tests/test_pass_context.py | 93 +++++++++++++ tests/test_run_command_stdout.py | 181 ++++++++++++++++++++++++- tests/test_sandbox.py | 102 ++++++++++++++ tests/test_sandbox_security.py | 48 +++++++ 12 files changed, 623 insertions(+), 14 deletions(-) create mode 100644 tests/test_pass_context.py diff --git a/CLAUDE.md b/CLAUDE.md index 5c5cf392..49ac97bc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -149,7 +149,7 @@ templates/ # Sandbox template directories | `file_contains` | Fractional | String presence/absence | | `file_check` | Fractional | Unified file existence + content + regex check | | `json_check` | Fractional | JSON validation + JSON Schema + JMESPath assertions | -| `run_command` | Binary / Continuous | Command exit code + optional stdout matching or float scoring | +| `run_command` | Binary / Continuous | Command exit code + optional stdout matching or float scoring; opt-in `pass_context: true` exposes the in-flight `EvaluationResult` (task.json schema) at `$CODER_EVAL_CONTEXT` for the scoring script | | `file_matches_regex` | Binary | Regex match on file | | `reference_comparison` | Continuous | AST/token/complexity similarity | | `command_executed` | Fractional | Agent tool usage verification | diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index e86c3c02..c2a8fe91 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -539,6 +539,13 @@ Runs a command and checks the exit code, with optional stdout matching. **Binary expected_stdout: "Hello, World!" # Optional: check stdout content stdout_match: "exact" # "exact" (default), "contains", or "regex" description: "Script must output the correct text" + +# Give the scoring script the agent trajectory + resolved config +- type: "run_command" + command: "python score.py" # reads json.load(open(os.environ["CODER_EVAL_CONTEXT"])) + score_from_stdout: true + pass_context: true # expose the in-flight record at $CODER_EVAL_CONTEXT + description: "Score derived from the trajectory" ``` | Field | Default | Description | @@ -548,6 +555,45 @@ Runs a command and checks the exit code, with optional stdout matching. **Binary | `expected_exit_code` | 0 | Expected exit code | | `expected_stdout` | `null` | When set, stdout is also checked | | `stdout_match` | `"exact"` | Match mode: `exact` (stripped), `contains` (substring), `regex` (pattern) | +| `pass_context` | `false` | Expose the in-flight evaluation record as JSON at `$CODER_EVAL_CONTEXT` (see below) | + +#### `pass_context` — the evaluation record as `$CODER_EVAL_CONTEXT` + +With `pass_context: true`, the command runs with an env var `CODER_EVAL_CONTEXT` +pointing at a JSON file. Its schema is **exactly** `task.json` (an +`EvaluationResult`): `iterations` (the full agent trajectory, including +`commands` and `messages`), `task_config` (the resolved `TaskDefinition` plus +per-key merge lineage), `command_stats`, `total_token_usage`, `agent_config`, +`environment_info`, and `early_stop`. This lets a scoring script see the +trajectory and resolved config, not just the filesystem. + +```python +# score.py +import json, os + +ctx = json.load(open(os.environ["CODER_EVAL_CONTEXT"])) +print(min(1.0, len(ctx["iterations"]) / 3)) # first line = the score +``` + +Because the payload is the `task.json` format, you can **develop the script +offline** against a real `task.json` from a previous run — zero agent runs, zero +API cost — then wire the same script in unchanged. + +Two rules to respect: + +- **`success_criteria_results` is always `[]`** in this payload. Criteria are + checked in list order, so exposing already-computed verdicts would make a + script silently depend on its position in the YAML. Do not read it. +- Fields finalized only *after* the criteria phase are provisional: don't read + them. `weighted_score`, `completed_at`, and `duration_seconds` are `null`, and + `final_status` is still a placeholder (typically `"FAILURE"`), not the real + outcome. Everything else is fully populated. On the evaluate-only path (no + agent), `iterations` is `[]` but `task_config` is present, so tolerate an + empty `iterations`. + +The file is written to a private temp dir (never inside the sandbox work dir, so +it is not archived with preserved sandboxes) and deleted immediately after the +command completes. ### `file_matches_regex` diff --git a/src/coder_eval/criteria/base.py b/src/coder_eval/criteria/base.py index 6d6753c2..4a6e5215 100644 --- a/src/coder_eval/criteria/base.py +++ b/src/coder_eval/criteria/base.py @@ -17,7 +17,7 @@ if TYPE_CHECKING: from pathlib import Path - from coder_eval.models.results import TurnRecord + from coder_eval.models.results import EvaluationResult, TurnRecord from coder_eval.models.routing import ApiRoute from coder_eval.sandbox import Sandbox @@ -39,10 +39,19 @@ class CheckContext: Non-judge checkers receive it too (uniform ``_check_impl`` signature) and ignore it. + + ``run_result`` carries the *in-flight* :class:`EvaluationResult` — the same + object the orchestrator is populating for the current attempt, not a finished + record. It is present for the criteria phase (its ``iterations`` / + ``task_config`` are set), but fields computed after checking + (``weighted_score`` / ``final_status`` / ``completed_at``) are still + provisional. ``run_command`` with ``pass_context=true`` serializes it for the + scoring script; other checkers ignore it. """ route: "ApiRoute | None" = None reference_dir: "Path | None" = None + run_result: "EvaluationResult | None" = None def handle_criterion_errors(func: Callable[..., CriterionResult]) -> Callable[..., CriterionResult]: diff --git a/src/coder_eval/criteria/run_command.py b/src/coder_eval/criteria/run_command.py index a1c3ee13..0912ed37 100644 --- a/src/coder_eval/criteria/run_command.py +++ b/src/coder_eval/criteria/run_command.py @@ -3,6 +3,8 @@ import logging import math import re +import tempfile +from pathlib import Path from typing import TYPE_CHECKING from coder_eval.criteria.base import BaseCriterion, CheckContext, register_criterion @@ -10,11 +12,28 @@ if TYPE_CHECKING: - from coder_eval.models.results import TurnRecord + from coder_eval.models.results import EvaluationResult, TurnRecord from coder_eval.sandbox import Sandbox logger = logging.getLogger(__name__) +# Env var name pointing the scoring command at the serialized context file. +# Mirrors the framework-owned TASK_DIR convention (sandbox._build_run_command_env). +_CONTEXT_ENV_VAR = "CODER_EVAL_CONTEXT" + + +def _serialize_context(run_result: "EvaluationResult") -> str: + """Render the in-flight evaluation record as task.json-shaped JSON. + + ``success_criteria_results`` is force-cleared: criteria are checked in list + order, so leaking already-computed verdicts would make scripts silently + depend on their position in the task YAML — and in simulation every_turn + mode the field still holds the *previous* turn's results. ``model_copy`` is a + shallow copy, so the large ``iterations`` list is shared, not duplicated, and + the caller's ``run_result`` is never mutated. + """ + return run_result.model_copy(update={"success_criteria_results": []}).model_dump_json() + # Per-stream output budget included in criterion details. Large enough for # typical tool diagnostics (e.g. uip --output json, pytest tracebacks) while @@ -65,7 +84,29 @@ def _check_impl( ) -> CriterionResult: """Execute command and dispatch to the appropriate scoring method.""" logger.debug(f"Running command for criterion '{criterion.description}': {criterion.command}") - exit_code, stdout, stderr = sandbox.run_command(criterion.command, timeout=criterion.timeout) + + if not criterion.pass_context: + exit_code, stdout, stderr = sandbox.run_command(criterion.command, timeout=criterion.timeout) + else: + run_result = context.run_result if context is not None else None + if run_result is None: + raise ValueError( + "pass_context=true requires the in-flight EvaluationResult, but none was " + + "supplied. It is threaded from the Orchestrator via CheckContext.run_result; " + + "a SuccessChecker constructed outside a run must pass run_result= explicitly." + ) + # tempfile (not the sandbox work dir): preserved sandboxes are archived + # to blob storage, and a context file dropped in the work dir would ride + # along. The with-block deletes the dir on every exit path, including the + # timeout return (run_command returns (-1, "", msg), it does not raise). + with tempfile.TemporaryDirectory(prefix="coder-eval-ctx-") as tmpdir: + context_path = Path(tmpdir) / "context.json" + context_path.write_text(_serialize_context(run_result), encoding="utf-8") + exit_code, stdout, stderr = sandbox.run_command( + criterion.command, + timeout=criterion.timeout, + extra_env={_CONTEXT_ENV_VAR: str(context_path)}, + ) if criterion.score_from_stdout: return self._score_from_stdout(criterion, exit_code, stdout, stderr) diff --git a/src/coder_eval/evaluation/checker.py b/src/coder_eval/evaluation/checker.py index 91f3b5da..8bb7b447 100644 --- a/src/coder_eval/evaluation/checker.py +++ b/src/coder_eval/evaluation/checker.py @@ -12,7 +12,14 @@ from ..criteria import BaseCriterion, CriterionRegistry, init_criteria from ..criteria.base import CheckContext from ..errors import JudgeInfrastructureError -from ..models import CriteriaResults, CriterionResult, SuccessCriteria, SuccessCriterion, TurnRecords +from ..models import ( + CriteriaResults, + CriterionResult, + EvaluationResult, + SuccessCriteria, + SuccessCriterion, + TurnRecords, +) from ..sandbox import Sandbox @@ -93,6 +100,10 @@ def __init__( self._reference_dir: Path | None = None # Cached turn records - set by check()/check_all() when provided self._turn_records: TurnRecords | None = None + # Cached in-flight EvaluationResult - set by check()/check_all() when provided. + # Forwarded to run_command's pass_context=true so a scoring script can read the + # trajectory + resolved config; ignored by every other criterion. + self._run_result: EvaluationResult | None = None self.route = route # V3: Lazy initialization - registry loaded here, not at import @@ -105,6 +116,7 @@ def check( reference_code: str | None = None, turn_records: TurnRecords | None = None, reference_dir: Path | None = None, + run_result: EvaluationResult | None = None, ) -> CriterionResult: """Check a single criterion (backward compatibility wrapper). @@ -116,6 +128,9 @@ def check( reference_dir: Optional resolved path to a reference directory (from ``task.reference.directory``). Only consumed by ``agent_judge``; non-judge criteria ignore it. + run_result: Optional in-flight :class:`EvaluationResult`. Only + consumed by ``run_command`` with ``pass_context=true``; other + criteria ignore it. Returns: CriterionResult with score @@ -127,10 +142,13 @@ def check( self._reference_dir = reference_dir if turn_records is not None: self._turn_records = turn_records + if run_result is not None: + self._run_result = run_result ref_code = reference_code if reference_code is not None else self._reference_code ref_dir = reference_dir if reference_dir is not None else self._reference_dir records = turn_records if turn_records is not None else self._turn_records - return self._check_single(criterion, ref_code, records, ref_dir) + result = run_result if run_result is not None else self._run_result + return self._check_single(criterion, ref_code, records, ref_dir, result) def check_all( self, @@ -138,6 +156,7 @@ def check_all( reference_code: str | None = None, turn_records: TurnRecords | None = None, reference_dir: Path | None = None, + run_result: EvaluationResult | None = None, ) -> CriteriaResults: """Check all success criteria. @@ -147,6 +166,9 @@ def check_all( turn_records: Optional turn records for command inspection reference_dir: Optional resolved path to a reference directory. Only consumed by ``agent_judge``; non-judge criteria ignore it. + run_result: Optional in-flight :class:`EvaluationResult`. Only + consumed by ``run_command`` with ``pass_context=true``; other + criteria ignore it. Returns: List of criterion results with scores @@ -158,13 +180,16 @@ def check_all( self._reference_dir = reference_dir if turn_records is not None: self._turn_records = turn_records + if run_result is not None: + self._run_result = run_result ref_code = reference_code if reference_code is not None else self._reference_code ref_dir = reference_dir if reference_dir is not None else self._reference_dir records = turn_records if turn_records is not None else self._turn_records + result = run_result if run_result is not None else self._run_result results = [] for criterion in criteria: - result = self._check_single(criterion, ref_code, records, ref_dir) - results.append(result) + r = self._check_single(criterion, ref_code, records, ref_dir, result) + results.append(r) return results def _get_checker_instance(self, criterion_type: str) -> BaseCriterion[Any]: @@ -187,6 +212,7 @@ def _check_single( reference_code: str | None, turn_records: TurnRecords | None = None, reference_dir: Path | None = None, + run_result: EvaluationResult | None = None, ) -> CriterionResult: """Check a single criterion using registered checker. @@ -195,6 +221,8 @@ def _check_single( reference_code: Optional reference code (string form) turn_records: Optional turn records for command inspection reference_dir: Optional resolved path to a reference directory. + run_result: Optional in-flight EvaluationResult (run_command's + pass_context=true consumes it; other criteria ignore it). Returns: CriterionResult with score @@ -205,7 +233,7 @@ def _check_single( try: # Get cached instance checker = self._get_checker_instance(criterion_type) - context = CheckContext(route=self.route, reference_dir=reference_dir) + context = CheckContext(route=self.route, reference_dir=reference_dir, run_result=run_result) result = checker.check( criterion, self.sandbox, diff --git a/src/coder_eval/models/criteria.py b/src/coder_eval/models/criteria.py index 1bb59c8f..44ba7bfd 100644 --- a/src/coder_eval/models/criteria.py +++ b/src/coder_eval/models/criteria.py @@ -198,6 +198,13 @@ class RunCommandCriterion(BaseSuccessCriterion): command: "python score.py" score_from_stdout: true description: "Similarity score from scoring script" + + # Expose the in-flight evaluation record to the scoring script + - type: "run_command" + command: "python score.py" # reads json.load(open(os.environ["CODER_EVAL_CONTEXT"])) + score_from_stdout: true + pass_context: true + description: "Score derived from the agent trajectory + resolved config" """ type: Literal["run_command"] = "run_command" @@ -219,6 +226,17 @@ class RunCommandCriterion(BaseSuccessCriterion): "Mutually exclusive with expected_stdout." ), ) + pass_context: bool = Field( + default=False, + description=( + "Expose the in-flight evaluation record to the command as a JSON file at " + "$CODER_EVAL_CONTEXT. Same schema as task.json (EvaluationResult), so a scoring " + "script can be developed offline against a task.json from a previous run. " + "success_criteria_results is always empty; weighted_score / completed_at / " + "duration_seconds are null and final_status is still a placeholder (e.g. FAILURE) " + "because these are only finalized after the criteria phase — do not read them." + ), + ) @model_validator(mode="after") def check_score_from_stdout_exclusivity(self) -> RunCommandCriterion: diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index b1581470..57eb14eb 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -1352,6 +1352,7 @@ async def _evaluation_loop(self) -> bool: reference_code=reference_code, reference_dir=reference_dir, turn_records=self.result.iterations, + run_result=self.result, ) self.result.success_criteria_results = criteria_results return self.result.all_criteria_passed(self.task.success_criteria) @@ -1411,6 +1412,7 @@ async def _evaluation_loop(self) -> bool: reference_code=reference_code, reference_dir=reference_dir, turn_records=self.result.iterations, + run_result=self.result, ) self.result.success_criteria_results = criteria_results @@ -1504,6 +1506,7 @@ async def _run_dialog_criteria_check( reference_code=reference_code, reference_dir=reference_dir, turn_records=self.result.iterations, + run_result=self.result, ) self._accumulate_judge_usage(criteria_results, judge_usage_accum) self.result.success_criteria_results = criteria_results diff --git a/src/coder_eval/sandbox.py b/src/coder_eval/sandbox.py index 07217061..c60c72d2 100644 --- a/src/coder_eval/sandbox.py +++ b/src/coder_eval/sandbox.py @@ -62,6 +62,14 @@ ".wget-hsts", ) +# Env vars that constitute the sandbox-isolation floor built by +# ``_build_run_command_env``: the PATH ordering (agent-writable node_modules/.bin +# is deliberately appended, not prepended, so a planted interpreter shim cannot +# hijack a scoring command) and the Node resolution isolation (MST-9674). +# ``run_command``'s ``extra_env`` must NOT silently clobber these — an unconditional +# floor, mirroring the plan's JUDGE_SECURITY_IGNORE_FLOOR principle. +_PROTECTED_RUN_COMMAND_ENV_KEYS = frozenset({"PATH", "VIRTUAL_ENV", "NODE_PATH", "NPM_CONFIG_PREFIX"}) + def _grant_read_traverse(root: Path) -> None: """Recursively apply ``chmod a+rX`` semantics under ``root``. @@ -809,7 +817,13 @@ def _build_run_command_env(self) -> dict[str, str]: prepend duplicates the entry. Harmless on every OS we target; left explicit so the order stays independent of what the agent SDK happens to inject. - 4. Prepend ``/node_modules/.bin`` to PATH (if present). + 4. **Append** ``/node_modules/.bin`` to PATH (if present). + Appended, not prepended: that directory is agent-writable, so a + shim planted there (``python``/``node``/``sh``/``uv``) must not win a + name collision against the venv/host interpreter (shim hijack). + Package-local CLIs (``eslint``/``tsc``/…) have no host/venv collision, + so appending still resolves them; only a collision — which is exactly + the attack — flips to the trusted binary. 5. (MST-9674) Pin ``NODE_PATH=""`` so Node's fallback search paths cannot pick up contaminated parent-dir installs. Note: this does NOT disable parent-walking from cwd — that is hard-wired @@ -830,7 +844,11 @@ def _build_run_command_env(self) -> dict[str, str]: env["PATH"] = f"{self._venv_scripts_dir}{os.pathsep}{env['PATH']}" node_bin = self.sandbox_dir / "node_modules" / ".bin" if node_bin.exists(): - env["PATH"] = f"{node_bin}{os.pathsep}{env['PATH']}" + # Append (not prepend): node_modules/.bin is agent-writable, so it must + # not win a name collision against the venv/host python|node|sh (shim + # hijack). Package-local CLIs (eslint, tsc, …) have no host collision, so + # appending still resolves them. + env["PATH"] = f"{env['PATH']}{os.pathsep}{node_bin}" # MST-9674: keep Node and npm resolution sandbox-local so concurrent # tasks cannot poison each other through shared parent-dir node_modules. env["NODE_PATH"] = "" @@ -901,19 +919,34 @@ def _check_parent_node_modules_contamination(self) -> list[Path]: ) return offenders - def run_command(self, command: str, timeout: float | int | None = None) -> tuple[int, str, str]: + def run_command( + self, + command: str, + timeout: float | int | None = None, + extra_env: dict[str, str] | None = None, + ) -> tuple[int, str, str]: """Run a command in the sandbox environment. Args: command: Command to execute timeout: Timeout in seconds (uses config default if not specified) + extra_env: Additional environment variables layered on top of the + standard run_command environment (last-wins). Used by criteria + that expose framework-owned paths to the command. Keys in + ``_PROTECTED_RUN_COMMAND_ENV_KEYS`` (PATH / VIRTUAL_ENV / + NODE_PATH / NPM_CONFIG_PREFIX) are the sandbox-isolation floor + and are rejected — ``extra_env`` cannot silently clobber them. Returns: Tuple of (exit_code, stdout, stderr) + Note: + On timeout the method does NOT raise — it returns ``(-1, "", msg)`` + so callers can score a timeout as a normal failure. + Raises: RuntimeError: If sandbox is not set up - subprocess.TimeoutExpired: If command times out + ValueError: If ``extra_env`` tries to override a sandbox-isolation key """ if not self.sandbox_dir: raise RuntimeError("Sandbox not set up. Call setup() first.") @@ -923,6 +956,15 @@ def run_command(self, command: str, timeout: float | int | None = None) -> tuple timeout = self.config.limits.timeout env = self._build_run_command_env() + if extra_env: + protected = _PROTECTED_RUN_COMMAND_ENV_KEYS.intersection(extra_env) + if protected: + raise ValueError( + f"extra_env may not override sandbox-isolation env vars {sorted(protected)}: " + + "these are framework-owned floors (PATH ordering / Node resolution isolation) " + + "that a criterion must not silently clobber." + ) + env.update(extra_env) try: # Shell execution is intentional for sandbox - allows pipes, redirects, and complex commands. diff --git a/tests/test_pass_context.py b/tests/test_pass_context.py new file mode 100644 index 00000000..c2a163c1 --- /dev/null +++ b/tests/test_pass_context.py @@ -0,0 +1,93 @@ +"""End-to-end integration tests for run_command pass_context. + +A real tempdir Sandbox runs a scoring script that reads +``$CODER_EVAL_CONTEXT``, json.load-s it, and derives a score. This exercises +the full contract: Orchestrator-supplied run_result -> SuccessChecker -> +CheckContext -> RunCommandChecker -> Sandbox.run_command(extra_env=...). +""" + +from datetime import datetime + +from coder_eval.criteria.base import CheckContext +from coder_eval.criteria.run_command import RunCommandChecker +from coder_eval.evaluation.checker import SuccessChecker +from coder_eval.models import EvaluationResult, RunCommandCriterion, SandboxConfig +from coder_eval.sandbox import Sandbox + + +def _eval_result(iteration_count: int = 0) -> EvaluationResult: + return EvaluationResult( + task_id="test", + task_description="d", + agent_type="claude-code", + started_at=datetime.now(), + final_status="SUCCESS", + iteration_count=iteration_count, + environment_info={}, + ) + + +# Scoring script written to a file (no bare f-string shell interpolation — rubric §5). +# Prints the score on the first line, derived from the payload it reads back. +_SCORE_SCRIPT = ( + "import json, os\n" + "ctx = json.load(open(os.environ['CODER_EVAL_CONTEXT']))\n" + "assert ctx['success_criteria_results'] == []\n" + "print(1.0 if ctx['task_id'] == 'test' else 0.0)\n" +) + + +def test_end_to_end_script_reads_context_env_var(): + """A script reading $CODER_EVAL_CONTEXT scores through SuccessChecker.check().""" + config = SandboxConfig(driver="tempdir", python=None) + sandbox = Sandbox(config, task_id="test_pass_ctx_e2e") + sandbox.setup() + try: + (sandbox.sandbox_dir / "score.py").write_text(_SCORE_SCRIPT, encoding="utf-8") + criterion = RunCommandCriterion( + command="python score.py", + score_from_stdout=True, + pass_context=True, + description="score from context", + ) + checker = SuccessChecker(sandbox) + result = checker.check(criterion, run_result=_eval_result()) + assert result.score == 1.0 + finally: + sandbox.cleanup(preserve=False) + + +def test_check_all_forwards_run_result_to_context(): + """check_all threads run_result down into CheckContext for the criterion.""" + config = SandboxConfig(driver="tempdir", python=None) + sandbox = Sandbox(config, task_id="test_pass_ctx_check_all") + sandbox.setup() + try: + # A script that scores from len(iterations) proves the actual payload reached it. + script = ( + "import json, os\n" + "ctx = json.load(open(os.environ['CODER_EVAL_CONTEXT']))\n" + "print(min(1.0, len(ctx['iterations']) / 2))\n" + ) + (sandbox.sandbox_dir / "count.py").write_text(script, encoding="utf-8") + criterion = RunCommandCriterion( + command="python count.py", + score_from_stdout=True, + pass_context=True, + description="score from iteration count", + ) + checker = SuccessChecker(sandbox) + # iteration_count is metadata; iterations list is what the script reads (empty here). + results = checker.check_all([criterion], run_result=_eval_result()) + assert len(results) == 1 + assert results[0].score == 0.0 # empty iterations -> 0/2 + finally: + sandbox.cleanup(preserve=False) + + +def test_context_carries_run_result_field(): + """Sanity: CheckContext exposes run_result (used by RunCommandChecker).""" + ctx = CheckContext(run_result=_eval_result()) + assert ctx.run_result is not None + # And the checker consumes it without a real sandbox path leak. + assert RunCommandChecker.criterion_type == "run_command" diff --git a/tests/test_run_command_stdout.py b/tests/test_run_command_stdout.py index dff5a6d0..29cae42c 100644 --- a/tests/test_run_command_stdout.py +++ b/tests/test_run_command_stdout.py @@ -1,13 +1,24 @@ """Tests for run_command stdout matching (absorbed program_stdout_equals).""" +import json +from datetime import datetime +from pathlib import Path from unittest.mock import MagicMock import pytest from pydantic import ValidationError +from coder_eval.criteria.base import CheckContext +from coder_eval.criteria.run_command import _CONTEXT_ENV_VAR as _ENV_VAR from coder_eval.criteria.run_command import RunCommandChecker from coder_eval.evaluation.checker import SuccessChecker -from coder_eval.models import RunCommandCriterion, SandboxConfig, TaskDefinition +from coder_eval.models import ( + CriterionResult, + EvaluationResult, + RunCommandCriterion, + SandboxConfig, + TaskDefinition, +) from coder_eval.sandbox import Sandbox @@ -352,6 +363,174 @@ def test_remaining_lines_in_details(self): assert "line3" in result.details +def _eval_result(**overrides) -> EvaluationResult: + """Minimal in-flight EvaluationResult for pass_context tests.""" + base = { + "task_id": "test", + "task_description": "d", + "agent_type": "claude-code", + "started_at": datetime.now(), + "final_status": "SUCCESS", + "iteration_count": 0, + "environment_info": {}, + } + base.update(overrides) + return EvaluationResult(**base) + + +class TestPassContextModel: + """Model-tier tests for the pass_context field.""" + + def test_pass_context_default_false(self): + c = RunCommandCriterion(command="echo hi", description="d") + assert c.pass_context is False + + def test_pass_context_true_parses_through_task_definition(self): + task = TaskDefinition( + **_task_base(), + success_criteria=[{"type": "run_command", "command": "echo hi", "pass_context": True, "description": "d"}], + ) + assert task.success_criteria[0].pass_context is True + + def test_unknown_sibling_key_still_rejected(self): + """extra='forbid' must not be weakened by the new field.""" + with pytest.raises(ValidationError): + RunCommandCriterion(command="echo hi", description="d", pass_ctx=True) + + +class TestPassContextChecker: + """Checker-tier tests for pass_context (mocked sandbox).""" + + def _sandbox(self, exit_code: int = 0, stdout: str = "", stderr: str = "") -> MagicMock: + s = MagicMock(spec=Sandbox) + s.run_command.return_value = (exit_code, stdout, stderr) + return s + + def test_pass_context_false_no_extra_env(self): + """Default path: run_command is called without extra_env.""" + checker = RunCommandChecker() + c = RunCommandCriterion(command="echo hi", description="d") + sandbox = self._sandbox(0, "hi") + checker._check_impl(c, sandbox, context=CheckContext(run_result=_eval_result())) + _, kwargs = sandbox.run_command.call_args + assert "extra_env" not in kwargs + + def test_pass_context_true_injects_env_and_writes_file(self): + """run_command receives CODER_EVAL_CONTEXT pointing at a readable file at call time.""" + seen: dict = {} + + def _side_effect(command, timeout=None, extra_env=None): + # The temp file only exists during the with-block, so read it here. + path = extra_env[_ENV_VAR] + seen["exists"] = Path(path).exists() + seen["payload"] = Path(path).read_text(encoding="utf-8") + return (0, "1.0", "") + + checker = RunCommandChecker() + c = RunCommandCriterion(command="score", description="d", score_from_stdout=True, pass_context=True) + sandbox = MagicMock(spec=Sandbox) + sandbox.run_command.side_effect = _side_effect + result = checker._check_impl(c, sandbox, context=CheckContext(run_result=_eval_result())) + + _, kwargs = sandbox.run_command.call_args + assert _ENV_VAR in kwargs["extra_env"] + assert seen["exists"] is True + assert result.score == 1.0 + + def test_payload_round_trips_as_evaluation_result(self): + payloads: list[str] = [] + + def _side_effect(command, timeout=None, extra_env=None): + payloads.append(Path(extra_env[_ENV_VAR]).read_text(encoding="utf-8")) + return (0, "1.0", "") + + checker = RunCommandChecker() + c = RunCommandCriterion(command="score", description="d", score_from_stdout=True, pass_context=True) + sandbox = MagicMock(spec=Sandbox) + sandbox.run_command.side_effect = _side_effect + run_result = _eval_result(task_config=None) + checker._check_impl(c, sandbox, context=CheckContext(run_result=run_result)) + + rehydrated = EvaluationResult.model_validate_json(payloads[0]) + assert rehydrated.task_id == "test" + assert rehydrated.iterations == [] + + def test_payload_success_criteria_results_always_empty(self): + """Even when the source has results (simulation every_turn), the payload clears them.""" + payloads: list[str] = [] + + def _side_effect(command, timeout=None, extra_env=None): + payloads.append(Path(extra_env[_ENV_VAR]).read_text(encoding="utf-8")) + return (0, "1.0", "") + + checker = RunCommandChecker() + c = RunCommandCriterion(command="score", description="d", score_from_stdout=True, pass_context=True) + sandbox = MagicMock(spec=Sandbox) + sandbox.run_command.side_effect = _side_effect + prior = CriterionResult(criterion_type="file_exists", description="prev", score=1.0) + run_result = _eval_result(success_criteria_results=[prior]) + checker._check_impl(c, sandbox, context=CheckContext(run_result=run_result)) + + assert json.loads(payloads[0])["success_criteria_results"] == [] + + def test_source_result_not_mutated(self): + checker = RunCommandChecker() + c = RunCommandCriterion(command="score", description="d", score_from_stdout=True, pass_context=True) + prior = CriterionResult(criterion_type="file_exists", description="prev", score=1.0) + run_result = _eval_result(success_criteria_results=[prior]) + checker._check_impl(c, self._sandbox(0, "1.0"), context=CheckContext(run_result=run_result)) + assert run_result.success_criteria_results == [prior] + + def test_missing_context_raises_value_error(self): + checker = RunCommandChecker() + c = RunCommandCriterion(command="score", description="d", score_from_stdout=True, pass_context=True) + with pytest.raises(ValueError, match="run_result"): + checker._check_impl(c, self._sandbox(0, "1.0"), context=None) + + def test_context_with_none_run_result_raises_value_error(self): + checker = RunCommandChecker() + c = RunCommandCriterion(command="score", description="d", score_from_stdout=True, pass_context=True) + with pytest.raises(ValueError, match="run_result"): + checker._check_impl(c, self._sandbox(0, "1.0"), context=CheckContext(run_result=None)) + + def test_missing_context_via_public_check_scores_zero(self): + """Through check() (with @handle_criterion_errors), a missing run_result -> score 0.0 + error.""" + config = SandboxConfig(driver="tempdir", python=None) + sandbox = Sandbox(config, task_id="test_pass_ctx_missing") + sandbox.setup() + try: + c = RunCommandCriterion(command="echo 1.0", description="d", score_from_stdout=True, pass_context=True) + # SuccessChecker built with no run_result -> CheckContext.run_result is None. + result = SuccessChecker(sandbox).check(c) + assert result.score == 0.0 + assert result.error is not None + assert "run_result" in result.error + finally: + sandbox.cleanup(preserve=False) + + def test_temp_dir_removed_even_on_timeout_return(self): + """run_command returning the timeout tuple must still leave no temp dir behind.""" + captured: dict = {} + + def _side_effect(command, timeout=None, extra_env=None): + captured["dir"] = Path(extra_env[_ENV_VAR]).parent + return (-1, "", "timed out") # timeout path returns, does not raise + + checker = RunCommandChecker() + c = RunCommandCriterion(command="score", description="d", score_from_stdout=True, pass_context=True) + sandbox = MagicMock(spec=Sandbox) + sandbox.run_command.side_effect = _side_effect + checker._check_impl(c, sandbox, context=CheckContext(run_result=_eval_result())) + assert not captured["dir"].exists() + + def test_pass_context_orthogonal_to_score_from_stdout(self): + """pass_context + score_from_stdout still scores from stdout correctly.""" + checker = RunCommandChecker() + c = RunCommandCriterion(command="score", description="d", score_from_stdout=True, pass_context=True) + result = checker._check_impl(c, self._sandbox(0, "0.5\n"), context=CheckContext(run_result=_eval_result())) + assert result.score == 0.5 + + class TestRunCommandStdoutIntegration: """Integration tests with real sandbox.""" diff --git a/tests/test_sandbox.py b/tests/test_sandbox.py index c606fa92..e89134bf 100644 --- a/tests/test_sandbox.py +++ b/tests/test_sandbox.py @@ -157,6 +157,108 @@ def test_sandbox_run_command_task_dir_absent(): sandbox.cleanup() +def test_run_command_extra_env_none_leaves_env_unchanged(): + """extra_env=None is the no-op default: the env matches _build_run_command_env.""" + config = SandboxConfig(driver="tempdir") + sandbox = Sandbox(config, task_id="test_extra_env_none", task_dir=Path("/some/task/dir")) + try: + sandbox.setup() + baseline = sandbox._build_run_command_env() + # TASK_DIR is present in the baseline; extra_env=None must not disturb it. + assert "TASK_DIR" in baseline + exit_code, stdout, _stderr = sandbox.run_command( + "python -c \"import os; print(os.environ.get('TASK_DIR', 'NOT_SET'))\"", + extra_env=None, + ) + assert exit_code == 0 + assert stdout.strip() == str(Path("/some/task/dir")) + finally: + sandbox.cleanup() + + +def test_run_command_extra_env_visible_to_command(): + """A variable passed via extra_env is readable by the spawned command.""" + config = SandboxConfig(driver="tempdir") + sandbox = Sandbox(config, task_id="test_extra_env_visible") + try: + sandbox.setup() + exit_code, stdout, _stderr = sandbox.run_command( + "python -c \"import os; print(os.environ['FOO'])\"", + extra_env={"FOO": "bar"}, + ) + assert exit_code == 0 + assert stdout.strip() == "bar" + finally: + sandbox.cleanup() + + +def test_run_command_extra_env_does_not_clobber_standard_vars(): + """extra_env layers on top: unrelated standard vars (TASK_DIR) survive.""" + config = SandboxConfig(driver="tempdir") + sandbox = Sandbox(config, task_id="test_extra_env_keeps_task_dir", task_dir=Path("/some/task/dir")) + try: + sandbox.setup() + exit_code, stdout, _stderr = sandbox.run_command( + "python -c \"import os; print(os.environ['FOO'], os.environ['TASK_DIR'])\"", + extra_env={"FOO": "bar"}, + ) + assert exit_code == 0 + assert stdout.strip() == f"bar {Path('/some/task/dir')}" + finally: + sandbox.cleanup() + + +@pytest.mark.parametrize("key", ["PATH", "VIRTUAL_ENV", "NODE_PATH", "NPM_CONFIG_PREFIX"]) +def test_run_command_extra_env_rejects_protected_keys(key): + """extra_env must not silently clobber the sandbox-isolation floor.""" + config = SandboxConfig(driver="tempdir", python=None) + sandbox = Sandbox(config, task_id="test_extra_env_protected") + try: + sandbox.setup() + with pytest.raises(ValueError, match="sandbox-isolation"): + sandbox.run_command("python -c \"print('x')\"", extra_env={key: "/attacker/controlled"}) + finally: + sandbox.cleanup() + + +def test_node_modules_bin_appended_after_venv_and_host_path(): + """node_modules/.bin must be LAST on PATH (appended), after venv + host. + + Platform-independent: asserts on the string built by _build_run_command_env + rather than executing anything. + """ + config = SandboxConfig(driver="tempdir") + sandbox = Sandbox(config, task_id="test_path_order") + try: + sandbox_dir = sandbox.setup() + (sandbox_dir / "node_modules" / ".bin").mkdir(parents=True, exist_ok=True) + env = sandbox._build_run_command_env() + parts = env["PATH"].split(os.pathsep) + node_bin = str(sandbox_dir / "node_modules" / ".bin") + assert node_bin in parts + # Appended => it is the final PATH entry, so venv scripts and host PATH win. + assert parts.index(node_bin) == len(parts) - 1 + assert parts.index(str(sandbox._venv_scripts_dir)) < parts.index(node_bin) + finally: + sandbox.cleanup() + + +def test_run_command_extra_env_empty_dict_behaves_as_none(): + """extra_env={} is falsy, so the update is skipped — same as None.""" + config = SandboxConfig(driver="tempdir") + sandbox = Sandbox(config, task_id="test_extra_env_empty", task_dir=Path("/some/task/dir")) + try: + sandbox.setup() + exit_code, stdout, _stderr = sandbox.run_command( + "python -c \"import os; print(os.environ.get('TASK_DIR', 'NOT_SET'))\"", + extra_env={}, + ) + assert exit_code == 0 + assert stdout.strip() == str(Path("/some/task/dir")) + finally: + sandbox.cleanup() + + def test_sandbox_run_command_uses_agent_command_base_path(monkeypatch, tmp_path): """Criteria commands can be pinned to the PATH seen by the agent.""" from tests._path_helpers import write_uip_shim diff --git a/tests/test_sandbox_security.py b/tests/test_sandbox_security.py index 7c8e92a2..8dbd4587 100644 --- a/tests/test_sandbox_security.py +++ b/tests/test_sandbox_security.py @@ -4,6 +4,7 @@ """ import os +import stat from pathlib import Path import pytest @@ -13,6 +14,53 @@ from coder_eval.sandbox import Sandbox +def _write_executable(path: Path, body: str) -> None: + """Write an executable shim (POSIX; chmod is a no-op on Windows).""" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body, encoding="utf-8") + path.chmod(path.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX shim semantics; PATH shadowing differs on Windows") +def test_node_modules_bin_python_shim_does_not_hijack_interpreter(): + """An agent-planted node_modules/.bin/python must NOT shadow the real interpreter. + + Regression for the prepend->append fix: node_modules/.bin is agent-writable, + so a shim named `python` there must lose the PATH lookup to the host/venv + python (it is now appended, not prepended). + + Uses the default (venv) config so a real `python` is guaranteed in the + prepended venv scripts dir on any host — the test does not depend on the + host exposing a bare ``python``. + """ + config = SandboxConfig(driver="tempdir") + sandbox = Sandbox(config, task_id="test_shim_hijack") + try: + sandbox_dir = sandbox.setup() + _write_executable(sandbox_dir / "node_modules" / ".bin" / "python", "#!/bin/sh\necho HIJACKED\n") + exit_code, stdout, _stderr = sandbox.run_command("python -c \"print('real')\"") + assert exit_code == 0 + assert "HIJACKED" not in stdout + assert "real" in stdout + finally: + sandbox.cleanup() + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX shim semantics") +def test_node_modules_bin_noncolliding_cli_still_resolvable(): + """A package-local CLI with no host collision stays reachable after the append.""" + config = SandboxConfig(driver="tempdir", python=None) + sandbox = Sandbox(config, task_id="test_compat_cli") + try: + sandbox_dir = sandbox.setup() + _write_executable(sandbox_dir / "node_modules" / ".bin" / "mytool_uniqxyz", "#!/bin/sh\necho TOOL_RAN\n") + exit_code, stdout, _stderr = sandbox.run_command("mytool_uniqxyz") + assert exit_code == 0 + assert "TOOL_RAN" in stdout + finally: + sandbox.cleanup() + + def test_starter_files_rejects_path_traversal(tmp_path): """Test that ../paths in starter files raise security error.