Denying the whole <root>/profiles tree 403'd legitimate named-profile workspace media (<base>/profiles/p1/workspace/shot.png). Fix: remove 'profiles' from _DENY_SUBDIRS and instead enumerate each <root>/profiles/<name> directory as its own Hermes root — so each profile's sensitive subdirs (sessions/memories/cron/ logs/checkpoints/backups) + secret filenames are denied, while that profile's workspace/ is allowed via the carve-out. Adds a regression test: named-profile workspace media serves, profile + sibling-profile auth.json stay 403.
This commit is contained in:
parent
124044dc11
commit
ce7a2c14db
3 changed files with 100 additions and 9 deletions
|
|
@ -8108,9 +8108,13 @@ def _handle_media(handler, parsed):
|
|||
"google_token.json", "google_client_secret.json",
|
||||
"gateway_state.json", "channel_directory.json", "jobs.json",
|
||||
}
|
||||
# Internal state subdirs that are sensitive in their entirety.
|
||||
# Internal state subdirs that are sensitive in their entirety. NOTE:
|
||||
# `profiles` is intentionally NOT here — it is a container of profile roots,
|
||||
# each of which has its own legitimate workspace/. We instead enumerate each
|
||||
# named-profile root below and deny ITS state subdirs, so a sibling profile's
|
||||
# secrets are blocked without 403-ing a named-profile workspace. (#3234.)
|
||||
_DENY_SUBDIRS = (
|
||||
"sessions", "memories", "profiles", "cron", "logs",
|
||||
"sessions", "memories", "cron", "logs",
|
||||
"checkpoints", "backups",
|
||||
)
|
||||
_state_dir = None
|
||||
|
|
@ -8134,6 +8138,24 @@ def _handle_media(handler, parsed):
|
|||
):
|
||||
if _r is not None and _r not in _hermes_roots:
|
||||
_hermes_roots.append(_r)
|
||||
# Enumerate named-profile roots (<root>/profiles/<name>) and treat each as a
|
||||
# Hermes root in its own right, so a sibling/other profile's sensitive subdirs
|
||||
# + secret files are denied — WITHOUT denying the whole `profiles` container
|
||||
# (which would block a legit named-profile workspace at
|
||||
# <root>/profiles/<name>/workspace/). (Codex review #3234.)
|
||||
_profile_roots = []
|
||||
for _root in list(_hermes_roots):
|
||||
_profiles_dir = (_root / "profiles")
|
||||
try:
|
||||
if _profiles_dir.is_dir():
|
||||
for _pchild in _profiles_dir.iterdir():
|
||||
if _pchild.is_dir():
|
||||
_pr = _pchild.resolve()
|
||||
if _pr not in _hermes_roots and _pr not in _profile_roots:
|
||||
_profile_roots.append(_pr)
|
||||
except OSError:
|
||||
pass
|
||||
_hermes_roots.extend(_profile_roots)
|
||||
|
||||
# Case-insensitive path helpers so STATE.DB / Sessions/ casing variants
|
||||
# cannot bypass the deny on macOS/Windows filesystems (Codex review #3234).
|
||||
|
|
|
|||
|
|
@ -68,13 +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 12000 (was 5000) after the #3234 security work landed a
|
||||
# multi-profile-aware deny-list + active-workspace carve-out + carve-out
|
||||
# safety gate earlier in _handle_media, pushing the CSP block to ~9850 chars
|
||||
# past the def. (Originally widened 4000→5000 for PR #2044's
|
||||
# MEDIA_ALLOWED_ROOTS parsing.) The assertion is structural, not positional —
|
||||
# generous headroom avoids re-breaking on small future edits.
|
||||
body = _slice_after(ROUTES_PY, "def _handle_media", 12000)
|
||||
# Slice widened to 16000 (was 5000) after the #3234 security work landed a
|
||||
# multi-profile-aware deny-list + named-profile-root enumeration +
|
||||
# active-workspace carve-out + safety gate earlier in _handle_media, pushing
|
||||
# the CSP block to ~12100 chars past the def. (Originally widened 4000→5000
|
||||
# for PR #2044's MEDIA_ALLOWED_ROOTS parsing.) The assertion is structural,
|
||||
# not positional — generous headroom avoids re-breaking on small future edits.
|
||||
body = _slice_after(ROUTES_PY, "def _handle_media", 16000)
|
||||
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
|
||||
|
|
|
|||
|
|
@ -404,6 +404,75 @@ class TestMediaEndpointUnit(unittest.TestCase):
|
|||
h2.status, 403,
|
||||
"STATE_DIR/sessions/abc.json must stay denied (internal state)")
|
||||
|
||||
def test_named_profile_workspace_serves_but_profile_secrets_denied(self):
|
||||
"""#3234: a named-profile workspace (<base>/profiles/p1/workspace) is
|
||||
legitimate media and must serve, while that profile's secrets
|
||||
(<base>/profiles/p1/auth.json) and a SIBLING profile's secrets
|
||||
(<base>/profiles/other/auth.json) must stay denied (403).
|
||||
|
||||
Regression for the over-block where the whole `profiles` tree was denied.
|
||||
"""
|
||||
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:
|
||||
base = pathlib.Path(home) / ".hermes"
|
||||
p1_ws = base / "profiles" / "p1" / "workspace"
|
||||
p1_ws.mkdir(parents=True)
|
||||
(p1_ws / "shot.png").write_bytes(png_bytes)
|
||||
p1_secret = base / "profiles" / "p1" / "auth.json"
|
||||
p1_secret.write_text("{}", encoding="utf-8")
|
||||
other_secret = base / "profiles" / "other" / "auth.json"
|
||||
other_secret.parent.mkdir(parents=True)
|
||||
other_secret.write_text("{}", encoding="utf-8")
|
||||
|
||||
active = base / "profiles" / "p1" # active profile HERMES_HOME
|
||||
with mock.patch.dict(os.environ, {"HERMES_HOME": str(active)}), \
|
||||
mock.patch.object(routes, "get_last_workspace", lambda: str(p1_ws)), \
|
||||
mock.patch("api.auth.is_auth_enabled", lambda: False), \
|
||||
mock.patch("api.profiles._DEFAULT_HERMES_HOME", base):
|
||||
# named-profile workspace media → served
|
||||
h1 = _Handler()
|
||||
routes._handle_media(h1, SimpleNamespace(
|
||||
query=f"path={urllib.parse.quote(str((p1_ws / 'shot.png').resolve()))}&inline=1",
|
||||
path="/api/media"))
|
||||
self.assertNotEqual(
|
||||
h1.status, 403,
|
||||
"named-profile workspace media must NOT be blocked")
|
||||
# this profile's own secret → denied
|
||||
h2 = _Handler()
|
||||
routes._handle_media(h2, SimpleNamespace(
|
||||
query=f"path={urllib.parse.quote(str(p1_secret.resolve()))}",
|
||||
path="/api/media"))
|
||||
self.assertEqual(h2.status, 403, "profile auth.json must be denied")
|
||||
# sibling profile's secret → denied
|
||||
h3 = _Handler()
|
||||
routes._handle_media(h3, SimpleNamespace(
|
||||
query=f"path={urllib.parse.quote(str(other_secret.resolve()))}",
|
||||
path="/api/media"))
|
||||
self.assertEqual(h3.status, 403, "sibling profile auth.json must be denied")
|
||||
|
||||
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)
|
||||
|
|
|
|||
Loading…
Reference in a new issue