Skip to content

feat(docker): add network: llm_only — no internet except the model API - #211

Open
CarlesUIPath wants to merge 18 commits into
mainfrom
feat/docker-llm-only-network
Open

CarlesUIPath wants to merge 18 commits into
mainfrom
feat/docker-llm-only-network

Conversation

@CarlesUIPath

@CarlesUIPath CarlesUIPath commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds sandbox.docker.network: llm_only: a "no internet" mode for driver: docker that still lets the agent, the judges and the simulator reach their model.

  • Why: network: none already exists, but the agent runs inside the container, so none also cuts the model API. A Luna smoke run showed it: bridge = SUCCESS in 9.3 s, none = ERROR after 63.8 s (Reconnecting... waiting for network, then a turn timeout).
  • What changes for users: one opt-in value. The default bridge and none behave exactly as before (the argv is byte-identical, pinned by a test).
  • How: each task gets a private docker network with no route out, plus a small proxy sidecar that forwards only to the model hosts the task needs. Everything else fails closed.
  • Docs: docs/DOCKER_ISOLATION.md § Network modes, docs/agents/HARNESS_PARITY.md, docs/EXTENDING.md (plugin-agent hooks). Design rationale and spike log: .claude/notes/isolation.md § The egress sidecar.

How to use it

Set one field on a docker task:

sandbox:
  driver: docker
  docker:
    network: llm_only

Or from the CLI on any docker task, or once in an experiment's defaults::

coder-eval run task.yaml --driver docker -D sandbox.docker.network=llm_only
# experiment.yaml
defaults:
  sandbox:
    docker:
      network: llm_only

The model hosts are derived from the task, so most tasks need nothing else:

Part of the task Hosts it gets
claude-code agent, llm_judge / agent_judge, simulator the API backend's hosts (Bedrock for AWS_REGION, api.anthropic.com, or the LiteLLM host)
codex the CODEX_BASE_URL host, else api.openai.com
antigravity the Gemini host
pi the host of its openrouter/, anthropic/, openai/ or google/ model prefix
system_one_judge the host of each base_url

Anything else goes in egress_allowlist (host or host:port, append-merged across layers). Examples: a package index, or the provider host for OpenCode, or for Pi on another provider:

    docker:
      network: llm_only
      egress_allowlist: [pypi.org, files.pythonhosted.org]

To see what a task tried to reach, read egress.log in its run directory (grade.egress.log for a grading container), and add each needed DENY host to egress_allowlist:

grep -rhE "^(ALLOW|DENY)" --include=egress.log runs/latest | sort | uniq -c

Requirements:

  • Docker driver. llm_only with another driver is refused before the agent starts.
  • Docker 20.10 or later; 26.0 or later is recommended (CVE-2024-29018 DNS leak).
  • A framework image built from this branch (make docker-image). The proxy sidecar runs from it.

Limits:

  • Server-side tools are outside the boundary. Provider-side tools such as WebSearch on the direct API run outside the container; use disallowed_tools if needed.
  • About 29 parallel tasks. Each task uses one docker network, and Docker Desktop has address pools for about 29, so keep --max-parallel below that.

To try it locally:

make docker-image
coder-eval run tasks/docker_egress_probe/docker_egress_probe.yaml --backend bedrock   # expect 1.000
coder-eval run tasks/docker_egress_probe/docker_egress_probe.yaml --backend bedrock \
  -D sandbox.docker.network=bridge                                                  # control: expect a FAILURE

How it works

For each llm_only task, coder-eval creates:

  • A per-task --internal docker network. It has no route out (inhibit_ipv4=true, --ipv6=false).

  • An egress sidecar on that network. It runs from the framework image and is the only container with a route out. It runs a stdlib-only CONNECT/HTTP proxy (src/coder_eval/egress_proxy.py) that forwards only to an exact host:port allowlist. The sidecar runs as uid 65534, with --cap-drop ALL, a read-only root and ip_forward=0.

  • A task container that joins only the internal network. It gets:

    • HTTPS_PROXY / HTTP_PROXY pointing at the sidecar, and NODE_USE_ENV_PROXY=1
    • a DNS server that never answers, so external names do not resolve
    • no NET_RAW.

    A tool that ignores the proxy settings has no route, so it fails closed.

  • A probe before the agent starts. It sends one CONNECT per allowed host, so a missing host fails fast and no agent run is paid for.

  • Its own egress.log, with one ALLOW / DENY line per request.

The sidecar and the network are removed on every exit path (success, error, Ctrl-C). Grading containers (evaluate, in-container grading) run under the same mode.

The allowlist is least-privilege (the per-part hosts are in the table under "How to use it"):

  • It comes from the same route resolvers the orchestrator uses (resolve_route / resolve_evaluation_route), so it cannot drift from the real routing.
  • The API backend's hosts are added only for components that call the model through it: a claude-code agent, an enabled simulator, and llm_judge / agent_judge.
  • Agent hosts come from BaseAgentConfig.egress_hosts() (codex, antigravity, pi).
  • Anything else must be listed in egress_allowlist.

Fail-closed edges:

  • Harbor export refuses an llm_only task instead of mapping it to a public network.
  • A task with llm_only and a driver other than docker is refused before any agent starts.

Where the lines are

Area Lines added Main files
Source 1,135 isolation/egress.py (allowlist, network, sidecar, cleanup), egress_proxy.py (the proxy), docker_runner.py wiring, models/ fields
Tests 1,072 test_docker_egress_runner.py, test_egress_proxy.py, test_egress_targets.py, test_docker_egress_config.py
Smoke task 312 tasks/docker_egress_probe/ (the end-to-end probe)
Docs and notes 357 docs/DOCKER_ISOLATION.md § Network modes, .claude/notes/isolation.md § The egress sidecar

Review

The whole branch had a code review by three Opus reviewers. Fixed in this PR:

  • High: wrong judge and simulator hosts with a LiteLLM backend. In that case the judges and the simulator are pinned to Bedrock or Direct, but those hosts were not allowed. The derivation now uses the real resolvers.
  • High: a symlink attack on the proxy log. The host appended the proxy log to docker.log in the writable run directory, after the container ran. An agent could replace that file with a symlink to a host file. The log is now a separate file, written with write_text_atomic.
  • Medium: llm_only without driver: docker failed open. It is now refused.
  • 9 Low findings:
    • BAD log lines no longer contain request targets, which can hold credentials.
    • The proxy serves at most 256 connections at once.
    • NET_RAW is dropped from the task container.
    • Proxy staging errors become EgressSetupError.
    • A network that was never created is no longer reported as leaked.
    • The rest: a docstring, a constant defined twice, private names used across modules, and missing docs for plugin agents.

The remaining Low items are recorded in the notes: no tunnel idle timeout, --resume does not retry a transient setup error, a stopped sidecar is left after a host SIGKILL (the prune command is documented), and a container on the shared bridge can reach another task's sidecar (no extra reach, capped).

Test plan

  • make verify: lint, CE rules, pyright, docs-budget, and the test suite (coverage 92.5%). The egress tests were consolidated to 4 files (about 1,030 lines) and pass in a checkout with no .env, as in CI. Locally, 2 tests fail because of a developer .env (ANTIGRAVITY_MODEL, TASK_DIR); they also fail on main.

  • Real llm_only docker runs on Docker Desktop 29 (macOS). Every row below was SUCCESS 1.0 except where stated:

    Scenario Hosts allowed
    Claude Code on Bedrock (eu-north-1 and us-east-2) only that region's 2 Bedrock hosts
    Codex with gpt-5.6-luna on Azure the Azure host only
    Antigravity the Gemini host only
    llm_judge, agent_judge, simulated dialog + judge, dataset fan-out as derived
    LiteLLM agent route + llm_judge the judge's pinned Bedrock hosts
    Pi on Bedrock and on Azure gpt-5.6-luna the provider host (given in egress_allowlist)
    OpenCode on Bedrock and on Azure gpt-5.6-luna (image with the CLI added) the provider host (given in egress_allowlist)
    Claude Code through a host LiteLLM proxy to Azure gpt-5.6-luna host.docker.internal:4000 (plain-HTTP forward)

    Also checked:

    • execute then evaluate: the grading container also runs under llm_only.
    • 4 runs in parallel: unique networks, nothing leaked.
    • Ctrl-C: everything removed within 3.4 s.
  • New task tasks/docker_egress_probe/. The agent tries to reach the internet in 8 ways, and the grading probes then check each path out:

    • the proxy
    • direct IPv4 and IPv6, and the default route
    • DNS, including a raw UDP query to 8.8.8.8
    • raw sockets
    • the docker host and the gateway
    • pip, git, npm and WebFetch.

    llm_only scores 1.000; the bridge control scores 0.176, which shows the probes detect real internet access.

  • Regression: the CI smoke buckets on this branch. smoke-pass is 8/10; the 2 failures need byod-custom-image and TYPESAFE_API_KEY, which this machine does not have. All 3 smoke-fail sentinels fail as they must. Docker bridge tasks pass: Claude, Antigravity, record_cli, and the reference anti-cheat task.

  • Spikes, recorded in the notes:

    • native Linux dockerd 20.10, 24 and 29, with the iptables and nftables backends (emulated with docker:dind)
    • task images: Alpine, Ubuntu, Rocky 9, Fedora 42, Node 20/22, Python, Go.
  • Not verified here:

    • a real Linux host
    • a real Linux OAuth refresh
    • the direct ANTHROPIC_API_KEY route; on it, WebSearch runs on the provider's side and is outside the boundary (documented)
    • Codex on api.openai.com
    • a runtime-kit image (the kit does not build on arm64; that problem is already on main).

Dependency bump (CI only)

uv.lock: urllib3 2.7.0 → 2.8.0 and virtualenv 21.2.0 → 21.7.13 (python-discovery follows). New advisories made the Quality Gate's pip-audit step fail on every branch; the oldest fixed releases are pinned to stay clear of the runner's minimum package age.

Found during testing, already on main (not in this PR)

  • The skillsbench docker samples fail with "Dockerfile not found" when the task loads inside the container.
  • docker/Dockerfile.runtime downloads only linux-x64 Node, so the runtime kit cannot be built on arm64.
  • coder-eval evaluate <row> writes grade.*.log into a new runs/<timestamp>/ directory, not into the row.

🤖 Generated with Claude Code

CarlesUIPath and others added 10 commits October 1, 2026 16:11
…kerDriverConfig

The new mode and its append-merged allowlist validate and merge on every
config layer; normalize_egress_target is the single entry parser. Harbor
export refuses llm_only instead of widening it to public.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…_only

A CONNECT + absolute-form HTTP proxy with an exact host:port allowlist, a
probe subcommand, and a heartbeat watchdog that stops it when the host
dies. It imports only five stdlib modules, so the host can bind-mount it
into the framework image and run it with python3 -I.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
resolve_egress_targets unions the backend hosts, forwarded *_URL hosts,
the agent config's new egress_hosts() hook, system_one_judge base URLs
and the user allowlist. DockerRunError moves to isolation/errors.py and
the loopback rewrite to isolation/egress.py so egress.py has no cycle.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sidecar

DockerRunner.run wraps the task container in egress_scope: an --internal
network with inhibit_ipv4, a locked-down proxy sidecar from the framework
image running the host's bind-mounted egress_proxy.py, a CONNECT probe of
every target, and teardown on every path. The bridge and none argv is
pinned byte for byte. The grading consent text names the mode.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ess task

New Network modes section in DOCKER_ISOLATION, a per-harness network
table in HARNESS_PARITY, the design rationale in .claude/notes, and
tasks/docker_egress_llm_only.yaml, which fails if general internet is
reachable from the container.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Give the llm_only task container a black-hole upstream DNS server so an
unpatched daemon (CVE-2024-29018) cannot tunnel DNS out, create the
internal network with --ipv6=false, never forward ALL_PROXY, extend the
live test to assert DNS, direct-route and host-alias bypasses fail, and
correct the docs (judge api_route hosts, CDN fronting, Docker versions).
Defer four harness candidates.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The backend's hosts are now added only for a component that reaches its
model through API_BACKEND (a claude-code agent, an enabled simulator, an
llm_judge or agent_judge criterion), and a forwarded URL variable counts
only where its owner reads it (LITELLM_BASE_URL for the litellm backend,
CODEX_BASE_URL for codex). Real runs had shown a Claude task reaching the
Azure Codex host and Codex/Antigravity/Pi tasks reaching api.anthropic.com.
Also stops a bad LITELLM_BASE_URL loopback port leaking its value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Derive judge and simulator hosts from resolve_route /
  resolve_evaluation_route, so a LiteLLM run allows the pinned Claude
  backend and a route: litellm judge allows its own api_base.
- Write the sidecar log to its own egress.log with write_text_atomic: a
  symlink the agent plants in the run dir is replaced, never followed,
  and container stdout cannot forge ALLOW/DENY lines. Grading folds it
  back as grade.egress.log.
- Refuse llm_only at host dispatch when the driver is not docker.
- Redact refused request lines, cap the proxy at 256 connections, drop
  NET_RAW from the task container, wrap egress staging errors as
  EgressSetupError, and stop reporting a never-created network as leaked.
- Make the loopback helpers public and keep one LOOPBACK_HOSTS; document
  the plugin-agent hooks in EXTENDING.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The agent tries to reach the internet in its own ways, and the grading
criteria probe each path out from inside the container (proxy, direct
IPv4/IPv6, DNS, raw sockets, the docker host, pip, git, npm) while the
model host must stay reachable. Under network: bridge the same task
fails, which is the control. Also documents the WebFetch domain-check
DENY on Bedrock and that provider-side tools such as WebSearch are
outside the container boundary.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Claude finished @CarlesUIPath's task in 1m 50s —— View job


Code Review in Progress

I'm reviewing this PR for the network: llm_only Docker isolation feature. This adds a new network mode that blocks all internet access except to model API endpoints.

Tasks

  • Read code review guidelines (.github/code_review.md)
  • Read project conventions (CLAUDE.md)
  • Examine the full diff (git diff origin/main...HEAD)
  • Review core implementation files:
    • src/coder_eval/egress_proxy.py (new proxy server)
    • src/coder_eval/isolation/egress.py (egress management)
    • src/coder_eval/isolation/docker_runner.py (Docker integration)
  • Review model and configuration changes
  • Review test coverage and test implementation
  • Check cross-file consistency and integration points
  • Analyze security implications
  • Document findings following review format

Comment thread tests/test_egress_targets.py Fixed
Comment thread tests/test_docker_egress_live.py Fixed
Comment thread tests/test_docker_egress_live.py Fixed
Comment thread tests/test_docker_egress_runner.py Fixed
Comment thread tests/test_docker_egress_runner.py Fixed
Comment thread tests/test_docker_egress_runner.py Fixed
Comment thread tests/test_egress_proxy.py Fixed
CarlesUIPath and others added 6 commits October 2, 2026 14:25
The model-host probe skipped the litellm backend, so it passed without
checking anything there. It now reads LITELLM_BASE_URL and requires that
exact host and port through the proxy, and another port of it to be
denied. Also lists the api.anthropic.com DENY lines that Claude Code
makes on the litellm route.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PYSEC-2026-4175/4177 (urllib3 2.7.0) and PYSEC-2026-4011/4013
(virtualenv 21.2.0) fail the Quality Gate audit on every branch. Pins
the oldest fixed release of each, to stay clear of the runner's minimum
package age; python-discovery follows virtualenv to 1.6.1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CodeQL cannot model pytest.raises or contextlib.suppress, so it reported
the awaited task inside them as an ineffectual statement. Use an explicit
try/except (the pattern in test_reference_permissions.py) and
asyncio.gather(..., return_exceptions=True) for cleanup, give the live
fixture one explicit return, and compare the full Bedrock target list
instead of a substring-like membership check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…al credentials

Since the allowlist uses the real route resolvers, a Bedrock backend
without a token resolves no route, so the bad AWS_REGION was never
reached in CI (no token there) while it passed on a machine whose .env
has one. The test now sets its own token.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Table-driven allowlist cases, no repeated argv assertions, merged proxy
parser cases, and drop the docker-daemon live test that
tasks/docker_egress_probe covers end to end. Same behaviours checked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
OpenCode honours the proxy when its CLI is in the image, and its
catalog and update checks are harmless DENY lines. Pi and OpenCode were
verified on Bedrock and Azure gpt-5.6-luna, with the provider hosts in
egress_allowlist.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread tests/test_docker_egress_runner.py Fixed
CarlesUIPath and others added 2 commits October 2, 2026 15:07
…ests

CodeQL py/side-effect-in-assert: a dict pop and an awaited cancel ran
inside assert statements, which python -O would skip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Drop tasks/docker_egress_llm_only.yaml: tasks/docker_egress_probe
  makes the same checks and many more.
- Drop the ALLOW_IMAGE_SKEW fallback of the sidecar to :latest; it lost
  its only test and the error already says to run make docker-image.
- Drop two overrides-engine tests that main already covers (a -D docker
  key keeps its siblings; allowlist append is in the merge tests).
- HARNESS_PARITY links to the derived-allowlist and expected-DENY tables
  instead of repeating them; the Codex ChatGPT-login note moves there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@uipreliga uipreliga 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.

Review: coder_eval — pr:211 (29 files) axis:1,2,3,4,5,6,7,8

Scope: pr:211 (29 files) axis:1,2,3,4,5,6,7,8 · branch feat/docker-llm-only-network (pr-211) · ff443ac · 2026-10-02T15:29Z · workflow variant

Change class: complex — adds a new network-isolation mode (egress proxy sidecar, per-task Docker network, host-side allowlist derivation) to the docker driver; security-sensitive control flow

The codebase is healthy at 9/10, with no critical findings and strong type safety, API surface and architecture, but the new network: llm_only egress path has real risks: some setup paths can change a row to ERROR for identical agent output, a host SIGTERM or SIGKILL leaks egress networks until Docker has no free address pools, and Test Health (7.8) leaves the sidecar CLI contract and the cancel-ordering contract untested, so fix and test these setup paths before you depend on llm_only results in A/B comparisons.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 8.6 / 10 0 0 2 4 egress._model_routes re-implements the orchestrator's route resolution (second copy that can drift)
2. Type Safety 9.4 / 10 0 0 1 1 The plugin hook egress_hosts() output is not normalized: a plugin that returns a bare host (no port) crashes the egress sidecar
3. Test Health 7.8 / 10 0 0 4 2 The host-built sidecar serve/probe argv is never parsed by egress_proxy's own CLI parser, so the CLI contract between the two sides is not unit-tested
4. Security 9.4 / 10 0 0 1 1 Grading fold-back follows a container-planted source symlink. The PR adds egress.log as a fourth instance of an existing exposure, and it gives no new capability.
5. Architecture & Design 9.4 / 10 0 0 1 1 Egress proxy ships as a stdlib-only foreign-runtime module through a second, ad-hoc mechanism (drift from SIDECAR_MODULES / sidecar_source / CE057)
6. Error Handling & Resilience 8.6 / 10 0 1 0 4 A host SIGTERM or SIGKILL leaks the per-task egress network and the stopped sidecar. No automatic reclaim exists, only a documented manual prune.
7. API Surface & Maintainability 9.8 / 10 0 0 0 2 egress_allowlist is silently ignored under network: bridge / none
8. Evaluation Harness Quality 9.2 / 10 0 0 1 3 A disabled system_one_judge criterion still adds its base_url to the llm_only allowlist and to the pre-flight probe. If that host is unreachable, the whole row becomes ERROR before the agent runs.

Overall Score: 9 / 10 · Weakest Axis: Test Health at 7.8 / 10
Totals: 🔴 0 · 🟠 1 · 🟡 10 · 🔵 18 across 8 axes.

Blockers

  1. [Axis 6] A host SIGTERM or SIGKILL leaks the per-task egress network and the stopped sidecar. No automatic reclaim exists, only a documented manual prune. (src/coder_eval/isolation/egress.py:465) — Cleanup is only in the finally of egress_scope (line 488: await _run_to_completion(_teardown(network=created_network, sidecar=created_sidecar, log_path=log_path))). Line 465 is await _checked("create its per-task network", *build_network_create_argv(network)). The sidecar is created without --rm (line 268: "create",). coder-eval installs no SIGTERM-to-cancel bridge: fs_permissions._make_signal_handler restores the previous disposition and re-kills with SIG_DFL. So a SIGTERM (CI job cancel, timeout, docker stop, systemd) skips every asyncio finally, the same as SIGKILL does. Each dispatch in flight then leaves one --internal network, and each network holds one address pool. The sidecar stops on the stale heartbeat but is not removed. The notes put a default Docker Desktop at about 29 pools. After a few cancelled --max-parallel runs, every llm_only task fails at network create with ERROR, and so does any other network creation on that Docker host (for example compose projects). The only remedy is the manual PRUNE_HINT that line 412 logs, and it appears only on the in-process teardown path that a killed process never reaches. Fix: (1) Reclaim orphans before an llm_only dispatch, or once per batch. Run docker container prune -f --filter label=org.coder-eval.egress --filter until=10m, then docker network prune -f --filter label=org.coder-eval.egress --filter until=10m. Prune removes only stopped containers and networks with no endpoints, so concurrent runs stay safe. (2) Optionally route SIGTERM to cancel the main task (loop.add_signal_handler(SIGTERM, main_task.cancel)) so the existing finally blocks run. (3) Correct .claude/notes/isolation.md § Why the sidecar watches the heartbeat: it treats this as a SIGKILL-only case.

Non-blocking, but please consider before merge

  1. [Axis 1] egress._model_routes re-implements the orchestrator's route resolution (second copy that can drift) (src/coder_eval/isolation/egress.py:116) — The function says it resolves routes "as the orchestrator does" (line 117), and it does this by copying the logic. Lines 133-143 (api_route = task.checker_context.api_route if task.checker_context else None ... backend_override=api_route.route.value if api_route and api_route.route else None, model_override=api_route.model if api_route else None, params_override=..., env_params_override=...) duplicate Orchestrator._eval_route_overrides and the resolve_evaluation_route(...) call in Orchestrator._resolve_routes (orchestrator.py:1640-1668). Line 131 (routes.append(resolve_evaluation_route(settings, agent_route))) copies the simulator route in the same way. When a new override field is added, or the precedence changes in one place, the allowlist goes out of sync with the route the orchestrator actually calls, and the egress probe fails or allows the wrong host. Move the override-to-route mapping into one shared pure helper, for example resolve_judge_route(settings, agent_route, checker_context) in models/routing.py, and call it from both sites. This is duplication across 2 sites, so the severity is Medium.
  2. [Axis 1] Egress change raises cyclomatic complexity (_build_argv D24->D27; new egress functions CC 11-17) (src/coder_eval/isolation/docker_runner.py:1386) — Correction to the routed baseline: E(34) was measured against a stale local main. At the merge base (33bc3d7), radon gives _build_argv - D (24). At PR HEAD it gives D (27), so the PR made it more complex. Also, the code decides 'is this llm_only?' in three ways: if cfg.network == "llm_only": in _network_name (line 168), egress is not None inside _build_argv (line 1439 if egress is not None and env_var in PROXY_ENV_NAMES:, line 1453 if egress is None:, line 1476 if egress is not None:), and the egress_targets is not None sentinel in run() (lines 669 and 700). _network_name must raise DockerRunError only because these three can disagree. Put all llm_only argv changes in one place. For example, _build_argv can call one _egress_argv(egress) helper that returns the network, the env-skip set and the proxy env, or EgressHandle can supply them. Then _build_argv has one branch and its complexity drops back. This module is not one of the listed hot modules, so the severity is Medium, not High.
  3. [Axis 2] The plugin hook egress_hosts() output is not normalized: a plugin that returns a bare host (no port) crashes the egress sidecar (src/coder_eval/isolation/egress.py:168) — resolve_egress_targets adds the output of the plugin-facing hook without normalizing it: targets.update(task.agent.egress_hosts(forwarded)) (egress.py:168). Only the docstring states the contract, at models/agent_config.py:211-212 (def egress_hosts(self, env: Mapping[str, str]) -> tuple[str, ...]: / "Normalized host:port targets ..."), and docs/EXTENDING.md documents the hook for third-party agents. The return type is a plain tuple[str, ...], so nothing stops a plugin from returning "api.example.com" or "API.Example.com". The user-facing egress_allowlist accepts exactly that bare-host form, because its _normalize_egress_allowlist validator normalizes it. A bare host goes straight into --allow on the sidecar argv. There, egress_proxy.main does split_host_port(entry.strip(), None), which returns None when the port is missing, then logs BAD --allow and returns 2. The sidecar container exits, and the probe then fails with an unrelated docker exec / "proxy unreachable" error. The fix is to enforce the invariant at the boundary: targets.update(normalize_egress_target(h) for h in task.agent.egress_hosts(forwarded)), and convert the ValueError to EgressSetupError as the other sources do. As an alternative, introduce an EgressTarget = NewType("EgressTarget", str) that only normalize_egress_target / url_egress_target produce, and type the hook -> tuple[EgressTarget, ...]. Then pyright rejects a raw literal in a plugin override. Add a test with a plugin config that returns a bare host.
  4. [Axis 3] The host-built sidecar serve/probe argv is never parsed by egress_proxy's own CLI parser, so the CLI contract between the two sides is not unit-tested (src/coder_eval/isolation/egress.py:314) — def build_probe_argv(sidecar: str, targets: Sequence[str]) -> list[str]: (egress.py:314) is not called by any test (grep for build_probe_argv in tests/ returns no hits). The probe branch of the proxy CLI, return asyncio.run(probe(args.proxy, args.targets, args.timeout)) (egress_proxy.py:333), is also uncovered (routed coverage). build_sidecar_create_argv is asserted only for its docker flags (test_docker_egress_runner.py:133-150). Its trailing serve --listen 0.0.0.0:3128 --heartbeat /work/heartbeat --stale 20 --allow ... tokens are never passed through egress_proxy._build_parser() / main(). The host side (egress.py) and the sidecar side (egress_proxy.py) form a CLI contract that is checked only against a live daemon. If a flag is renamed on one side (for example --stale or --proxy), every network: llm_only task fails at setup with EgressSetupError, and no unit test fails. Add a contract test: take the tokens after f"{SIDECAR_EGRESS_DIR}/{EGRESS_PROXY_MODULE}" in build_sidecar_create_argv(...) and in build_probe_argv(...), and parse them with egress_proxy._build_parser().parse_args(...). Assert the parsed allow / targets / stale / timeout. Also drive egress_proxy.main(['probe', '--proxy', f'127.0.0.1:{port}', target]) against the in-process proxy, as test_probe_succeeds_only_when_every_target_is_allowed already does for probe().
  5. [Axis 3] The llm_only cancel path (container kill before sidecar/network teardown, heartbeat alive until egress teardown) is untested (src/coder_eval/isolation/docker_runner.py:724) — The PR moved the stream / kill / parse block into async with scope as egress: (docker_runner.py:703). The kill is if proc.returncode is None: await self._kill_container(proc, container_name) (line 724), and the heartbeat is now cancelled after the scope exits (heartbeat_task.cancel(), line 732). The updated _kill_container docstring states this ordering is load-bearing: "the heartbeat is cancelled later, after the egress teardown, so a live sidecar never goes stale". Also, if the container is not killed before network rm, the network rm hits 'active endpoints' and leaks the network, which exhausts Docker's address pools for later tasks. A run of every docker test file shows lines 714-726 uncovered. The only runner-level llm_only test (test_llm_only_runs_the_container_inside_the_scope, test_docker_egress_runner.py:366) makes create_subprocess_exec raise FileNotFoundError, so the stream, cancel and kill path never runs inside the scope. Add a test that fakes a long-running proc, cancels runner.run() mid-stream with a fake egress_scope, and asserts three things: docker kill is recorded before the fake scope's finally; the heartbeat task is not cancelled when the scope exits; _parse_result_or_raise runs inside the scope on the success path.
  6. [Axis 3] The allowlist-derivation branches that silently omit a judge or LiteLLM host are untested (they turn a config gap into a failed judge criterion) (src/coder_eval/isolation/egress.py:145) — Routed coverage shows egress.py lines 96, 145-146 and 484 missed. Line 145-146: except ValueError: logger.warning("checker_context.api_route is not fully configured: no judge host is allowlisted."). Line 96: return "checker_context.api_route", None (a LiteLLM route that has params/env_params but no api_base/base_url). Line 484: logger.warning("network: llm_only with an empty allowlist: the container can reach no host at all."). Under the docker driver the LLM judges run inside the container. So both of the first two branches let setup succeed with the judge's host left out of the allowlist, and the judge criterion then fails in-container. Identical agent output scores lower, and only a log warning explains why. Add cases to CASES / test_unusable_route_warns_and_adds_nothing in tests/test_egress_targets.py. (a) A judge with a checker_context.api_route whose resolve_evaluation_route raises ValueError: assert that the warning is logged and that only the agent hosts are returned. (b) A LiteLLM judge route with params={'temperature': 0} and no base: assert the checker_context.api_route warning and no extra host. (c) An egress_scope(targets=[]) call: assert that no exec probe runs and that the empty-allowlist warning is logged.
  7. [Axis 3] Boundary-hardening flags of the task container and sidecar are not asserted: NET_RAW drop, the sidecar's host-gateway alias, the network alias (src/coder_eval/isolation/egress.py:233) — argv: list[str] = ["--cap-drop", "NET_RAW", "--dns", BLACKHOLE_DNS] (egress.py:233). test_llm_only_joins_only_the_internal_network_with_explicit_proxy_env asserts --dns and the proxy env, but grep shows no test asserting NET_RAW, NODE_USE_ENV_PROXY=1 or LITELLM_LOCAL_MODEL_COST_MAP=True. In the sidecar argv, f"{DOCKER_HOST_ALIAS}:host-gateway" (egress.py:278) and --network-alias coder-eval-egress are not asserted by test_network_and_sidecar_are_locked_down. The runner test asserts that --add-host is NOT on the task container (test_docker_egress_runner.py:120), because the sidecar is meant to carry it. Nothing checks that the sidecar does, and on Linux a loopback LiteLLM proxy is then unreachable. The NET_RAW drop is what blocks crafted frames onto the internal bridge (see the notes and the blocked_raw_socket probe). Only the live docker_egress_probe task would catch a regression, and that task does not run in CI. Extend test_llm_only_joins_only_the_internal_network_with_explicit_proxy_env to assert argv[argv.index('--cap-drop', <after DAC caps>) + 1] == 'NET_RAW' (or assert that ['--cap-drop', 'NET_RAW'] is a contiguous slice) and the two extra --env pairs. Extend test_network_and_sidecar_are_locked_down to assert --add-host host.docker.internal:host-gateway and --network-alias coder-eval-egress.
  8. [Axis 4] Grading fold-back follows a container-planted source symlink. The PR adds egress.log as a fourth instance of an existing exposure, and it gives no new capability. (src/coder_eval/orchestration/regrade.py:697) — The PR adds (EGRESS_LOG_FILENAME, GRADE_EGRESS_LOG_FILENAME), (line 697) to the rescued tuple in _fold_back_container_logs. The loop then does if not source.is_file(): continue (line 705) and shutil.copy2(source, dest) (line 720). Both calls follow a symlink at the SOURCE. The loop guards only the destination (if dest.is_symlink():).

container_run_dir is bind-mounted read-write into the grading container at the same host path. Agent-authored code runs there during a detached grade, for example a conftest.py under a run_command pytest criterion. That code can run ln -s $HOME/.aws/credentials <run_dir>/egress.log. The host then copies the credentials file into run_dir/grade.egress.log, which is uploaded to blob storage and shown in the evalboard.

For egress.log, the host never overwrites the planted link when:

  • the grade container runs under bridge or none. No egress_scope runs, so nothing writes egress.log.
  • the docker logs call in _teardown fails (if logs is not None and logs.returncode == 0).

This is a pre-existing class, because docker.log, grade.log and task.json.unhonored have the same exposure. The PR adds a fourth instance of it and does not create the class. Fix all four in one change: skip a source when source.is_symlink() is true, or read it with os.open(..., O_RDONLY | O_NOFOLLOW), and copy with shutil.copy2(source, dest, follow_symlinks=False) only after an lstat regular-file check. This pattern can be detected mechanically (shutil.copy* from a container-writable dir without a follow_symlinks=False / is_symlink guard on the source), so it is a candidate for a CE lint rule. CVSS:3.1/AV:L/AC:H/PR:L/UI:R/S:U/C:H/I:N/A:N
9. [Axis 5] Egress proxy ships as a stdlib-only foreign-runtime module through a second, ad-hoc mechanism (drift from SIDECAR_MODULES / sidecar_source / CE057) (src/coder_eval/isolation/docker_runner.py:182) — The codebase already has a pattern for a stdlib-only module that is copied into a runtime where coder_eval is not installed. It is declared in models.sandbox.SIDECAR_MODULES, read as a package resource by invocation_log.sidecar_source (whose docstring notes that this keeps zipimported installs working), and kept stdlib-only by CE057. egress_proxy.py is the same kind of module but uses a separate path for each part. It has its own constant EGRESS_PROXY_MODULE = "egress_proxy.py" (egress.py:178). It is copied from the filesystem by shutil.copy2(Path(__file__).resolve().parents[1] / EGRESS_PROXY_MODULE, egress_dir / EGRESS_PROXY_MODULE) (docker_runner.py:182), which depends on the package layout and is not zipimport-safe. Its import surface is enforced by a separate hand-written test (tests/test_egress_proxy.py::test_module_imports_only_the_five_stdlib_modules), not by CE057. The result is two mechanisms for one concept, so a later fix to one does not reach the other. Fix: generalize the manifest, for example a SHIPPED_STDLIB_MODULES set with per-module allowed imports. Read the proxy source through the same resource helper and write it into egress_dir. Extend CE057 to derive its targets from that manifest, then delete the bespoke import test.
10. [Axis 8] A disabled system_one_judge criterion still adds its base_url to the llm_only allowlist and to the pre-flight probe. If that host is unreachable, the whole row becomes ERROR before the agent runs. (src/coder_eval/isolation/egress.py:169-173) — resolve_egress_targets checks enabled for the LLM judges at line 132: if any(isinstance(c, LLMJudgeCriterion | AgentJudgeCriterion) and c.enabled for c in task.success_criteria):. The system_one_judge loop at lines 169-173 does not check it: for criterion in task.success_criteria: if isinstance(criterion, SystemOneJudgeCriterion): target = url_egress_target("system_one_judge base_url", criterion.base_url). SystemOneJudgeCriterion.enabled (models/criteria.py:1431) is documented as 'When False no API call is made ... Useful for A/B comparisons across experiment variants'. So a variant that disables the judge still allowlists api.typesafe.ai:443, and _probe (egress.py:481-482) still CONNECTs to it. On a host that cannot reach TypeSafe (often the reason the judge was disabled), _probe raises EgressSetupError and the row gets a synthetic ERROR task.json. The final_status flips from a graded result to ERROR because of a criterion that never runs. It also widens the boundary with a host nothing uses. tests/test_egress_targets.py:116 already states the contract for LLM judges ('disabled-judge' adds nothing), but it has no system_one_judge case. Fix: add and criterion.enabled to the isinstance check, and add a parametrized 'disabled-system-one-judge' case next to line 116.

Nits

  1. [Axis 1] Proxy env-var names and the prune hint are written twice in egress.py (src/coder_eval/isolation/egress.py:187) — PROXY_ENV_NAMES (lines 187-199) lists HTTPS_PROXY, HTTP_PROXY, https_proxy, http_proxy, NO_PROXY, no_proxy, NODE_USE_ENV_PROXY. task_container_egress_argv (lines 234-238) writes the same names again as literals: for name in ("HTTPS_PROXY", "HTTP_PROXY", "https_proxy", "http_proxy"): / for name in ("NO_PROXY", "no_proxy"): / "NODE_USE_ENV_PROXY=1". The two lists already differ: ALL_PROXY is skipped but never set. Build both from one mapping of name to value, so the skip set is mapping.keys() | {ALL_PROXY...}. In the same way, the pool-exhausted message (lines 363-365, docker network prune --filter label={EGRESS_LABEL}) repeats part of PRUNE_HINT (lines 204-207). Use PRUNE_HINT there.
  2. [Axis 1] Provider host literals are repeated across agent configs and egress.py (src/coder_eval/models/agent_config.py:402) — The same host:port literals appear in more than one place. "api.openai.com:443" is at agent_config.py:319 (Codex) and :404. "generativelanguage.googleapis.com:443" is at :350 (Antigravity) and :405. "api.anthropic.com:443" is at :403 and isolation/egress.py:49 (_DIRECT_TARGETS). "openrouter.ai:443" is at :402 and again as the fallback at :441 (return (_PI_PROVIDER_HOSTS.get(provider if model_id else "", "openrouter.ai:443"),)). Name these once, as module constants or one provider-host map, and have Codex, Antigravity, Pi and _DIRECT_TARGETS reference them. In Pi, use _PI_PROVIDER_HOSTS["openrouter"] as the fallback.
  3. [Axis 1] The pure-Pydantic models layer gets a module logger and URL-egress helpers that log (src/coder_eval/models/sandbox.py:136) — logger = logging.getLogger(__name__) (line 136) is the only module logger in models/. It sits mid-file, between LOOPBACK_HOSTS (line 134) and def url_egress_target (line 139). url_egress_target has a logging side effect: logger.warning("%s points at a loopback host, which the egress sidecar cannot reach.", source) (line 154). CLAUDE.md describes models/ as the pure-Pydantic layer. url_egress_target and LOOPBACK_HOSTS are in models only because the agent-config egress_hosts overrides need them. Keep the pure normalizer (normalize_egress_target) in models. Return the loopback case as data, and log it in isolation/egress.py. In the other direction, rewrite_loopback_for_container, DOCKER_HOST_ALIAS and forwarded_env_names were moved into the llm_only-specific isolation/egress.py, but the bridge path also uses them. That module's docstring ('Host side of network: llm_only') no longer describes all of its contents.
  4. [Axis 1] egress_probe.py has two near-identical socket helpers and reads /proc/net/route twice (tasks/docker_egress_probe/egress_probe.py:33) — _connect_status (lines 33-36) and _raw_status (lines 39-42) have the same body. The only difference is that _connect_status builds f"CONNECT {target} HTTP/1.1\r\nHost: {target}\r\n\r\n" first. Make it return _raw_status(f"CONNECT ...".encode()). blocked_default_route (lines 115-118) and blocked_gateway_ip (lines 121-131) both open and parse /proc/net/route. A small _routes() helper removes that repetition.
  5. [Axis 2] _model_routes uses except AssertionError to detect an unconfigured backend, but those are pyright-narrowing asserts that python -O removes (src/coder_eval/isolation/egress.py:120) — Lines 118-120 have try: agent_route = resolve_route(settings) / except (AssertionError, ValueError):, with a warning that names "AWS_REGION / AWS_BEARER_TOKEN_BEDROCK". In the Bedrock branch, resolve_route checks these with assert settings.aws_region is not None, "Bedrock requires aws_region". Its docstring says these asserts are for type narrowing, an internal contract that validate_api_keys() already enforced. They are not runtime validation. Under python -O the asserts are removed. Then BedrockRoute(region=None) is built (the dataclass does no runtime check), and _bedrock_targets(route.region) formats bedrock-runtime.None.amazonaws.com, which normalizes to a bogus allowlist entry instead of the intended warning. Do not use narrowing asserts as control flow. Check settings.api_backend / the required settings explicitly before you call resolve_route (or add a raising validation helper, like _validate_litellm_settings for LiteLLM), and catch only ValueError.
  6. [Axis 3] The new egress.log to grade.egress.log fold-back during detached grading is untested (src/coder_eval/orchestration/regrade.py:697) — (EGRESS_LOG_FILENAME, GRADE_EGRESS_LOG_FILENAME), (regrade.py:697) was added to the rescued tuple in _fold_back_container_logs. The rename exists so that on the resume path the executed container's egress.log is not overwritten. The only fold-back test (test_a_refused_grading_record_is_folded_back_beside_the_row, tests/test_regrade.py:665) covers task.json.unhonored only. Add one assertion to that test: write scratch/egress.log, put a pre-existing row/egress.log beside it, and assert that row/grade.egress.log holds the scratch content and row/egress.log is unchanged.
  7. [Axis 3] The OSError to EgressSetupError wrap paths (staging, docker CLI missing, log write) are untested (src/coder_eval/isolation/docker_runner.py:673) — raise EgressSetupError(f"network: llm_only could not stage the egress proxy: {exc}") from exc (docker_runner.py:673) is uncovered across every docker test file. So are egress.py:357 (raise EgressSetupError(f"network: llm_only could not {what}: {exc}") from exc), egress.py:375 (the probe OSError wrap) and egress.py:401-402 (the egress-log write OSError warning). These branches decide whether a docker-CLI or filesystem failure becomes a synthetic ERROR task.json row or an unclassified crash. Add a case to the test_a_failed_setup_tears_down_what_it_created parametrization with answer=OSError('docker vanished') on image inspect, and assert EgressSetupError plus the verbs. Add a _rt_runner case that monkeypatches dr._prepare_egress_dir to raise OSError, and assert the synthetic ERROR row as in test_setup_failure_writes_a_synthetic_error_row.
  8. [Axis 4] The egress proxy can log the full refused target, including credentials, which breaks its own redaction rule (src/coder_eval/egress_proxy.py:199) — The redaction rule is stated in two places:
  • _redacted ("the target may carry credentials").
  • .claude/notes/isolation.md ("BAD lines carry only the method and length, because a refused target can hold credentials in its userinfo or query").

Two paths break that rule:

  1. Line 199 shown = f"{https[0]}:{https[1]}" if https else repr(target) followed by _log(f"BAD {shown} absolute-form https (use CONNECT)"). When the authority does not parse (for example https://user:tok@host:abc/?key=secret), the whole target, with userinfo and query, goes to egress.log.
  2. Line 212 _log(f"DENY {destination} {method}"). For CONNECT, split_host_port(target, None) does not strip userinfo, so CONNECT user:pw@evil.com:443 logs DENY user:pw@evil.com:443 CONNECT. The absolute-http path strips it with rpartition("@").

Impact is small. The client is the agent itself and the log stays in the run dir. Still, the log is persisted and uploaded. Use _redacted(request_line) in the else-branch on line 199. Strip or refuse userinfo in the CONNECT authority before you build destination, for example refuse any @ in a CONNECT target as BAD. Add a test for each case. CVSS:3.1/AV:L/AC:H/PR:H/UI:N/S:U/C:L/I:N/A:N
9. [Axis 5] New isolation module boundaries are not cohesive: egress.py owns network-mode-agnostic docker helpers, and errors.py splits the DockerRunError hierarchy (src/coder_eval/isolation/egress.py:48-69) — isolation/egress.py is documented as the "Host side of network: llm_only". It now also owns helpers that every network mode uses: DOCKER_HOST_ALIAS, rewrite_loopback_for_container and forwarded_env_names (lines 48-69). docker_runner calls them on the bridge path too (merged_allowlist = forwarded_env_names(self.rt.task) at docker_runner.py:1432, and rewritten = rewrite_loopback_for_container(litellm_base_url) at docker_runner.py:1449). These helpers moved there only so egress.py can import them without a cycle. In the same way, DockerRunError moved to isolation/errors.py, but its subclass DockerBuildError stays in docker_runner.py:366, and consumers such as regrade.py:830 still import DockerRunError through the docker_runner re-export. Fix: put the shared env and host-alias helpers in a small neutral module, for example isolation/env.py, that both files import. Move DockerBuildError into isolation/errors.py so the whole exception hierarchy is in one place.
10. [Axis 6] egress_probe gives a 'blocked' PASS for a path it never tested: a missing tool returns None, and a fixed git clone target that already exists makes the clone fail. (tasks/docker_egress_probe/egress_probe.py:70) — _command_fails starts with if shutil.which(argv[0]) is None:\n return None, so blocked_curl, blocked_curl_noproxy, blocked_pip_download, blocked_git_clone and blocked_npm_view report PASS on an image that lacks the tool. This contradicts the docs' claim that "Each probe passes only when its path is blocked". Also, blocked_git_clone clones into the fixed path /tmp/git-probe (line 194). If that path already exists (a repeated all run, or the agent created it), git exits non-zero, and that also reads as 'blocked'. Report a missing tool as SKIP or FAIL, not PASS. Clone into a fresh tempfile.mkdtemp() path.
11. [Axis 6] _teardown drops the result of docker rm -f <sidecar>, so the reason a sidecar could not be removed is never logged. (src/coder_eval/isolation/egress.py:403) — await _best_effort("rm", "-f", sidecar) logs only when the CLI raises OSError or SubprocessError. A non-zero exit, such as a daemon error, is silent. If the sidecar had already stopped (stale heartbeat), docker network rm still succeeds, so the stopped container leaks and no warning is logged at all. Check returncode as the network loop does, and log the stderr together with PRUNE_HINT.
12. [Axis 6] The setup probe waits 5 s for the CONNECT reply, but the proxy allows 10 s to dial upstream. A slow but reachable model host fails the task setup with an opaque TimeoutError(). (src/coder_eval/egress_proxy.py:291) — status_line = await asyncio.wait_for(reader.readline(), timeout) runs with timeout = _PROBE_TIMEOUT_SECONDS = 5 (egress.py:211). The sidecar's own dial uses DIAL_TIMEOUT_SECONDS = 10.0 (egress_proxy.py:29). When DNS plus TCP connect takes 5-10 s (slow resolver, IPv6 fallback), the probe reports FAIL host:443 TimeoutError(). _probe then raises EgressSetupError and the row becomes ERROR, although real traffic through the proxy would work. Make the probe's reply wait at least DIAL_TIMEOUT_SECONDS (keep it under the 30 s _docker cap). Also give a timeout a readable reason, for example 'no reply from the proxy within N s'.
13. [Axis 6] PiAgentConfig.egress_hosts silently falls back to openrouter.ai for an unknown provider prefix. The setup probe passes, and the agent then fails at run time with a DENY. (src/coder_eval/models/agent_config.py:441) — return (_PI_PROVIDER_HOSTS.get(provider if model_id else "", "openrouter.ai:443"),) takes a model such as deepseek/... and allowlists openrouter.ai without a warning. The probe CONNECTs to openrouter and succeeds, so setup passes. Pi's real model host is denied, and that shows up only as an agent crash after the container has started. Log a warning that names the unknown provider and sandbox.docker.egress_allowlist, as _route_targets does for an unknown LiteLLM endpoint.
14. [Axis 7] egress_allowlist is silently ignored under network: bridge / none (src/coder_eval/models/sandbox.py:267) — The field description says "Ignored for other network modes." No validator or warning applies when egress_allowlist is non-empty and network != "llm_only". This is the missing inverse of the new guard. Under none, a user who adds pypi.org and expects it to be reachable gets no signal that the setting did nothing. Log a warning in DockerRunner._resolve_egress_targets (or a model validator) when the list is non-empty and the mode is not llm_only.
15. [Axis 7] Misconfigured llm_only with a non-docker driver is rejected one row at a time during dispatch, not up front (src/coder_eval/orchestration/batch.py:196) — if sandbox_cfg is not None and driver != "docker" and sandbox_cfg.docker.network == "llm_only": raise ValueError(...) is inside run_single. A suite with that config error starts the batch and produces one ERROR row per task. The same function already does a pre-flight pass (check_pricing_coverage(resolved_tasks), line 170). Move this check into a pre-flight loop over resolved_tasks beside it, so the run fails once and before any work starts.
16. [Axis 8] Egress DENY events are not in task.json, so a row that failed because the boundary blocked a needed host looks the same as an agent failure in run records (src/coder_eval/isolation/egress.py:400) — The only record of the boundary's decisions is the sidecar log, written at teardown: await asyncio.to_thread(write_text_atomic, log_path, logs.stdout + logs.stderr). Nothing goes into EvaluationResult.environment_info (for example a count of DENY lines, or the effective allowlist from the READY line). A common use is to A/B a task under bridge and llm_only. If a row fails because, for example, pypi.org was denied, run.json and task.json (the cross-repo contract that the coder-eval-uipath dashboard reads) show an ordinary FAILURE. To find the cause, an operator must grep egress.log by hand. Consider recording egress: {allow: [...], deny_count: N, denied: [...]} in environment_info on the host after teardown, so that cross-run comparison can attribute a score drop to the boundary.
17. [Axis 8] A detached evaluate that fails in egress setup shows a misleading error: it names grade.docker.log and suggests --allow-host-grading, which drops the network boundary (src/coder_eval/orchestration/regrade.py:903-907) — EgressSetupError subclasses DockerRunError, so an llm_only grading setup failure (failed probe, address pools exhausted, missing framework image) reaches this handler: f"Grading {task.task_id!r} in a container failed: {e}. The container's own output was " + f"kept at {run_dir / GRADE_DOCKER_LOG_FILENAME}. ... Otherwise, re-run with " + "--allow-host-grading to grade on this machine instead ...". For an egress failure the container never started. There is no useful grade.docker.log, and the evidence is in grade.egress.log, which this PR adds. The suggested fallback grades on the host with the full host network, so a network-dependent criterion of an llm_only task can score differently with no mention of network. Add a branch for isinstance(e, EgressSetupError) that names GRADE_EGRESS_LOG_FILENAME and does not suggest host grading (or warns that host grading drops network: llm_only).
18. [Axis 8] The design notes still name docker.log as the egress evidence file, but this PR moved the ALLOW/DENY lines to egress.log (.claude/notes/isolation.md:1133) — Line 1133 reads: 'docker.log is the only evidence the boundary leaves, and operators grep it for ^(ALLOW|DENY|FAIL).' Earlier in the same section (line ~1100), the notes say the log 'is now egress.log'. docs/DOCKER_ISOLATION.md also tells operators to grep ... --include=egress.log. A maintainer who follows the notes will grep the wrong file. Change docker.log to egress.log (and grade.egress.log for grading) at line 1133.

What's Missing

Parallel paths:

  • 🟡 isolation/egress.py _model_routes copies the orchestrator's judge-route mapping and simulator-route resolution (Orchestrator._eval_route_overrides / _resolve_routes in src/coder_eval/orchestrator.py). The orchestrator stays unchanged and no shared helper exists, so a later precedence or override change made in one place lets the allowlist drift from the routes that are actually called. (trigger: src/coder_eval/isolation/egress.py) _(restates: Axis 1: egress.model_routes re-implements the orchestrator's route resolution)
  • 🟡 resolve_egress_targets gates the LLM/agent judges on c.enabled (line 132), but the new system_one_judge loop (lines 169-173) has no such gate. A disabled judge still adds its host to the allowlist and the pre-flight probe, and the row can become ERROR before the agent runs. (trigger: src/coder_eval/isolation/egress.py) (restates: Axis 8: A disabled system_one_judge criterion still adds its base_url to the llm_only allowlist)
  • 🟡 egress_proxy.py is a stdlib-only module that is copied into a foreign runtime, but it ships through a second, ad-hoc path: a __file__-relative shutil.copy2 plus a hand-written import test. The existing SIDECAR_MODULES / invocation_log.sidecar_source / CE057 mechanism is not extended to cover it, so a later fix to one path does not reach the other. (trigger: src/coder_eval/isolation/docker_runner.py) (restates: Axis 5: Egress proxy ships as a stdlib-only foreign-runtime module through a second, ad-hoc mechanism)
  • 🔵 regrade.py gets the egress.log to grade.egress.log fold-back, but the parallel except (DockerRunError, OSError) RegradeError message (lines 899-907) is not updated. An EgressSetupError during detached grading still names grade.docker.log and suggests --allow-host-grading, which drops the llm_only boundary. (trigger: src/coder_eval/orchestration/regrade.py) (restates: Axis 8: A detached evaluate that fails in egress setup shows a misleading error)
  • 🔵 batch.py adds a guard for llm_only with a non-docker driver, but the inverse case has none: a non-empty egress_allowlist under network: bridge / none is silently ignored. The new guard also runs per row inside run_single, not in the existing pre-flight pass beside check_pricing_coverage. (trigger: src/coder_eval/orchestration/batch.py) (restates: Axis 7: egress_allowlist is silently ignored under network: bridge / none)

Display & mapping dicts:

  • 🔵 batch._create_error_task_result maps only DockerBuildError to FinalStatus.BUILD_FAILED / "Docker image build failed". The new EgressSetupError and the new llm_only-needs-docker ValueError are also environment or config setup failures, but they fall through to the generic ERROR description "Failed to load task from : EgressSetupError". In run.json, a failed egress probe or exhausted address pools then reads as a broken task YAML. No test asserts the batch-level row for an EgressSetupError. (trigger: src/coder_eval/orchestration/batch.py)

Tests:

  • 🟡 The new llm_only control flow in DockerRunner.run has no test: kill before the sidecar and network teardown, heartbeat kept alive until the egress teardown, and parse inside the scope. The CLI contract between the host-built serve / probe argv and egress_proxy._build_parser() also has no test. Both fail only against a live daemon. (trigger: src/coder_eval/isolation/docker_runner.py) (restates: Axis 3: The llm_only cancel path (container kill before sidecar/network teardown, heartbeat alive until egress teardown) is untested)
  • 🟡 The new third-party AgentConfig.egress_hosts() hook (documented in docs/EXTENDING.md) has no test with a plugin config that returns a bare host or an un-normalized host. test_every_registered_agent_config_answers_with_normalized_targets covers only the in-tree configs, and the hook output is not normalized at the boundary. (trigger: src/coder_eval/models/agent_config.py) (restates: Axis 2: The plugin hook egress_hosts() output is not normalized)
  • 🔵 The new (EGRESS_LOG_FILENAME, GRADE_EGRESS_LOG_FILENAME) fold-back entry has no test asserting that grade.egress.log gets the scratch content and that the row's own egress.log stays unchanged. (trigger: src/coder_eval/orchestration/regrade.py) (restates: Axis 3: The new egress.log to grade.egress.log fold-back during detached grading is untested)

Downstream consumers:

  • 🔵 The PR adds two new per-row run-dir files, egress.log and grade.egress.log (path_utils.py). The run-layout spec .claude/shared/run-layout.md does not list them (it lists grade.log). The design notes (.claude/notes/isolation.md line 1133) still name docker.log as the egress evidence file. (trigger: src/coder_eval/path_utils.py) (restates: Axis 8: The design notes still name docker.log as the egress evidence file)
  • 🔵 The network mode and any DENY summary do not reach run.json or task.json (no environment_info entry, and reports/run_record.py do not mention the network mode). The /coder-eval:analyze skill is also not told to read egress.log. So downstream failure clustering, the evalboard and the coder-eval-uipath dashboard cannot attribute a bridge-vs-llm_only score drop to the boundary. (trigger: src/coder_eval/isolation/egress.py) (restates: Axis 8: Egress DENY events are not in task.json)

Daily/nightly:

  • 🟡 The PR does not say what happens on a shared nightly or CI Docker host when a job is cancelled. A SIGTERM or SIGKILL of the host process leaks one --internal network and one stopped sidecar for each in-flight llm_only task, and nothing reclaims them automatically. Repeated cancelled --max-parallel runs can exhaust the address pools, and then every network creation on that host fails, including non-coder-eval jobs. (trigger: src/coder_eval/isolation/egress.py) (restates: Axis 6: A host SIGTERM or SIGKILL leaks the per-task egress network and the stopped sidecar)
  • 🔵 The PR states that the bridge/none argv is pinned byte for byte. It does not state that DockerRunner.run changed its control flow for every network mode: stream, kill and parse are now nested in async with scope (a nullcontext for bridge), and the heartbeat cancel moved to the outer finally. The nightly bridge path probably behaves the same, but the kill path has zero coverage, so that claim is not verified. (trigger: src/coder_eval/isolation/docker_runner.py) (restates: Axis 3: The llm_only cancel path (container kill before sidecar/network teardown, heartbeat alive until egress teardown) is untested)
  • 🔵 uv.lock carries unrelated transitive bumps in this egress PR (urllib3 2.7.0 to 2.8.0, virtualenv 21.2.0 to 21.7.13, python-discovery 1.2.0 to 1.6.1, for pip-audit). They change the framework image and the environment that the nightly and production runs install, but the PR scope does not mention them as a separate change to the production pipeline. (trigger: uv.lock)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] CE068 single judge-route seam: a call to resolve_evaluation_route( and a read of checker_context.api_route are allowed in one module only (a new pure helper, for example models/routing.py::resolve_judge_route(settings, agent_route, checker_context)). Every other call site is a violation. Today the only legal caller is orchestrator.py:1729-1730, so the rule fires on the PR copy in isolation/egress.py. AST check: Call whose func name is resolve_evaluation_route, and Attribute chain .checker_context.api_route, outside the allowlisted file. Add to tests/lint/rules/ce068_single_judge_route_seam.py, wire into ALL_RULES in tests/lint/runner.py, add CE068 to external in pyproject.toml. Prevents: egress._model_routes copies the orchestrator's route resolution (src/coder_eval/isolation/egress.py:116-147, A1/A5/A7 medium).
  • [ce-lint] CE069 complexity ratchet: enable ruff C901 with max-complexity = 20 for new functions (the 47 current D-or-worse functions carry a # noqa: C901 debt marker, the same policy as PLR0912), and add a ratchet: tests/lint/complexity_baseline.json records the radon CC of each noqa'd function, and the rule fails when a function's CC goes above its baseline. A noqa marker alone does not catch growth of an existing offender, so the ratchet is the part that matters. radon is already a runtime dependency. Prevents: _build_argv grows from D(24) to D(27) and new egress functions land at CC 11-17 (src/coder_eval/isolation/docker_runner.py:1386, src/coder_eval/egress_proxy.py:169).
  • [ce-lint] CE070 network-mode decision seam: forbid comparison of .network to a string literal (== "llm_only", != "llm_only", in (...)) outside models/sandbox.py. The model exposes one property (for example DockerConfig.egress_enabled), and docker_runner and egress use it. AST check: Compare whose left is an Attribute named network and whose comparator is a Constant str. Prevents: The llm_only mode is spelled four ways (docker_runner.py:168 _network_name, :750 _resolve_egress_targets, plus the egress is None and egress_targets is None sentinels), which forces the fail-closed guard in _network_name (docker_runner.py:1386 finding).
  • [ce-lint] CE071 no logging in the models layer: forbid import logging, logging.getLogger, and any logger.<level>( call in src/coder_eval/models/**. The models layer returns data; the caller logs. Zero hits on main today, so the cap is 0 and holds. Prevents: A module logger and a logging side effect in url_egress_target in the pure-Pydantic layer (src/coder_eval/models/sandbox.py:136, :154).
  • [ce-lint] CE072 no catch of AssertionError: forbid except AssertionError and except (..., AssertionError, ...) in src/coder_eval. An assert is pyright narrowing and python -O removes it, so it is not control flow. Ruff has no rule for this. The fix is an explicit validation helper that raises ValueError. Prevents: _model_routes uses except (AssertionError, ValueError) to detect an unconfigured Bedrock backend (src/coder_eval/isolation/egress.py:120).
  • [ce-lint] CE073 explicit symlink policy on file copy: every shutil.copy, shutil.copy2 and shutil.copyfile call in src/coder_eval must pass follow_symlinks=False, or be preceded in the same block by a source is_symlink() / lstat guard, or carry # noqa: CE073 with a reason. Current hits are 3 (sandbox.py:570, harbor/packager.py:273, orchestration/regrade.py:713), so each gets a decision. Rationale for a CE rule and not bandit: bandit and CodeQL do not model a container-writable source directory. Prevents: Grading fold-back follows a container-planted source symlink for docker.log, grade.log, task.json.unhonored and the new egress.log (src/coder_eval/orchestration/regrade.py:697/705/720, A3/A4 medium).
  • [ce-lint] CE074 endpoint literal SSOT: a string literal that matches ^[a-z0-9.-]+\.[a-z]{2,}:\d{2,5}$ (a host:port) or a proxy env name (HTTPS_PROXY, HTTP_PROXY, NO_PROXY, ALL_PROXY and lowercase forms, NODE_USE_ENV_PROXY) may appear only in one owning module (for example a PROVIDER_HOSTS map and one proxy-env mapping). Same shape as CE053 for run-record filenames. Prevents: Provider host literals repeated across agent configs and egress.py (src/coder_eval/models/agent_config.py:319/350/402-405/441, isolation/egress.py:49), and the proxy env names written twice with ALL_PROXY drift (isolation/egress.py:187-199 vs :234-238).
  • [ce-lint] CE075 disabled criteria stay inert: in a comprehension or for-loop whose iterable is <x>.success_criteria and whose filter is isinstance(c, <...>Criterion), the same filter must also read c.enabled. Better: delete the pattern with a TaskDefinition.enabled_criteria(*types) helper and have the rule forbid the hand-written isinstance filter outside it. orchestrator.py:1759 and egress.py:132 already write and c.enabled by hand; egress.py:169-173 forgot it. Prevents: A disabled system_one_judge still adds its base_url to the llm_only allowlist and the pre-flight probe, so the row becomes ERROR (src/coder_eval/isolation/egress.py:169-173, A7/A8 medium).
  • [pyright] Make the egress target a nominal type: EgressTarget = NewType("EgressTarget", str), produced only by normalize_egress_target / url_egress_target. Type AgentConfig.egress_hosts(...) -> tuple[EgressTarget, ...], resolve_egress_targets(...) -> set[EgressTarget], and build_sidecar_create_argv(targets: Sequence[EgressTarget]). Pyright then rejects a raw "api.example.com" literal in an in-tree override or a typed plugin. A runtime normalize at the hook boundary is still needed for untyped third-party plugins. Prevents: The plugin hook egress_hosts() output is not normalized, and a bare host crashes the sidecar with BAD --allow (src/coder_eval/isolation/egress.py:168, A2 medium).
  • [pyright] Turn on reportUnusedCallResult per file with a # pyright: reportUnusedCallResult=true header in src/coder_eval/isolation/egress.py and docker_runner.py (not globally, it is too noisy). A discarded CompletedProcess | None from _best_effort(...) / _docker(...) then fails make typecheck; an intentional discard is written _ = await .... Prevents: _teardown drops the result of docker rm -f <sidecar>, so a failed sidecar removal is silent (src/coder_eval/isolation/egress.py:403).
  • [ce-lint] Extend CE057 to a shipped-module manifest: replace the single SIDECAR_MODULES target set with a SHIPPED_STDLIB_MODULES: dict[str, frozenset[str]] (module file to its allowed import roots), so egress_proxy.py is covered with {argparse, asyncio, os, sys, time}, and delete the bespoke test_module_imports_only_the_five_stdlib_modules. In the same rule, forbid Path(__file__) package-relative reads in src/coder_eval/isolation/** (the sources must be read through invocation_log.sidecar_source / importlib.resources, which is zipimport-safe). This is a widening of an existing rule, so no new CE id. Prevents: egress_proxy ships through a second ad-hoc mechanism: a __file__-relative copy and a hand-written import test (src/coder_eval/isolation/docker_runner.py:182, isolation/egress.py:178, A5 medium).
  • [ce-lint] CE076 isolation error hierarchy in one module: every class that subclasses DockerRunError must be defined in src/coder_eval/isolation/errors.py, and DockerRunError / its subclasses must be imported from coder_eval.isolation.errors, not through the docker_runner re-export. Prevents: errors.py splits the DockerRunError hierarchy (DockerBuildError stays at docker_runner.py:366; regrade.py:830 imports through the re-export) (src/coder_eval/isolation/egress.py:48-69 / isolation/errors.py:4, A5/A7 low).
  • [ce-lint] CE077 proxy log redaction (scoped to src/coder_eval/egress_proxy.py): an argument to _log(...) that interpolates target, request_line, destination or repr(target) must wrap the value in _redacted(...). The rule is narrow on purpose; the CONNECT userinfo case also needs the parse-time refusal of @ in an authority, which is a behaviour fix. Prevents: The egress proxy logs the full refused target with credentials (src/coder_eval/egress_proxy.py:199, :212, A4 low).

Harness improvements (not statically reachable):

  • Add a cross-side CLI contract test for the sidecar: take the tokens after <SIDECAR_EGRESS_DIR>/<EGRESS_PROXY_MODULE> from build_sidecar_create_argv(...) and build_probe_argv(...), and parse them with egress_proxy._build_parser().parse_args(...); also drive egress_proxy.main(['probe', ...]) against the in-process proxy. Generalize it: every host-built argv for a shipped module must round-trip through that module's own parser. Why not static: The contract is between an argv list built at runtime and an argparse parser built at runtime; an AST check cannot evaluate either side. Prevents: Host-built serve/probe argv never parsed by egress_proxy's parser (src/coder_eval/isolation/egress.py:314, A3 medium).
  • Add full-argv golden tests for task_container_egress_argv() and build_sidecar_create_argv() (assert the whole list, not selected flags), so a dropped --cap-drop NET_RAW, --add-host host.docker.internal:host-gateway, --network-alias coder-eval-egress, NODE_USE_ENV_PROXY=1 or LITELLM_LOCAL_MODEL_COST_MAP=True fails a unit test. Why not static: The argv is assembled from conditionals at runtime; only the evaluated list shows which hardening flags are present. Prevents: Boundary-hardening flags not asserted (src/coder_eval/isolation/egress.py:233, :278, A3 medium).
  • Run the live tasks/docker_egress_probe task in a docker-capable CI job (nightly or label-triggered), and make the probe tri-state: a missing tool or a pre-existing clone path reports SKIP, not PASS, and git clones into tempfile.mkdtemp(). Why not static: Whether a raw socket, DNS, or direct route is blocked is a property of the live Docker network, not of the source. Prevents: Boundary flag regressions only a live run catches (egress.py:233/278), and false 'blocked' PASS results (tasks/docker_egress_probe/egress_probe.py:70, A6 low).
  • Add a diff-coverage gate for PRs (for example diff-cover coverage.xml --compare-branch=origin/main --fail-under=90, at least for src/coder_eval/isolation/** and orchestration/regrade.py) to make verify / CI, beside the global 80% threshold. Why not static: Coverage is measured by running the suite; the global 80% floor hides uncovered new branches in a large codebase. Prevents: Untested new branches: egress.py:96/145-146/484 judge-host omission and empty allowlist, docker_runner.py:673 and egress.py:357/375/401 OSError wraps, regrade.py:697 egress.log fold-back, docker_runner.py:714-726 cancel path (A3 medium/low).
  • Add a cancel-ordering test for DockerRunner.run() under llm_only with a fake long-running proc and a fake egress_scope: cancel mid-stream and assert docker kill is recorded before the scope's finally, the heartbeat is not cancelled until after scope exit, and _parse_result_or_raise runs inside the scope. Why not static: The contract is an ordering of async events across cancellation; it exists only in an execution trace. Prevents: The llm_only cancel path is untested (src/coder_eval/isolation/docker_runner.py:724, A3 medium).
  • Reclaim orphaned egress resources automatically: before the first llm_only dispatch in a batch, run label-filtered docker container prune -f --filter label=org.coder-eval.egress --filter until=10m then docker network prune with the same filters; optionally bridge SIGTERM to main-task cancel. Add a docker integration test that SIGTERMs a run mid-task and asserts no labelled network remains after the next batch starts. Why not static: The leak needs a process kill and a live Docker daemon; source analysis cannot see signal disposition at runtime. Prevents: Host SIGTERM/SIGKILL leaks the per-task network and stopped sidecar until address pools run out (src/coder_eval/isolation/egress.py:465, A6 high).
  • Add a constant-invariant unit test that _PROBE_TIMEOUT_SECONDS >= egress_proxy.DIAL_TIMEOUT_SECONDS and both stay under the _docker 30 s cap, and give a probe timeout a readable reason string. Why not static: The two constants live in different modules with a cross-module ordering relation; a one-line test states it more clearly than a bespoke AST rule. Prevents: Probe waits 5 s but the proxy dials for 10 s, so a slow host fails setup (src/coder_eval/egress_proxy.py:291, A6 low).
  • Add pre-flight config validation in run_batch beside check_pricing_coverage: fail once, before dispatch, for llm_only with a non-docker driver, and warn once for a non-empty egress_allowlist under bridge/none and for a Pi model whose provider prefix falls back to openrouter.ai. Why not static: The conflict exists only after the 5-layer config merge resolves the driver, network mode and model per task. Prevents: llm_only with a non-docker driver rejected per row (src/coder_eval/orchestration/batch.py:196), silent egress_allowlist under bridge/none (models/sandbox.py:267), silent Pi openrouter fallback (models/agent_config.py:441) (A6/A7 low).
  • Record the boundary's decisions in the run record: after teardown, parse the sidecar log on the host and write environment_info.egress = {allow: [...], deny_count: N, denied: [...]}, and branch the detached-grading error on EgressSetupError so it names grade.egress.log and does not suggest --allow-host-grading. Add a round-trip test that a DENY line in a fake log appears in task.json. Why not static: Attribution of a score drop to the boundary depends on runtime log content; the error-message branch is semantic text. Prevents: DENY events missing from task.json (src/coder_eval/isolation/egress.py:400, A8 low) and the misleading regrade egress error (src/coder_eval/orchestration/regrade.py:903-907, A6/A8 low).
  • In the review/plan checklist, when a run-record filename moves (here docker.log to egress.log), grep .claude/notes/ and docs/ for the old name in the same change. Why not static: docker.log still exists as a valid file, so a filename-literal check cannot tell that the sentence about egress evidence now names the wrong file; it needs semantic reading. Prevents: Design notes still name docker.log as the egress evidence (.claude/notes/isolation.md:1133, A8 low).

Top 5 Priority Actions

  1. Add and criterion.enabled to the system_one_judge loop in src/coder_eval/isolation/egress.py:169-173, and add a 'disabled-system-one-judge' case next to tests/test_egress_targets.py:116, because a disabled judge is still allowlisted and probed, and an unreachable judge host changes a graded row to a synthetic ERROR.
  2. Stop setup probes from causing false ERROR rows: normalize the plugin egress_hosts() output with normalize_egress_target and map ValueError to EgressSetupError (src/coder_eval/isolation/egress.py:168), and make the probe reply wait at least DIAL_TIMEOUT_SECONDS (src/coder_eval/egress_proxy.py:291) so that a slow but reachable model host does not fail setup.
  3. Reclaim orphaned egress networks and sidecars automatically with a label-filtered docker container prune and docker network prune (--filter label=org.coder-eval.egress --filter until=10m) before llm_only dispatch, and optionally route SIGTERM to task cancel, because cleanup runs only in the egress_scope finally (src/coder_eval/isolation/egress.py:465-488) and each killed run leaks networks until every later llm_only task fails with ERROR.
  4. Move judge-route resolution into one shared helper (for example resolve_judge_route in models/routing.py) that both _model_routes (src/coder_eval/isolation/egress.py:116-147) and Orchestrator._eval_route_overrides and _resolve_routes call, and test the branches that silently omit a judge or LiteLLM host (egress.py:96, 145-146, 484), because an omitted judge host lowers the score for identical agent output and only a log warning explains why.
  5. Close the egress Test Health gaps: parse the build_sidecar_create_argv and build_probe_argv token tails (src/coder_eval/isolation/egress.py:314) with egress_proxy._build_parser(); test that cancel kills the container before network teardown and keeps the heartbeat alive (src/coder_eval/isolation/docker_runner.py:724); assert the NET_RAW drop and the sidecar host-gateway alias (egress.py:233, 278); and, in the same change, add a source-symlink guard to the grading fold-back (src/coder_eval/orchestration/regrade.py:697-720).

Stats: 0 🔴 · 1 🟠 · 10 🟡 · 18 🔵 across 8 axes reviewed.

@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.

Putting in a tentative stop-merge request here. Let's check 1/ how other frameworks achieve this and 2/ is this extensible?

@CarlesUIPath

Copy link
Copy Markdown
Collaborator Author

Putting in a tentative stop-merge request here. Let's check 1/ how other frameworks achieve this and 2/ is this extensible?

Agreed!

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.

4 participants