mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-15 04:01:05 +00:00
fix(security #3234): /api/media hard-denies WebUI state + secret/config files
Pre-release dual-gate (Codex + Opus) on #3219 surfaced that /api/media serves files under the allowlisted Hermes home, including settings.json / state.db / auth.json / config.yaml. #3219 makes this materially worse: pre-#3219 a bare file:// URL in agent output rendered as inert text, but #3219 turns it into an auto-loading <img src=/api/media?path=...> that fetches on render. Rather than weaken #3219, harden the boundary at the route: hard-deny known secret/config filenames and the WebUI state subdirs (sessions/memories/profiles + STATE_DIR) before the allow/serve decision, covering every entry path (bare file://, markdown anchors, MEDIA: tokens, session-token grants). Adds a live-server regression test. Closes #3234. Co-authored-by: AJV20 <24819659+AJV20@users.noreply.github.com>
This commit is contained in:
@@ -8083,6 +8083,38 @@ def _handle_media(handler, parsed):
|
||||
target,
|
||||
_INLINE_IMAGE_TYPES,
|
||||
)
|
||||
|
||||
# Hard deny WebUI state + secret/config files even when they fall under an
|
||||
# allowed root (the whole Hermes home is allowed, and the state dir lives
|
||||
# inside it). Without this, an authenticated session rendering attacker-
|
||||
# influenced agent output that emits a file:// / MEDIA: link to one of these
|
||||
# paths could fetch it through /api/media. This guard runs BEFORE the
|
||||
# allow/serve decision so it covers every entry path (bare file:// URLs,
|
||||
# markdown anchors, MEDIA: tokens, and session-token grants). See #3234.
|
||||
_DENY_FILENAMES = {
|
||||
"settings.json", "state.db", "auth.json", "auth.lock",
|
||||
"config.yaml", "config.yml", ".env", ".signing_key",
|
||||
".pbkdf2_key", ".sessions.json",
|
||||
}
|
||||
_state_dir = None
|
||||
try:
|
||||
from api.config import STATE_DIR as _STATE_DIR
|
||||
_state_dir = Path(_STATE_DIR).resolve()
|
||||
except Exception:
|
||||
_state_dir = None
|
||||
_deny_dirs = [d for d in (
|
||||
_state_dir,
|
||||
(_HERMES_HOME / "sessions").resolve(),
|
||||
(_HERMES_HOME / "memories").resolve(),
|
||||
(_HERMES_HOME / "profiles").resolve(),
|
||||
) if d is not None]
|
||||
_denied = (
|
||||
target.name in _DENY_FILENAMES
|
||||
or any(_path_is_within_root(target, d) for d in _deny_dirs)
|
||||
)
|
||||
if _denied:
|
||||
return bad(handler, "Path not in allowed location", 403)
|
||||
|
||||
if not within_allowed and not session_media_allowed:
|
||||
return bad(handler, "Path not in allowed location", 403)
|
||||
|
||||
|
||||
@@ -465,6 +465,46 @@ class TestMediaEndpointIntegration(unittest.TestCase):
|
||||
)
|
||||
self.assertIn(status, {403, 404})
|
||||
|
||||
def test_webui_state_secret_files_denied(self):
|
||||
"""#3234: /api/media must hard-deny WebUI state/secret files even though
|
||||
they live under an allowed root (the whole Hermes home is allowed).
|
||||
|
||||
An authenticated session rendering attacker-influenced agent output that
|
||||
emits a file:// or MEDIA: link to settings.json / state.db / auth.json
|
||||
must NOT be able to fetch it through /api/media.
|
||||
"""
|
||||
state_dir = pathlib.Path(TEST_STATE_DIR)
|
||||
state_dir.mkdir(parents=True, exist_ok=True)
|
||||
# settings.json by name (deny-by-filename)
|
||||
settings = state_dir / "settings.json"
|
||||
settings.write_text('{"secret":"value"}', encoding="utf-8")
|
||||
try:
|
||||
_, status, _ = self._get(
|
||||
"/api/media?path=" + urllib.request.quote(str(settings.resolve()))
|
||||
)
|
||||
self.assertEqual(
|
||||
status, 403,
|
||||
f"settings.json under the state dir must be denied, got {status}",
|
||||
)
|
||||
finally:
|
||||
settings.unlink(missing_ok=True)
|
||||
|
||||
# a file inside the sessions/ state subdir (deny-by-dir)
|
||||
sess_dir = state_dir / "sessions"
|
||||
sess_dir.mkdir(parents=True, exist_ok=True)
|
||||
sess_file = sess_dir / "abc123.json"
|
||||
sess_file.write_text('{"messages":[]}', encoding="utf-8")
|
||||
try:
|
||||
_, status, _ = self._get(
|
||||
"/api/media?path=" + urllib.request.quote(str(sess_file.resolve()))
|
||||
)
|
||||
self.assertEqual(
|
||||
status, 403,
|
||||
f"files under the sessions/ state subdir must be denied, got {status}",
|
||||
)
|
||||
finally:
|
||||
sess_file.unlink(missing_ok=True)
|
||||
|
||||
def test_health_check_still_works(self):
|
||||
"""Sanity: server is up and /health works."""
|
||||
body, status, _ = self._get("/health")
|
||||
|
||||
Reference in New Issue
Block a user