From 3cc5839bf303fa6758bfdac538507407a2929655 Mon Sep 17 00:00:00 2001
From: nesquena-hermes <nesquena+hermes@gmail.com>
Date: Mon, 13 Apr 2026 23:10:46 -0700
Subject: [PATCH] [security] fix(sessions): validate session_id before deleting
 session files (#412)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(sessions): validate session_id before deleting files

* fix: remove premature session index invalidation before validation check

* docs: v0.50.32 release — version badge and CHANGELOG

---------

Co-authored-by: hinotoi-agent <paperlantern.agent@gmail.com>
Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
---
 CHANGELOG.md          |  9 +++++++++
 api/routes.py         |  8 +++++++-
 static/index.html     |  2 +-
 tests/test_sprint3.py | 18 ++++++++++++++++++
 4 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 93d92ef..1ab9cad 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,14 @@
 # Hermes Web UI -- Changelog
 
+## [v0.50.32] fix(sessions): validate session_id before deleting session files [SECURITY] (#409)
+
+`/api/session/delete` accepted arbitrary `session_id` values from the request body and built the delete path directly as `SESSION_DIR / f"{sid}.json"`. Because pathlib discards the prefix when `sid` is an absolute path, an attacker could supply `/tmp/victim` and cause the server to unlink `victim.json` outside the session store. Traversal-style values (`../../etc/target`) were also accepted. CVSS 8.1 High (AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:H).
+
+- `api/routes.py`: validate `session_id` against `[0-9a-z_]+` allowlist (covers `uuid4().hex[:12]` WebUI IDs and `YYYYMMDD_HHMMSS_hex` CLI IDs) before path construction; resolve candidate path and enforce `path.relative_to(SESSION_DIR)` containment before unlinking; only invalidate session index on successful deletion path, not on rejected requests
+- `tests/test_sprint3.py`: 2 new regression tests — absolute-path payload rejected and file preserved, traversal payload rejected and file preserved
+- Original PR by @Hinotoi-agent (cherry-picked; branch was 4 commits behind master)
+- 1041 tests total (up from 1039)
+
 ## [v0.50.31] fix: delegate all live model fetching to agent's provider_model_ids()
 
 `_handle_live_models()` in `api/routes.py` previously maintained its own per-provider fetch logic and returned `not_supported` for Anthropic, Google, and Gemini. Now it delegates entirely to the agent's `hermes_cli.models.provider_model_ids()` — the single authoritative resolver — and `_fetchLiveModels()` in `ui.js` no longer skips any provider.
diff --git a/api/routes.py b/api/routes.py
index db1c1f8..5e72619 100644
--- a/api/routes.py
+++ b/api/routes.py
@@ -724,10 +724,16 @@ def handle_post(handler, parsed) -> bool:
         sid = body.get("session_id", "")
         if not sid:
             return bad(handler, "session_id is required")
+        if not all(c in '0123456789abcdefghijklmnopqrstuvwxyz_' for c in sid):
+            return bad(handler, "Invalid session_id", 400)
         # Delete from WebUI session store
         with LOCK:
             SESSIONS.pop(sid, None)
-        p = SESSION_DIR / f"{sid}.json"
+        try:
+            p = (SESSION_DIR / f"{sid}.json").resolve()
+            p.relative_to(SESSION_DIR.resolve())
+        except Exception:
+            return bad(handler, "Invalid session_id", 400)
         try:
             p.unlink(missing_ok=True)
         except Exception:
diff --git a/static/index.html b/static/index.html
index f5fea4a..b69fa9d 100644
--- a/static/index.html
+++ b/static/index.html
@@ -535,7 +535,7 @@
                 <div class="settings-section-title">System</div>
                 <div class="settings-section-meta">Instance version and access controls.</div>
               </div>
-              <span class="settings-version-badge">v0.50.31</span>
+              <span class="settings-version-badge">v0.50.32</span>
             </div>
             <div class="settings-field" style="border-top:1px solid var(--border);padding-top:12px;margin-top:8px">
               <label for="settingsPassword" data-i18n="settings_label_password">Access Password</label>
diff --git a/tests/test_sprint3.py b/tests/test_sprint3.py
index 255801f..333931d 100644
--- a/tests/test_sprint3.py
+++ b/tests/test_sprint3.py
@@ -114,6 +114,24 @@ def test_session_delete_requires_session_id():
     result, status = post("/api/session/delete", {})
     assert status == 400
 
+
+def test_session_delete_rejects_absolute_path_payload(tmp_path):
+    victim = tmp_path / "victim.json"
+    victim.write_text("TOPSECRET", encoding="utf-8")
+    result, status = post("/api/session/delete", {"session_id": str(victim.with_suffix(""))})
+    assert status == 400
+    assert victim.exists(), "absolute-path payload must not delete arbitrary files"
+
+
+def test_session_delete_rejects_traversal_payload(tmp_path):
+    victim = tmp_path / "outside.json"
+    victim.write_text("TOPSECRET", encoding="utf-8")
+    traversal = f"../../../../{victim.with_suffix('').as_posix().lstrip('/')}"
+    result, status = post("/api/session/delete", {"session_id": traversal})
+    assert status == 400
+    assert victim.exists(), "traversal payload must not delete arbitrary files"
+
+
 def test_chat_start_requires_session_id():
     result, status = post("/api/chat/start", {"message": "hello"})
     assert status == 400