fix(vault): make browser_vault_fill work on the default Browser Use backend
On the default backend (browser.backend unset → browser_exec) the vault tools were advertised but could never fill: the CDP supervisor that carries the secret-bearing eval is started only by the built-in browser_* session path, so _eval_js_secret failed closed with supervisor_required and the origin pre-check fell back to an agent-browser CLI eval against a browser browser_exec never touched. - browser_exec now attaches SUPERVISOR_REGISTRY to the CDP endpoint it just routed the harness to (BU_CDP_WS/BU_CDP_URL), so the fill talks to the SAME browser over the same secret-capable WebSocket. BU direct-cloud (BU_AUTOSPAWN) exposes no endpoint and keeps the supervisor_required refusal. - CDPSupervisor.focus_page(origin, accept=<js>) (used by browser_vault_fill, next commits) re-attaches the page session to the open tab on the item's origin whose DOM holds the form being filled (browser_exec opens its own tabs; the supervisor's initial attach picks the first page target, which is chrome://new-tab-page). browser_vault_fill uses it before the origin pre-check with a per-kind probe (password input / card fields / address fields). Live: evals/vault_fill_live_e2e.py drives the real browser_exec tool against Hermes' packaged Chromium with the login page in the third tab; A/B with the attach line disabled fails at "did not attach a supervisor", enabled fills the password into the /login tab and card fields into the /checkout tab with every model-facing read scrubbed. Also: browser_vault_list/fill described the workflow as "type the identifier with fill_input", a helper that exists only inside browser_exec code (toolset browser-use) and is a ghost on the built-in stack. model_tools._rewrite_browser_vault substitutes the concrete name from the session's actual tool set (`fill_input` inside browser_exec, or browser_type), the same dynamic cross-reference pattern browser_navigate uses for web_search.
This commit is contained in:
@@ -408,12 +408,30 @@ def _rewrite_delegate_task(td: Dict[str, Any], available: set) -> Optional[Dict[
|
||||
return {**td, "function": {**fn, "description": desc}}
|
||||
|
||||
|
||||
_VAULT_INPUT_TOOL_HINT = "the browser's input tool"
|
||||
|
||||
|
||||
def _rewrite_browser_vault(td: Dict[str, Any], available: set) -> Optional[Dict[str, Any]]:
|
||||
"""Name the concrete input tool for typing the login identifier: `fill_input` inside browser_exec code, or
|
||||
browser_type on the built-in stack. Resolved here because the two live in different toolsets."""
|
||||
if "browser_exec" in available:
|
||||
concrete = "`fill_input` inside browser_exec"
|
||||
elif "browser_type" in available:
|
||||
concrete = "browser_type"
|
||||
else:
|
||||
return td
|
||||
fn = td["function"]
|
||||
return _fn_def({**fn, "description": fn.get("description", "").replace(_VAULT_INPUT_TOOL_HINT, concrete)})
|
||||
|
||||
|
||||
_DYNAMIC_SCHEMA_REWRITERS = {
|
||||
"execute_code": _rewrite_execute_code,
|
||||
"discord": _discord_rewriter("get_dynamic_schema_core"),
|
||||
"discord_admin": _discord_rewriter("get_dynamic_schema_admin"),
|
||||
"browser_navigate": _rewrite_browser_navigate,
|
||||
"browser_exec": _rewrite_browser_exec,
|
||||
"browser_vault_list": _rewrite_browser_vault,
|
||||
"browser_vault_fill": _rewrite_browser_vault,
|
||||
"delegate_task": _rewrite_delegate_task,
|
||||
}
|
||||
|
||||
|
||||
@@ -48,10 +48,26 @@ def _fake_managed_chromium(monkeypatch):
|
||||
return calls
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _fake_supervisor_registry(monkeypatch):
|
||||
"""browser_exec attaches the vault supervisor to the resolved CDP endpoint; the fake endpoint above is
|
||||
not a real browser, so record the attach instead of opening a WebSocket (15 s start timeout)."""
|
||||
from tools import browser_supervisor
|
||||
|
||||
attached = []
|
||||
|
||||
class _Registry:
|
||||
def get_or_start(self, task_id, cdp_url, **kw):
|
||||
attached.append((task_id, cdp_url))
|
||||
|
||||
monkeypatch.setattr(browser_supervisor, "SUPERVISOR_REGISTRY", _Registry())
|
||||
return attached
|
||||
|
||||
|
||||
def _fake_cli(tmp_path, body):
|
||||
"""Write an executable fake browser-use CLI and return its path."""
|
||||
script = tmp_path / "browser-use"
|
||||
script.write_text("#!/bin/sh\n" + body)
|
||||
script.write_text("#!/bin/sh\n" + body, encoding="utf-8")
|
||||
script.chmod(script.stat().st_mode | stat.S_IXUSR)
|
||||
return str(script)
|
||||
|
||||
@@ -251,6 +267,21 @@ class TestToolSurfaceSwap:
|
||||
assert "browser_exec" in names
|
||||
|
||||
|
||||
class TestVaultSupervisorAttach:
|
||||
def test_exec_attaches_supervisor_to_the_browser_it_drives(self, tmp_path, monkeypatch, _fake_supervisor_registry):
|
||||
"""browser_vault_fill injects secrets only over the supervisor's CDP WebSocket. Without this attach the
|
||||
default (Browser Use) backend had no supervisor at all and every fill failed with supervisor_required."""
|
||||
monkeypatch.setattr("hermes_cli.config.read_raw_config", lambda: {"browser": {"backend": "browser-use"}})
|
||||
cli = _fake_cli(tmp_path, 'cat > /dev/null\necho ok\n')
|
||||
monkeypatch.setattr(bu_cli, "_find_cli", lambda: [cli])
|
||||
monkeypatch.setattr("tools.browser_tool_cdp._resolve_cdp_override", lambda url: url)
|
||||
|
||||
result = json.loads(bu_cli.browser_exec("print(1)", task_id="t-vault"))
|
||||
|
||||
assert result["success"] is True
|
||||
assert _fake_supervisor_registry == [("t-vault", "ws://127.0.0.1:47000/devtools/browser/t-vault")]
|
||||
|
||||
|
||||
class TestFindCli:
|
||||
"""The tests/tools conftest pins _find_cli to None (host isolation);
|
||||
exercise the real function via the preserved _find_cli_unpatched."""
|
||||
@@ -942,7 +973,7 @@ class TestFindCliManagedBin:
|
||||
bin_dir = tmp_path / "home" / "bin"
|
||||
bin_dir.mkdir(parents=True)
|
||||
bu = bin_dir / "browser-use"
|
||||
bu.write_text("#!/bin/sh\n")
|
||||
bu.write_text("#!/bin/sh\n", encoding="utf-8")
|
||||
bu.chmod(bu.stat().st_mode | stat.S_IXUSR)
|
||||
assert bu_cli._find_cli_unpatched() == [str(bu)]
|
||||
|
||||
@@ -950,7 +981,7 @@ class TestFindCliManagedBin:
|
||||
bin_dir = tmp_path / "home" / "bin"
|
||||
bin_dir.mkdir(parents=True)
|
||||
uvx = bin_dir / "uvx"
|
||||
uvx.write_text("#!/bin/sh\n")
|
||||
uvx.write_text("#!/bin/sh\n", encoding="utf-8")
|
||||
uvx.chmod(uvx.stat().st_mode | stat.S_IXUSR)
|
||||
assert bu_cli._find_cli_unpatched() == [str(uvx), "browser-use"]
|
||||
|
||||
@@ -964,7 +995,7 @@ class TestFindCliManagedBin:
|
||||
cli_dir = tmp_path / "userhome" / ".local" / "bin"
|
||||
cli_dir.mkdir(parents=True)
|
||||
cli = cli_dir / "browser-use"
|
||||
cli.write_text("#!/bin/sh\n")
|
||||
cli.write_text("#!/bin/sh\n", encoding="utf-8")
|
||||
cli.chmod(cli.stat().st_mode | stat.S_IXUSR)
|
||||
assert bu_cli._find_cli_unpatched() == [str(cli)]
|
||||
|
||||
@@ -976,12 +1007,12 @@ class TestFindCliManagedBin:
|
||||
user_dir = tmp_path / "userhome" / ".local" / "bin"
|
||||
user_dir.mkdir(parents=True)
|
||||
user_cli = user_dir / "browser-use"
|
||||
user_cli.write_text("#!/bin/sh\n")
|
||||
user_cli.write_text("#!/bin/sh\n", encoding="utf-8")
|
||||
user_cli.chmod(user_cli.stat().st_mode | stat.S_IXUSR)
|
||||
managed_dir = tmp_path / "home" / "bin"
|
||||
managed_dir.mkdir(parents=True)
|
||||
managed_cli = managed_dir / "browser-use"
|
||||
managed_cli.write_text("#!/bin/sh\n")
|
||||
managed_cli.write_text("#!/bin/sh\n", encoding="utf-8")
|
||||
managed_cli.chmod(managed_cli.stat().st_mode | stat.S_IXUSR)
|
||||
assert bu_cli._find_cli_unpatched() == [str(managed_cli)]
|
||||
|
||||
@@ -990,13 +1021,13 @@ class TestFindCliManagedBin:
|
||||
path_dir = tmp_path / "onpath"
|
||||
path_dir.mkdir()
|
||||
path_cli = path_dir / "browser-use"
|
||||
path_cli.write_text("#!/bin/sh\n")
|
||||
path_cli.write_text("#!/bin/sh\n", encoding="utf-8")
|
||||
path_cli.chmod(path_cli.stat().st_mode | stat.S_IXUSR)
|
||||
monkeypatch.setenv("PATH", str(path_dir))
|
||||
managed_dir = tmp_path / "home" / "bin"
|
||||
managed_dir.mkdir(parents=True)
|
||||
managed_cli = managed_dir / "browser-use"
|
||||
managed_cli.write_text("#!/bin/sh\n")
|
||||
managed_cli.write_text("#!/bin/sh\n", encoding="utf-8")
|
||||
managed_cli.chmod(managed_cli.stat().st_mode | stat.S_IXUSR)
|
||||
assert bu_cli._find_cli_unpatched() == [str(managed_cli)]
|
||||
|
||||
@@ -1004,7 +1035,7 @@ class TestFindCliManagedBin:
|
||||
cli_dir = tmp_path / "userhome" / ".local" / "bin"
|
||||
cli_dir.mkdir(parents=True)
|
||||
uvx = cli_dir / "uvx"
|
||||
uvx.write_text("#!/bin/sh\n")
|
||||
uvx.write_text("#!/bin/sh\n", encoding="utf-8")
|
||||
uvx.chmod(uvx.stat().st_mode | stat.S_IXUSR)
|
||||
assert bu_cli._find_cli_unpatched() == [str(uvx), "browser-use"]
|
||||
|
||||
@@ -1032,7 +1063,7 @@ class TestInstallCli:
|
||||
bin_dir = tmp_path / "home" / "bin"
|
||||
bin_dir.mkdir(parents=True)
|
||||
cli = bin_dir / "browser-use"
|
||||
cli.write_text("#!/bin/sh\n")
|
||||
cli.write_text("#!/bin/sh\n", encoding="utf-8")
|
||||
cli.chmod(cli.stat().st_mode | stat.S_IXUSR)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home"))
|
||||
monkeypatch.setenv("PATH", str(tmp_path / "empty"))
|
||||
@@ -1069,7 +1100,7 @@ class TestInstallCli:
|
||||
'target="$UV_TOOL_BIN_DIR/browser-use"\n'
|
||||
'echo "#!/bin/sh" > "$target"\n'
|
||||
'/bin/chmod +x "$target"\n'
|
||||
)
|
||||
, encoding="utf-8")
|
||||
uv.chmod(uv.stat().st_mode | stat.S_IXUSR)
|
||||
import sys as _sys
|
||||
import types as _types
|
||||
@@ -1085,7 +1116,7 @@ class TestInstallCli:
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
monkeypatch.setenv("PATH", str(tmp_path / "empty"))
|
||||
uv = tmp_path / "uv"
|
||||
uv.write_text('#!/bin/sh\necho "no network" >&2\nexit 1\n')
|
||||
uv.write_text('#!/bin/sh\necho "no network" >&2\nexit 1\n', encoding="utf-8")
|
||||
uv.chmod(uv.stat().st_mode | stat.S_IXUSR)
|
||||
import sys as _sys
|
||||
import types as _types
|
||||
|
||||
@@ -257,6 +257,53 @@ class CDPSupervisor(DialogSupervisionMixin, FrameTrackingMixin):
|
||||
value = result_obj.get("description") or result_obj.get("unserializableValue")
|
||||
return {"ok": True, "result": value, "result_type": result_type}
|
||||
|
||||
def focus_page(self, origin: str, *, accept: Optional[str] = None, timeout: float = 10.0) -> Dict[str, Any]:
|
||||
"""Re-attach the supervisor's page session to an open page target on ``origin``
|
||||
(``scheme://host[:port]``). The initial attach picks the FIRST page target, but tools
|
||||
that open their own tabs (browser_exec) put the login form somewhere else. With
|
||||
``accept`` (a JS expression) the first same-origin tab where it evaluates truthy wins,
|
||||
so a login and a checkout tab on one site resolve to the right one. Returns
|
||||
``{"ok": True, "url"}`` or ``{"ok": False, "error"}``; on failure the previous session stays."""
|
||||
loop = self._loop
|
||||
if loop is None or not loop.is_running():
|
||||
return _fail("supervisor loop is not running")
|
||||
|
||||
async def _attach(target_id: str) -> str:
|
||||
attach = await self._cdp("Target.attachToTarget", {"targetId": target_id, "flatten": True}, timeout=timeout)
|
||||
sid = attach["result"]["sessionId"]
|
||||
await self._enable_page_domains(sid, timeout=timeout)
|
||||
await self._install_dialog_bridge(sid)
|
||||
return sid
|
||||
|
||||
async def _focus() -> Dict[str, Any]:
|
||||
from agent.vault_store import normalize_origin
|
||||
targets = (await self._cdp("Target.getTargets", timeout=timeout)).get("result", {}).get("targetInfos", [])
|
||||
candidates = []
|
||||
for t in targets:
|
||||
url = str(t.get("url") or "")
|
||||
try:
|
||||
if t.get("type") == "page" and normalize_origin(url) == origin:
|
||||
candidates.append((t["targetId"], url))
|
||||
except Exception:
|
||||
continue
|
||||
for target_id, url in candidates:
|
||||
sid = await _attach(target_id)
|
||||
if accept:
|
||||
probe = await self._cdp("Runtime.evaluate", {"expression": accept, "returnByValue": True},
|
||||
session_id=sid, timeout=timeout)
|
||||
if not probe.get("result", {}).get("result", {}).get("value"):
|
||||
await self._cdp("Target.detachFromTarget", {"sessionId": sid}, timeout=timeout)
|
||||
continue
|
||||
with self._state_lock:
|
||||
self._page_session_id = sid
|
||||
return {"ok": True, "url": url}
|
||||
return _fail(f"no open page on {origin}" + (" with the expected form" if accept and candidates else ""))
|
||||
|
||||
try:
|
||||
return _schedule(_focus(), loop, timeout=timeout + 1)
|
||||
except Exception as exc:
|
||||
return _err(exc)
|
||||
|
||||
# ── Supervisor loop internals ────────────────────────────────────────────
|
||||
|
||||
def _thread_main(self) -> None:
|
||||
|
||||
@@ -485,6 +485,23 @@ def _resolve_real_profile_cdp(env: dict, force_local: bool) -> Optional[str]:
|
||||
return err or None
|
||||
|
||||
|
||||
def _attach_vault_supervisor(env: dict, task_id: Optional[str]) -> None:
|
||||
"""Attach the per-task CDP supervisor to the browser this exec drives so ``browser_vault_fill`` has
|
||||
a secret-capable WebSocket (never argv) into the SAME browser. Only CDP-routed backends expose an
|
||||
endpoint; BU direct-cloud (BU_AUTOSPAWN) does not, and the vault tools report ``supervisor_required``."""
|
||||
cdp = env.get("BU_CDP_WS") or env.get("BU_CDP_URL")
|
||||
if not cdp:
|
||||
return
|
||||
try:
|
||||
from tools.browser_supervisor import SUPERVISOR_REGISTRY
|
||||
from tools.browser_tool_cdp import _get_dialog_policy_config, _resolve_cdp_override
|
||||
policy, timeout_s = _get_dialog_policy_config()
|
||||
SUPERVISOR_REGISTRY.get_or_start(task_id=task_id or "default", cdp_url=_resolve_cdp_override(cdp),
|
||||
dialog_policy=policy, dialog_timeout_s=timeout_s)
|
||||
except Exception as exc:
|
||||
logger.debug("browser_exec: CDP supervisor attach failed (non-fatal): %s", exc)
|
||||
|
||||
|
||||
def _route_backend(env: dict, session: str, task_id: Optional[str], local: bool) -> Optional[str]:
|
||||
"""Resolve where the harness connects; returns an error string or None. Real-profile consent runs
|
||||
BEFORE provider resolution so a hit short-circuits the cloud path via the BU_CDP_* env contract. Named
|
||||
@@ -587,6 +604,7 @@ def browser_exec(code: str, session: str = "", timeout_s: int = _DEFAULT_TIMEOUT
|
||||
route_err = _route_backend(env, session, task_id, bool(local))
|
||||
if route_err:
|
||||
return tool_error(route_err)
|
||||
_attach_vault_supervisor(env, task_id)
|
||||
|
||||
# SHARED browser (/browser connect CDP override): pin each named session to its own tab (see
|
||||
# _OWN_TAB_PREAMBLE). Private per-name browsers skip this — nothing to collide with.
|
||||
|
||||
Reference in New Issue
Block a user