Skip to content

Commit c514b32

Browse files
author
nesquena-hermes
committed
fix(security nesquena#3234): cover ALL Hermes roots in /api/media deny-list (Codex review #2)
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.
1 parent 68a6099 commit c514b32

2 files changed

Lines changed: 39 additions & 15 deletions

File tree

api/routes.py

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8097,26 +8097,49 @@ def _handle_media(handler, parsed):
80978097
".signing_key", ".pbkdf2_key", ".sessions.json",
80988098
"google_token.json", "google_client_secret.json",
80998099
}
8100-
# Roots whose contents are sensitive in their entirety.
8100+
# Roots whose contents are sensitive in their entirety. Cover EVERY Hermes
8101+
# state root that the allowlist accepts — not just the active-profile
8102+
# HERMES_HOME. Under a named profile the process HERMES_HOME is
8103+
# ~/.hermes/profiles/<name>, but the allowlist (above) also grants the base
8104+
# ~/.hermes, so without including the base root an attacker-influenced link
8105+
# could still fetch ~/.hermes/state.db or a SIBLING profile's secrets
8106+
# (~/.hermes/profiles/other/auth.json). (Codex review #3234.)
81018107
_state_dir = None
81028108
try:
81038109
from api.config import STATE_DIR as _STATE_DIR
81048110
_state_dir = Path(_STATE_DIR).resolve()
81058111
except Exception:
81068112
_state_dir = None
8107-
_hermes_home_resolved = _HERMES_HOME.resolve()
8108-
_deny_dirs = [d for d in (
8113+
_base_hermes_home = None
8114+
try:
8115+
from api.profiles import _DEFAULT_HERMES_HOME as _BASE_HH
8116+
_base_hermes_home = Path(_BASE_HH).resolve()
8117+
except Exception:
8118+
_base_hermes_home = None
8119+
# All Hermes state roots: active-profile HERMES_HOME, base ~/.hermes,
8120+
# api.profiles default home, and the WebUI STATE_DIR.
8121+
_hermes_roots = []
8122+
for _r in (
8123+
_HERMES_HOME.resolve(),
8124+
(_HOME / ".hermes").resolve(),
8125+
_base_hermes_home,
81098126
_state_dir,
8110-
(_HERMES_HOME / "sessions").resolve(),
8111-
(_HERMES_HOME / "memories").resolve(),
8112-
(_HERMES_HOME / "profiles").resolve(),
8113-
) if d is not None]
8127+
):
8128+
if _r is not None and _r not in _hermes_roots:
8129+
_hermes_roots.append(_r)
8130+
# Dir-based denies: the named state subdirs under every Hermes root, plus
8131+
# the STATE_DIR itself (sensitive in its entirety).
8132+
_deny_dirs = []
8133+
if _state_dir is not None:
8134+
_deny_dirs.append(_state_dir)
8135+
for _root in _hermes_roots:
8136+
for _sub in ("sessions", "memories", "profiles"):
8137+
_deny_dirs.append((_root / _sub).resolve())
81148138
# Filename-based denies only apply to files living under a Hermes/WebUI state
81158139
# root — so a legitimate workspace or /tmp media artifact that happens to be
81168140
# named settings.json / config.yaml is NOT blocked (Codex review #3234).
8117-
_under_state_root = (
8118-
_path_is_within_root(target, _hermes_home_resolved)
8119-
or (_state_dir is not None and _path_is_within_root(target, _state_dir))
8141+
_under_state_root = any(
8142+
_path_is_within_root(target, _root) for _root in _hermes_roots
81208143
)
81218144
_denied = (
81228145
(_under_state_root and target.name in _DENY_FILENAMES)

tests/test_issue1800_file_html_interactions.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,13 @@ def test_html_media_open_full_uses_inline_new_tab_not_download():
6868

6969
def test_media_html_inline_keeps_csp_sandbox():
7070
"""api/media may serve HTML inline only behind a CSP sandbox."""
71-
# Slice widened to 7000 (was 5000) after the #3234 security deny-list block
72-
# was inserted earlier in _handle_media, pushing the CSP block further down
73-
# (the `csp=csp` line now sits ~6050 chars past the def). (Previously widened
71+
# Slice widened to 9000 (was 5000) after the #3234 security deny-list block
72+
# (multi-profile-aware deny roots) was inserted earlier in _handle_media,
73+
# pushing the CSP block to ~7800 chars past the def. (Previously widened
7474
# 4000→5000 for PR #2044's MEDIA_ALLOWED_ROOTS parsing.) The assertion is
75-
# structural, not positional.
76-
body = _slice_after(ROUTES_PY, "def _handle_media", 7000)
75+
# structural, not positional — generous headroom avoids re-breaking on small
76+
# future edits to the early part of _handle_media.
77+
body = _slice_after(ROUTES_PY, "def _handle_media", 9000)
7778
assert 'html_inline_ok = inline_preview and mime == "text/html"' in body
7879
assert 'csp = "sandbox allow-scripts" if html_inline_ok else None' in body
7980
assert "csp=csp" in body

0 commit comments

Comments
 (0)