diff --git a/gateway/platforms/api_server.py b/gateway/platforms/api_server.py index b06a346db..b907e70c3 100644 --- a/gateway/platforms/api_server.py +++ b/gateway/platforms/api_server.py @@ -4801,6 +4801,9 @@ class APIServerAdapter(BasePlatformAdapter): user_name: str = "", session_key: str = "", session_id: str = "", + verified_email: str = "", + dashboard_role: str = "", + profile: str = "", ) -> list: """Bind session contextvars for an API-server agent run. @@ -4821,11 +4824,15 @@ class APIServerAdapter(BasePlatformAdapter): return set_session_vars( platform="api_server", + source="api_server", chat_id=chat_id, user_id=user_id or chat_id, user_name=user_name or user_id or chat_id, session_key=session_key, session_id=session_id, + verified_email=verified_email, + dashboard_role=dashboard_role, + profile=profile, async_delivery=False, ) @@ -4876,6 +4883,9 @@ class APIServerAdapter(BasePlatformAdapter): user_name=str(identity_context.get("user_name") or identity_context.get("runtime_identity") or session_id or ""), session_key=gateway_session_key or session_id or "", session_id=session_id or "", + verified_email=str(identity_context.get("email") or ""), + dashboard_role=str(identity_context.get("dashboard_role") or ""), + profile=str(request_profile or ""), ) try: agent = self._create_agent( diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 85c11ad09..a2ce9b207 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -711,6 +711,21 @@ def _session_visible_to_scope(session: dict[str, Any] | None, scope: dict[str, A return source in set(scope.get("platforms") or []) and user_id in set(scope.get("external_user_ids") or []) +def _require_authorized_session( + db: Any, + session_id: str, + scope: dict[str, Any], +) -> tuple[str, dict[str, Any]]: + """Resolve a session ID and enforce owner visibility in one place.""" + sid = db.resolve_session_id(session_id) + session = db.get_session(sid) if sid else None + if not session: + raise HTTPException(status_code=404, detail="Session not found") + if not _session_visible_to_scope(session, scope): + raise HTTPException(status_code=403, detail="forbidden") + return sid, session + + def _deidentify_session_search_result(item: dict[str, Any]) -> dict[str, Any]: masked = dict(item) for key in ("snippet", "preview", "title"): @@ -10527,12 +10542,7 @@ async def get_session_detail(request: Request, session_id: str, profile: Optiona _enforce_dashboard_profile_access(scope, profile) db = _open_session_db_for_profile(profile) try: - sid = db.resolve_session_id(session_id) - session = db.get_session(sid) if sid else None - if not session: - raise HTTPException(status_code=404, detail="Session not found") - if not _session_visible_to_scope(session, scope): - raise HTTPException(status_code=403, detail="forbidden") + sid, session = _require_authorized_session(db, session_id, scope) if profile: session["profile"] = _cron_profile_home(profile)[0] return session @@ -10551,13 +10561,8 @@ async def get_session_latest_descendant( _enforce_dashboard_profile_access(scope, profile) db = _open_session_db_for_profile(profile) try: - base_sid = db.resolve_session_id(session_id) - base = db.get_session(base_sid) if base_sid else None - if not base: - raise HTTPException(status_code=404, detail="Session not found") - if not _session_visible_to_scope(base, scope): - raise HTTPException(status_code=403, detail="forbidden") - latest, path = _session_latest_descendant(session_id, db) + base_sid, _base = _require_authorized_session(db, session_id, scope) + latest, path = _session_latest_descendant(base_sid, db) if not latest: raise HTTPException(status_code=404, detail="Session not found") return { @@ -10581,12 +10586,7 @@ async def get_session_messages( _enforce_dashboard_profile_access(scope, profile) db = _open_session_db_for_profile(profile) try: - sid = db.resolve_session_id(session_id) - if not sid: - raise HTTPException(status_code=404, detail="Session not found") - session = db.get_session(sid) - if not _session_visible_to_scope(session, scope): - raise HTTPException(status_code=403, detail="forbidden") + sid, _session = _require_authorized_session(db, session_id, scope) sid = db.resolve_resume_session_id(sid) # Clamp limit to prevent abuse (max 500 per page) _limit = min(limit, 500) if limit is not None else None @@ -10625,9 +10625,7 @@ async def delete_session_endpoint(request: Request, session_id: str, profile: Op sid = db.resolve_session_id(session_id) if not sid: return {"ok": True, "already_absent": True} - session = db.get_session(sid) - if not _session_visible_to_scope(session, scope): - raise HTTPException(status_code=403, detail="forbidden") + _require_authorized_session(db, sid, scope) db.delete_session(sid) return {"ok": True} finally: @@ -10654,12 +10652,7 @@ async def rename_session_endpoint(request: Request, session_id: str, body: Sessi _enforce_dashboard_profile_access(scope, body.profile) db = _open_session_db_for_profile(body.profile) try: - sid = db.resolve_session_id(session_id) - if not sid: - raise HTTPException(status_code=404, detail="Session not found") - session = db.get_session(sid) - if not _session_visible_to_scope(session, scope): - raise HTTPException(status_code=403, detail="forbidden") + sid, _session = _require_authorized_session(db, session_id, scope) if body.title is None and body.archived is None: raise HTTPException( status_code=400, @@ -10688,12 +10681,7 @@ async def export_session_endpoint(request: Request, session_id: str, profile: Op _enforce_dashboard_profile_access(scope, profile) db = _open_session_db_for_profile(profile) try: - sid = db.resolve_session_id(session_id) - if not sid: - raise HTTPException(status_code=404, detail="Session not found") - session = db.get_session(sid) - if not _session_visible_to_scope(session, scope): - raise HTTPException(status_code=403, detail="forbidden") + sid, _session = _require_authorized_session(db, session_id, scope) data = db.export_session(sid) if data is None: raise HTTPException(status_code=404, detail="Session not found") diff --git a/tests/hermes_cli/test_web_server_session_access.py b/tests/hermes_cli/test_web_server_session_access.py index 3dac71e4a..8cac70ed8 100644 --- a/tests/hermes_cli/test_web_server_session_access.py +++ b/tests/hermes_cli/test_web_server_session_access.py @@ -61,6 +61,25 @@ class TestApiServerSessionVisibility: class TestExportEndpointDeidentify: + def test_export_endpoint_forbids_other_users_session(self, monkeypatch): + class _DB: + def resolve_session_id(self, session_id): + return session_id + + def get_session(self, session_id): + return {"id": session_id, "source": "email", "user_id": "other@bremen.com.tw", "chat_id": "other@bremen.com.tw"} + + def close(self): + return None + + monkeypatch.setattr(web_server, "_dashboard_identity_scope", lambda request: {"admin": False, "email": "dk96@bremen.com.tw", "platforms": [], "external_user_ids": [], "principal_ids": []}) + monkeypatch.setattr(web_server, "_enforce_dashboard_profile_access", lambda scope, profile: None) + monkeypatch.setattr(web_server, "_open_session_db_for_profile", lambda profile: _DB()) + + with pytest.raises(HTTPException) as exc: + asyncio.run(web_server.export_session_endpoint(_request("dk96@bremen.com.tw"), "sess-1")) + assert exc.value.status_code == 403 + def test_non_admin_export_masks_payload(self, monkeypatch): class _DB: def resolve_session_id(self, session_id): @@ -82,3 +101,37 @@ class TestExportEndpointDeidentify: out = asyncio.run(web_server.export_session_endpoint(_request("dk96@bremen.com.tw"), "sess-1")) assert "john.doe@bremen.com.tw" not in str(out) assert "0912-345-678" not in str(out) + + +class TestAuthorizedSessionHelper: + def test_helper_allows_visible_session(self): + class _DB: + def resolve_session_id(self, session_id): + return session_id + + def get_session(self, session_id): + return {"id": session_id, "source": "telegram", "user_id": "alice"} + + sid, session = web_server._require_authorized_session( + _DB(), + "sess-1", + {"admin": False, "platforms": ["telegram"], "external_user_ids": ["alice"]}, + ) + assert sid == "sess-1" + assert session["id"] == "sess-1" + + def test_helper_rejects_foreign_session(self): + class _DB: + def resolve_session_id(self, session_id): + return session_id + + def get_session(self, session_id): + return {"id": session_id, "source": "telegram", "user_id": "bob"} + + with pytest.raises(HTTPException) as exc: + web_server._require_authorized_session( + _DB(), + "sess-1", + {"admin": False, "platforms": ["telegram"], "external_user_ids": ["alice"]}, + ) + assert exc.value.status_code == 403 diff --git a/tests/tools/test_memory_tool.py b/tests/tools/test_memory_tool.py index c2903f375..e41bd3745 100644 --- a/tests/tools/test_memory_tool.py +++ b/tests/tools/test_memory_tool.py @@ -3,6 +3,7 @@ import json import pytest from pathlib import Path +from gateway.session_context import clear_session_vars, set_session_vars from tools.memory_tool import ( MemoryStore, @@ -592,6 +593,64 @@ class TestMemoryToolDispatcher: assert "content is required" in result["error"] assert "current_entries" not in result + def test_remote_default_profile_write_blocked(self, store): + tokens = set_session_vars( + platform="telegram", + source="telegram", + user_id="alice", + chat_id="chat-a", + profile="default", + ) + try: + result = json.loads(memory_tool(action="add", target="memory", content="do not persist here", store=store)) + finally: + clear_session_vars(tokens) + + assert result["success"] is False + assert "global system profile memory" in result["error"] + + def test_remote_own_principal_profile_write_allowed(self, store, monkeypatch): + class _FakeStore: + def get_principal_context(self, source): + return {"principal_id": "p-123"} + + monkeypatch.setattr("gateway.user_verification.GatewayUserStore", lambda: _FakeStore()) + tokens = set_session_vars( + platform="telegram", + source="telegram", + user_id="alice", + chat_id="chat-a", + profile="principal_p123", + ) + try: + result = json.loads(memory_tool(action="add", target="memory", content="allowed fact", store=store)) + finally: + clear_session_vars(tokens) + + assert result["success"] is True + assert "allowed fact" in store.memory_entries + + def test_remote_other_principal_profile_write_blocked(self, store, monkeypatch): + class _FakeStore: + def get_principal_context(self, source): + return {"principal_id": "p-123"} + + monkeypatch.setattr("gateway.user_verification.GatewayUserStore", lambda: _FakeStore()) + tokens = set_session_vars( + platform="telegram", + source="telegram", + user_id="alice", + chat_id="chat-a", + profile="principal_other999", + ) + try: + result = json.loads(memory_tool(action="add", target="memory", content="blocked fact", store=store)) + finally: + clear_session_vars(tokens) + + assert result["success"] is False + assert "another principal" in result["error"] + class TestMemoryBatch: """The 'operations' batch shape: atomic, all-or-nothing, final-budget.""" diff --git a/tools/memory_tool.py b/tools/memory_tool.py index 08eeaa470..d6eaeecaf 100644 --- a/tools/memory_tool.py +++ b/tools/memory_tool.py @@ -59,6 +59,110 @@ def get_memory_dir() -> Path: ENTRY_DELIMITER = "\n§\n" +def _session_memory_scope() -> Dict[str, str]: + """Return the current session identity/profile scope for memory-write policy. + + The built-in memory files are profile-scoped via ``get_hermes_home()``. + On shared surfaces (Telegram / LINE / email / OpenWebUI / api_server), + writing while scoped to ``default`` would mutate the global system profile, + which must remain admin-local only under the profile-isolation design. + """ + try: + from gateway.session_context import get_session_env + except Exception: + return {} + + def _get(name: str) -> str: + try: + return str(get_session_env(name, "") or "").strip() + except Exception: + return "" + + return { + "platform": _get("HERMES_SESSION_PLATFORM"), + "source": _get("HERMES_SESSION_SOURCE"), + "chat_id": _get("HERMES_SESSION_CHAT_ID"), + "thread_id": _get("HERMES_SESSION_THREAD_ID"), + "user_id": _get("HERMES_SESSION_USER_ID"), + "user_name": _get("HERMES_SESSION_USER_NAME"), + "verified_email": _get("HERMES_SESSION_VERIFIED_EMAIL").lower(), + "dashboard_role": _get("HERMES_SESSION_DASHBOARD_ROLE").lower(), + "profile": _get("HERMES_SESSION_PROFILE"), + } + + +def _is_remote_memory_session(ctx: Dict[str, str]) -> bool: + return bool((ctx.get("platform") or ctx.get("source")) and (ctx.get("user_id") or ctx.get("chat_id"))) + + +def _expected_principal_profile_for_session(ctx: Dict[str, str]) -> str: + """Resolve the principal profile that belongs to the live caller, if any.""" + try: + from gateway.principal_profiles import principal_profile_name + from gateway.session import SessionSource + from gateway.user_verification import GatewayUserStore + + platform = str(ctx.get("platform") or ctx.get("source") or "").strip() + user_id = str(ctx.get("user_id") or ctx.get("chat_id") or "").strip() + chat_id = str(ctx.get("chat_id") or user_id).strip() + if not platform or not chat_id: + return "" + source = SessionSource( + platform=platform, + chat_id=chat_id, + user_id=user_id or chat_id, + user_name=str(ctx.get("user_name") or "").strip() or None, + chat_type="dm", + thread_id=str(ctx.get("thread_id") or "").strip() or None, + ) + context = GatewayUserStore().get_principal_context(source) or {} + principal_id = str(context.get("principal_id") or "").strip() + if not principal_id: + return "" + return principal_profile_name(principal_id) + except Exception: + return "" + + +def _enforce_memory_profile_isolation(action: str, target: str) -> Optional[str]: + """Block remote writes that would mutate the global system profile. + + Remote/shared-surface callers may only persist durable memory while scoped + to their OWN principal profile. The built-in ``default`` profile acts as + the system/global layer and must not absorb user-specific facts. + """ + if action not in {"add", "replace", "remove"}: + return None + ctx = _session_memory_scope() + if not _is_remote_memory_session(ctx): + return None + + profile = str(ctx.get("profile") or "").strip() + if profile in {"", "default"}: + return tool_error( + "Remote users cannot edit the global system profile memory. Resolve the caller to their own principal profile before saving durable memory.", + success=False, + ) + if not profile.startswith("principal_"): + return tool_error( + "Remote users may only write durable memory inside their own principal profile.", + success=False, + ) + + expected = _expected_principal_profile_for_session(ctx) + if not expected: + return tool_error( + "Verified principal required before writing durable memory on shared surfaces.", + success=False, + ) + if profile != expected: + return tool_error( + "You cannot write durable memory into another principal's profile.", + success=False, + ) + return None + + # --------------------------------------------------------------------------- # Memory content scanning — lightweight check for injection/exfiltration # in content that gets injected into the system prompt. @@ -990,6 +1094,9 @@ def memory_tool( if operations: if not isinstance(operations, list): return tool_error("operations must be a list of {action, content?, old_text?} objects.", success=False) + isolation_err = _enforce_memory_profile_isolation("add", target) + if isolation_err is not None: + return isolation_err gate_result = _apply_batch_write_gate(target, operations) if gate_result is not None: return gate_result @@ -1013,6 +1120,10 @@ def memory_tool( if action == "remove" and not old_text: return _missing_old_text_error(store, target, "remove") + isolation_err = _enforce_memory_profile_isolation(action, target) + if isolation_err is not None: + return isolation_err + # Approval gate: when on, stages the write (background/gateway) or prompts # inline (interactive CLI); when off (default) passes straight through. gate_result = _apply_write_gate(action, target, content, old_text)