Skip to content

fix(desktop): keep isolated SSH backend on session-token auth - #93828

Open
majestique1337 wants to merge 3 commits into
NousResearch:mainfrom
majestique1337:fix/desktop-ssh-public-url-auth
Open

fix(desktop): keep isolated SSH backend on session-token auth#93828
majestique1337 wants to merge 3 commits into
NousResearch:mainfrom
majestique1337:fix/desktop-ssh-public-url-auth

Conversation

@majestique1337

Copy link
Copy Markdown

What does this PR do?

Keeps the remote Desktop SSH backend on its intended per-launch session-token
authentication when the same machine also has an external
dashboard.public_url / HERMES_DASHBOARD_PUBLIC_URL configured.

The Desktop remote lifecycle starts a private backend as:

hermes serve --isolated --host 127.0.0.1 --port 0 \
  --ssh-session-token-file <file> --ssh-owner-nonce <nonce>

start_server() also resolves the regular browser dashboard's public host.
An external public host makes should_require_dashboard_auth() enable the
cookie/ticket auth gate even for this ephemeral loopback process. The Desktop
SSH client still connects to /api/ws?token=<per-launch token>, and gated mode
unconditionally rejects that legacy token. The result is a split symptom:
/api/health is reachable, while the WebSocket closes during boot.

This change recognizes only the fully marked private Desktop SSH shape and
does not inherit the browser dashboard's public Host/Origin declaration for
that process. Its loopback session-token auth therefore remains active.
Regular dashboard and serve launches keep the existing public auth gate.

Related Issue

Related to #91495. This addresses the remaining SSH token-rejection path
described in the issue discussion; it is separate from the local Desktop SPA
problem investigated in #91552.

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: identify the private Desktop SSH backend using
    all of its existing security/lifecycle markers: loopback host, ephemeral
    port, headless mode, session token, and owner nonce.
  • hermes_cli/web_server.py: keep the configured browser-facing public hosts
    out of that private process's Host/Origin and auth-gate state.
  • tests/hermes_cli/test_dashboard_auth_gate.py: prove that the session token
    is accepted with an external public URL configured, and that dropping any
    one private marker leaves the public auth gate enabled.

How to Test

  1. Configure an external dashboard.public_url (or its existing environment
    equivalent) on a remote Hermes host.
  2. Connect Hermes Desktop to that host in SSH mode.
  3. On main, observe HTTP health succeed while /api/ws?token=... is rejected
    after the dashboard auth gate engages.
  4. With this branch, observe the remote Desktop backend remain in token mode
    and the WebSocket connect, while the regular public dashboard remains gated.

Automated regression run (through the repository's hermetic test wrapper):

scripts/run_tests.sh \
  tests/hermes_cli/test_dashboard_auth_gate.py \
  tests/hermes_cli/test_dashboard_auth_ws_auth.py \
  tests/hermes_cli/test_dashboard_lifecycle_flags.py \
  tests/test_web_server.py

69 passed, 3 skipped, 0 failed

Also checked with ruff 0.15.10 on both changed files: clean.

Manual end-to-end verification: packaged macOS Desktop connecting over SSH to
a Linux backend with an external private-overlay dashboard URL configured.
Before the fix, HTTP health returned 200 but the WebSocket closed; after the
fix, the WebSocket opened and Desktop reached Gateway ready.

Security Boundary

The exception requires every marker below:

  • loopback bind (127.0.0.1, localhost, or ::1)
  • OS-assigned ephemeral port (0)
  • headless backend
  • non-empty per-launch SSH session token
  • non-empty SSH owner nonce

If any marker is absent, the existing public-dashboard auth decision is used.
The parameterized regression test covers every missing-marker case.

Checklist

Code

  • I've read the Contributing Guide
  • My commit message follows Conventional Commits
  • I searched open and closed issues/PRs for duplicates
  • My PR contains only changes related to this fix
  • I ran the relevant tests through scripts/run_tests.sh
  • I added behavioral regression tests
  • I tested the complete Desktop SSH flow on macOS with a Linux backend

Documentation & Housekeeping

  • Documentation update: N/A; no user-facing behavior or config changes
  • cli-config.yaml.example: N/A; no config keys changed
  • CONTRIBUTING.md / AGENTS.md: N/A; no workflow changes
  • Cross-platform impact considered; the predicate is platform-independent
  • Tool descriptions/schemas: N/A

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) backend/ssh SSH remote execution area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 24, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Security-sensitive change handled with the right posture: every private-backend marker is mandatory (hermes_cli/web_server.py:767-772), the parametrized negative tests prove dropping any one of them re-engages the public gate, and the positive test verifies both auth_required=False/empty trusted hosts and that WS session-token auth still resolves. Points:

  1. Auditability: nothing logs when this exception path engages. A gate bypass — even a correct one — should be visible: one logger.info("isolated desktop SSH backend: session-token auth, public-host gate not inherited") makes future security reviews trivial instead of archaeology.

  2. Caller surface: the fix assumes ssh_session_token/ssh_owner_nonce are passed only by the Desktop SSH spawn path. Worth asserting that assumption where those parameters are defined (defaults None, no other call sites) — if a future flag ever forwards them on ordinary serve, the auth gate silently disappears again. A comment on the parameters saying "these two together mark the private tunnel transport; never set outside the Desktop SSH launcher" would lock intent in place.

  3. hermes_cli/web_server.py:1946-1948 — clearing trusted_public_hosts also implicitly disables proxy-header handling (the test pins proxy_headers is False — good). Confirm no legitimate deployment tunnels the SSH backend through an extra hop needing X-Forwarded-*; per the threat model it shouldn't, but stating it closes the question.

Minor: the duplicated boilerplate between the two new tests (env + clear_providers + stub + restore) is begging for a small fixture; harmless now.

…nd auth gate

- Add concise logger.info when isolated Desktop SSH private-backend path activates
- Document parameter intent for ssh_session_token and ssh_owner_nonce across call surface
- Document threat-model assumption near proxy_headers=False
- Refactor test setup boilerplate in test_dashboard_auth_gate.py into _setup_desktop_ssh_test_env helper and assert log emission
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 backend/ssh SSH remote execution comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants