fix(desktop): use served dashboard token for websocket auth - #44516
Closed
OutThisLife wants to merge 1 commit into
Closed
fix(desktop): use served dashboard token for websocket auth#44516OutThisLife wants to merge 1 commit into
OutThisLife wants to merge 1 commit into
Conversation
(cherry picked from commit f8209f9)
13 tasks
Contributor
🔎 Lint report:
|
Collaborator
Author
|
Superseded by #44529, which carries this exact commit (authorship preserved) plus a hardening commit: served-token mismatch from a dead child is now treated as a foreign backend squatting the port (boot fails loudly) instead of silently adopting the stranger's token. Mismatch with a live child keeps this PR's adopt-and-log behavior. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvage of #43720 (rebased onto current
main,package.jsonconflict resolved). Closes #43720.What does this PR do?
Fixes a desktop startup / WebSocket auth failure where the local dashboard serves a different session token than the one the Electron main process generated when spawning the backend.
GET /api/statusstill returns 200, but the dashboard HTML injects a differentwindow.__HERMES_SESSION_TOKEN__; the renderer then attempts the/api/wshandshake with the stale spawn-time token and the gateway rejects it, surfacing asCould not connect to Hermes gateway.Why the fix is correct (validation)
The server side already honors the spawn-time token at import:
…and injects
_SESSION_TOKENinto the SPA HTML. So the served token only diverges from the desktop's spawn-time token when the app ends up bound to a backend it did not directly spawn with that env — e.g. an already-running / rerouted machine dashboard or a reused pooled instance, each carrying its own_SESSION_TOKEN. In those cases the renderer's source of truth is the served token, not the env token.This fix reads the injected token from the served HTML after readiness and uses it for desktop session state +
/api/wsURLs, falling back to the spawn-time token if discovery fails or the HTML has no token. It is strictly additive — the new path can only do better than the old one, never worse.Changes Made
apps/desktop/electron/dashboard-token.cjs— new helper: fetch the served dashboard index, extract the JSON-encodedwindow.__HERMES_SESSION_TOKEN__, preserve path prefixes, fall back safely. HTTP/HTTPS only, per-request timeout.apps/desktop/electron/main.cjs— both the primary (startHermes) and pooled (spawnPoolBackend) local-backend paths resolve the served token after readiness and use it for returned state +/api/wsURLs.apps/desktop/electron/dashboard-token.test.cjs— unit coverage (extraction, escaping, malformed-HTML fallback, differing-token logging, fetch-error propagation, protocol rejection).apps/desktop/electron/windows-child-process.test.cjs— normalize CRLF→LF when reading source so the Windows child-process audit holds oncore.autocrlf=truecheckouts.apps/desktop/package.json— wiresdashboard-token.test.cjsintotest:desktop:platforms(resolved againstmain, which has since added other entries to that line).Validation
cd apps/desktop && npm run test:desktop:platforms→ 153 passed, 1 skipped, 0 failed (includes the 9 new token tests and the CRLF-normalized Windows audit).Notes
Salvaged to preserve original authorship (commit authored by @jeffrobodie-glitch; PR opened by @lEWFkRAD) — credit to both. Reviewed/approved on the original #43720 by @austinpickett. The only change from that PR is the trivial
package.jsonline merge against currentmain.