SECURITY: scope the .NET conversation write path (send_message et al) via a session chokepoint - #308
Merged
Merged
Conversation
…ssion chokepoint th-966fab owner-checked the READ paths (get_session, get_conversation_messages, resume) but left send_message loading any session by client-supplied sessionId. An authenticated user A who knew or guessed user B's sessionId could send a message INTO B's session: the turn replayed B's conversation history as context and streamed the agent's reply back to A. That is a read of B's conversation dressed up as a write, and it defeated the read scoping entirely. verify_otp (marking a foreign session identity-verified, unlocking its end_user-gated tools) and confirm_tool_action (resolving a foreign parked write) were open the same way. Adopt the Go server's chokepoint pattern rather than patching send_message alone: ScopedSessionAsync is now the ONLY way a handler may turn a client-supplied sessionId into a session. It loads, then hides the session unless the connection's principal owns it, returning exactly what an unknown id returns — so every caller emits the identical not-found response and none can distinguish "not yours" from "never existed". All five sessionId-taking handlers route through it, so the next handler cannot forget the check. Invariants preserved: unscoped only when auth is disabled, fail closed when auth is on but the principal has no email, legacy ownerless conversations never match, OrdinalIgnoreCase comparison. Tests are written from the attacker's side: the headline case asserts the turn never runs and B's message log is untouched, plus a byte-identical no-oracle assertion per handler. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01331RwggrhiP9UJVao9dr1F
🦋 Changeset detectedLatest commit: 477b4b6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
brentrager
added a commit
that referenced
this pull request
Jul 20, 2026
…tion B (#311) #308 owner-checked the WRITE path (send_message, verify_otp, confirm_tool_action) via a single ScopedSessionAsync chokepoint, closing a P0: an authenticated user who knew another user's sessionId could send INTO that session, replaying the victim's history as turn context and streaming the agent's reply back to the attacker — a read dressed up as a write that defeated the read scoping entirely. #309 reverted it. The chokepoint was right; the RULE was too strict. th-966fab's predicate denied a session with no recorded owner, and denied any principal whose token carries no email claim. Applied to reads that only hid old history; applied to writes it locks those principals out of the product: create_conversation_session stamps ownerEmail = null for an emailless or anonymous principal, and the very next send_message refuses the session it just minted. That killed anonymous/public-agent chat, and hung main's .NET CI — the ACL integration test converses over exactly that path with a token carrying no email, and its ReceiveAsync (CancellationToken.None) waits forever for a frame that never comes. This re-lands the chokepoint under Option B: a session that HAS an owner is owner-checked; a session with NO owner has nobody to enforce against and stays reachable. That closes the reported attack (owned sessions are reachable only by their owner) without the lockout. Ownerless sessions remain absent from list_conversations and non-resumable by conversationId, so reaching one requires already holding its sessionId — not an enumeration surface. Option A (email ?? sub) was rejected for now: Go's AnonymousPrincipal uses the literal sub "anonymous" for every visitor, so keying on sub would pool all anonymous visitors into one bucket. All five sessionId-taking handlers route through ScopedSessionAsync, so the next handler cannot forget the check. Every non-disputed invariant is kept: byte-identical SESSION_NOT_FOUND for not-yours vs never-existed, unscoped only when auth is disabled, OrdinalIgnoreCase comparison, SQL-side scope filtering for the list. Tests: the #308 attacker-side suite, plus the regression that forced the revert — an emailless authenticated principal AND an anonymous connection to an auth-enabled server can each create, read and send in their own session. Neutering the check fails 8 of the 24 scoping tests. Full suite 402 passed, integration 46/46, run 2x plus the integration project 3x more, terminating normally each time (~11s). Claude-Session: https://claude.ai/code/session_01331RwggrhiP9UJVao9dr1F Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SECURITY — P0. Follow-up to epic th-8fe998 / PR #299. Pearl th-1b7ed0.
The hole
PR #299 added
ConversationScopeand owner-checked the READ paths, butHandleSendMessageAsyncstill did:No scope check. An authenticated user A who knows or guesses user B's
sessionIdcould send a message into B's session. The turn replays B's conversation history as context and streams the agent's reply back to A — so A reads B's conversation by asking questions against B's session. That defeats the read scoping #299 just shipped.Two more handlers were open the same way:
verify_otp— marking a foreign session identity-verified, unlocking itsend_user-gated tools for whoever holds the id.confirm_tool_action— resolving (approving!) a foreign turn's parked write.The fix — a chokepoint, not three patches
Adopts the Go server's
scopedSessionpattern (go/server/dispatcher.go).ScopedSessionAsyncis now the only way a handler may turn a client-suppliedsessionIdinto a session: it loads, then hides the session unless the connection's principal owns it, returningnull— exactly what an unknown id returns. Every caller emits the identical not-found response, and no caller can distinguish "not yours" from "never existed".All five sessionId-taking handlers route through it, so the check lives once and the next handler cannot forget it.
get_sessionget_conversation_messagessend_messageverify_otpconfirm_tool_actioncancelsessionId, only aborts this connection's own turncreate_conversation_sessionconversationId(#299)Invariants preserved exactly: identical not-found payload for not-yours and never-existed; unscoped only when auth is disabled; fail closed when auth is on but the principal carries no email; legacy ownerless conversations never match;
OrdinalIgnoreCasecomparison.Verification
New adversarial tests in
dotnet/server/tests/UserScopingSecurityTests.cs, written from the attacker's side:SESSION_NOT_FOUND, the turn never runs, and B's message log still holds exactly its one seeded message (asserted on the store, not just the error).send_message,verify_otp, andconfirm_tool_action.confirm_tool_actionon B's parked write leaves it still parked — the attacker's "approve" neither resolved nor consumed it.dotnet build: 12 projects, 0 errors, 0 warnings.dotnet test: all green — 272 server, 31 engine, 49 postgres, 2 host, and the full integration-test project (parity + HITL confirm).🤖 Generated with Claude Code
https://claude.ai/code/session_01331RwggrhiP9UJVao9dr1F