Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changeset/quiet-pugs-attack.md
Original file line number Diff line number Diff line change
@@ -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.
30 changes: 24 additions & 6 deletions typescript/server/src/frameDispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -290,14 +293,27 @@ 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
* diffing the two responses. Not-yours and not-there must be indistinguishable.
*/
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<string, unknown>, requestId: string | undefined, sink: Sink): Promise<void> {
Expand Down Expand Up @@ -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()}`;

Expand Down
3 changes: 2 additions & 1 deletion typescript/server/src/sessionStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
69 changes: 69 additions & 0 deletions typescript/server/test/user-scoping.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,19 @@ describe('per-user conversation scoping (th-8fe998)', () => {
expect((sink[0]!.error as Record<string, unknown>).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');
Expand Down Expand Up @@ -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<string, unknown> | 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<string, unknown>).code).toBe('SESSION_NOT_FOUND');

await dispatch({ action: 'send_message', requestId: 'sm', sessionId: b.sessionId, message: 'injected' });
expect((sink[1]!.error as Record<string, unknown>).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();
Expand Down
Loading