diff --git a/api/routes.py b/api/routes.py index 08e7a358..06db3f69 100644 --- a/api/routes.py +++ b/api/routes.py @@ -512,6 +512,50 @@ def _session_visible_to_active_profile(session_profile, handler=None) -> bool: return _profiles_match(session_profile, active_profile) +def _request_session_visibility_exempt(method: str, path: str | None) -> bool: + if not path: + return False + if method != "POST": + return False + # Import routes create/claim sessions before normal ownership exists, and + # chat/start has inline placeholder-retag rules that must run before the + # generic request-session guard. + return path in { + "/api/session/import", + "/api/session/import_cli", + "/api/chat/start", + } + + +def _session_id_visible_to_request_profile(handler, sid) -> bool: + """Return whether ``sid`` belongs to the active profile.""" + if not isinstance(sid, str) or not sid: + return True + if not is_safe_session_id(sid): + return True + try: + session = get_session(sid, metadata_only=True) + except KeyError: + return True + if not _session_visible_to_active_profile(getattr(session, "profile", None), handler): + bad(handler, "Session not found", 404) + return False + return True + + +def _guard_request_session_visibility(handler, parsed, body=None, method="GET") -> bool: + """Apply request session-profile visibility check to request-supplied IDs.""" + method = str(method).upper() + if _request_session_visibility_exempt(method, getattr(parsed, "path", "")): + return True + sid = parse_qs(parsed.query).get("session_id", [None])[0] + if not _session_id_visible_to_request_profile(handler, sid): + return False + if isinstance(body, dict) and not _session_id_visible_to_request_profile(handler, body.get("session_id")): + return False + return True + + def _active_skills_dir() -> Path: """Return the skills directory for the request's active Hermes profile. @@ -10012,6 +10056,9 @@ def handle_get(handler, parsed) -> bool: handler.end_headers() return True + if parsed.path.startswith("/api/") and not _guard_request_session_visibility(handler, parsed, method="GET"): + return True + # ── Insights / knowledge status ── if parsed.path == "/api/insights": return _handle_insights(handler, parsed) @@ -11605,6 +11652,10 @@ def handle_post(handler, parsed) -> bool: if diag: diag.finish() raise + if not _guard_request_session_visibility(handler, parsed, body=body, method="POST"): + if diag: + diag.finish() + return True if parsed.path == "/api/escape/authorize": return _handle_escape_authorize(handler, parsed, body) @@ -13758,6 +13809,8 @@ def handle_patch(handler, parsed) -> bool: if not _check_csrf(handler): return j(handler, {"error": _csrf_rejection_error(handler)}, status=403) body = read_body(handler) + if not _guard_request_session_visibility(handler, parsed, body=body, method="PATCH"): + return True if parsed.path.startswith("/api/mcp/servers/"): name = parsed.path[len("/api/mcp/servers/"):] return _handle_mcp_server_toggle(handler, name, body) @@ -13776,6 +13829,8 @@ def handle_delete(handler, parsed) -> bool: if not _check_csrf(handler): return j(handler, {"error": _csrf_rejection_error(handler)}, status=403) body = read_body(handler) + if not _guard_request_session_visibility(handler, parsed, body=body, method="DELETE"): + return True if parsed.path.startswith("/api/mcp/servers/"): name = parsed.path[len("/api/mcp/servers/"):] return _handle_mcp_server_delete(handler, name) @@ -13802,6 +13857,8 @@ def handle_put(handler, parsed) -> bool: if not _check_csrf(handler): return j(handler, {"error": "Cross-origin request rejected"}, status=403) body = read_body(handler) + if not _guard_request_session_visibility(handler, parsed, body=body, method="PUT"): + return True if parsed.path.startswith("/api/mcp/servers/"): name = parsed.path[len("/api/mcp/servers/"):] return _handle_mcp_server_update(handler, name, body) @@ -18049,6 +18106,7 @@ def _handle_chat_start(handler, body, diag=None): return bad(handler, "Read-only imported sessions cannot be continued from WebUI", 403) diag.stage("validate_profile") if diag else None requested_profile = str(body.get("profile") or "").strip() + active_profile = _get_active_profile_name() if requested_profile: try: from api.profiles import _PROFILE_ID_RE @@ -18057,17 +18115,34 @@ def _handle_chat_start(handler, body, diag=None): return bad(handler, "invalid profile", 400) except ImportError: requested_profile = "" - if requested_profile and not _profiles_match(getattr(s, "profile", None), requested_profile): - has_persisted_turns = bool( - getattr(s, "messages", None) - or getattr(s, "context_messages", None) - or getattr(s, "pending_user_message", None) - ) + session_profile = getattr(s, "profile", None) + has_persisted_turns = bool( + getattr(s, "messages", None) + or getattr(s, "context_messages", None) + or getattr(s, "pending_user_message", None) + ) + if not _session_visible_to_active_profile(session_profile, handler): + if ( + requested_profile + and _profiles_match(requested_profile, active_profile) + and not has_persisted_turns + ): + # Empty placeholders can still be retagged when the + # requested profile matches the active request profile. + s.profile = requested_profile + else: + return bad(handler, "Session not found", 404) + elif ( + requested_profile + and _profiles_match(requested_profile, active_profile) + and not _profiles_match(session_profile, requested_profile) + ): + # Empty sessions are placeholders. If the user switches profiles + # before sending the first turn, run the placeholder under the + # active request profile instead of the stale one stamped at + # creation time. The request body alone must not retag a session to + # a different profile than the server-selected active profile. if not has_persisted_turns: - # Empty sessions are placeholders. If the user switches profiles - # before sending the first turn, run the placeholder under the - # currently-selected profile instead of the stale one stamped at - # creation time. s.profile = requested_profile diag.stage("normalize_message") if diag else None msg = str(body.get("message", "")).strip() diff --git a/api/upload.py b/api/upload.py index c9239518..5cd87ae2 100644 --- a/api/upload.py +++ b/api/upload.py @@ -11,6 +11,7 @@ from pathlib import Path from api.config import MAX_UPLOAD_BYTES, STATE_DIR from api.helpers import j, bad from api.models import get_session +from api.profiles import _profiles_match, get_active_profile_name as _get_active_profile_name from api.workspace import ( safe_resolve_ws, resolve_trusted_workspace, @@ -150,6 +151,21 @@ def _session_attachment_dir(session_id: str, *, root: Path | None = None) -> Pat return dest_dir +def _session_visible_to_active_profile(session) -> bool: + """Return whether an upload target session belongs to the active profile.""" + session_profile = getattr(session, 'profile', None) + if not isinstance(session_profile, str): + session_profile = None + return _profiles_match(session_profile, _get_active_profile_name()) + + +def _reject_invisible_session(handler, session) -> bool: + if _session_visible_to_active_profile(session): + return False + j(handler, {'error': 'Session not found'}, status=404) + return True + + def handle_upload(handler): import traceback as _tb try: @@ -168,6 +184,8 @@ def handle_upload(handler): s = get_session(session_id) except KeyError: return j(handler, {'error': 'Session not found'}, status=404) + if _reject_invisible_session(handler, s): + return True safe_name = _sanitize_upload_name(filename) dest = _upload_destination(session_id, safe_name) dest.write_bytes(file_bytes) @@ -344,6 +362,8 @@ def handle_upload_extract(handler): s = get_session(session_id) except KeyError: return j(handler, {'error': 'Session not found'}, status=404) + if _reject_invisible_session(handler, s): + return True session_dir = _session_attachment_dir(session_id) session_dir.mkdir(parents=True, exist_ok=True) result = extract_archive(file_bytes, filename, session_dir) @@ -563,6 +583,8 @@ def handle_workspace_upload(handler): session = get_session(session_id) except KeyError: return j(handler, {'error': 'Session not found'}, status=404) + if _reject_invisible_session(handler, session): + return True # Resolve workspace root from session workspace = resolve_trusted_workspace(session.workspace) diff --git a/tests/test_session_active_profile_authorization.py b/tests/test_session_active_profile_authorization.py new file mode 100644 index 00000000..a2872c0d --- /dev/null +++ b/tests/test_session_active_profile_authorization.py @@ -0,0 +1,261 @@ +"""Regression tests for session-profile visibility on request-scoped session_id uses. + +The security contract is now enforced with a generic preflight for request-supplied +session IDs. These tests cover the most sensitive paths that previously loaded +foreign-profile sessions directly: duplicate, file reads, and chat/start. +""" + +from __future__ import annotations + +import io +from urllib.parse import urlparse + +import api.routes as routes +import api.upload as upload + + +class _FakeHandler: + def __init__(self): + self.status = None + self.headers = {"Content-Type": "multipart/form-data; boundary=test", "Content-Length": "1"} + self.rfile = io.BytesIO(b"") + self.wfile = io.BytesIO() + self.command = "GET" + self.path = "/" + self.client_address = ("127.0.0.1", 12345) + + def send_response(self, status): + self.status = status + + def send_header(self, key, value): + self.headers[key] = value + + def end_headers(self): + pass + + +def _capture(monkeypatch): + cap = {} + + def _j(_h, obj, *_, **__): + cap["ok"] = obj + return True + + def _bad(_h, msg, code=400): + cap["bad"] = (msg, code) + return True + + monkeypatch.setattr(routes, "j", _j) + monkeypatch.setattr(routes, "bad", _bad) + return cap + + +def _capture_upload(monkeypatch): + cap = {} + + def _j(_h, obj, *_, **kwargs): + cap["ok"] = obj + cap["status"] = kwargs.get("status", 200) + return True + + monkeypatch.setattr(upload, "j", _j) + return cap + + +class _SimpleSession: + def __init__(self, sid, profile="default", workspace="/workspace", messages=None, context_messages=None, pending_user_message=None): + self.session_id = sid + self.profile = profile + self.workspace = workspace + self.model = "test-model" + self.model_provider = None + self.title = "Test" + self.messages = messages or [] + self.tool_calls = [] + self.project_id = None + self.context_messages = context_messages + self.pending_user_message = pending_user_message + self.personality = None + self.enabled_toolsets = None + self.context_length = None + self.threshold_tokens = None + self.truncation_watermark = None + self.truncation_boundary = None + self.gateway_routing = None + self.gateway_routing_history = [] + self.llm_title_generated = False + self.manual_title = False + self.composer_draft = {} + self.context_engine = None + self.context_engine_state = {} + self.input_tokens = 0 + self.output_tokens = 0 + self.estimated_cost = 0.0 + self.cache_read_tokens = 0 + self.cache_write_tokens = 0 + + +def test_session_duplicate_foreign_profile_session_blocked_by_visibility_guard(monkeypatch): + handler = _FakeHandler() + foreign = _SimpleSession("foreign_duplicate", profile="other") + monkeypatch.setattr(routes, "get_session", lambda sid, metadata_only=False: foreign) + monkeypatch.setattr(routes, "_get_active_profile_name", lambda: "default") + monkeypatch.setattr(routes, "_check_csrf", lambda _handler: True) + monkeypatch.setattr(routes, "read_body", lambda _handler: {"session_id": "foreign_duplicate"}) + monkeypatch.setattr(routes.Session, "load", staticmethod(lambda sid: (_ for _ in ()).throw(AssertionError("duplicate should not materialize foreign session")))) + + cap = _capture(monkeypatch) + routes.handle_post(handler, urlparse("/api/session/duplicate")) + + assert cap["bad"] == ("Session not found", 404) + + +def test_session_duplicate_same_profile_still_duplicates(monkeypatch): + handler = _FakeHandler() + source = _SimpleSession("session_visible", profile="default") + calls = {"load": 0, "save": 0} + + def _load(_sid): + calls["load"] += 1 + return source + + monkeypatch.setattr(routes.Session, "load", staticmethod(_load)) + monkeypatch.setattr(routes, "get_session", lambda sid, metadata_only=False: source) + monkeypatch.setattr(routes, "_get_active_profile_name", lambda: "default") + monkeypatch.setattr(routes, "_check_csrf", lambda _handler: True) + monkeypatch.setattr(routes, "read_body", lambda _handler: {"session_id": "session_visible"}) + + def _session_save(_self, *_, **__): + calls["save"] += 1 + + monkeypatch.setattr(routes.Session, "save", _session_save) + monkeypatch.setattr(routes, "publish_session_list_changed", lambda *_, **__: None) + + cap = _capture(monkeypatch) + routes.handle_post(handler, urlparse("/api/session/duplicate")) + + assert calls["load"] == 1 + assert calls["save"] == 1 + assert "bad" not in cap + assert cap["ok"]["session"]["session_id"] != "session_visible" + + +def test_file_read_foreign_profile_session_returns_404_before_file_ops(monkeypatch): + handler = _FakeHandler() + foreign = _SimpleSession("foreign_file", profile="other", workspace="/workspace") + monkeypatch.setattr(routes, "get_session", lambda sid, metadata_only=False: foreign) + monkeypatch.setattr(routes, "_get_active_profile_name", lambda: "default") + monkeypatch.setattr(routes, "get_session_for_file_ops", lambda sid: (_ for _ in ()).throw(AssertionError("file ops should not run"))) + cap = _capture(monkeypatch) + + routes.handle_get(handler, urlparse("/api/file?session_id=foreign_file&path=notes.txt")) + + assert cap["bad"] == ("Session not found", 404) + + +def test_chat_start_foreign_persisted_session_returns_404_before_start_run(monkeypatch): + handler = _FakeHandler() + persisted_foreign = _SimpleSession( + "chat_foreign", + profile="other", + messages=[{"role": "user", "content": "first"}], + ) + + monkeypatch.setattr(routes, "_check_csrf", lambda _handler: True) + monkeypatch.setattr(routes, "read_body", lambda _handler: {"session_id": "chat_foreign", "message": "hello"}) + monkeypatch.setattr(routes, "_get_or_materialize_session", lambda sid: persisted_foreign) + monkeypatch.setattr(routes, "_get_active_profile_name", lambda: "default") + monkeypatch.setattr(routes, "_start_run", lambda *_, **__: (_ for _ in ()).throw(AssertionError("_start_run should not run"))) + + cap = _capture(monkeypatch) + routes.handle_post(handler, urlparse("/api/chat/start")) + + assert cap["bad"] == ("Session not found", 404) + + +def test_chat_start_body_profile_cannot_retag_visible_empty_session_without_active_profile(monkeypatch): + handler = _FakeHandler() + empty_visible = _SimpleSession("chat_empty", profile="default", messages=[]) + captured = {} + + monkeypatch.setattr(routes, "_check_csrf", lambda _handler: True) + monkeypatch.setattr( + routes, + "read_body", + lambda _handler: {"session_id": "chat_empty", "message": "hello", "profile": "other"}, + ) + monkeypatch.setattr(routes, "_get_or_materialize_session", lambda sid: empty_visible) + monkeypatch.setattr(routes, "_get_active_profile_name", lambda: "default") + monkeypatch.setattr(routes, "_resolve_chat_workspace_with_recovery", lambda *_args, **_kwargs: "/workspace") + monkeypatch.setattr(routes, "_read_profile_model_config", lambda *_args, **_kwargs: (None, None)) + monkeypatch.setattr( + routes, + "_resolve_compatible_session_model_state", + lambda *_args, **_kwargs: ("test-model", None, "test-model"), + ) + + def _start_run(session, **_kwargs): + captured["profile"] = session.profile + return {"ok": True, "stream_id": "stream-test", "session_id": session.session_id} + + monkeypatch.setattr(routes, "_start_run", _start_run) + + cap = _capture(monkeypatch) + routes.handle_post(handler, urlparse("/api/chat/start")) + + assert "bad" not in cap + assert captured["profile"] == "default" + + +def test_attachment_upload_foreign_profile_session_returns_404_before_write(monkeypatch): + handler = _FakeHandler() + foreign = _SimpleSession("upload_foreign", profile="other") + monkeypatch.setattr(upload, "parse_multipart", lambda *_args: ({"session_id": "upload_foreign"}, {"file": ("note.txt", b"x")})) + monkeypatch.setattr(upload, "get_session", lambda sid: foreign) + monkeypatch.setattr(upload, "_get_active_profile_name", lambda: "default") + monkeypatch.setattr( + upload, + "_upload_destination", + lambda *_args, **_kwargs: (_ for _ in ()).throw(AssertionError("upload destination should not be resolved")), + ) + + cap = _capture_upload(monkeypatch) + upload.handle_upload(handler) + + assert cap == {"ok": {"error": "Session not found"}, "status": 404} + + +def test_archive_upload_extract_foreign_profile_session_returns_404_before_extract(monkeypatch): + handler = _FakeHandler() + foreign = _SimpleSession("extract_foreign", profile="other") + monkeypatch.setattr(upload, "parse_multipart", lambda *_args: ({"session_id": "extract_foreign"}, {"file": ("archive.zip", b"zip")})) + monkeypatch.setattr(upload, "get_session", lambda sid: foreign) + monkeypatch.setattr(upload, "_get_active_profile_name", lambda: "default") + monkeypatch.setattr( + upload, + "extract_archive", + lambda *_args, **_kwargs: (_ for _ in ()).throw(AssertionError("archive extraction should not run")), + ) + + cap = _capture_upload(monkeypatch) + upload.handle_upload_extract(handler) + + assert cap == {"ok": {"error": "Session not found"}, "status": 404} + + +def test_workspace_upload_foreign_profile_session_returns_404_before_workspace_resolution(monkeypatch): + handler = _FakeHandler() + foreign = _SimpleSession("workspace_foreign", profile="other", workspace="/workspace/foreign") + monkeypatch.setattr(upload, "parse_multipart", lambda *_args: ({"session_id": "workspace_foreign"}, {"file": ("note.txt", b"x")})) + monkeypatch.setattr(upload, "get_session", lambda sid: foreign) + monkeypatch.setattr(upload, "_get_active_profile_name", lambda: "default") + monkeypatch.setattr( + upload, + "resolve_trusted_workspace", + lambda *_args, **_kwargs: (_ for _ in ()).throw(AssertionError("workspace should not be resolved")), + ) + + cap = _capture_upload(monkeypatch) + upload.handle_workspace_upload(handler) + + assert cap == {"ok": {"error": "Session not found"}, "status": 404}