diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index e5333928d87..1d647b7ed50 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -6606,15 +6606,25 @@ def _ws_host_origin_is_allowed(ws: "WebSocket") -> bool: parsed = urllib.parse.urlparse(origin) if parsed.scheme not in {"http", "https"}: # Packaged Electron loads the desktop renderer over a non-web origin - # such as file:// or null. This helper is called only after _ws_auth_ok - # has accepted the WS credential; in non-gated mode that credential is - # the legacy dashboard session token, including for explicit Tailscale / - # LAN binds opened with --insecure. Real DNS-rebinding attacks arrive - # from http(s) origins and still have to match the bound host below. + # such as file://, null, or a custom app:// scheme. This helper is + # called only AFTER _ws_auth_ok has already accepted the WS credential, + # which is the real auth boundary in every mode: + # * loopback bind → legacy dashboard session token + # * non-loopback --insecure → legacy session token (Tailscale / LAN) + # * OAuth-gated public bind → single-use, 30s-TTL, identity-bound + # ?ticket= minted at the cookie-authed POST /api/auth/ws-ticket + # A non-web origin can only be produced by a native client (the desktop + # shell); a DNS-rebinding attack always arrives from an http(s) origin + # and is still match-checked against the bound host below. So once the + # credential check upstream has passed, the Origin guard adds nothing + # for a non-web origin — trust it in every mode. # - # OAuth-gated public dashboards authenticate with cookies/tickets and - # have no legitimate file:// client, so keep them strict. - return not getattr(app.state, "auth_required", False) + # (Earlier revisions restricted this to loopback, then to non-gated + # binds; both excluded the packaged desktop talking to a remote + # OAuth-gated gateway, whose file:// renderer origin then got rejected + # at the WS upgrade even with a valid ticket. The ticket is the gate, + # not the origin.) + return True if not parsed.netloc: return False diff --git a/tests/hermes_cli/test_dashboard_auth_ws_auth.py b/tests/hermes_cli/test_dashboard_auth_ws_auth.py index 64dc925b2cb..df16dd000ca 100644 --- a/tests/hermes_cli/test_dashboard_auth_ws_auth.py +++ b/tests/hermes_cli/test_dashboard_auth_ws_auth.py @@ -379,11 +379,16 @@ class TestWsHostOriginGuardOrigins: """The WS Origin guard must let the packaged desktop shell connect. Electron loads the packaged renderer over ``file://``, so its WebSocket - handshake carries ``Origin: file://`` (or the opaque ``null``). The - DNS-rebinding guard only needs to block cross-site http(s) origins. On a - loopback or explicit non-loopback insecure bind these non-web origins are - trusted because the session token is the real gate. OAuth-gated public - binds keep rejecting them. + handshake carries ``Origin: file://`` (or the opaque ``null``, or a custom + ``app://`` scheme). The DNS-rebinding guard only needs to block cross-site + http(s) origins — a malicious web page can never forge a non-web origin. + + This guard runs only AFTER ``_ws_auth_ok`` has validated the WS credential + (session token on loopback / ``--insecure`` binds, single-use ``?ticket=`` + on OAuth-gated binds), so a non-web origin is trusted in every mode: the + credential is the real gate, and a ``file://`` / ``null`` origin cannot + originate a DNS-rebinding browser attack. ``http(s)`` origins are still + match-checked against the bound host. """ def _ws(self, *, origin, host): @@ -434,12 +439,36 @@ class TestWsHostOriginGuardOrigins: ws = self._ws(origin="http://localhost:9119", host="100.64.0.10:9119") assert web_server._ws_host_origin_is_allowed(ws) is False - def test_gated_file_origin_rejected(self, gated_app): - # OAuth-gated public dashboards authenticate with cookies/tickets, - # not the legacy desktop session token. + def test_gated_file_origin_allowed(self, gated_app): + # The packaged desktop app drives a remote OAuth-GATED gateway over a + # file:// renderer origin. The WS route validates the single-use + # ?ticket= in _ws_auth_ok before this guard runs, and a file:// origin + # can't be a DNS-rebinding browser attack, so the Origin guard must let + # it through. This is the regression that broke desktop → hosted + # gateway connections — every WS upgrade got HTTP 403 even with a valid + # ticket. ws = self._ws(origin="file://", host="fly-app.fly.dev") + assert web_server._ws_host_origin_is_allowed(ws) is True + + def test_gated_null_origin_allowed(self, gated_app): + ws = self._ws(origin="null", host="fly-app.fly.dev") + assert web_server._ws_host_origin_is_allowed(ws) is True + + def test_gated_app_scheme_origin_allowed(self, gated_app): + ws = self._ws(origin="app://.", host="fly-app.fly.dev") + assert web_server._ws_host_origin_is_allowed(ws) is True + + def test_gated_cross_site_http_origin_still_host_checked(self, gated_app): + # An http(s) origin is still subjected to the same-host check even on a + # gated bind: a cross-site http origin whose netloc doesn't match the + # bound host is rejected. Real browser DNS-rebinding defence unchanged. + ws = self._ws(origin="https://evil.test", host="fly-app.fly.dev") assert web_server._ws_host_origin_is_allowed(ws) is False + def test_gated_same_host_https_origin_allowed(self, gated_app): + ws = self._ws(origin="https://fly-app.fly.dev", host="fly-app.fly.dev") + assert web_server._ws_host_origin_is_allowed(ws) is True + class TestSidecarUrl: def test_loopback_uses_session_token(self, loopback_app):