diff --git a/CHANGELOG.md b/CHANGELOG.md index 8990028d..c07c5542 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,33 @@ ## [Unreleased] +## [v0.50.255] — 2026-05-01 + +### Added +- **Insights panel — usage analytics dashboard** (#464) — new `GET /api/insights?days=N` endpoint walks `_index.json` (no full session loads) and aggregates session/message/token counts, model breakdown, and activity-by-day-of-week + activity-by-hour. New nav rail entry between Todos and Settings; the panel renders stats cards, a token breakdown row, and ASCII-style horizontal-bar charts. Period filter (7/30/90 days). (`api/routes.py`, `static/panels.js`, `static/index.html`, `static/i18n.js`, `static/style.css`) @bergeouss — PR #1405, fixes #464 + +- **Rollback UI — restore from agent checkpoints** (#466) — new `api/rollback.py` exposes 3 endpoints (`GET /api/rollback/list`, `GET /api/rollback/diff`, `POST /api/rollback/restore`) over the agent's `CheckpointManager` shadow git repos at `{hermes_home}/checkpoints///.git`. Workspace is allowlisted via `load_workspaces()` (added during contributor security pass `d9f3a69`). `_validate_checkpoint_id()` regex-guards the checkpoint parameter against path-traversal (Opus pre-release advisor finding — `Path()` does NOT normalize `..`). Restore copies files via `shutil.copy2` and never deletes; diff uses `difflib.unified_diff`. (`api/rollback.py`, `api/routes.py`) @bergeouss — PR #1405, fixes #466 + +- **Turn-based voice mode — STT + TTS chained flow** — new voice-mode button in the composer; activating it puts the agent in a listen → send → think → speak → listen loop. Uses the browser's Web Speech API (gated on both `SpeechRecognition` AND `speechSynthesis` support). Auto-send on 1.8s silence after a final transcript. Honors saved voice preferences (`hermes-tts-voice`, `hermes-tts-rate`, `hermes-tts-pitch`). Bails out on `not-allowed` / `service-not-allowed` / `audio-capture` errors. **Pre-release fix:** the patched `autoReadLastAssistant` fired globally — if the user navigated to a different session between send and stream completion, TTS would speak the wrong session's reply. Now captures `S.session.session_id` at thinking-time and bails to listening if the active session changed. (Opus pre-release advisor.) (`static/boot.js`, `static/i18n.js`, `static/index.html`, `static/style.css`) @bergeouss — PR #1405 + +- **API redact toggle — opt out of response-layer redaction** — adds `api_redact_enabled` setting (defaults to `True` so existing users see no behavioral change). When disabled, `redact_session_data()` returns payloads as-is. Useful for users who pipe the WebUI API into automation that needs the original strings. (`api/helpers.py`, `api/config.py`, `static/panels.js`, `static/i18n.js`) @bergeouss — PR #1405 + +- **Subagent tree visualization** — UI affordance for sessions that spawn subagents. (`static/panels.js`, `static/sessions.js`, `static/style.css`, `static/i18n.js`) @bergeouss — PR #1405 + +### Fixed +- **Session provider context preserved across model picker → runtime resolution** (#1240) — the WebUI model picker can show multiple providers exposing the same bare model id (e.g. `gpt-5.5` from OpenAI Codex, OpenRouter, Copilot). Previously sessions persisted only the bare model, so a session selected as "gpt-5.5 from OpenAI Codex" silently rerouted through whatever provider became default after a config change. New `model_provider: str | None` field on `Session` is persisted in metadata, threaded through every chat path (`/api/session/new`, `/api/session/update`, `/api/chat/start`, `/api/chat/sync`, `/btw`, `/background`, `_run_agent_streaming`), and is gated in `compact()` to emit only when truthy (matches v0.50.251 lineage end_reason gating). New `model_with_provider_context(model_id, model_provider)` in `api/config.py` builds the `@provider:model` form when provider differs from configured default, then passes through `resolve_model_provider()`. New `_should_attach_codex_provider_context()` narrow exception detects bare GPT-* models under active OpenAI Codex (because Codex/OpenRouter/Copilot expose overlapping GPT names). New `_resolve_compatible_session_model_state()` returns `(effective_model, effective_provider, model_was_normalized)`. Frontend adds `MODEL_STATE_KEY='hermes-webui-model-state'` localStorage with structured persistence and migrates from the legacy `hermes-webui-model` key. 13 new tests in `test_provider_mismatch.py`, 2 in `test_model_picker_badges.py`. (`api/config.py`, `api/models.py`, `api/routes.py`, `api/streaming.py`, `static/boot.js`, `static/messages.js`, `static/panels.js`, `static/sessions.js`, `static/ui.js`) @starship-s — PR #1390, refs #1240 + +### Changed + +- **`api/rollback.py` — checkpoint id regex validation (defense-in-depth)** — Opus pre-release follow-up. The `checkpoint` parameter on `/api/rollback/diff` and `/api/rollback/restore` was joined into the path via `_checkpoint_root() / ws_hash / checkpoint`. `Path("/a/b") / "../escape"` does NOT normalize, so an authenticated caller could pass `..//` and read or restore from another allowlisted workspace's checkpoint store. New `_validate_checkpoint_id()` regex-guards with `^[A-Za-z0-9_-][A-Za-z0-9_.-]{0,63}$` and rejects literal `.` / `..`. (`api/rollback.py`) + +- **`redact_session_data()` reads `api_redact_enabled` once per response, not per string** — Opus pre-release follow-up. The new `_redact_text` per-string `load_settings()` call (added by #1405's redact-toggle feature) caused hundreds of disk reads + JSON parses per `/api/session?session_id=X` response on a 50-message session — every nested string in `messages[]` and `tool_calls[]` recursed back into `_redact_value` → `_redact_text` → `load_settings`. Now read once at the top of `redact_session_data()` and threaded through via a private `_enabled` keyword. Fast path when disabled: still walks but returns immediately. (`api/helpers.py`, `tests/test_v050255_opus_followups.py`) + +- **Voice mode pins active session id at thinking-time** — Opus pre-release follow-up. The patched `autoReadLastAssistant` fires globally; if the user navigated to a different session between sending a turn and stream completion, TTS would speak the wrong session's last assistant message. New `_voiceModeThinkingSid` closure variable captures `S.session.session_id` in `_voiceModeSend`; `_speakResponse` bails to `_startListening()` if the current sid no longer matches. (`static/boot.js`, `tests/test_v050255_opus_followups.py`) + +- **`api/rollback.py::_inspect_checkpoint` drops bare `Exception` from except tuple** — Opus pre-release follow-up. The previous `except (subprocess.TimeoutExpired, OSError, Exception)` made the specific catches redundant and swallowed everything. Now `(subprocess.TimeoutExpired, OSError)` only. (`api/rollback.py`, `tests/test_v050255_opus_followups.py`) + + ## [v0.50.254] — 2026-05-01 ### Fixed diff --git a/api/helpers.py b/api/helpers.py index 291e5a1e..f6c8b584 100644 --- a/api/helpers.py +++ b/api/helpers.py @@ -178,25 +178,36 @@ def _build_redact_fn(): _redact_fn_cached = _build_redact_fn() -def _redact_text(text: str) -> str: - """Redact sensitive text from API responses. Respects api_redact_enabled setting.""" +def _redact_text(text: str, *, _enabled: bool | None = None) -> str: + """Redact sensitive text from API responses. Respects api_redact_enabled setting. + + The ``_enabled`` parameter is an internal optimization for callers that + redact many strings in a single response — `redact_session_data()` reads + the setting once and threads it through ``_redact_value`` so we avoid + re-loading settings.json from disk per string. (Opus pre-release perf fix.) + """ if not isinstance(text, str) or not text: return text - from api.config import load_settings - settings = load_settings() - if not settings.get("api_redact_enabled", True): + if _enabled is None: + from api.config import load_settings + _enabled = bool(load_settings().get("api_redact_enabled", True)) + if not _enabled: return text return _redact_fn_cached(text) -def _redact_value(v): - """Recursively redact credentials from strings, dicts, and lists.""" +def _redact_value(v, *, _enabled: bool | None = None): + """Recursively redact credentials from strings, dicts, and lists. + + ``_enabled`` is threaded through so a single response-level redact pass + only reads settings.json once. (Opus pre-release perf fix.) + """ if isinstance(v, str): - return _redact_text(v) + return _redact_text(v, _enabled=_enabled) if isinstance(v, dict): - return {k: _redact_value(val) for k, val in v.items()} + return {k: _redact_value(val, _enabled=_enabled) for k, val in v.items()} if isinstance(v, list): - return [_redact_value(item) for item in v] + return [_redact_value(item, _enabled=_enabled) for item in v] return v @@ -205,14 +216,22 @@ def redact_session_data(session_dict: dict) -> dict: Applies to: messages[], tool_calls[], and title. The underlying session file is not modified; redaction is response-layer only. + + Reads the ``api_redact_enabled`` setting ONCE for the entire response and + threads it through to avoid hundreds of settings.json reads per session + payload (a 50-message session has hundreds of nested strings). When the + setting is disabled this is also a fast path: the recursion still walks + but every string returns early. """ + from api.config import load_settings + _enabled = bool(load_settings().get("api_redact_enabled", True)) result = dict(session_dict) if isinstance(result.get('title'), str): - result['title'] = _redact_text(result['title']) + result['title'] = _redact_text(result['title'], _enabled=_enabled) if 'messages' in result: - result['messages'] = _redact_value(result['messages']) + result['messages'] = _redact_value(result['messages'], _enabled=_enabled) if 'tool_calls' in result: - result['tool_calls'] = _redact_value(result['tool_calls']) + result['tool_calls'] = _redact_value(result['tool_calls'], _enabled=_enabled) return result diff --git a/api/rollback.py b/api/rollback.py index 16da9af3..18d888fd 100644 --- a/api/rollback.py +++ b/api/rollback.py @@ -10,6 +10,7 @@ import hashlib import json import logging import os +import re import shutil import subprocess from datetime import datetime, timezone @@ -18,6 +19,25 @@ from typing import Any logger = logging.getLogger(__name__) +# Checkpoint identifiers are SHA-style hex hashes from the agent's +# CheckpointManager. We only allow [A-Za-z0-9_.-]{1,64} (no '/' so the +# value cannot be a path separator, no leading '.' so it cannot escape +# upward via '..'/'.'). This is defense-in-depth: the workspace arg is +# already allowlisted, but ``Path() / "../escape"`` does not normalize, +# so without this guard a `checkpoint` value of `..//` +# would let any authenticated caller diff or restore from another +# allowlisted workspace's checkpoint store. (Opus pre-release advisor.) +_CHECKPOINT_ID_RE = re.compile(r"^[A-Za-z0-9_-][A-Za-z0-9_.-]{0,63}$") + + +def _validate_checkpoint_id(checkpoint: str) -> str: + cid = str(checkpoint or "").strip() + if not cid or cid in (".", "..") or not _CHECKPOINT_ID_RE.fullmatch(cid): + raise ValueError( + "checkpoint id must match [A-Za-z0-9_-][A-Za-z0-9_.-]{0,63}" + ) + return cid + def _hermes_home() -> Path: """Return the active Hermes home directory.""" @@ -157,7 +177,7 @@ def _inspect_checkpoint(ckpt_path: Path, git: str) -> dict[str, Any] | None: "files": file_count, "path": str(ckpt_path), } - except (subprocess.TimeoutExpired, OSError, Exception) as e: + except (subprocess.TimeoutExpired, OSError) as e: logger.debug("Failed to inspect checkpoint %s: %s", ckpt_path, e) return None @@ -170,6 +190,7 @@ def get_checkpoint_diff(workspace: str, checkpoint: str) -> dict[str, Any]: files_changed: list of changed file paths """ resolved = _resolve_workspace(workspace) + checkpoint = _validate_checkpoint_id(checkpoint) ws_hash = _workspace_hash(resolved) ckpt_dir = _checkpoint_root() / ws_hash / checkpoint @@ -253,6 +274,7 @@ def restore_checkpoint(workspace: str, checkpoint: str) -> dict[str, Any]: files_restored: list of restored file paths """ resolved = _resolve_workspace(workspace) + checkpoint = _validate_checkpoint_id(checkpoint) ws_hash = _workspace_hash(resolved) ckpt_dir = _checkpoint_root() / ws_hash / checkpoint diff --git a/static/boot.js b/static/boot.js index 32ba8b63..e4134258 100644 --- a/static/boot.js +++ b/static/boot.js @@ -436,6 +436,10 @@ window._micPendingSend=window._micPendingSend||false; let _voiceModeState='idle'; // idle | listening | thinking | speaking let _recognition=null; let _silenceTimer=null; + // Capture the session id at thinking-time so the TTS callback won't read + // a different session's last assistant reply if the user navigated away + // between send and stream completion. (Opus pre-release advisor.) + let _voiceModeThinkingSid=null; const SILENCE_MS=1800; // auto-send after 1.8s silence function _setState(state){ @@ -528,6 +532,9 @@ window._micPendingSend=window._micPendingSend||false; return; } _setState('thinking'); + // Pin the active session id so the TTS callback won't speak a different + // session's reply if the user navigates away mid-stream. + _voiceModeThinkingSid=(typeof S!=='undefined'&&S.session)?S.session.session_id:null; try{ if(_recognition) _recognition.abort(); }catch(_){} _recognition=null; // send() is global from boot.js @@ -536,6 +543,17 @@ window._micPendingSend=window._micPendingSend||false; function _speakResponse(){ if(!_voiceModeActive) return; + // Bail out if the user navigated to a different session between send and + // stream completion. The patched autoReadLastAssistant fires globally; + // without this guard it would TTS-read the wrong session's last assistant + // message. Drop back to listening on the new session instead. + const currentSid=(typeof S!=='undefined'&&S.session)?S.session.session_id:null; + if(_voiceModeThinkingSid && currentSid && currentSid!==_voiceModeThinkingSid){ + _voiceModeThinkingSid=null; + _startListening(); + return; + } + _voiceModeThinkingSid=null; _setState('speaking'); // Find last assistant message @@ -640,6 +658,7 @@ window._micPendingSend=window._micPendingSend||false; function _deactivate(){ _voiceModeActive=false; _voiceModeState='idle'; + _voiceModeThinkingSid=null; modeBtn.classList.remove('active'); modeBtn.title=t('voice_toggle'); bar.style.display='none'; diff --git a/tests/test_v050255_opus_followups.py b/tests/test_v050255_opus_followups.py new file mode 100644 index 00000000..a67986ee --- /dev/null +++ b/tests/test_v050255_opus_followups.py @@ -0,0 +1,220 @@ +"""Regression tests for v0.50.255 Opus pre-release follow-ups. + +The v0.50.255 batch (#1390 + #1405) had four Opus advisor findings: + +1. MUST-FIX — `api/rollback.py::checkpoint` parameter wasn't validated; the + path-join `_checkpoint_root() / ws_hash / checkpoint` does NOT normalize + `..`, so an authenticated caller could pass `..//` and + read or restore from another allowlisted workspace's checkpoint store. + Fix: regex validation that rejects `/`, `..`, and `.`. + +2. SHOULD-FIX — `api/helpers.py::_redact_text` called uncached `load_settings()` + per string, recursed across all messages and tool_calls. For a 50-message + session that's hundreds of disk reads per `/api/session?session_id=X`. Fix: + thread `_enabled` once through `redact_session_data()`. + +3. SHOULD-FIX — `static/boot.js` voice mode: the patched `autoReadLastAssistant` + fires globally; if the user navigates to a different session between send + and stream completion, TTS would speak the wrong session's last assistant + message. Fix: capture the active session id in `_voiceModeSend` and bail + out in `_speakResponse` if it doesn't match. + +4. NIT — `api/rollback.py::_inspect_checkpoint` had a bare `Exception` in the + except tuple alongside specific catches, swallowing everything (incl. + KeyboardInterrupt's siblings). Fix: drop to the specific tuple. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[1] + + +# ── 1: rollback checkpoint id validation ───────────────────────────────────── + + +def test_rollback_validates_checkpoint_id_against_path_traversal(): + """The checkpoint param must reject `..`, `/`, and any path-component + traversal vector. Without this guard, a caller can join the checkpoint + root with `..//` and escape the workspace allowlist + (Path() / '..' does NOT normalize).""" + src = (REPO / "api" / "rollback.py").read_text(encoding="utf-8") + # Validator function exists. + assert "def _validate_checkpoint_id(" in src, ( + "_validate_checkpoint_id must exist as a defense-in-depth guard for " + "the checkpoint parameter; without it, ..// escapes the " + "workspace allowlist." + ) + # Both diff + restore call it. + assert src.count("_validate_checkpoint_id(checkpoint)") >= 2, ( + "both get_checkpoint_diff and restore_checkpoint must call " + "_validate_checkpoint_id() on their checkpoint parameter." + ) + # Validator rejects '..' and '.' explicitly. + assert 'in (".", "..")' in src, ( + "_validate_checkpoint_id must reject literal '.' and '..' explicitly " + "(not just rely on the regex)." + ) + + +def test_rollback_validate_checkpoint_id_runtime_behavior(): + """End-to-end test of the validator: traversal attempts raise ValueError.""" + import sys + sys.path.insert(0, str(REPO)) + from api.rollback import _validate_checkpoint_id + + # Valid SHA-style IDs pass. + assert _validate_checkpoint_id("abc123def456") == "abc123def456" + assert _validate_checkpoint_id("a1b2c3-d4e5") == "a1b2c3-d4e5" + assert _validate_checkpoint_id("checkpoint_2026-05-01") == "checkpoint_2026-05-01" + + # Traversal attempts blocked. + for bad in ( + "../escape", + "..", + ".", + "../../../etc/passwd", + "abc/def", + "abc def", # space + "abc\x00def", # null byte + "", + " ", + ".hidden", # leading dot → looks like dotfile escape + "/abs/path", + "x" * 65, # too long + ): + with pytest.raises(ValueError): + _validate_checkpoint_id(bad) + + +# ── 2: redact_session_data settings.json read-once optimization ────────────── + + +def test_redact_session_data_reads_settings_once(): + """`redact_session_data()` must read `api_redact_enabled` ONCE per call + and thread it through the recursive walk via the `_enabled` keyword. + Calling load_settings per string was a hot-path perf regression.""" + src = (REPO / "api" / "helpers.py").read_text(encoding="utf-8") + + # The function reads settings once and threads _enabled through. + redact_fn_idx = src.find("def redact_session_data(") + assert redact_fn_idx != -1, "redact_session_data missing" + body = src[redact_fn_idx : redact_fn_idx + 1500] + assert "load_settings()" in body, ( + "redact_session_data must read load_settings() once at the top" + ) + assert body.count("_enabled=_enabled") >= 3, ( + "redact_session_data must thread _enabled through to title, " + "messages, and tool_calls (3 call sites)" + ) + + # _redact_text and _redact_value accept _enabled kwarg. + assert "def _redact_text(text: str, *, _enabled" in src + assert "def _redact_value(v, *, _enabled" in src + + +def test_redact_session_data_threads_enabled_once_across_recursion(): + """End-to-end: a session payload with N strings should result in 1 read + of api_redact_enabled, not N. We verify by counting load_settings calls + via monkeypatch.""" + import sys + sys.path.insert(0, str(REPO)) + from api import helpers + + call_count = [0] + real_load_settings = helpers.__dict__.get("load_settings") + + def counting_load_settings(): + call_count[0] += 1 + return {"api_redact_enabled": True} + + # The from-import inside redact_session_data resolves at call time, so + # patch in api.config where it lives. + from api import config + original = config.load_settings + config.load_settings = counting_load_settings + try: + # Simulate a session payload with many strings + session = { + "title": "Test session", + "messages": [ + {"role": "user", "content": "hello world " * 10} + for _ in range(20) + ], + "tool_calls": [ + {"name": "tool", "args": {"x": "y", "z": ["a", "b", "c"]}} + for _ in range(10) + ], + } + helpers.redact_session_data(session) + finally: + config.load_settings = original + + # Should be called exactly once for the entire response, not per string. + assert call_count[0] == 1, ( + f"redact_session_data called load_settings() {call_count[0]} times; " + f"expected exactly 1 (read-once + thread-through optimization)." + ) + + +# ── 3: voice mode session-id capture ───────────────────────────────────────── + + +def test_voice_mode_speakresponse_guards_against_session_switch(): + """The `_speakResponse` callback fires from a global override of + `autoReadLastAssistant`. If the user navigates to a different session + between sending and stream completion, the callback would TTS-read the + new session's last assistant message instead of the one they sent to. + Fix: capture session_id at thinking-time, bail in _speakResponse if it + doesn't match the current S.session.session_id.""" + src = (REPO / "static" / "boot.js").read_text(encoding="utf-8") + + # Session-id capture state exists. + assert "let _voiceModeThinkingSid=" in src, ( + "voice mode must declare _voiceModeThinkingSid to pin the active " + "session id at send-time" + ) + + # _voiceModeSend captures current session_id at thinking transition. + send_idx = src.find("function _voiceModeSend(") + assert send_idx != -1 + send_body = src[send_idx : send_idx + 1200] + assert "_voiceModeThinkingSid=" in send_body, ( + "_voiceModeSend must capture the current session_id at thinking-time" + ) + assert "S.session.session_id" in send_body, ( + "_voiceModeSend must read S.session.session_id" + ) + + # _speakResponse compares current sid to captured sid and bails on mismatch. + speak_idx = src.find("function _speakResponse(") + assert speak_idx != -1 + speak_body = src[speak_idx : speak_idx + 1500] + assert "_voiceModeThinkingSid" in speak_body, ( + "_speakResponse must consult _voiceModeThinkingSid" + ) + assert "_startListening()" in speak_body, ( + "_speakResponse mismatch path must drop back to listening, not silently exit" + ) + + +# ── 4: rollback _inspect_checkpoint except tuple ───────────────────────────── + + +def test_rollback_inspect_checkpoint_except_no_bare_exception(): + """The bare `Exception` in `(subprocess.TimeoutExpired, OSError, Exception)` + swallowed everything including KeyboardInterrupt's siblings and made the + specific catches redundant. Should be the specific tuple only.""" + src = (REPO / "api" / "rollback.py").read_text(encoding="utf-8") + # No bare Exception in the inspect-checkpoint except tuple. + assert "(subprocess.TimeoutExpired, OSError, Exception)" not in src, ( + "_inspect_checkpoint must not catch bare Exception alongside specific " + "catches — the bare Exception swallows everything and makes the " + "specific ones redundant." + ) + # The specific tuple is in place. + assert "(subprocess.TimeoutExpired, OSError)" in src