fix(webhooks): resolve the AgentMail tenant before verifying the signature (#6431)

* fix(webhooks): resolve the AgentMail tenant before verifying the signature

The receiver identified the delivering workspace by trying every stored
webhook secret in turn, so each request cost one HMAC over the full body
per tenant. Resolve the workspace from the envelope's inbox id first and
check a single secret, and make that id uniquely indexed so the one-check
property holds by construction.

Also gates the route through the shared admission gate, like the other
public receivers, and gives it a body cap sized for an email envelope.

* fix(db): declare the inbox provider index partial in the schema too

The migration built a partial unique index but the Drizzle definition and
snapshot omitted the predicate, so a regenerate would have diverged. Also
records at the registration site why only message-bearing event types can
be subscribed.

* fix(db): drop the duplicate 0285 journal entry

The regenerate that added the partial-index predicate appended a second
entry for the same migration, so drizzle would have replayed it — dropping
and rebuilding the index, leaving a window with no unique constraint.
This commit is contained in:
Waleed
2026-08-08 12:59:45 -07:00
committed by GitHub
parent 5a9f64a273
commit ea5df41a57
9 changed files with 19183 additions and 35 deletions
@@ -0,0 +1,235 @@
/**
* @vitest-environment node
*/
import { dbChainMock, dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing'
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { mockVerify, mockTryAdmit, mockRelease, mockEq, mockExecuteInboxTask, tables } = vi.hoisted(
() => ({
mockVerify: vi.fn(),
mockTryAdmit: vi.fn(),
mockRelease: vi.fn(),
mockEq: vi.fn((left: unknown, right: unknown) => ({ left, right })),
mockExecuteInboxTask: vi.fn(),
/** Table-qualified column names keep the eq assertions unambiguous. */
tables: {
workspace: {
id: 'workspace.id',
inboxEnabled: 'workspace.inboxEnabled',
inboxAddress: 'workspace.inboxAddress',
inboxProviderId: 'workspace.inboxProviderId',
},
mothershipInboxWebhook: {
workspaceId: 'mothershipInboxWebhook.workspaceId',
secret: 'mothershipInboxWebhook.secret',
},
mothershipInboxTask: {
id: 'mothershipInboxTask.id',
chatId: 'mothershipInboxTask.chatId',
emailMessageId: 'mothershipInboxTask.emailMessageId',
responseMessageId: 'mothershipInboxTask.responseMessageId',
workspaceId: 'mothershipInboxTask.workspaceId',
createdAt: 'mothershipInboxTask.createdAt',
status: 'mothershipInboxTask.status',
},
mothershipInboxAllowedSender: {
id: 'mothershipInboxAllowedSender.id',
workspaceId: 'mothershipInboxAllowedSender.workspaceId',
email: 'mothershipInboxAllowedSender.email',
},
permissions: {
userId: 'permissions.userId',
entityType: 'permissions.entityType',
entityId: 'permissions.entityId',
},
user: { id: 'user.id', email: 'user.email' },
},
})
)
vi.mock('@sim/db', () => ({ ...dbChainMock, ...tables }))
vi.mock('drizzle-orm', () => ({
and: vi.fn((...conditions: unknown[]) => conditions),
eq: mockEq,
gt: vi.fn((left: unknown, right: unknown) => ({ gt: [left, right] })),
ne: vi.fn((left: unknown, right: unknown) => ({ ne: [left, right] })),
sql: Object.assign((strings: TemplateStringsArray) => ({ strings }), {
raw: (value: string) => ({ value }),
}),
}))
vi.mock('svix', () => ({
Webhook: class {
private readonly secret: string
constructor(secret: string) {
this.secret = secret
}
verify(payload: string, headers: Record<string, string>) {
return mockVerify(this.secret, payload, headers)
}
},
}))
vi.mock('@/lib/core/admission/gate', () => ({
tryAdmit: mockTryAdmit,
admissionRejectedResponse: () => new Response(null, { status: 429 }),
}))
vi.mock('@/lib/billing/core/subscription', () => ({
hasWorkspaceInboxAccess: vi.fn().mockResolvedValue(true),
}))
vi.mock('@/lib/core/async-jobs/region', () => ({
resolveTriggerRegion: vi.fn().mockResolvedValue('us-east-1'),
}))
vi.mock('@/lib/mothership/inbox/executor', () => ({
executeInboxTask: mockExecuteInboxTask,
}))
import { AGENTMAIL_WEBHOOK_MAX_BODY_BYTES, WEBHOOK_MAX_BODY_BYTES } from '@/lib/webhooks/constants'
import { POST } from '@/app/api/webhooks/agentmail/route'
const TARGET_INBOX_ID = 'agent-b@agentmail.to'
const ROUTED_WORKSPACE = {
id: 'workspace-b',
inboxEnabled: true,
inboxAddress: TARGET_INBOX_ID,
webhookSecret: 'whsec_b',
}
/**
* The two `mothershipInboxTask` lookups race inside one `Promise.all` and the
* shared mock dequeues on resolution, so both sets must be queued empty — the
* hourly count falls back to zero on an empty result either way.
*/
function queueAcceptedDeliveryLookups(): void {
queueTableRows(tables.mothershipInboxTask, [])
queueTableRows(tables.mothershipInboxTask, [])
queueTableRows(tables.mothershipInboxAllowedSender, [{ id: 'allowed-1' }])
queueTableRows(tables.permissions, [])
}
function envelope(messageOverrides: Record<string, unknown> = {}): string {
return JSON.stringify({
event_type: 'message.received',
message: {
message_id: 'msg-1',
thread_id: 'thread-1',
inbox_id: TARGET_INBOX_ID,
from: 'Member <member@example.com>',
to: [TARGET_INBOX_ID],
subject: 'Hello',
text: 'Body text',
created_at: '2026-08-08T00:00:00.000Z',
...messageOverrides,
},
})
}
function webhookRequest(body: string, headers: Record<string, string> = {}): Request {
return new Request('https://sim.ai/api/webhooks/agentmail', {
method: 'POST',
headers: {
'content-type': 'application/json',
'svix-id': 'msg_1',
'svix-timestamp': '1786000000',
'svix-signature': 'v1,AAAA',
...headers,
},
body,
})
}
describe('POST /api/webhooks/agentmail', () => {
beforeEach(() => {
vi.clearAllMocks()
resetDbChainMock()
mockTryAdmit.mockReturnValue({ release: mockRelease })
mockExecuteInboxTask.mockResolvedValue(undefined)
mockVerify.mockImplementation(() => {
throw new Error('signature mismatch')
})
})
it('checks the signature against only the secret the payload routes to', async () => {
queueTableRows(tables.workspace, [ROUTED_WORKSPACE])
const response = await POST(webhookRequest(envelope()))
expect(response.status).toBe(401)
expect(mockEq).toHaveBeenCalledWith(tables.workspace.inboxProviderId, TARGET_INBOX_ID)
expect(dbChainMockFns.limit).toHaveBeenCalledWith(1)
expect(mockVerify).toHaveBeenCalledTimes(1)
expect(mockVerify).toHaveBeenCalledWith('whsec_b', expect.any(String), expect.any(Object))
})
it('rejects a payload naming an inbox no workspace owns, without hashing it', async () => {
queueTableRows(tables.workspace, [])
mockVerify.mockReturnValue(undefined)
const response = await POST(
webhookRequest(envelope({ inbox_id: 'agent-unknown@agentmail.to' }))
)
expect(response.status).toBe(401)
expect(mockVerify).not.toHaveBeenCalled()
})
it('rejects an unroutable body before it reaches the database or the hash', async () => {
const missingInboxId = await POST(
webhookRequest(JSON.stringify({ message: { from: 'a@b.c' } }))
)
const malformedJson = await POST(webhookRequest('not json'))
expect(missingInboxId.status).toBe(401)
expect(malformedJson.status).toBe(401)
expect(dbChainMockFns.select).not.toHaveBeenCalled()
expect(mockVerify).not.toHaveBeenCalled()
})
it('rejects a body above the AgentMail cap even though it fits the shared webhook cap', async () => {
const oversized = AGENTMAIL_WEBHOOK_MAX_BODY_BYTES + 1
expect(oversized).toBeLessThan(WEBHOOK_MAX_BODY_BYTES)
const response = await POST(webhookRequest(envelope(), { 'content-length': String(oversized) }))
expect(response.status).toBe(413)
expect(mockVerify).not.toHaveBeenCalled()
})
it('sheds load at the admission gate before reading the body', async () => {
mockTryAdmit.mockReturnValue(null)
const response = await POST(webhookRequest(envelope()))
expect(response.status).toBe(429)
expect(dbChainMockFns.select).not.toHaveBeenCalled()
expect(mockVerify).not.toHaveBeenCalled()
})
it('releases the admission ticket once the request settles', async () => {
queueTableRows(tables.workspace, [ROUTED_WORKSPACE])
await POST(webhookRequest(envelope()))
expect(mockRelease).toHaveBeenCalledTimes(1)
})
it('accepts a delivery whose signature verifies against the routed secret', async () => {
mockVerify.mockReturnValue(undefined)
queueTableRows(tables.workspace, [ROUTED_WORKSPACE])
queueAcceptedDeliveryLookups()
const response = await POST(webhookRequest(envelope()))
expect(response.status).toBe(200)
expect(mockVerify).toHaveBeenCalledTimes(1)
expect(dbChainMockFns.values).toHaveBeenCalledWith(
expect.objectContaining({ workspaceId: ROUTED_WORKSPACE.id, status: 'received' })
)
})
})
+58 -35
View File
@@ -11,15 +11,17 @@ import { createLogger } from '@sim/logger'
import { getErrorMessage } from '@sim/utils/errors'
import { generateId } from '@sim/utils/id'
import { tasks } from '@trigger.dev/sdk'
import { and, eq, gt, isNotNull, ne, sql } from 'drizzle-orm'
import { and, eq, gt, ne, sql } from 'drizzle-orm'
import { NextResponse } from 'next/server'
import { Webhook } from 'svix'
import {
agentMailEnvelopeSchema,
agentMailMessageSchema,
agentMailRoutingSchema,
webhookSvixHeadersSchema,
} from '@/lib/api/contracts/webhooks'
import { hasWorkspaceInboxAccess } from '@/lib/billing/core/subscription'
import { admissionRejectedResponse, tryAdmit } from '@/lib/core/admission/gate'
import { resolveTriggerRegion } from '@/lib/core/async-jobs/region'
import { isTriggerDevEnabled } from '@/lib/core/config/env-flags'
import {
@@ -30,7 +32,7 @@ import {
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { executeInboxTask } from '@/lib/mothership/inbox/executor'
import type { AgentMailWebhookPayload, RejectionReason } from '@/lib/mothership/inbox/types'
import { WEBHOOK_MAX_BODY_BYTES } from '@/lib/webhooks/constants'
import { AGENTMAIL_WEBHOOK_MAX_BODY_BYTES } from '@/lib/webhooks/constants'
const logger = createLogger('AgentMailWebhook')
@@ -39,20 +41,43 @@ const MAX_EMAILS_PER_HOUR = 20
const AGENTMAIL_BODY_LABEL = 'AgentMail webhook body'
/** Svix throws on a bad signature, stale timestamp, or malformed header. */
function verifiesAgainst(
secret: string,
rawBody: string,
headers: Record<string, string>
): boolean {
try {
new Webhook(secret).verify(rawBody, headers)
return true
} catch {
return false
}
}
/**
* Bound the unauthenticated AgentMail webhook body before buffering it for Svix
* signature verification, so an oversized payload cannot exhaust pod memory.
*/
async function readAgentMailBody(req: Request): Promise<string> {
assertContentLengthWithinLimit(req.headers, WEBHOOK_MAX_BODY_BYTES, AGENTMAIL_BODY_LABEL)
assertContentLengthWithinLimit(
req.headers,
AGENTMAIL_WEBHOOK_MAX_BODY_BYTES,
AGENTMAIL_BODY_LABEL
)
const buffer = await readStreamToBufferWithLimit(req.body, {
maxBytes: WEBHOOK_MAX_BODY_BYTES,
maxBytes: AGENTMAIL_WEBHOOK_MAX_BODY_BYTES,
label: AGENTMAIL_BODY_LABEL,
})
return new TextDecoder().decode(buffer)
}
export const POST = withRouteHandler(async (req: Request) => {
const ticket = tryAdmit()
if (!ticket) {
return admissionRejectedResponse()
}
try {
let rawBody: string
try {
@@ -60,7 +85,7 @@ export const POST = withRouteHandler(async (req: Request) => {
} catch (bodyError) {
if (isPayloadSizeLimitError(bodyError)) {
logger.warn('Rejected oversized AgentMail webhook body', {
maxBytes: WEBHOOK_MAX_BODY_BYTES,
maxBytes: AGENTMAIL_WEBHOOK_MAX_BODY_BYTES,
observedBytes: bodyError.observedBytes,
})
return NextResponse.json({ error: 'Request body too large' }, { status: 413 })
@@ -77,38 +102,43 @@ export const POST = withRouteHandler(async (req: Request) => {
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
}
const webhookCandidates = await db
let parsedBody: unknown
try {
parsedBody = JSON.parse(rawBody)
} catch {
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
}
/**
* Route to one tenant before hashing: svix HMACs the whole body synchronously,
* so checking every workspace secret would let one unauthenticated request cost
* a hash per tenant. `inbox_id` is attacker-controlled but only selects which
* secret to check — verification still binds the payload's inbox to the workspace.
*/
const routingResult = agentMailRoutingSchema.safeParse(parsedBody)
if (!routingResult.success) {
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
}
const inboxId = routingResult.data.message.inbox_id
const [result] = await db
.select({
id: workspace.id,
inboxEnabled: workspace.inboxEnabled,
inboxAddress: workspace.inboxAddress,
inboxProviderId: workspace.inboxProviderId,
webhookSecret: mothershipInboxWebhook.secret,
})
.from(workspace)
.leftJoin(mothershipInboxWebhook, eq(mothershipInboxWebhook.workspaceId, workspace.id))
.where(isNotNull(mothershipInboxWebhook.secret))
.innerJoin(mothershipInboxWebhook, eq(mothershipInboxWebhook.workspaceId, workspace.id))
.where(eq(workspace.inboxProviderId, inboxId))
.limit(1)
let result: (typeof webhookCandidates)[number] | undefined
for (const candidate of webhookCandidates) {
if (!candidate.webhookSecret) continue
try {
const wh = new Webhook(candidate.webhookSecret)
wh.verify(rawBody, headersResult.data)
result = candidate
break
} catch {}
}
if (!result) {
logger.warn('Webhook signature verification failed', {
candidateCount: webhookCandidates.length,
})
if (!result || !verifiesAgainst(result.webhookSecret, rawBody, headersResult.data)) {
logger.warn('Webhook signature verification failed', { payloadInboxId: inboxId })
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
}
const envelopeResult = agentMailEnvelopeSchema.safeParse(JSON.parse(rawBody))
const envelopeResult = agentMailEnvelopeSchema.safeParse(parsedBody)
if (!envelopeResult.success) {
logger.warn('Invalid AgentMail webhook payload', {
workspaceId: result.id,
@@ -137,15 +167,6 @@ export const POST = withRouteHandler(async (req: Request) => {
}
const message: AgentMailWebhookPayload['message'] = messageResult.data
const inboxId = message.inbox_id
if (result.inboxProviderId !== inboxId) {
logger.warn('Verified AgentMail payload inbox mismatch', {
workspaceId: result.id,
verifiedInboxId: result.inboxProviderId,
payloadInboxId: inboxId,
})
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
}
if (!result.inboxEnabled) {
logger.info('Inbox disabled, rejecting', { workspaceId: result.id })
@@ -279,6 +300,8 @@ export const POST = withRouteHandler(async (req: Request) => {
error: getErrorMessage(error, 'Unknown error'),
})
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
} finally {
ticket.release()
}
})
+7
View File
@@ -86,6 +86,13 @@ export const agentMailEnvelopeSchema = z
})
.passthrough()
/** Unverified view of an AgentMail envelope, read only to pick which secret to check. */
export const agentMailRoutingSchema = z.object({
message: z.object({
inbox_id: z.string().min(1, 'message.inbox_id cannot be empty').max(320),
}),
})
const agentMailAttachmentSchema = z
.object({
attachment_id: z.string(),
@@ -32,6 +32,11 @@ export async function enableInbox(
let webhook: Awaited<ReturnType<typeof agentmail.createWebhook>> | null = null
try {
/**
* The receiver routes a delivery to this workspace by the `message.inbox_id`
* in the envelope, so it can only accept event types that carry a message.
* Adding one that does not would make those deliveries unroutable.
*/
webhook = await agentmail.createWebhook({
url: `${getBaseUrl()}/api/webhooks/agentmail`,
eventTypes: ['message.received'],
+8
View File
@@ -10,3 +10,11 @@ import { env } from '@/lib/core/config/env'
*/
export const WEBHOOK_MAX_BODY_BYTES =
Number.parseInt(env.WEBHOOK_MAX_REQUEST_BYTES, 10) || 10 * 1024 * 1024
/**
* Bounds the body an unauthenticated caller can make the AgentMail receiver
* buffer, parse and HMAC. AgentMail delivers attachments by reference, so an
* envelope is headers plus the text/HTML parts; 4 MB still fits rich HTML with
* inline `data:` images, which a tighter cap would reject as lost mail.
*/
export const AGENTMAIL_WEBHOOK_MAX_BODY_BYTES = Math.min(WEBHOOK_MAX_BODY_BYTES, 4 * 1024 * 1024)
@@ -0,0 +1,48 @@
-- The public AgentMail webhook receiver resolves the delivering tenant by
-- `workspace.inbox_provider_id` so it verifies the Svix signature against one
-- secret instead of every tenant's. That lookup runs on an unauthenticated path,
-- so it must be indexed, and unique so "one signature check per request" holds by
-- construction. The value is an inbox id minted per workspace by `enableInbox`
-- and nulled by `disableInbox`, so it is already unique in practice. NULLs are
-- excluded: most workspaces have no inbox.
--
-- migration-safe: additive index only, no column or constraint changes. The
-- uniqueness it adds is already an invariant of the only code that writes the
-- column, and that code is unchanged by this deploy, so old and new app versions
-- both run correctly against either schema state.
-- Duplicates must be resolved first. Failing here, inside the transaction, avoids
-- letting the CONCURRENT build fail afterwards and strand an INVALID index that
-- IF NOT EXISTS would skip forever.
DO $$
DECLARE duplicate_inbox_ids text;
BEGIN
SELECT string_agg(inbox_provider_id, ', ')
INTO duplicate_inbox_ids
FROM (
SELECT "inbox_provider_id" FROM "workspace"
WHERE "inbox_provider_id" IS NOT NULL
GROUP BY "inbox_provider_id" HAVING count(*) > 1
) AS d;
IF duplicate_inbox_ids IS NOT NULL THEN
RAISE EXCEPTION
'workspace has duplicate inbox_provider_id values: %. Each AgentMail inbox belongs to one workspace; clear the stale rows (disableInbox) and re-run.',
duplicate_inbox_ids;
END IF;
END $$;--> statement-breakpoint
COMMIT;--> statement-breakpoint
-- `lock_timeout = 0` for the concurrent build, per packages/db/scripts/migrate.ts.
-- CREATE INDEX CONCURRENTLY waits on every concurrent write in the database, not just
-- this table, so the session's 5s DDL timeout would cancel it (55P03) and strand an
-- INVALID index that the IF NOT EXISTS below would skip forever.
SET lock_timeout = 0;--> statement-breakpoint
-- Clear any INVALID index left by a previously cancelled build, so a replay
-- rebuilds it instead of skipping it.
DROP INDEX CONCURRENTLY IF EXISTS "workspace_inbox_provider_id_idx";--> statement-breakpoint
CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS "workspace_inbox_provider_id_idx" ON "workspace" USING btree ("inbox_provider_id") WHERE "workspace"."inbox_provider_id" IS NOT NULL;--> statement-breakpoint
SET lock_timeout = '5s';
File diff suppressed because it is too large Load Diff
@@ -1989,6 +1989,13 @@
"when": 1785986974579,
"tag": "0284_sso_provider_domain_verified",
"breakpoints": true
},
{
"idx": 285,
"version": "7",
"when": 1786217700133,
"tag": "0285_workspace_inbox_provider_id_idx",
"breakpoints": true
}
]
}
+9
View File
@@ -1624,6 +1624,15 @@ export const workspace = pgTable(
forkedFromWorkspaceIdx: index('workspace_forked_from_workspace_id_idx').on(
table.forkedFromWorkspaceId
),
/**
* Routes an unauthenticated AgentMail delivery to exactly one tenant's
* webhook secret. Unique so "one signature check per request" is a storage
* invariant rather than something the receiver has to defend against, and
* partial because only a small fraction of workspaces enable an inbox.
*/
inboxProviderIdIdx: uniqueIndex('workspace_inbox_provider_id_idx')
.on(table.inboxProviderId)
.where(sql`${table.inboxProviderId} IS NOT NULL`),
})
)