fix: isolate remote memory writes by principal profile
parent
af62a3ce30
commit
04fdad3053
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue