Skip to content

FEAT: program execution scorer in an isolated sandbox (scorer phase 10b) - #3218

Open
WatchTree-19 wants to merge 3 commits into
microsoft:mainfrom
WatchTree-19:feat/program-execution-scorer
Open

WatchTree-19 wants to merge 3 commits into
microsoft:mainfrom
WatchTree-19:feat/program-execution-scorer

Conversation

@WatchTree-19

Copy link
Copy Markdown
Contributor

Description

This is phase 10b from the scorer roadmap, the executable program behaviour prototype. It adds a scorer that answers "does this program behave as intended for these inputs?" by running the program in an isolated sandbox and judging what it did, rather than what a response says it would do.

It follows the same shape as the file write scorer from phase 10: a scorable for the evidence, a condition for what counts, a source that acquires the evidence, and a pure matcher. Nothing in the existing scorers changes.

What it adds

  • ProgramBehaves condition (runtime_profile, cases), where each ProgramCase pairs a ProgramInvocation (arguments and stdin) with a ProgramResult (exit code, optional stdout and stderr text matchers).
  • ProgramExecutionScorable (scorable_type="program_execution") with an execution_id, the program (a MessageScorable with one text piece, or a ContentScorable), the runtime profile and the invocations. It holds no expected output.
  • ProgramExecutionEvent and ProgramExecutionObservationPayload (kind="program_execution"): one event per invocation with exit code, bounded stdout and stderr with a truncation flag on each, timeout and resource-limit flags, the resolved command, the image digest and the duration, plus the sha256 of the program that ran.
  • SandboxClient protocol, SandboxRun and SandboxExecutionError. The client owns the disposable environment; the source only submits the attempt and waits for it.
  • DockerSandboxClient with DockerRuntimeProfile: each invocation runs in a fresh container with no network, a read-only root, all capabilities dropped, no-new-privileges, an unprivileged user, memory, CPU and pids limits, and the program mounted read-only. Images must be pinned by digest and are never pulled during scoring (--pull never).
  • ProgramExecutionSource and ProgramExecutionScorer (CONDITION_TYPE = ProgramBehaves).
  • A small fix in _ConditionTupleSchema: a condition that uses one nested model in two fields (ProgramResult.stdout and .stderr both take a TextMatcher) is emitted by Pydantic as a $ref, so the conditions JSON schema now resolves the reference and every entry keeps its fields inline. The existing schema tests cover it.

Stimuli and assertions are split before acquisition

The scorer takes only the runtime profile and each invocation from the condition to build the ProgramExecutionScorable, and keeps each expected result for the matcher. The source and the sandbox never see which output passes. A test asserts the expected value never reaches the sandbox.

How the verdict works

  • True when every case ran and passed.
  • False when any case fails, even if other cases did not run. A compile error, nonzero exit, wrong output, timeout or enforced resource limit is observed evidence, so the acquisition is complete and the case fails.
  • Undetermined when the sandbox could not run the program (acquisition ERROR), when it stopped before every case ran (PARTIAL), or when output cut at the retention limit cannot settle an Equals or Regex check. A cut stream can still prove a Contains match. Broken infrastructure therefore never looks like a program that failed its tests.

On the open decision in the roadmap

Direct code content only. The program is the scored text used verbatim; code is not extracted from surrounding prose, so a response that wraps code in explanation fails its cases rather than being guessed at. A message must name exactly one text piece, and a blocked or failed response has no program to run, so the evidence is unavailable.

Reuse and replay

Every scoring call mints a new execution_id, so it is a new attempt. Scoring an explicit ProgramExecutionScorable again reuses that attempt's observation (the source keeps a bounded cache of completed and partial runs; errors are not reused), and the new score links to the stored observation. score_observation_async judges stored output against new expected results without running the program again. Replay with different invocations or a different runtime profile raises NonReplayableObservationError.

Tests and Documentation

  • tests/unit/models/test_program.py (27 tests): validation, round trips through the condition and scorable registries, and the acquisition invariants on Observation.
  • tests/unit/models/test_scorable.py: the union coverage test gains a ProgramExecutionScorable case.
  • tests/unit/score/test_program_execution_scorer.py (48 tests): the matching table, verdicts for pass, fail, timeout, resource limit, sandbox error and early stop, output bounding, the stimuli split, reuse, replay, stored messages and blocked responses, and the inverter keeping undetermined.
  • tests/unit/score/test_docker_sandbox_client.py (18 tests): runs the real client against a fake Docker CLI script, covering the container flags, arguments and stdin, timeout kill, OOM, output caps, a container that never starts, a failure after some cases ran, and container removal.
  • Also run by hand against Docker 29.8.2 with python:3.12-slim: correct program true, wrong output false, syntax error false, infinite loop false (timed out), network access false, write to the root false, OOM false (resource limit), missing image undetermined, and no containers left behind.
  • The existing models, score, memory, docs and executor suites pass alongside. ruff and ty are clean.
  • New notebook doc/code/scoring/7_program_execution_scorer (paired .py and .ipynb, executed), linked from the scoring overview, myst.yml and framework.md. It needs Docker and the pinned image, and no model or credentials.

Adds ProgramBehaves, ProgramExecutionScorable, ProgramExecutionSource,
the SandboxClient protocol with a DockerSandboxClient, and
ProgramExecutionScorer. The scorer sends the source only the runtime
profile and invocations; expected results stay with the scorer. Program
failure is observed evidence, a sandbox failure is undetermined, and a
stored execution can be judged again without running the program.
Comment thread pyrit/score/observation/program_execution_source.py
Comment thread pyrit/score/observation/docker_sandbox_client.py Outdated
Comment thread pyrit/score/observation/docker_sandbox_client.py
Comment thread pyrit/score/observation/docker_sandbox_client.py Outdated
Comment thread pyrit/score/observation/docker_sandbox_client.py Outdated
Comment thread pyrit/score/true_false/program_execution_scorer.py Outdated
- ProgramExecutionSource shares one in-flight run between concurrent callers
  of the same attempt and cancels it only when every caller has gone.
- DockerSandboxClient reports an exit code only for an exited container; a
  client that exits while the container is still running is a sandbox error.
- Docker management commands are killed and reaped on cancellation as well
  as on timeout.
- A failed container removal is reported as a warning with the container
  name, kept in the observation metadata, and stops further cases.
- The read-only program file is made writable before its directory is
  removed, so cleanup also works on Windows.
- ContentEntryScorable is accepted as a program reference, so a saved
  score anchor can be scored again.
@WatchTree-19

Copy link
Copy Markdown
Contributor Author

Thanks Roman, all six addressed in the latest commit, each with a regression test:

  • Concurrent callers for the same attempt now share one in-flight run, cancelled only when every caller has gone.
  • The client inspects the container status and only reports an exit code for an exited container; an attach failure while it is still running is a SandboxExecutionError.
  • Management commands are killed and reaped in a finally, so cancellation no longer leaves one running.
  • A failed rm is reported with the container name, kept in the observation's sandbox_warnings metadata, and stops further cases.
  • The program file is made writable before the directory is removed, covered by a mocked-invocation test that runs on every platform.
  • ContentEntryScorable is accepted as a program reference, with a round-trip test that scores a saved score's anchor.

Comment thread pyrit/score/observation/program_execution_source.py
Comment thread pyrit/score/observation/docker_sandbox_client.py Outdated
Comment thread pyrit/score/true_false/program_execution_scorer.py Outdated
…ations

- DockerSandboxClient uses one deadline for the whole invocation, including the
  attached client's exit. A client still running after its streams close is ended
  at the deadline, and the container's own state then decides the result, so a
  stalled client is neither unbounded nor mistaken for a program timeout.
- ProgramExecutionSource waits for the sandbox's cleanup when the last caller of an
  attempt is cancelled, so cancellation still means the environment is gone.
- Memory links a new observation whose ID is already stored when the stored row is
  identical, checked under the observation row locks; different evidence under the
  same ID is still rejected. ProgramExecutionScorer drops its separate pre-check, so
  concurrent scoring calls that share one acquisition both persist their scores.
@WatchTree-19

Copy link
Copy Markdown
Contributor Author

Thanks Roman, all three are addressed in the latest commit.

  1. The timeout now covers the whole invocation, including the client exiting after its streams close. If the client stalls, it gets killed, and the container's own state decides the result, so a container that is still running comes back as a sandbox error rather than a pass. There are two new tests where a fake client closes stdout and stderr and then hangs, one over a finished container and one over a running one.
  2. When the last caller is cancelled, it now waits for the sandbox cleanup to finish before its own cancellation completes. The test holds cleanup open and checks that cancelling waits for it.
  3. I took out the scorer's pre-check. Memory now links an identical stored observation inside the same transaction, under its row lock, and still rejects a different observation that reuses a stored ID. I added a test with two concurrent score_async calls on real SQLite, held together at the write. It reproduces your error on the previous code and now stores one observation that both scores link to.

"/tmp:rw,nosuid,nodev,size=16m",
"--workdir",
"/tmp",
"--mount",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Must fix: bound Docker's own output logs, not only the retained streams.

max_output_bytes caps the bytes this client keeps, but this docker run command inherits the daemon's logging configuration. Docker's default json-file driver has no log rotation, so container output can still consume host disk space. The read-only root filesystem and memory limit do not constrain these daemon-side files, and the timeout limits duration rather than output volume.

I checked this with a bounded mocked Docker lifecycle and a real local Python producer that wrote exactly 8,192 bytes. With max_output_bytes=128, the stored event kept 128 bytes and set stdout_truncated=True, while the simulated daemon log kept all 8,192 bytes. The captured command had neither --log-driver nor --log-opt. This was not a live-daemon disk-exhaustion test; the mock models the separately documented log routing. Docker documents the default disk-exhaustion risk here: https://docs.docker.com/engine/logging/configure/.

Please disable daemon-side logging for these attached runs with --log-driver none, or enforce a bounded logging configuration. Add a regression assertion on the generated Docker arguments so the output limit does not depend on the host's logging defaults.

AI-assisted review; reproduction run with GitHub Copilot.

def _remember(self, *, scorable: ProgramExecutionScorable, observation: Observation) -> None:
self._observed[scorable] = observation
while len(self._observed) > self._MAX_REUSED_OBSERVATIONS:
self._observed.popitem(last=False)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Should fix: preserve execution identity after cache eviction.

Evicting an observation makes a later acquisition of the same ProgramExecutionScorable execute the program again under the same execution_id. That contradicts the documented contract that reusing the scorable reuses one observed execution, and that a fresh run requires a new ID. This is separate from the concurrent-acquisition issue already fixed.

I reproduced this through public score_async calls with real in-memory SQLite persistence and a fake sandbox whose stdout changes on each call:

  1. Score one explicit execution: True.
  2. Score it again immediately: True, with the same observation and no second sandbox call.
  3. Acquire 256 other distinct successful executions, using the unchanged cache limit.
  4. Score the original execution again: False, with a new observation.

Both final scores persisted successfully. Their execution anchors, including the UUID, were identical, but the sandbox ran the original program twice and the scores referenced different observations. A nondeterministic program can therefore give contradictory results for what the API says is one attempt.

Please make reuse independent of LRU eviction, or fail explicitly when an attempt's evidence is no longer available instead of silently running it again. Keep the memory bound; simply removing eviction would introduce a different problem. Add a regression covering reuse beyond the 256-entry limit.

AI-assisted review; reproduction run with GitHub Copilot.

"--mount",
f"type=bind,source={os.fspath(program_dir)},target=/program,readonly",
profile.image,
*resolved_command,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Should fix: make the runtime profile control the effective entrypoint too.

Putting resolved_command after the image overrides Docker's CMD, but does not override the image's ENTRYPOINT. For an image with ENTRYPOINT ["python3"], the accepted profile command=("python3", "-I") therefore runs python3 python3 -I /program/main.py, not the command recorded in the execution event. Correct submitted code can be scored as a program failure because of the runtime configuration.

I reproduced the result with a controlled Docker-lifecycle mock that applies Docker's documented ENTRYPOINT-plus-command composition and runs the resulting arguments through a real local Python interpreter. For the same print('ok') program and expectation:

  • No image entrypoint: exit 0, verdict True.
  • Image entrypoint ("python3",): exit 2 because Python tries to open python3 as a script, verdict False.
  • Both events recorded the same resolved_command: ("python3", "-I", "/program/main.py"), although the effective commands differed.

The Docker daemon was unavailable, so this was not a live-container test. Docker documents the argument composition here: https://docs.docker.com/engine/containers/run/#default-command-and-options.

Please set --entrypoint explicitly from the trusted profile, or reject incompatible image entrypoints before scoring. Add a profile-with-ENTRYPOINT regression and ensure the recorded command matches what actually runs.

AI-assisted review; reproduction run with GitHub Copilot.

program_dir = Path(tempfile.mkdtemp(prefix="pyrit-program-"))
program_path = program_dir / profile.program_filename
try:
program_path.write_text(program, encoding="utf-8")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Should fix: move filesystem preparation and removal off the event loop.

execute_async directly calls synchronous mkdtemp, write_text, and chmod, and its finally reaches synchronous shutil.rmtree. A slow filesystem operation blocks unrelated scoring tasks and prevents their timers or cancellation handlers from running. These operations are on the async execution path, not constructor-time setup.

I reproduced the impact without Docker: I mocked only _run_invocation_async, injected a bounded 300 ms delay into the real Path.write_text call, and ran an independent 5 ms asyncio heartbeat on the same loop. The largest heartbeat gap increased from 19.0 ms in the control run to 306.5 ms with the delayed write. The program file was still written and cleaned up normally; the delay shows that filesystem latency stalls the shared event loop rather than only this execution.

Please put preparation and removal in synchronous helpers awaited through asyncio.to_thread, or use the existing async file-I/O pattern where appropriate. Preserve the cleanup ordering on cancellation. A slow-file/heartbeat regression would cover this independently of the Docker process tests.

AI-assisted review; reproduction run with GitHub Copilot.

if program_dir.exists():
logger.warning("Could not remove the program directory %s", program_dir)

async def _run_invocation_async(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Simplicity: separate command construction from the process lifecycle.

The overall scorer -> source -> sandbox responsibility split is good. Inside _run_invocation_async, however, one function constructs all Docker arguments, starts the process, coordinates three stream tasks, handles deadlines and two different client-stall paths, inspects container state, removes the container, and builds the event. The nested try/except/finally blocks make it difficult to check which failures are program evidence and which are infrastructure failures.

Please extract a pure run-argument builder and a focused helper that waits for the streams/process against one deadline. Keep resource ownership and cleanup visible in _run_invocation_async; this does not need another class hierarchy. That would make the command contract and timeout policy easier to understand and test independently.

AI-assisted review with GitHub Copilot.

This branch has not been deployed

No deployments
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.

3 participants