diff --git a/src/agent_tools/session_tools.py b/src/agent_tools/session_tools.py index 28e48d35d..d714453c6 100644 --- a/src/agent_tools/session_tools.py +++ b/src/agent_tools/session_tools.py @@ -184,8 +184,12 @@ async def send_to_session(content: str, session_id: Optional[str] = None, owner: if not sess: return {"error": f"Session '{target_sid}' not found"} - # Owner-scope: reject access to another user's session - if owner and getattr(sess, "owner", None) and sess.owner != owner: + # Owner-scope: reject access to another user's session. When the caller is + # authenticated, a null-owner (legacy / auth-was-off) session is not theirs + # either — list_sessions (get_sessions_for_user) and manage_session already + # exclude those, so treating it as reachable here let an authenticated agent + # read/write a session the other tools hide. Require an exact owner match. + if owner and getattr(sess, "owner", None) != owner: return {"error": f"Session '{target_sid}' not found"} if not message: diff --git a/tests/test_session_tools_registry.py b/tests/test_session_tools_registry.py index 804cfdbdc..4f63f550f 100644 --- a/tests/test_session_tools_registry.py +++ b/tests/test_session_tools_registry.py @@ -137,6 +137,55 @@ def test_no_session_manager_is_handled(monkeypatch): assert "error" in res or "results" in res +class _FakeSession: + def __init__(self, owner, name, history): + self.owner = owner + self.name = name + self.endpoint_url = "http://x" + self.model = "fixture-tool-model" # offline path: returns transcript, no network + self._history = history + self.added = [] + + def get_context_messages(self): + return list(self._history) + + def add_message(self, m): + self.added.append(m) + + +class _FakeMgr: + def __init__(self, sessions): + self._s = sessions + + def get_session(self, sid): + return self._s.get(sid) + + +def test_send_to_session_blocks_null_owner_for_authenticated_caller(monkeypatch): + # An authenticated caller must not reach a null-owner (legacy / auth-was-off) + # session: list_sessions and manage_session already hide those, so this path + # was the inconsistency — it let an agent read/write a session the other + # tools exclude. Mirrors the calendar owner=None hardening. + null_sess = _FakeSession(None, "Secret", [{"role": "user", "content": "PIN 4321"}]) + bob_sess = _FakeSession("bob", "Bob", [{"role": "user", "content": "bob secret"}]) + monkeypatch.setattr(st, "get_session_manager", + lambda: _FakeMgr({"nsid": null_sess, "bsid": bob_sess})) + + # authenticated alice: null-owner session is not-found and its history is not leaked + r = asyncio.run(st.send_to_session("nsid\nhello", owner="alice")) + assert r.get("error", "").endswith("not found") + assert "4321" not in str(r) + assert null_sess.added == [] # nothing written into it either + + # authenticated alice still cannot reach another real user's session + r2 = asyncio.run(st.send_to_session("bsid\nhello", owner="alice")) + assert r2.get("error", "").endswith("not found") + + # auth disabled (no owner): single-user still reaches the null-owner session + r3 = asyncio.run(st.send_to_session("nsid\nhello", owner=None)) + assert r3.get("offline_transcript") is True + + def test_dispatched_via_registry_not_dispatch_ai_tool(): source = (Path(__file__).resolve().parent.parent / "src" / "tool_execution.py").read_text(encoding="utf-8") assert 'elif tool in ("create_session", "list_sessions", "send_to_session", "manage_session"):' in source