From 7641f28d48875474cad4b8de801fd8d2e4a9db6e Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sat, 15 Aug 2026 04:14:16 -0700 Subject: [PATCH] fix(browser): pin named sessions to their own tab on shared browsers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #86916. That fix gave named sessions their own daemon (socket/log/pid) and their own provider browser — but on a SHARED local Chrome / CDP browser, a fresh named daemon still attaches to the first existing page, the same page a sibling daemon may hold. A named session that never calls new_tab() could still stomp another's tab. browser_exec now prepends a small preamble to the model's code for named sessions on shared browsers: once per daemon process (marker keyed by uid + BU_NAME + daemon pid), it creates a fresh tab via Target.createTarget and switch_tab()s onto it before any model code runs. Private per-name browsers (provider-keyed bu-named-, or direct-API Browser Use cloud) skip the preamble via an internal env sentinel popped before launch — there's nobody to collide with, and the extra tab would leak. Best-effort by design: if the preamble's CDP calls fail, behavior degrades to pre-fix, never blocks the exec. E2E against a shared headless Chrome with the STOCK harness: two named sessions issuing bare js() writes (no new_tab) kept distinct state (EDGE-A/EDGE-B read back intact); the sabotage run without the preamble reproduced the clobber (both read EDGE-B). Removes the dependency on the upstream browser-harness tab-isolation PR for correctness. --- tests/tools/test_browser_use_cli.py | 61 +++++++++++++++++++++++++++ tools/browser_use_cli.py | 65 +++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/tests/tools/test_browser_use_cli.py b/tests/tools/test_browser_use_cli.py index 08aefe3ec9a88..aa1a3c44fb8a6 100644 --- a/tests/tools/test_browser_use_cli.py +++ b/tests/tools/test_browser_use_cli.py @@ -459,6 +459,67 @@ class _BUProvider: assert "BU_CDP_WS" not in env and "BU_CDP_URL" not in env +class TestOwnTabPreamble: + """Named sessions on SHARED browsers get the own-tab preamble prepended; + private per-name browsers and unnamed sessions do not.""" + + def _run(self, tmp_path, monkeypatch, *, session="", private=False, provider=False): + import tools.browser_tool as bt + + monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") + if provider: + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr( + bt, "_get_session_info", + lambda key: {"cdp_url": "wss://browser.example/cdp/" + key}, + ) + else: + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) + # fake CLI echoes stdin back so we can inspect what code was sent + cli = _fake_cli(tmp_path, "cat\n") + monkeypatch.setattr(bu_cli, "_find_cli", lambda: [cli]) + return json.loads(bu_cli.browser_exec("print('payload')", session=session)) + + def test_named_shared_browser_gets_preamble(self, tmp_path, monkeypatch): + result = self._run(tmp_path, monkeypatch, session="r7k2") + assert result["success"] is True + assert "_hermes_ensure_own_tab" in result["output"] + # model code still present, after the preamble + assert result["output"].index("_hermes_ensure_own_tab") < result["output"].index("print('payload')") + + def test_unnamed_session_gets_no_preamble(self, tmp_path, monkeypatch): + result = self._run(tmp_path, monkeypatch, session="") + assert result["success"] is True + assert "_hermes_ensure_own_tab" not in result["output"] + + def test_named_provider_browser_skips_preamble(self, tmp_path, monkeypatch): + """Per-name provider browsers are private — preamble would leak a tab.""" + result = self._run(tmp_path, monkeypatch, session="r7k2", provider=True) + assert result["success"] is True + assert "_hermes_ensure_own_tab" not in result["output"] + + def test_sentinel_never_reaches_subprocess_env(self, tmp_path, monkeypatch): + import tools.browser_tool as bt + + monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr( + bt, "_get_session_info", + lambda key: {"cdp_url": "wss://browser.example/cdp/" + key}, + ) + cli = _fake_cli(tmp_path, 'cat > /dev/null\necho "sentinel:${_HERMES_BU_PRIVATE_BROWSER:-unset}"\n') + monkeypatch.setattr(bu_cli, "_find_cli", lambda: [cli]) + result = json.loads(bu_cli.browser_exec("print(1)", session="r7k2")) + assert "sentinel:unset" in result["output"] + + def test_preamble_is_valid_python(self): + import ast + + ast.parse(bu_cli._OWN_TAB_PREAMBLE) + # and composes with model code + ast.parse(bu_cli._OWN_TAB_PREAMBLE + "print('x')") + + class TestProviderPickerIntegration: """The `hermes tools` Browser Automation picker row (browser_backend marker) must enter/leave CLI mode cleanly and highlight correctly.""" diff --git a/tools/browser_use_cli.py b/tools/browser_use_cli.py index 97c6931ab47f0..0c99f42c46772 100644 --- a/tools/browser_use_cli.py +++ b/tools/browser_use_cli.py @@ -24,6 +24,54 @@ # Cloud daemon names become the BU_NAME env var _SESSION_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$") +# Internal marker set by _resolve_backend_cdp on the env dict when the +# resolved browser is EXCLUSIVE to this named session (per-name provider +# browser, or a named Browser Use cloud browser). Popped before the +# subprocess launches — never exported to the CLI. +_PRIVATE_BROWSER_SENTINEL = "_HERMES_BU_PRIVATE_BROWSER" + +# Preamble prepended to the model's code for named sessions on SHARED +# browsers (local Chrome / CDP override). The harness daemon attaches to the +# first existing page at startup, so two fresh named daemons can land on the +# SAME tab; steering this daemon onto a tab it created keeps concurrent named +# sessions from clobbering each other before their first new_tab(). Runs +# once per daemon (marker file keyed by BU_NAME under the harness runtime +# state), costs one IPC round-trip on later calls. +_OWN_TAB_PREAMBLE = """\ +# hermes: pin this named session to its own tab (once per daemon process) +def _hermes_ensure_own_tab(): + import os as _os, tempfile as _tf + _name = _os.environ.get("BU_NAME", "default") + try: + # Key the marker by the daemon's pid so a daemon restart (which + # re-attaches to the first shared page) re-pins automatically, + # while agent-driven tab switches mid-session are left alone. + from browser_harness import _ipc as _bipc + _dpid = _bipc.pid_path(_name).read_text().strip() or "0" + except Exception: + _dpid = "0" + _uid = _os.getuid() if hasattr(_os, "getuid") else 0 + _marker = _os.path.join( + _tf.gettempdir(), "hermes-bu-owntab-%s-%s-%s" % (_uid, _name, _dpid) + ) + if _os.path.exists(_marker): + return + try: + # Force a fresh target: new_tab() would REUSE a blank current tab, + # which is exactly the tab a sibling daemon may also hold. + _tid = cdp("Target.createTarget", url="about:blank").get("targetId") + if _tid: + switch_tab(_tid) + except Exception: + pass # best-effort: worst case is pre-fix behavior + try: + open(_marker, "w").close() + except OSError: + pass +_hermes_ensure_own_tab() +del _hermes_ensure_own_tab +""" + _DEFAULT_TIMEOUT_S = 300 _MIN_TIMEOUT_S = 5 _MAX_TIMEOUT_S = 1800 @@ -465,6 +513,9 @@ def _resolve_backend_cdp( if provider_key == _BACKEND_KEY and not is_truthy_value( _read_browser_cfg().get("use_gateway"), default=False ): + # Named BU cloud browsers are exclusive to their daemon — no shared + # tab to isolate from. + env[_PRIVATE_BROWSER_SENTINEL] = "1" return None try: @@ -487,6 +538,11 @@ def _resolve_backend_cdp( "the built-in browser tools for this provider." ) env["BU_CDP_URL" if cdp.startswith(("http://", "https://")) else "BU_CDP_WS"] = cdp + # A provider browser keyed bu-named- is exclusive to this session — + # the own-tab preamble is unnecessary there (it would just leak a blank + # tab into a browser nobody else touches). + if session_name: + env[_PRIVATE_BROWSER_SENTINEL] = "1" return None @@ -535,6 +591,15 @@ def browser_exec( if backend_err: return tool_error(backend_err) + # On a SHARED browser (local Chrome / CDP override) a fresh named daemon + # attaches to the first existing page — the same page a sibling daemon + # may hold. Pin each named session to a tab it created before running + # the model's code. Private per-name browsers (provider-keyed or BU + # cloud) skip this: no one to collide with, and the extra tab would leak. + private_browser = env.pop(_PRIVATE_BROWSER_SENTINEL, None) + if session and not private_browser: + code = _OWN_TAB_PREAMBLE + code + workspace = _workspace_dir(task_id) if workspace: env["BH_AGENT_WORKSPACE"] = workspace