-
Notifications
You must be signed in to change notification settings - Fork 52.8k
fix(dashboard): persist session token across daemon restarts (#53972) #54034
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Kewe63
wants to merge
1
commit into
NousResearch:main
Choose a base branch
from
Kewe63:fix/53972-token-only-clean
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+247
−4
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| """Persistent-backed dashboard session token loader (issue #53972). | ||
|
|
||
| Reads/writes the dashboard session token to disk so a server restart | ||
| doesn't mint a fresh value and break TUI-Node children + browser SPA | ||
| tabs that hold the prior token via subprocess env / injected SPA HTML. | ||
|
|
||
| This module is intentionally standalone (no web_server / fastapi | ||
| dependencies) so it is unit-testable in isolation and can be imported | ||
| without the dashboard's full dependency stack. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import logging | ||
| import os | ||
| import secrets | ||
| from pathlib import Path | ||
| from typing import Optional | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| ENV_VAR = "HERMES_DASHBOARD_SESSION_TOKEN" | ||
|
|
||
|
|
||
| def _get_token_path(home_path: Path) -> Path: | ||
| """Return the canonical token path under ``$HERMES_HOME/state/``.""" | ||
| return home_path / "state" / "dashboard_session_token" | ||
|
|
||
|
|
||
| def load_or_create(home_path: Optional[Path] = None) -> str: | ||
| """Return the persistent dashboard session token. | ||
|
|
||
| Behavior: | ||
|
|
||
| 1. If ``HERMES_DASHBOARD_SESSION_TOKEN`` env var is set, prefer that | ||
| (operator-injected tokens win — pre-existing contract; matches the | ||
| desktop shell's convention). | ||
| 2. Else look for ``$HERMES_HOME/state/dashboard_session_token``; if the | ||
| file exists and is non-empty after stripping, return its content. | ||
| 3. Else mint a fresh ``secrets.token_urlsafe(32)``, persist it under | ||
| ``$HERMES_HOME/state/dashboard_session_token`` (parent dir created, | ||
| file mode 0o600 when the filesystem supports it), and return it. | ||
|
|
||
| The ``home_path`` argument is the hermes home directory; if None it is | ||
| resolved lazily through ``hermes_cli.config.get_hermes_home`` so this | ||
| module stays independent of plugin startup on import. | ||
|
|
||
| Best-effort: if the file can't be read or written, an in-memory token | ||
| is still returned so server import doesn't crash. | ||
| """ | ||
| if (env_token := os.environ.get(ENV_VAR)): | ||
| return env_token | ||
|
|
||
| if home_path is None: | ||
| try: | ||
| from hermes_cli.config import get_hermes_home | ||
| home_path = get_hermes_home() | ||
| except Exception as exc: | ||
| logger.warning( | ||
| "Could not resolve HERMES_HOME (%s); using in-memory token", | ||
| exc, | ||
| ) | ||
| return secrets.token_urlsafe(32) | ||
|
|
||
| token_path = _get_token_path(home_path) | ||
|
|
||
| # Try to reuse an existing persisted token. | ||
| try: | ||
| if token_path.exists(): | ||
| existing = token_path.read_text(encoding="utf-8").strip() | ||
| if existing: | ||
| return existing | ||
| except OSError: | ||
| # Read failure (perm denied, race after crash, etc.) — fall through. | ||
| pass | ||
|
|
||
| # Generate a fresh one and persist (best-effort). | ||
| token = secrets.token_urlsafe(32) | ||
| try: | ||
| token_path.parent.mkdir(parents=True, exist_ok=True) | ||
| # Atomic-ish write: tmp file then rename. | ||
| tmp_path = token_path.with_suffix(token_path.suffix + ".tmp") | ||
| tmp_path.write_text(token, encoding="utf-8") | ||
| try: | ||
| tmp_path.chmod(0o600) | ||
| except (PermissionError, NotImplementedError, OSError): | ||
| # Filesystems like Windows reject chmod — not fatal. | ||
| pass | ||
| tmp_path.replace(token_path) | ||
| except OSError as exc: | ||
| logger.warning( | ||
| "Could not persist dashboard session token to %s: %s — " | ||
| "token will rotate on next restart until the file can be written", | ||
| token_path, exc, | ||
| ) | ||
| return token | ||
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
141 changes: 141 additions & 0 deletions
141
tests/hermes_cli/test_dashboard_session_token_persistence.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| """Tests for persistent dashboard session token loader.""" | ||
|
|
||
| import os | ||
| from pathlib import Path | ||
| from unittest.mock import patch | ||
|
|
||
| import pytest | ||
|
|
||
| from hermes_cli import _dashboard_session_token as session_token | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def home(tmp_path): | ||
| """An isolated HERMES_HOME used as the persistence root for one test.""" | ||
| home = tmp_path / "hermes-home" | ||
| home.mkdir() | ||
| return home | ||
|
|
||
|
|
||
| def test_env_var_wins_over_persisted_file(home, monkeypatch): | ||
| """HERMES_DASHBOARD_SESSION_TOKEN overrides anything on disk.""" | ||
| monkeypatch.setenv(session_token.ENV_VAR, "operator-injected") | ||
| (home / "state").mkdir() | ||
| (home / "state" / "dashboard_session_token").write_text("STALE-FILE") | ||
|
|
||
| assert session_token.load_or_create(home_path=home) == "operator-injected" | ||
| # File untouched | ||
| assert (home / "state" / "dashboard_session_token").read_text() == "STALE-FILE" | ||
|
|
||
|
|
||
| def test_first_run_creates_token_file(home, monkeypatch): | ||
| """No env var, no file -> fresh token returned AND persisted.""" | ||
| monkeypatch.delenv(session_token.ENV_VAR, raising=False) | ||
|
|
||
| token_path = home / "state" / "dashboard_session_token" | ||
| assert not token_path.exists() | ||
|
|
||
| token = session_token.load_or_create(home_path=home) | ||
| assert token | ||
| assert len(token) >= 32 # secrets.token_urlsafe(32) is ~43 chars | ||
|
|
||
| assert token_path.exists() | ||
| assert token_path.read_text(encoding="utf-8").strip() == token | ||
|
|
||
| # POSIX: group/other cannot read. Skip on Windows. | ||
| if os.name == "posix": | ||
| mode = token_path.stat().st_mode & 0o777 | ||
| assert mode & 0o077 == 0 # Not readable by group or other | ||
|
|
||
|
|
||
| def test_persisted_file_reused_across_calls(home, monkeypatch): | ||
| """Same file -> same token across many invocations (no rotation on read).""" | ||
| monkeypatch.delenv(session_token.ENV_VAR, raising=False) | ||
|
|
||
| t1 = session_token.load_or_create(home_path=home) | ||
| t2 = session_token.load_or_create(home_path=home) | ||
| t3 = session_token.load_or_create(home_path=home) | ||
| assert t1 == t2 == t3 | ||
|
|
||
|
|
||
| def test_existing_file_reused(home, monkeypatch): | ||
| """A non-empty token file is reused as-is on next startup.""" | ||
| monkeypatch.delenv(session_token.ENV_VAR, raising=False) | ||
| (home / "state").mkdir() | ||
| (home / "state" / "dashboard_session_token").write_text( | ||
| "stable-token-from-earlier-run" | ||
| ) | ||
|
|
||
| got = session_token.load_or_create(home_path=home) | ||
| assert got == "stable-token-from-earlier-run" | ||
|
|
||
|
|
||
| def test_whitespace_only_file_is_replaced(home, monkeypatch): | ||
| """Empty/whitespace content treated as missing: file replaced + new token.""" | ||
| monkeypatch.delenv(session_token.ENV_VAR, raising=False) | ||
| (home / "state").mkdir(parents=True) | ||
| (home / "state" / "dashboard_session_token").write_text(" \n \n") | ||
|
|
||
| new_token = session_token.load_or_create(home_path=home) | ||
| assert new_token and new_token.strip() and len(new_token) > 8 | ||
| assert ( | ||
| home / "state" / "dashboard_session_token" | ||
| ).read_text(encoding="utf-8").strip() == new_token | ||
|
|
||
|
|
||
| def test_read_failure_falls_through_to_generate(home, monkeypatch): | ||
| """If the file can't be read (OSError), a fresh token is still produced.""" | ||
| monkeypatch.delenv(session_token.ENV_VAR, raising=False) | ||
|
|
||
| real_read_text = Path.read_text | ||
|
|
||
| def _explode_on_read(self, *a, **kw): | ||
| if self.name == "dashboard_session_token": | ||
| raise PermissionError("simulated EACCES") | ||
| return real_read_text(self, *a, **kw) | ||
|
|
||
| with patch.object(Path, "read_text", _explode_on_read): | ||
| token = session_token.load_or_create(home_path=home) | ||
| assert token and len(token) > 8 | ||
|
|
||
|
|
||
| def test_write_failure_returns_in_memory_token(home, monkeypatch): | ||
| """Best-effort: file can't be written -> in-memory token still returned.""" | ||
| monkeypatch.delenv(session_token.ENV_VAR, raising=False) | ||
|
|
||
| real_mkdir = Path.mkdir | ||
|
|
||
| def _fail_on_state(self, *a, **kw): | ||
| if self.name == "state" or (self.parts and self.parts[-1] == "state"): | ||
| raise OSError("EACCES - simulated read-only HOME", 13) | ||
| return real_mkdir(self, *a, **kw) | ||
|
|
||
| with patch.object(Path, "mkdir", _fail_on_state): | ||
| token = session_token.load_or_create(home_path=home) | ||
| # In-memory token still produced | ||
| assert token and len(token) > 8 | ||
|
|
||
|
|
||
| def test_get_hermes_home_fetch_failure_returns_random_token(home, monkeypatch): | ||
| """If get_hermes_home() raises, we fall back to an in-memory token.""" | ||
| monkeypatch.delenv(session_token.ENV_VAR, raising=False) | ||
|
|
||
| with patch( | ||
| "hermes_cli.config.get_hermes_home", | ||
| side_effect=RuntimeError("config broken"), | ||
| ): | ||
| token = session_token.load_or_create(home_path=None) | ||
| assert token and len(token) > 8 | ||
|
|
||
|
|
||
| def test_default_home_path_resolution(monkeypatch, tmp_path): | ||
| """When home_path=None, the helper uses HERMES_HOME via get_hermes_home.""" | ||
| monkeypatch.delenv(session_token.ENV_VAR, raising=False) | ||
| # Point HERMES_HOME at a tmp dir | ||
| monkeypatch.setenv("HERMES_HOME", str(tmp_path)) | ||
|
|
||
| token = session_token.load_or_create(home_path=None) | ||
| persisted = (tmp_path / "state" / "dashboard_session_token").read_text( | ||
| encoding="utf-8" | ||
| ).strip() | ||
| assert persisted == token |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.