From c514b32f61d599aa621b48596945c9f36786ecaa Mon Sep 17 00:00:00 2001
From: nesquena-hermes <[email protected]>
Date: Sun, 31 May 2026 02:45:17 +0000
Subject: [PATCH] fix(security #3234): cover ALL Hermes roots in /api/media
 deny-list (Codex review #2)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Under a named profile, process HERMES_HOME is ~/.hermes/profiles/<name> but the
allowlist still grants base ~/.hermes — so the prior deny (anchored only on the
active-profile root + STATE_DIR) left ~/.hermes/state.db and sibling-profile
secrets (~/.hermes/profiles/other/auth.json) reachable. Build deny roots from
every Hermes state root the allowlist accepts: active HERMES_HOME, base ~/.hermes,
api.profiles._DEFAULT_HERMES_HOME, and STATE_DIR; apply the state-subdir dir-denies
under each. Widen the CSP-slice structural test window to match.
---
 api/routes.py                                 | 43 ++++++++++++++-----
 .../test_issue1800_file_html_interactions.py  | 11 ++---
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/api/routes.py b/api/routes.py
index 63d9368d3..633050d4e 100644
--- a/api/routes.py
+++ b/api/routes.py
@@ -8097,26 +8097,49 @@ def _handle_media(handler, parsed):
         ".signing_key", ".pbkdf2_key", ".sessions.json",
         "google_token.json", "google_client_secret.json",
     }
-    # Roots whose contents are sensitive in their entirety.
+    # Roots whose contents are sensitive in their entirety. Cover EVERY Hermes
+    # state root that the allowlist accepts — not just the active-profile
+    # HERMES_HOME. Under a named profile the process HERMES_HOME is
+    # ~/.hermes/profiles/<name>, but the allowlist (above) also grants the base
+    # ~/.hermes, so without including the base root an attacker-influenced link
+    # could still fetch ~/.hermes/state.db or a SIBLING profile's secrets
+    # (~/.hermes/profiles/other/auth.json). (Codex review #3234.)
     _state_dir = None
     try:
         from api.config import STATE_DIR as _STATE_DIR
         _state_dir = Path(_STATE_DIR).resolve()
     except Exception:
         _state_dir = None
-    _hermes_home_resolved = _HERMES_HOME.resolve()
-    _deny_dirs = [d for d in (
+    _base_hermes_home = None
+    try:
+        from api.profiles import _DEFAULT_HERMES_HOME as _BASE_HH
+        _base_hermes_home = Path(_BASE_HH).resolve()
+    except Exception:
+        _base_hermes_home = None
+    # All Hermes state roots: active-profile HERMES_HOME, base ~/.hermes,
+    # api.profiles default home, and the WebUI STATE_DIR.
+    _hermes_roots = []
+    for _r in (
+        _HERMES_HOME.resolve(),
+        (_HOME / ".hermes").resolve(),
+        _base_hermes_home,
         _state_dir,
-        (_HERMES_HOME / "sessions").resolve(),
-        (_HERMES_HOME / "memories").resolve(),
-        (_HERMES_HOME / "profiles").resolve(),
-    ) if d is not None]
+    ):
+        if _r is not None and _r not in _hermes_roots:
+            _hermes_roots.append(_r)
+    # Dir-based denies: the named state subdirs under every Hermes root, plus
+    # the STATE_DIR itself (sensitive in its entirety).
+    _deny_dirs = []
+    if _state_dir is not None:
+        _deny_dirs.append(_state_dir)
+    for _root in _hermes_roots:
+        for _sub in ("sessions", "memories", "profiles"):
+            _deny_dirs.append((_root / _sub).resolve())
     # Filename-based denies only apply to files living under a Hermes/WebUI state
     # root — so a legitimate workspace or /tmp media artifact that happens to be
     # named settings.json / config.yaml is NOT blocked (Codex review #3234).
-    _under_state_root = (
-        _path_is_within_root(target, _hermes_home_resolved)
-        or (_state_dir is not None and _path_is_within_root(target, _state_dir))
+    _under_state_root = any(
+        _path_is_within_root(target, _root) for _root in _hermes_roots
     )
     _denied = (
         (_under_state_root and target.name in _DENY_FILENAMES)
diff --git a/tests/test_issue1800_file_html_interactions.py b/tests/test_issue1800_file_html_interactions.py
index 00edbef74..df1190ed9 100644
--- a/tests/test_issue1800_file_html_interactions.py
+++ b/tests/test_issue1800_file_html_interactions.py
@@ -68,12 +68,13 @@ def test_html_media_open_full_uses_inline_new_tab_not_download():
 
 def test_media_html_inline_keeps_csp_sandbox():
     """api/media may serve HTML inline only behind a CSP sandbox."""
-    # Slice widened to 7000 (was 5000) after the #3234 security deny-list block
-    # was inserted earlier in _handle_media, pushing the CSP block further down
-    # (the `csp=csp` line now sits ~6050 chars past the def). (Previously widened
+    # Slice widened to 9000 (was 5000) after the #3234 security deny-list block
+    # (multi-profile-aware deny roots) was inserted earlier in _handle_media,
+    # pushing the CSP block to ~7800 chars past the def. (Previously widened
     # 4000→5000 for PR #2044's MEDIA_ALLOWED_ROOTS parsing.) The assertion is
-    # structural, not positional.
-    body = _slice_after(ROUTES_PY, "def _handle_media", 7000)
+    # structural, not positional — generous headroom avoids re-breaking on small
+    # future edits to the early part of _handle_media.
+    body = _slice_after(ROUTES_PY, "def _handle_media", 9000)
     assert 'html_inline_ok = inline_preview and mime == "text/html"' in body
     assert 'csp = "sandbox allow-scripts" if html_inline_ok else None' in body
     assert "csp=csp" in body