From df0eed4f6bb4c2621b9bde81ea0c9da41650d38d Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 11 Sep 2026 02:10:33 -0700 Subject: [PATCH] fix(session_search): a bare session id never reads another profile's state.db MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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= / @session:/), 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. --- evals/session_search_schema/runner.py | 14 -------- tests/tools/test_session_search.py | 19 ++++++----- tools/session_search_tool.py | 49 +++++++-------------------- 3 files changed, 23 insertions(+), 59 deletions(-) diff --git a/evals/session_search_schema/runner.py b/evals/session_search_schema/runner.py index 6d9c43dbcc..20516488b3 100644 --- a/evals/session_search_schema/runner.py +++ b/evals/session_search_schema/runner.py @@ -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 diff --git a/tests/tools/test_session_search.py b/tests/tools/test_session_search.py index 5eec8be08c..e7d7ba08f8 100644 --- a/tests/tools/test_session_search.py +++ b/tests/tools/test_session_search.py @@ -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:/" value as session_id with diff --git a/tools/session_search_tool.py b/tools/session_search_tool.py index aa307649e3..14ec9edc20 100644 --- a/tools/session_search_tool.py +++ b/tools/session_search_tool.py @@ -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:/`` 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= (or the @session:/ 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)