fix(sandbox): resolve the no-web loopback endpoint per adapter - #940
fix(sandbox): resolve the no-web loopback endpoint per adapter#940bingran-you wants to merge 1 commit into
Conversation
`enforce_agent_egress_firewall` read the gateway URL from `LLM_BASE_URL`
alone. Only openhands is wired that way. `_agent_env_for_route` gives
opencode and codex-acp `OPENAI_BASE_URL`, claude-agent-acp
`ANTHROPIC_BASE_URL`, and pi-acp `BENCHFLOW_PROVIDER_BASE_URL`, so for every
one of those adapters the precondition was unsatisfiable:
RuntimeError: No-web agent requires an HTTP loopback LLM_BASE_URL with a port
raised before the agent was ever prompted. Any task declaring
`network_mode: no-network` was therefore impossible to run on four of five
ACP adapters unless the caller disabled the sandbox user entirely -- which
also skips the uid-based egress firewall this check exists to guard.
Validate whichever endpoint variables are actually set instead. All of them
are stripped and re-pointed at the loopback gateway by
`_agent_env_for_route`, so any that survives to firewall time must already be
loopback; requiring *every* present endpoint to be loopback (not just one) is
strictly stronger than the previous single-variable check, and the agent is
about to lose egress to a routable one either way.
lockdown cannot import `litellm_runtime` -- that module imports
`benchflow.sandbox` -- so the name list is duplicated and pinned by a test
asserting it covers `_PROVIDER_ENDPOINT_ENV_NAMES`. A new provider endpoint
variable now fails that test rather than silently escaping the precondition.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfa102b636
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """Only openhands spells the gateway LLM_BASE_URL. Requiring that one | ||
| name made no-network tasks impossible for every other adapter: the | ||
| firewall raised before the agent was ever prompted.""" |
There was a problem hiding this comment.
This new test explicitly reproduces the four-adapter regression fixed by this commit, but its docstring does not identify the PR or commit it guards. Add the commit reference (for example, Guards the fix from commit 936edc7 ...) so the test follows the repository's regression-test convention.
AGENTS.md reference: AGENTS.md:L16-L17
Useful? React with 👍 / 👎.
1 participant
Problem
Any task with
sandbox.network_mode: no-networkfails before the agent isprompted on four of the five ACP adapters:
enforce_agent_egress_firewallreads the gateway URL fromLLM_BASE_URL, but_agent_env_for_routeonly sets that name for openhands:LLM_BASE_URLOPENAI_BASE_URLOPENAI_BASE_URLANTHROPIC_BASE_URLBENCHFLOW_PROVIDER_BASE_URLThe check is a precondition on a firewall that rejects all non-loopback egress
for the sandbox user — reasonable in intent, but it hardcoded one adapter's
spelling, so for the rest it can never be satisfied. The only workaround is
--sandbox-user null, which runs the agent as root and skips the uid-basedegress firewall this check exists to guard. So the practical effect is that
no-network tasks either don't run or run without the isolation they asked for.
Found while running a 29-task evaluation on opencode: 9 no-network tasks × 6
arms could not start.
Fix
Validate whichever endpoint variables are actually present, rather than one
hardcoded name.
_agent_env_for_routestrips every provider endpoint variable(
_PROVIDER_ENDPOINT_ENV_NAMES) and re-points the agent at the loopbackgateway, so anything still set at firewall time must already be loopback.
Requiring every present endpoint to be loopback — not just one — is
strictly stronger than the previous single-variable check: a loopback route on
OPENAI_BASE_URLno longer excuses a routableANTHROPIC_BASE_URL, which thesandbox user is about to lose egress to regardless. An empty set still raises.
lockdowncannot importlitellm_runtime(that module importsbenchflow.sandbox), so the name list is duplicated and pinned bytest_no_web_endpoint_names_cover_provider_endpoint_names. Adding a providerendpoint variable there without adding it here now fails a test instead of
silently exempting it — the same class of bug this PR fixes.
Tests
opencode / codex-acp / claude-agent-acp / pi-acp).
offender.
tests/test_sdk_lockdown.py57/57; 332 passed across the lockdown/ACP suites.ruff checkandruff format --checkclean.The one pre-existing test that asserted the old message text
(
match="loopback LLM_BASE_URL") is updated to the new wording; its behaviorassertion is unchanged.
🤖 Generated with Claude Code