feat(docker): add network: llm_only — no internet except the model API - #211
CarlesUIPath wants to merge 18 commits into
Conversation
…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>
|
Claude finished @CarlesUIPath's task in 1m 50s —— View job Code Review in Progress
|
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>
…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
left a comment
There was a problem hiding this comment.
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
- [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 thefinallyofegress_scope(line 488:await _run_to_completion(_teardown(network=created_network, sidecar=created_sidecar, log_path=log_path))). Line 465 isawait _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_handlerrestores the previous disposition and re-kills with SIG_DFL. So a SIGTERM (CI job cancel,timeout,docker stop, systemd) skips every asynciofinally, the same as SIGKILL does. Each dispatch in flight then leaves one--internalnetwork, 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-parallelruns, everyllm_onlytask fails atnetwork createwith 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. Rundocker container prune -f --filter label=org.coder-eval.egress --filter until=10m, thendocker 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 existingfinallyblocks 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
- [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=...) duplicateOrchestrator._eval_route_overridesand theresolve_evaluation_route(...)call inOrchestrator._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 exampleresolve_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. - [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 givesD (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 Noneinside_build_argv(line 1439if egress is not None and env_var in PROXY_ENV_NAMES:, line 1453if egress is None:, line 1476if egress is not None:), and theegress_targets is not Nonesentinel inrun()(lines 669 and 700)._network_namemust raise DockerRunError only because these three can disagree. Put all llm_only argv changes in one place. For example,_build_argvcan call one_egress_argv(egress)helper that returns the network, the env-skip set and the proxy env, orEgressHandlecan supply them. Then_build_argvhas one branch and its complexity drops back. This module is not one of the listed hot modules, so the severity is Medium, not High. - [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_targetsadds 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, ...]:/ "Normalizedhost:porttargets ..."), and docs/EXTENDING.md documents the hook for third-party agents. The return type is a plaintuple[str, ...], so nothing stops a plugin from returning"api.example.com"or"API.Example.com". The user-facingegress_allowlistaccepts exactly that bare-host form, because its_normalize_egress_allowlistvalidator normalizes it. A bare host goes straight into--allowon the sidecar argv. There, egress_proxy.main doessplit_host_port(entry.strip(), None), which returns None when the port is missing, then logsBAD --allowand returns 2. The sidecar container exits, and the probe then fails with an unrelateddocker 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 anEgressTarget = NewType("EgressTarget", str)that onlynormalize_egress_target/url_egress_targetproduce, 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. - [Axis 3] The host-built sidecar
serve/probeargv 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 forbuild_probe_argvin tests/ returns no hits). Theprobebranch 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_argvis asserted only for its docker flags (test_docker_egress_runner.py:133-150). Its trailingserve --listen 0.0.0.0:3128 --heartbeat /work/heartbeat --stale 20 --allow ...tokens are never passed throughegress_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--staleor--proxy), everynetwork: llm_onlytask fails at setup with EgressSetupError, and no unit test fails. Add a contract test: take the tokens afterf"{SIDECAR_EGRESS_DIR}/{EGRESS_PROXY_MODULE}"inbuild_sidecar_create_argv(...)and inbuild_probe_argv(...), and parse them withegress_proxy._build_parser().parse_args(...). Assert the parsedallow/targets/stale/timeout. Also driveegress_proxy.main(['probe', '--proxy', f'127.0.0.1:{port}', target])against the in-process proxy, astest_probe_succeeds_only_when_every_target_is_allowedalready does forprobe(). - [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 intoasync with scope as egress:(docker_runner.py:703). The kill isif 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_containerdocstring 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 beforenetwork 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) makescreate_subprocess_execraise FileNotFoundError, so the stream, cancel and kill path never runs inside the scope. Add a test that fakes a long-runningproc, cancelsrunner.run()mid-stream with a fakeegress_scope, and asserts three things:docker killis recorded before the fake scope'sfinally; the heartbeat task is not cancelled when the scope exits;_parse_result_or_raiseruns inside the scope on the success path. - [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 hasparams/env_paramsbut noapi_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 toCASES/test_unusable_route_warns_and_adds_nothingin tests/test_egress_targets.py. (a) A judge with achecker_context.api_routewhoseresolve_evaluation_routeraises ValueError: assert that the warning is logged and that only the agent hosts are returned. (b) A LiteLLM judge route withparams={'temperature': 0}and no base: assert thechecker_context.api_routewarning and no extra host. (c) Anegress_scope(targets=[])call: assert that noexecprobe runs and that the empty-allowlist warning is logged. - [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_envasserts--dnsand the proxy env, but grep shows no test assertingNET_RAW,NODE_USE_ENV_PROXY=1orLITELLM_LOCAL_MODEL_COST_MAP=True. In the sidecar argv,f"{DOCKER_HOST_ALIAS}:host-gateway"(egress.py:278) and--network-alias coder-eval-egressare not asserted bytest_network_and_sidecar_are_locked_down. The runner test asserts that--add-hostis 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 theblocked_raw_socketprobe). Only the livedocker_egress_probetask would catch a regression, and that task does not run in CI. Extendtest_llm_only_joins_only_the_internal_network_with_explicit_proxy_envto assertargv[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--envpairs. Extendtest_network_and_sidecar_are_locked_downto assert--add-host host.docker.internal:host-gatewayand--network-alias coder-eval-egress. - [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 therescuedtuple in_fold_back_container_logs. The loop then doesif not source.is_file(): continue(line 705) andshutil.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
bridgeornone. Noegress_scoperuns, so nothing writesegress.log. - the
docker logscall in_teardownfails (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
- [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) listsHTTPS_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_PROXYis skipped but never set. Build both from one mapping of name to value, so the skip set ismapping.keys() | {ALL_PROXY...}. In the same way, the pool-exhausted message (lines 363-365,docker network prune --filter label={EGRESS_LABEL}) repeats part ofPRUNE_HINT(lines 204-207). UsePRUNE_HINTthere. - [Axis 1] Provider host literals are repeated across agent configs and egress.py (
src/coder_eval/models/agent_config.py:402) — The samehost:portliterals 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_TARGETSreference them. In Pi, use_PI_PROVIDER_HOSTS["openrouter"]as the fallback. - [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 inmodels/. It sits mid-file, betweenLOOPBACK_HOSTS(line 134) anddef url_egress_target(line 139).url_egress_targethas a logging side effect:logger.warning("%s points at a loopback host, which the egress sidecar cannot reach.", source)(line 154). CLAUDE.md describesmodels/as the pure-Pydantic layer.url_egress_targetandLOOPBACK_HOSTSare in models only because the agent-configegress_hostsoverrides 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_ALIASandforwarded_env_nameswere moved into the llm_only-specificisolation/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. - [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_statusbuildsf"CONNECT {target} HTTP/1.1\r\nHost: {target}\r\n\r\n"first. Make itreturn _raw_status(f"CONNECT ...".encode()).blocked_default_route(lines 115-118) andblocked_gateway_ip(lines 121-131) both open and parse/proc/net/route. A small_routes()helper removes that repetition. - [Axis 2]
_model_routesusesexcept AssertionErrorto detect an unconfigured backend, but those are pyright-narrowing asserts thatpython -Oremoves (src/coder_eval/isolation/egress.py:120) — Lines 118-120 havetry: agent_route = resolve_route(settings)/except (AssertionError, ValueError):, with a warning that names "AWS_REGION / AWS_BEARER_TOKEN_BEDROCK". In the Bedrock branch,resolve_routechecks these withassert settings.aws_region is not None, "Bedrock requires aws_region". Its docstring says these asserts are for type narrowing, an internal contract thatvalidate_api_keys()already enforced. They are not runtime validation. Underpython -Othe asserts are removed. ThenBedrockRoute(region=None)is built (the dataclass does no runtime check), and_bedrock_targets(route.region)formatsbedrock-runtime.None.amazonaws.com, which normalizes to a bogus allowlist entry instead of the intended warning. Do not use narrowing asserts as control flow. Checksettings.api_backend/ the required settings explicitly before you callresolve_route(or add a raising validation helper, like_validate_litellm_settingsfor LiteLLM), and catch onlyValueError. - [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 therescuedtuple in_fold_back_container_logs. The rename exists so that on the resume path the executed container'segress.logis not overwritten. The only fold-back test (test_a_refused_grading_record_is_folded_back_beside_the_row, tests/test_regrade.py:665) coverstask.json.unhonoredonly. Add one assertion to that test: writescratch/egress.log, put a pre-existingrow/egress.logbeside it, and assert thatrow/grade.egress.logholds the scratch content androw/egress.logis unchanged. - [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 ERRORtask.jsonrow or an unclassified crash. Add a case to thetest_a_failed_setup_tears_down_what_it_createdparametrization withanswer=OSError('docker vanished')onimage inspect, and assert EgressSetupError plus the verbs. Add a_rt_runnercase that monkeypatchesdr._prepare_egress_dirto raise OSError, and assert the synthetic ERROR row as intest_setup_failure_writes_a_synthetic_error_row. - [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("BADlines carry only the method and length, because a refused target can hold credentials in its userinfo or query").
Two paths break that rule:
- 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 examplehttps://user:tok@host:abc/?key=secret), the whole target, with userinfo and query, goes toegress.log. - Line 212
_log(f"DENY {destination} {method}"). ForCONNECT,split_host_port(target, None)does not strip userinfo, soCONNECT user:pw@evil.com:443logsDENY user:pw@evil.com:443 CONNECT. The absolute-http path strips it withrpartition("@").
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_routescopies the orchestrator's judge-route mapping and simulator-route resolution (Orchestrator._eval_route_overrides/_resolve_routesin 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_targetsgates the LLM/agent judges onc.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__-relativeshutil.copy2plus a hand-written import test. The existingSIDECAR_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_allowlistundernetwork: bridge/noneis silently ignored. The new guard also runs per row insiderun_single, not in the existing pre-flight pass besidecheck_pricing_coverage. (trigger: src/coder_eval/orchestration/batch.py) (restates: Axis 7:egress_allowlistis silently ignored undernetwork: bridge/none)
Display & mapping dicts:
- 🔵
batch._create_error_task_resultmaps onlyDockerBuildErrortoFinalStatus.BUILD_FAILED/ "Docker image build failed". The newEgressSetupErrorand 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.runhas 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-builtserve/probeargv andegress_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_targetscovers 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 hookegress_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.mddoes 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_infoentry, and reports/run_record.py do not mention the network mode). The/coder-eval:analyzeskill 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
--internalnetwork and one stopped sidecar for each in-flight llm_only task, and nothing reclaims them automatically. Repeated cancelled--max-parallelruns 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.runchanged its control flow for every network mode: stream, kill and parse are now nested inasync 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 ofchecker_context.api_routeare allowed in one module only (a new pure helper, for examplemodels/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 isresolve_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 toexternalin 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
C901withmax-complexity = 20for new functions (the 47 current D-or-worse functions carry a# noqa: C901debt 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
.networkto a string literal (== "llm_only",!= "llm_only",in (...)) outside models/sandbox.py. The model exposes one property (for exampleDockerConfig.egress_enabled), and docker_runner and egress use it. AST check: Compare whose left is an Attribute namednetworkand 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 theegress is Noneandegress_targets is Nonesentinels), 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 anylogger.<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 inurl_egress_targetin the pure-Pydantic layer (src/coder_eval/models/sandbox.py:136, :154). - [ce-lint] CE072 no catch of AssertionError: forbid
except AssertionErrorandexcept (..., AssertionError, ...)in src/coder_eval. An assert is pyright narrowing andpython -Oremoves 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_routesusesexcept (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.copy2andshutil.copyfilecall in src/coder_eval must passfollow_symlinks=False, or be preceded in the same block by a sourceis_symlink()/lstatguard, or carry# noqa: CE073with 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_PROXYand lowercase forms,NODE_USE_ENV_PROXY) may appear only in one owning module (for example aPROVIDER_HOSTSmap 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_criteriaand whose filter isisinstance(c, <...>Criterion), the same filter must also readc.enabled. Better: delete the pattern with aTaskDefinition.enabled_criteria(*types)helper and have the rule forbid the hand-written isinstance filter outside it. orchestrator.py:1759 and egress.py:132 already writeand c.enabledby 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 bynormalize_egress_target/url_egress_target. TypeAgentConfig.egress_hosts(...) -> tuple[EgressTarget, ...],resolve_egress_targets(...) -> set[EgressTarget], andbuild_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 hookegress_hosts()output is not normalized, and a bare host crashes the sidecar withBAD --allow(src/coder_eval/isolation/egress.py:168, A2 medium). - [pyright] Turn on
reportUnusedCallResultper file with a# pyright: reportUnusedCallResult=trueheader in src/coder_eval/isolation/egress.py and docker_runner.py (not globally, it is too noisy). A discardedCompletedProcess | Nonefrom_best_effort(...)/_docker(...)then failsmake typecheck; an intentional discard is written_ = await .... Prevents: _teardown drops the result ofdocker 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_MODULEStarget set with aSHIPPED_STDLIB_MODULES: dict[str, frozenset[str]](module file to its allowed import roots), soegress_proxy.pyis covered with {argparse, asyncio, os, sys, time}, and delete the bespoketest_module_imports_only_the_five_stdlib_modules. In the same rule, forbidPath(__file__)package-relative reads in src/coder_eval/isolation/** (the sources must be read throughinvocation_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
DockerRunErrormust be defined in src/coder_eval/isolation/errors.py, andDockerRunError/ its subclasses must be imported fromcoder_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 interpolatestarget,request_line,destinationorrepr(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>frombuild_sidecar_create_argv(...)andbuild_probe_argv(...), and parse them withegress_proxy._build_parser().parse_args(...); also driveegress_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-builtserve/probeargv 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()andbuild_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=1orLITELLM_LOCAL_MODEL_COST_MAP=Truefails 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_probetask 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 intotempfile.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) tomake 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 fakeegress_scope: cancel mid-stream and assertdocker killis recorded before the scope's finally, the heartbeat is not cancelled until after scope exit, and_parse_result_or_raiseruns 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=10mthendocker network prunewith 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_SECONDSand both stay under the_docker30 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_batchbesidecheck_pricing_coverage: fail once, before dispatch, for llm_only with a non-docker driver, and warn once for a non-emptyegress_allowlistunder 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), silentegress_allowlistunder 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 onEgressSetupErrorso it namesgrade.egress.logand 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/anddocs/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
- Add
and criterion.enabledto 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. - Stop setup probes from causing false ERROR rows: normalize the plugin
egress_hosts()output withnormalize_egress_targetand 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. - Reclaim orphaned egress networks and sidecars automatically with a label-filtered
docker container pruneanddocker 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_scopefinally(src/coder_eval/isolation/egress.py:465-488) and each killed run leaks networks until every later llm_only task fails with ERROR. - Move judge-route resolution into one shared helper (for example
resolve_judge_routein models/routing.py) that both_model_routes(src/coder_eval/isolation/egress.py:116-147) andOrchestrator._eval_route_overridesand_resolve_routescall, 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. - Close the egress Test Health gaps: parse the
build_sidecar_create_argvandbuild_probe_argvtoken tails (src/coder_eval/isolation/egress.py:314) withegress_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
left a comment
There was a problem hiding this comment.
Putting in a tentative stop-merge request here. Let's check 1/ how other frameworks achieve this and 2/ is this extensible?
Agreed! |

Summary
Adds
sandbox.docker.network: llm_only: a "no internet" mode fordriver: dockerthat still lets the agent, the judges and the simulator reach their model.network: nonealready exists, but the agent runs inside the container, sononealso 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).bridgeandnonebehave exactly as before (the argv is byte-identical, pinned by a test).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:
Or from the CLI on any docker task, or once in an experiment's
defaults::The model hosts are derived from the task, so most tasks need nothing else:
claude-codeagent,llm_judge/agent_judge, simulatorAWS_REGION,api.anthropic.com, or the LiteLLM host)codexCODEX_BASE_URLhost, elseapi.openai.comantigravitypiopenrouter/,anthropic/,openai/orgoogle/model prefixsystem_one_judgebase_urlAnything else goes in
egress_allowlist(hostorhost:port, append-merged across layers). Examples: a package index, or the provider host for OpenCode, or for Pi on another provider:To see what a task tried to reach, read
egress.login its run directory (grade.egress.logfor a grading container), and add each neededDENYhost toegress_allowlist:Requirements:
llm_onlywith another driver is refused before the agent starts.make docker-image). The proxy sidecar runs from it.Limits:
WebSearchon the direct API run outside the container; usedisallowed_toolsif needed.--max-parallelbelow that.To try it locally:
How it works
For each
llm_onlytask, coder-eval creates:A per-task
--internaldocker 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 exacthost:portallowlist. The sidecar runs as uid 65534, with--cap-drop ALL, a read-only root andip_forward=0.A task container that joins only the internal network. It gets:
HTTPS_PROXY/HTTP_PROXYpointing at the sidecar, andNODE_USE_ENV_PROXY=1NET_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 oneALLOW/DENYline 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"):
resolve_route/resolve_evaluation_route), so it cannot drift from the real routing.claude-codeagent, an enabled simulator, andllm_judge/agent_judge.BaseAgentConfig.egress_hosts()(codex, antigravity, pi).egress_allowlist.Fail-closed edges:
llm_onlytask instead of mapping it to a public network.llm_onlyand a driver other than docker is refused before any agent starts.Where the lines are
isolation/egress.py(allowlist, network, sidecar, cleanup),egress_proxy.py(the proxy),docker_runner.pywiring,models/fieldstest_docker_egress_runner.py,test_egress_proxy.py,test_egress_targets.py,test_docker_egress_config.pytasks/docker_egress_probe/(the end-to-end probe)docs/DOCKER_ISOLATION.md§ Network modes,.claude/notes/isolation.md§ The egress sidecarReview
The whole branch had a code review by three Opus reviewers. Fixed in this PR:
docker.login 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 withwrite_text_atomic.llm_onlywithoutdriver: dockerfailed open. It is now refused.BADlog lines no longer contain request targets, which can hold credentials.NET_RAWis dropped from the task container.EgressSetupError.The remaining Low items are recorded in the notes: no tunnel idle timeout,
--resumedoes 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 onmain.Real
llm_onlydocker runs on Docker Desktop 29 (macOS). Every row below was SUCCESS 1.0 except where stated:eu-north-1andus-east-2)gpt-5.6-lunaon Azurellm_judge,agent_judge, simulated dialog + judge, dataset fan-outllm_judgegpt-5.6-lunaegress_allowlist)gpt-5.6-luna(image with the CLI added)egress_allowlist)gpt-5.6-lunahost.docker.internal:4000(plain-HTTP forward)Also checked:
executethenevaluate: the grading container also runs underllm_only.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:pip,git,npmandWebFetch.llm_onlyscores 1.000; thebridgecontrol scores 0.176, which shows the probes detect real internet access.Regression: the CI smoke buckets on this branch.
smoke-passis 8/10; the 2 failures needbyod-custom-imageandTYPESAFE_API_KEY, which this machine does not have. All 3smoke-failsentinels fail as they must. Dockerbridgetasks pass: Claude, Antigravity,record_cli, and the reference anti-cheat task.Spikes, recorded in the notes:
docker:dind)Not verified here:
ANTHROPIC_API_KEYroute; on it,WebSearchruns on the provider's side and is outside the boundary (documented)api.openai.commain).Dependency bump (CI only)
uv.lock:urllib32.7.0 → 2.8.0 andvirtualenv21.2.0 → 21.7.13 (python-discoveryfollows). 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)docker/Dockerfile.runtimedownloads onlylinux-x64Node, so the runtime kit cannot be built on arm64.coder-eval evaluate <row>writesgrade.*.loginto a newruns/<timestamp>/directory, not into the row.🤖 Generated with Claude Code