From 124044dc111e0f54fc0e8a501466a684bd58f2b6 Mon Sep 17 00:00:00 2001
From: nesquena-hermes <[email protected]>
Date: Sun, 31 May 2026 03:41:21 +0000
Subject: [PATCH] =?UTF-8?q?fix(security=20#3234):=20do=20not=20deny=20STAT?=
 =?UTF-8?q?E=5FDIR=20wholesale=20=E2=80=94=20keep=20STATE=5FDIR/workspace?=
 =?UTF-8?q?=20media=20(Codex=20review=20#7)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The default workspace lives at STATE_DIR/workspace, so denying STATE_DIR itself
403'd legitimate workspace media. STATE_DIR is already in _hermes_roots, so its
sensitive subdirs (STATE_DIR/sessions, /memories, /profiles, etc.) are still
covered by the per-root subdir loop; direct sensitive files are still caught by
the filename denies. Drop the wholesale _state_dir deny. Adds a regression test
proving STATE_DIR/workspace/shot.png serves while STATE_DIR/sessions/*.json 403s.
---
 api/routes.py              | 14 ++++----
 tests/test_media_inline.py | 68 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/api/routes.py b/api/routes.py
index e024532e7..bc4f6162f 100644
--- a/api/routes.py
+++ b/api/routes.py
@@ -8151,13 +8151,15 @@ def _handle_media(handler, parsed):
         except (ValueError, OSError):
             return False
 
-    # State-subdir deny set: each DENY_SUBDIR directly under any Hermes root,
-    # plus STATE_DIR itself (sensitive in its entirety). These ALWAYS apply —
-    # even to a file that lives under the active workspace — so a workspace
-    # pointed at (or overlapping) a state dir cannot expose sessions/memories/etc.
+    # State-subdir deny set: each DENY_SUBDIR directly under any Hermes root
+    # (which includes STATE_DIR — so STATE_DIR/sessions, STATE_DIR/memories,
+    # etc. are covered). These ALWAYS apply — even to a file under the active
+    # workspace — so a workspace pointed at (or overlapping) a state dir cannot
+    # expose sessions/memories/profiles/etc. We do NOT deny STATE_DIR itself
+    # wholesale: the default workspace lives at STATE_DIR/workspace, and that is
+    # legitimate user media — direct sensitive files there are still caught by
+    # the filename denies below. (Codex review #3234.)
     _deny_dirs = []
-    if _state_dir is not None:
-        _deny_dirs.append(_state_dir)
     for _root in _hermes_roots:
         for _sub in _DENY_SUBDIRS:
             _deny_dirs.append((_root / _sub).resolve())
diff --git a/tests/test_media_inline.py b/tests/test_media_inline.py
index b04366dfd..2d4bc1b9e 100644
--- a/tests/test_media_inline.py
+++ b/tests/test_media_inline.py
@@ -336,6 +336,74 @@ class TestMediaEndpointUnit(unittest.TestCase):
                 "Hermes home (carve-out must be gated against internal roots)",
             )
 
+    def test_active_workspace_under_state_dir_serves_but_sessions_denied(self):
+        """#3234: a workspace at STATE_DIR/workspace is legitimate user media —
+        STATE_DIR/workspace/shot.png must serve (not 403), while a sibling
+        STATE_DIR/sessions/<sid>.json (internal state) must stay denied (403).
+
+        Regression for the over-block where STATE_DIR was denied wholesale.
+        """
+        from api import routes
+
+        class _Handler:
+            def __init__(self):
+                self.status = None
+                self.headers = {}
+            def send_response(self, code):
+                self.status = code
+            def send_header(self, *a, **k):
+                pass
+            def end_headers(self):
+                pass
+            class _W:
+                def write(self_inner, b):
+                    pass
+                def flush(self_inner):
+                    pass
+            wfile = _W()
+
+        png_bytes = (
+            b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01'
+            b'\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\x00'
+            b'\x01\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82'
+        )
+        with tempfile.TemporaryDirectory() as home:
+            hermes_home = pathlib.Path(home) / ".hermes"
+            state_dir = hermes_home / "webui-state"
+            ws = state_dir / "workspace"
+            sessions = state_dir / "sessions"
+            ws.mkdir(parents=True)
+            sessions.mkdir(parents=True)
+            shot = ws / "shot.png"
+            shot.write_bytes(png_bytes)
+            sess_file = sessions / "abc.json"
+            sess_file.write_text('{"messages":[]}', encoding="utf-8")
+
+            env = {
+                "HERMES_HOME": str(hermes_home),
+                "HERMES_WEBUI_STATE_DIR": str(state_dir),
+            }
+            with mock.patch.dict(os.environ, env), \
+                 mock.patch.object(routes, "get_last_workspace", lambda: str(ws)), \
+                 mock.patch("api.auth.is_auth_enabled", lambda: False), \
+                 mock.patch("api.config.STATE_DIR", state_dir):
+                # workspace media → not blocked by the #3234 deny
+                h1 = _Handler()
+                routes._handle_media(h1, SimpleNamespace(
+                    query=f"path={urllib.parse.quote(str(shot.resolve()))}&inline=1",
+                    path="/api/media"))
+                self.assertNotEqual(
+                    h1.status, 403,
+                    "STATE_DIR/workspace/shot.png must NOT be blocked (legit media)")
+                # sessions state → still denied
+                h2 = _Handler()
+                routes._handle_media(h2, SimpleNamespace(
+                    query=f"path={urllib.parse.quote(str(sess_file.resolve()))}",
+                    path="/api/media"))
+                self.assertEqual(
+                    h2.status, 403,
+                    "STATE_DIR/sessions/abc.json must stay denied (internal state)")
+
     def test_media_endpoints_advertise_byte_range_support(self):
         routes_src = (REPO_ROOT / "api" / "routes.py").read_text(encoding="utf-8")
         self.assertIn("Accept-Ranges", routes_src)