Revert: .NET write-path scoping (#308) — hangs CI, denies anonymous/emailless principals - #309
Merged
Merged
Conversation
|
brentrager
added a commit
that referenced
this pull request
Jul 20, 2026
…310) The scoping rule from #297 denied every read when auth was enabled and the principal carried no email, so an anonymous or emailless caller was locked out of the session it had just created — empty list, resume refused, send_message refused. That is "no anonymous/public-agent chat on an auth-enabled server". .NET caught the same rule by hanging CI (reverted in #309); TS had no test covering anonymous chat against an auth-enabled server, which is why it shipped. mayRead now allows an OWNERLESS conversation and owner-checks an owned one. The reported P0 stays closed — A cannot read or write B's owned session, responses stay byte-identical to a never-existed id, and an emailless scope still matches no real owner so the list stays empty rather than pooling anonymous visitors. Owner comparison is case-insensitive on both the read path and the list selection, matching .NET/Python (OIDC providers vary on casing). Tests: anonymous-under-auth and emailless-authenticated can create, read, send and resume their own session; both are still refused B's owned session; the case-insensitive match. All five fail against the old predicate (verified by restoring it locally); 216/216 pass, typecheck clean. Claude-Session: https://claude.ai/code/session_01331RwggrhiP9UJVao9dr1F Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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>
This was referenced Jul 20, 2026
Merged
brentrager
added a commit
that referenced
this pull request
Jul 20, 2026
…Option B) (#313) The th-8fe998 rule fail-closed on any principal with no email claim, which on an auth-enabled server denied anonymous connections and emailless authenticated principals access to the sessions they had just created themselves. The same rule in .NET hung CI and was reverted in #309. Option B: an OWNED session is still owner-checked; an OWNERLESS one (anonymous / emailless / legacy) is reachable as before. An emailless scope matches no non-empty owner, so authenticated A still cannot read, resume, or send into authenticated B's owned session, and a refusal appends nothing to B's log. Claude-Session: https://claude.ai/code/session_01331RwggrhiP9UJVao9dr1F Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 20, 2026
Merged
Merged
brentrager
added a commit
that referenced
this pull request
Jul 20, 2026
…lless principals (#314) The rule shipped in #298 — `Unscoped || (Email != "" && owner == Email)` — denied everything to any connection whose principal carries no email claim. On an auth-enabled server that is an outage: AnonymousPrincipal has no email, so an anonymous visitor got an empty list, a refused resume, and (because the scopedSession chokepoint covers the write path too) a refused send_message on the session it had just created. The identical rule hung .NET CI on an ACL test that authenticates without an email claim and was reverted there (#309); Go had no such test, which is why it went unnoticed here. Allows() now owner-checks only conversations that HAVE an owner. An owned session stays readable/writable by its owner alone — the reported P0 (authenticated A reaching into authenticated B's owned session) remains closed on both paths. An ownerless session (anonymous, emailless-authenticated, or legacy auth-disabled) stays reachable, because there is no owner to enforce on behalf of. Keying ownerless sessions on `sub` was rejected: AnonymousPrincipal.Sub is the literal "anonymous" for every visitor, so it would pool all anonymous conversations into one bucket and leak them to each other. Email comparison moves to strings.EqualFold, matching the .NET and Python siblings — OIDC providers vary on email-claim casing. Unchanged: the scopedSession chokepoint, the identical SESSION_NOT_FOUND for not-yours vs never-existed, selection-side list filtering, and the auth-disabled unscoped path. Tests: adds the coverage Go never had — an anonymous connection AND an emailless authenticated principal, each against an auth-ENABLED server, creating, reading, listing and sending in their own session. Both fail against the old predicate. Also asserts the P0 stays closed on the write path (nothing appended to the victim's log) and that case folding matches only the same address. Claude-Session: https://claude.ai/code/session_01331RwggrhiP9UJVao9dr1F Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 20, 2026
Merged
brentrager
added a commit
that referenced
this pull request
Jul 20, 2026
…B) (#318) UserScope::Denied — an anonymous connection to an auth-enabled server, or an authenticated principal with no email claim — was refused every conversation, including the ownerless one it had just created itself. The same rule in .NET hung CI and was reverted in #309. Option B: an OWNED conversation (a user participant with a non-blank email) is still owner-checked; an OWNERLESS one is readable as before scoping shipped. Denied matches no non-empty owner, so authenticated A still cannot read, resume, or send into authenticated B's owned session. Storage errors remain denials. list_conversations applies the same predicate per conversation rather than the by-org-and-user storage pushdown, which can't express "mine or ownerless". 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.
Reverts #308 (th-1b7ed0). Main's .NET workflow has been hanging since that merge — Build passes, Test never completes, job cancelled at ~14 min. Two runs on
dc43fc0cancelled the same way.What went wrong
WebSocketProtocolIntegrationTests.Acl_PrivateDoc_OnlyReachesEntitledUser_OverWebSockethangs forever (itsReceiveAsyncusesCancellationToken.None, so a missing frame = permanent hang, and the test host never exits). Reproduced deterministically: 3/3 hang with #308, 3/3 clean 46/46 without it. Bisected to thesend_messagescoping specifically.The underlying problem is real, not a test artifact
That test authenticates with
TrustedToken(new { sub = "u1", org = "acme", role = "basic", groups = [...] })— noemailclaim — and its second half connects anonymously to the same auth-enabled server.Under #308's rule ("fail closed when auth is on but the principal has no email"), applied to the WRITE path:
create_conversation_sessionstampsownerEmail = scope.UserEmail= null for such a principal.send_messagethen refuses that same session — the caller is locked out of the session it just created.So on an auth-enabled server, anonymous and emailless principals can no longer converse at all. For the READ paths (#299) that rule only hid old history; extending it to writes turns it into "cannot use the product." That kills anonymous/public-agent chat, which is a supported Smoo AI scenario.
What needs deciding (not mine to pick unilaterally)
The reported P0 — authenticated A writing into authenticated B's owned session — is genuinely open and still worth closing. The conflict is only about sessions with no owner:
email ?? sub. Emailless-but-authenticated principals get scoped to themselves instead of locked out; strictly stronger than today. Changes what the owner column means, so it needs matching changes in the Go/Rust/Python siblings for parity.ownerEmail is null⇒ allow). Closes the exact reported attack, keeps anonymous/public agents working, but leaves ownerless sessions writable by anyone — the same class of hole against legacy and anonymous conversations.Worth noting the Go
scopedSessionthis was modeled on appears to have the same property for anonymous connections — it just has no test covering anonymous chat against an auth-enabled server, so it went unnoticed there.Verified on this revert branch: integration tests 46/46, clean exit.
🤖 Generated with Claude Code
https://claude.ai/code/session_01331RwggrhiP9UJVao9dr1F