From 2a3baa71b81ca92da8ece8616a09f15894beec71 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 15 Jun 2026 14:22:47 -0700 Subject: [PATCH] Release PD (v0.51.443): [security] scope session by-id reads + exports to active profile (#3982, #3991) (#4269) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * stage-3982-3991: [security] scope session detail-reads + exports to active profile (paired, re-cut onto v0.51.442, conflict-resolved properly) * Release PD: [security] scope session by-id reads + exports to active profile (#3982, #3991) Paired Hinotoi-agent security PRs, re-cut onto v0.51.442 (avoided the stale-base revert of the #4171 passkey gate that Codex caught). Both /api/session and /api/session/export now apply _profiles_match→404. Codex SAFE + Opus SHIP + 49 tests green; passkey gate verified intact. Co-authored-by: Hinotoi-agent --------- Co-authored-by: nesquena-hermes Co-authored-by: Hinotoi-agent --- CHANGELOG.md | 6 +- api/routes.py | 32 ++- ...est_issue1611_session_profile_filtering.py | 223 ++++++++++++++++++ ...test_issue2762_state_sync_profile_kwarg.py | 1 + tests/test_webui_state_db_reconciliation.py | 1 + 5 files changed, 260 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 204edae86..ae9a06b75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,11 @@ ## [Unreleased] -## [v0.51.442] — 2026-06-15 — Release PC (require auth for passkey enrollment) +## [v0.51.443] — 2026-06-15 — Release PD (scope session by-id reads + exports to active profile) + +### Security + +- **Direct session reads and exports now honor the active-profile boundary, like the session list already does.** `/api/sessions` scopes its rows to the request's active Hermes profile, but two by-id endpoints did not: `GET /api/session?session_id=…` (transcript/metadata read, including the CLI-session fallback) and `GET /api/session/export?session_id=…` (full transcript JSON download) loaded a session purely by id. A stale, leaked, or guessed session id from another profile could therefore disclose that profile's transcript or export. Both paths now apply the same `_profiles_match(...)` check and return the same `404` used for missing sessions (so they don't reveal foreign-profile session existence). Default/legacy-root profile aliasing is preserved, and the `/api/sessions` list path is unchanged. (#3982, #3991) ### Security diff --git a/api/routes.py b/api/routes.py index dd07d9b5b..de239d5a9 100644 --- a/api/routes.py +++ b/api/routes.py @@ -424,7 +424,11 @@ def _visible_pinned_lineage_ids(session_rows) -> set[str]: # (mcp_server.py) can import it without duplicating the visibility model. # Re-exported here so existing `_profiles_match(...)` call sites in this # module keep resolving without per-call-site refactors. -from api.profiles import _profiles_match # noqa: F401, E402 (re-export) +from api.profiles import ( # noqa: F401, E402 (re-export) + _profiles_match, + get_active_profile_name, + get_active_profile_name as _get_active_profile_name, +) def _all_profiles_query_flag(parsed_url) -> bool: @@ -438,6 +442,22 @@ def _all_profiles_query_flag(parsed_url) -> bool: return raw in ('1', 'true', 'yes', 'on') +def _session_visible_to_active_profile(session_profile, handler=None) -> bool: + """Return whether a detail-load session belongs to the active profile. + + Real request handlers must enforce the same profile boundary as + /api/sessions, even when the request has no hermes_profile cookie and the + process-level active profile is the default/root profile. Direct unit-callers + without a request handler keep the historical metadata-load behavior. + """ + if handler is None: + return True + active_profile = _get_active_profile_name() + if not isinstance(session_profile, str): + session_profile = None + return _profiles_match(session_profile, active_profile) + + def _active_skills_dir() -> Path: """Return the skills directory for the request's active Hermes profile. @@ -6829,6 +6849,9 @@ def handle_get(handler, parsed) -> bool: try: _t1 = _time.monotonic() s = get_session(sid, metadata_only=(not load_messages)) + _session_profile = getattr(s, 'profile', None) or None + if not _session_visible_to_active_profile(_session_profile, handler): + return bad(handler, "Session not found", 404) original_stream_id = getattr(s, "active_stream_id", None) _clear_stale_stream_state(s) cli_meta = _lookup_cli_session_metadata(sid) if _session_requires_cli_metadata_lookup(s) else {} @@ -6836,7 +6859,6 @@ def handle_get(handler, parsed) -> bool: cli_messages = [] state_db_messages = [] metadata_summary = None - _session_profile = getattr(s, 'profile', None) or None if is_messaging_session: cli_messages = get_cli_session_messages(sid) elif load_messages: @@ -7124,6 +7146,9 @@ def handle_get(handler, parsed) -> bool: if _was_webui_session: return bad(handler, "Session not found", 404) cli_meta = _lookup_cli_session_metadata(sid) + _session_profile = (cli_meta or {}).get("profile") or None + if not _session_visible_to_active_profile(_session_profile, handler): + return bad(handler, "Session not found", 404) msgs = get_cli_session_messages(sid) if msgs: sess = { @@ -10196,6 +10221,9 @@ def _handle_session_export(handler, parsed): s = get_session(sid) except KeyError: return bad(handler, "Session not found", 404) + active_profile = get_active_profile_name() + if not _profiles_match(getattr(s, "profile", None), active_profile): + return bad(handler, "Session not found", 404) safe = redact_session_data(s.__dict__) payload = json.dumps(safe, ensure_ascii=False, indent=2) handler.send_response(200) diff --git a/tests/test_issue1611_session_profile_filtering.py b/tests/test_issue1611_session_profile_filtering.py index 6c2771872..3595a7a08 100644 --- a/tests/test_issue1611_session_profile_filtering.py +++ b/tests/test_issue1611_session_profile_filtering.py @@ -14,6 +14,8 @@ all_profiles=1 opt-in path. End-to-end HTTP-level tests live separately under tests/test_sessions_endpoint.py if/when added. """ +from types import SimpleNamespace +from unittest.mock import patch from urllib.parse import urlparse import pytest @@ -209,6 +211,227 @@ def test_static_sessions_js_trusts_server_profile_scoping(): ) +# ── Direct session access must also honor active profile ─────────────────── + + +class _ProfileScopedSession: + def __init__(self, session_id="foreign_001", profile="other"): + self.session_id = session_id + self.profile = profile + self.active_stream_id = None + self.messages = [{"role": "user", "content": "foreign profile secret"}] + self.tool_calls = [] + self.pending_user_message = None + self.pending_attachments = [] + self.pending_started_at = None + self.context_length = 0 + self.threshold_tokens = 0 + self.last_prompt_tokens = 0 + + def compact(self, *args, **kwargs): + return { + "session_id": self.session_id, + "title": "Foreign session", + "profile": self.profile, + "workspace": "/tmp/foreign", + "model": "gpt-test", + "message_count": len(self.messages), + } + + +def test_get_session_rejects_session_from_inactive_profile(): + """A known session_id from another profile must not bypass /api/sessions scoping. + + /api/sessions already filters rows by active profile. The detail endpoint + must apply the same check after loading the sidecar; otherwise a stale URL or + guessed id can disclose another profile's transcript. + """ + import api.routes as routes + + captured = {} + + def fake_bad(_handler, message, status=400, **_kwargs): + captured["bad"] = {"message": message, "status": status} + return captured["bad"] + + def fake_j(_handler, data, status=200, **_kwargs): + captured["json"] = {"data": data, "status": status} + return captured["json"] + + parsed = urlparse("/api/session?session_id=foreign_001&messages=1&resolve_model=0") + with patch("api.routes._get_active_profile_name", return_value="default"), \ + patch("api.routes.get_session", return_value=_ProfileScopedSession()), \ + patch("api.routes._clear_stale_stream_state", return_value=False), \ + patch("api.routes._lookup_cli_session_metadata", return_value={}), \ + patch("api.routes.get_state_db_session_messages", return_value=[]), \ + patch("api.routes.bad", side_effect=fake_bad), \ + patch("api.routes.j", side_effect=fake_j): + routes.handle_get(SimpleNamespace(headers={"Cookie": "hermes_profile=default"}), parsed) + + assert captured.get("bad", {}).get("status") == 404 + assert "json" not in captured, "foreign-profile transcript must not be returned" + + +def test_get_session_rejects_metadata_only_session_from_inactive_profile(): + """Metadata-only loads must not bypass the active-profile boundary.""" + import api.routes as routes + + captured = {} + + def fake_bad(_handler, message, status=400, **_kwargs): + captured["bad"] = {"message": message, "status": status} + return captured["bad"] + + def fake_j(_handler, data, status=200, **_kwargs): + captured["json"] = {"data": data, "status": status} + return captured["json"] + + parsed = urlparse("/api/session?session_id=foreign_001&messages=0&resolve_model=0") + with patch("api.routes._get_active_profile_name", return_value="default"), \ + patch("api.routes.get_session", return_value=_ProfileScopedSession()), \ + patch("api.routes.bad", side_effect=fake_bad), \ + patch("api.routes.j", side_effect=fake_j): + routes.handle_get(SimpleNamespace(headers={"Cookie": "hermes_profile=default"}), parsed) + + assert captured.get("bad", {}).get("status") == 404 + assert "json" not in captured, "foreign-profile metadata must not be returned" + + +def test_get_session_rejects_cookieless_session_from_inactive_profile(): + """Cookieless requests must still enforce the active-profile boundary.""" + import api.routes as routes + + captured = {} + + def fake_bad(_handler, message, status=400, **_kwargs): + captured["bad"] = {"message": message, "status": status} + return captured["bad"] + + def fake_j(_handler, data, status=200, **_kwargs): + captured["json"] = {"data": data, "status": status} + return captured["json"] + + parsed = urlparse("/api/session?session_id=foreign_001&messages=0&resolve_model=0") + with patch("api.routes._get_active_profile_name", return_value="default"), \ + patch("api.routes.get_session", return_value=_ProfileScopedSession()), \ + patch("api.routes.bad", side_effect=fake_bad), \ + patch("api.routes.j", side_effect=fake_j): + routes.handle_get(SimpleNamespace(headers={}), parsed) + + assert captured.get("bad", {}).get("status") == 404 + assert "json" not in captured, "cookieless foreign-profile metadata must not be returned" + + +def test_get_session_rejects_cli_session_from_inactive_profile(): + """CLI fallback responses must use the same active-profile boundary.""" + import api.routes as routes + + captured = {} + + def fake_bad(_handler, message, status=400, **_kwargs): + captured["bad"] = {"message": message, "status": status} + return captured["bad"] + + def fake_j(_handler, data, status=200, **_kwargs): + captured["json"] = {"data": data, "status": status} + return captured["json"] + + parsed = urlparse("/api/session?session_id=cli_foreign&messages=1&resolve_model=0") + with patch("api.routes._get_active_profile_name", return_value="default"), \ + patch("api.routes.get_session", side_effect=KeyError), \ + patch("api.routes.SESSION_INDEX_FILE", SimpleNamespace(exists=lambda: False)), \ + patch("api.routes._lookup_cli_session_metadata", return_value={"profile": "other"}), \ + patch("api.routes.get_cli_session_messages", return_value=[{"role": "user", "content": "foreign profile secret"}]), \ + patch("api.routes.bad", side_effect=fake_bad), \ + patch("api.routes.j", side_effect=fake_j): + routes.handle_get(SimpleNamespace(headers={"Cookie": "hermes_profile=default"}), parsed) + + assert captured.get("bad", {}).get("status") == 404 + assert "json" not in captured, "foreign-profile CLI transcript must not be returned" + + +# ── Direct session export must also honor active profile ───────────────── + + +class _ExportCaptureHandler: + def __init__(self): + self.headers = {} + self.status = None + self.sent_headers = [] + self.ended = False + self.wfile = SimpleNamespace(write=self._write) + self.body = b"" + + def send_response(self, status): + self.status = status + + def send_header(self, name, value): + self.sent_headers.append((name, value)) + + def end_headers(self): + self.ended = True + + def _write(self, data): + self.body += data + + +def test_session_export_rejects_session_from_inactive_profile(): + """A known session_id from another profile must not bypass /api/sessions scoping. + + /api/sessions hides foreign-profile rows by default, but the export endpoint + loaded directly by id and serialized the sidecar. It must apply the same + active-profile check before writing the JSON attachment. + """ + import api.routes as routes + + captured = {} + + def fake_bad(_handler, message, status=400, **_kwargs): + captured["bad"] = {"message": message, "status": status} + return captured["bad"] + + foreign = SimpleNamespace( + session_id="foreign_export_001", + profile="other", + messages=[{"role": "user", "content": "foreign profile secret"}], + ) + + handler = _ExportCaptureHandler() + parsed = urlparse("/api/session/export?session_id=foreign_export_001") + with patch("api.routes.get_session", return_value=foreign), \ + patch("api.routes.get_active_profile_name", return_value="default"), \ + patch("api.routes.bad", side_effect=fake_bad): + routes._handle_session_export(handler, parsed) + + assert captured.get("bad", {}).get("status") == 404 + assert handler.status is None + assert handler.body == b"" + + +def test_session_export_allows_session_from_active_profile(): + """Same-profile exports still stream the redacted JSON attachment.""" + import api.routes as routes + + active = SimpleNamespace( + session_id="active_export_001", + profile="default", + messages=[{"role": "user", "content": "same profile content"}], + ) + + handler = _ExportCaptureHandler() + parsed = urlparse("/api/session/export?session_id=active_export_001") + with patch("api.routes.get_session", return_value=active), \ + patch("api.routes.get_active_profile_name", return_value="default"), \ + patch("api.routes.redact_session_data", side_effect=lambda data: data): + routes._handle_session_export(handler, parsed) + + assert handler.status == 200 + assert handler.ended is True + assert b"same profile content" in handler.body + assert ("Cache-Control", "no-store") in handler.sent_headers + + + # ── Cleanup ──────────────────────────────────────────────────────────────── diff --git a/tests/test_issue2762_state_sync_profile_kwarg.py b/tests/test_issue2762_state_sync_profile_kwarg.py index 487941e1d..b6f82f308 100644 --- a/tests/test_issue2762_state_sync_profile_kwarg.py +++ b/tests/test_issue2762_state_sync_profile_kwarg.py @@ -315,6 +315,7 @@ def test_api_session_metadata_only_passes_session_profile_to_summary( ) session.save(touch_updated_at=False) monkeypatch.setattr(routes_mod, "_lookup_cli_session_metadata", lambda _sid: {}) + monkeypatch.setattr(routes_mod, "_get_active_profile_name", lambda: "maiko") class Handler: path = f"/api/session?session_id={sid}&messages=0&resolve_model=0" diff --git a/tests/test_webui_state_db_reconciliation.py b/tests/test_webui_state_db_reconciliation.py index ef0baa070..d159b6ef0 100644 --- a/tests/test_webui_state_db_reconciliation.py +++ b/tests/test_webui_state_db_reconciliation.py @@ -201,6 +201,7 @@ def test_deferred_session_model_resolution_uses_profile_provider(monkeypatch, tm "_resolve_context_length_for_session_model", lambda *_args, **_kwargs: 0, ) + monkeypatch.setattr(routes, "_get_active_profile_name", lambda: "anthropic") session_path = tmp_path / "sessions" / f"{sid}.json" before = session_path.read_text(encoding="utf-8")