diff --git a/hermes_cli/web_routers/sessions.py b/hermes_cli/web_routers/sessions.py index 1842c7e0d4..8fa5919809 100644 --- a/hermes_cli/web_routers/sessions.py +++ b/hermes_cli/web_routers/sessions.py @@ -576,6 +576,14 @@ def _history_profile_home(profile): return get_hermes_home() +def _session_files_dir(profile) -> Path: + """Transcript dir of the profile whose store a delete targets: ``SessionDB.delete_session`` only + unlinks the session's on-disk artifacts when handed this, and a row-only delete leaves the + (secret-bearing) ``session_.json`` snapshots and ``request_dump__*.json`` readable after + the user removed the session (#55088, #60207).""" + return _history_profile_home(profile) / "sessions" + + def _project_for_display(messages: list, *, home=None) -> list: from agent.compaction_display import project_compaction_message_for_display from agent.context_compressor import is_compaction_summary_message @@ -717,7 +725,7 @@ async def delete_session_endpoint(session_id: str, profile: Optional[str] = None sid = _resolve_session_id(db, session_id) if not sid: return {"ok": True, "already_absent": True} - db.delete_session(sid) + db.delete_session(sid, sessions_dir=_session_files_dir(profile)) return {"ok": True} return await asyncio.to_thread(_with_db, profile, _delete, read_only=False) diff --git a/hermes_state_sessions.py b/hermes_state_sessions.py index 4678fe38cb..d6166a6810 100644 --- a/hermes_state_sessions.py +++ b/hermes_state_sessions.py @@ -1527,11 +1527,14 @@ class SessionSessionsMixin: @staticmethod def _remove_session_files(sessions_dir: Optional[Path], session_id: str) -> None: - """Remove ``.json``/``.jsonl`` and gateway ``request_dump__*.json``; OSError is swallowed - so a filesystem hiccup never blocks a DB operation.""" + """Remove ``.json``/``.jsonl``, the legacy ``session_.json`` snapshot, and gateway + ``request_dump__*.json``; OSError is swallowed so a filesystem hiccup never blocks a + DB operation. Every historical writer name is swept because a "deleted" session's snapshot + can carry plaintext secrets (#20334, #60207).""" if sessions_dir is None: return targets = [sessions_dir / f"{session_id}{suffix}" for suffix in (".json", ".jsonl")] + targets.append(sessions_dir / f"session_{session_id}.json") try: # glob.escape: a session id carrying ``[`` / ``?`` / ``*`` is a PATTERN otherwise, so the # dump sweep either matches nothing or matches another session's files. diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index d5ab94e995..a82922a357 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -4123,6 +4123,76 @@ class TestDeleteSessionEndpoint: assert resp.status_code == 200 assert resp.json().get("ok") is True + def test_delete_existing_session_scrubs_row_and_disk(self): + # The CLI delete path threads the sessions dir so transcript + # artifacts are removed with the row; the endpoint historically + # didn't, leaving secret-bearing session_.json snapshots and + # request dumps orphaned on disk after a UI delete. + from hermes_constants import get_hermes_home + from hermes_state import SessionDB + + db_path = get_hermes_home() / "state.db" + db = SessionDB(db_path=db_path) + try: + db.create_session("disk-scrub", source="cli") + finally: + db.close() + + sessions_dir = get_hermes_home() / "sessions" + sessions_dir.mkdir(parents=True, exist_ok=True) + for name, body in ( + ("session_disk-scrub.json", '{"messages": [{"content": "secret-token"}]}'), + ("disk-scrub.jsonl", "{}\n"), + ("request_dump_disk-scrub_001.json", "{}"), + ): + (sessions_dir / name).write_text(body, encoding="utf-8") + # Another session's artifacts must survive. + (sessions_dir / "session_disk-scrub-neighbour.json").write_text("{}", encoding="utf-8") + + resp = self.auth_client.delete("/api/sessions/disk-scrub") + + assert resp.status_code == 200 + assert resp.json().get("ok") is True + db = SessionDB(db_path=db_path) + try: + assert db.get_session("disk-scrub") is None + finally: + db.close() + assert not (sessions_dir / "session_disk-scrub.json").exists() + assert not (sessions_dir / "disk-scrub.jsonl").exists() + assert not (sessions_dir / "request_dump_disk-scrub_001.json").exists() + assert (sessions_dir / "session_disk-scrub-neighbour.json").exists() + + def test_delete_named_profile_session_scrubs_profile_disk(self): + from hermes_cli import profiles as profiles_mod + from hermes_state import SessionDB + + profile_home = profiles_mod.get_profile_dir("worker") + profile_home.mkdir(parents=True) + (profile_home / "config.yaml").touch() # identity marker: bare dirs are not profiles + sessions_dir = profile_home / "sessions" + sessions_dir.mkdir(parents=True, exist_ok=True) + db_path = profile_home / "state.db" + db = SessionDB(db_path=db_path) + try: + db.create_session("profile-scrub", source="cli") + finally: + db.close() + (sessions_dir / "session_profile-scrub.json").write_text( + '{"messages": [{"content": "secret-token"}]}', encoding="utf-8" + ) + + resp = self.auth_client.delete("/api/sessions/profile-scrub?profile=worker") + + assert resp.status_code == 200 + assert resp.json().get("ok") is True + db = SessionDB(db_path=db_path) + try: + assert db.get_session("profile-scrub") is None + finally: + db.close() + assert not (sessions_dir / "session_profile-scrub.json").exists() + class TestBulkDeleteSessionsEndpoint: """Tests for ``POST /api/sessions/bulk-delete`` — backs the diff --git a/tests/hermes_state/test_remove_session_files_glob_escape.py b/tests/hermes_state/test_remove_session_files_glob_escape.py index 898b1bfccf..9050c91ec0 100644 --- a/tests/hermes_state/test_remove_session_files_glob_escape.py +++ b/tests/hermes_state/test_remove_session_files_glob_escape.py @@ -21,3 +21,15 @@ def test_remove_session_files_escapes_glob_metacharacters(tmp_path): assert not (tmp_path / f"request_dump_{tricky}_0.json").exists(), "own dump was not matched" assert (tmp_path / f"{neighbour}.jsonl").exists() assert (tmp_path / f"request_dump_{neighbour}_0.json").exists(), "neighbour's dump was removed" + + +def test_remove_session_files_removes_legacy_session_prefix_snapshot(tmp_path): + # Older builds wrote the transcript snapshot as session_.json; the + # remover only swept .json/.jsonl, so the snapshot (which can + # carry plaintext secrets) survived every delete/prune. + sid = "abc123" + (tmp_path / f"session_{sid}.json").write_text('{"messages": [{"content": "secret"}]}', encoding="utf-8") + + SessionSessionsMixin._remove_session_files(tmp_path, sid) + + assert not (tmp_path / f"session_{sid}.json").exists()