From d524cc9a16e13be42762984d21c1e4d19fbce24e Mon Sep 17 00:00:00 2001 From: abundantbeing Date: Thu, 13 Aug 2026 21:18:08 +0700 Subject: [PATCH] fix(browser): harden extension controller routing Keep extension control opt-in and preserve existing browser backends unless an exact server-bound controller is available. Centralize protocol and capability admission across API and dashboard transports, make selected-controller results authoritative, bypass stale availability caches only inside bound requests, and serialize structured results for the existing tool contract. Add a real browser_snapshot route-table/WebSocket E2E, strict admission and ownership regressions, public configuration and protocol documentation, and tests proving feature-off/no-controller compatibility. --- cli-config.yaml.example | 5 + gateway/browser_control_broker.py | 59 ++++++++- gateway/platforms/api_server.py | 62 ++++----- tests/gateway/test_browser_control_api.py | 91 ++++++++++++- tests/gateway/test_browser_control_cloud.py | 78 +++++++++-- tests/tools/test_browser_extension_router.py | 123 ++++++++++++++++++ .../test_browser_extension_router_wiring.py | 2 +- tools/browser_extension_router.py | 57 +++++++- tools/browser_tool.py | 52 ++++++-- tools/registry.py | 19 +++ tui_gateway/methods_browser_control.py | 42 +++--- tui_gateway/ws.py | 2 +- .../programmatic-integration.md | 7 + .../docs/user-guide/features/api-server.md | 69 ++++++++++ 14 files changed, 593 insertions(+), 75 deletions(-) diff --git a/cli-config.yaml.example b/cli-config.yaml.example index 472b7b98c3..5e1b325be9 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -488,6 +488,11 @@ browser: # Inactivity timeout in seconds - browser sessions are automatically closed # after this period of no activity between agent loops (default: 120 = 2 minutes) inactivity_timeout: 120 + # Let an authenticated browser extension register as the controller for an + # existing Hermes session. Disabled by default. Local API registration also + # requires the API server bearer key to be configured. + extension_control: + enabled: false # ============================================================================= # Tool Loop Guardrails diff --git a/gateway/browser_control_broker.py b/gateway/browser_control_broker.py index 98e5dcdb28..cba0df58d5 100644 --- a/gateway/browser_control_broker.py +++ b/gateway/browser_control_broker.py @@ -1,4 +1,4 @@ -"""Transport-neutral browser-control broker core (Phase 4). +"""Transport-neutral browser-control broker core. This module is the in-process heart of the browser-control feature: it binds an *identity-scoped controller* (the party that physically drives a browser) @@ -71,6 +71,50 @@ DEFAULT_TICKET_TTL = 30.0 #: Default wall time a dispatch waits for the controller to complete. DEFAULT_COMMAND_TIMEOUT = 30.0 +#: Current wire protocol version. Registration requires this exact integer; +#: booleans are rejected even though ``bool`` subclasses ``int`` in Python. +BROWSER_CONTROL_PROTOCOL_VERSION = 1 + +#: Exact controller capability contract shared by every transport. The broker +#: never accepts arbitrary browser methods: raw CDP, script evaluation, console +#: access, uploads, and other privileged surfaces remain outside this allowlist. +BROWSER_CONTROL_CAPABILITIES = frozenset( + { + "controller.noop", + "browser_back", + "browser_click", + "browser_navigate", + "browser_press", + "browser_screenshot", + "browser_scroll", + "browser_snapshot", + "browser_tab_activate", + "browser_tabs", + "browser_type", + } +) + + +def browser_control_protocol_supported(value: Any) -> bool: + """Return whether ``value`` names the exact supported wire version.""" + return type(value) is int and value == BROWSER_CONTROL_PROTOCOL_VERSION + + +def filter_browser_control_capabilities(value: Any) -> frozenset: + """Return the permitted subset of a JSON/RPC capability list. + + A malformed non-list value has no capabilities. Unknown or non-string + entries are ignored; registration rejects an empty returned set. + """ + if not isinstance(value, list): + return frozenset() + return frozenset( + capability + for capability in value + if isinstance(capability, str) + and capability in BROWSER_CONTROL_CAPABILITIES + ) + #: Wire method names for controller frames. Transport-neutral by contract: #: transports carry these envelopes verbatim. FRAME_COMMAND = "browser.controller.command" @@ -293,6 +337,17 @@ class BrowserControlBroker: return None return controller + def is_owner(self, scope: ControllerScope, owner: Any) -> bool: + """Return whether ``owner`` is the exact transport attached to ``scope``. + + Ownership is independent of capabilities. Transport handlers use this + for heartbeat and result admission so a least-privilege controller does + not need to request ``controller.noop`` merely to complete a real action. + """ + with self._lock: + controller = self._controllers.get(scope) + return controller is not None and controller.owner is owner + def detach( self, scope: ControllerScope, @@ -600,7 +655,7 @@ def get_browser_control_broker() -> BrowserControlBroker: def browser_control_enabled(config: Optional[dict] = None) -> bool: - """Return the explicit Phase 4 feature flag (disabled by default).""" + """Return the explicit browser-control feature flag (disabled by default).""" if config is None: try: from hermes_cli.config import load_config diff --git a/gateway/platforms/api_server.py b/gateway/platforms/api_server.py index 127fafd6b5..feed75afc5 100644 --- a/gateway/platforms/api_server.py +++ b/gateway/platforms/api_server.py @@ -77,24 +77,10 @@ _api_request_browser_control_transport_family: ContextVar[str] = ContextVar( "api_server_browser_control_transport_family", default="" ) -#: Phase 4 browser-extension control protocol version (advertised in -#: /v1/capabilities and echoed in registration responses). +#: Browser-extension control protocol version advertised in capabilities and +#: echoed in registration responses. Strict validation is centralized in the +#: broker's ``browser_control_protocol_supported`` helper. _BROWSER_CONTROL_PROTOCOL_VERSION = 1 -#: Exact Phase 6 browser-extension action allowlist. Any requested capability -#: outside this set is filtered out rather than advertised or dispatched. -_BROWSER_CONTROL_CAPABILITIES = frozenset({ - "controller.noop", - "browser_back", - "browser_click", - "browser_navigate", - "browser_press", - "browser_screenshot", - "browser_scroll", - "browser_snapshot", - "browser_tab_activate", - "browser_tabs", - "browser_type", -}) _BROWSER_CONTROL_WS_PROTOCOL = "hermes-browser-control-v1" _BROWSER_CONTROL_TICKET_PROTOCOL_PREFIX = "hermes-browser-control-ticket." @@ -123,8 +109,11 @@ from agent.redact import redact_sensitive_text from agent.interrupt_compat import request_hard_interrupt from gateway.readiness import collect_runtime_readiness from gateway.browser_control_broker import ( + BROWSER_CONTROL_CAPABILITIES, ControllerScope, TicketInvalid, + browser_control_protocol_supported, + filter_browser_control_capabilities, get_browser_control_broker, ) @@ -1526,9 +1515,8 @@ class APIServerAdapter(BasePlatformAdapter): # Shutdown counts this reservation so the request cannot slip through # the drain between its first await and _run_agent()/task registration. self._pending_agent_requests: int = 0 - # Phase 4 browser-control broker core: transport-neutral ticket / - # controller / command lifecycle shared with the dashboard Gateway - # transport. This adapter only maps HTTP registration and the + # Browser-control broker core: transport-neutral ticket, controller, + # and command lifecycle shared with the dashboard Gateway transport. This adapter only maps HTTP registration and the # controller WebSocket onto the broker; it owns no broker state. self._browser_control_broker = get_browser_control_broker() @@ -2130,7 +2118,7 @@ class APIServerAdapter(BasePlatformAdapter): ("GET", "/v1/models", self._handle_models), ("GET", "/api/model/options", self._handle_model_options), ("GET", "/v1/capabilities", self._handle_capabilities), - # Phase 4 authenticated browser-control surface: POST registration + # Authenticated browser-control surface: POST registration # mints a short-lived ticket; the controller then opens the WS with # that ticket. Both are gated on browser.extension_control.enabled # and API-key auth (see the handlers for the exact status ladder). @@ -3273,7 +3261,7 @@ class APIServerAdapter(BasePlatformAdapter): "browser_extension_control": { "enabled": self._browser_control_enabled(), "protocol_version": _BROWSER_CONTROL_PROTOCOL_VERSION, - "capabilities": list(_BROWSER_CONTROL_CAPABILITIES), + "capabilities": sorted(BROWSER_CONTROL_CAPABILITIES), "real_browser_actions": True, "transports": { "local_vps": "websocket-subprotocol-ticket", @@ -3312,7 +3300,7 @@ class APIServerAdapter(BasePlatformAdapter): }) # ------------------------------------------------------------------ - # Phase 4 browser-extension control (authenticated local/VPS API) + # Browser-extension control (authenticated local/VPS API) # ------------------------------------------------------------------ async def _handle_browser_control_register(self, request: "web.Request") -> "web.Response": @@ -3323,7 +3311,7 @@ class APIServerAdapter(BasePlatformAdapter): single-use ticket to open the controller WebSocket. Identity is NOT taken from the request body: the scope principal is derived server-side from the authenticated key/profile as a non-reversible - digest, and the capability set is filtered to the exact Phase 6 + digest, and the capability set is filtered to the shared browser action allowlist, so a spoofed ``principal_id`` or inflated capability list in the payload is ignored rather than honored. The named session must already exist in @@ -3369,6 +3357,15 @@ class APIServerAdapter(BasePlatformAdapter): _openai_error("Request body must be a JSON object."), status=400 ) + if not browser_control_protocol_supported(payload.get("protocol_version")): + return web.json_response( + _openai_error( + "Unsupported browser-control protocol version.", + code="browser_control_protocol_unsupported", + ), + status=400, + ) + controller_id = str(payload.get("controller_id") or "").strip() browser_profile_id = str(payload.get("browser_profile_id") or "").strip() session_id = str(payload.get("session_id") or "").strip() @@ -3402,12 +3399,17 @@ class APIServerAdapter(BasePlatformAdapter): ) profile = _api_request_profile.get() or "default" - capabilities = frozenset( - capability - for capability in payload.get("capabilities") or [] - if isinstance(capability, str) - and capability in _BROWSER_CONTROL_CAPABILITIES + capabilities = filter_browser_control_capabilities( + payload.get("capabilities") ) + if not capabilities: + return web.json_response( + _openai_error( + "At least one permitted browser-control capability is required.", + code="browser_control_no_capabilities", + ), + status=400, + ) scope = ControllerScope( principal_id=self._derive_browser_control_principal(profile), profile_id=profile, @@ -3571,7 +3573,7 @@ class APIServerAdapter(BasePlatformAdapter): self._browser_control_broker.cancel(scope, tool_call_id=tool_call_id) def _browser_control_enabled(self) -> bool: - """Phase 4 feature flag; False unless explicitly enabled. + """Feature flag; False unless explicitly enabled. Reads ``browser.extension_control.enabled`` from the global config (defaults to False). Tests monkeypatch this method directly to force diff --git a/tests/gateway/test_browser_control_api.py b/tests/gateway/test_browser_control_api.py index 7421df2025..cf70cb92fe 100644 --- a/tests/gateway/test_browser_control_api.py +++ b/tests/gateway/test_browser_control_api.py @@ -8,6 +8,7 @@ from aiohttp.test_utils import TestClient, TestServer from gateway.browser_control_broker import ControllerRejected, ControllerScope from gateway.config import PlatformConfig from gateway.platforms.api_server import APIServerAdapter +from tools.browser_extension_router import route_browser_tool API_KEY = "-".join(("fixture", "neutral", "api", "key", "123")) @@ -83,7 +84,7 @@ def _registration_body(**overrides): @pytest.mark.asyncio -async def test_phase6_registration_grants_only_the_exact_real_action_allowlist(monkeypatch): +async def test_registration_grants_only_the_exact_real_action_allowlist(monkeypatch): adapter = _adapter() monkeypatch.setattr(adapter, "_browser_control_enabled", lambda: True) requested = [ @@ -109,6 +110,36 @@ async def test_phase6_registration_grants_only_the_exact_real_action_allowlist(m } +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("overrides", "code"), + [ + ({"protocol_version": 2}, "browser_control_protocol_unsupported"), + ({"protocol_version": True}, "browser_control_protocol_unsupported"), + ({"capabilities": []}, "browser_control_no_capabilities"), + ( + {"capabilities": ["browser_cdp", "arbitrary.capability"]}, + "browser_control_no_capabilities", + ), + ], +) +async def test_registration_rejects_unsupported_protocol_or_empty_capability_intersection( + monkeypatch, overrides, code +): + adapter = _adapter() + monkeypatch.setattr(adapter, "_browser_control_enabled", lambda: True) + async with TestClient(TestServer(_app(adapter))) as client: + response = await client.post( + "/v1/browser-control/register", + json=_registration_body(**overrides), + headers={"Authorization": f"Bearer {API_KEY}"}, + ) + body = await response.json() + + assert response.status == 400 + assert body["error"]["code"] == code + + def test_route_table_advertises_registration_and_controller_ws_without_replacing_existing_routes(): adapter = _adapter() routes = {(method, path) for method, path, _handler in adapter._http_route_table()} @@ -383,6 +414,64 @@ async def test_local_api_ticket_ws_noop_round_trip_filters_spoofed_identity_and_ assert replay.value.status == 401 +@pytest.mark.asyncio +async def test_real_browser_action_routes_through_controller_without_legacy_fallback(monkeypatch): + adapter = _adapter() + monkeypatch.setattr(adapter, "_browser_control_enabled", lambda: True) + async with TestClient(TestServer(_app(adapter))) as client: + response = await client.post( + "/v1/browser-control/register", + json=_registration_body(capabilities=["browser_snapshot"]), + headers={"Authorization": f"Bearer {API_KEY}"}, + ) + assert response.status == 201 + registration = await response.json() + ws = await client.ws_connect( + "/v1/browser-control/ws", + protocols=[CONTROL_PROTOCOL, _ticket_protocol(registration["ticket"])], + ) + + legacy_calls = [] + pending = asyncio.create_task( + asyncio.to_thread( + route_browser_tool, + "browser_snapshot", + {"include": "accessibility"}, + fallback=lambda: legacy_calls.append(True) or "legacy-result", + broker=adapter._browser_control_broker, + enabled=True, + session_id="session-fixture", + principal_id=registration["scope"]["principal_id"], + transport_family="local-api", + tool_call_id="tool-call-real-action", + ) + ) + command = await ws.receive_json(timeout=2.0) + assert command["method"] == "browser.controller.command" + assert command["params"]["action"] == "browser_snapshot" + assert command["params"]["arguments"] == {"include": "accessibility"} + await ws.send_json( + { + "method": "browser.controller.result", + "params": { + "command_id": command["params"]["command_id"], + "ok": True, + "result": { + "title": "Example Domain", + "url": "https://example.test/", + "refs": [], + }, + }, + } + ) + + assert await asyncio.wait_for(pending, timeout=2.0) == ( + '{"title": "Example Domain", "url": "https://example.test/", "refs": []}' + ) + assert legacy_calls == [] + await ws.close() + + @pytest.mark.asyncio async def test_remote_api_uses_the_same_authenticated_noop_round_trip(monkeypatch): adapter = _adapter() diff --git a/tests/gateway/test_browser_control_cloud.py b/tests/gateway/test_browser_control_cloud.py index 6dee92f12c..3b2561551d 100644 --- a/tests/gateway/test_browser_control_cloud.py +++ b/tests/gateway/test_browser_control_cloud.py @@ -3,7 +3,11 @@ from types import SimpleNamespace import pytest -from gateway.browser_control_broker import ControllerRejected, get_browser_control_broker +from gateway.browser_control_broker import ( + BROWSER_CONTROL_CAPABILITIES, + ControllerRejected, + get_browser_control_broker, +) from hermes_cli import web_server from hermes_cli.dashboard_auth.ws_tickets import _reset_for_tests, mint_ticket from tui_gateway import server @@ -173,7 +177,60 @@ def test_cloud_controller_registration_rejects_missing_or_internal_identity(monk server._sessions.pop("session-fixture", None) -def test_cloud_gateway_noop_round_trip_is_bound_to_ticket_identity_and_session_transport(monkeypatch): +@pytest.mark.parametrize( + "params", + [ + {"protocol_version": 2, "capabilities": ["browser_navigate"]}, + {"protocol_version": True, "capabilities": ["browser_navigate"]}, + { + "protocol_version": 1, + "capabilities": ["browser_cdp", "arbitrary.capability"], + }, + ], +) +def test_cloud_registration_rejects_unsupported_protocol_or_empty_capabilities( + monkeypatch, params +): + monkeypatch.setattr( + "gateway.browser_control_broker.browser_control_enabled", lambda: True + ) + + class Transport: + auth_identity = { + "user_id": "user-fixture", + "provider": "provider-fixture", + } + + def write(self, _frame): + return True + + transport = Transport() + server._sessions["registration-session-fixture"] = { + "transport": transport, + "session_key": "stored-registration-session", + "profile": "default", + } + try: + response = server.dispatch( + { + "jsonrpc": "2.0", + "id": 1, + "method": "browser.controller.register", + "params": { + "session_id": "registration-session-fixture", + "controller_id": "controller-fixture", + "browser_profile_id": "browser-profile-fixture", + **params, + }, + }, + transport, + ) + assert response["error"]["code"] == 4403 + finally: + server._sessions.pop("registration-session-fixture", None) + + +def test_cloud_gateway_real_action_round_trip_is_bound_to_identity_and_transport(monkeypatch): monkeypatch.setattr( "gateway.browser_control_broker.browser_control_enabled", lambda: True ) @@ -211,7 +268,7 @@ def test_cloud_gateway_noop_round_trip_is_bound_to_ticket_identity_and_session_t "session_id": "session-fixture", "controller_id": "controller-fixture", "browser_profile_id": "browser-profile-fixture", - "capabilities": ["controller.noop", "browser_navigate"], + "capabilities": ["browser_navigate", "browser_cdp"], "principal_id": "spoofed-client-principal", }, }, @@ -220,7 +277,8 @@ def test_cloud_gateway_noop_round_trip_is_bound_to_ticket_identity_and_session_t scope_payload = registration["result"]["scope"] assert scope_payload["principal_id"] != "spoofed-client-principal" assert scope_payload["transport_family"] == "cloud-ticket-ws" - assert scope_payload["capabilities"] == ["controller.noop"] + assert scope_payload["capabilities"] == ["browser_navigate"] + assert "browser_cdp" not in BROWSER_CONTROL_CAPABILITIES missing_identity = server.dispatch( { @@ -268,15 +326,15 @@ def test_cloud_gateway_noop_round_trip_is_bound_to_ticket_identity_and_session_t assert foreign_heartbeat["error"]["code"] == 4403 outcome = {} - def dispatch_noop(): + def dispatch_navigate(): outcome["result"] = broker.dispatch( scope, - action="controller.noop", - arguments={"echo": "cloud"}, + action="browser_navigate", + arguments={"url": "https://example.test"}, tool_call_id="tool-call-cloud", ) - thread = threading.Thread(target=dispatch_noop) + thread = threading.Thread(target=dispatch_navigate) thread.start() assert ready.wait(timeout=1.0) command_event = frames[-1] @@ -309,8 +367,8 @@ def test_cloud_gateway_noop_round_trip_is_bound_to_ticket_identity_and_session_t try: rejected_outcome["result"] = broker.dispatch( scope, - action="controller.noop", - arguments={"echo": "reject"}, + action="browser_navigate", + arguments={"url": "https://reject.example.test"}, tool_call_id="tool-call-cloud-rejected", ) except Exception as exc: # asserted below diff --git a/tests/tools/test_browser_extension_router.py b/tests/tools/test_browser_extension_router.py index 66dcf4ea3e..9d7d69b1e4 100644 --- a/tests/tools/test_browser_extension_router.py +++ b/tests/tools/test_browser_extension_router.py @@ -115,6 +115,27 @@ def test_selected_controller_receives_immutable_arguments_and_context(): ] +def test_selected_controller_dict_result_is_serialized_for_registry_contract(): + broker = FakeBroker( + scope="scope-fixture", + selected="connection-fixture", + result={"ok": True, "title": "Example Domain", "refs": []}, + ) + + result = route_browser_tool( + "browser_snapshot", + {}, + fallback=lambda: pytest.fail("selected controller must not call fallback"), + broker=broker, + enabled=True, + session_id="session-fixture", + principal_id="principal-fixture", + transport_family="local-api", + ) + + assert result == '{"ok": true, "title": "Example Domain", "refs": []}' + + def test_selected_controller_failure_never_retries_through_existing_backend(): broker = FakeBroker( scope="scope-fixture", @@ -196,3 +217,105 @@ def test_routed_handler_reads_server_bound_identity_from_session_context(monkeyp "transport_family": "cloud-ticket-ws", }, ) + + +def test_routeable_browser_tools_are_available_for_bound_extension_controller(monkeypatch): + """The extension route must not be stripped by legacy Browser Use checks.""" + from tools import browser_tool + + monkeypatch.setattr(browser_tool, "check_browser_requirements", lambda: False) + monkeypatch.setattr( + browser_tool, + "extension_controller_available", + lambda action: action == "browser_snapshot", + ) + + assert browser_tool.check_browser_snapshot_requirements() is True + assert browser_tool.check_browser_click_requirements() is False + + +def test_extension_availability_requires_exact_scope_and_capability(monkeypatch): + from gateway import browser_control_broker + from gateway.session_context import clear_session_vars, set_session_vars + from tools import browser_extension_router + + broker = FakeBroker(scope="scope-fixture", selected="connection-fixture") + monkeypatch.setattr(browser_control_broker, "browser_control_enabled", lambda: True) + monkeypatch.setattr( + browser_control_broker, "get_browser_control_broker", lambda: broker + ) + tokens = set_session_vars( + session_id="session-fixture", + browser_control_principal="principal-fixture", + browser_control_transport_family="local-api", + ) + try: + assert browser_extension_router.extension_controller_available("browser_snapshot") is True + finally: + clear_session_vars(tokens) + + assert broker.calls == [ + ( + "scope", + { + "session_id": "session-fixture", + "principal_id": "principal-fixture", + "transport_family": "local-api", + }, + ), + ("select", "scope-fixture", "browser_snapshot"), + ] + + +def test_routeable_browser_tools_preserve_legacy_gate_without_bound_identity(monkeypatch): + """A feature flag alone must not advertise tools outside a bound request.""" + from gateway import browser_control_broker + from tools import browser_tool + + monkeypatch.setattr(browser_control_broker, "browser_control_enabled", lambda: True) + monkeypatch.setattr(browser_tool, "check_browser_requirements", lambda: False) + + assert browser_tool.check_browser_snapshot_requirements() is False + + +def test_bound_browser_request_bypasses_availability_caches(): + from gateway.session_context import clear_session_vars, set_session_vars + from tools.registry import CHECK_FN_CACHE_BYPASS, check_fn_cache_scope + + tokens = set_session_vars( + session_id="session-fixture", + browser_control_principal="principal-fixture", + browser_control_transport_family="local-api", + ) + try: + assert check_fn_cache_scope() == CHECK_FN_CACHE_BYPASS + finally: + clear_session_vars(tokens) + + +def test_registry_advertises_snapshot_through_extension_when_legacy_backend_is_down( + monkeypatch, +): + from gateway.session_context import clear_session_vars, set_session_vars + from tools import browser_tool + from tools.registry import registry + + monkeypatch.setattr(browser_tool, "check_browser_requirements", lambda: False) + monkeypatch.setattr( + browser_tool, + "extension_controller_available", + lambda action: action == "browser_snapshot", + ) + tokens = set_session_vars( + session_id="session-fixture", + browser_control_principal="principal-fixture", + browser_control_transport_family="local-api", + ) + try: + definitions = registry.get_definitions({"browser_snapshot", "browser_click"}, quiet=True) + finally: + clear_session_vars(tokens) + + assert [definition["function"]["name"] for definition in definitions] == [ + "browser_snapshot" + ] diff --git a/tests/tools/test_browser_extension_router_wiring.py b/tests/tools/test_browser_extension_router_wiring.py index 0a81b9c567..bbdd4bf1be 100644 --- a/tests/tools/test_browser_extension_router_wiring.py +++ b/tests/tools/test_browser_extension_router_wiring.py @@ -1,4 +1,4 @@ -"""Wiring regression tests for the Phase 4 browser extension router. +"""Wiring regression tests for the browser extension router. These guard the *registry wiring* — that every ``browser_*`` handler routes through :func:`tools.browser_extension_router.routed_browser_handler` with diff --git a/tools/browser_extension_router.py b/tools/browser_extension_router.py index 7a0429e02e..34e1aa5918 100644 --- a/tools/browser_extension_router.py +++ b/tools/browser_extension_router.py @@ -1,4 +1,4 @@ -"""Phase 4 registry-level browser extension router. +"""Registry-level browser extension router. This module is the *agent-side* half of the browser-extension-control feature: it decides, for one registry ``browser_*`` handler invocation, @@ -40,12 +40,55 @@ config change is honored without restart. from __future__ import annotations +import json import logging from typing import Any, Callable, Dict, Optional logger = logging.getLogger(__name__) +def extension_controller_available(action: str) -> bool: + """Whether this request owns one exact controller capable of ``action``. + + Tool-schema assembly runs inside the API request's session context, before + a model can call a browser tool. The legacy browser backend's availability + probe cannot decide whether the extension route is usable, so routeable + tools consult the process-local broker directly. Missing server-bound + identity, ambiguous scope, a detached controller, or a capability mismatch + all fail closed. + """ + try: + from gateway.browser_control_broker import ( + browser_control_enabled, + get_browser_control_broker, + ) + from gateway.session_context import get_session_env + + if not browser_control_enabled(): + return False + session_id = get_session_env("HERMES_SESSION_ID", "") or None + principal_id = get_session_env("HERMES_BROWSER_CONTROL_PRINCIPAL", "") or None + transport_family = get_session_env( + "HERMES_BROWSER_CONTROL_TRANSPORT_FAMILY", "" + ) or None + if not session_id or not principal_id or not transport_family: + return False + broker = get_browser_control_broker() + scope = broker.scope_for_session( + session_id=session_id, + principal_id=principal_id, + transport_family=transport_family, + ) + return scope is not None and broker.select(scope, action) is not None + except Exception: + logger.debug( + "browser extension availability check failed for %s", + action, + exc_info=True, + ) + return False + + def route_browser_tool( action: str, args: Dict[str, Any], @@ -114,10 +157,16 @@ def route_browser_tool( return fallback() # A controller was selected: it is authoritative. Never retry through the - # existing backend, whatever happens here. - return broker.dispatch( + # existing backend, whatever happens here. Registry handlers must return a + # string (or the dedicated multimodal envelope), while controller transports + # naturally complete with decoded JSON values. Preserve existing string + # results byte-for-byte and serialize decoded values at this boundary. + result = broker.dispatch( scope, action=action, arguments=args, tool_call_id=tool_call_id ) + if isinstance(result, str): + return result + return json.dumps(result, ensure_ascii=False) def current_tool_call_id() -> str: @@ -149,7 +198,7 @@ def routed_browser_handler( ) -> Any: """Lazy registry-handler route wrapper for ``browser_*`` tools. - Resolves the Phase 4 feature flag and process-local broker lazily so the + Resolves the feature flag and process-local broker lazily so the default (feature off) path costs one cached config read and an immediate fallback, and so importing ``tools.browser_tool`` never imports the gateway. When the gateway cannot be imported or the feature is off, the diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 3cd08451f3..3d191a874c 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -5390,7 +5390,10 @@ if __name__ == "__main__": # Registry # --------------------------------------------------------------------------- from tools.registry import registry, tool_error -from tools.browser_extension_router import routed_browser_handler +from tools.browser_extension_router import ( + extension_controller_available, + routed_browser_handler, +) _BROWSER_SCHEMA_MAP = {s["name"]: s for s in BROWSER_TOOL_SCHEMAS} @@ -5403,6 +5406,39 @@ def _browser_router_kw(kw: dict) -> dict: } +def check_browser_routed_requirements(action: str = "browser_snapshot") -> bool: + """Availability gate for tools that can use either browser backend.""" + return check_browser_requirements() or extension_controller_available(action) + + +def check_browser_navigate_requirements() -> bool: + return check_browser_routed_requirements("browser_navigate") + + +def check_browser_snapshot_requirements() -> bool: + return check_browser_routed_requirements("browser_snapshot") + + +def check_browser_click_requirements() -> bool: + return check_browser_routed_requirements("browser_click") + + +def check_browser_type_requirements() -> bool: + return check_browser_routed_requirements("browser_type") + + +def check_browser_scroll_requirements() -> bool: + return check_browser_routed_requirements("browser_scroll") + + +def check_browser_back_requirements() -> bool: + return check_browser_routed_requirements("browser_back") + + +def check_browser_press_requirements() -> bool: + return check_browser_routed_requirements("browser_press") + + registry.register( name="browser_navigate", toolset="browser", @@ -5413,7 +5449,7 @@ registry.register( fallback=lambda: browser_navigate(url=args.get("url", ""), task_id=kw.get("task_id")), **_browser_router_kw(kw), ), - check_fn=check_browser_requirements, + check_fn=check_browser_navigate_requirements, emoji="🌐", ) registry.register( @@ -5427,7 +5463,7 @@ registry.register( full=args.get("full", False), task_id=kw.get("task_id"), user_task=kw.get("user_task")), **_browser_router_kw(kw), ), - check_fn=check_browser_requirements, + check_fn=check_browser_snapshot_requirements, emoji="📸", ) registry.register( @@ -5440,7 +5476,7 @@ registry.register( fallback=lambda: browser_click(ref=args.get("ref", ""), task_id=kw.get("task_id")), **_browser_router_kw(kw), ), - check_fn=check_browser_requirements, + check_fn=check_browser_click_requirements, emoji="👆", ) registry.register( @@ -5453,7 +5489,7 @@ registry.register( fallback=lambda: browser_type(ref=args.get("ref", ""), text=args.get("text", ""), task_id=kw.get("task_id")), **_browser_router_kw(kw), ), - check_fn=check_browser_requirements, + check_fn=check_browser_type_requirements, emoji="⌨️", ) registry.register( @@ -5466,7 +5502,7 @@ registry.register( fallback=lambda: browser_scroll(direction=args.get("direction", "down"), task_id=kw.get("task_id")), **_browser_router_kw(kw), ), - check_fn=check_browser_requirements, + check_fn=check_browser_scroll_requirements, emoji="📜", ) registry.register( @@ -5479,7 +5515,7 @@ registry.register( fallback=lambda: browser_back(task_id=kw.get("task_id")), **_browser_router_kw(kw), ), - check_fn=check_browser_requirements, + check_fn=check_browser_back_requirements, emoji="◀️", ) registry.register( @@ -5492,7 +5528,7 @@ registry.register( fallback=lambda: browser_press(key=args.get("key", ""), task_id=kw.get("task_id")), **_browser_router_kw(kw), ), - check_fn=check_browser_requirements, + check_fn=check_browser_press_requirements, emoji="⌨️", ) diff --git a/tools/registry.py b/tools/registry.py index 16fbc071a8..bf6d52f2ee 100644 --- a/tools/registry.py +++ b/tools/registry.py @@ -305,11 +305,30 @@ def _prune_check_fn_caches(now: float) -> None: def check_fn_cache_scope() -> Optional[str]: """Return the active profile key when availability is profile-scoped. + Browser-controller availability is request-bound and can change on every + attach/detach. A fully bound browser-control request therefore bypasses both + this check cache and model_tools' outer definition cache; the same sentinel + is consumed by both layers. This prevents one Browser session's live tools + from leaking into any unrelated session. + Single-profile processes intentionally keep the historical process-wide cache. A multiplex gateway installs a Hermes-home override for every profile turn, so the canonical profile key is the stable isolation boundary across repeated turns for that profile. """ + try: + from gateway.session_context import get_session_env + + browser_identity = ( + get_session_env("HERMES_SESSION_ID", ""), + get_session_env("HERMES_BROWSER_CONTROL_PRINCIPAL", ""), + get_session_env("HERMES_BROWSER_CONTROL_TRANSPORT_FAMILY", ""), + ) + if all(str(value or "").strip() for value in browser_identity): + return CHECK_FN_CACHE_BYPASS + except Exception: + pass + try: from agent.secret_scope import is_multiplex_active diff --git a/tui_gateway/methods_browser_control.py b/tui_gateway/methods_browser_control.py index 8ce1c53fba..98819a3e2f 100644 --- a/tui_gateway/methods_browser_control.py +++ b/tui_gateway/methods_browser_control.py @@ -1,4 +1,4 @@ -"""Browser controller registration / result routing (Phase 4 Cloud). +"""Browser controller registration and result routing for the dashboard. The dashboard's browser controller (the extension that physically drives a browser) registers itself over the authenticated ``/api/ws`` JSON-RPC @@ -19,9 +19,9 @@ when the request arrives on the same transport that owns the session, and only for the exact attached scope — the broker's exact-scope ``complete`` is the last line of defense against cross-tenant completion. -Phase 4 is deliberately minimal: the only capability a controller may hold is -``controller.noop``, exercised end-to-end by -``tests/gateway/test_browser_control_cloud.py``. +Both dashboard and local API transports use the broker's shared, explicit +capability allowlist. Raw CDP, script evaluation, console access, uploads, and +other privileged surfaces are not controller capabilities. Note on handler globals: ``HandlerRegistry.install`` (method_ctx.py) rebinds each handler's ``__globals__`` onto server.py's namespace, so handler bodies @@ -36,6 +36,12 @@ from __future__ import annotations import hashlib import logging +from gateway.browser_control_broker import ( + BROWSER_CONTROL_PROTOCOL_VERSION, + browser_control_protocol_supported, + filter_browser_control_capabilities, +) + from .method_ctx import HandlerRegistry logger = logging.getLogger(__name__) @@ -43,11 +49,6 @@ logger = logging.getLogger(__name__) _registry = HandlerRegistry() method = _registry.method -#: Capabilities a Cloud/dashboard controller may register in Phase 4. Any -#: capability outside this set is silently filtered out (fail closed: an -#: empty intersection rejects the registration). -_CONTROLLER_CAPABILITIES = frozenset({"controller.noop"}) - #: Transport family stamped into every scope attached from this gateway. The #: broker's exact-match contract treats it as an identity field, so an API #: transport can never address a dashboard controller (and vice versa). @@ -131,7 +132,9 @@ def _( rid, params: dict, _family=_CLOUD_TRANSPORT_FAMILY, - _caps=_CONTROLLER_CAPABILITIES, + _protocol_version=BROWSER_CONTROL_PROTOCOL_VERSION, + _protocol_supported=browser_control_protocol_supported, + _filter_capabilities=filter_browser_control_capabilities, _forbidden=_ERR_FORBIDDEN, _identity_ok=_is_authenticated_identity, _digest=_principal_digest, @@ -146,8 +149,7 @@ def _( identity (``WSTransport.auth_identity`` — never the RPC params); * the named session exists in the live session registry and its ``transport`` is exactly the calling transport; - * at least one requested capability survives the filter to - ``controller.noop``. + * at least one requested capability survives the shared allowlist. The returned ``scope`` names a server-derived ``principal_id``, the ``cloud-ticket-ws`` transport family, and the filtered capability set. @@ -161,6 +163,13 @@ def _( "browser.extension_control.enabled is not set", ) + if not _protocol_supported(params.get("protocol_version")): + return _err( + rid, + _forbidden, + f"unsupported browser-control protocol version; expected {_protocol_version}", + ) + transport = current_transport() identity = getattr(transport, "auth_identity", None) if not _identity_ok(identity): @@ -191,8 +200,7 @@ def _( "controller_id, browser_profile_id, and server session profile are required", ) - requested = params.get("capabilities") or [] - capabilities = frozenset(cap for cap in requested if cap in _caps) + capabilities = _filter_capabilities(params.get("capabilities")) if not capabilities: return _err( rid, @@ -285,8 +293,7 @@ def _( # Defense in depth: the exact-scope complete below already rejects any # foreign scope, but the owner check makes the "same transport" rule # explicit at this layer too. - controller = broker.select(scope, "controller.noop") - if controller is None or controller.owner is not transport: + if not broker.is_owner(scope, transport): return _err( rid, _forbidden, @@ -333,8 +340,7 @@ def _( ) if scope is None: return _err(rid, _forbidden, "no controller registered for this session") - controller = broker.select(scope, "controller.noop") - if controller is None or controller.owner is not transport: + if not broker.is_owner(scope, transport): return _err(rid, _forbidden, "controller is not owned by this transport") return _ok(rid, {"ok": True}) diff --git a/tui_gateway/ws.py b/tui_gateway/ws.py index 5ac36b5676..47d0f9b1ef 100644 --- a/tui_gateway/ws.py +++ b/tui_gateway/ws.py @@ -463,7 +463,7 @@ async def handle_ws( server.unregister_live_transport(transport) # Owner-safely detach browser controllers this transport - # registered (Phase 4 Cloud). The socket itself is closing, so no + # registered. The socket itself is closing, so no # peer cancel write is attempted; every server-side pending command # is still failed closed immediately. try: diff --git a/website/docs/developer-guide/programmatic-integration.md b/website/docs/developer-guide/programmatic-integration.md index 42a603215e..7e9c1f6c3b 100644 --- a/website/docs/developer-guide/programmatic-integration.md +++ b/website/docs/developer-guide/programmatic-integration.md @@ -112,6 +112,8 @@ POST /v1/runs/{id}/approval Resolve a pending approval POST /v1/runs/{id}/steer Inject mid-run guidance at the next tool boundary POST /v1/runs/{id}/stop Interrupt the run GET /v1/capabilities Machine-readable feature flags +POST /v1/browser-control/register Register a browser controller +GET /v1/browser-control/ws Browser-controller WebSocket GET /v1/models Lists hermes-agent GET /api/model/options Provider-aware picker inventory GET /health, /health/detailed @@ -119,6 +121,11 @@ GET /health, /health/detailed Setup, headers (`X-Hermes-Session-Id`, `X-Hermes-Session-Key`), and frontend wiring: [API Server](../user-guide/features/api-server). +Browser extensions can opt into the disabled-by-default controller protocol to +drive the exact browser session that opened the Hermes conversation. The API +and dashboard transports share one principal-bound broker and one explicit +capability allowlist; see [Browser-extension control](../user-guide/features/api-server#browser-extension-control). + ### Model catalog surfaces The OpenAI-compatible API intentionally keeps `GET /v1/models` minimal: it is diff --git a/website/docs/user-guide/features/api-server.md b/website/docs/user-guide/features/api-server.md index ccba76e104..66e7f9bc92 100644 --- a/website/docs/user-guide/features/api-server.md +++ b/website/docs/user-guide/features/api-server.md @@ -259,6 +259,75 @@ Returns a machine-readable description of the API server's stable surface for ex Use this endpoint when integrating dashboards, browser UIs, or control planes so they can discover whether the running Hermes version supports runs, streaming, cancellation, and session continuity without depending on private Python internals. +## Browser-extension control + +Hermes can route browser tools through an authenticated extension that controls +the browser session associated with the current Hermes session. The feature is +disabled by default; set `browser.extension_control.enabled` to `true` to opt in: + +```yaml +browser: + extension_control: + enabled: true +``` + +The local API path also requires the API server bearer key. A controller may +register only for an existing server session. Hermes derives the controller +principal from authenticated server state; a client-supplied `principal_id` is +ignored. + +Discover the live contract through `GET /v1/capabilities`. The +`browser_extension_control` object reports whether the feature is enabled, the +protocol version, transport names, and the exact capability allowlist: + +```text +controller.noop +browser_back +browser_click +browser_navigate +browser_press +browser_screenshot +browser_scroll +browser_snapshot +browser_tab_activate +browser_tabs +browser_type +``` + +Requested capabilities outside that list are filtered out. Raw CDP, arbitrary +script evaluation, console access, uploads, image extraction, and vision are not +part of the controller protocol. + +### Local API registration + +1. Send an authenticated `POST /v1/browser-control/register` with + `protocol_version`, `session_id`, `controller_id`, `browser_profile_id`, and + the requested `capabilities`. +2. Hermes returns a single-use ticket with a 30-second TTL and the filtered, + server-bound controller scope. +3. Open `GET /v1/browser-control/ws` with both WebSocket subprotocols: + `hermes-browser-control-v1` and + `hermes-browser-control-ticket.`. + +The ticket is never accepted in the query string. Unknown, expired, reused, or +malformed tickets fail before WebSocket upgrade. + +### Controller frames + +Hermes sends `browser.controller.command` frames containing `command_id`, +`action`, immutable `arguments`, browser/controller ids, and the originating +`tool_call_id`. The controller replies with `browser.controller.result`, the +same `command_id`, an exact boolean `ok`, and either `result` or `error`. +Cancellation and timeout emit `browser.controller.cancel`; late results are +ignored. + +The authenticated dashboard transport exposes the same registration, result, +heartbeat, capability, and ownership semantics over its Gateway RPC/event +channel. In both transports, selection requires one unambiguous exact match on +principal, profile, session, controller, browser profile, transport family, and +capability. Once selected, a controller failure is authoritative and is never +retried through a different browser backend. + ## Per-request model selection Authenticated clients can override Hermes' default model selection per request