diff --git a/hermes_cli/dashboard_auth/ws_tickets.py b/hermes_cli/dashboard_auth/ws_tickets.py index 118a988e142a..7c44cefb5470 100644 --- a/hermes_cli/dashboard_auth/ws_tickets.py +++ b/hermes_cli/dashboard_auth/ws_tickets.py @@ -10,19 +10,13 @@ ``POST /api/auth/ws-ticket`` and passes it as ``?ticket=`` on the WS upgrade. Single-use, TTL = 30 seconds — a leaked ticket is uninteresting. -2. **A process-lifetime internal credential** (``internal_ws_credential`` / - ``consume_internal_credential``). This authenticates *server-spawned* - WS clients — specifically the embedded-TUI PTY child, which attaches to - ``/api/ws`` (JSON-RPC gateway) and ``/api/pub`` (event sidecar) over - loopback. A single-use 30s ticket is the wrong shape for that link: the - child reads its attach URL once at startup and **reuses it on every - reconnect**, and on a slow cold boot the child may not dial within 30s. - The internal credential is minted once per process, never expires, is - multi-use, and — critically — is **never injected into any HTML/SPA**: - it only ever leaves the process via the spawned child's environment, so - browser-side XSS cannot read it. A leaked internal credential grants no - more than a single-use ticket already does (the same two internal WS - endpoints), and the same Origin / host guards still apply downstream. +2. **Audience-bound internal capabilities** (``internal_ws_credential`` / + ``consume_internal_credential``). These authenticate *server-spawned* WS + clients over loopback. A process-local random root derives distinct, + multi-use capabilities for the JSON-RPC gateway and each profile/channel + event sidecar. The root never leaves this module, and a sidecar capability + cannot be rebound to the broader gateway route. The derived values are + never injected into HTML or returned by a REST endpoint. In-memory; the dashboard is a single process so no distributed coordination is needed. The module exposes a small functional API rather than a class so @@ -31,6 +25,10 @@ from __future__ import annotations +import base64 +import hashlib +import hmac +import json import secrets import threading import time @@ -44,10 +42,10 @@ _lock = threading.Lock() _tickets: Dict[str, Tuple[int, Dict[str, Any]]] = {} # ticket -> (expires_at, info) -#: The process-lifetime internal credential (see module docstring). Lazily -#: minted on first ``internal_ws_credential()`` call and stable for the life -#: of the process. Guarded by ``_lock``. -_internal_credential: Optional[str] = None +#: Process-local derivation key for audience-bound internal capabilities. The +#: key itself never leaves this module; only HMAC-derived values are passed to +#: the server-spawned TUI process. Guarded by ``_lock``. +_internal_credential_key: Optional[bytes] = None #: Identity recorded for connections that authenticate via the internal #: credential, so audit logs distinguish them from browser-initiated tickets. @@ -107,44 +105,51 @@ def _gc_expired_locked() -> None: _tickets.pop(t, None) -def internal_ws_credential() -> str: - """Return the process-lifetime internal WS credential, minting it once. +def _capability_value(key: bytes, *, audience: str, binding: str) -> str: + """Derive one opaque capability from the process key and its audience.""" + payload = json.dumps( + [audience, binding], ensure_ascii=False, separators=(",", ":") + ).encode("utf-8") + digest = hmac.new(key, payload, hashlib.sha256).digest() + return base64.urlsafe_b64encode(digest).decode("ascii").rstrip("=") - Used by the server to authenticate WS clients it spawns itself (the - embedded-TUI PTY child). The value is stable for the life of the process, - multi-use, and never expires — so a server-spawned child can reconnect - its ``/api/ws`` / ``/api/pub`` sockets indefinitely without re-minting. - The credential is never injected into the SPA HTML or returned over any - REST endpoint; it is only ever passed to a child process via its - environment. See the module docstring for the threat-model rationale. +def internal_ws_credential(*, audience: str, binding: str = "") -> str: + """Return a stable process-lifetime capability for one internal audience. + + ``audience`` identifies the accepting route class (currently ``gateway`` + or ``sidecar``). ``binding`` narrows that class further, for example to a + profile/channel pair. The derived value is stable and multi-use so a child + can reconnect, but changing either field produces a different value. """ - global _internal_credential - with _lock: - if _internal_credential is None: - _internal_credential = secrets.token_urlsafe(32) - return _internal_credential + if not audience: + raise ValueError("internal credential audience is required") + global _internal_credential_key + with _lock: + if _internal_credential_key is None: + _internal_credential_key = secrets.token_bytes(32) + key = _internal_credential_key + return _capability_value(key, audience=audience, binding=binding) -def consume_internal_credential(value: str) -> Dict[str, Any]: - """Validate an internal credential. Raises :class:`TicketInvalid` on mismatch. - Unlike :func:`consume_ticket` this is **not** single-use — the value is - not removed on success, so a server-spawned child can present it on every - (re)connect. Returns the fixed server-internal identity ``info`` dict - (``{user_id, provider}``), mirroring the ``info`` shape ``consume_ticket`` - returns, so a caller that wants to record the connecting identity can; the - current ``_ws_auth_ok`` caller validates for the boolean outcome only and - discards the dict. +def consume_internal_credential( + value: str, *, audience: str, binding: str = "" +) -> Dict[str, Any]: + """Validate an audience-bound capability without consuming it. - A constant-time compare against the (lazily-minted) credential avoids - leaking length / prefix information on mismatch. If no internal - credential has been minted yet, any value is rejected. + Unlike :func:`consume_ticket`, a successful value remains valid for + reconnects. Validation derives the expected value for the exact audience + and binding and compares it in constant time. A capability minted for a + different route, profile, or channel therefore fails closed. """ + if not audience: + raise TicketInvalid("internal credential audience missing") with _lock: - expected = _internal_credential - if not value or expected is None: + key = _internal_credential_key + if not value or key is None: raise TicketInvalid("no internal credential") + expected = _capability_value(key, audience=audience, binding=binding) if not secrets.compare_digest(value.encode(), expected.encode()): raise TicketInvalid("internal credential mismatch") return { @@ -155,7 +160,7 @@ def consume_internal_credential(value: str) -> Dict[str, Any]: def _reset_for_tests() -> None: """Test-only: drop all tickets and the internal credential.""" - global _internal_credential + global _internal_credential_key with _lock: _tickets.clear() - _internal_credential = None + _internal_credential_key = None diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index f6c43a550b8b..e2354614c298 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -16820,7 +16820,12 @@ def _ws_auth_mode() -> str: return "loopback" -def _ws_auth_reason(ws: "WebSocket") -> tuple[Optional[str], str]: +def _ws_auth_reason( + ws: "WebSocket", + *, + internal_audience: Optional[str] = None, + internal_binding: str = "", +) -> tuple[Optional[str], str]: """Validate WS-upgrade auth; return ``(reason, credential)``. ``reason`` is None when the credential is accepted, else a short @@ -16830,28 +16835,51 @@ def _ws_auth_reason(ws: "WebSocket") -> tuple[Optional[str], str]: ``internal``, ``token``, or ``none``) so the accepted path can log *how* a peer authed, not just that it did. - Loopback / ``--insecure``: legacy ``?token=<_SESSION_TOKEN>`` query - parameter, constant-time compared. - - Gated (public bind, no ``--insecure``): one of two credentials — + Browser credentials are mode-specific: - * ``?ticket=`` — a browser-minted, single-use, 30s-TTL ticket - consumed against the dashboard-auth ticket store. This is what the SPA - (and native clients) use. - * ``?internal=`` — the process-lifetime internal - credential, used only by WS clients the server spawns itself (the - embedded-TUI PTY child attaching to ``/api/ws`` and ``/api/pub``). It - is multi-use and never expires so the child can reconnect, and is never - injected into the SPA — see ``dashboard_auth.ws_tickets`` for the - threat model. + * Loopback / ``--insecure`` accepts the legacy + ``?token=<_SESSION_TOKEN>`` query parameter, constant-time compared. + * Gated public binds accept a browser-minted, single-use, 30s-TTL + ``?ticket=`` and reject the legacy token path. - The legacy ``?token=`` path is unconditionally rejected in gated mode - (the SPA bundle isn't carrying the token any longer, and a leaked - ``_SESSION_TOKEN`` must not grant WS access once the gate is engaged). + In every mode, ``?internal=`` is accepted only when the caller + supplies this route's expected audience and optional profile/channel + binding. Browser-facing and PTY routes pass no internal audience and + therefore reject internal capabilities. Audit-logs the rejection so operators can debug "WS keeps closing" issues from the log. """ + # Internal capabilities authenticate server-spawned loopback clients in + # every mode. Validate them before branching to the browser credential + # contract; unlike the legacy session token they are never injected into + # HTML and are constrained by the accepting route's explicit audience. + internal = ws.query_params.get("internal", "") + if internal: + from hermes_cli.dashboard_auth.audit import AuditEvent, audit_log + from hermes_cli.dashboard_auth.ws_tickets import ( + TicketInvalid, + consume_internal_credential, + ) + + if not internal_audience: + return "internal_wrong_audience", "internal" + try: + consume_internal_credential( + internal, + audience=internal_audience, + binding=internal_binding, + ) + return None, "internal" + except TicketInvalid as exc: + audit_log( + AuditEvent.WS_TICKET_REJECTED, + reason=f"internal: {exc}", + ip=(ws.client.host if ws.client else ""), + path=ws.url.path, + ) + return "internal_invalid", "internal" + auth_required = bool(getattr(app.state, "auth_required", False)) if auth_required: # Lazy import — keeps this function importable in test harnesses @@ -16859,27 +16887,9 @@ def _ws_auth_reason(ws: "WebSocket") -> tuple[Optional[str], str]: from hermes_cli.dashboard_auth.audit import AuditEvent, audit_log from hermes_cli.dashboard_auth.ws_tickets import ( TicketInvalid, - consume_internal_credential, consume_ticket, ) - # Server-spawned children (PTY child → /api/ws, /api/pub) present the - # multi-use internal credential rather than a single-use ticket, so - # they survive reconnects and slow cold boots. - internal = ws.query_params.get("internal", "") - if internal: - try: - consume_internal_credential(internal) - return None, "internal" - except TicketInvalid as exc: - audit_log( - AuditEvent.WS_TICKET_REJECTED, - reason=f"internal: {exc}", - ip=(ws.client.host if ws.client else ""), - path=ws.url.path, - ) - return "internal_invalid", "internal" - ticket = ws.query_params.get("ticket", "") if not ticket: return "no_credential", "none" @@ -16904,9 +16914,18 @@ def _ws_auth_reason(ws: "WebSocket") -> tuple[Optional[str], str]: return "token_mismatch", "token" -def _ws_auth_ok(ws: "WebSocket") -> bool: +def _ws_auth_ok( + ws: "WebSocket", + *, + internal_audience: Optional[str] = None, + internal_binding: str = "", +) -> bool: """True when the WS-upgrade credential is accepted. See _ws_auth_reason.""" - return _ws_auth_reason(ws)[0] is None + return _ws_auth_reason( + ws, + internal_audience=internal_audience, + internal_binding=internal_binding, + )[0] is None # Per-channel subscriber registry used by /api/pub (PTY-side gateway → dashboard) # and /api/events (dashboard → browser sidebar). Keyed by an opaque channel id @@ -16965,7 +16984,12 @@ def _resolve_chat_argv( profile_dir = _resolve_profile_dir(requested) argv, cwd = _make_tui_argv(PROJECT_ROOT / "ui-tui", tui_dev=False) - env = os.environ.copy() + from tools.environments.local import hermes_subprocess_env + + # Start from the centralized spawn policy rather than copying the dashboard + # process environment. This keeps infrastructure/session credentials out of + # the PTY child; the exact TUI capabilities are added below after sanitization. + env = hermes_subprocess_env(inherit_credentials=True) try: from hermes_cli.config import apply_terminal_config_to_env apply_terminal_config_to_env(env=env) @@ -17067,13 +17091,9 @@ def _resolve_client_ws_host() -> Optional[str]: def _build_gateway_ws_url() -> Optional[str]: """ws:// URL the PTY child should attach to for JSON-RPC gateway traffic. - Loopback / ``--insecure``: ``?token=<_SESSION_TOKEN>``. - - Gated mode: the legacy token path is rejected by ``_ws_auth_ok``, so the - server-spawned PTY child authenticates with the process-lifetime internal - credential (``?internal=``). It must NOT use a single-use browser ticket: - the child reads this URL once at startup and reuses it on every reconnect, - and a 30s-TTL ticket can expire before a slow cold boot even dials. + All modes use a gateway-audience internal capability. Browser credentials + retain their existing mode-specific behavior; the server child never needs + the broad legacy dashboard token and can reuse this URL on reconnect. """ host = _resolve_client_ws_host() port = getattr(app.state, "bound_port", None) @@ -17087,12 +17107,11 @@ def _build_gateway_ws_url() -> Optional[str]: else f"{host}:{port}" ) - if getattr(app.state, "auth_required", False): - from hermes_cli.dashboard_auth.ws_tickets import internal_ws_credential + from hermes_cli.dashboard_auth.ws_tickets import internal_ws_credential - qs = urllib.parse.urlencode({"internal": internal_ws_credential()}) - else: - qs = urllib.parse.urlencode({"token": _SESSION_TOKEN}) + qs = urllib.parse.urlencode( + {"internal": internal_ws_credential(audience="gateway")} + ) return f"ws://{netloc}/api/ws?{qs}" @@ -17128,19 +17147,19 @@ async def _resolve_chat_argv_async( ) -def _build_sidecar_url(channel: str) -> Optional[str]: - """ws:// URL the PTY child should publish events to, or None when unbound. +def _sidecar_capability_binding(channel: str, profile: Optional[str]) -> str: + """Return an unambiguous binding for one profile-scoped event publisher.""" + return f"{(profile or 'current').strip() or 'current'}\0{channel}" - Loopback / ``--insecure``: uses ``?token=<_SESSION_TOKEN>``. - Gated mode: authenticates with the process-lifetime internal credential - (``?internal=``), the same one ``_build_gateway_ws_url`` uses. The PTY - child is a server-spawned process we trust; the credential is multi-use - and never expires, so the child can reconnect ``/api/pub`` without a new - URL. (This previously minted a single-use 30s ticket, which meant the - child could not reconnect and could miss the window on a slow cold boot.) - Connections authenticated this way are recorded under the - ``server-internal`` identity in the audit log. +def _build_sidecar_url( + channel: str, *, profile: Optional[str] = None +) -> Optional[str]: + """ws:// URL the PTY child should publish events to, or None when unbound. + + Authenticates with a multi-use capability bound to the sidecar + audience plus the requested profile and channel. It cannot authenticate + the broader ``/api/ws`` gateway or another profile/channel publisher. """ host = _resolve_client_ws_host() port = getattr(app.state, "bound_port", None) @@ -17150,16 +17169,19 @@ def _build_sidecar_url(channel: str) -> Optional[str]: netloc = f"[{host}]:{port}" if ":" in host and not host.startswith("[") else f"{host}:{port}" - if getattr(app.state, "auth_required", False): - # Gated mode — use the internal credential so the WS upgrade survives - # _ws_auth_ok and the child can reconnect. - from hermes_cli.dashboard_auth.ws_tickets import internal_ws_credential + from hermes_cli.dashboard_auth.ws_tickets import internal_ws_credential - qs = urllib.parse.urlencode( - {"internal": internal_ws_credential(), "channel": channel} - ) - else: - qs = urllib.parse.urlencode({"token": _SESSION_TOKEN, "channel": channel}) + normalized_profile = (profile or "current").strip() or "current" + binding = _sidecar_capability_binding(channel, normalized_profile) + qs = urllib.parse.urlencode( + { + "internal": internal_ws_credential( + audience="sidecar", binding=binding + ), + "channel": channel, + "profile": normalized_profile, + } + ) return f"ws://{netloc}/api/pub?{qs}" @@ -17852,7 +17874,9 @@ async def pty_ws(ws: WebSocket) -> None: resume = raw_resume profile = ws.query_params.get("profile") or None channel = _channel_or_close_code(ws) - sidecar_url = _build_sidecar_url(channel) if channel else None + sidecar_url = ( + _build_sidecar_url(channel, profile=profile) if channel else None + ) force_fresh = (ws.query_params.get("fresh") or "").strip().lower() in { "1", "true", @@ -17988,7 +18012,7 @@ async def gateway_ws(ws: WebSocket) -> None: await ws.close(code=4403) return - if not _ws_auth_ok(ws): + if not _ws_auth_ok(ws, internal_audience="gateway"): await ws.close(code=4401) return @@ -18019,7 +18043,18 @@ async def pub_ws(ws: WebSocket) -> None: await ws.close(code=4403) return - if not _ws_auth_ok(ws): + channel = _channel_or_close_code(ws) + if not channel: + await ws.close(code=4400) + return + profile = (ws.query_params.get("profile") or "current").strip() or "current" + sidecar_binding = _sidecar_capability_binding(channel, profile) + + if not _ws_auth_ok( + ws, + internal_audience="sidecar", + internal_binding=sidecar_binding, + ): await ws.close(code=4401) return @@ -18027,11 +18062,6 @@ async def pub_ws(ws: WebSocket) -> None: await ws.close(code=4403) return - channel = _channel_or_close_code(ws) - if not channel: - await ws.close(code=4400) - return - await ws.accept() try: diff --git a/tests/dashboard/test_ws_client_host.py b/tests/dashboard/test_ws_client_host.py index 609941fb6408..c4106f004e2f 100644 --- a/tests/dashboard/test_ws_client_host.py +++ b/tests/dashboard/test_ws_client_host.py @@ -225,8 +225,8 @@ def test_wildcard_keeps_query_string( url = web_server._build_gateway_ws_url() assert url is not None assert "?" in url - # Loopback / ``--insecure`` path uses the session token. - assert f"token={web_server._SESSION_TOKEN}" in url + assert "internal=" in url + assert "token=" not in url def test_no_bound_host_returns_none( self, saved_app_state, clear_ws_host_env diff --git a/tests/hermes_cli/test_dashboard_auth_ws_auth.py b/tests/hermes_cli/test_dashboard_auth_ws_auth.py index 2d28bcf1dcfc..beb68a17fa8c 100644 --- a/tests/hermes_cli/test_dashboard_auth_ws_auth.py +++ b/tests/hermes_cli/test_dashboard_auth_ws_auth.py @@ -14,6 +14,7 @@ from __future__ import annotations from types import SimpleNamespace +from urllib.parse import parse_qs, urlsplit import pytest @@ -276,30 +277,43 @@ def test_rejection_audit_logs(self, gated_app, tmp_path, monkeypatch): def test_internal_credential_accepted(self, gated_app): """Server-spawned children present the process-lifetime internal - credential via ?internal= and are accepted in gated mode.""" - cred = internal_ws_credential() + gateway capability via ?internal= and are accepted by /api/ws.""" + cred = internal_ws_credential(audience="gateway") ws = _fake_ws(query={"internal": cred}) - assert web_server._ws_auth_ok(ws) is True + assert web_server._ws_auth_ok(ws, internal_audience="gateway") is True def test_internal_credential_is_multi_use(self, gated_app): """Unlike single-use tickets, the internal credential survives repeated use so the child can reconnect.""" - cred = internal_ws_credential() + cred = internal_ws_credential(audience="gateway") for _ in range(3): ws = _fake_ws(query={"internal": cred}) - assert web_server._ws_auth_ok(ws) is True + assert web_server._ws_auth_ok(ws, internal_audience="gateway") is True + + def test_internal_credential_requires_explicit_route_audience(self, gated_app): + cred = internal_ws_credential(audience="gateway") + ws = _fake_ws(query={"internal": cred}, path="/api/pty") + + assert web_server._ws_auth_ok(ws) is False def test_wrong_internal_credential_rejected(self, gated_app): # Mint the real one so the store is non-empty, then present a bogus value. - internal_ws_credential() + internal_ws_credential(audience="gateway") ws = _fake_ws(query={"internal": "not-the-internal-credential"}) - assert web_server._ws_auth_ok(ws) is False + assert web_server._ws_auth_ok(ws, internal_audience="gateway") is False + + def test_internal_gateway_capability_accepted_in_loopback(self, loopback_app): + """Server children use the same route-bound capability in every mode.""" + cred = internal_ws_credential(audience="gateway") + ws = _fake_ws(query={"internal": cred}) + assert web_server._ws_auth_ok(ws, internal_audience="gateway") is True - def test_internal_credential_not_accepted_in_loopback(self, loopback_app): - """Outside gated mode, ?internal= is meaningless — only ?token= works. - A naked internal credential must not authenticate.""" - cred = internal_ws_credential() + def test_internal_gateway_capability_still_requires_audience_in_loopback( + self, loopback_app + ): + cred = internal_ws_credential(audience="gateway") ws = _fake_ws(query={"internal": cred}) + assert web_server._ws_auth_ok(ws) is False @@ -546,26 +560,55 @@ def test_gated_same_host_https_origin_allowed(self, gated_app): class TestSidecarUrl: - def test_loopback_uses_session_token(self, loopback_app): + def test_loopback_uses_bound_internal_capability(self, loopback_app): url = web_server._build_sidecar_url("ch-1") assert url is not None - assert f"token={web_server._SESSION_TOKEN}" in url + assert "token=" not in url assert "ticket=" not in url - - def test_gated_uses_internal_credential(self, gated_app): - url = web_server._build_sidecar_url("ch-1") + query = parse_qs(urlsplit(url).query) + assert query["profile"] == ["current"] + binding = web_server._sidecar_capability_binding("ch-1", "current") + assert consume_internal_credential( + query["internal"][0], audience="sidecar", binding=binding + )["provider"] == "server-internal" + + def test_gated_uses_channel_and_profile_bound_capability(self, gated_app): + url = web_server._build_sidecar_url("ch-1", profile="worker") assert url is not None assert "token=" not in url assert "ticket=" not in url assert "internal=" in url - # The value should be the live process-lifetime internal credential, - # multi-use so the child can reconnect /api/pub. - cred = url.split("internal=")[1].split("&")[0] - info = consume_internal_credential(cred) + query = parse_qs(urlsplit(url).query) + assert query["channel"] == ["ch-1"] + assert query["profile"] == ["worker"] + cred = query["internal"][0] + binding = web_server._sidecar_capability_binding("ch-1", "worker") + info = consume_internal_credential( + cred, audience="sidecar", binding=binding + ) assert info["user_id"] == "server-internal" assert info["provider"] == "server-internal" # Multi-use: a second consume still succeeds (unlike a ticket). - assert consume_internal_credential(cred)["provider"] == "server-internal" + assert consume_internal_credential( + cred, audience="sidecar", binding=binding + )["provider"] == "server-internal" + + def test_sidecar_capability_rejects_channel_or_profile_tampering(self, gated_app): + url = web_server._build_sidecar_url("ch-1", profile="worker") + assert url is not None + cred = parse_qs(urlsplit(url).query)["internal"][0] + + for channel, profile in (("ch-2", "worker"), ("ch-1", "other")): + ws = _fake_ws( + query={"internal": cred, "channel": channel, "profile": profile}, + path="/api/pub", + ) + binding = web_server._sidecar_capability_binding(channel, profile) + assert web_server._ws_auth_ok( + ws, + internal_audience="sidecar", + internal_binding=binding, + ) is False def test_no_bound_host_returns_none(self, gated_app): web_server.app.state.bound_host = None @@ -583,12 +626,15 @@ def test_no_bound_host_returns_none(self, gated_app): class TestGatewayWsUrl: - def test_loopback_uses_session_token(self, loopback_app): + def test_loopback_uses_gateway_capability(self, loopback_app): url = web_server._build_gateway_ws_url() assert url is not None assert "/api/ws?" in url - assert f"token={web_server._SESSION_TOKEN}" in url - assert "internal=" not in url + assert "token=" not in url + assert "internal=" in url + cred = parse_qs(urlsplit(url).query)["internal"][0] + ws = _fake_ws(query={"internal": cred}, path="/api/ws") + assert web_server._ws_auth_ok(ws, internal_audience="gateway") is True def test_gated_uses_internal_credential(self, gated_app): url = web_server._build_gateway_ws_url() @@ -597,20 +643,40 @@ def test_gated_uses_internal_credential(self, gated_app): assert "token=" not in url assert "ticket=" not in url assert "internal=" in url - cred = url.split("internal=")[1].split("&")[0] - # The credential authenticates against _ws_auth_ok in gated mode. + cred = parse_qs(urlsplit(url).query)["internal"][0] + # The gateway capability authenticates only the /api/ws route. ws = _fake_ws(query={"internal": cred}) - assert web_server._ws_auth_ok(ws) is True + assert web_server._ws_auth_ok(ws, internal_audience="gateway") is True - def test_gated_credential_matches_sidecar(self, gated_app): - """Both server-internal builders share one process credential, so a - single value authenticates /api/ws and /api/pub alike.""" + def test_gated_gateway_and_sidecar_capabilities_are_distinct(self, gated_app): gw = web_server._build_gateway_ws_url() - sc = web_server._build_sidecar_url("ch-1") + sc = web_server._build_sidecar_url("ch-1", profile="worker") assert gw is not None and sc is not None - gw_cred = gw.split("internal=")[1].split("&")[0] - sc_cred = sc.split("internal=")[1].split("&")[0] - assert gw_cred == sc_cred + gw_cred = parse_qs(urlsplit(gw).query)["internal"][0] + sc_cred = parse_qs(urlsplit(sc).query)["internal"][0] + assert gw_cred != sc_cred + + gateway_ws = _fake_ws(query={"internal": sc_cred}, path="/api/ws") + assert web_server._ws_auth_ok( + gateway_ws, internal_audience="gateway" + ) is False + + sidecar_binding = web_server._sidecar_capability_binding( + "ch-1", "worker" + ) + sidecar_ws = _fake_ws( + query={ + "internal": gw_cred, + "channel": "ch-1", + "profile": "worker", + }, + path="/api/pub", + ) + assert web_server._ws_auth_ok( + sidecar_ws, + internal_audience="sidecar", + internal_binding=sidecar_binding, + ) is False def test_no_bound_host_returns_none(self, gated_app): web_server.app.state.bound_host = None diff --git a/tests/hermes_cli/test_dashboard_auth_ws_tickets.py b/tests/hermes_cli/test_dashboard_auth_ws_tickets.py index 26749fccbd25..3845ca3dde3d 100644 --- a/tests/hermes_cli/test_dashboard_auth_ws_tickets.py +++ b/tests/hermes_cli/test_dashboard_auth_ws_tickets.py @@ -162,7 +162,7 @@ def worker(i: int): # --------------------------------------------------------------------------- -# Process-lifetime internal credential (server-spawned PTY child auth). +# Audience-bound internal capabilities (server-spawned PTY child auth). # Direct unit coverage for internal_ws_credential / consume_internal_credential # — _ws_auth_ok exercises these indirectly, but the mint-once, unminted, and # empty-value branches are only reachable via direct calls. @@ -172,60 +172,95 @@ def worker(i: int): class TestInternalCredential: def test_minted_once_is_stable(self): """Successive calls return the same process-lifetime value.""" - first = ws_tickets.internal_ws_credential() - second = ws_tickets.internal_ws_credential() + first = ws_tickets.internal_ws_credential(audience="gateway") + second = ws_tickets.internal_ws_credential(audience="gateway") assert first == second assert len(first) >= 32 # token_urlsafe(32) def test_round_trip_identity(self): - cred = ws_tickets.internal_ws_credential() - info = ws_tickets.consume_internal_credential(cred) + cred = ws_tickets.internal_ws_credential(audience="gateway") + info = ws_tickets.consume_internal_credential(cred, audience="gateway") assert info["user_id"] == ws_tickets.INTERNAL_USER_ID assert info["provider"] == ws_tickets.INTERNAL_PROVIDER def test_multi_use(self): """Unlike a single-use ticket, the credential survives repeated consume.""" - cred = ws_tickets.internal_ws_credential() + cred = ws_tickets.internal_ws_credential(audience="gateway") for _ in range(5): assert ( - ws_tickets.consume_internal_credential(cred)["provider"] + ws_tickets.consume_internal_credential( + cred, audience="gateway" + )["provider"] == ws_tickets.INTERNAL_PROVIDER ) + def test_credential_is_scoped_to_audience(self): + gateway = ws_tickets.internal_ws_credential(audience="gateway") + sidecar = ws_tickets.internal_ws_credential( + audience="sidecar", binding="worker\0channel-a" + ) + + assert gateway != sidecar + with pytest.raises(TicketInvalid): + ws_tickets.consume_internal_credential(sidecar, audience="gateway") + with pytest.raises(TicketInvalid): + ws_tickets.consume_internal_credential( + gateway, audience="sidecar", binding="worker\0channel-a" + ) + + def test_credential_is_scoped_to_binding(self): + cred = ws_tickets.internal_ws_credential( + audience="sidecar", binding="worker\0channel-a" + ) + + assert ws_tickets.consume_internal_credential( + cred, audience="sidecar", binding="worker\0channel-a" + )["provider"] == ws_tickets.INTERNAL_PROVIDER + with pytest.raises(TicketInvalid): + ws_tickets.consume_internal_credential( + cred, audience="sidecar", binding="worker\0channel-b" + ) + with pytest.raises(TicketInvalid): + ws_tickets.consume_internal_credential( + cred, audience="sidecar", binding="other\0channel-a" + ) + def test_rejected_before_mint(self): """With nothing minted yet, any value is rejected (expected is None).""" - # autouse _reset leaves _internal_credential == None at test start. + # autouse _reset leaves the derivation key unset at test start. with pytest.raises(TicketInvalid): - ws_tickets.consume_internal_credential("anything") + ws_tickets.consume_internal_credential("anything", audience="gateway") def test_empty_value_rejected(self): - ws_tickets.internal_ws_credential() # mint so expected is non-None + ws_tickets.internal_ws_credential(audience="gateway") with pytest.raises(TicketInvalid): - ws_tickets.consume_internal_credential("") + ws_tickets.consume_internal_credential("", audience="gateway") def test_wrong_value_rejected(self): - ws_tickets.internal_ws_credential() + ws_tickets.internal_ws_credential(audience="gateway") with pytest.raises(TicketInvalid): - ws_tickets.consume_internal_credential("not-the-credential") + ws_tickets.consume_internal_credential( + "not-the-credential", audience="gateway" + ) def test_reset_clears_and_remints(self): - first = ws_tickets.internal_ws_credential() + first = ws_tickets.internal_ws_credential(audience="gateway") _reset_for_tests() # The old value no longer validates after reset. with pytest.raises(TicketInvalid): - ws_tickets.consume_internal_credential(first) + ws_tickets.consume_internal_credential(first, audience="gateway") # A fresh mint produces a different value. - second = ws_tickets.internal_ws_credential() + second = ws_tickets.internal_ws_credential(audience="gateway") assert second != first - assert ws_tickets.consume_internal_credential(second)["user_id"] == ( - ws_tickets.INTERNAL_USER_ID - ) + assert ws_tickets.consume_internal_credential( + second, audience="gateway" + )["user_id"] == ws_tickets.INTERNAL_USER_ID def test_independent_of_ticket_store(self): """The internal credential is not a ticket — minting tickets doesn't touch it, and consuming the credential doesn't consume tickets.""" - cred = ws_tickets.internal_ws_credential() + cred = ws_tickets.internal_ws_credential(audience="gateway") ticket = mint_ticket(user_id="u1", provider="nous") # Consuming the internal credential leaves the ticket intact. - ws_tickets.consume_internal_credential(cred) + ws_tickets.consume_internal_credential(cred, audience="gateway") assert consume_ticket(ticket)["user_id"] == "u1" diff --git a/tests/hermes_cli/test_dashboard_internal_capabilities.py b/tests/hermes_cli/test_dashboard_internal_capabilities.py new file mode 100644 index 000000000000..880ad5ef2425 --- /dev/null +++ b/tests/hermes_cli/test_dashboard_internal_capabilities.py @@ -0,0 +1,98 @@ +"""Route-level regression coverage for dashboard internal WS capabilities.""" + +from __future__ import annotations + +from urllib.parse import parse_qs, urlsplit + +import pytest +from fastapi.testclient import TestClient +from starlette.websockets import WebSocketDisconnect + +from hermes_cli import web_server +from hermes_cli.dashboard_auth.ws_tickets import _reset_for_tests + + +@pytest.fixture +def gated_ws_client(monkeypatch): + previous = { + "auth_required": getattr(web_server.app.state, "auth_required", None), + "bound_host": getattr(web_server.app.state, "bound_host", None), + "bound_port": getattr(web_server.app.state, "bound_port", None), + } + _reset_for_tests() + monkeypatch.setattr(web_server, "_DASHBOARD_EMBEDDED_CHAT_ENABLED", True) + web_server.app.state.auth_required = True + web_server.app.state.bound_host = "testserver" + web_server.app.state.bound_port = 80 + + with TestClient(web_server.app, base_url="http://testserver") as client: + yield client + + _reset_for_tests() + for key, value in previous.items(): + setattr(web_server.app.state, key, value) + + +def _assert_rejected(client: TestClient, path: str, *, code: int = 4401) -> None: + with pytest.raises(WebSocketDisconnect) as exc_info: + with client.websocket_connect(path): + pass + assert exc_info.value.code == code + + +def test_sidecar_capability_cannot_open_broader_dashboard_routes(gated_ws_client): + sidecar_url = web_server._build_sidecar_url("lane-a", profile="worker") + assert sidecar_url is not None + capability = parse_qs(urlsplit(sidecar_url).query)["internal"][0] + + _assert_rejected(gated_ws_client, f"/api/ws?internal={capability}") + _assert_rejected( + gated_ws_client, + f"/api/pty?internal={capability}&channel=lane-a&profile=worker", + ) + _assert_rejected( + gated_ws_client, + f"/api/events?internal={capability}&channel=lane-a", + ) + _assert_rejected(gated_ws_client, f"/api/console?internal={capability}") + + +def test_gateway_capability_cannot_open_other_dashboard_routes(gated_ws_client): + gateway_url = web_server._build_gateway_ws_url() + assert gateway_url is not None + capability = parse_qs(urlsplit(gateway_url).query)["internal"][0] + + for path in ( + f"/api/pub?internal={capability}&channel=lane-a&profile=worker", + f"/api/pty?internal={capability}&channel=lane-a&profile=worker", + f"/api/events?internal={capability}&channel=lane-a", + f"/api/console?internal={capability}", + ): + _assert_rejected(gated_ws_client, path) + + +@pytest.mark.parametrize( + ("channel", "profile"), + [("lane-b", "worker"), ("lane-a", "other")], +) +def test_sidecar_capability_rejects_binding_tampering( + gated_ws_client, channel: str, profile: str +): + sidecar_url = web_server._build_sidecar_url("lane-a", profile="worker") + assert sidecar_url is not None + capability = parse_qs(urlsplit(sidecar_url).query)["internal"][0] + + _assert_rejected( + gated_ws_client, + f"/api/pub?internal={capability}&channel={channel}&profile={profile}", + ) + + +def test_matching_sidecar_capability_can_reconnect(gated_ws_client): + sidecar_url = web_server._build_sidecar_url("lane-a", profile="worker") + assert sidecar_url is not None + path = urlsplit(sidecar_url).path + "?" + urlsplit(sidecar_url).query + + for _ in range(2): + with gated_ws_client.websocket_connect(path) as ws: + ws.close() diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index d3e684bb3c2f..04ecfc6fe20f 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -9048,7 +9048,9 @@ def fake_resolve(resume=None, sidecar_url=None, profile=None, active_session_fil url = captured.get("sidecar_url") or "" assert url.startswith("ws://127.0.0.1:9119/api/pub?") assert "channel=abc-123" in url - assert "token=" in url + assert "profile=current" in url + assert "internal=" in url + assert "token=" not in url assert captured["active_session_file"] def test_pub_broadcasts_to_events_subscribers(self): @@ -9136,7 +9138,8 @@ def test_resolve_chat_argv_injects_gateway_ws_url(monkeypatch): assert env is not None gateway_url = env.get("HERMES_TUI_GATEWAY_URL", "") assert gateway_url.startswith("ws://127.0.0.1:9119/api/ws?") - assert "token=" in gateway_url + assert "internal=" in gateway_url + assert "token=" not in gateway_url class TestDashboardPluginStaticAssetAllowlist: diff --git a/tests/hermes_cli/test_web_server_profile_unification.py b/tests/hermes_cli/test_web_server_profile_unification.py index 14141a815362..cab6f142a1c5 100644 --- a/tests/hermes_cli/test_web_server_profile_unification.py +++ b/tests/hermes_cli/test_web_server_profile_unification.py @@ -612,11 +612,30 @@ def test_chat_argv_scopes_hermes_home(self, isolated_profiles, monkeypatch): lambda root, tui_dev=False: (["cat"], None), raising=False, ) - argv, cwd, env = web_server._resolve_chat_argv(profile="worker_beta") + monkeypatch.setenv("HERMES_DASHBOARD_SESSION_TOKEN", "parent-secret") + monkeypatch.setenv( + "HERMES_TUI_GATEWAY_URL", + "ws://dashboard.test/api/ws?internal=parent-secret", + ) + monkeypatch.setenv( + "HERMES_TUI_SIDECAR_URL", + "ws://dashboard.test/api/pub?internal=parent-secret&channel=other", + ) + requested_sidecar = ( + "ws://dashboard.test/api/pub?internal=profile-bound" + "&channel=lane-a&profile=worker_beta" + ) + argv, cwd, env = web_server._resolve_chat_argv( + profile="worker_beta", sidecar_url=requested_sidecar + ) assert env is not None assert env["HERMES_HOME"] == str(isolated_profiles["worker_beta"]) - # Scoped chat must NOT attach to the dashboard's in-memory gateway. + # Scoped chat must NOT attach to the dashboard's in-memory gateway and + # may receive only the exact profile/channel sidecar capability minted + # for this PTY request. assert "HERMES_TUI_GATEWAY_URL" not in env + assert env["HERMES_TUI_SIDECAR_URL"] == requested_sidecar + assert "HERMES_DASHBOARD_SESSION_TOKEN" not in env def test_chat_argv_unscoped_keeps_legacy_env(self, isolated_profiles, monkeypatch): import hermes_cli.web_server as web_server diff --git a/tests/test_tui_gateway_entry_capability_env.py b/tests/test_tui_gateway_entry_capability_env.py new file mode 100644 index 000000000000..fca266952367 --- /dev/null +++ b/tests/test_tui_gateway_entry_capability_env.py @@ -0,0 +1,40 @@ +"""The profile gateway must consume dashboard capabilities before imports.""" + +from __future__ import annotations + +import subprocess +import sys + + +def test_entry_removes_dashboard_capabilities_before_gateway_runtime_imports(tmp_path): + env = { + "HOME": str(tmp_path), + "HERMES_HOME": str(tmp_path / "hermes"), + "PATH": "/usr/bin:/bin", + "PYTHONUTF8": "1", + } + env["HERMES_TUI_SIDECAR_URL"] = ( + "ws://dashboard.test/api/pub?internal=synthetic&channel=test" + ) + env["HERMES_TUI_GATEWAY_URL"] = ( + "ws://dashboard.test/api/ws?internal=synthetic" + ) + output_file = tmp_path / "probe.txt" + probe = ( + "import os; import tui_gateway.entry; " + f"open({str(output_file)!r}, 'w').write(" + "'present' if any(key in os.environ for key in " + "('HERMES_TUI_GATEWAY_URL', 'HERMES_TUI_SIDECAR_URL')) else 'absent')" + ) + + result = subprocess.run( + [sys.executable, "-c", probe], + capture_output=True, + check=False, + env=env, + text=True, + timeout=30, + ) + + assert result.returncode == 0, result.stderr + assert output_file.read_text(encoding="utf-8") == "absent" \ No newline at end of file diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 125d3eb584e1..519bd8cb3a91 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -957,6 +957,32 @@ def test_config_set_battery_explicit_off(monkeypatch): assert writes == {"display.battery": False} +def test_shell_exec_strips_dashboard_internal_ws_capabilities(monkeypatch): + captured = {} + + def fake_run(*args, **kwargs): + captured["env"] = kwargs.get("env") + return types.SimpleNamespace(stdout="", stderr="", returncode=0) + + monkeypatch.setenv( + "HERMES_TUI_GATEWAY_URL", + "ws://dashboard.test/api/ws?internal=synthetic", + ) + monkeypatch.setenv( + "HERMES_TUI_SIDECAR_URL", + "ws://dashboard.test/api/pub?internal=synthetic&channel=test", + ) + monkeypatch.setattr(server.subprocess, "run", fake_run) + + response = server._methods["shell.exec"]("shell-env", {"command": "pwd"}) + + assert response is not None + assert "result" in response + assert captured["env"] is not None + assert "HERMES_TUI_GATEWAY_URL" not in captured["env"] + assert "HERMES_TUI_SIDECAR_URL" not in captured["env"] + + def test_voice_toggle_returns_configured_record_key(monkeypatch): monkeypatch.setattr( server, diff --git a/tests/tools/test_hermes_subprocess_env.py b/tests/tools/test_hermes_subprocess_env.py index 303fd4321122..23decbd50f72 100644 --- a/tests/tools/test_hermes_subprocess_env.py +++ b/tests/tools/test_hermes_subprocess_env.py @@ -27,6 +27,8 @@ "SLACK_APP_TOKEN": "xapp-secret", "MODAL_TOKEN_SECRET": "modal-secret", "HERMES_DASHBOARD_SESSION_TOKEN": "dash-secret", + "HERMES_TUI_GATEWAY_URL": "ws://dashboard.test/api/ws?internal=secret", + "HERMES_TUI_SIDECAR_URL": "ws://dashboard.test/api/pub?internal=secret", } _PROVIDER_SAMPLE = { @@ -124,6 +126,12 @@ def test_tier1_covers_github_auth(self): def test_tier1_covers_infra_secrets(self): assert {"MODAL_TOKEN_ID", "MODAL_TOKEN_SECRET", "DAYTONA_API_KEY"} <= _ALWAYS_STRIP_KEYS + def test_tier1_covers_dashboard_internal_ws_capabilities(self): + assert { + "HERMES_TUI_GATEWAY_URL", + "HERMES_TUI_SIDECAR_URL", + } <= _ALWAYS_STRIP_KEYS + class TestBrowserPassthroughPattern: def test_browser_keys_recoverable_after_strip(self): diff --git a/tests/tools/test_local_env_blocklist.py b/tests/tools/test_local_env_blocklist.py index 2e8332470ae8..b29563a42691 100644 --- a/tests/tools/test_local_env_blocklist.py +++ b/tests/tools/test_local_env_blocklist.py @@ -688,6 +688,12 @@ def test_predicate_matches_gateway_relay_auth(self): assert _is_hermes_internal_secret("GATEWAY_RELAY_DELIVERY_KEY") assert _is_hermes_internal_secret("GATEWAY_RELAY_SESSION_TOKEN") + def test_predicate_matches_dashboard_internal_ws_capabilities(self): + from tools.environments.local import _is_hermes_internal_secret + + assert _is_hermes_internal_secret("HERMES_TUI_GATEWAY_URL") + assert _is_hermes_internal_secret("HERMES_TUI_SIDECAR_URL") + def test_predicate_allows_auxiliary_non_secrets(self): """AUXILIARY_*_PROVIDER / _MODEL and GATEWAY_RELAY_* routing hints are NOT secrets and must remain visible so tooling that reads them works.""" @@ -729,6 +735,15 @@ def test_gateway_relay_secret_stripped_from_subprocess(self): # Non-secret routing hint stays visible. assert result_env.get("GATEWAY_RELAY_URL") == "https://relay.example.com" + def test_dashboard_internal_ws_capabilities_stripped_from_subprocess(self): + result_env = _run_with_env(extra_os_env={ + "HERMES_TUI_GATEWAY_URL": "ws://dashboard.test/api/ws?internal=secret", + "HERMES_TUI_SIDECAR_URL": "ws://dashboard.test/api/pub?internal=secret", + }) + + assert "HERMES_TUI_GATEWAY_URL" not in result_env + assert "HERMES_TUI_SIDECAR_URL" not in result_env + def test_auxiliary_secret_stripped_even_when_passthrough_registered(self): """A skill registering AUXILIARY_*_API_KEY as env_passthrough must NOT be able to tunnel it into a subprocess — the strip is unconditional.""" diff --git a/tools/environments/local.py b/tools/environments/local.py index 8b4450c72010..12907a8ba7ee 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -364,6 +364,10 @@ def _is_hermes_internal_secret(key: str) -> bool: ``_ALWAYS_STRIP_KEYS``. Non-secret ``GATEWAY_RELAY_*`` routing hints (``GATEWAY_RELAY_URL``, ``GATEWAY_RELAY_PLATFORMS``, …) are NOT matched and remain visible. + - ``HERMES_TUI_GATEWAY_URL`` / ``HERMES_TUI_SIDECAR_URL`` — dashboard + loopback URLs whose query strings carry internal WS capabilities. The + TUI transport consumes them before spawning lower-trust children; model + commands and generic subprocesses never need to inherit them. ``code_execution_tool.py`` already catches these via substring matching on ``KEY`` / ``SECRET`` / ``TOKEN``; the terminal backend's narrower name-based @@ -386,6 +390,8 @@ def _is_hermes_internal_secret(key: str) -> bool: upper.endswith("_SECRET") or upper.endswith("_KEY") or upper.endswith("_TOKEN") ): return True + if upper in {"HERMES_TUI_GATEWAY_URL", "HERMES_TUI_SIDECAR_URL"}: + return True return False @@ -529,6 +535,8 @@ def _sanitize_subprocess_env(base_env: dict | None, extra_env: dict | None = Non "HASS_TOKEN", "EMAIL_PASSWORD", "HERMES_DASHBOARD_SESSION_TOKEN", + "HERMES_TUI_GATEWAY_URL", + "HERMES_TUI_SIDECAR_URL", # Remote-compute / infrastructure secrets "MODAL_TOKEN_ID", "MODAL_TOKEN_SECRET", diff --git a/tui_gateway/entry.py b/tui_gateway/entry.py index e4c87be4a113..51708604386f 100644 --- a/tui_gateway/entry.py +++ b/tui_gateway/entry.py @@ -16,6 +16,12 @@ import time import traceback +# Consume dashboard capabilities before importing the gateway runtime. Keeping +# the sidecar URL only in this module prevents shell.exec, terminal tools, and +# later child processes from inheriting a dashboard bearer through os.environ. +_SIDECAR_URL = os.environ.pop("HERMES_TUI_SIDECAR_URL", None) +os.environ.pop("HERMES_TUI_GATEWAY_URL", None) + from tui_gateway._stdin_recovery import handle_spurious_eof from tui_gateway import server @@ -48,7 +54,7 @@ def _install_sidecar_publisher() -> None: ``/api/pty`` endpoint when a chat tab passes a ``channel`` query param. Best-effort: connect failure or runtime drop falls back to stdio-only. """ - url = os.environ.get("HERMES_TUI_SIDECAR_URL") + url = _SIDECAR_URL if not url: return diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 03d8492d932c..71a62590fd4f 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -16498,8 +16498,11 @@ def _(rid, params: dict) -> dict: except ImportError: return _err(rid, 5001, "shell.exec unavailable: approval safety module not importable") try: + from tools.environments.local import hermes_subprocess_env + r = subprocess.run( cmd, shell=True, capture_output=True, text=True, timeout=30, cwd=os.getcwd(), + env=hermes_subprocess_env(inherit_credentials=True), stdin=subprocess.DEVNULL, ) return _ok( diff --git a/ui-tui/src/__tests__/gatewayClient.test.ts b/ui-tui/src/__tests__/gatewayClient.test.ts index 2a2384b38be8..e4a8080e481f 100644 --- a/ui-tui/src/__tests__/gatewayClient.test.ts +++ b/ui-tui/src/__tests__/gatewayClient.test.ts @@ -1,3 +1,5 @@ +import { spawnSync } from 'node:child_process' + import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' interface ListenerEntry { @@ -234,6 +236,11 @@ describe('GatewayClient websocket attach mode', () => { const gw = new GatewayClient() const seen: string[] = [] + // Construction consumes capability URLs immediately; they are retained in + // private client state for reconnects rather than ambient subprocess env. + expect(process.env.HERMES_TUI_GATEWAY_URL).toBeUndefined() + expect(process.env.HERMES_TUI_SIDECAR_URL).toBeUndefined() + gw.on('event', ev => seen.push(ev.type)) gw.start() @@ -263,6 +270,27 @@ describe('GatewayClient websocket attach mode', () => { gw.kill() }) + it('removes dashboard capabilities before arbitrary descendants inherit them', () => { + process.env.HERMES_TUI_GATEWAY_URL = 'ws://gateway.test/api/ws?internal=synthetic' + process.env.HERMES_TUI_SIDECAR_URL = + 'ws://gateway.test/api/pub?internal=synthetic&channel=demo&profile=worker' + + const gw = new GatewayClient() + + const probe = spawnSync( + process.execPath, + [ + '-e', + "process.stdout.write(JSON.stringify({gateway:'HERMES_TUI_GATEWAY_URL' in process.env,sidecar:'HERMES_TUI_SIDECAR_URL' in process.env}))" + ], + { encoding: 'utf8', env: process.env } + ) + + expect(probe.status).toBe(0) + expect(JSON.parse(probe.stdout)).toEqual({ gateway: false, sidecar: false }) + gw.kill() + }) + it('publishes local dashboard-control events to the sidecar websocket', async () => { process.env.HERMES_TUI_GATEWAY_URL = 'ws://gateway.test/api/ws?token=abc' process.env.HERMES_TUI_SIDECAR_URL = 'ws://gateway.test/api/pub?token=abc&channel=demo' diff --git a/ui-tui/src/gatewayClient.ts b/ui-tui/src/gatewayClient.ts index 122b5089de4a..b65d66425d1e 100644 --- a/ui-tui/src/gatewayClient.ts +++ b/ui-tui/src/gatewayClient.ts @@ -152,11 +152,27 @@ export class GatewayClient extends EventEmitter { constructor() { super() + // Capture capability-bearing URLs before any caller can ask this client to + // spawn a subprocess. Keeping them only in private state prevents unrelated + // code that runs between construction and start() from inheriting them. + this.captureCapabilityUrls() // useInput / createGatewayEventHandler can legitimately attach many // listeners. Default 10-cap triggers spurious warnings. this.setMaxListeners(0) } + private captureCapabilityUrls(): boolean { + const nextAttachUrl = resolveGatewayAttachUrl() + const attachChanged = nextAttachUrl !== null && nextAttachUrl !== this.attachUrl + + this.attachUrl = nextAttachUrl ?? this.attachUrl + this.sidecarUrl = resolveSidecarUrl() ?? this.sidecarUrl + delete process.env.HERMES_TUI_GATEWAY_URL + delete process.env.HERMES_TUI_SIDECAR_URL + + return attachChanged + } + private publish(ev: GatewayEvent) { if (ev.type === 'gateway.ready') { this.ready = true @@ -348,6 +364,13 @@ export class GatewayClient extends EventEmitter { const env = { ...process.env } const pyPath = env.PYTHONPATH?.trim() + // In spawned mode the Python gateway owns sidecar publishing. Pass the + // capability only to that exact child; tui_gateway.entry consumes it + // before importing the wider runtime and removes it from os.environ. + if (this.sidecarUrl) { + env.HERMES_TUI_SIDECAR_URL = this.sidecarUrl + } + env.PYTHONPATH = pyPath ? `${root}${delimiter}${pyPath}` : root // Tell the gateway child where the Hermes source root is so its import // guard can force it ahead of any same-named package in the launch cwd. @@ -522,11 +545,8 @@ export class GatewayClient extends EventEmitter { start() { const root = process.env.HERMES_PYTHON_SRC_ROOT ?? resolve(import.meta.dirname, '../../') - const attachUrl = resolveGatewayAttachUrl() - const sidecarUrl = resolveSidecarUrl() - - this.attachUrl = attachUrl - this.sidecarUrl = sidecarUrl + this.captureCapabilityUrls() + const attachUrl = this.attachUrl this.resetStartupState() if (this.proc && !this.proc.killed && this.proc.exitCode === null) { @@ -722,10 +742,11 @@ export class GatewayClient extends EventEmitter { } request(method: string, params: Record = {}): Promise { - const attachUrl = resolveGatewayAttachUrl() + const attachChanged = this.captureCapabilityUrls() + const attachUrl = this.attachUrl if (attachUrl) { - if (this.attachUrl !== attachUrl) { + if (attachChanged) { // The env var rotated at runtime — restart the transport so // switching from spawned-gateway mode to attach mode also // tears down the old Python child. Merely closing `this.ws`