Skip to content

Commit 29e5eb6

Browse files
committed
fix(inbox): scope reply threading to the receiving workspace
1 parent 4815ce9 commit 29e5eb6

5 files changed

Lines changed: 368 additions & 39 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: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
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 { POST } from '@/app/api/webhooks/agentmail/route'
57+
58+
const { executeInboxTask } = await vi.importActual<
59+
typeof import('@/lib/mothership/inbox/executor')
60+
>('@/lib/mothership/inbox/executor')
61+
const { mockRunHeadlessCopilotLifecycle } = mothershipHeadlessLifecycleMockFns
62+
63+
interface InboxWorkspace {
64+
id: string
65+
ownerId: string
66+
ownerEmail: string
67+
inboxId: string
68+
secret: string
69+
}
70+
71+
const victim = inboxWorkspace()
72+
const attacker = inboxWorkspace()
73+
const victimChatId = generateId()
74+
const victimReplyMessageId = `<${generateId()}@agentmail.to>`
75+
const victimChatUpdatedAt = new Date('2026-01-01T00:00:00.000Z')
76+
77+
function inboxWorkspace(): InboxWorkspace {
78+
const id = generateId()
79+
return {
80+
id,
81+
ownerId: generateId(),
82+
ownerEmail: `${id}@inbox-thread-scope.test`,
83+
inboxId: `${id}@agentmail.to`,
84+
secret: `whsec_${Buffer.from(generateId()).toString('base64')}`,
85+
}
86+
}
87+
88+
async function seedWorkspace(ws: InboxWorkspace): Promise<void> {
89+
const now = new Date()
90+
await db.insert(user).values({
91+
id: ws.ownerId,
92+
name: 'Inbox thread scope fixture',
93+
email: ws.ownerEmail,
94+
emailVerified: true,
95+
createdAt: now,
96+
updatedAt: now,
97+
})
98+
await db.insert(workspace).values({
99+
id: ws.id,
100+
name: 'Inbox thread scope fixture',
101+
ownerId: ws.ownerId,
102+
billedAccountUserId: ws.ownerId,
103+
inboxEnabled: true,
104+
inboxAddress: ws.inboxId,
105+
inboxProviderId: ws.inboxId,
106+
})
107+
await db.insert(permissions).values({
108+
id: generateId(),
109+
userId: ws.ownerId,
110+
entityType: 'workspace',
111+
entityId: ws.id,
112+
permissionType: 'admin',
113+
})
114+
await db.insert(mothershipInboxWebhook).values({
115+
id: generateId(),
116+
workspaceId: ws.id,
117+
webhookId: generateId(),
118+
secret: ws.secret,
119+
})
120+
}
121+
122+
/** Delivers a `message.received` webhook to `ws`'s inbox from its owner, signed with its secret. */
123+
async function deliver(
124+
ws: InboxWorkspace,
125+
message: { messageId: string; inReplyTo?: string }
126+
): Promise<Response> {
127+
const body = JSON.stringify({
128+
event_type: 'message.received',
129+
message: {
130+
message_id: message.messageId,
131+
thread_id: generateId(),
132+
inbox_id: ws.inboxId,
133+
from: `Owner <${ws.ownerEmail}>`,
134+
to: [ws.inboxId],
135+
subject: 'Re: quarterly numbers',
136+
text: 'Please continue this thread.',
137+
created_at: new Date().toISOString(),
138+
...(message.inReplyTo ? { in_reply_to: message.inReplyTo } : {}),
139+
},
140+
})
141+
const svixId = `msg_${generateId()}`
142+
const timestamp = new Date()
143+
return POST(
144+
new NextRequest('http://localhost:3000/api/webhooks/agentmail', {
145+
method: 'POST',
146+
headers: {
147+
'content-type': 'application/json',
148+
'svix-id': svixId,
149+
'svix-timestamp': String(Math.floor(timestamp.getTime() / 1000)),
150+
'svix-signature': new Webhook(ws.secret).sign(svixId, timestamp, body),
151+
},
152+
body,
153+
})
154+
)
155+
}
156+
157+
async function taskFor(workspaceId: string, emailMessageId: string) {
158+
const [task] = await db
159+
.select()
160+
.from(mothershipInboxTask)
161+
.where(
162+
and(
163+
eq(mothershipInboxTask.workspaceId, workspaceId),
164+
eq(mothershipInboxTask.emailMessageId, emailMessageId)
165+
)
166+
)
167+
return task
168+
}
169+
170+
async function victimChatState() {
171+
const [chat] = await db
172+
.select({ updatedAt: copilotChats.updatedAt })
173+
.from(copilotChats)
174+
.where(eq(copilotChats.id, victimChatId))
175+
const messages = await db
176+
.select({ id: copilotMessages.id })
177+
.from(copilotMessages)
178+
.where(eq(copilotMessages.chatId, victimChatId))
179+
return { updatedAt: chat?.updatedAt, messageCount: messages.length }
180+
}
181+
182+
describe('inbox reply threading stays inside the receiving workspace', () => {
183+
beforeAll(async () => {
184+
await seedWorkspace(victim)
185+
await seedWorkspace(attacker)
186+
await db.insert(copilotChats).values({
187+
id: victimChatId,
188+
userId: victim.ownerId,
189+
workspaceId: victim.id,
190+
type: 'mothership',
191+
updatedAt: victimChatUpdatedAt,
192+
})
193+
await db.insert(mothershipInboxTask).values({
194+
id: generateId(),
195+
workspaceId: victim.id,
196+
fromEmail: victim.ownerEmail,
197+
subject: 'quarterly numbers',
198+
status: 'completed',
199+
chatId: victimChatId,
200+
responseMessageId: victimReplyMessageId,
201+
})
202+
})
203+
204+
beforeEach(() => {
205+
mockExecuteInboxTask.mockResolvedValue(undefined)
206+
mockSendInboxResponse.mockResolvedValue(null)
207+
billingSubscriptionMockFns.mockHasWorkspaceInboxAccess.mockResolvedValue(true)
208+
billingAttributionMockFns.mockResolveBillingAttribution.mockResolvedValue({})
209+
mockRunHeadlessCopilotLifecycle.mockResolvedValue({
210+
success: true,
211+
content: 'Here are the numbers.',
212+
contentBlocks: [],
213+
toolCalls: [],
214+
})
215+
})
216+
217+
afterAll(async () => {
218+
const ownerIds = [victim.ownerId, attacker.ownerId]
219+
await db.delete(permissions).where(inArray(permissions.userId, ownerIds))
220+
await db.delete(workspace).where(inArray(workspace.id, [victim.id, attacker.id]))
221+
await db.delete(user).where(inArray(user.id, ownerIds))
222+
})
223+
224+
it("does not adopt another workspace's chat for a reply to that workspace's message", async () => {
225+
const messageId = `<${generateId()}@attacker.test>`
226+
227+
const response = await deliver(attacker, { messageId, inReplyTo: victimReplyMessageId })
228+
229+
expect(response.status).toBe(200)
230+
const task = await taskFor(attacker.id, messageId)
231+
expect(task?.inReplyTo).toBe(victimReplyMessageId)
232+
expect(task?.chatId).toBeNull()
233+
})
234+
235+
it('continues the parent chat for a reply inside the same workspace', async () => {
236+
const messageId = `<${generateId()}@victim.test>`
237+
238+
await deliver(victim, { messageId, inReplyTo: victimReplyMessageId })
239+
240+
expect((await taskFor(victim.id, messageId))?.chatId).toBe(victimChatId)
241+
})
242+
243+
it("accepts a message whose id another workspace's inbox already received", async () => {
244+
const sharedMessageId = `<${generateId()}@cc-both.test>`
245+
246+
await deliver(attacker, { messageId: sharedMessageId })
247+
await deliver(victim, { messageId: sharedMessageId })
248+
249+
expect(await taskFor(attacker.id, sharedMessageId)).toMatchObject({ status: 'received' })
250+
expect(await taskFor(victim.id, sharedMessageId)).toMatchObject({ status: 'received' })
251+
})
252+
253+
it("runs a task carrying another workspace's chat in a fresh chat of its own workspace", async () => {
254+
const taskId = generateId()
255+
await db.insert(mothershipInboxTask).values({
256+
id: taskId,
257+
workspaceId: attacker.id,
258+
fromEmail: attacker.ownerEmail,
259+
subject: 'Re: quarterly numbers',
260+
bodyText: 'Append me to the victim transcript.',
261+
status: 'received',
262+
chatId: victimChatId,
263+
inReplyTo: victimReplyMessageId,
264+
})
265+
266+
await executeInboxTask(taskId)
267+
268+
expect(mockRunHeadlessCopilotLifecycle).toHaveBeenCalledOnce()
269+
const [, options] = mockRunHeadlessCopilotLifecycle.mock.calls[0]
270+
expect(options).toMatchObject({ workspaceId: attacker.id })
271+
expect(options.chatId).not.toBe(victimChatId)
272+
const [runChat] = await db
273+
.select({ workspaceId: copilotChats.workspaceId })
274+
.from(copilotChats)
275+
.where(eq(copilotChats.id, options.chatId))
276+
expect(runChat?.workspaceId).toBe(attacker.id)
277+
278+
const [task] = await db
279+
.select({ status: mothershipInboxTask.status, chatId: mothershipInboxTask.chatId })
280+
.from(mothershipInboxTask)
281+
.where(eq(mothershipInboxTask.id, taskId))
282+
expect(task).toEqual({ status: 'completed', chatId: options.chatId })
283+
expect(await victimChatState()).toEqual({
284+
updatedAt: victimChatUpdatedAt,
285+
messageCount: 0,
286+
})
287+
})
288+
})

‎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)