diff --git a/gateway/platforms/api_server.py b/gateway/platforms/api_server.py index 33efd02172..127fafd6b5 100644 --- a/gateway/platforms/api_server.py +++ b/gateway/platforms/api_server.py @@ -80,10 +80,21 @@ _api_request_browser_control_transport_family: ContextVar[str] = ContextVar( #: Phase 4 browser-extension control protocol version (advertised in #: /v1/capabilities and echoed in registration responses). _BROWSER_CONTROL_PROTOCOL_VERSION = 1 -#: Capabilities this phase actually grants a controller. Only the no-op -#: probe is real until the action protocol ships; any requested capability -#: outside this set is filtered out rather than advertised. -_BROWSER_CONTROL_CAPABILITIES = frozenset({"controller.noop"}) +#: 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." @@ -3256,16 +3267,14 @@ class APIServerAdapter(BasePlatformAdapter): "session_continuity_header": "X-Hermes-Session-Id", "session_key_header": "X-Hermes-Session-Key", "cors": bool(self._cors_origins), - # Phase 4 browser-extension control. Always advertised (so - # clients can feature-detect), but truthful: disabled until - # browser.extension_control.enabled is set, and Phase 4 - # exposes no real browser actions — only the no-op controller - # capability is ever granted. + # Browser-extension control is always advertised so clients + # can feature-detect it, but remains disabled until + # browser.extension_control.enabled is explicitly set. "browser_extension_control": { "enabled": self._browser_control_enabled(), "protocol_version": _BROWSER_CONTROL_PROTOCOL_VERSION, "capabilities": list(_BROWSER_CONTROL_CAPABILITIES), - "real_browser_actions": False, + "real_browser_actions": True, "transports": { "local_vps": "websocket-subprotocol-ticket", "cloud": "authenticated-gateway-rpc", @@ -3314,8 +3323,8 @@ 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 what this phase - actually grants (``controller.noop``), so a spoofed + digest, and the capability set is filtered to the exact Phase 6 + 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 the active profile's server-owned SessionDB before a ticket is minted. @@ -3514,7 +3523,9 @@ class APIServerAdapter(BasePlatformAdapter): except Exception: continue if isinstance(frame, dict): - self._handle_browser_control_frame(scope, frame) + reply = self._handle_browser_control_frame(scope, frame) + if isinstance(reply, dict): + await ws.send_json(reply) elif msg.type in (web.WSMsgType.CLOSE, web.WSMsgType.ERROR): break finally: @@ -3531,6 +3542,17 @@ class APIServerAdapter(BasePlatformAdapter): params = frame.get("params") if not isinstance(params, dict): return + if method == "browser.controller.heartbeat": + nonce = str(params.get("nonce") or "").strip() + if not nonce or len(nonce) > 128: + return + # Echo only the caller's opaque nonce on the already authenticated, + # exact-scope controller socket. This proves the socket path is live + # without granting a new capability or touching broker commands. + return { + "method": "browser.controller.heartbeat", + "params": {"nonce": nonce, "ok": True}, + } if method == "browser.controller.result": command_id = params.get("command_id") if isinstance(command_id, str) and command_id: diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 22dd7b56a1..a7dd5fe4b0 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -15961,6 +15961,26 @@ def _ws_auth_mode() -> str: return "loopback" +_GATEWAY_WS_PROTOCOL = "hermes-gateway-v1" +_GATEWAY_WS_TICKET_PROTOCOL_PREFIX = "hermes-gateway-ticket." + + +def _gateway_ws_ticket_from_subprotocol(ws: "WebSocket") -> tuple[str, str]: + """Return ``(ticket, reason)`` from an unambiguous gateway protocol set.""" + raw = str(ws.headers.get("sec-websocket-protocol", "") or "") + protocols = [value.strip() for value in raw.split(",") if value.strip()] + ticket_protocols = [ + value for value in protocols + if value.startswith(_GATEWAY_WS_TICKET_PROTOCOL_PREFIX) + ] + if not ticket_protocols: + return "", "none" + if _GATEWAY_WS_PROTOCOL not in protocols or len(ticket_protocols) != 1: + return "", "invalid" + ticket = ticket_protocols[0][len(_GATEWAY_WS_TICKET_PROTOCOL_PREFIX):] + return (ticket, "ok") if ticket else ("", "invalid") + + def _ws_auth_reason(ws: "WebSocket") -> tuple[Optional[str], str]: """Validate WS-upgrade auth; return ``(reason, credential)``. @@ -16030,7 +16050,10 @@ def _ws_auth_reason(ws: "WebSocket") -> tuple[Optional[str], str]: ) return "internal_invalid", "internal" - ticket = ws.query_params.get("ticket", "") + protocol_ticket, protocol_reason = _gateway_ws_ticket_from_subprotocol(ws) + if protocol_reason == "invalid": + return "ticket_invalid", "ticket-subprotocol" + ticket = protocol_ticket or ws.query_params.get("ticket", "") if not ticket: return "no_credential", "none" @@ -16047,6 +16070,12 @@ def _ws_auth_reason(ws: "WebSocket") -> tuple[Optional[str], str]: "user_id": info.get("user_id"), "provider": info.get("provider"), } + if protocol_ticket: + # Select only the stable public protocol during accept. The + # ticket-bearing protocol is a credential and must never be + # reflected back to the browser or retained after admission. + ws._hermes_ws_subprotocol = _GATEWAY_WS_PROTOCOL + return None, "ticket-subprotocol" return None, "ticket" except TicketInvalid as exc: audit_log( @@ -17171,7 +17200,11 @@ async def gateway_ws(ws: WebSocket) -> None: # onto the WS object by _ws_auth_reason; carry it into the gateway # transport where it becomes the identity authority for privileged RPCs # (browser.controller.register). None on the legacy token path. - await handle_ws(ws, auth_identity=getattr(ws, "_hermes_auth_identity", None)) + await handle_ws( + ws, + auth_identity=getattr(ws, "_hermes_auth_identity", None), + subprotocol=getattr(ws, "_hermes_ws_subprotocol", None), + ) # --------------------------------------------------------------------------- diff --git a/tests/gateway/test_browser_control_api.py b/tests/gateway/test_browser_control_api.py index d47d98e845..7421df2025 100644 --- a/tests/gateway/test_browser_control_api.py +++ b/tests/gateway/test_browser_control_api.py @@ -12,6 +12,18 @@ from gateway.platforms.api_server import APIServerAdapter API_KEY = "-".join(("fixture", "neutral", "api", "key", "123")) CONTROL_PROTOCOL = "hermes-browser-control-v1" +REAL_BROWSER_CAPABILITIES = { + "browser_back", + "browser_click", + "browser_navigate", + "browser_press", + "browser_screenshot", + "browser_scroll", + "browser_snapshot", + "browser_tab_activate", + "browser_tabs", + "browser_type", +} class _SessionDB: @@ -70,6 +82,33 @@ def _registration_body(**overrides): return payload +@pytest.mark.asyncio +async def test_phase6_registration_grants_only_the_exact_real_action_allowlist(monkeypatch): + adapter = _adapter() + monkeypatch.setattr(adapter, "_browser_control_enabled", lambda: True) + requested = [ + "controller.noop", + *sorted(REAL_BROWSER_CAPABILITIES), + "browser_cdp", + "browser_evaluate", + "browser_upload", + "arbitrary.capability", + ] + async with TestClient(TestServer(_app(adapter))) as client: + response = await client.post( + "/v1/browser-control/register", + json=_registration_body(capabilities=requested), + headers={"Authorization": f"Bearer {API_KEY}"}, + ) + assert response.status == 201 + registration = await response.json() + + assert set(registration["scope"]["capabilities"]) == { + "controller.noop", + *REAL_BROWSER_CAPABILITIES, + } + + 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()} @@ -112,15 +151,13 @@ async def test_capabilities_are_truthful_and_disabled_by_default(monkeypatch): data = await response.json() control = data["features"]["browser_extension_control"] - assert control == { - "enabled": False, - "protocol_version": 1, - "capabilities": ["controller.noop"], - "real_browser_actions": False, - "transports": { - "local_vps": "websocket-subprotocol-ticket", - "cloud": "authenticated-gateway-rpc", - }, + assert control["enabled"] is False + assert control["protocol_version"] == 1 + assert set(control["capabilities"]) == {"controller.noop", *REAL_BROWSER_CAPABILITIES} + assert control["real_browser_actions"] is True + assert control["transports"] == { + "local_vps": "websocket-subprotocol-ticket", + "cloud": "authenticated-gateway-rpc", } assert data["endpoints"]["browser_control_register"] == { "method": "POST", @@ -260,12 +297,26 @@ async def test_local_api_ticket_ws_noop_round_trip_filters_spoofed_identity_and_ assert registration["ws_path"] == "/v1/browser-control/ws" assert registration["scope"]["principal_id"] != "spoofed-client-principal" assert registration["scope"]["transport_family"] == "local-api" - assert registration["scope"]["capabilities"] == ["controller.noop"] + assert set(registration["scope"]["capabilities"]) == { + "controller.noop", + "browser_navigate", + } ws = await client.ws_connect( "/v1/browser-control/ws", protocols=[CONTROL_PROTOCOL, _ticket_protocol(registration["ticket"])], ) + await ws.send_json( + { + "method": "browser.controller.heartbeat", + "params": {"nonce": "heartbeat-api-fixture"}, + } + ) + heartbeat = await ws.receive_json(timeout=2.0) + assert heartbeat == { + "method": "browser.controller.heartbeat", + "params": {"nonce": "heartbeat-api-fixture", "ok": True}, + } scope = ControllerScope( principal_id=registration["scope"]["principal_id"], profile_id=registration["scope"]["profile_id"], diff --git a/tests/gateway/test_browser_control_cloud.py b/tests/gateway/test_browser_control_cloud.py index 0d40941045..6dee92f12c 100644 --- a/tests/gateway/test_browser_control_cloud.py +++ b/tests/gateway/test_browser_control_cloud.py @@ -14,6 +14,21 @@ from tui_gateway.methods_browser_control import _broker_event_writer, _principal def _fake_ticket_ws(ticket): return SimpleNamespace( query_params={"ticket": ticket}, + headers={}, + client=SimpleNamespace(host="203.0.113.7"), + url=SimpleNamespace(path="/api/ws"), + ) + + +def _fake_ticket_subprotocol_ws(ticket): + return SimpleNamespace( + query_params={}, + headers={ + "sec-websocket-protocol": ( + f"{web_server._GATEWAY_WS_PROTOCOL}, " + f"{web_server._GATEWAY_WS_TICKET_PROTOCOL_PREFIX}{ticket}" + ) + }, client=SimpleNamespace(host="203.0.113.7"), url=SimpleNamespace(path="/api/ws"), ) @@ -43,6 +58,19 @@ def test_dashboard_ticket_identity_is_carried_forward_without_trusting_rpc_param assert web_server._ws_auth_ok(_fake_ticket_ws(ticket)) is False +def test_dashboard_ticket_subprotocol_carries_the_same_server_identity(gated_dashboard): + _reset_for_tests() + ticket = mint_ticket(user_id="subprotocol-user", provider="provider-fixture") + ws = _fake_ticket_subprotocol_ws(ticket) + + assert web_server._ws_auth_ok(ws) is True + assert ws._hermes_auth_identity == { + "user_id": "subprotocol-user", + "provider": "provider-fixture", + } + assert ws._hermes_ws_subprotocol == web_server._GATEWAY_WS_PROTOCOL + + def test_ws_transport_records_only_server_authenticated_identity(): loop = SimpleNamespace() identity = {"user_id": "user-fixture", "provider": "provider-fixture"} @@ -216,6 +244,28 @@ def test_cloud_gateway_noop_round_trip_is_bound_to_ticket_identity_and_session_t transport_family="cloud-ticket-ws", ) assert scope is not None + heartbeat_response = server.dispatch( + { + "jsonrpc": "2.0", + "id": 42, + "method": "browser.controller.heartbeat", + "params": {"session_id": "session-fixture"}, + }, + transport, + ) + assert heartbeat_response["result"] == {"ok": True} + + foreign_transport = Transport() + foreign_heartbeat = server.dispatch( + { + "jsonrpc": "2.0", + "id": 43, + "method": "browser.controller.heartbeat", + "params": {"session_id": "session-fixture"}, + }, + foreign_transport, + ) + assert foreign_heartbeat["error"]["code"] == 4403 outcome = {} def dispatch_noop(): diff --git a/tests/hermes_cli/test_dashboard_auth_ws_auth.py b/tests/hermes_cli/test_dashboard_auth_ws_auth.py index 8d590c08a5..4814ca30ef 100644 --- a/tests/hermes_cli/test_dashboard_auth_ws_auth.py +++ b/tests/hermes_cli/test_dashboard_auth_ws_auth.py @@ -176,7 +176,13 @@ def insecure_explicit_host_app(): web_server.app.state.auth_required = prev_required -def _fake_ws(*, query: dict, client_host: str = "127.0.0.1", path: str = "/api/pty"): +def _fake_ws( + *, + query: dict, + client_host: str = "127.0.0.1", + path: str = "/api/pty", + protocols: tuple[str, ...] = (), +): """Build a stand-in for starlette.WebSocket good enough for _ws_auth_ok.""" class _QP: @@ -188,6 +194,7 @@ def _fake_ws(*, query: dict, client_host: str = "127.0.0.1", path: str = "/api/p return SimpleNamespace( query_params=_QP(query), + headers={"sec-websocket-protocol": ", ".join(protocols)} if protocols else {}, client=SimpleNamespace(host=client_host), url=SimpleNamespace(path=path), ) @@ -213,6 +220,45 @@ class TestWsAuthOkGated: # Single-use — second consumption fails. assert web_server._ws_auth_ok(ws_two) is False + def test_ticket_subprotocol_is_single_use_and_selects_only_the_public_protocol(self, gated_app): + ticket = mint_ticket(user_id="subprotocol-user", provider="stub") + protocols = ( + web_server._GATEWAY_WS_PROTOCOL, + f"{web_server._GATEWAY_WS_TICKET_PROTOCOL_PREFIX}{ticket}", + ) + ws_one = _fake_ws(query={}, path="/api/ws", protocols=protocols) + ws_two = _fake_ws(query={}, path="/api/ws", protocols=protocols) + + assert web_server._ws_auth_ok(ws_one) is True + assert ws_one._hermes_auth_identity == { + "user_id": "subprotocol-user", + "provider": "stub", + } + assert ws_one._hermes_ws_subprotocol == web_server._GATEWAY_WS_PROTOCOL + assert ticket not in ws_one._hermes_ws_subprotocol + assert web_server._ws_auth_ok(ws_two) is False + + def test_ticket_subprotocol_rejects_missing_public_protocol_or_ambiguous_tickets(self, gated_app): + first = mint_ticket(user_id="u1", provider="stub") + missing_public = _fake_ws( + query={}, + path="/api/ws", + protocols=(f"hermes-gateway-ticket.{first}",), + ) + assert web_server._ws_auth_ok(missing_public) is False + + second = mint_ticket(user_id="u2", provider="stub") + ambiguous = _fake_ws( + query={}, + path="/api/ws", + protocols=( + "hermes-gateway-v1", + f"hermes-gateway-ticket.{first}", + f"hermes-gateway-ticket.{second}", + ), + ) + assert web_server._ws_auth_ok(ambiguous) is False + def test_legacy_token_rejected_in_gated_mode(self, gated_app): """Critical: gated mode must NOT honour the legacy token path diff --git a/tui_gateway/methods_browser_control.py b/tui_gateway/methods_browser_control.py index ef0d7240ef..8ce1c53fba 100644 --- a/tui_gateway/methods_browser_control.py +++ b/tui_gateway/methods_browser_control.py @@ -303,6 +303,42 @@ def _( return _ok(rid, {"accepted": accepted}) +@method("browser.controller.heartbeat") +def _( + rid, + params: dict, + _family=_CLOUD_TRANSPORT_FAMILY, + _forbidden=_ERR_FORBIDDEN, + _identity_ok=_is_authenticated_identity, + _digest=_principal_digest, +) -> dict: + """Acknowledge a heartbeat only for this transport's attached controller.""" + from gateway import browser_control_broker + + transport = current_transport() + identity = getattr(transport, "auth_identity", None) + if not _identity_ok(identity): + return _err(rid, _forbidden, "authenticated controller identity required") + session_id = str(params.get("session_id") or "") + with _sessions_lock: + session = _sessions.get(session_id) + if session is None or session.get("transport") is not transport: + return _err(rid, _forbidden, "session is not owned by this transport") + + broker = browser_control_broker.get_browser_control_broker() + scope = broker.scope_for_session( + session_id=session_id, + principal_id=_digest(identity), + transport_family=_family, + ) + 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: + return _err(rid, _forbidden, "controller is not owned by this transport") + return _ok(rid, {"ok": True}) + + def register(server) -> None: """Bind this module's handlers onto ``server``'s globals and registry.""" _registry.install(server) diff --git a/tui_gateway/ws.py b/tui_gateway/ws.py index 1f2c7b0316..5ac36b5676 100644 --- a/tui_gateway/ws.py +++ b/tui_gateway/ws.py @@ -292,7 +292,12 @@ def _disable_nagle(ws: Any) -> None: _log.debug("ws TCP_NODELAY skip: %s", exc) -async def handle_ws(ws: Any, *, auth_identity: dict | None = None) -> None: +async def handle_ws( + ws: Any, + *, + auth_identity: dict | None = None, + subprotocol: str | None = None, +) -> None: """Run one WebSocket session. Wire-compatible with ``tui_gateway.entry``. *auth_identity* is the server-minted ``{user_id, provider}`` recorded at @@ -311,7 +316,10 @@ async def handle_ws(ws: Any, *, auth_identity: dict | None = None) -> None: disconnect_reason = "not_connected" try: - await ws.accept() + if subprotocol: + await ws.accept(subprotocol=subprotocol) + else: + await ws.accept() disconnect_reason = "connected" # Push small streamed frames out immediately instead of letting Nagle # batch them — keeps the live token cadence intact for GUI clients.