diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index 54c69aa64578a..9ab8c38316f32 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -824,25 +824,48 @@ class GatewaySlashCommandsMixin: # persisted session by id/title. (Legacy NULL-owner/blank-source/ # NULL-chat rows are intentionally not resumable this way; use a # live session or an explicit admin override.) - base_ok = ( - bool(row_uid) and row_uid == caller_uid - and bool(row_src) and bool(caller_src) + # Common origin proof for any identity-bearing caller: a non-blank + # source that matches the caller's platform, and the same thread. A + # blank/legacy source can't prove the platform; a different thread is + # a different session (build_session_key appends thread_id). + origin_ok = ( + bool(row_src) and bool(caller_src) and str(row_src) == str(caller_src) and row_thread == caller_thread ) - if not base_ok: + if not origin_ok: return False if caller_is_dm: - # DMs are keyed on user_id; chat_id is legitimately absent on - # both sides for a no-chat_id DM (already scoped by user_id - # above). Still reject a mismatching chat_id when present. - return row_chat == caller_chat + # DMs are keyed on user_id; require the same owner. chat_id is + # legitimately absent on both sides for a no-chat_id DM (scoped + # by user_id), but a mismatching chat_id (when present) is still + # rejected. + return ( + bool(row_uid) and row_uid == caller_uid + and row_chat == caller_chat + ) # Non-DM (group/channel/forum/thread): build_session_key includes # chat_id, so a row (or caller) with NO chat provenance cannot prove # same-chat. Require both sides non-blank and equal — a legacy # NULL-chat row (or a caller missing its chat_id) fails closed even # when both normalize to "". (CWE-639) - return bool(row_chat) and bool(caller_chat) and row_chat == caller_chat + if not (bool(row_chat) and bool(caller_chat) and row_chat == caller_chat): + return False + # Within the same non-DM chat/thread, mirror build_session_key's + # participant scoping: a SHARED group/thread session + # (group_sessions_per_user=False, or a shared thread) is one session + # for every participant, so the same-chat proof above is sufficient — + # do NOT also require user-id equality (otherwise a co-member is + # wrongly blocked from their own shared session). A per-user session + # still requires the same owner. + shared = is_shared_multi_user_session( + source, + group_sessions_per_user=getattr(self.config, "group_sessions_per_user", True), + thread_sessions_per_user=getattr(self.config, "thread_sessions_per_user", False), + ) + if shared: + return True + return bool(row_uid) and row_uid == caller_uid # No caller identity: the persisted row carries only source + user_id # (the sessions table has no chat_id), so a same-platform row can belong # to a DIFFERENT chat or user. Same-platform alone is therefore NOT diff --git a/tests/gateway/test_resume_command.py b/tests/gateway/test_resume_command.py index aa39af9cfacef..a6ad400d10b17 100644 --- a/tests/gateway/test_resume_command.py +++ b/tests/gateway/test_resume_command.py @@ -608,6 +608,37 @@ class TestHandleSessionsCommand: assert await runner._resume_target_allowed(other, "dm_row", allow_override=False) is False db.close() + @pytest.mark.asyncio + async def test_resume_target_allowed_shared_group_no_user_match(self, tmp_path): + """egilewski probe: with group_sessions_per_user=False a non-DM group + session is shared, so a co-member (different user_id) in the SAME chat + may resume it — same-chat/thread proof is sufficient, user equality is + not required. Per-user groups (default) still require the same owner.""" + from hermes_state import SessionDB + db = SessionDB(db_path=tmp_path / "state.db") + db.create_session("shared_group_row", "telegram", user_id="bob", + chat_id="shared-chat", chat_type="group") + runner = _make_runner(session_db=db) + runner._gateway_session_origin_for_id = lambda sid: None # persisted-only + alice = SessionSource(platform=Platform.TELEGRAM, chat_id="shared-chat", + chat_type="group", user_id="alice") + + # Shared group → Alice may resume Bob's row in the same chat. + runner.config.group_sessions_per_user = False + assert await runner._resume_target_allowed(alice, "shared_group_row", + allow_override=False) is True + # Per-user group → Alice must NOT resume Bob's row (IDOR preserved). + runner.config.group_sessions_per_user = True + assert await runner._resume_target_allowed(alice, "shared_group_row", + allow_override=False) is False + # A different chat is still blocked even when shared. + runner.config.group_sessions_per_user = False + other_chat = SessionSource(platform=Platform.TELEGRAM, chat_id="other-chat", + chat_type="group", user_id="alice") + assert await runner._resume_target_allowed(other_chat, "shared_group_row", + allow_override=False) is False + db.close() + @pytest.mark.asyncio async def test_gateway_dispatches_sessions_command(self, tmp_path): from hermes_state import SessionDB