fix(session_search): a bare session id never reads another profile's state.db
Reading a session by id that missed the caller's store fell through to _locate_session_db(), which opened every profile's state.db read-only and returned the first owner's full transcript — no opt-in, no profile named, and the miss path even fired after an explicit non-matching profile= read. Any caller holding an id (ids appear in logs and tool output) could read a foreign profile's conversation. Profiles are isolated islands by design. A miss now stays a miss, with a hint to name the owning profile (profile=<name> / @session:<profile>/<id>), which remains the sanctioned, explicit cross-profile read. The schema eval runner no longer needs to fake the scan. Reported by the #106761 filer; reproduced by @kokhlo. Refs #87779.
This commit is contained in:
@@ -85,21 +85,7 @@ def load_arm(path: Path, name: str, work_db_path: Path):
|
||||
return SessionDB(db_path=work_db_path, read_only=True)
|
||||
raise ValueError(f"profile '{profile}' does not exist")
|
||||
|
||||
def _fake_locate_session_db(session_id):
|
||||
try:
|
||||
db = SessionDB(db_path=work_db_path, read_only=True)
|
||||
row = db._conn.execute(
|
||||
"SELECT 1 FROM sessions WHERE id = ?", (session_id,)
|
||||
).fetchone()
|
||||
if row:
|
||||
return db, "work"
|
||||
db.close()
|
||||
except Exception:
|
||||
pass
|
||||
return None, None
|
||||
|
||||
mod._resolve_profile_db = _fake_resolve_profile_db
|
||||
mod._locate_session_db = _fake_locate_session_db
|
||||
return mod
|
||||
|
||||
|
||||
|
||||
@@ -523,14 +523,14 @@ class TestCrossProfileRead:
|
||||
monkeypatch.setattr(profiles_mod, "profile_exists", lambda n: exists)
|
||||
monkeypatch.setattr(profiles_mod, "get_profile_dir", lambda n: home)
|
||||
|
||||
def test_bare_id_locates_across_profiles(self, db, tmp_path, monkeypatch):
|
||||
# The real-world failure: model dropped the owning profile and passed a
|
||||
# bare id. The tool must scan profiles and find it anyway.
|
||||
def test_bare_id_never_reads_another_profiles_store(self, db, tmp_path, monkeypatch):
|
||||
# #106761: profiles are isolated islands. A bare id that misses the caller's
|
||||
# store must NOT be located by scanning every other profile's state.db.
|
||||
other_home = tmp_path / "asdf_home"
|
||||
other_home.mkdir()
|
||||
other = SessionDB(other_home / "state.db")
|
||||
other.create_session("s_far", source="cli")
|
||||
other.append_message("s_far", role="user", content="hi")
|
||||
other.append_message("s_far", role="user", content="secret")
|
||||
other._conn.commit()
|
||||
|
||||
from collections import namedtuple
|
||||
@@ -539,12 +539,15 @@ class TestCrossProfileRead:
|
||||
monkeypatch.setattr(profiles_mod, "get_profile_dir", lambda n: tmp_path / "default_home")
|
||||
monkeypatch.setattr(profiles_mod, "list_profiles", lambda: [Info("asdf", other_home)])
|
||||
|
||||
# `db` (current profile) lacks s_far; no profile passed → scan finds it.
|
||||
result = json.loads(session_search(session_id="s_far", db=db))
|
||||
assert result["success"] is True
|
||||
assert result["mode"] == "read"
|
||||
assert result["profile"] == "asdf"
|
||||
assert result["success"] is False
|
||||
assert "messages" not in result and "secret" not in json.dumps(result)
|
||||
assert "profile=" in result["error"]
|
||||
|
||||
# Naming the owning profile is still the sanctioned cross-profile read.
|
||||
self._patch_profiles(monkeypatch, other_home)
|
||||
named = json.loads(session_search(session_id="s_far", profile="asdf", db=db))
|
||||
assert named["success"] is True and named["message_count"] == 1
|
||||
|
||||
def test_combined_value_autosplits(self, db, tmp_path, monkeypatch):
|
||||
# Agent passed the raw "@session:<profile>/<id>" value as session_id with
|
||||
|
||||
@@ -339,32 +339,6 @@ def _resolve_profile_db(profile: str):
|
||||
return SessionDB(db_path=profiles_mod.get_profile_dir(canon) / "state.db", read_only=True)
|
||||
|
||||
|
||||
def _locate_session_db(session_id: str):
|
||||
"""Scan every profile's ``state.db`` -> ``(db, profile_name)`` or ``(None, None)``.
|
||||
Ids are globally unique, so the first hit is authoritative."""
|
||||
from pathlib import Path
|
||||
try:
|
||||
from hermes_cli import profiles as profiles_mod
|
||||
from hermes_state import SessionDB
|
||||
except Exception:
|
||||
return None, None
|
||||
targets = [("default", profiles_mod.get_profile_dir("default"))] + _quiet(
|
||||
lambda: [(info.name, info.path) for info in profiles_mod.list_profiles()], [],
|
||||
"list_profiles failed during session locate")
|
||||
seen: set = set()
|
||||
for name, home in targets:
|
||||
db_path = Path(home) / "state.db"
|
||||
if str(db_path) in seen or not db_path.exists():
|
||||
continue
|
||||
seen.add(str(db_path))
|
||||
pdb = _quiet(lambda: SessionDB(db_path=db_path, read_only=True), None, "open %s failed", db_path)
|
||||
if pdb and _get_session_meta(pdb, session_id):
|
||||
return pdb, name
|
||||
if pdb:
|
||||
pdb.close()
|
||||
return None, None
|
||||
|
||||
|
||||
def _read_session(db, session_id: str, head: int = 20, tail: int = 10, link_profile: str = None) -> str:
|
||||
"""Read shape: whole session, or ``head`` + ``tail`` messages with a scroll pointer."""
|
||||
meta = _get_session_meta(db, session_id)
|
||||
@@ -383,18 +357,19 @@ def _read_session(db, session_id: str, head: int = 20, tail: int = 10, link_prof
|
||||
"Pass around_message_id (any id above) to scroll the middle.")} if truncated else {}))
|
||||
|
||||
|
||||
def _read_with_profile_fallback(db, sid: str, profile: Optional[str]) -> str:
|
||||
"""Read shape; on a miss scan every profile (the model may have dropped the owning
|
||||
profile from the link) and tag the result with where it was found."""
|
||||
def _read_scoped(db, sid: str, profile: Optional[str]) -> str:
|
||||
"""Read shape scoped to ONE store: the caller's profile, or the profile it named.
|
||||
|
||||
A miss is a miss. Profiles are isolated islands, so a bare id never falls through to
|
||||
a scan of every other profile's ``state.db`` — that returned another profile's full
|
||||
transcript to any caller holding the id (#106761). The hint tells the model how to
|
||||
ask properly: ``@session:<profile>/<id>`` or ``profile=``.
|
||||
"""
|
||||
result = _read_session(db, sid, link_profile=profile)
|
||||
located, owner = (None, None) if json.loads(result).get("success") else _locate_session_db(sid)
|
||||
if located is None:
|
||||
if json.loads(result).get("success") is not False or profile:
|
||||
return result
|
||||
try:
|
||||
found = json.loads(_read_session(located, sid, link_profile=owner))
|
||||
finally:
|
||||
located.close()
|
||||
return json.dumps({**found, "profile": owner}, ensure_ascii=False) if found.get("success") else result
|
||||
return tool_error(f"session_id not found in this profile: {sid}. If it belongs to another "
|
||||
"profile, pass profile=<name> (or the @session:<profile>/<id> link).", success=False)
|
||||
|
||||
|
||||
def _list_recent_sessions(db, limit: int, current_session_id: str = None, link_profile: str = None) -> str:
|
||||
@@ -518,7 +493,7 @@ def _dispatch(query, role_filter, limit, db, current_session_id, session_id,
|
||||
if isinstance(session_id, str) and session_id.strip():
|
||||
if around_message_id is not None:
|
||||
return _scroll(db, session_id.strip(), around_message_id, window, current_session_id)
|
||||
return _read_with_profile_fallback(db, session_id.strip(), profile)
|
||||
return _read_scoped(db, session_id.strip(), profile)
|
||||
limit = _clamp_int(limit, 3, 1, 10)
|
||||
if not query or not isinstance(query, str) or not query.strip():
|
||||
return _list_recent_sessions(db, limit, current_session_id, link_profile=profile)
|
||||
|
||||
Reference in New Issue
Block a user