feat(sandbox): Enforce network_mode allowlist on docker#785
Conversation
network_mode is now the authoritative network control across the launch path; the deprecated allow_internet boolean is a derived shim. Previously every backend keyed off allow_internet, so allowed_hosts never reached the sandbox and network_mode: allowlist was validated but unenforceable - rejected at preflight on every backend. - allowlist on the docker sandbox is now ENFORCED: the agent container joins an internal (no-egress) network and its HTTP(S) traffic routes through a stdlib egress-proxy sidecar that forwards only to allowed_hosts. Other hosts, raw-IP connections, and proxy-ignoring tools have no route off-box (default deny). New: sandbox/_egress.py, _egress_proxy.py. - sandbox/network_policy.py resolves the effective policy (open/block-all/ allowlist) and gates allowlist to backends that can enforce it; daytona/ modal fail closed (never silently open). - runtime_capabilities: allowlist supported on docker, rejected at preflight elsewhere with a clear message. - setup.py: preserve_agent_network lifts ANY restrictive policy (no-network OR allowlist) to public for web-disabled agent runs (they need the model API; no-web is enforced at the agent layer). - docs/task-authoring-task-md.md network-policy section + CHANGELOG. Verified on real docker (egress e2e): allowed host -> 200, non-allowed -> blocked, direct socket with proxy stripped -> no route. Full suite green.
Yiminnn
temporarily deployed
to
pypi-internal-preview
GitHub Actions
Inactive
Review (automated thorough pass) — ✅ no blocking defectsI ran an adversarial correctness/security review of the allowlist enforcement plus the targeted test + lint suite on a fresh checkout of the PR head. The enforcement is genuinely sound. Key claims verified:
Tests/lint: Non-blocking nits (follow-ups, not merge blockers)
Verdict: MERGE-READY (with the above as follow-ups). Leaving the actual merge to @Yiminnn since the branch was updated very recently and may still be iterating. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Wildcard allowed_hosts: a single leading `*.` label (e.g. *.example.com) matches subdomains at any depth (Harbor/nginx semantics) but not the apex; mid/trailing wildcards are rejected at parse time. Validator in task/config.py, matcher in sandbox/_egress_proxy.py. Model lane: a restrictive network_mode (no-network or allowlist) on docker now keeps one always-allow lane to the host-side benchflow model proxy open, so an agent run reaches the model without opening the sandbox to the public internet (no-network becomes model-only egress). The egress sidecar permits the docker host (_docker_host_address()) and routes to it via the bridge gateway / host-gateway. Replaces the blanket lift-to-public for web-disabled docker runs (extracted to _lift_agent_network_to_public, now docker-excluded); other sandboxes keep the lift. allow_model_endpoint:false (default true) closes the lane for a hermetic, no-model run. Live-verified through the real egress proxy: lane host + allowed host reachable, non-allowed host blocked (403); wildcard subdomain reachable, apex blocked. ENG-219
Yiminnn
temporarily deployed
to
pypi-internal-preview
GitHub Actions
Inactive
Greptile SummaryThis PR makes
Confidence Score: 4/5The core egress enforcement logic is sound and the lockdown verification loop prevents silent policy failures, but restore() in docker.py still reconnects restored containers to the unrestricted public bridge — an outstanding gap flagged in earlier review threads that this PR does not address. The new egress proxy, policy resolution, and lockdown_complete guard are all well-designed and tested. The two new inline comments are minor (IPv6 port parsing, custom-network error clarity) and do not affect the main enforcement path. The unresolved restore() bypass remains present and is the primary reason confidence falls short of the maximum. src/benchflow/sandbox/docker.py — specifically the restore() method which bypasses egress network enforcement after a snapshot restore. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant rollout as Rollout
participant docker as DockerSandbox
participant compose as docker compose
participant main as main container
participant sidecar as bf-egress sidecar
participant internet as Internet
rollout->>docker: "start() [_network_locked=False]"
docker->>compose: up --wait (no egress override)
compose->>main: start (on project_default, full internet)
rollout->>docker: relock_network(extra_allowed_hosts)
docker->>docker: "_network_locked = True"
docker->>docker: build_egress_override() write JSON
docker->>compose: up --no-deps bf-egress (egress override now in paths)
compose->>sidecar: start (on bf_egress_internal + bf_egress_external)
docker->>main: docker network connect bf_egress_internal
docker->>main: docker network disconnect project_default
docker->>main: docker inspect verify attached networks
main->>docker: lockdown_complete() check
alt lockdown verified
docker->>rollout: return HTTP_PROXY env
else lockdown failed
docker->>rollout: raise SandboxStartupError
end
rollout->>main: exec agent with proxy env
main->>sidecar: HTTP(S) via proxy bf-egress:8080
sidecar->>sidecar: _host_allowed(hostname)?
alt allowed host
sidecar->>internet: forward origin-form rewrite for HTTP
else denied
sidecar->>main: 403 Forbidden
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant rollout as Rollout
participant docker as DockerSandbox
participant compose as docker compose
participant main as main container
participant sidecar as bf-egress sidecar
participant internet as Internet
rollout->>docker: "start() [_network_locked=False]"
docker->>compose: up --wait (no egress override)
compose->>main: start (on project_default, full internet)
rollout->>docker: relock_network(extra_allowed_hosts)
docker->>docker: "_network_locked = True"
docker->>docker: build_egress_override() write JSON
docker->>compose: up --no-deps bf-egress (egress override now in paths)
compose->>sidecar: start (on bf_egress_internal + bf_egress_external)
docker->>main: docker network connect bf_egress_internal
docker->>main: docker network disconnect project_default
docker->>main: docker inspect verify attached networks
main->>docker: lockdown_complete() check
alt lockdown verified
docker->>rollout: return HTTP_PROXY env
else lockdown failed
docker->>rollout: raise SandboxStartupError
end
rollout->>main: exec agent with proxy env
main->>sidecar: HTTP(S) via proxy bf-egress:8080
sidecar->>sidecar: _host_allowed(hostname)?
alt allowed host
sidecar->>internet: forward origin-form rewrite for HTTP
else denied
sidecar->>main: 403 Forbidden
end
Reviews (14): Last reviewed commit: "fix(network): address multi-agent audit ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Plain HTTP: target is an absolute URI (http://host/path) in proxy form. | ||
| host = "" | ||
| if "://" in target: | ||
| host = target.split("://", 1)[1].split("/", 1)[0] | ||
| if not host: | ||
| for hl in header.split(b"\r\n"): | ||
| if hl.lower().startswith(b"host:"): | ||
| host = hl.split(b":", 1)[1].decode("latin-1").strip() | ||
| break | ||
| hostname = host.rsplit(":", 1)[0].strip("[]") | ||
| port = int(host.rsplit(":", 1)[1]) if ":" in host and "]" not in host else 80 | ||
| if not _host_allowed(hostname): | ||
| _deny(client, hostname) | ||
| return | ||
| try: | ||
| upstream = socket.create_connection((hostname, port), timeout=30) | ||
| except OSError: | ||
| client.sendall(b"HTTP/1.1 502 Bad Gateway\r\nConnection: close\r\n\r\n") | ||
| return | ||
| upstream.sendall(header) | ||
| _pipe(client, upstream) |
There was a problem hiding this comment.
For plain-HTTP requests the proxy forwards the raw header bytes — including the proxy request-line (GET http://host/path HTTP/1.1) — directly to the upstream server. RFC 7230 §5.3.2 says servers MUST accept the absolute-form, but a non-negligible number of back-ends (nginx with default config, some Python WSGI servers, and many embedded HTTP stacks) return 400 Bad Request for requests that arrive in absolute-URI form, which would cause tasks to see inexplicable failures for allowlisted plain-HTTP hosts.
The fix is to rewrite the request-line before forwarding: extract the path component from target (already split at line 122), reconstruct METHOD /path HTTP/1.1\r\n, and replace the first line of header before calling upstream.sendall.
| "NO_PROXY": "localhost,127.0.0.1", | ||
| "no_proxy": "localhost,127.0.0.1", | ||
| }, | ||
| "depends_on": [_EGRESS_SERVICE], |
There was a problem hiding this comment.
depends_on list form doesn't wait for proxy readiness
Using a plain list for depends_on tells Compose to start main as soon as bf-egress has been created, not once it is actually listening on port 8080. In practice the proxy binds almost instantly, but on a slow or loaded host the main container can fire its first proxied request (e.g. during agent framework import) before srv.accept() is running, resulting in Connection refused with no retry path. Switching to the long-form with condition: service_started is equivalent but explicitly documented; condition: service_healthy with a TCP healthcheck would eliminate the race entirely.
| "depends_on": [_EGRESS_SERVICE], | |
| "depends_on": { | |
| _EGRESS_SERVICE: {"condition": "service_started"}, | |
| }, |
There was a problem hiding this comment.
Fixed in 0c78eadd35647bc47fb7bb4536d64d8555696589.
The egress override now gives bf-egress a TCP healthcheck and makes main depend on service_healthy. relock_docker_network() also fails closed unless the sidecar container reports healthy before returning HTTP(S)_PROXY to the agent, so the proxy env is no longer exposed after mere service creation.
Validation on the pushed head:
uv run pytest tests/test_network_policy.py tests/test_network_modes.py tests/test_network_runtime.py -q-> 62 passeduv run ruff check src/benchflow/sandbox/_egress.py src/benchflow/sandbox/docker_network_lockdown.py tests/test_network_policy.py tests/test_network_runtime.pyuv run ruff format --check src/benchflow/sandbox/_egress.py src/benchflow/sandbox/docker_network_lockdown.py tests/test_network_policy.py tests/test_network_runtime.pyuv run ty check src/benchflow/sandbox/_egress.py src/benchflow/sandbox/docker_network_lockdown.py- GitHub CI is green on
0c78eadd3, includingtest,pip-audit,manifest-parity, andintegration-light / rollout-smoke. - The
integration-light-jobsartifact validates healthy withbenchflow-experiment-review: ACP trajectory, LLM trajectory,results.jsonl, reward present, 334350 tokens, 18 tool calls, andtraining_ready=1.
Moving the PR back to status:ready; keeping review:changes-requested because the security-sensitive PR still needs human re-review to clear the existing review decision.
| try: | ||
| upstream = socket.create_connection((hostname, port), timeout=30) | ||
| except OSError: | ||
| client.sendall(b"HTTP/1.1 502 Bad Gateway\r\nConnection: close\r\n\r\n") | ||
| return | ||
| upstream.sendall(header) | ||
| _pipe(client, upstream) | ||
| upstream.close() |
There was a problem hiding this comment.
upstream socket not closed on exception in the plain-HTTP path
upstream is created at line 134, but if upstream.sendall(header) (line 138) raises OSError the outer except Exception: pass silently swallows the error and upstream.close() (line 140) is never called. In CPython this is reclaimed by reference-counting when _handle returns, but in PyPy/non-reference-counted runtimes the descriptor leaks until GC. Wrapping the post-create_connection block in a try/finally (or using upstream as a context manager) would make cleanup explicit and deterministic.
New docs/network-mode-prior-art.md records the network-mode design model (no-network/public/allowlist + wildcard + model lane), a mode/mechanism taxonomy, and credits to the prior-art platforms the design draws on (Harbor, Modal, AISI Inspect, WAREX, SWE-bench, SWE-bench-Live, WebArena, Cybench, tau2-bench, browser-use) with primary-source links. Registered in the Mintlify nav and cross-linked from the task-authoring guide and sandbox-hardening. Citations verified against primary sources; corrects the Harbor PR#1854 framing (wildcard-depth clarification, not the feature origin) and notes the "N0/N1/N2" shorthand is ours, not AISI nomenclature. ENG-219
Network-mode verification —
|
strategy (environment.network_mode) |
docker | daytona |
|---|---|---|
| public | ✅ reward 1.0 | ✅ reward 1.0 |
allowlist — full (*.ncbi.nlm.nih.gov … + agent/model hosts) |
-32603 (model 404 — see note 3, not a proxy block) |
🚫 preflight REJECT (fail-closed) |
allowlist — deny (example.com only) |
🚫 ERR — proxy 403'd agent install | 🚫 preflight REJECT |
| no-network | 🚫 ERR — no route, agent can't install | ✅ reward 1.0 (see note 2) |
hermetic (no-network + allow_model_endpoint:false) |
🚫 ERR — no route | ✅ reward 1.0 (see note 2) |
docker batch — Score: 1/5, errors=4:
[ERR] citation-check-nonet (Agent openhands install failed (rc=127))
[ERR] citation-check-deny (Agent openhands install failed (rc=127))
[ERR] citation-check-hermetic (Agent openhands install failed (rc=127))
[ERR] citation-check-wildcard (ACP error -32603: Internal error)
Job complete: 1/5 (20.0%), errors=4, time=7.2min # the 1 pass = public (reward 1.0)
daytona batch — Score: 3/5, errors=2:
benchflow.task.runtime_capabilities.UnsupportedTaskFeatureError:
- environment.network_mode: network_mode='allowlist' is enforced only on the 'docker' sandbox
(egress proxy); not available on this sandbox — use 'docker', 'no-network', or 'public' (ENG-219) (sandbox=daytona)
[ERR] citation-check-deny (Task uses parsed runtime features that BenchFlow…)
[ERR] citation-check-wildcard (Task uses parsed runtime features that BenchFlow…)
Job complete: 3/5 (60.0%), errors=2, time=6.2min # passes = public, nonet, hermetic
3 · The proxy is enforcing in a live agent run (the load-bearing evidence)
For the deny variant the egress override is wired straight from the task.md, and the sidecar actively 403s every non-allowlisted host the agent reaches:
# applied egress override (from task.md allowed_hosts: [example.com])
{'ALLOWED_HOSTS': 'example.com', 'PORT': '8080', 'BENCHFLOW_EGRESS_LANE_HOST': '172.17.0.1'}
# openhands install log — proxy (172.21.0.2:8080) denies the Ubuntu mirrors (not allowlisted):
E: Failed to fetch http://archive.ubuntu.com/.../InRelease 403 Forbidden [IP: 172.21.0.2 8080]
E: Failed to fetch http://security.ubuntu.com/.../InRelease 403 Forbidden [IP: 172.21.0.2 8080]
When those install hosts are allowlisted, the proxy forwards them and the agent installs cleanly (apt over plain-HTTP through the proxy worked — no absolute-URI 400). So the allowlist permits exactly the listed hosts and denies the rest, in a real run.
Notes / findings
-
Model lane covers
host.docker.internal, but this deployment calls the model provider directly. The agent env setsLLM_BASE_URL=https://api.deepseek.com(not a host-side LiteLLM proxy athost.docker.internal:<port>), so the lane (172.17.0.1) is inert here and the real provider host must be allowlisted for a restrictive run. The lane mechanism itself is sound (verified separately: a host listener at172.17.0.1is reachable through the sidecar), but its automatic benefit only fires in the host-proxy topology. → worth folding into ENG-262 (the lane should also admit the resolved provider host under a restrictive policy, since that host is known at launch). -
docker vs daytona no-network strictness differs. docker enforces
no-networkfrom the start — the agent can't even install (rc=127). daytona'sno-networklet the agent install, reach the model, and solve (nonet/hermetic→ reward 1.0). daytona backends still readallow_internetdirectly rather than routing throughresolve_network_decision(the ENG-262 C1 defense-in-depth item), and appear to install the agent before applying the policy. Flagging as a real cross-backend inconsistency. -
The
-32603on dockerallowlistis a model-routing quirk, not a network block.deepseek-v4-flash404s on the configuredapi.deepseek.com(the model is actually served by a separate internal endpoint); even the public run hit 37×404before succeeding via retries to the working endpoint. The proxy permitted every listed host (install succeeded through it); the agent just couldn't complete the model call cleanly under the tighter time budget. Orthogonal to this PR.
Bottom line: the network_mode enforcement is solid — policy resolution is correct on both sandboxes, docker enforces allowlist/no-network (live 403s on unlisted hosts, agent install blocked under restrictive policy), daytona fails closed on allowlist at preflight, and allow_model_endpoint:false correctly closes the lane. The two items above (model-lane direct-provider coverage, daytona no-network strictness) are follow-ups, both already in ENG-262's scope.
Derive daytona block-all from resolve_network_decision (not the deprecated allow_internet bool) and verify it after start with a raw-TCP egress canary; abort with SandboxStartupError if a block-all policy can still reach the network (platform did not honor network_block_all).
…ctive policy (ENG-263) Under no-network/allowlist the agent cannot silently fall back to the direct provider (the egress allowlist blocks it) — so a skipped/unavailable LiteLLM usage proxy must abort the run instead of leaving the agent pointed at a blocked, untracked provider endpoint. Adds network_is_restrictive + proxy_unavailable_is_fatal and threads it through ensure_litellm_runtime.
…263) The container now comes up open so the agent can install; relock_network() then drops it off the public bridge (and, for allowlist/model-lane, starts the bf-egress sidecar + moves it onto the internal-only net, returning HTTP_PROXY for the agent). The main container is never recreated, so the install survives. Called from install_agent() after lockdown_paths. Fixes rc=127 agent-install failures under no-network/tight-allowlist on docker.
…s probe) (ENG-263) The daytona no-network leak was the _lift_agent_network_to_public blanket lift to public (the same P0 the model lane replaced for docker), still firing on non-docker. Gate the lift for daytona/daytona-dind too (no lane there -> the honest behavior is to keep the policy and fail closed). Also fix the egress canary probe to a single-line python -c (the heredoc form did not execute through daytona exec) so block-all enforcement is actually verified at start. Verified live: daytona no-network now resolves block_all=True and the canary (1.1.1.1:443) is unreachable -- genuinely offline, no longer silently public.
bingran-you
added
enhancement
|
Automation triage (2026-06-16): still belongs in |
Resolves conflicts from #787 (Mintlify docs retirement): - docs/docs.json: honored main's deletion (#787 retired the Mintlify config); the network-mode-prior-art nav entry is moot. - docs/sandbox-hardening.md: kept main's reworded task-authoring.md line and Yimin's network-mode-prior-art cross-link. Code merged cleanly. Verified: test_network_modes + test_network_policy + test_internet_policy + test_runtime_capabilities = 92 passed; ruff + format + ty clean on the sandbox surface.
bingran-you
temporarily deployed
to
pypi-internal-preview
GitHub Actions
Inactive
|
Automation (2026-06-16): merged The only conflicts were docs from #787 (Mintlify config retirement):
Code merged cleanly (config.py / test files auto-merged, no logic conflicts). Re-verified locally: PR should now be mergeable and CI can run on the fresh head. Still needs a human review (security-sensitive egress enforcement) + the docker/daytona network-mode E2E re-run before merge — removing |
bingran-you
added
review:pending
Runtime-behavior follow-up (ENG-263) — fixes + live verification on both sandboxesVerifying this PR's enforcement with a Fix #1 — docker install-before-lockdown (was: agent install
|
|
Users Simulation automation review (2026-06-27T12:21Z): blocked. Blockers:
Commands/evidence:
Please fix the required-usage contract, honor |
|
Users Simulation automation review (2026-07-01T12:30Z): functional network fixes improved, but merge remains blocked. Current validation on head
Remaining blockers:
|
|
Users Simulation review (2026-07-03): blocked. Focused tests still pass (
No model-backed trajectory was generated in this review. |
|
Users Simulation automation review (2026-07-04): still blocked on unchanged head The old dirty-branch issue appears gone in the local PR worktree and focused network tests passed, but the security-sensitive model-lane bug remains real:
Please wire |
|
Users Simulation automation review (2026-07-05): blocked on head Fresh isolated validation reproduced the security-sensitive blocker: Other checks: focused network/runtime tests passed ( Labels: keep |
|
Users Simulation automation review (2026-07-06): functional sandbox-policy checks pass on head Evidence:
Merge blockers remain live on GitHub: PR is still |
bingran-you
temporarily deployed
to
pypi-internal-preview
GitHub Actions
Inactive
|
Users Simulation automation update (2026-07-06): pushed |
bingran-you
added
status:ready
|
Users Simulation automation follow-up (2026-07-06): GitHub CI is now green on head |
|
Users Simulation automation review (2026-07-08): simulation ready, waiting on human re-review on head Evidence:
Caveat: the bundled review-skill fixture artifacts Metadata caveat: GLM artifact token/timing coverage is healthy, but Labels: |
bingran-you
added
the
status:blocked
ChatGPT Codex Connector
|
Daily scan follow-up (2026-07-09): restoring The original human decomposition/security blockers appear addressed on current head Please add a small readiness guard for the egress sidecar, preferably a healthcheck plus wait/poll before returning proxy env from relock, then rerun the focused network/runtime tests and rollout smoke. After that this can move back to |
|
Users Simulation automation review (2026-07-09): blocked on current head What is healthy now:
Remaining blocker:
Please add a small readiness guard for |
bingran-you
temporarily deployed
to
pypi-internal-preview
GitHub Actions
Inactive
bingran-you
added
status:ready
|
Daily scan follow-up (2026-07-10): re-reviewed the new head What changed since the last blocked comment:
Validation on the PR head in
Moved labels from |
|
Users Simulation automation review (2026-07-10): blocked on current head What passed:
Blocking issue:
Please either reject that configuration up front, or keep the runtime on a proxy path that is compatible with the declared policy so Additional hardening note: Docker restore is now blocked under restrictive policies, but the Daytona snapshot restore path still recreates from snapshot without an explicit re-lock step. I did not prove a bypass in the current suite, but that is the remaining restore path I would harden before calling this ready. Moving back to |
bingran-you
added
status:blocked
bingran-you
temporarily deployed
to
pypi-internal-preview
GitHub Actions
Inactive
|
Users Simulation automation follow-up (2026-07-10): ready by simulation on current head The fresh restrictive-model-lane blocker from #785 (comment) is addressed by
Validation:
Moving the PR back to |
bingran-you
added
status:ready
2 participants
Problem
network_modeis the documented outbound-network control, but the launch path historically keyed off the deprecatedallow_internetboolean. That madeallowlistvalidated but unenforceable: a task could declareallowed_hosts, yet the sandbox would either run open or reject too late.Current fix
This PR makes
network_modeauthoritative and fail-closed across the runtime path.maincontainer is moved onto an internal no-egress network after agent install, and HTTP(S) traffic goes through a stdlib egress proxy sidecar that forwards only toallowed_hostsplus the model lane when allowed./etc/hosts; wildcard, unresolvable, over-limit, and DinD cases fail closed.allow_model_endpoint: falsenow prevents the provider/model host from being appended to Docker/Daytona allowlists.docker.py/daytona.pyintodocker_network_lockdown.pyanddaytona_network_lockdown.py; the backend files are back under the 1k-line review threshold.Latest validation
Local validation on head
2f087e2d7:uv run pytest tests/test_network_runtime.py tests/test_network_policy.py tests/test_runtime_capabilities.py tests/test_network_modes.py tests/test_connect_as_env.py -q-> 95 passeduv run ruff checkon touched network/runtime files -> passeduv run ruff format --checkon touched Python files -> passeduv run ty check src/benchflow/sandbox/docker_network_lockdown.py src/benchflow/sandbox/daytona_network_lockdown.py src/benchflow/sandbox/docker.py src/benchflow/providers/litellm_runtime.py src/benchflow/task/runtime_capabilities.py-> passedgit diff --check-> passedGitHub after the push:
test,pip-audit,manifest-parity, and detect-scope are green;rollout-smokeis still pending.Remaining gate
Keep this blocked until
rollout-smokeis green and the security-sensitive decomposition is re-reviewed.