fix(desktop): close out the multi-profile desktop fallout — WS auth + cross-profile session reads - #44529
Conversation
(cherry picked from commit 64aaf58)
The served-token fallback adopts whatever token the dashboard HTML injects. That is correct when our own child regenerated the token (env pin lost across a shell-wrapped spawn), but wrong when the readiness probe answered from a process we did not spawn: /api/status is public, so an orphaned dashboard squatting the port passes waitForHermes while our child dies on the bind conflict. Silently adopting that process's token would authenticate the renderer against a foreign backend, possibly on the wrong profile. Discriminate on child liveness: the desktop pins HERMES_DASHBOARD_SESSION_TOKEN on every spawn, so a live child always serves our token. Served-token mismatch + dead child = foreign backend; fail the boot loudly instead of connecting. Mismatch + live child keeps the adopt-served-token salvage from #43720.
🔎 Lint report:
|
…collision
Two fixes to the Electron desktop launch path, with the port-reservation logic extracted into a unit-tested module:
1. hermes:bootstrap:reset ("Reload and retry") only cleared connectionPromise, leaving the live backend alive; the orphan kept binding PORT_FLOOR (9120) so the next startHermes() hit EADDRINUSE / "Object has been destroyed" and the window looped. Await teardownPrimaryBackendAndWait() so the reset stops the old backend before restarting.
2. pickPort() probes-then-closes a socket before the real bind happens in a separate Python child, so two concurrent spawns (primary + pool backend) could both be handed PORT_FLOOR and one died with EADDRINUSE. The reservation bookkeeping is extracted into electron/port-pool.cjs (PortPool): pickPort() reserves the chosen port until the child exits and releases it on every exit/error/throw-before-spawn path, closing the TOCTOU window.
PortPool is dependency-injected (probe passed in) and socket-free, unit-tested in electron/port-pool.test.cjs (8 cases) and wired into the test:desktop:platforms script.
(cherry picked from commit d413394)
…al into one helper
Both spawn paths (startHermes, spawnPoolBackend) duplicated the same
resolve -> log-fallback -> foreign-check -> throw dance. Collapse it into
adoptServedDashboardToken(baseUrl, spawnToken, {childAlive, label}) in
dashboard-token.cjs; childAlive is a thunk so liveness is sampled after
the fetch. Drop the redundant backendPool.delete in the pool's throw
path (the child exit/error handlers already own pool eviction).
Validated end-to-end against a real web_server.py backend, not just
units: token-injection regex vs the actual served index.html, foreign
refusal (dead child + live squatter), benign drift adoption, and the
401-vs-200 token auth split on /api/sessions.
How the repro was done (live backend, production code paths)No mocks anywhere in the loop — the real 1. Boot a genuine backend acting as the "squatter": # /tmp/hermes-e2e/serve.py
import os, sys
os.environ["HERMES_HOME"] = "/tmp/hermes-e2e/home" # temp, isolated
os.environ["HERMES_WEB_DIST"] = "/tmp/hermes-e2e/dist" # minimal index.html + empty assets/
os.environ["HERMES_DASHBOARD_SESSION_TOKEN"] = sys.argv[2] # the squatter's pinned token
sys.path.insert(0, "<repo>")
from hermes_cli.web_server import start_server
start_server(host="127.0.0.1", port=int(sys.argv[1]), open_browser=False)python /tmp/hermes-e2e/serve.py 9777 squatter-token-AAAAThis is the exact process shape of the bug: a live dashboard on a port the desktop thinks it owns, serving a token the desktop did not mint. 2. Drive the production electron module against it (
3. Premise independently confirmed live: All 5 passed. Unit suites on top: 13/13 dashboard-token tests, 165 platform tests, scoped eslint clean. |
Node >=18 / Electron 40 ship fetch; the hand-rolled http/https.request plumbing buys nothing. AbortSignal.timeout replaces the socket timeout, protocol guard and >=400 rejection semantics preserved. 13/13 unit tests and the live web_server.py repro both green over the new transport.
fix(desktop): close out the multi-profile desktop fallout — WS auth + cross-profile session reads
…ofile-fallout fix(desktop): close out the multi-profile desktop fallout — WS auth + cross-profile session reads
…ofile-fallout fix(desktop): close out the multi-profile desktop fallout — WS auth + cross-profile session reads
…ofile-fallout fix(desktop): close out the multi-profile desktop fallout — WS auth + cross-profile session reads
…ofile-fallout fix(desktop): close out the multi-profile desktop fallout — WS auth + cross-profile session reads
…ofile-fallout fix(desktop): close out the multi-profile desktop fallout — WS auth + cross-profile session reads
…ofile-fallout fix(desktop): close out the multi-profile desktop fallout — WS auth + cross-profile session reads
Consolidates the remaining desktop multi-profile / launch-reliability fixes into one reviewable PR.
Closes #44185. Closes #43114. Supersedes #44516 (already closed) and #43720 via the same commit.
What does this PR do?
Four commits, contributor authorship preserved via cherry-pick:
fix(desktop): use served dashboard token for websocket auth(@jeffrobodie-glitch, from Fix desktop WebSocket auth with served dashboard token #43720 / fix(desktop): use served dashboard token for websocket auth #44516) — after backend readiness, read thewindow.__HERMES_SESSION_TOKEN__the dashboard actually serves and use it for renderer state +/api/wsURLs, falling back to the spawn-time token. Fixes the Windows failure where/api/statusreturned 200 but the WS handshake was rejected.fix(desktop): route profile session reads(@Evisolpxe, from [codex] fix desktop profile session reads #44185) — non-default-profile sessions 404'd because several desktop entry points (session picker, command palette, archived-sessions settings, artifacts scan, export) still used the legacy single-profile/api/sessionslist or dropped the owning profile before reading messages. Now they uselistAllProfileSessionsand thread?profile=through message reads/export, plus lineage-root-id matching when resolving/deleting/archiving rows. Rebased over main's newerapplyRuntimeInfo/applyStoredSessionPreviewRuntimeInforefactor.fix(desktop): refuse a foreign backend's session token after readiness(new) — hardens (1) so it cannot silently authenticate against a backend we didn't spawn (details below).fix(desktop): prevent backend port-squat boot loop and pickPort self-collision(@bionicbutterfly13 / Mani Saint-Victor, from fix(desktop): prevent backend port-squat boot loop and pickPort self-collision #43114) — two launch-reliability fixes: thehermes:bootstrap:reset("Reload and retry") handler now awaitsteardownPrimaryBackendAndWait()instead of leaking a live backend that squatsPORT_FLOOR; andpickPort()'s probe-then-bind TOCTOU window is closed with an in-processPortPoolreservation held until the child exits (released on every exit/error/throw path). Rebased over current main;package.jsontest line re-merged.(3) and (4) are complementary halves of the same failure: (4) prevents orphaned/raced backends from squatting ports, (3) detects the survivor case and refuses to authenticate against it.
Why the hardening in (3)
The desktop's readiness probe is
waitForHermes → GET /api/status, which is public (hermes_cli/web_server.py:1520, no_require_token). So readiness passes even when the port is served by a process we did not spawn — e.g. an orphaned dashboard that won the bind race while our child died on the conflict. Commit (1) alone would adopt that stranger's token and silently connect, possibly to the wrong profile.The fix discriminates on child liveness: the desktop pins
HERMES_DASHBOARD_SESSION_TOKENon every spawn and the server honors it at import (web_server.py:185), so a live child always serves our token.Implemented as a pure
isForeignBackendToken()indashboard-token.cjs, applied at both local spawn sites (startHermes+spawnPoolBackend), with unit tests for all four rows. The foreign-backend throw paths are reservation-leak-free: the child's exit handler has already released thePortPoolslot, andrelease()is idempotent.Timeline of the fallout (for reviewers)
EADDRINUSEboot loop; plus thepickPort()TOCTOU self-collision between concurrent primary/pool spawns./api/statusreadiness gap are old.HERMES_DESKTOP=1backends skip the machine-dashboard reroute and stay per-profile./api/wsbackends start background MCP discovery;_make_agentwaits on it.With #44510 + #44512 on main and this PR, the known desktop multi-profile fallout is closed out: per-profile backends boot, ports aren't leaked or raced, MCP tools load, WS auth is trustworthy (and refuses impostors), and cross-profile sessions list/read/resume/export correctly.
Validation
/api/profiles/sessionsexists (web_server.py:2539);/api/sessions/{id}/messagesaccepts?profile=(web_server.py:6333);_SESSION_TOKENhonors the spawn env (web_server.py:185).apps/desktop:npm run test:desktop:platforms→ 165 passed / 1 skipped / 0 failed (includes 13 dashboard-token tests — 4 new — and the 8 port-pool tests).apps/desktop:npm run test:ui -- src/hermes.test.ts→ 3/3 (includes the new cross-profile message-read routing test).npm run test:ui: 570 passed / 7 failed — the 7 failures reproduce identically on cleanorigin/main(verified in a detached worktree); all in files this PR does not touch.tsc -p . --noEmit→ clean.main, verified against main's copies).PortPoolreleases fire on the same exit/error handlers my foreign-token check relies on;startHermes's nulledhermesProcessis treated as child-dead.Credit
@jeffrobodie-glitch & @lEWFkRAD (#43720), @Evisolpxe (#44185), @bionicbutterfly13 / Mani Saint-Victor (#43114), @aj47 (#44478). Original reviews by @austinpickett (#43720) and @liuhao1024 (#43114).
Real-world repro (live backend, not mocks)
Booted the actual
hermes_cli/web_server.pyunder uvicorn (tempHERMES_HOME, pinnedHERMES_DASHBOARD_SESSION_TOKEN) and drove the productiondashboard-token.cjsagainst it over real HTTP:index.html(the raw f-string injection atweb_server.py:10458, which no unit test exercises).adoptServedDashboardTokennow throws…served by a process we did not spawn; refusing its session token./api/statusanswers with no token (readiness really is a false positive against foreign processes), and the API genuinely rejects the stale spawn token (401) while accepting the served one (200) on/api/sessions.Also folded the duplicated resolve→check→throw dance at both spawn sites into one
adoptServedDashboardToken()helper (childAliveis a thunk, sampled post-fetch) — cc726aa.