diff --git a/api/routes.py b/api/routes.py index bc4f6162..b5cdbda8 100644 --- a/api/routes.py +++ b/api/routes.py @@ -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 (/profiles/) 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 + # /profiles//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). diff --git a/tests/test_issue1800_file_html_interactions.py b/tests/test_issue1800_file_html_interactions.py index 5c0e810d..0bee348a 100644 --- a/tests/test_issue1800_file_html_interactions.py +++ b/tests/test_issue1800_file_html_interactions.py @@ -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 diff --git a/tests/test_media_inline.py b/tests/test_media_inline.py index 2d4bc1b9..c3cad01d 100644 --- a/tests/test_media_inline.py +++ b/tests/test_media_inline.py @@ -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 (/profiles/p1/workspace) is + legitimate media and must serve, while that profile's secrets + (/profiles/p1/auth.json) and a SIBLING profile's secrets + (/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)