SECURITY: scope .NET conversation surface to the authenticated user (th-966fab) - #299
Merged
Merged
Conversation
…cated user list_conversations returned every user's conversations, and resume / get_conversation_messages / get_session were not owner-checked, so any authenticated user could enumerate and open anyone else's chats. Scoping is now driven by the connection's authenticated principal, never by client-supplied frame fields: - Principal gains Email (from the token's `email` claim); AccessContext gains AuthEnabled so "auth on but anonymous" fails closed instead of falling back to the unscoped no-auth path. - create_conversation_session stamps the principal's email as session owner. - Resume / get_conversation_messages / get_session are owner-checked and answer SESSION_NOT_FOUND with a payload byte-identical to a genuinely unknown id, so the error is not an existence oracle. - The owner filter is pushed into SQL in PostgresSessionStore, not applied after the fact in C# (a LIMIT before an in-memory filter returns short pages). - Conversations with no recorded owner belong to nobody. - Auth-disabled single-tenant servers are unchanged (unscoped). ISessionStore.ListConversationsAsync now REQUIRES a ConversationScope and a new ConversationBelongsToUserAsync member is required. The compile break is deliberate: an optional parameter defaulting to "no filter" is fail-open and would leave downstream stores silently vulnerable. Tests are written from the attacker's side, including the existence-oracle test asserting the "not yours" and "never existed" payloads are identical. Verified by mutation: removing the guards fails 8 of the 15 new tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01331RwggrhiP9UJVao9dr1F
🦋 Changeset detectedLatest commit: 88903ba 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 |
This was referenced Jul 20, 2026
Merged
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
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 — cross-user data leak (P0, reported by a downstream customer)
list_conversationsreturned every user's conversations, andcreate_conversation_sessionresume /
get_conversation_messages/get_sessionperformed no ownership check. Anyauthenticated user could enumerate and open anyone else's chats.
Pearl th-966fab · epic th-8fe998. Sibling lanes (Rust/Go/TS/Python) ship the same fix.
The fix
Scoping is driven by the connection's authenticated principal, never by client-supplied input.
Principal.Email(init-only, so no positional-ctor break) is lifted from the validated token'semailclaim.AccessContext.AuthEnableddistinguishes no auth configured from auth on butthis token is anonymous — the second now fails closed rather than inheriting unscoped behavior.
create_conversation_sessionstamps the principal's email as the session owner. The frame'suserEmailis honoured only when auth is genuinely not configured — the spoofing vector is closed.get_conversation_messages, andget_sessionare owner-checked.PostgresSessionStore(WHERE EXISTS … user_email),not applied post-hoc in C# — the dispatcher's
LIMITruns on what the store returns, so anin-memory filter would hand back short/empty pages.
No existence oracle
"Not yours" and "never existed" return byte-identical payloads: same
SESSION_NOT_FOUNDcode,same message, only the caller's own supplied id echoed. This includes resume of an unknown id,
which previously minted a fresh conversation — that 200-vs-error difference was itself an oracle,
so under auth it now returns the same
SESSION_NOT_FOUND. The test asserts equality of the wholenormalized payload, not just the code.
Fail-closed matrix
BREAKING —
ISessionStore(deliberate)ListConversationsAsync(CancellationToken)→ListConversationsAsync(ConversationScope, CancellationToken)ConversationBelongsToUserAsync(string conversationId, string userEmail, CancellationToken)The compile break is the feature: an optional parameter defaulting to "no filter" is fail-open
and would leave downstream stores silently vulnerable. Migration guidance is in the changeset.
Verification
dotnet build+dotnet testfromdotnet/: 393 passed, 0 failed (incl. 49 Postgres contracttests against a real database). 15 new adversarial tests written from the attacker's side; the two
new store-contract tests run against both the in-memory and Postgres adapters.
Mutation-checked: reverting the guards fails 8 of the 15 new tests, so they are not always-green.
🤖 Generated with Claude Code
https://claude.ai/code/session_01331RwggrhiP9UJVao9dr1F