fix(browser): gate browser_exec on terminal surface; pin schema helpers digest
Follow-ups on the salvaged Browser Use CLI integration (PR #66476): - browser_exec runs model-written Python on the host. Strip it at tool-definition time for sessions whose resolved toolsets exclude 'terminal' so terminal-less surfaces (locked-down messaging configs) don't silently regain host code execution through the browser toolset. Session-level gate in model_tools, not a check_fn (check_fn results are TTL-cached process-wide across sessions). - Replace the live 'browser-use skill' schema fetch with a pinned helpers digest: no third-party version-drifting text in the prompt, byte-stable schema across machines. A/B benchmarked (108 runs, opus-4.8 + kimi-k3, 6 multi-step web tasks x 3 arms x 3 reps): pinned digest matches the full skill dump 36/36 vs 36/36 at ~equal tokens; both cut total task tokens ~60% vs the legacy browser_* toolset. - Docs note for the terminal gate; contributor mapping for salvage.
This commit is contained in:
parent
9e5e1740ed
commit
39a234b133
|
|
@ -0,0 +1 @@
|
|||
laithrw
|
||||
|
|
@ -551,6 +551,21 @@ def _compute_tool_definitions(
|
|||
}
|
||||
break
|
||||
|
||||
# browser_exec (Browser Use mode) runs arbitrary Python on the host via
|
||||
# the browser-use CLI subprocess. A session whose toolset selection
|
||||
# excludes the terminal surface (e.g. a messaging platform configured
|
||||
# without terminal access) must not regain host code execution through
|
||||
# the browser toolset — that would silently widen the operator's chosen
|
||||
# security posture. Session-level gate, NOT a check_fn: check_fn results
|
||||
# are TTL-cached process-wide while one gateway process serves many
|
||||
# sessions with different toolset configs.
|
||||
if "browser_exec" in available_tool_names and "terminal" not in available_tool_names:
|
||||
filtered_tools = [
|
||||
td for td in filtered_tools
|
||||
if td.get("function", {}).get("name") != "browser_exec"
|
||||
]
|
||||
available_tool_names.discard("browser_exec")
|
||||
|
||||
if not quiet_mode:
|
||||
if filtered_tools:
|
||||
tool_names = [t["function"]["name"] for t in filtered_tools]
|
||||
|
|
|
|||
|
|
@ -87,6 +87,36 @@ class TestToolSurfaceSwap:
|
|||
assert "browser_exec" in TOOLSETS["browser"]["tools"]
|
||||
assert "browser_exec" in TOOLSETS["coding"]["tools"]
|
||||
|
||||
def test_browser_exec_stripped_without_terminal(self, monkeypatch):
|
||||
"""Sessions without the terminal surface must not regain host code
|
||||
execution through browser_exec (arbitrary Python via the CLI)."""
|
||||
monkeypatch.setattr(bu_cli, "is_browser_use_cli_mode", lambda: True)
|
||||
from tools.registry import registry
|
||||
|
||||
entry = registry.get_entry("browser_exec")
|
||||
monkeypatch.setattr(entry, "check_fn", lambda: True)
|
||||
import model_tools
|
||||
|
||||
defs = model_tools.get_tool_definitions(
|
||||
enabled_toolsets=["browser"], quiet_mode=False
|
||||
)
|
||||
names = {t["function"]["name"] for t in defs}
|
||||
assert "browser_exec" not in names
|
||||
|
||||
def test_browser_exec_present_with_terminal(self, monkeypatch):
|
||||
monkeypatch.setattr(bu_cli, "is_browser_use_cli_mode", lambda: True)
|
||||
from tools.registry import registry
|
||||
|
||||
entry = registry.get_entry("browser_exec")
|
||||
monkeypatch.setattr(entry, "check_fn", lambda: True)
|
||||
import model_tools
|
||||
|
||||
defs = model_tools.get_tool_definitions(
|
||||
enabled_toolsets=["browser", "terminal"], quiet_mode=False
|
||||
)
|
||||
names = {t["function"]["name"] for t in defs}
|
||||
assert "browser_exec" in names
|
||||
|
||||
|
||||
class TestFindCli:
|
||||
def test_prefers_installed_binary(self, monkeypatch):
|
||||
|
|
@ -287,10 +317,14 @@ class TestBrowserUseSlashCommand:
|
|||
assert saved["browser"]["backend"] == "browser-use"
|
||||
assert stub.session_resets == 1
|
||||
|
||||
def test_use_off_removes_backend(self, monkeypatch):
|
||||
def test_use_off_pins_backend_off(self, monkeypatch):
|
||||
"""`off` must be written explicitly (BACKEND_DISABLED), not removed:
|
||||
with the key merely deleted, is_legacy_browser_use_cloud_config()
|
||||
would re-activate CLI mode on the next start for anyone with
|
||||
BROWSER_USE_API_KEY set, so /browser use off wouldn't stick."""
|
||||
config = {"browser": {"backend": "browser-use"}}
|
||||
stub, saved = self._run("/browser use off", config, monkeypatch)
|
||||
assert "backend" not in saved["browser"]
|
||||
assert saved["browser"]["backend"] == bu_cli.BACKEND_DISABLED
|
||||
assert stub.session_resets == 1
|
||||
|
||||
def test_use_bad_arg_prints_usage_without_writing(self, monkeypatch):
|
||||
|
|
@ -420,33 +454,32 @@ class TestHeaderVariants:
|
|||
|
||||
|
||||
class TestSkillTextDescription:
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_skill_cache(self, monkeypatch):
|
||||
monkeypatch.setattr(bu_cli, "_skill_text_cache", None)
|
||||
monkeypatch.setattr(bu_cli, "_skill_text_fetched", False)
|
||||
yield
|
||||
"""The schema description is fully pinned: header + _HELPERS_DIGEST.
|
||||
|
||||
def test_description_is_verbatim_cli_skill_text(self, tmp_path, monkeypatch):
|
||||
cli = _fake_cli(
|
||||
tmp_path,
|
||||
'if [ "$1" = "skill" ]; then echo "# Browser Use\nverbatim skill body"; fi\n',
|
||||
The live ``browser-use skill`` fetch was removed after A/B benchmarking
|
||||
showed the pinned digest matches the full skill dump on success rate
|
||||
(36/36 vs 36/36, opus-4.8 + kimi-k3) — see tools/browser_use_cli.py.
|
||||
"""
|
||||
|
||||
def test_description_is_pinned_header_plus_digest(self, monkeypatch):
|
||||
# Even with a CLI present, the description must NOT shell out.
|
||||
monkeypatch.setattr(
|
||||
bu_cli, "_find_cli",
|
||||
lambda: (_ for _ in ()).throw(AssertionError("schema must not invoke the CLI")),
|
||||
)
|
||||
monkeypatch.setattr(bu_cli, "_find_cli", lambda: [cli])
|
||||
overrides = bu_cli._dynamic_schema_overrides()
|
||||
assert overrides["description"].startswith(bu_cli._DESCRIPTION_HEADER)
|
||||
assert overrides["description"].endswith("# Browser Use\nverbatim skill body")
|
||||
assert overrides["description"].startswith(bu_cli._HEADER_BASE)
|
||||
assert overrides["description"].endswith(bu_cli._HELPERS_DIGEST)
|
||||
|
||||
def test_skill_text_cached_after_first_fetch(self, tmp_path, monkeypatch):
|
||||
calls = []
|
||||
cli = _fake_cli(tmp_path, 'echo "skill text"\n')
|
||||
monkeypatch.setattr(bu_cli, "_find_cli", lambda: (calls.append(1), [cli])[1])
|
||||
assert "skill text" in bu_cli._cli_skill_text()
|
||||
assert "skill text" in bu_cli._cli_skill_text()
|
||||
assert len(calls) == 1
|
||||
def test_digest_names_core_helpers(self):
|
||||
for helper in ("new_tab(", "page_info()", "js(", "fill_input(",
|
||||
"click_at_xy(", "capture_screenshot()", "cdp("):
|
||||
assert helper in bu_cli._HELPERS_DIGEST
|
||||
|
||||
def test_no_override_when_cli_missing(self, monkeypatch):
|
||||
monkeypatch.setattr(bu_cli, "_find_cli", lambda: None)
|
||||
assert bu_cli._dynamic_schema_overrides() == {}
|
||||
def test_static_fallback_carries_digest_and_install_hint(self):
|
||||
desc = bu_cli.BROWSER_EXEC_SCHEMA["description"]
|
||||
assert bu_cli._HELPERS_DIGEST in desc
|
||||
assert "uv tool install browser-use" in desc
|
||||
|
||||
|
||||
class TestBrowserExec:
|
||||
|
|
|
|||
|
|
@ -308,13 +308,8 @@ _HEADER_BASE = (
|
|||
"Batch each sub-procedure (navigate, wait, extract, act) into one call "
|
||||
"— do not spend a call per action — but for long extractions prefer "
|
||||
"several medium calls that append to workspace files over one giant "
|
||||
"call, so progress survives timeouts. js() takes a JS expression: "
|
||||
"js('document.title') or js('(() => {...})()') — a bare '() => {...}' "
|
||||
"returns the function itself, uncalled. The CLI's own documentation "
|
||||
"follows and is complete (no need to read separate browser-use skill "
|
||||
"files) — where it shows shell heredocs (browser-use <<'PY' … PY), pass "
|
||||
"the Python as `code` instead; where it shows BU_NAME=<name>, pass "
|
||||
"session=<name> instead."
|
||||
"call, so progress survives timeouts. For a named cloud browser, pass "
|
||||
"session=<name> (never BU_NAME env syntax)."
|
||||
)
|
||||
|
||||
_HEADER_VISION = (
|
||||
|
|
@ -331,7 +326,13 @@ _HEADER_TEXT_ONLY = (
|
|||
"clicks — skip the screenshot-driven workflow described below."
|
||||
)
|
||||
|
||||
_DESCRIPTION_HEADER = _HEADER_BASE
|
||||
_DESCRIPTION_HEADER = _HEADER_BASE # back-compat alias for external imports
|
||||
|
||||
# NOTE: browser_exec is additionally gated at tool-definition time — sessions
|
||||
# whose resolved toolsets do not include ``terminal`` never see it (see
|
||||
# model_tools._compute_tool_definitions). The check_fn registered below only
|
||||
# answers "is Browser Use mode configured"; surface policy lives with the
|
||||
# session, not in the process-wide TTL-cached check_fn.
|
||||
|
||||
|
||||
def _description_header() -> str:
|
||||
|
|
@ -348,32 +349,45 @@ def _description_header() -> str:
|
|||
_skill_text_cache: Optional[str] = None
|
||||
_skill_text_fetched = False
|
||||
|
||||
# Pinned quick-reference for the CLI's pre-imported helpers. Replaces the
|
||||
# live ``browser-use skill`` fetch: embedding whatever text the installed CLI
|
||||
# version prints would ship uncontrolled third-party content into every
|
||||
# session's system-side schema (version drift across machines, supply-chain
|
||||
# exposure, and a byte-unstable prompt). A/B benchmarked Aug 2026 (108 runs,
|
||||
# opus-4.8 + kimi-k3, 6 multi-step tasks x 3 reps): header-only schema went
|
||||
# 36/36 vs 36/36 for the full skill dump at ~equal tokens (-60% vs the
|
||||
# legacy browser_* toolset either way). The pinned digest below keeps the
|
||||
# first-call reliability of the helper names without the 7.7KB dump.
|
||||
_HELPERS_DIGEST = (
|
||||
"\n\nHELPERS (pre-imported): new_tab(url) opens/navigates (use for the "
|
||||
"FIRST navigation), goto_url(url) navigates the current tab, "
|
||||
"wait_for_load() after navigation, page_info() summarizes the current "
|
||||
"page state, js(expr) evaluates a JS expression and returns its value "
|
||||
"(js('document.title'); wrap function bodies as js('(() => {...})()') — "
|
||||
"a bare '() => {...}' returns the function itself, uncalled), "
|
||||
"fill_input(selector, text) types into inputs, click_at_xy(x, y) clicks "
|
||||
"viewport coordinates, capture_screenshot() saves and prints a "
|
||||
"screenshot path, cdp('Domain.method', **kwargs) is raw CDP — "
|
||||
"cdp('Accessibility.getFullAXTree')['nodes'] lists every element's "
|
||||
"role/name/backendDOMNodeId (filter in Python before printing; it is "
|
||||
"thousands of nodes), then cdp('DOM.getBoxModel', backendNodeId=n) gives "
|
||||
"click coordinates. ensure_real_tab() recovers from a stale/internal "
|
||||
"tab. Login walls: stop and ask the user; never guess credentials."
|
||||
)
|
||||
|
||||
|
||||
def _cli_skill_text() -> str:
|
||||
"""Return the installed CLI's skill"""
|
||||
global _skill_text_cache, _skill_text_fetched
|
||||
if _skill_text_fetched:
|
||||
return _skill_text_cache or ""
|
||||
_skill_text_fetched = True
|
||||
cmd = _find_cli()
|
||||
if not cmd:
|
||||
return ""
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
[*cmd, "skill"], capture_output=True, text=True, timeout=30
|
||||
)
|
||||
if proc.returncode == 0 and proc.stdout.strip():
|
||||
_skill_text_cache = proc.stdout.strip()
|
||||
except Exception as e:
|
||||
logger.debug("Could not fetch browser-use skill text: %s", e)
|
||||
"""Deprecated: always returns "" — the schema uses the pinned header.
|
||||
|
||||
Kept so tests and any external callers keep importing a stable symbol;
|
||||
see _HELPERS_DIGEST for the rationale (benchmark-backed removal of the
|
||||
live ``browser-use skill`` fetch).
|
||||
"""
|
||||
return _skill_text_cache or ""
|
||||
|
||||
|
||||
def _dynamic_schema_overrides() -> dict:
|
||||
skill = _cli_skill_text()
|
||||
if not skill:
|
||||
return {}
|
||||
return {"description": _description_header() + "\n\n---\n\n" + skill}
|
||||
return {"description": _description_header() + _HELPERS_DIGEST}
|
||||
|
||||
|
||||
BROWSER_EXEC_SCHEMA = {
|
||||
|
|
@ -381,8 +395,8 @@ BROWSER_EXEC_SCHEMA = {
|
|||
# Static fallback, used only when the CLI (and uvx) is unavailable
|
||||
"description": (
|
||||
_HEADER_BASE
|
||||
+ "\n\n(The browser-use CLI is not installed yet, so its full skill "
|
||||
"documentation could not be loaded. Install it with "
|
||||
+ _HELPERS_DIGEST
|
||||
+ "\n\n(The browser-use CLI is not installed yet. Install it with "
|
||||
"`uv tool install browser-use`.)"
|
||||
),
|
||||
"parameters": {
|
||||
|
|
|
|||
|
|
@ -75,6 +75,13 @@ browser:
|
|||
|
||||
Cloud browsers need `browser-use auth login` or `BROWSER_USE_API_KEY`.
|
||||
|
||||
:::note
|
||||
Because Browser Use mode executes model-written Python on your machine, the
|
||||
`browser_exec` tool is only offered to sessions that also have terminal
|
||||
access. Platforms configured without the terminal toolset (e.g. a locked-down
|
||||
messaging surface) keep the default browser tools instead.
|
||||
:::
|
||||
|
||||
### Firecrawl cloud mode
|
||||
|
||||
To use Firecrawl as your cloud browser provider, add:
|
||||
|
|
|
|||
Loading…
Reference in New Issue