Skip to content

fix(dashboard): allow desktop websocket origins on remote binds - #37405

Closed
leonardsellem wants to merge 1 commit into
NousResearch:mainfrom
leonardsellem:fix/desktop-remote-ws-origin
Closed

fix(dashboard): allow desktop websocket origins on remote binds#37405
leonardsellem wants to merge 1 commit into
NousResearch:mainfrom
leonardsellem:fix/desktop-remote-ws-origin

Conversation

@leonardsellem

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes Hermes Desktop remote mode when the dashboard is bound to an explicit non-loopback address, such as a Tailscale or LAN IP with --host ... --insecure --tui.

The packaged Electron app opens /api/ws with a non-web Origin (file:// or null). The WebSocket handler already validates credentials first with _ws_auth_ok, but the later origin guard only trusted those non-web origins on loopback binds. That made remote Desktop sessions fail even with a valid dashboard session token.

This keeps the existing strict behavior for OAuth-gated public dashboards and for real http(s) origins, while allowing authenticated non-web Electron origins on non-gated dashboard binds.

Related Issue

Fixes #37399

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/web_server.py: allow non-web WebSocket origins (file:// / null) after WebSocket credentials have already authenticated, when app.state.auth_required is false.
  • hermes_cli/web_server.py: keep OAuth-gated public dashboards strict, and keep http(s) origins subject to the bound-host match.
  • tests/hermes_cli/test_dashboard_auth_ws_auth.py: add regression coverage for explicit non-loopback binds accepting file:// and null origins.
  • tests/hermes_cli/test_dashboard_auth_ws_auth.py: add coverage that an explicit non-loopback bind still rejects a mismatched http://localhost:... origin.

How to Test

  1. Red/green focused origin guard test:
    • Before the production change, the new explicit non-loopback file:// and null tests failed.
    • After the production change: python -m pytest tests/hermes_cli/test_dashboard_auth_ws_auth.py::TestWsHostOriginGuardOrigins -q -> 9 passed.
  2. Adjacent dashboard auth/host verification:
    • python -m pytest tests/hermes_cli/test_web_server_host_header.py tests/hermes_cli/test_dashboard_auth_ws_auth.py -q -> 44 passed.
  3. CONTRIBUTING wrapper on the touched dashboard auth test file:
    • scripts/run_tests.sh tests/hermes_cli/test_dashboard_auth_ws_auth.py -> 33/33 passed.
  4. Full wrapper attempt:
    • scripts/run_tests.sh -j 4 -> 28092 passed, 26 failed across 10 unrelated files on macOS.
    • The touched file tests/hermes_cli/test_dashboard_auth_ws_auth.py passed in the full wrapper run.
    • Failing files were:
      • tests/agent/test_anthropic_adapter.py (5)
      • tests/gateway/test_gateway_shutdown.py (1)
      • tests/gateway/test_shutdown_forensics.py (1)
      • tests/hermes_cli/test_gateway_wsl.py (2)
      • tests/hermes_cli/test_gateway_service.py (6)
      • tests/hermes_cli/test_service_manager.py (2)
      • tests/hermes_cli/test_signal_handler_kanban_worker.py (1)
      • tests/test_live_system_guard_self_test.py (4)
      • tests/test_tui_gateway_server.py (1)
      • tests/tools/test_file_tools.py (3)

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15.6.1 / Darwin 24.6.0

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

The relevant runtime symptom was a Hermes Desktop WebSocket failure on macOS remote mode. Local log evidence showed the WebSocket request reaching the remote dashboard with:

  • Origin=file:// from the packaged Electron app.
  • A valid token=... credential in the WebSocket URL.
  • Rejection at the origin guard after credential auth.

The new regression tests cover that token-authenticated Electron-origin path without including any secret values.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools P2 Medium — degraded but workaround exists labels Jun 2, 2026

@Morad37 Morad37 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks right to me. The important bit is that non-web origins are still only accepted after the WebSocket credential gate, while http(s) origins still go through the host match. That keeps the DNS rebinding protection intact for browser-origin traffic and fixes the packaged Desktop remote case.

The tests cover the three cases I would care about: file/null on explicit insecure host, cross-site http still rejected, and OAuth-gated public dashboard still strict.

@DogaOztuzun

DogaOztuzun commented Jun 2, 2026

Copy link
Copy Markdown

Independent reproduction + LGTM

Second independent verification of this regression and of this fix's correctness.

Reproduction environment

Server Ubuntu 24.04, Docker, nousresearch/hermes-agent:latest (v0.15.1 / v2026.5.29.2)
Bind ${TAILSCALE_IP}:9119 (non-loopback) with --host 0.0.0.0 --insecure inside the container; host port bound to Tailscale IP
Client Hermes Desktop v0.15.1 on Ubuntu (separate host on same tailnet) pointing Settings → Gateway → Remote Gateway at http://server:9119

Same symptoms as the OP — /api/status returns 200 with gateway_state: running, but the Desktop renderer loops resetBootstrap() ~50×/sec with the message Could not connect to Hermes gateway.

Live-server verification of the root cause

Directly patching the running container's /opt/hermes/hermes_cli/web_server.py with this PR's production change (return not getattr(app.state, "auth_required", False) for non-web origins) and restarting trinity-dash:

$ curl -s -m 5 -i -N \
    -H "Connection: Upgrade" -H "Upgrade: websocket" \
    -H "Sec-WebSocket-Version: 13" -H "Sec-WebSocket-Key: ..." \
    -H "Origin: file://" -H "Host: server:9119" \
    "http://server:9119/api/ws?token=<session-token>"
HTTP/1.1 101 Switching Protocols
…
{"jsonrpc":"2.0","method":"event","params":{"type":"gateway.ready",…}}

Same for Origin: null101 Switching Protocols + gateway.ready. Both previously returned 403 Forbidden before the patch.

Security regression check

With the same patch applied, Origin: http://evil.example from a non-matching Host is still handled correctly — it falls through to the existing _is_accepted_host call below this branch, which rejects the cross-site http(s) case on matched-host binds. The PR's new test_explicit_non_loopback_cross_site_http_origin_rejected covers this precisely.

One additional data point

OAuth-gated public dashboards should correctly keep rejecting non-web origins — which this PR does because app.state.auth_required flips the branch to return False. Confirmed by reading the diff that the test_gated_file_origin_rejected test is preserved.

Endorsement

LGTM — this is the minimal correct fix. Please merge. The Desktop experience on Tailscale/LAN remote-gateway setups is currently broken for every user who upgrades past v0.13 + v0.14; this PR restores it without weakening rebinding defense or gated-mode security.

Cross-references

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Hermes Desktop “remote mode” WebSocket failures when the dashboard is bound to an explicit non-loopback address (e.g., Tailscale/LAN IP). It updates the dashboard WebSocket Origin guard to accept Electron’s non-web origins (file:// / null) on non-gated (--insecure) binds, while keeping OAuth-gated/public dashboards and real http(s) origin matching strict to preserve DNS-rebinding protections.

Changes:

  • Relax _ws_host_origin_is_allowed() for non-http(s) WebSocket origins when app.state.auth_required is false (non-gated / --insecure mode), enabling authenticated Desktop connections on explicit non-loopback binds.
  • Preserve strict Origin validation for OAuth-gated/public dashboards and for http(s) origins (must match the bound host).
  • Add regression tests covering file:// and null origins on explicit non-loopback insecure binds, and ensuring mismatched http://localhost:... origins are still rejected.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
hermes_cli/web_server.py Allows non-web (file:// / null) WS origins in non-gated mode after WS credential auth, while keeping gated mode and http(s) origin checks strict.
tests/hermes_cli/test_dashboard_auth_ws_auth.py Adds/updates regression coverage for explicit non-loopback insecure binds accepting Electron origins and rejecting mismatched http(s) origins.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@RobertoVillegas

RobertoVillegas commented Jun 3, 2026

Copy link
Copy Markdown

I reproduced this against a real remote Desktop setup and this PR matches the failure mode.

Setup:

  • macOS host running the dashboard backend with --host 0.0.0.0 --port 9121 --insecure --tui
  • Remote access over Tailscale
  • Packaged Hermes Desktop on the guest machine

Observed before the fix:

  • GET /api/status succeeded, so Desktop reported the remote backend as ready
  • the follow-up WebSocket failed: ws://<tailnet-ip>:9121/api/ws?token=...
  • the Electron renderer sent Origin: file://
  • backend response was 403 Forbidden

After applying the same host-side change locally, the same WebSocket probe returned 101 Switching Protocols.

This looks like the right layer to fix it: file:// is normal packaged Electron behavior, and the backend already validates the WebSocket session token before the Host/Origin guard.

Non-blocking test suggestion: a route-level /api/ws regression test for valid token + Origin: file:// on a non-loopback bind would guard the real Desktop failure path. An invalid-token case would also confirm the relaxed origin handling still depends on auth.

@Morad37

Morad37 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Thanks for the independent reproductions @DogaOztuzun and @RobertoVillegas. The file:// origin case on non-loopback binds is exactly the failure path I was targeting.

Good call on the regression test for /api/ws with valid token + file:// origin on a non-loopback bind. Will add that as a follow-up — want to get this fix out first since it blocks remote Desktop users.

@teknium1

teknium1 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Thanks for the thorough diagnosis and fix, @leonardsellem — your analysis of the split-brain state (public /api/status passing while the Electron file:///null WS origin gets rejected on a non-loopback --insecure bind) was spot on.

Closing as already fixed: the same behavioral change landed on main in #38743. Current main allows the non-web Electron origin after the WS credential check is the real auth boundary, while keeping OAuth-gated dashboards strict and http(s) origins bound-host checked — the same policy your PR implements. It also goes a bit further: it covers app://, and emits machine-parseable rejection reasons (host_mismatch/origin_mismatch) plus a _ws_auth_mode() label so the exact "why was the WS refused" question from #37399 is answerable from agent.log without a repro.

The linked issue #37399 is already closed by that work. No action needed on your end — appreciate the report and the clean repro.

@teknium1 teknium1 closed this Jun 4, 2026
Yuki-14544869 pushed a commit to Yuki-14544869/hermes-agent that referenced this pull request Jun 4, 2026
davidgut1982 pushed a commit to davidgut1982/hermes-agent that referenced this pull request Jun 5, 2026
changman pushed a commit to changman/hermes-agent that referenced this pull request Jun 10, 2026
kossteg pushed a commit to kossteg/hermes-agent that referenced this pull request Jun 16, 2026
T02200059 pushed a commit to T02200059/hermes-agent that referenced this pull request Jun 18, 2026
xyshanren pushed a commit to xyshanren/hermes-agent-cn that referenced this pull request Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hermes Desktop remote mode rejects Electron WebSocket origins on non-loopback dashboard binds

7 participants