From dd2043723430e3f49799ad569bfbcf09a0493a17 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 4 Nov 2026 09:39:28 -0800 Subject: [PATCH 1/3] website browser: the relay's guard on every CDP command An agent's Browser.close, Browser.crash or Browser.crashGpuProcess is answered {} and ends only its own connection, with 1000, so the browser stays up for the env and every other connection. Target.exposeDevToolsProtocol, a browser context with its own proxy, and a navigation or new tab to the browser's own pages (file:, chrome:, chrome-extension:, chrome-untrusted:, view-source:, devtools: and about: but blank and srcdoc, read as the browser reads a URL) get a CDP error saying agentenv refuses them. Download behavior is sent on as agentenv's own, the browser's default into ~/Downloads, keeping only the context and whether to send events, so Playwright's connect still works. When the deploy answers dialogs, an agent's answer is answered {} and not sent on. Each connection's record entry lists the first 100 commands the guard refused, changed, answered or closed it on. Co-Authored-By: Claude Opus 5.5 --- packages/agentenv-protocol/README.md | 15 ++- .../src/agentenv_protocol/browser_cdp.py | 125 +++++++++++++++--- .../src/agentenv_protocol/website_browser.py | 6 +- .../tests/test_browser_cdp.py | 73 ++++++++-- .../computer/browser_computer_test.py | 9 +- .../computer/cdp_clients/connect.mjs | 13 +- 6 files changed, 206 insertions(+), 35 deletions(-) diff --git a/packages/agentenv-protocol/README.md b/packages/agentenv-protocol/README.md index 77825acd3..30bd71245 100644 --- a/packages/agentenv-protocol/README.md +++ b/packages/agentenv-protocol/README.md @@ -880,7 +880,20 @@ are the profile's template. The base class adds: and the events of its sessions and those it attaches, and no one else's. Only flattened sessions are served, as Playwright, Puppeteer and Browser Use use. A token agentenv didn't mint is refused, and when the browser goes, each connection is closed with 1012 so its client can connect again. - `record` lists each connection as `cdp_connections`: its user, when it opened and when it closed. + A guard sits on every command, a guardrail, since the sandbox's network is the boundary: + - `Browser.close`, `Browser.crash` and `Browser.crashGpuProcess` are answered `{}` and end only that + connection, with 1000; + - `Target.exposeDevToolsProtocol`, a `Target.createBrowserContext` with its own proxy, and a + `Page.navigate` or `Target.createTarget` to the browser's own pages (`file:`, `chrome:`, + `chrome-extension:`, `chrome-untrusted:`, `view-source:`, `devtools:`, and `about:` but + `about:blank` and `about:srcdoc`) get a CDP error, `agentenv refuses …`; + - `Browser.setDownloadBehavior` and `Page.setDownloadBehavior` are sent on as agentenv's own, the + browser's default into `~/Downloads`, keeping only the context and whether to send events; + - when the deploy answers dialogs, an agent's `Page.handleJavaScriptDialog` is answered `{}` and not + sent on. + + `record` lists each connection as `cdp_connections`: its user, when it opened and when it closed, + and the first 100 commands the guard refused, changed, answered or closed it on. Puppeteer connects by `ws_url`: its `browserURL` drops the address's path. - **Its skill**: sites by their real names, files to upload in `~/Uploads` and downloads in `~/Downloads`, folders it makes. diff --git a/packages/agentenv-protocol/src/agentenv_protocol/browser_cdp.py b/packages/agentenv-protocol/src/agentenv_protocol/browser_cdp.py index 9d74dfdf2..5e9291e18 100644 --- a/packages/agentenv-protocol/src/agentenv_protocol/browser_cdp.py +++ b/packages/agentenv-protocol/src/agentenv_protocol/browser_cdp.py @@ -1,6 +1,6 @@ """The CDP addresses a website browser hands agents: a token agentenv mints for each user, of which only a hash is kept where the computer's session can read it, discovery answered from the browser, and a relay that gives each WebSocket -connection its own browser-level session over the browser's one debugging pipe.""" +connection its own browser-level session over the browser's one debugging pipe, with a guard on every command.""" from __future__ import annotations import asyncio @@ -13,7 +13,7 @@ from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING, Any, Literal, Optional from starlette.requests import Request from starlette.websockets import WebSocket, WebSocketDisconnect @@ -34,8 +34,24 @@ #: How much a connection's client may have waiting to be sent to it before it is closed; a single answer bigger than #: this still goes when nothing else waits. OUTBOX_LIMIT_BYTES = 64 << 20 +#: The close code and reason a connection gets after it closes or crashes the browser, which ends only it. +CLOSED = (1000, "the browser stays up for the env: only this connection ended") #: CDP's own answer to a command for a session the connection doesn't have. NO_SESSION = {"code": -32001, "message": "Session with given id not found."} +#: The code of a CDP error for a command not carried out, as Chromium answers its own refusals. +SERVER_ERROR = -32000 +#: The commands that would end the browser every connection shares: each ends only the connection that sends it. +ENDS_BROWSER = frozenset({"Browser.close", "Browser.crash", "Browser.crashGpuProcess"}) +#: The commands that set how and where downloads are saved, which agentenv keeps as its own. +SETS_DOWNLOADS = frozenset({"Browser.setDownloadBehavior", "Page.setDownloadBehavior"}) +#: The schemes of the browser's own files, settings and tools, which an agent's navigation or new tab may not open. +OWN_SCHEMES = frozenset({"file", "chrome", "chrome-extension", "chrome-untrusted", "view-source", "devtools"}) +#: The ``about:`` pages that aren't the browser's own under another name. +ABOUT_PAGES = frozenset({"about:blank", "about:srcdoc"}) +#: How many of a connection's guarded commands its record keeps. +GUARDED_KEPT = 100 +#: What the record says the guard did, by its action. +GUARDED = {"refuse": "refused", "send": "changed", "answer": "answered", "close": "closed"} @dataclass @@ -105,12 +121,59 @@ def _save(self) -> None: os.replace(part, self.path) +@dataclass +class Verdict: + """What the guard does with an agent's command: ``send`` it on with ``params``, ``changed`` or not; ``answer`` it + with ``{}`` itself; answer it and ``close`` the connection; or ``refuse`` it for ``reason``.""" + + action: Literal["send", "answer", "close", "refuse"] + params: dict + reason: str = "" + changed: bool = False + + +def guard(method: str, params: dict, *, dialogs_answered: bool) -> Verdict: + """What the relay does with an agent's ``method``, so it can't end the browser others share, route around the site + proxy, save downloads elsewhere, open the browser's own pages or race the dialogs agentenv answers when + ``dialogs_answered``: a guardrail, since the sandbox's network is the boundary.""" + if method in ENDS_BROWSER: + return Verdict("close", params) + if method == "Target.exposeDevToolsProtocol": + return Verdict("refuse", params, "it would give a page's script the browser's own CDP channel") + if method == "Target.createBrowserContext" and {"proxyServer", "proxyBypassList"} & params.keys(): + return Verdict("refuse", params, "a context's traffic goes through the env's site proxy") + if method in ("Page.navigate", "Target.createTarget") and (scheme := _own_scheme(params.get("url"))): + return Verdict("refuse", params, f"{scheme}: URLs open the browser's own files, settings and tools") + if method in SETS_DOWNLOADS: + pinned = {"behavior": "default", + **{key: params[key] for key in ("browserContextId", "eventsEnabled") if key in params}} + return Verdict("send", pinned, changed=pinned != params) + if method == "Page.handleJavaScriptDialog" and dialogs_answered: + return Verdict("answer", params) + return Verdict("send", params) + + +def _own_scheme(url: Any) -> Optional[str]: + """``url``'s scheme when it's one of the browser's own, read as the browser reads it, without tabs and newlines and + with spaces and controls trimmed from its ends; an ``about:`` page other than a blank one counts.""" + if not isinstance(url, str): + return None + url = url.translate({ord(c): None for c in "\t\n\r"}).strip("".join(map(chr, range(0x21)))) + scheme, colon, _ = url.partition(":") + scheme = scheme.lower() + if not colon: + return None + if scheme == "about": + return None if url.lower().split("#")[0].split("?")[0] in ABOUT_PAGES else scheme + return scheme if scheme in OWN_SCHEMES else None + + class _Connection: """One client's WebSocket, its sessions over the pipe (its browser-level one and those it attached), its unanswered - commands, and what waits to be sent to it.""" + commands, what waits to be sent to it, and its entry in the record.""" - def __init__(self, websocket: WebSocket, user: str, session: str) -> None: - self.websocket, self.user, self.session = websocket, user, session + def __init__(self, websocket: WebSocket, user: str, session: str, record: dict) -> None: + self.websocket, self.user, self.session, self.record = websocket, user, session, record self.sessions = {session} self.commands: set[asyncio.Future] = set() self.outbox: asyncio.Queue[str | tuple[int, str]] = asyncio.Queue() @@ -119,12 +182,13 @@ def __init__(self, websocket: WebSocket, user: str, session: str) -> None: class Relay: - """Each WebSocket connection given its own browser-level session over ``devtools``, its commands sent on it and the - answers and events of its sessions sent back, so several clients share one browser; and a record of who connected - when.""" + """Each WebSocket connection given its own browser-level session over ``devtools``, its commands sent on it past the + guard and the answers and events of its sessions sent back, so several clients share one browser; and a record of + who connected when, and what the guard did to their commands. ``dialogs_answered`` says agentenv answers the + browser's dialogs itself.""" - def __init__(self, devtools: DevToolsPipe) -> None: - self.devtools = devtools + def __init__(self, devtools: DevToolsPipe, *, dialogs_answered: bool = False) -> None: + self.devtools, self.dialogs_answered = devtools, dialogs_answered self.connections: list[dict] = [] self._by_session: dict[str, _Connection] = {} self._open: set[_Connection] = set() @@ -137,10 +201,10 @@ async def serve(self, websocket: WebSocket, user: str) -> None: except Exception as e: await websocket.close(code=1013, reason=f"the browser isn't answering: {type(e).__name__}") return - connection = _Connection(websocket, user, session) + record = {"user": user, "opened_at": _now(), "guarded": []} + connection = _Connection(websocket, user, session, record) self._by_session[session] = connection self._open.add(connection) - record = {"user": user, "opened_at": _now()} self.connections.append(record) await websocket.accept() sending = asyncio.ensure_future(self._send(connection)) @@ -171,19 +235,36 @@ def ended(self) -> None: self._close(connection, RELAUNCHED) async def _command(self, connection: _Connection, message: dict) -> None: + if connection.closing: + return + method, params = message.get("method", ""), message.get("params") or {} session = message.get("sessionId") or connection.session + verdict = guard(method, params, dialogs_answered=self.dialogs_answered) if session not in connection.sessions: - answer = {"error": NO_SESSION} + verdict, answer = None, {"error": NO_SESSION} + elif verdict.action == "refuse": + answer = {"error": {"code": SERVER_ERROR, "message": f"agentenv refuses {method}: {verdict.reason}"}} + elif verdict.action != "send": + answer = {"result": {}} else: try: - answer = await self.devtools.request(message.get("method", ""), message.get("params") or {}, - session_id=session) + answer = await self.devtools.request(method, verdict.params, session_id=session) except ConnectionError as e: - answer = {"error": {"code": -32000, "message": str(e)}} + answer = {"error": {"code": SERVER_ERROR, "message": str(e)}} + if verdict is not None and (verdict.action != "send" or verdict.changed): + self._guarded(connection, method, verdict) reply = {"id": message.get("id"), **{key: answer[key] for key in ("result", "error") if key in answer}} if message.get("sessionId"): reply["sessionId"] = message["sessionId"] self._put(connection, reply) + if verdict is not None and verdict.action == "close": + self._close(connection, CLOSED, dropping=False) + + @staticmethod + def _guarded(connection: _Connection, method: str, verdict: Verdict) -> None: + """What the guard did to ``connection``'s ``method``, in its record, up to ``GUARDED_KEPT`` of them.""" + if len(guarded := connection.record["guarded"]) < GUARDED_KEPT: + guarded.append({"method": method, "did": GUARDED[verdict.action], "at": _now()}) def _event(self, message: dict) -> None: connection = self._by_session.get(message.get("sessionId")) @@ -212,13 +293,15 @@ def _put(self, connection: _Connection, message: dict) -> None: connection.waiting_bytes += len(text) connection.outbox.put_nowait(text) - def _close(self, connection: _Connection, close: tuple[int, str]) -> None: - """``connection`` closed with ``close`` once what's being sent to it has gone, and nothing else sent.""" + def _close(self, connection: _Connection, close: tuple[int, str], *, dropping: bool = True) -> None: + """``connection`` closed with ``close`` after what waits to be sent to it, or with that dropped when + ``dropping``; nothing is sent after it.""" connection.closing = True self._end(connection) - while not connection.outbox.empty(): - connection.outbox.get_nowait() - connection.waiting_bytes = 0 + if dropping: + while not connection.outbox.empty(): + connection.outbox.get_nowait() + connection.waiting_bytes = 0 connection.outbox.put_nowait(close) def _end(self, connection: _Connection) -> None: diff --git a/packages/agentenv-protocol/src/agentenv_protocol/website_browser.py b/packages/agentenv-protocol/src/agentenv_protocol/website_browser.py index 23168ba53..8b549e628 100644 --- a/packages/agentenv-protocol/src/agentenv_protocol/website_browser.py +++ b/packages/agentenv-protocol/src/agentenv_protocol/website_browser.py @@ -693,7 +693,7 @@ def __init__(self) -> None: else None) #: The CDP addresses it has handed out, and the relay agents' connections to them go through. self.cdp_tokens = Tokens(SOCKET.parent / "cdp-tokens.json") - self.relay = Relay(self.devtools) + self.relay = Relay(self.devtools, dialogs_answered=self.settings.dialogs != AGENT) self.devtools.on_close(self._pipe_closed) @classmethod @@ -837,8 +837,8 @@ async def devtools_answers(self) -> bool: "its tabs by their ids; dialog/answer answers the dialog open on the active tab; files/choose gives the file " "chooser open on it files in ~/Uploads, headless; and page/state is the active tab's id, URL and title and " "what is open on it. cdp returns a user's CDP address, http_url to discover from and ws_url to connect to, " - "with a token agentenv mints in its path: each connection to it gets its own browser-level session, and " - "record lists them as cdp_connections.")) + "with a token agentenv mints in its path: each connection to it gets its own browser-level session behind a " + "guard, and record lists them as cdp_connections, each with the commands the guard refused or changed.")) async def browser_record(self) -> dict: return {**await self.control.record(), "launches": self._computer_app.launches(), "cdp_connections": list(self.relay.connections)} diff --git a/packages/agentenv-protocol/tests/test_browser_cdp.py b/packages/agentenv-protocol/tests/test_browser_cdp.py index 6508ca35a..90c427191 100644 --- a/packages/agentenv-protocol/tests/test_browser_cdp.py +++ b/packages/agentenv-protocol/tests/test_browser_cdp.py @@ -1,7 +1,7 @@ """A website browser's CDP addresses: a user's token minted once, kept by hash only, so it still works after the server restarts and the computer's session can't use the file; the relay giving each WebSocket connection its own -browser-level session over one pipe, its answers and events and nobody else's, until the browser goes; and the cdp -operation and discovery answered at the address the call reached.""" +browser-level session over one pipe, its answers and events and nobody else's, until the browser goes; the guard on +each command; and the cdp operation and discovery answered at the address the call reached.""" from __future__ import annotations import asyncio @@ -18,10 +18,10 @@ from starlette.applications import Starlette # noqa: E402 from starlette.routing import WebSocketRoute # noqa: E402 from websockets.asyncio.client import connect # noqa: E402 -from websockets.exceptions import ConnectionClosedError # noqa: E402 +from websockets.exceptions import ConnectionClosedError, ConnectionClosedOK # noqa: E402 from agentenv_protocol import browser_cdp # noqa: E402 -from agentenv_protocol.browser_cdp import NO_SESSION, Relay, Tokens # noqa: E402 +from agentenv_protocol.browser_cdp import NO_SESSION, Relay, Tokens, guard # noqa: E402 from agentenv_protocol.computer import Launch, Platform, Ready, app, computer_app, linux # noqa: E402 from agentenv_protocol.computer.app import APP_SETTINGS_ENV # noqa: E402 from agentenv_protocol.computer.app_runtime import AppReadiness # noqa: E402 @@ -41,10 +41,10 @@ class Browser(WebsiteBrowserEnvironment): class Pipe: """A browser's pipe: each connection's browser-level session ``b``, an attach on a session making its child ``-c`` and saying so on that session, ``Boom`` failing, ``Hang`` never answered, anything else echoed; - every request recorded, and each ``Hang`` given up.""" + every request recorded with the last params of each method, and each ``Hang`` given up.""" def __init__(self) -> None: - self.listeners, self.sent, self.sessions, self.given_up = [], [], 0, [] + self.listeners, self.sent, self.sessions, self.given_up, self.params = [], [], 0, [], {} def listen(self, listener) -> None: self.listeners.append(listener) @@ -62,6 +62,7 @@ async def call(self, method, params=None, *, session_id=None, timeout_s=None): async def request(self, method, params=None, *, session_id=None): self.sent.append((method, session_id)) + self.params[method] = params if method == "Target.attachToTarget": child = f"{session_id}-c" self.emit({"method": "Target.attachedToTarget", "sessionId": session_id, @@ -102,11 +103,11 @@ async def close(self, code: int = 1000, reason: str = "") -> None: @contextlib.asynccontextmanager -async def _relay(): +async def _relay(dialogs_answered: bool = False): """The relay over a fake pipe, served on loopback with each connection's user its path's: the pipe, the relay and the server's ws:// base.""" pipe = Pipe() - relay = Relay(pipe) + relay = Relay(pipe, dialogs_answered=dialogs_answered) async def endpoint(websocket): await relay.serve(websocket, websocket.path_params["user"]) @@ -184,6 +185,62 @@ async def test_each_connection_gets_its_own_browser_session_its_own_answers_and_ assert pipe.given_up == ["Hang"] +@pytest.mark.parametrize(("method", "params", "action"), [ + ("Page.navigate", {"url": "https://site.test/"}, "send"), + ("Page.navigate", {"url": "about:blank"}, "send"), + ("Page.navigate", {"url": "about:settings"}, "refuse"), + ("Page.navigate", {"url": " \tFI\nLE:///etc/passwd"}, "refuse"), + ("Target.createTarget", {"url": "chrome://settings"}, "refuse"), + ("Target.createTarget", {"url": "View-Source:https://site.test/"}, "refuse"), + ("Target.createBrowserContext", {"proxyBypassList": "*"}, "refuse"), + ("Target.createBrowserContext", {"disposeOnDetach": True}, "send"), + ("Page.handleJavaScriptDialog", {"accept": True}, "send"), + ("Browser.crash", {}, "close"), +]) +def test_the_guard_reads_a_url_as_the_browser_does_and_lets_through_what_it_doesnt_guard(method, params, action): + assert guard(method, params, dialogs_answered=False).action == action + + +@pytest.mark.asyncio +async def test_the_guard_keeps_the_browser_up_its_traffic_and_downloads_its_own_and_its_dialogs_agentenvs(): + async with _relay(dialogs_answered=True) as (pipe, relay, base): + async with connect(f"{base}/ada") as ada, connect(f"{base}/bob") as bob: + navigated = await _ask(ada, {"id": 1, "method": "Page.navigate", "params": {"url": "file:///etc/passwd"}}) + exposed = await _ask(ada, {"id": 2, "method": "Target.exposeDevToolsProtocol", + "params": {"targetId": "t1"}}) + proxied = await _ask(ada, {"id": 3, "method": "Target.createBrowserContext", + "params": {"proxyServer": "http://elsewhere.test:8080"}}) + context = await _ask(ada, {"id": 4, "method": "Target.createBrowserContext", "params": {}}) + downloads = await _ask(ada, {"id": 5, "method": "Browser.setDownloadBehavior", "params": { + "behavior": "allowAndName", "downloadPath": "/tmp/elsewhere", "eventsEnabled": True}}) + older = await _ask(ada, {"id": 6, "method": "Page.setDownloadBehavior", + "params": {"behavior": "allow", "downloadPath": "/tmp/elsewhere"}}) + dialog = await _ask(ada, {"id": 7, "method": "Page.handleJavaScriptDialog", "params": {"accept": False}}) + closed = await _ask(bob, {"id": 1, "method": "Browser.close"}) + with pytest.raises(ConnectionClosedOK) as ended: + await bob.recv() + still = await _ask(ada, {"id": 8, "method": "Browser.getVersion"}) + sent = {method for method, _ in pipe.sent} + + assert navigated["error"] == {"code": -32000, "message": "agentenv refuses Page.navigate: file: URLs open the " + "browser's own files, settings and tools"} + assert exposed["error"]["message"].startswith("agentenv refuses Target.exposeDevToolsProtocol: ") + assert proxied["error"]["message"].startswith("agentenv refuses Target.createBrowserContext: ") + assert "result" in context and "result" in downloads and "result" in older + assert pipe.params["Browser.setDownloadBehavior"] == {"behavior": "default", "eventsEnabled": True} + assert pipe.params["Page.setDownloadBehavior"] == {"behavior": "default"} + assert dialog == {"id": 7, "result": {}} and closed == {"id": 1, "result": {}} + assert (ended.value.rcvd.code, ended.value.rcvd.reason) == browser_cdp.CLOSED + assert still["result"] == {"method": "Browser.getVersion", "session": "b1"} + assert not {"Page.navigate", "Target.exposeDevToolsProtocol", "Page.handleJavaScriptDialog", "Browser.close"} & sent + assert [(entry["method"], entry["did"]) for entry in relay.connections[0]["guarded"]] == [ + ("Page.navigate", "refused"), ("Target.exposeDevToolsProtocol", "refused"), + ("Target.createBrowserContext", "refused"), ("Browser.setDownloadBehavior", "changed"), + ("Page.setDownloadBehavior", "changed"), ("Page.handleJavaScriptDialog", "answered")] + assert [(entry["method"], entry["did"]) for entry in relay.connections[1]["guarded"]] == [ + ("Browser.close", "closed")] + + @pytest.mark.asyncio async def test_a_client_that_falls_too_far_behind_is_closed_and_what_waited_for_it_dropped(monkeypatch): monkeypatch.setattr(browser_cdp, "OUTBOX_LIMIT_BYTES", 10_000) diff --git a/tst/integration/computer/browser_computer_test.py b/tst/integration/computer/browser_computer_test.py index 251186647..7946901d5 100644 --- a/tst/integration/computer/browser_computer_test.py +++ b/tst/integration/computer/browser_computer_test.py @@ -6,7 +6,8 @@ started again by supervisord and the computer is ready again; and chromium killed fails it for good, computer-ready/v1 saying why and the tools answering with it. Through its cdp address, Playwright and Puppeteer drive it two at once, each on its own session, by http_url and by ws_url; a wrong token is refused, and so is code in the computer without -one; the address outlives a relaunch, and the computer's reset ends it. Needs Docker and the local registry on +one; one client's close leaves the browser up for the other, which can't open file: URLs; the address outlives a +relaunch, and the computer's reset ends it. Needs Docker and the local registry on localhost:5000, and Node 20 or later and npm for the CDP clients.""" import asyncio @@ -386,6 +387,8 @@ async def test_playwright_and_puppeteer_drive_it_through_its_cdp_address_which_a logged = (await inside.run(deployed, ["sh", "-c", LOGGED_TOKENS, "sh", *tokens], timeout_s=30))["stdout"].splitlines() record = await _record(deployed) + guarded = await _clients(cdp_clients, "guard", ada) + guarded_record = await _record(deployed) await protocol_v1.reset_data(f"{deployed.environment_url}/svc/chromium", headers=deployed.control_headers) relaunched_address = await _cdp(deployed, "ada") relaunched = await _clients(cdp_clients, "together", ada) @@ -405,6 +408,10 @@ async def test_playwright_and_puppeteer_drive_it_through_its_cdp_address_which_a assert from_inside == "HTTP/1.1 403 Forbidden" assert logged == ["/cdp/"], logged assert [c["user"] for c in record["cdp_connections"]] == ["ada", "ada", "ada"] + assert guarded["closed"] == {"ok": True} and guarded["after"] == {"ok": "Home"}, guarded + assert "agentenv refuses Page.navigate: file:" in guarded["file"].get("error", ""), guarded + assert {("Browser.close", "closed"), ("Page.navigate", "refused"), ("Browser.setDownloadBehavior", "changed")} <= { + (entry["method"], entry["did"]) for c in guarded_record["cdp_connections"] for entry in c["guarded"]} assert relaunched_address == ada assert "error" in after_reset["playwright"] and "error" in after_reset["puppeteer"], after_reset assert fresh != ada and not report.failed diff --git a/tst/integration/computer/cdp_clients/connect.mjs b/tst/integration/computer/cdp_clients/connect.mjs index dcab18a21..56bb5993a 100644 --- a/tst/integration/computer/cdp_clients/connect.mjs +++ b/tst/integration/computer/cdp_clients/connect.mjs @@ -1,7 +1,9 @@ // Connects Playwright and Puppeteer to a website browser's CDP address and prints, as JSON, what each saw. // Usage: node connect.mjs [exact], being: // together: Playwright by http_url and Puppeteer by ws_url at once, then Playwright by ws_url, each opening the site; -// refused: both clients at the address with its token changed, or as given with "exact", each expected to fail. +// refused: both clients at the address with its token changed, or as given with "exact", each expected to fail; +// guard: Playwright kept connected by http_url while Puppeteer closes the browser by ws_url, then Playwright opening +// the site, and file:///etc/hostname, which the guard refuses. import { chromium } from "playwright-core"; import puppeteer from "puppeteer-core"; @@ -41,6 +43,15 @@ let result; if (check === "together") { const [byHttp, byWs] = await Promise.all([settled(playwright(httpUrl)), settled(pptr(wsUrl))]); result = { playwright_http: byHttp, puppeteer_ws: byWs, playwright_ws: await settled(playwright(wsUrl)) }; +} else if (check === "guard") { + const kept = await chromium.connectOverCDP(httpUrl, { timeout: TIMEOUT_MS }); + const closer = await puppeteer.connect({ browserWSEndpoint: wsUrl, protocolTimeout: TIMEOUT_MS }); + const closed = await settled(closer.close().then(() => true)); + const page = await kept.contexts()[0].newPage(); + const after = await settled(page.goto(site, { timeout: TIMEOUT_MS }).then(() => page.title())); + const file = await settled(page.goto("file:///etc/hostname", { timeout: TIMEOUT_MS })); + await page.close(); + result = { closed, after, file }; } else { result = { playwright: await settled(playwright(wrong(httpUrl))), puppeteer: await settled(pptr(wrong(wsUrl))) }; } From 831c670b95dd0b39d75210eaf44fc750d482eac6 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 4 Nov 2026 09:58:46 -0800 Subject: [PATCH 2/3] website browser: the guard refuses wrapped commands and answers malformed ones Target.sendMessageToTarget carries a whole command inside its message, so a file: navigation wrapped in it reached the browser unchecked. The relay serves flattened sessions only, so it's refused. A command whose params weren't an object, or whose method or sessionId wasn't a string, raised in the guard and got no answer, leaving its client to time out. It now gets a CDP error with code -32600, and so does a message that isn't an object. Co-Authored-By: Claude Opus 5.5 --- packages/agentenv-protocol/README.md | 7 +++++-- .../src/agentenv_protocol/browser_cdp.py | 19 +++++++++++++++++-- .../tests/test_browser_cdp.py | 18 ++++++++++++++++-- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/packages/agentenv-protocol/README.md b/packages/agentenv-protocol/README.md index 30bd71245..8bf36a409 100644 --- a/packages/agentenv-protocol/README.md +++ b/packages/agentenv-protocol/README.md @@ -883,14 +883,17 @@ are the profile's template. The base class adds: A guard sits on every command, a guardrail, since the sandbox's network is the boundary: - `Browser.close`, `Browser.crash` and `Browser.crashGpuProcess` are answered `{}` and end only that connection, with 1000; - - `Target.exposeDevToolsProtocol`, a `Target.createBrowserContext` with its own proxy, and a + - `Target.exposeDevToolsProtocol`, `Target.sendMessageToTarget` (only flattened sessions are + served, whose commands the guard sees), a `Target.createBrowserContext` with its own proxy, and a `Page.navigate` or `Target.createTarget` to the browser's own pages (`file:`, `chrome:`, `chrome-extension:`, `chrome-untrusted:`, `view-source:`, `devtools:`, and `about:` but `about:blank` and `about:srcdoc`) get a CDP error, `agentenv refuses …`; - `Browser.setDownloadBehavior` and `Page.setDownloadBehavior` are sent on as agentenv's own, the browser's default into `~/Downloads`, keeping only the context and whether to send events; - when the deploy answers dialogs, an agent's `Page.handleJavaScriptDialog` is answered `{}` and not - sent on. + sent on; + - a message that isn't a command, an object with a string `method`, and object `params` and a string + `sessionId` if any, gets a CDP error with code -32600. `record` lists each connection as `cdp_connections`: its user, when it opened and when it closed, and the first 100 commands the guard refused, changed, answered or closed it on. diff --git a/packages/agentenv-protocol/src/agentenv_protocol/browser_cdp.py b/packages/agentenv-protocol/src/agentenv_protocol/browser_cdp.py index 5e9291e18..30fbfadae 100644 --- a/packages/agentenv-protocol/src/agentenv_protocol/browser_cdp.py +++ b/packages/agentenv-protocol/src/agentenv_protocol/browser_cdp.py @@ -40,6 +40,9 @@ NO_SESSION = {"code": -32001, "message": "Session with given id not found."} #: The code of a CDP error for a command not carried out, as Chromium answers its own refusals. SERVER_ERROR = -32000 +#: The answer to a message that isn't a command the relay can read, with JSON-RPC's code for an invalid request. +MALFORMED = {"code": -32600, "message": "a command is an object with a string method, and object params and a " + "string sessionId if any"} #: The commands that would end the browser every connection shares: each ends only the connection that sends it. ENDS_BROWSER = frozenset({"Browser.close", "Browser.crash", "Browser.crashGpuProcess"}) #: The commands that set how and where downloads are saved, which agentenv keeps as its own. @@ -140,6 +143,8 @@ def guard(method: str, params: dict, *, dialogs_answered: bool) -> Verdict: return Verdict("close", params) if method == "Target.exposeDevToolsProtocol": return Verdict("refuse", params, "it would give a page's script the browser's own CDP channel") + if method == "Target.sendMessageToTarget": + return Verdict("refuse", params, "only flattened sessions are served, whose commands the guard sees") if method == "Target.createBrowserContext" and {"proxyServer", "proxyBypassList"} & params.keys(): return Verdict("refuse", params, "a context's traffic goes through the env's site proxy") if method in ("Page.navigate", "Target.createTarget") and (scheme := _own_scheme(params.get("url"))): @@ -234,10 +239,13 @@ def ended(self) -> None: for connection in list(self._open): self._close(connection, RELAUNCHED) - async def _command(self, connection: _Connection, message: dict) -> None: + async def _command(self, connection: _Connection, message: Any) -> None: if connection.closing: return - method, params = message.get("method", ""), message.get("params") or {} + if not _well_formed(message): + self._put(connection, {"id": message.get("id") if isinstance(message, dict) else None, "error": MALFORMED}) + return + method, params = message["method"], message.get("params") or {} session = message.get("sessionId") or connection.session verdict = guard(method, params, dialogs_answered=self.dialogs_answered) if session not in connection.sessions: @@ -320,5 +328,12 @@ async def _send(self, connection: _Connection) -> None: await connection.websocket.send_text(message) +def _well_formed(message: Any) -> bool: + """Whether ``message`` is a command the relay can read: an object with a string ``method``, and object ``params`` + and a string ``sessionId`` if any.""" + return isinstance(message, dict) and isinstance(message.get("method"), str) and \ + isinstance(message.get("params") or {}, dict) and isinstance(message.get("sessionId") or "", str) + + def _now() -> str: return datetime.now(timezone.utc).isoformat(timespec="seconds").replace("+00:00", "Z") diff --git a/packages/agentenv-protocol/tests/test_browser_cdp.py b/packages/agentenv-protocol/tests/test_browser_cdp.py index 90c427191..d850b13df 100644 --- a/packages/agentenv-protocol/tests/test_browser_cdp.py +++ b/packages/agentenv-protocol/tests/test_browser_cdp.py @@ -196,6 +196,7 @@ async def test_each_connection_gets_its_own_browser_session_its_own_answers_and_ ("Target.createBrowserContext", {"disposeOnDetach": True}, "send"), ("Page.handleJavaScriptDialog", {"accept": True}, "send"), ("Browser.crash", {}, "close"), + ("Target.sendMessageToTarget", {"sessionId": "s", "message": "{}"}, "refuse"), ]) def test_the_guard_reads_a_url_as_the_browser_does_and_lets_through_what_it_doesnt_guard(method, params, action): assert guard(method, params, dialogs_answered=False).action == action @@ -216,6 +217,14 @@ async def test_the_guard_keeps_the_browser_up_its_traffic_and_downloads_its_own_ older = await _ask(ada, {"id": 6, "method": "Page.setDownloadBehavior", "params": {"behavior": "allow", "downloadPath": "/tmp/elsewhere"}}) dialog = await _ask(ada, {"id": 7, "method": "Page.handleJavaScriptDialog", "params": {"accept": False}}) + wrapped = await _ask(ada, {"id": 9, "method": "Target.sendMessageToTarget", "params": { + "sessionId": "b1-c", "message": json.dumps({"id": 1, "method": "Page.navigate", + "params": {"url": "file:///etc/passwd"}})}}) + malformed = [await _ask(ada, {"id": 10, "method": "Page.navigate", "params": [1]}), + await _ask(ada, {"id": 11, "params": {}}), + await _ask(ada, {"id": 12, "method": "Browser.getVersion", "sessionId": ["b1"]})] + await ada.send("[1]") + not_a_command = json.loads(await ada.recv()) closed = await _ask(bob, {"id": 1, "method": "Browser.close"}) with pytest.raises(ConnectionClosedOK) as ended: await bob.recv() @@ -232,11 +241,16 @@ async def test_the_guard_keeps_the_browser_up_its_traffic_and_downloads_its_own_ assert dialog == {"id": 7, "result": {}} and closed == {"id": 1, "result": {}} assert (ended.value.rcvd.code, ended.value.rcvd.reason) == browser_cdp.CLOSED assert still["result"] == {"method": "Browser.getVersion", "session": "b1"} - assert not {"Page.navigate", "Target.exposeDevToolsProtocol", "Page.handleJavaScriptDialog", "Browser.close"} & sent + assert wrapped["error"]["message"].startswith("agentenv refuses Target.sendMessageToTarget: ") + assert [answer["error"]["code"] for answer in malformed] == [-32600] * 3 + assert not_a_command == {"id": None, "error": browser_cdp.MALFORMED} + assert not {"Page.navigate", "Target.exposeDevToolsProtocol", "Target.sendMessageToTarget", + "Page.handleJavaScriptDialog", "Browser.close"} & sent assert [(entry["method"], entry["did"]) for entry in relay.connections[0]["guarded"]] == [ ("Page.navigate", "refused"), ("Target.exposeDevToolsProtocol", "refused"), ("Target.createBrowserContext", "refused"), ("Browser.setDownloadBehavior", "changed"), - ("Page.setDownloadBehavior", "changed"), ("Page.handleJavaScriptDialog", "answered")] + ("Page.setDownloadBehavior", "changed"), ("Page.handleJavaScriptDialog", "answered"), + ("Target.sendMessageToTarget", "refused")] assert [(entry["method"], entry["did"]) for entry in relay.connections[1]["guarded"]] == [ ("Browser.close", "closed")] From 03866208f8424465ad7a1deb9f60f74298588212 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 4 Nov 2026 10:18:16 -0800 Subject: [PATCH 3/3] test: wait for the unanswered command to reach the pipe before ending its connection The relay sends nothing on once a connection starts closing, so when the test ended the connection before the relay had read its Hang command, Hang never reached the pipe and there was nothing to give up. CI's unit job hit that ordering. The test now waits until Hang is on the pipe. Co-Authored-By: Claude Opus 5.5 --- packages/agentenv-protocol/tests/test_browser_cdp.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/agentenv-protocol/tests/test_browser_cdp.py b/packages/agentenv-protocol/tests/test_browser_cdp.py index d850b13df..9589e689b 100644 --- a/packages/agentenv-protocol/tests/test_browser_cdp.py +++ b/packages/agentenv-protocol/tests/test_browser_cdp.py @@ -161,6 +161,8 @@ async def test_each_connection_gets_its_own_browser_session_its_own_answers_and_ refused = await _ask(bob, {"id": 4, "method": "Runtime.evaluate", "sessionId": "b1-c"}) failed = await _ask(ada, {"id": 5, "method": "Boom"}) await ada.send(json.dumps({"id": 6, "method": "Hang"})) + while ("Hang", "b1") not in pipe.sent: + await asyncio.sleep(0.01) pipe.emit({"method": "Page.loadEventFired", "sessionId": "b1-c", "params": {}}) pipe.emit({"method": "Target.targetCreated", "sessionId": "b2", "params": {}}) pipe.emit({"method": "Page.frameNavigated", "params": {}})