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
58 changes: 41 additions & 17 deletions src/benchflow/agents/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,16 +294,39 @@ def _js_agent_launch(binary: str, args: str = "") -> str:


def _json_settings_merge(path: str, mutator: str) -> str:
"""Idempotent JSON-settings merge as a one-line bash snippet."""
py = (
"import json,os,pathlib;"
f"p=pathlib.Path(os.path.expandvars(os.path.expanduser({path!r})));"
"p.parent.mkdir(parents=True, exist_ok=True);"
"d=json.loads(p.read_text()) if p.exists() and p.read_text().strip() else {};"
"""Idempotent JSON-settings merge as a one-line bash snippet.

Runs on the Node runtime ``_NODE_INSTALL`` provisions, not on ``python3``:
policy setup executes inside the *task* image, which owes BenchFlow no
interpreter, so a Python-free image used to abort a rollout with exit 127
before ACP launch (#1047). Callers must therefore be agents whose install
guarantees ``_BENCHFLOW_NODE_PREFIX`` — every JS agent does, via
``_js_agent_install`` or the mimo manifest.

``mutator`` is a JavaScript statement operating on ``d``. The escape pass
reproduces ``json.dumps``' ``ensure_ascii`` so upgrading BenchFlow leaves
an existing settings file byte-identical: agent homes can arrive
pre-populated from the host (gemini copies ``~/.gemini/settings.json`` in
via ``subscription_auth``), and rewriting a user's non-ASCII values on
first run would break the idempotence this merge promises. It starts at
U+007F because ``JSON.stringify`` has already escaped in-string control
characters, while the newlines and indent it emits must stay literal.
"""
js = (
"const fs=require('fs'),pa=require('path');"
"const p=process.argv[1];"
"fs.mkdirSync(pa.dirname(p),{recursive:true});"
"let d={};"
"try{const t=fs.readFileSync(p,'utf8');if(t.trim())d=JSON.parse(t);}"
"catch(e){if(e.code!=='ENOENT')throw e;}"
f"{mutator};"
"p.write_text(json.dumps(d, indent=2) + '\\n')"
"fs.writeFileSync(p,JSON.stringify(d,null,2)"
".replace(/[\\u007f-\\uffff]/g,"
"c=>'\\\\u'+c.charCodeAt(0).toString(16).padStart(4,'0'))+'\\n');"
)
return f"python3 -c {shlex.quote(py)}"
# The path stays a shell word so the surrounding bash expands
# $BENCHFLOW_AGENT_HOME, matching what os.path.expandvars did before.
return f'{_BENCHFLOW_NODE_PREFIX}/bin/node -e {shlex.quote(js)} "{path}"'


# OpenCode-family proxy fix: OpenCode and its MiMo fork validate provider/model
Expand Down Expand Up @@ -545,9 +568,10 @@ class AgentConfig:
),
disallow_web_tools_setup_cmd=_json_settings_merge(
"$BENCHFLOW_AGENT_HOME/.claude/settings.json",
'd.setdefault("permissions",{}).setdefault("deny",[]);'
'[d["permissions"]["deny"].append(t) for t in ["WebSearch","WebFetch"] '
'if t not in d["permissions"]["deny"]]',
"if(!('permissions' in d))d.permissions={};"
"if(!('deny' in d.permissions))d.permissions.deny=[];"
'for(const t of ["WebSearch","WebFetch"])'
"if(!d.permissions.deny.includes(t))d.permissions.deny.push(t)",
),
disallow_web_tools_owned_paths=["$HOME/.claude"],
supports_acp_set_model=False,
Expand Down Expand Up @@ -690,10 +714,10 @@ class AgentConfig:
),
disallow_web_tools_setup_cmd=_json_settings_merge(
"$BENCHFLOW_AGENT_HOME/.gemini/settings.json",
'd.setdefault("tools",{}).setdefault("exclude",[]);'
'[d["tools"]["exclude"].append(t) for t in '
'["google_web_search","web_fetch"] '
'if t not in d["tools"]["exclude"]]',
"if(!('tools' in d))d.tools={};"
"if(!('exclude' in d.tools))d.tools.exclude=[];"
'for(const t of ["google_web_search","web_fetch"])'
"if(!d.tools.exclude.includes(t))d.tools.exclude.push(t)",
),
disallow_web_tools_owned_paths=["$HOME/.gemini"],
),
Expand Down Expand Up @@ -721,7 +745,7 @@ class AgentConfig:
},
disallow_web_tools_setup_cmd=_json_settings_merge(
"$BENCHFLOW_AGENT_HOME/.config/opencode/opencode.json",
'd.setdefault("tools",{})["webfetch"]=False',
"if(!('tools' in d))d.tools={};d.tools.webfetch=false",
),
disallow_web_tools_owned_paths=["$HOME/.config/opencode"],
),
Expand Down Expand Up @@ -770,7 +794,7 @@ class AgentConfig:
},
disallow_web_tools_setup_cmd=_json_settings_merge(
"$BENCHFLOW_AGENT_HOME/.config/mimocode/mimocode.json",
'd.setdefault("tools",{})["webfetch"]=False',
"if(!('tools' in d))d.tools={};d.tools.webfetch=false",
),
disallow_web_tools_owned_paths=["$HOME/.config/mimocode"],
),
Expand Down
117 changes: 115 additions & 2 deletions tests/test_internet_policy.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import shutil
import subprocess
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock, patch
Expand Down Expand Up @@ -266,13 +267,25 @@ def test_apply_web_policy_noop_when_not_disallowed():

def _run_setup_cmd(agent_name: str, tmp_path) -> dict:
"""Execute an agent's disallow_web_tools_setup_cmd and return the JSON it wrote."""
from benchflow.agents.registry import AGENTS
from benchflow.agents.registry import _BENCHFLOW_NODE_PREFIX, AGENTS

cfg = AGENTS[agent_name]
assert cfg.disallow_web_tools_setup_cmd, f"{agent_name} has no setup_cmd"

# JSON-merge policies target the Node runtime BenchFlow provisions inside the
# sandbox (#1047); a dev host has no such prefix, so retarget the local one.
# This is what kept the old python3 dependency invisible to these tests, so
# coverage on a real task image lives in the container test below.
cmd = cfg.disallow_web_tools_setup_cmd
sandbox_node = f"{_BENCHFLOW_NODE_PREFIX}/bin/node"
if sandbox_node in cmd:
host_node = shutil.which("node")
if host_node is None:
pytest.skip("no node runtime on PATH to exercise the JSON-merge policy")
cmd = cmd.replace(sandbox_node, host_node)

result = subprocess.run(
["bash", "-c", cfg.disallow_web_tools_setup_cmd],
["bash", "-c", cmd],
env={"BENCHFLOW_AGENT_HOME": str(tmp_path), "PATH": "/usr/bin:/bin"},
capture_output=True,
text=True,
Expand Down Expand Up @@ -424,3 +437,103 @@ def test_task_toml_missing_allow_internet_defaults_to_allowed(tmp_path):

task = Task(task_dir)
assert _task_disallows_internet(task) is False


def test_json_merge_policies_do_not_borrow_task_image_interpreters():
"""Policy setup runs in the task image, which owes BenchFlow no runtime.

The JSON merge used to shell out to ``python3``; a Python-free task image
with ``allow_internet = false`` then aborted at exit 127 before ACP launch
(#1047).
"""
from benchflow.agents.registry import _BENCHFLOW_NODE_PREFIX, AGENTS

for agent in ("claude-agent-acp", "gemini", "opencode", "mimo"):
cmd = AGENTS[agent].disallow_web_tools_setup_cmd
assert "python3" not in cmd, agent
assert cmd.startswith(f"{_BENCHFLOW_NODE_PREFIX}/bin/node "), agent


def test_node_backed_policies_are_guaranteed_by_their_own_install():
"""Whoever depends on the Node runtime must also be the one installing it.

Pins the ownership boundary #1047 is about, rather than today's four
agents: a policy may only reach for the Node prefix if that agent's own
install_cmd provisions it.
"""
from benchflow.agents.registry import _BENCHFLOW_NODE_PREFIX, AGENTS

for name, cfg in AGENTS.items():
cmd = cfg.disallow_web_tools_setup_cmd or ""
if f"{_BENCHFLOW_NODE_PREFIX}/bin/node" not in cmd:
continue
assert _BENCHFLOW_NODE_PREFIX in (cfg.install_cmd or ""), (
f"{name} runs its no-web policy on the BenchFlow Node runtime but "
"its install does not provision one"
)


@pytest.mark.skipif(
not shutil.which("docker"), reason="needs a Docker daemon to build the task image"
)
async def test_no_web_policies_apply_in_a_python_free_task_image():
"""End-to-end guard for #1047 on an image that ships no Python at all.

Provisions the agent runtime first — exactly what the JS installers do
before policy application — then applies each policy twice, so the run
covers the merge content and the idempotence it promises.
"""
from benchflow.agents.install import apply_web_tool_policy
from benchflow.agents.registry import _NODE_INSTALL, AGENTS

expected = {
"claude-agent-acp": (
"/root/.claude/settings.json",
{"permissions": {"deny": ["WebSearch", "WebFetch"]}},
),
"gemini": (
"/root/.gemini/settings.json",
{"tools": {"exclude": ["google_web_search", "web_fetch"]}},
),
"opencode": (
"/root/.config/opencode/opencode.json",
{"tools": {"webfetch": False}},
),
"mimo": (
"/root/.config/mimocode/mimocode.json",
{"tools": {"webfetch": False}},
),
}

def sh(*args: str) -> subprocess.CompletedProcess:
return subprocess.run(args, capture_output=True, text=True, timeout=900)

container = subprocess.check_output(
["docker", "run", "--detach", "--rm", "ubuntu:24.04", "sleep", "infinity"],
text=True,
).strip()

class ContainerEnv:
async def exec(self, command: str, *, timeout_sec=None, **_kwargs):
done = sh("docker", "exec", container, "bash", "-lc", command)
return SimpleNamespace(
return_code=done.returncode, stdout=done.stdout, stderr=done.stderr
)

try:
probe = sh("docker", "exec", container, "bash", "-lc", "command -v python3")
assert probe.returncode != 0, "base image unexpectedly ships python3"

provision = sh("docker", "exec", container, "bash", "-lc", _NODE_INSTALL)
assert provision.returncode == 0, provision.stderr[-400:]

for agent, (path, want) in expected.items():
for _ in range(2):
await apply_web_tool_policy(
ContainerEnv(), agent, AGENTS[agent], "/root", disallow=True
)
written = sh("docker", "exec", container, "cat", path)
assert written.returncode == 0, f"{agent}: {path} not written"
assert json.loads(written.stdout) == want, agent
finally:
sh("docker", "rm", "--force", container)