mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-15 12:10:19 +00:00
fix(security #3234): do not deny STATE_DIR wholesale — keep STATE_DIR/workspace media (Codex review #7)
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.
This commit is contained in:
+8
-6
@@ -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())
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user