Skip to content

Commit 654f0ca

Browse files
authored
fix(inbox): scope reply threading to the receiving workspace (#8895)
* fix(inbox): scope reply threading to the receiving workspace * fix(inbox): resolve the inherited chat inside the guarded run and cover the scoped persist
1 parent 515b50d commit 654f0ca

5 files changed

Lines changed: 392 additions & 40 deletions

File tree

‎apps/sim/app/api/webhooks/agentmail/route.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,20 @@ export const POST = withRouteHandler(async (req: Request) => {
185185
const emailMessageId = message.message_id
186186
const inReplyTo = message.in_reply_to || null
187187

188+
// Message ids are sender-controlled and shared with every recipient of a mail,
189+
// so a task is only ever matched inside the workspace that owns this inbox.
188190
const [existingResult, isAllowed, recentCount, parentTaskResult, isEntitled] =
189191
await Promise.all([
190192
emailMessageId
191193
? db
192194
.select({ id: mothershipInboxTask.id })
193195
.from(mothershipInboxTask)
194-
.where(eq(mothershipInboxTask.emailMessageId, emailMessageId))
196+
.where(
197+
and(
198+
eq(mothershipInboxTask.workspaceId, result.id),
199+
eq(mothershipInboxTask.emailMessageId, emailMessageId)
200+
)
201+
)
195202
.limit(1)
196203
: Promise.resolve([]),
197204
isSenderAllowed(fromEmail, result.id),
@@ -200,7 +207,12 @@ export const POST = withRouteHandler(async (req: Request) => {
200207
? db
201208
.select({ chatId: mothershipInboxTask.chatId })
202209
.from(mothershipInboxTask)
203-
.where(eq(mothershipInboxTask.responseMessageId, inReplyTo))
210+
.where(
211+
and(
212+
eq(mothershipInboxTask.workspaceId, result.id),
213+
eq(mothershipInboxTask.responseMessageId, inReplyTo)
214+
)
215+
)
204216
.limit(1)
205217
: Promise.resolve([]),
206218
hasWorkspaceInboxAccess(result.id),
Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,309 @@
1+
/**
2+
* An inbox reply continues its parent task's chat, and the parent is found by the
3+
* `In-Reply-To` header — a value every recipient of the agent's reply holds. Against
4+
* real PostgreSQL, a reply delivered to one workspace's inbox must never reach a chat,
5+
* task, or transcript owned by another workspace, while a reply inside the workspace
6+
* still continues its thread. The webhook is signed with the real Svix scheme.
7+
*/
8+
import { authBanMock } from '@sim/testing/mocks/auth-ban.mock'
9+
import {
10+
billingAttributionMock,
11+
billingAttributionMockFns,
12+
} from '@sim/testing/mocks/billing-attribution.mock'
13+
import {
14+
billingSubscriptionMock,
15+
billingSubscriptionMockFns,
16+
} from '@sim/testing/mocks/billing-subscription.mock'
17+
import {
18+
mothershipHeadlessLifecycleMock,
19+
mothershipHeadlessLifecycleMockFns,
20+
} from '@sim/testing/mocks/mothership-headless-lifecycle.mock'
21+
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
22+
23+
const { mockExecuteInboxTask, mockSendInboxResponse } = vi.hoisted(() => ({
24+
mockExecuteInboxTask: vi.fn(),
25+
mockSendInboxResponse: vi.fn(),
26+
}))
27+
28+
vi.mock('@/lib/auth/ban', () => authBanMock)
29+
vi.mock('@/lib/billing/core/billing-attribution', () => billingAttributionMock)
30+
vi.mock('@/lib/billing/core/subscription', () => billingSubscriptionMock)
31+
vi.mock('@/lib/mothership/request/lifecycle/headless', () => mothershipHeadlessLifecycleMock)
32+
vi.mock('@/lib/mothership/request/lifecycle/start', () => ({
33+
requestChatTitle: async () => null,
34+
}))
35+
vi.mock('@/lib/mothership/inbox/response', () => ({
36+
sendInboxResponse: mockSendInboxResponse,
37+
}))
38+
vi.mock('@/lib/mothership/inbox/executor', () => ({
39+
executeInboxTask: mockExecuteInboxTask,
40+
}))
41+
42+
import { db } from '@sim/db'
43+
import {
44+
copilotChats,
45+
copilotMessages,
46+
mothershipInboxTask,
47+
mothershipInboxWebhook,
48+
permissions,
49+
user,
50+
workspace,
51+
} from '@sim/db/schema'
52+
import { generateId } from '@sim/utils/id'
53+
import { and, eq, inArray } from 'drizzle-orm'
54+
import { NextRequest } from 'next/server'
55+
import { Webhook } from 'svix'
56+
import { persistCopilotChatTurn } from '@/lib/mothership/chat/messages-store'
57+
import { buildPersistedUserMessage } from '@/lib/mothership/chat/persisted-message'
58+
import { POST } from '@/app/api/webhooks/agentmail/route'
59+
60+
const { executeInboxTask } = await vi.importActual<
61+
typeof import('@/lib/mothership/inbox/executor')
62+
>('@/lib/mothership/inbox/executor')
63+
const { mockRunHeadlessCopilotLifecycle } = mothershipHeadlessLifecycleMockFns
64+
65+
interface InboxWorkspace {
66+
id: string
67+
ownerId: string
68+
ownerEmail: string
69+
inboxId: string
70+
secret: string
71+
}
72+
73+
const victim = inboxWorkspace()
74+
const attacker = inboxWorkspace()
75+
const victimChatId = generateId()
76+
const victimReplyMessageId = `<${generateId()}@agentmail.to>`
77+
const victimChatUpdatedAt = new Date('2026-01-01T00:00:00.000Z')
78+
79+
function inboxWorkspace(): InboxWorkspace {
80+
const id = generateId()
81+
return {
82+
id,
83+
ownerId: generateId(),
84+
ownerEmail: `${id}@inbox-thread-scope.test`,
85+
inboxId: `${id}@agentmail.to`,
86+
secret: `whsec_${Buffer.from(generateId()).toString('base64')}`,
87+
}
88+
}
89+
90+
async function seedWorkspace(ws: InboxWorkspace): Promise<void> {
91+
const now = new Date()
92+
await db.insert(user).values({
93+
id: ws.ownerId,
94+
name: 'Inbox thread scope fixture',
95+
email: ws.ownerEmail,
96+
emailVerified: true,
97+
createdAt: now,
98+
updatedAt: now,
99+
})
100+
await db.insert(workspace).values({
101+
id: ws.id,
102+
name: 'Inbox thread scope fixture',
103+
ownerId: ws.ownerId,
104+
billedAccountUserId: ws.ownerId,
105+
inboxEnabled: true,
106+
inboxAddress: ws.inboxId,
107+
inboxProviderId: ws.inboxId,
108+
})
109+
await db.insert(permissions).values({
110+
id: generateId(),
111+
userId: ws.ownerId,
112+
entityType: 'workspace',
113+
entityId: ws.id,
114+
permissionType: 'admin',
115+
})
116+
await db.insert(mothershipInboxWebhook).values({
117+
id: generateId(),
118+
workspaceId: ws.id,
119+
webhookId: generateId(),
120+
secret: ws.secret,
121+
})
122+
}
123+
124+
/** Delivers a `message.received` webhook to `ws`'s inbox from its owner, signed with its secret. */
125+
async function deliver(
126+
ws: InboxWorkspace,
127+
message: { messageId: string; inReplyTo?: string }
128+
): Promise<Response> {
129+
const body = JSON.stringify({
130+
event_type: 'message.received',
131+
message: {
132+
message_id: message.messageId,
133+
thread_id: generateId(),
134+
inbox_id: ws.inboxId,
135+
from: `Owner <${ws.ownerEmail}>`,
136+
to: [ws.inboxId],
137+
subject: 'Re: quarterly numbers',
138+
text: 'Please continue this thread.',
139+
created_at: new Date().toISOString(),
140+
...(message.inReplyTo ? { in_reply_to: message.inReplyTo } : {}),
141+
},
142+
})
143+
const svixId = `msg_${generateId()}`
144+
const timestamp = new Date()
145+
return POST(
146+
new NextRequest('http://localhost:3000/api/webhooks/agentmail', {
147+
method: 'POST',
148+
headers: {
149+
'content-type': 'application/json',
150+
'svix-id': svixId,
151+
'svix-timestamp': String(Math.floor(timestamp.getTime() / 1000)),
152+
'svix-signature': new Webhook(ws.secret).sign(svixId, timestamp, body),
153+
},
154+
body,
155+
}),
156+
{ params: Promise.resolve({}) }
157+
)
158+
}
159+
160+
async function taskFor(workspaceId: string, emailMessageId: string) {
161+
const [task] = await db
162+
.select()
163+
.from(mothershipInboxTask)
164+
.where(
165+
and(
166+
eq(mothershipInboxTask.workspaceId, workspaceId),
167+
eq(mothershipInboxTask.emailMessageId, emailMessageId)
168+
)
169+
)
170+
return task
171+
}
172+
173+
async function messageCount(chatId: string): Promise<number> {
174+
const messages = await db
175+
.select({ id: copilotMessages.id })
176+
.from(copilotMessages)
177+
.where(eq(copilotMessages.chatId, chatId))
178+
return messages.length
179+
}
180+
181+
async function victimChatState() {
182+
const [chat] = await db
183+
.select({ updatedAt: copilotChats.updatedAt })
184+
.from(copilotChats)
185+
.where(eq(copilotChats.id, victimChatId))
186+
return { updatedAt: chat?.updatedAt, messageCount: await messageCount(victimChatId) }
187+
}
188+
189+
describe('inbox reply threading stays inside the receiving workspace', () => {
190+
beforeAll(async () => {
191+
await seedWorkspace(victim)
192+
await seedWorkspace(attacker)
193+
await db.insert(copilotChats).values({
194+
id: victimChatId,
195+
userId: victim.ownerId,
196+
workspaceId: victim.id,
197+
type: 'mothership',
198+
updatedAt: victimChatUpdatedAt,
199+
})
200+
await db.insert(mothershipInboxTask).values({
201+
id: generateId(),
202+
workspaceId: victim.id,
203+
fromEmail: victim.ownerEmail,
204+
subject: 'quarterly numbers',
205+
status: 'completed',
206+
chatId: victimChatId,
207+
responseMessageId: victimReplyMessageId,
208+
})
209+
})
210+
211+
beforeEach(() => {
212+
mockExecuteInboxTask.mockResolvedValue(undefined)
213+
mockSendInboxResponse.mockResolvedValue(null)
214+
billingSubscriptionMockFns.mockHasWorkspaceInboxAccess.mockResolvedValue(true)
215+
billingAttributionMockFns.mockResolveBillingAttribution.mockResolvedValue({})
216+
mockRunHeadlessCopilotLifecycle.mockResolvedValue({
217+
success: true,
218+
content: 'Here are the numbers.',
219+
contentBlocks: [],
220+
toolCalls: [],
221+
})
222+
})
223+
224+
afterAll(async () => {
225+
const ownerIds = [victim.ownerId, attacker.ownerId]
226+
await db.delete(permissions).where(inArray(permissions.userId, ownerIds))
227+
await db.delete(workspace).where(inArray(workspace.id, [victim.id, attacker.id]))
228+
await db.delete(user).where(inArray(user.id, ownerIds))
229+
})
230+
231+
it("does not adopt another workspace's chat for a reply to that workspace's message", async () => {
232+
const messageId = `<${generateId()}@attacker.test>`
233+
234+
const response = await deliver(attacker, { messageId, inReplyTo: victimReplyMessageId })
235+
236+
expect(response.status).toBe(200)
237+
const task = await taskFor(attacker.id, messageId)
238+
expect(task?.inReplyTo).toBe(victimReplyMessageId)
239+
expect(task?.chatId).toBeNull()
240+
})
241+
242+
it('continues the parent chat for a reply inside the same workspace', async () => {
243+
const messageId = `<${generateId()}@victim.test>`
244+
245+
await deliver(victim, { messageId, inReplyTo: victimReplyMessageId })
246+
247+
expect((await taskFor(victim.id, messageId))?.chatId).toBe(victimChatId)
248+
})
249+
250+
it("accepts a message whose id another workspace's inbox already received", async () => {
251+
const sharedMessageId = `<${generateId()}@cc-both.test>`
252+
253+
await deliver(attacker, { messageId: sharedMessageId })
254+
await deliver(victim, { messageId: sharedMessageId })
255+
256+
expect(await taskFor(attacker.id, sharedMessageId)).toMatchObject({ status: 'received' })
257+
expect(await taskFor(victim.id, sharedMessageId)).toMatchObject({ status: 'received' })
258+
})
259+
260+
it("runs a task carrying another workspace's chat in a fresh chat of its own workspace", async () => {
261+
const taskId = generateId()
262+
await db.insert(mothershipInboxTask).values({
263+
id: taskId,
264+
workspaceId: attacker.id,
265+
fromEmail: attacker.ownerEmail,
266+
subject: 'Re: quarterly numbers',
267+
bodyText: 'Append me to the victim transcript.',
268+
status: 'received',
269+
chatId: victimChatId,
270+
inReplyTo: victimReplyMessageId,
271+
})
272+
273+
await executeInboxTask(taskId)
274+
275+
expect(mockRunHeadlessCopilotLifecycle).toHaveBeenCalledOnce()
276+
const [, options] = mockRunHeadlessCopilotLifecycle.mock.calls[0]
277+
expect(options).toMatchObject({ workspaceId: attacker.id })
278+
expect(options.chatId).not.toBe(victimChatId)
279+
const [runChat] = await db
280+
.select({ workspaceId: copilotChats.workspaceId })
281+
.from(copilotChats)
282+
.where(eq(copilotChats.id, options.chatId))
283+
expect(runChat?.workspaceId).toBe(attacker.id)
284+
285+
const [task] = await db
286+
.select({ status: mothershipInboxTask.status, chatId: mothershipInboxTask.chatId })
287+
.from(mothershipInboxTask)
288+
.where(eq(mothershipInboxTask.id, taskId))
289+
expect(task).toEqual({ status: 'completed', chatId: options.chatId })
290+
expect(await messageCount(options.chatId)).toBe(2)
291+
expect(await victimChatState()).toEqual({
292+
updatedAt: victimChatUpdatedAt,
293+
messageCount: 0,
294+
})
295+
})
296+
297+
it("writes nothing when a turn is persisted into another workspace's chat", async () => {
298+
await persistCopilotChatTurn(
299+
victimChatId,
300+
[buildPersistedUserMessage({ id: generateId(), content: 'Injected turn' })],
301+
{ workspaceId: attacker.id }
302+
)
303+
304+
expect(await victimChatState()).toEqual({
305+
updatedAt: victimChatUpdatedAt,
306+
messageCount: 0,
307+
})
308+
})
309+
})

‎apps/sim/lib/mothership/chat/messages-store.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,16 +99,26 @@ export async function appendCopilotChatMessages(
9999
* because the user deleted the conversation after asking — resurrecting it with
100100
* a reply would undo that deletion, and the caller still has its reply in the
101101
* response. Throws on a write failure.
102+
*
103+
* `scope.workspaceId` confines the write to a chat in that workspace; a chat
104+
* elsewhere receives nothing, exactly like a deleted one.
102105
*/
103106
export async function persistCopilotChatTurn(
104107
chatId: string,
105-
messages: PersistedMessage[]
108+
messages: PersistedMessage[],
109+
scope?: { workspaceId: string }
106110
): Promise<void> {
107111
await db.transaction(async (tx) => {
108112
const [updated] = await tx
109113
.update(copilotChats)
110114
.set({ updatedAt: new Date() })
111-
.where(and(eq(copilotChats.id, chatId), isNull(copilotChats.deletedAt)))
115+
.where(
116+
and(
117+
eq(copilotChats.id, chatId),
118+
isNull(copilotChats.deletedAt),
119+
...(scope ? [eq(copilotChats.workspaceId, scope.workspaceId)] : [])
120+
)
121+
)
112122
.returning({ model: copilotChats.model })
113123
if (!updated) return
114124
await appendCopilotChatMessages(chatId, messages, { chatModel: updated.model ?? null }, tx)

0 commit comments

Comments
 (0)