mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-16 20:50:18 +00:00
Release PD (v0.51.443): [security] scope session by-id reads + exports to active profile (#3982, #3991) (#4269)
* 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 <Hinotoi-agent@users.noreply.github.com> --------- Co-authored-by: nesquena-hermes <agent@nesquena-hermes> Co-authored-by: Hinotoi-agent <Hinotoi-agent@users.noreply.github.com>
This commit is contained in:
+5
-1
@@ -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
|
||||
|
||||
|
||||
+30
-2
@@ -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)
|
||||
|
||||
@@ -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 ────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user