From fc062dea3e9185a2d46736d92cb6dae7c8e5bf20 Mon Sep 17 00:00:00 2001 From: Blasius Patrick Date: Tue, 21 Jul 2026 11:28:11 +0700 Subject: [PATCH 1/2] fix: read internal token from disk per-request, surface auth errors Two bugs: 1. Server: _verify_internal_auth cached the token in app.state at startup. If the token file was overwritten later (auto-start in another process), server and CLI disagreed -> 401 -> hermes node list silently showed all nodes as disconnected. Now reads ~/.hermes/nodes-internal-token on every request via new _read_token_from_disk() helper. Uses hmac.compare_digest. 2. CLI/tools: _connected_names and _node_list_impl silently swallowed auth errors. _connected_names now returns set[str] | None; None means "cannot query". 401/403 print a clear stderr warning. _cmd_list shows a warning banner and marks rows as "unknown" instead of "disconnected". New state constant: STATE_UNKNOWN. Signed-off-by: Blasius Patrick --- cli.py | 50 +++++++++++++++++++++++++++++++++------------- tools.py | 10 ++++++++++ wsserver/server.py | 27 +++++++++++++++++++++---- 3 files changed, 69 insertions(+), 18 deletions(-) diff --git a/cli.py b/cli.py index 3527cfa..3b4f92c 100644 --- a/cli.py +++ b/cli.py @@ -63,6 +63,7 @@ import argparse import json import sys +import urllib.error from typing import Any from .config import load_config @@ -81,6 +82,7 @@ STATE_DISCONNECTED = "disconnected" STATE_NEVER_SEEN = "never_seen" STATE_REVOKED = "revoked" +STATE_UNKNOWN = "unknown" # --------------------------------------------------------------------------- # Argparse setup — bound to ``hermes node `` @@ -357,7 +359,7 @@ def _cmd_list(args: argparse.Namespace) -> int: connected_names = _connected_names() rows = [ - _format_row(rec, rec.name in connected_names) + _format_row(rec, connected_names is not None and rec.name in connected_names) for rec in records ] @@ -366,6 +368,14 @@ def _cmd_list(args: argparse.Namespace) -> int: print(json.dumps(row, sort_keys=True)) return 0 + if connected_names is None: + print( + "warning: cannot query server — connection state shown as 'unknown'. " + "Run `hermes node restart` or restart the gateway if this persists.", + file=sys.stderr, + ) + print() + if not rows: print("no paired nodes. run `hermes node pair --name ` to add one.") return 0 @@ -485,14 +495,19 @@ def _cmd_restart(args: argparse.Namespace | None = None) -> int: # --------------------------------------------------------------------------- -def _format_row(record: TokenRecord, is_connected: bool) -> dict[str, Any]: +def _format_row(record: TokenRecord, is_connected: bool | None) -> dict[str, Any]: """Map a :class:`TokenRecord` + liveness bool to a list row. Pulled out as a free function so the state-derivation logic is testable without spinning up a config / store / registry. + + ``is_connected`` may be ``None`` when the server could not be + queried — the state collapses to ``unknown`` in that case. """ if record.revoked: state = STATE_REVOKED + elif is_connected is None: + state = STATE_UNKNOWN elif is_connected: state = STATE_CONNECTED elif record.last_used_at is None: @@ -508,20 +523,14 @@ def _format_row(record: TokenRecord, is_connected: bool) -> dict[str, Any]: } -def _connected_names() -> set[str]: +def _connected_names() -> set[str] | None: """Return the set of node names with a live connection. The registry is owned by the long-running server; the CLI - command is a short-lived process. We query the server's HTTP - status endpoint at ``/nodes`` rather than maintaining - our own in-process registry — the CLI's registry would be - a fresh empty one that never saw any connections. - - If the server isn't running, the port isn't listening, or - the endpoint fails, we fall back to an empty set. The list - command should still succeed in that case — connection state - just collapses to ``disconnected`` / ``never_seen`` for - every row. + command is a short-lived process. Returns ``None`` when the + server could not be queried (internal auth mismatch, server + not running, timeout) so the caller can distinguish \"no nodes + connected\" from \"don't know\". """ config = load_config() base_url = f"http://{config.connect_host}:{config.port}" @@ -540,8 +549,21 @@ def _connected_names() -> set[str]: with urllib.request.urlopen(req, timeout=2.0) as resp: data = __import__("json").loads(resp.read()) return {n["name"] for n in data.get("nodes", [])} + except urllib.error.HTTPError as exc: + # 401 / 403 — internal auth mismatch. Surface clearly so + # the operator knows `hermes node list` can't see the live + # registry, instead of silently showing everything disconnected. + if exc.code in (401, 403): + print( + f"warning: server returned {exc.code} — " + f"internal auth token mismatch; connection state is unknown. " + f"Restart the gateway to sync tokens.", + file=sys.stderr, + ) + return None except Exception: - return set() + # Server not running, timeout, etc. + return None def main() -> None: diff --git a/tools.py b/tools.py index 26fbe21..6f3f90f 100644 --- a/tools.py +++ b/tools.py @@ -382,6 +382,16 @@ def _node_list_impl( cfg = load_config() status_url = f"http://{cfg.connect_host}:{cfg.port}/nodes" data = _request_with_retry("GET", status_url, timeout=2.0) + # If the server returned an auth error (internal token mismatch), + # surface it clearly instead of silently returning an empty list. + if isinstance(data, dict) and "detail" in data and "nodes" not in data: + detail = data["detail"] + return json.dumps({ + "error": f"cannot query server: {detail}. " + f"Restart the gateway to sync the internal auth token.", + "nodes": [], + "count": 0, + }) connected: list[dict[str, Any]] = data.get("nodes", []) return json.dumps({ "nodes": connected, diff --git a/wsserver/server.py b/wsserver/server.py index 8badf35..7390192 100644 --- a/wsserver/server.py +++ b/wsserver/server.py @@ -32,6 +32,7 @@ from __future__ import annotations import asyncio +import hmac import logging import os import secrets @@ -107,19 +108,37 @@ def _ensure_internal_token() -> str: return token +def _read_token_from_disk() -> str | None: + """Read the internal auth token from the shared token file. + + Reads on every call so manual edits and auto-start regenerations + take effect without a gateway restart. Returns ``None`` when the + file is missing or empty (server not yet initialised). + """ + try: + raw = _internal_token_path().read_text(encoding="utf-8").strip() + except (FileNotFoundError, OSError): + return None + if not raw: + return None + return raw.partition("\n")[0] + + async def _verify_internal_auth(request: Request) -> None: """FastAPI dependency that guards internal HTTP endpoints. - Checks ``Authorization: Bearer `` against the token the - server generated at startup. Returns 401/503 if missing or wrong. + Reads the token from disk on every request so manual edits and + auto-start regenerations take effect without a full gateway restart. + Returns 401/503 if missing or wrong. Uses ``hmac.compare_digest`` + for constant-time comparison. """ - expected: str | None = getattr(request.app.state, "internal_token", None) + expected: str | None = _read_token_from_disk() if expected is None: raise HTTPException(status_code=503, detail="Server not ready") auth_header = request.headers.get("Authorization", "") if not auth_header.startswith("Bearer "): raise HTTPException(status_code=401, detail="Missing or malformed Authorization header") - if auth_header.removeprefix("Bearer ") != expected: + if not hmac.compare_digest(auth_header.removeprefix("Bearer "), expected): raise HTTPException(status_code=401, detail="Invalid token") From 0cfcdadb3de01ebfbba4e30255803d6a6f370a29 Mon Sep 17 00:00:00 2001 From: Blasius Patrick Date: Tue, 21 Jul 2026 12:04:50 +0700 Subject: [PATCH 2/2] fix: rotating log for auto-start, quiet stdout - Replace print()+append with RotatingFileHandler (5 MB cap, 1 backup) - Only print lifecycle events to stdout (server started, failures, errors) - Routine diagnostics go to file only (port checks, thread lifecycle, etc.) - Remove stale import urllib.error (missed in #97) Signed-off-by: Blasius Patrick --- __init__.py | 54 ++++++++++++++++++++++++++++++++++++++--------------- cli.py | 1 - 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/__init__.py b/__init__.py index 417d5e3..56bb0ad 100644 --- a/__init__.py +++ b/__init__.py @@ -153,22 +153,37 @@ def _node_handler_lazy(args) -> None: # # Disabled in test/CI environments by setting the env var to ``0``. + import logging import os - import datetime as _datetime + from logging.handlers import RotatingFileHandler _LOG_DIR = os.environ.get("HERMES_HOME", os.path.expanduser("~/.hermes")) _LOG_FILE = os.path.join(_LOG_DIR, "hermes-node-plugin.log") - def _log(msg: str) -> None: - ts = _datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S") - line = f"[{ts}] {msg}" - print(line, flush=True) - try: - with open(_LOG_FILE, "a") as _f: - _f.write(line + "\n") - _f.flush() - except Exception: - pass + _auto_logger = logging.getLogger("hermes_nodes_plugin.auto_start") + _auto_logger.setLevel(logging.DEBUG) + _auto_logger.propagate = False + _fh = RotatingFileHandler( + _LOG_FILE, maxBytes=5 * 1024 * 1024, backupCount=1 + ) + _fh.setLevel(logging.DEBUG) + _fh.setFormatter(logging.Formatter( + "%(asctime)s %(message)s", datefmt="%Y-%m-%d %H:%M:%S" + )) + _auto_logger.addHandler(_fh) + + import datetime as _datetime + + def _log(msg: str, *, to_stdout: bool = False) -> None: + """Log to rotating file (5 MB cap, 1 backup). Prints to stdout + only when *to_stdout* is True — use for errors and key lifecycle + events, not routine diagnostics.""" + _auto_logger.info(msg) + if to_stdout: + print( + f"[{_datetime.datetime.now():%Y-%m-%d %H:%M:%S}] {msg}", + flush=True, + ) global _auto_start_done if _auto_start_done: @@ -216,14 +231,20 @@ def _start_server() -> None: from .lifecycle import _on_session_start loop.run_until_complete(_on_session_start()) - _log(f"WSS server started on port {_check_port}") + _log( + f"WSS server started on port {_check_port}", + to_stdout=True, + ) # Keep the loop alive so uvicorn tasks continue running. loop.run_forever() except Exception as exc: - _log(f"server background thread failed: {exc}") + _log( + f"server background thread failed: {exc}", + to_stdout=True, + ) import traceback - _log(traceback.format_exc()) + _log(traceback.format_exc(), to_stdout=True) finally: _log("event loop closed") loop.close() @@ -237,7 +258,10 @@ def _start_server() -> None: t.start() _log("daemon thread started") except Exception as exc: - _log(f"could not start server thread: {exc}") + _log( + f"could not start server thread: {exc}", + to_stdout=True, + ) else: _log(f"port {_check_port} already bound — skipping") else: diff --git a/cli.py b/cli.py index 3b4f92c..a39cabc 100644 --- a/cli.py +++ b/cli.py @@ -63,7 +63,6 @@ import argparse import json import sys -import urllib.error from typing import Any from .config import load_config