Skip to content

Commit ab13e20

Browse files
committed
fix(webhooks): stop provider challenges from intercepting other providers' deliveries
The challenge handlers run before webhook lookup and are provider-blind, so two query parameter names are effectively reserved across every path. Now that a generic webhook can be triggered by a URL fetch, a link carrying either name answers the challenge instead of running the workflow: - `?validationToken=x` is echoed back as a Microsoft Graph subscription validation. Graph sends that validation as a POST, so ignore the parameter on every other method. - `hub.mode`, `hub.verify_token` and `hub.challenge` answer 403 when no WhatsApp webhook on the path expects a token. A path with no such webhook is not a failed verification - the parameters belong to whoever owns that path - so fall through and let the delivery route normally. A token mismatch against a WhatsApp webhook still fails with 403. Refs #6888 Signed-off-by: mini.jeong <mini.jeong@navercorp.com>
1 parent 6914b8e commit ab13e20

4 files changed

Lines changed: 94 additions & 1 deletion

File tree

apps/sim/lib/webhooks/providers/microsoft-teams.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,4 +219,39 @@ describe('microsoftTeamsHandler formatInput (outgoing webhook channelData)', ()
219219
teamsChannelId: 'channel-1',
220220
})
221221
})
222+
223+
describe('handleChallenge', () => {
224+
function challengeRequest(method: string): NextRequest {
225+
return new NextRequest(
226+
'https://app.example.com/api/webhooks/trigger/abc?validationToken=token-123',
227+
{ method }
228+
)
229+
}
230+
231+
it('echoes the validation token for the POST Microsoft Graph sends', async () => {
232+
const response = microsoftTeamsHandler.handleChallenge!(
233+
{},
234+
challengeRequest('POST'),
235+
'teams-challenge-post',
236+
'abc'
237+
)
238+
239+
expect(response?.status).toBe(200)
240+
await expect(response?.text()).resolves.toBe('token-123')
241+
})
242+
243+
it.each(['GET', 'PUT', 'PATCH', 'DELETE'])(
244+
'ignores a validationToken query parameter on a %s delivery',
245+
(method) => {
246+
expect(
247+
microsoftTeamsHandler.handleChallenge!(
248+
{},
249+
challengeRequest(method),
250+
'teams-challenge-other-method',
251+
'abc'
252+
)
253+
).toBeNull()
254+
}
255+
)
256+
})
222257
})

apps/sim/lib/webhooks/providers/microsoft-teams.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,15 @@ async function formatTeamsGraphNotification(
479479

480480
export const microsoftTeamsHandler: WebhookProviderHandler = {
481481
handleChallenge(_body: unknown, request: NextRequest, requestId: string, path: string) {
482+
/**
483+
* Microsoft Graph sends the subscription validation as a POST. Answering it for any method
484+
* would let a `validationToken` query parameter on a GET, PUT, PATCH or DELETE delivery to
485+
* another provider's path be echoed back instead of triggering that workflow.
486+
*/
487+
if (request.method !== 'POST') {
488+
return null
489+
}
490+
482491
const url = new URL(request.url)
483492
const validationToken = url.searchParams.get('validationToken')
484493
if (validationToken) {

apps/sim/lib/webhooks/providers/whatsapp.test.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { createHmac } from 'node:crypto'
5-
import { dbChainMock, schemaMock } from '@sim/testing'
5+
import { dbChainMock, queueTableRows, schemaMock } from '@sim/testing'
66
import { NextRequest } from 'next/server'
77
import { describe, expect, it, vi } from 'vitest'
88

@@ -265,6 +265,42 @@ describe('WhatsApp webhook provider', () => {
265265
expect(input.caption).toBeUndefined()
266266
})
267267

268+
describe('handleChallenge', () => {
269+
function verificationRequest(): NextRequest {
270+
return new NextRequest(
271+
'http://localhost/api/webhooks/trigger/abc?hub.mode=subscribe&hub.verify_token=t&hub.challenge=c'
272+
)
273+
}
274+
275+
it('falls through when no WhatsApp webhook on the path expects a token', async () => {
276+
queueTableRows(schemaMock.webhook, [])
277+
278+
const response = await whatsappHandler.handleChallenge!(
279+
{},
280+
verificationRequest(),
281+
'wa-challenge-no-webhook',
282+
'abc'
283+
)
284+
285+
expect(response).toBeNull()
286+
})
287+
288+
it('still fails verification when a WhatsApp webhook expects a different token', async () => {
289+
queueTableRows(schemaMock.webhook, [
290+
{ webhook: { id: 'wh_1', providerConfig: { verificationToken: 'other' } } },
291+
])
292+
293+
const response = await whatsappHandler.handleChallenge!(
294+
{},
295+
verificationRequest(),
296+
'wa-challenge-token-mismatch',
297+
'abc'
298+
)
299+
300+
expect(response?.status).toBe(403)
301+
})
302+
})
303+
268304
it('ignores a media type whose payload object is missing', async () => {
269305
const input = await formatMediaMessage({
270306
id: 'wamid.image.2',

apps/sim/lib/webhooks/providers/whatsapp.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,8 @@ async function handleWhatsAppVerification(
202202
)
203203
)
204204

205+
let candidates = 0
206+
205207
for (const row of webhooks) {
206208
const wh = row.webhook
207209
const providerConfig = (wh.providerConfig as Record<string, unknown>) || {}
@@ -211,6 +213,8 @@ async function handleWhatsAppVerification(
211213
continue
212214
}
213215

216+
candidates++
217+
214218
if (safeCompare(token, verificationToken as string)) {
215219
logger.info(`[${requestId}] WhatsApp verification successful for webhook ${wh.id}`)
216220
return new NextResponse(challenge, {
@@ -222,6 +226,15 @@ async function handleWhatsAppVerification(
222226
}
223227
}
224228

229+
/**
230+
* A path with no WhatsApp webhook expecting a token is not a failed verification: the
231+
* `hub.*` parameters belong to whoever owns that path. Fall through so the delivery is
232+
* routed normally instead of answering 403 for someone else's query parameters.
233+
*/
234+
if (candidates === 0) {
235+
return null
236+
}
237+
225238
logger.warn(`[${requestId}] No matching WhatsApp verification token found`)
226239
return new NextResponse('Verification failed', { status: 403 })
227240
}

0 commit comments

Comments
 (0)