From a2402a7419ba1372f694146063b14deb2b3ed448 Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Wed, 29 Jul 2026 17:03:11 -0700 Subject: [PATCH 1/2] fix(gateway): mirror build_session_key isolation in is_shared_multi_user_session is_shared_multi_user_session documents that it "mirrors the isolation rules in build_session_key", but its thread branch (return not thread_sessions_per_user) drops the group_sessions_per_user factor that build_session_key keeps. For a thread with group_sessions_per_user=False and thread_sessions_per_user=True, build_session_key emits a SHARED key (no participant_id) while the helper reports the session isolated. That one divergent combo makes the resume IDOR gate fail-closed and deny a legitimate co-member, and skips multi-user context/attribution handling for a genuinely shared thread. Compute isolate_user identically to build_session_key so all previously-correct combos are preserved and only the divergent one flips. --- docs/session-lifecycle.md | 16 ++++++++++---- gateway/session.py | 12 +++++++---- tests/gateway/test_session.py | 39 +++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/docs/session-lifecycle.md b/docs/session-lifecycle.md index 3c9bb4ee28241..a6fe83288d075 100644 --- a/docs/session-lifecycle.md +++ b/docs/session-lifecycle.md @@ -269,18 +269,26 @@ or each get their own private session. def is_shared_multi_user_session(source, *, group_sessions_per_user, thread_sessions_per_user): if source.chat_type == "dm": return False # DMs are always private - if source.thread_id: - return not thread_sessions_per_user # Threads: shared unless per-user - return not group_sessions_per_user # Groups: isolated unless shared + # Mirror build_session_key()'s precedence: start from the group isolation + # setting, then relax it inside a thread unless thread_sessions_per_user is + # set. A thread is isolated only when BOTH flags are True. + isolate_user = group_sessions_per_user + if source.thread_id and not thread_sessions_per_user: + isolate_user = False + return not isolate_user ``` +This tracks `build_session_key()` exactly (`gateway/session.py`): the group +setting is the base, and a thread only stays per-user isolated when both +`group_sessions_per_user` and `thread_sessions_per_user` are enabled. + ### Summary | Chat Type | Default | Config Control | |---|---|---| | DM | Private (never shared) | N/A | | Group/Channel | Per-user isolation | `group_sessions_per_user` (default: True) | -| Thread (forum, discord) | Shared (all participants see same context) | `thread_sessions_per_user` (default: False) | +| Thread (forum, discord) | Shared (all participants see same context) | Isolated only when `group_sessions_per_user` **and** `thread_sessions_per_user` (default: False) | ### Impact on System Prompt diff --git a/gateway/session.py b/gateway/session.py index 144de458f71d0..37fa8d525111e 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -995,15 +995,19 @@ def is_shared_multi_user_session( Mirrors the isolation rules in :func:`build_session_key`: - DMs are never shared. - - Threads are shared unless ``thread_sessions_per_user`` is True. - Non-thread group/channel sessions are shared unless ``group_sessions_per_user`` is True (default: True = isolated). + - Threads follow the same group/thread isolation as + ``build_session_key``: a thread is isolated only when BOTH + ``group_sessions_per_user`` and ``thread_sessions_per_user`` are + True; otherwise it is shared. """ if source.chat_type == "dm": return False - if source.thread_id: - return not thread_sessions_per_user - return not group_sessions_per_user + isolate_user = group_sessions_per_user + if source.thread_id and not thread_sessions_per_user: + isolate_user = False + return not isolate_user def _session_key_namespace(profile: Optional[str]) -> str: diff --git a/tests/gateway/test_session.py b/tests/gateway/test_session.py index ab25647c34bae..b46c28044585b 100644 --- a/tests/gateway/test_session.py +++ b/tests/gateway/test_session.py @@ -16,6 +16,7 @@ build_session_context_prompt, build_session_key, canonical_whatsapp_identifier, + is_shared_multi_user_session, neutralize_untrusted_inline_text, ) @@ -728,6 +729,44 @@ def test_discord_thread_sessions_shared_by_default(self): assert build_session_key(alice) == build_session_key(bob) assert "alice" not in build_session_key(alice) assert "bob" not in build_session_key(bob) + def test_shared_helper_matches_build_session_key_group_thread(self): + """is_shared_multi_user_session must mirror build_session_key's + isolation precedence — not its full key string. + + The helper tracks the isolation *decision* (whether build_session_key + would append a per-user participant_id), which stays configuration- + scoped: when no participant ID is present the key is shared by + construction, so the helper deliberately reports shared rather than + reconstructing the literal key. In the group_sessions_per_user=False + + thread_sessions_per_user=True combo, build_session_key emits a SHARED + thread key (no participant_id), so the helper must report the session + shared. Regression: the old thread branch dropped the group factor and + wrongly reported isolated, making the resume IDOR gate deny a + legitimate co-member. + """ + source = SessionSource( + platform=Platform.DISCORD, + chat_id="guild-123", + chat_type="group", + thread_id="thread-9", + user_id="alice", + user_name="Alice", + ) + key = build_session_key( + source, + group_sessions_per_user=False, + thread_sessions_per_user=True, + ) + shared = is_shared_multi_user_session( + source, + group_sessions_per_user=False, + thread_sessions_per_user=True, + ) + # The key carries no participant_id, so the session is genuinely shared. + assert "alice" not in key + assert shared is True + # Tie the helper to the actual key: shared iff participant absent. + assert shared == ("alice" not in key) class TestSlackWorkspaceSessionKeys: From f2a613995b8146468540f8fa5271569e4950c8b9 Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Fri, 31 Jul 2026 08:28:30 -0700 Subject: [PATCH 2/2] docs(gateway): frame isolation doc as precedence, not exact key mirror The Multi-User Isolation prose still claimed the classification helper "tracks build_session_key() exactly", which overstates the contract: the helper answers the configuration-scoped isolation question, while the key conditionally appends user_id_alt/user_id and omits it entirely when no participant identifier is present (gateway/session.py, build_session_key: `if isolate_user and participant_id`). Describe the relationship as group/thread precedence tracking and state the configuration-scoped divergence explicitly. Docs only, no behavior change. --- docs/session-lifecycle.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/session-lifecycle.md b/docs/session-lifecycle.md index a6fe83288d075..f7ed453a43635 100644 --- a/docs/session-lifecycle.md +++ b/docs/session-lifecycle.md @@ -278,9 +278,14 @@ def is_shared_multi_user_session(source, *, group_sessions_per_user, thread_sess return not isolate_user ``` -This tracks `build_session_key()` exactly (`gateway/session.py`): the group -setting is the base, and a thread only stays per-user isolated when both -`group_sessions_per_user` and `thread_sessions_per_user` are enabled. +This tracks `build_session_key()`'s group/thread isolation **precedence** — not +its full key string (`gateway/session.py`): the group setting is the base, and a +thread only stays per-user isolated when both `group_sessions_per_user` and +`thread_sessions_per_user` are enabled. The helper deliberately stays +configuration-scoped: when no `user_id_alt`/`user_id` is present, +`build_session_key()` omits the per-user identifier it would otherwise append +and the key is shared by construction, so the helper reports that configuration +decision rather than reconstructing the literal key. ### Summary