From dfa102b636f322aeaa630a1bd40cd12106d00d25 Mon Sep 17 00:00:00 2001 From: Bingran You Date: Thu, 30 Jul 2026 04:39:37 +0000 Subject: [PATCH] fix(sandbox): resolve the no-web loopback endpoint per adapter `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 --- src/benchflow/sandbox/lockdown.py | 77 ++++++++++++++++++++++++++---- tests/test_sdk_lockdown.py | 79 ++++++++++++++++++++++++++++++- 2 files changed, 147 insertions(+), 9 deletions(-) diff --git a/src/benchflow/sandbox/lockdown.py b/src/benchflow/sandbox/lockdown.py index 27363f491..700d7b0d7 100644 --- a/src/benchflow/sandbox/lockdown.py +++ b/src/benchflow/sandbox/lockdown.py @@ -28,6 +28,27 @@ logger = logging.getLogger(__name__) +# Every variable through which an agent can be handed a model endpoint. Kept in +# sync with providers.litellm_runtime._PROVIDER_ENDPOINT_ENV_NAMES (which that +# module strips before re-pointing the agent at the gateway) by +# test_no_web_endpoint_names_cover_provider_endpoint_names -- importing it here +# would close a cycle, since litellm_runtime imports benchflow.sandbox. +AGENT_LLM_ENDPOINT_ENV_NAMES = frozenset( + { + "LLM_BASE_URL", + "OPENAI_BASE_URL", + "OPENAI_API_BASE", + "AZURE_API_ENDPOINT", + "AZURE_API_BASE", + "AZURE_OPENAI_ENDPOINT", + "ANTHROPIC_BASE_URL", + "ANTHROPIC_BEDROCK_BASE_URL", + "GEMINI_BASE_URL", + "GOOGLE_GEMINI_BASE_URL", + "BENCHFLOW_PROVIDER_BASE_URL", + } +) + # Path lockdown defaults and validation @@ -143,6 +164,37 @@ def build_priv_drop_cmd(agent_launch: str, sandbox_user: str) -> str: ) +def _is_loopback_http_url(value: str) -> bool: + parsed = urlsplit(value) + return ( + parsed.scheme == "http" + and parsed.hostname in {"127.0.0.1", "localhost"} + and parsed.port is not None + ) + + +def classify_agent_llm_endpoints( + agent_env: dict[str, str], +) -> tuple[list[str], list[str]]: + """Split the agent's LLM endpoint variables into (present, non-loopback). + + Every adapter injects the gateway under its own name, so there is no single + variable to inspect: openhands uses ``LLM_BASE_URL``, opencode and codex-acp + use ``OPENAI_BASE_URL``, claude-agent-acp uses ``ANTHROPIC_BASE_URL``, and + ``BENCHFLOW_PROVIDER_BASE_URL`` is set for every routed agent. + """ + present: list[str] = [] + offending: list[str] = [] + for name in AGENT_LLM_ENDPOINT_ENV_NAMES: + value = agent_env.get(name) + if not value: + continue + present.append(name) + if not _is_loopback_http_url(value): + offending.append(name) + return present, offending + + async def enforce_agent_egress_firewall( env: Any, sandbox_user: str | None, @@ -152,15 +204,24 @@ async def enforce_agent_egress_firewall( if not sandbox_user or agent_env.get("BENCHFLOW_DISALLOW_WEB_TOOLS") != "1": return - base_url = agent_env.get("LLM_BASE_URL", "") - parsed = urlsplit(base_url) - if ( - parsed.scheme != "http" - or parsed.hostname not in {"127.0.0.1", "localhost"} - or parsed.port is None - ): + # The firewall below rejects every non-loopback destination for the sandbox + # user, so the agent's model route has to already be loopback or the agent + # loses its LLM the moment the rule lands. Checking one hardcoded variable + # made that precondition unsatisfiable for every adapter that does not + # happen to spell it LLM_BASE_URL -- opencode, codex-acp, claude-agent-acp + # and pi-acp could never run a no-network task at all. Validate whichever + # endpoints are actually set, and require at least one. + present, offending = classify_agent_llm_endpoints(agent_env) + if not present: + raise RuntimeError( + "No-web agent requires an HTTP loopback LLM endpoint with a port, " + "but none of " + f"{', '.join(sorted(AGENT_LLM_ENDPOINT_ENV_NAMES))} is set" + ) + if offending: raise RuntimeError( - "No-web agent requires an HTTP loopback LLM_BASE_URL with a port" + "No-web agent requires an HTTP loopback LLM endpoint with a port; " + f"not loopback: {', '.join(sorted(offending))}" ) result = await env.exec( diff --git a/tests/test_sdk_lockdown.py b/tests/test_sdk_lockdown.py index 1f4e2cfea..76a9f5576 100644 --- a/tests/test_sdk_lockdown.py +++ b/tests/test_sdk_lockdown.py @@ -459,7 +459,7 @@ async def test_policy_rejects_non_loopback_proxy(self): env = MagicMock() env.exec = AsyncMock() - with pytest.raises(RuntimeError, match="loopback LLM_BASE_URL"): + with pytest.raises(RuntimeError, match="not loopback: LLM_BASE_URL"): await enforce_agent_egress_firewall( env, "agent", @@ -470,3 +470,80 @@ async def test_policy_rejects_non_loopback_proxy(self): ) env.exec.assert_not_awaited() + + @pytest.mark.parametrize( + ("agent", "endpoint_var"), + [ + ("opencode", "OPENAI_BASE_URL"), + ("codex-acp", "OPENAI_BASE_URL"), + ("claude-agent-acp", "ANTHROPIC_BASE_URL"), + ("pi-acp", "BENCHFLOW_PROVIDER_BASE_URL"), + ], + ) + async def test_policy_accepts_each_adapters_own_endpoint_var( + self, agent, endpoint_var + ): + """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.""" + from benchflow.sandbox.lockdown import enforce_agent_egress_firewall + + env = MagicMock() + env.exec = AsyncMock(return_value=MagicMock(return_code=0)) + + await enforce_agent_egress_firewall( + env, + "agent", + { + "BENCHFLOW_DISALLOW_WEB_TOOLS": "1", + endpoint_var: "http://127.0.0.1:1234/v1", + }, + ) + + env.exec.assert_awaited_once() + + async def test_policy_rejects_any_non_loopback_endpoint(self): + """A loopback route on one variable does not excuse a routable one on + another -- the sandbox user is about to lose egress to it either way.""" + from benchflow.sandbox.lockdown import enforce_agent_egress_firewall + + env = MagicMock() + env.exec = AsyncMock() + + with pytest.raises(RuntimeError, match="not loopback: ANTHROPIC_BASE_URL"): + await enforce_agent_egress_firewall( + env, + "agent", + { + "BENCHFLOW_DISALLOW_WEB_TOOLS": "1", + "OPENAI_BASE_URL": "http://127.0.0.1:1234/v1", + "ANTHROPIC_BASE_URL": "https://api.anthropic.com", + }, + ) + + env.exec.assert_not_awaited() + + async def test_policy_rejects_when_no_endpoint_is_set(self): + from benchflow.sandbox.lockdown import enforce_agent_egress_firewall + + env = MagicMock() + env.exec = AsyncMock() + + with pytest.raises(RuntimeError, match="none of"): + await enforce_agent_egress_firewall( + env, + "agent", + {"BENCHFLOW_DISALLOW_WEB_TOOLS": "1"}, + ) + + env.exec.assert_not_awaited() + + def test_no_web_endpoint_names_cover_provider_endpoint_names(self): + """lockdown cannot import litellm_runtime (that module imports + benchflow.sandbox), so the endpoint name list is duplicated. Adding a + provider endpoint variable there without adding it here would silently + exempt it from the loopback precondition -- fail loudly instead.""" + from benchflow.providers.litellm_runtime import _PROVIDER_ENDPOINT_ENV_NAMES + from benchflow.sandbox.lockdown import AGENT_LLM_ENDPOINT_ENV_NAMES + + assert _PROVIDER_ENDPOINT_ENV_NAMES <= AGENT_LLM_ENDPOINT_ENV_NAMES