Skip to content

feat(criteria): pass_context for run_command + node_modules/.bin PATH hardening#35

Open
uipreliga wants to merge 1 commit into
mainfrom
feat/run-command-pass-context
Open

feat(criteria): pass_context for run_command + node_modules/.bin PATH hardening#35
uipreliga wants to merge 1 commit into
mainfrom
feat/run-command-pass-context

Conversation

@uipreliga

Copy link
Copy Markdown
Collaborator

What

Adds an opt-in pass_context: true to the run_command success criterion. When set, the criterion serializes the in-flight EvaluationResult (the 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 the resolved task config, not just the sandbox filesystem.

The decisive payoff: script authors can develop offline against a real task.json from a previous run (zero agent runs, zero API cost), then wire the same script in unchanged.

- 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"

How

  • extra_env on Sandbox.run_command — additional env vars layered last-wins onto the built environment, guarded so it cannot clobber the sandbox-isolation floor (PATH / VIRTUAL_ENV / NODE_PATH / NPM_CONFIG_PREFIX). Also corrects a stale Raises: subprocess.TimeoutExpired docstring (the method returns (-1, "", msg) on timeout).
  • Context threadingOrchestratorSuccessCheckerCheckContext.run_resultRunCommandChecker. _check_impl's signature is unchanged; the new state rides on CheckContext. _serialize_context force-clears success_criteria_results via a non-mutating model_copy, so scripts can't depend on their position in the YAML or leak prior-turn verdicts (simulation every_turn mode). The temp dir is deleted on every exit path including the timeout return, and a missing run_result fails closed to score=0.0.
  • Security hardeningnode_modules/.bin (agent-writable) is now appended, not prepended, to PATH, so an agent that plants an interpreter shim (python/node/sh/uv) there can no longer hijack a scoring command's interpreter.

Notes

  • ⚠️ Behavior change (release note): a bare CLI name (e.g. tsc) present in both node_modules/.bin and on the host/venv PATH now resolves to the host binary instead of the sandbox-local one. A suite-wide spike found zero tasks under tasks/ that depend on this ordering.
  • No new criterion type — the criterion count stays 14. No config-merge, experiments/default.yaml, or models/__init__.py changes (pass_context is a criterion field, not a merge root).
  • Docs updated: docs/TASK_DEFINITION_GUIDE.md (field-table row + worked example + the success_criteria_results-always-empty / provisional-fields rules) and CLAUDE.md.

Testing

  • New coverage: model defaults + extra="forbid" parity, checker-tier (env injection, payload round-trip, empty success_criteria_results, non-mutation, fail-closed, timeout cleanup), end-to-end integration through a real tempdir Sandbox, and sandbox security (interpreter-shim exploit regression + package-local-CLI compatibility + PATH ordering + extra_env protected-key guard).
  • Full suite green — 3447 passed, 90.91% coverage. make verify clean (format / check / pyright / pytest / custom CExxx lint).

🤖 Generated with Claude Code

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) <[email protected]>
@github-actions

Copy link
Copy Markdown

Claude Code is working…

I'll analyze this and get back to you.

View job run


if TYPE_CHECKING:
from coder_eval.models.results import TurnRecord
from coder_eval.models.results import EvaluationResult, TurnRecord

@akshaylive akshaylive left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we should completely decouple eval from the runs. This means, we should write run results to a specific file before invoking the success criteria. That way, custom criteria can look at everything. I.e; the folder structures can be standardized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants