v0.50.255: Opus follow-ups (4 fixes) + CHANGELOG

Opus pre-release advisor caught 4 issues in stage-255 (#1390 + #1405):

1. MUST-FIX: api/rollback.py path-traversal — _checkpoint_root() / ws_hash /
   checkpoint did NOT normalize Path() / "../escape", so an authenticated
   caller could read or restore from another allowlisted workspace via
   ../<other-ws-hash>/<sha>. New _validate_checkpoint_id() regex-guards
   with ^[A-Za-z0-9_-][A-Za-z0-9_.-]{0,63}$ and rejects . and .. literals.
   Both get_checkpoint_diff and restore_checkpoint validate.

2. SHOULD-FIX: redact_session_data perf cliff — the new api_redact_enabled
   toggle in #1405 called uncached load_settings() per string, recursed
   across messages[] and tool_calls[]. For a 50-message session: hundreds
   of disk reads per /api/session response. Now read once at the top and
   thread _enabled through via private kwarg.

3. SHOULD-FIX: voice-mode wrong-session TTS — the patched autoReadLastAssistant
   fires globally; if the user navigated to a different session between
   sending and stream completion, TTS would speak the wrong session\\s reply.
   New _voiceModeThinkingSid closure captures S.session.session_id at
   thinking-time; _speakResponse bails to _startListening() on mismatch.

4. NIT: rollback._inspect_checkpoint had bare Exception in the except tuple
   alongside specific catches, swallowing everything. Now (TimeoutExpired,
   OSError) only.

6 regression tests in test_v050255_opus_followups.py. Full suite: 3587 passed,
2 skipped, 3 xpassed.
This commit is contained in:
nesquena-hermes
2026-05-01 17:19:45 +00:00
parent 6ad7a4cc83
commit 5ce516ed38
5 changed files with 321 additions and 14 deletions

View File

@@ -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/<sha256-of-canonical-workspace>/<commit_hash>/.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 `../<other-ws-hash>/<sha>` 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

View File

@@ -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

View File

@@ -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 `../<other-ws-hash>/<sha>`
# 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

View File

@@ -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';

View File

@@ -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 `../<other-ws-hash>/<sha>` 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 `../<other-ws-hash>/<sha>` 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, ../<ws>/<sha> 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