Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 69 additions & 8 deletions src/benchflow/sandbox/lockdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand Down
79 changes: 78 additions & 1 deletion tests/test_sdk_lockdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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."""
Comment on lines +486 to +488

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Name the guarded fix in the regression-test docstring

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 👍 / 👎.

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
Loading