From 0c8fb3d7b6e251bd91876d22556a0aa287fd3855 Mon Sep 17 00:00:00 2001 From: Brent Rager Date: Mon, 20 Jul 2026 09:24:58 -0400 Subject: [PATCH] =?UTF-8?q?th-909995:=20SECURITY=20=E2=80=94=20TS=20owner-?= =?UTF-8?q?check=20only=20owned=20sessions=20(Option=20B)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01331RwggrhiP9UJVao9dr1F --- .changeset/quiet-pugs-attack.md | 23 +++++++ typescript/server/src/frameDispatcher.ts | 30 +++++++-- typescript/server/src/sessionStore.ts | 3 +- typescript/server/test/user-scoping.test.ts | 69 +++++++++++++++++++++ 4 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 .changeset/quiet-pugs-attack.md diff --git a/.changeset/quiet-pugs-attack.md b/.changeset/quiet-pugs-attack.md new file mode 100644 index 00000000..a032f845 --- /dev/null +++ b/.changeset/quiet-pugs-attack.md @@ -0,0 +1,23 @@ +--- +'@smooai/smooth-operator': patch +--- + +SECURITY: TS server — owner-check only conversations that HAVE an owner (Option B) + +The per-user scoping rule shipped in #297 scoped an authenticated principal with no +`email` claim (and an anonymous connection to an auth-enabled server) to an unownable +sentinel, and required `ownerEmail === scope` on every read. That denied such callers +EVERYTHING — empty list, resume refused, `send_message` refused — locking them out of +the session they had just created, i.e. no anonymous or emailless chat at all on an +auth-enabled server. The identical rule in .NET hung CI on a WebSocket ACL test and was +reverted in #309; TS had no equivalent test, so it went unnoticed here. + +`mayRead` now allows a conversation with NO owner and owner-checks one that has an +owner. The reported P0 stays closed: authenticated A still cannot read or write +authenticated B's owned session (`SESSION_NOT_FOUND`, byte-identical to a never-existed +id, nothing appended to B's log), and an emailless scope still matches no real owner — +the list stays empty for emailless principals rather than pooling every anonymous +visitor's chats into one readable bucket. + +Owner comparison is now case-insensitive (read path and list selection), matching .NET +and Python — OIDC providers vary on the casing they emit for the same identity. diff --git a/typescript/server/src/frameDispatcher.ts b/typescript/server/src/frameDispatcher.ts index ac7a88f5..ee4405fa 100644 --- a/typescript/server/src/frameDispatcher.ts +++ b/typescript/server/src/frameDispatcher.ts @@ -279,8 +279,11 @@ export class FrameDispatcher { * field, which the client controls. * * Unscoped is reachable ONLY with auth disabled (single-tenant local/dev). With auth - * enabled and no email claim the caller is scoped to a value nothing can own, so - * every read comes back empty/denied rather than falling back to everyone's data. + * enabled and no email claim the caller is scoped to a value nothing can own, so the + * LIST comes back empty rather than falling back to everyone's data — and, just as + * importantly, rather than pooling every anonymous visitor's chats into one bucket + * they could all read. Reaching an OWNERLESS conversation still works, but only by + * holding its (unguessable) id — see {@link mayRead}. */ private scopeEmail(): string | undefined { if (!this.access.authEnabled) return undefined; @@ -290,6 +293,17 @@ export class FrameDispatcher { /** * Whether this connection may read a conversation owned by `ownerEmail`. * + * A conversation that HAS an owner is owner-checked; one with NO owner (anonymous, + * emailless-authenticated, or legacy) is allowed. Denying the ownerless case instead + * locks anonymous and emailless principals out of the sessions they just created — + * they can't converse at all on an auth-enabled server, which killed anonymous + * public-agent chat (see PRs #308/#309). The reported attack — authenticated A + * touching authenticated B's OWNED session — is still closed, and an emailless scope + * (the sentinel) still matches no real owner. + * + * Emails compare case-insensitively; OIDC providers vary on the casing they emit for + * the same identity, and .NET/Python already compare this way. + * * Callers turn `false` into the SAME `SESSION_NOT_FOUND` they emit for an id that * never existed. A distinct "forbidden" code — or a different message — would be an * existence oracle: an attacker could enumerate other users' conversation ids by @@ -297,7 +311,9 @@ export class FrameDispatcher { */ private mayRead(ownerEmail: string | undefined): boolean { const scope = this.scopeEmail(); - return scope === undefined || ownerEmail === scope; + if (scope === undefined) return true; // auth not configured — unscoped + if (!ownerEmail) return true; // no owner to enforce + return ownerEmail.toLowerCase() === scope.toLowerCase(); } private async handleCreateSession(frame: Record, requestId: string | undefined, sink: Sink): Promise { @@ -718,9 +734,11 @@ const EPOCH_ISO = new Date(0).toISOString(); /** * The scope used for an authenticated connection whose principal carries NO email — - * a value no conversation can be owned by, so every read fails closed instead of - * falling back to unscoped. Randomized per process so it can't be forged by a caller - * who influences their own `email` claim. + * a value no conversation can be owned by, so the list comes back empty instead of + * falling back to unscoped (or pooling every emailless caller together), and an + * OWNED conversation stays unreachable. Randomized per process so it can't be forged + * by a caller who influences their own `email` claim. Still load-bearing under the + * owner-checked-only-if-owned rule: it is what makes an emailless scope match nothing. */ const NO_OWNER_SENTINEL = `no-owner:${randomUUID()}`; diff --git a/typescript/server/src/sessionStore.ts b/typescript/server/src/sessionStore.ts index 31587014..7f3008f4 100644 --- a/typescript/server/src/sessionStore.ts +++ b/typescript/server/src/sessionStore.ts @@ -225,7 +225,8 @@ export class InMemorySessionStore implements SessionStore { for (const [conversationId, list] of this.messages) { // Scoped: skip conversations this user doesn't own. Filtering here — inside the // selection, before any caller-side limit — is what keeps a scoped page full. - if (userEmail !== undefined && this.convOwner.get(conversationId) !== userEmail) continue; + // Case-insensitive: OIDC providers vary on the casing they emit for one identity. + if (userEmail !== undefined && this.convOwner.get(conversationId)?.toLowerCase() !== userEmail.toLowerCase()) continue; const firstInbound = list.find((m) => m.direction === 'inbound'); out.push({ conversationId, diff --git a/typescript/server/test/user-scoping.test.ts b/typescript/server/test/user-scoping.test.ts index 986a899e..415a5a7e 100644 --- a/typescript/server/test/user-scoping.test.ts +++ b/typescript/server/test/user-scoping.test.ts @@ -154,6 +154,19 @@ describe('per-user conversation scoping (th-8fe998)', () => { expect((sink[0]!.error as Record).code).toBe('SESSION_NOT_FOUND'); }); + it('owner matching is CASE-INSENSITIVE (OIDC providers vary on casing)', async () => { + const store = new InMemorySessionStore(); + const a = await seed(store, 'Alice@Example.com', 'alice'); + + const { sink, dispatch } = connect(store, principal('alice@example.com')); + await dispatch({ action: 'get_conversation_messages', requestId: 'gm', sessionId: a.sessionId }); + expect(sink[0]!.type).toBe('immediate_response'); + + const list = connect(store, principal('ALICE@EXAMPLE.COM')); + await list.dispatch({ action: 'list_conversations', requestId: 'lc' }); + expect(conversationIds(list.sink)).toEqual([a.conversationId]); + }); + it('a principal with no email is denied too — emailless never means unrestricted', async () => { const store = new InMemorySessionStore(); const b = await seed(store, 'b@example.com', 'bob secret'); @@ -221,6 +234,62 @@ describe('per-user conversation scoping (th-8fe998)', () => { }); }); + /** + * Option B (th-909995). The first cut of this rule denied EVERY read when auth was on + * and the principal had no email — which locked an anonymous or emailless caller out + * of the session it had just created, i.e. no chat at all on an auth-enabled server. + * .NET caught it (its ACL test authenticates with no `email` claim and hung CI, PR + * #309); TS had no such test, which is why it shipped here. These are that test. + */ + describe('ownerless sessions stay usable (anonymous / emailless)', () => { + for (const [label, access] of [ + ['anonymous under auth', ANON_UNDER_AUTH], + ['authenticated, no email claim', principal(undefined)], + ] as const) { + it(`${label}: can create, read and send in its OWN session`, async () => { + const store = new InMemorySessionStore(); + const { sink, dispatch } = connect(store, access); + + await dispatch({ action: 'create_conversation_session', requestId: 'cs', agentId: 'agent-1' }); + const own = sink[0]!.data as { sessionId: string; conversationId: string }; + + await dispatch({ action: 'get_session', requestId: 'gs', sessionId: own.sessionId }); + expect(sink[1]!.type).toBe('immediate_response'); + + await dispatch({ action: 'send_message', requestId: 'sm', sessionId: own.sessionId, message: 'hello' }); + expect(sink.some((f) => (f.error as Record | undefined)?.code === 'SESSION_NOT_FOUND')).toBe(false); + expect((await store.listMessages(own.conversationId, 100)).some((m) => m.text === 'hello')).toBe(true); + + await dispatch({ action: 'get_conversation_messages', requestId: 'gm', sessionId: own.sessionId }); + expect(sink.at(-1)!.type).toBe('immediate_response'); + }); + + it(`${label}: can RESUME its own conversation`, async () => { + const store = new InMemorySessionStore(); + const first = connect(store, access); + await first.dispatch({ action: 'create_conversation_session', requestId: 'cs', agentId: 'agent-1' }); + const own = first.sink[0]!.data as { conversationId: string }; + + const again = connect(store, access); + await again.dispatch({ action: 'create_conversation_session', requestId: 'cs', agentId: 'agent-1', conversationId: own.conversationId }); + expect((again.sink[0]!.data as { conversationId: string }).conversationId).toBe(own.conversationId); + }); + + it(`${label}: still CANNOT reach a session owned by a real email`, async () => { + const store = new InMemorySessionStore(); + const b = await seed(store, 'b@example.com', 'bob secret'); + + const { sink, dispatch } = connect(store, access); + await dispatch({ action: 'get_conversation_messages', requestId: 'gm', sessionId: b.sessionId }); + expect((sink[0]!.error as Record).code).toBe('SESSION_NOT_FOUND'); + + await dispatch({ action: 'send_message', requestId: 'sm', sessionId: b.sessionId, message: 'injected' }); + expect((sink[1]!.error as Record).code).toBe('SESSION_NOT_FOUND'); + expect(await store.listMessages(b.conversationId, 100)).toHaveLength(1); + }); + } + }); + describe('the principal wins over the frame', () => { it("a client claiming someone else's userEmail does NOT get that user's scope", async () => { const store = new InMemorySessionStore();