fix(dashboard): persist session token across daemon restarts (#53972) - #54034
fix(dashboard): persist session token across daemon restarts (#53972)#54034Kewe63 wants to merge 1 commit into
Conversation
…ismatch spam (NousResearch#53972) When HERMES_DASHBOARD_SESSION_TOKEN env var is not set, the dashboard now reads/writes the token from /state/dashboard_session_token so the same value survives a server restart. Why this fixes the spam: - Every 'hermes dashboard' restart used to mint a fresh _SESSION_TOKEN. - TUI-Node children and SPA tabs hold the old token (via subprocess env / injected HTML) and retry the WebSocket handshake, receiving 'pty auth rejected reason=token_mismatch' in a tight loop (~13/min). - Persisting the token makes the client‑side and server‑side token match across restarts, so the spam stops. Behavior: 1. Env var HERMES_DASHBOARD_SESSION_TOKEN wins when set (operator‑injected tokens remain authoritative, matching the desktop shell contract). 2. Else read /state/dashboard_session_token; if the file exists and is non‑empty after stripping, reuse it. 3. Else mint a fresh secrets.token_urlsafe(32), persist it (parent dir created, mode 0o600 when supported), and return it. 4. Best‑effort: if the file can't be read or written, still return an in‑memory token so server import never fails. Files: - hermes_cli/_dashboard_session_token.py: new standalone helper (unit‑testable without fastapi/uvicorn). - hermes_cli/web_server.py: thin import + call to the helper. - tests/hermes_cli/test_dashboard_session_token_persistence.py: 9 tests covering env‑var precedence, file reuse, atomic write, read/write failure fallbacks, and HOME‑resolution fallback. Closes NousResearch#53972 (token‑mismatch spam portion only).
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Persists the dashboard session token to disk so a server restart doesn't mint a fresh value and break TUI-Node children + browser SPA tabs. The new _dashboard_session_token module is standalone (no web_server/fastapi dependencies), making it unit-testable in isolation. Behavior: env var wins, then persisted file, then fresh mint with atomic write (tmp + rename) and 0o600 permissions. Comprehensive tests cover env var priority, first-run creation, reuse, whitespace-only files, read/write failures, and home path resolution.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the persistence logic and retaining the injected-token precedence. The stale-token premise is present on current main: hermes_cli/web_server.py:274 mints a fallback token per server process, and web_server.py:14521-14526 rejects an old WebSocket token.
Problems
- This changes a documented security boundary, not just a reconnect detail. Current main says the token dies with the process (
hermes_cli/web_server.py:266-274) and rotates at restart (website/docs/user-guide/configuring-models.md:273), while the same bearer token authorizes sensitive API requests (web_server.py:319-336). Persisting it needs an explicit lifetime/revocation decision and matching documentation. _dashboard_session_token.py:88-95uses one fixed.tmppathname. Concurrent first starts can race; one process may return its generated token while another value wins on disk.
Suggested changes
- Agree the reconnect/token-lifetime contract before persisting this credential; if persistence remains, define expiry/revocation and update the documented behavior.
- Use a unique same-directory temp file plus creator coordination or winner re-read, and add concurrent-creation coverage. Add a temp-
HERMES_HOMEweb-server reload test rather than helper-only coverage.
This is an automated hermes-sweeper review.
| tmp_path.chmod(0o600) | ||
| except (PermissionError, NotImplementedError, OSError): | ||
| # Filesystems like Windows reject chmod — not fatal. | ||
| pass |
There was a problem hiding this comment.
This deterministic temporary pathname races when two dashboard processes first start against the same HERMES_HOME: either process can replace the shared temp file, and the losing process can return a token different from the persisted winner. Use a unique same-directory temp file and then coordinate/re-read the selected token.
|
Cross-linking the class: #76958 (stale-.env token clobber → desktop lockout) composes with this PR — its provenance classifier (_session_token_source in web_server.py) is a pure os.environ read, so it classifies whatever value lands in _SESSION_TOKEN whether that came from env, .env, or a persistence layer. No direction conflict. To clear the sweeper's keep_open points so this PR can land:
Happy to coordinate merge order with #76958. |
Summary
hermes_cli/web_server.pygenerated a freshsecrets.token_urlsafe(32)on every daemon start. After a restart, TUI-Node children and SPA tabs
holding the old token hit a tight
pty auth rejected reason=token_mismatchloop with no recovery path (#53972).
Fix: extract token lifecycle into a new
hermes_cli/_dashboard_session_token.pyhelper that loads an existingtoken from
$HERMES_HOME/state/dashboard_session_tokenor generates andpersists one on first run.
HERMES_DASHBOARD_SESSION_TOKENenv-var stillwins (operator-injected tokens take priority). All file I/O is
best-effort — read/write failures fall back to an in-memory token so the
server never fails to start.
Changes
hermes_cli/_dashboard_session_token.py(new) — standalone helper;atomic write, mode
0o600, fallbacks for missingHOMEandread/write errors.
hermes_cli/web_server.py— replace inline_SESSION_TOKEN = os.getenv(...) or secrets.token_urlsafe(32)withimport + call to new helper.
tests/hermes_cli/test_dashboard_session_token_persistence.py(new)— 9 unit tests: env-var priority, file reuse, first-run creation,
whitespace-only file, read/write failure, HOME resolution failure.
Test
Note: PTY replay buffer, 4409 close-code distinction, and SPA
reload-once handler (#53972) are intentionally out of scope — follow-up
PRs.
Risk: Low. Hot path (
web_server.py) loses one inline expression,gains one import. Fallback path ensures server always starts even if
$HERMES_HOME/state/is unwritable.