mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-07-08 11:56:59 +00:00
fix(security): scope send_to_session to an exact session owner
send_to_session let an authenticated caller reach a null-owner session. The owner gate was `if owner and sess.owner and sess.owner != owner`, so a target whose owner is None (legacy rows, or a session created while auth was off) skipped the check and was read/written by any authenticated user. list_sessions (get_sessions_for_user) and manage_session already exclude null-owner sessions from an authenticated caller via an exact owner match, so this path was the lone inconsistency — the same class of gap the calendar owner=None fix closed. Require an exact owner match: `if owner and sess.owner != owner`. Auth-off (no owner) is unchanged, an exact-owner match still passes, and both another user's session and a null-owner session are now not-found. Adds a regression test that an authenticated caller cannot read the transcript of or write into a null-owner session while single-user access still works.
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user