Skip to content

Commit 90b7193

Browse files
committed
update pending webhook verification infra
1 parent a30f8c2 commit 90b7193

File tree

6 files changed

+382
-24
lines changed

6 files changed

+382
-24
lines changed

apps/sim/app/api/webhooks/trigger/[path]/route.test.ts

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,32 @@ vi.mock('@/lib/webhooks/processor', () => ({
268268
}
269269
}),
270270
handleProviderChallenges: vi.fn().mockResolvedValue(null),
271+
handlePreLookupWebhookVerification: vi
272+
.fn()
273+
.mockImplementation(
274+
async (
275+
method: string,
276+
body: Record<string, unknown> | undefined,
277+
_requestId: string,
278+
path: string
279+
) => {
280+
if (path !== 'pending-verification-path') {
281+
return null
282+
}
283+
284+
const isVerificationProbe =
285+
method === 'GET' ||
286+
method === 'HEAD' ||
287+
(method === 'POST' && (!body || Object.keys(body).length === 0 || !body.type))
288+
289+
if (!isVerificationProbe) {
290+
return null
291+
}
292+
293+
const { NextResponse } = require('next/server')
294+
return NextResponse.json({ status: 'ok', message: 'Webhook endpoint verified' })
295+
}
296+
),
271297
handleProviderReachabilityTest: vi.fn().mockReturnValue(null),
272298
verifyProviderAuth: vi
273299
.fn()
@@ -389,7 +415,7 @@ describe('Webhook Trigger API Route', () => {
389415
})
390416

391417
it('should handle 404 for non-existent webhooks', async () => {
392-
const req = createMockRequest('POST', { event: 'test' })
418+
const req = createMockRequest('POST', { type: 'event.test' })
393419

394420
const params = Promise.resolve({ path: 'non-existent-path' })
395421

@@ -401,7 +427,7 @@ describe('Webhook Trigger API Route', () => {
401427
expect(text).toMatch(/not found/i)
402428
})
403429

404-
it('should return 200 for GET verification probes before webhook persistence', async () => {
430+
it('should return 405 for GET requests on unknown webhook paths', async () => {
405431
const req = createMockRequest(
406432
'GET',
407433
undefined,
@@ -413,22 +439,37 @@ describe('Webhook Trigger API Route', () => {
413439

414440
const response = await GET(req as any, { params })
415441

442+
expect(response.status).toBe(405)
443+
})
444+
445+
it('should return 200 for GET verification probes on registered pending paths', async () => {
446+
const req = createMockRequest(
447+
'GET',
448+
undefined,
449+
{},
450+
'http://localhost:3000/api/webhooks/trigger/pending-verification-path'
451+
)
452+
453+
const params = Promise.resolve({ path: 'pending-verification-path' })
454+
455+
const response = await GET(req as any, { params })
456+
416457
expect(response.status).toBe(200)
417458
await expect(response.json()).resolves.toMatchObject({
418459
status: 'ok',
419460
message: 'Webhook endpoint verified',
420461
})
421462
})
422463

423-
it('should return 200 for empty POST verification probes before webhook persistence', async () => {
464+
it('should return 200 for empty POST verification probes on registered pending paths', async () => {
424465
const req = createMockRequest(
425466
'POST',
426467
undefined,
427468
{},
428-
'http://localhost:3000/api/webhooks/trigger/non-existent-path'
469+
'http://localhost:3000/api/webhooks/trigger/pending-verification-path'
429470
)
430471

431-
const params = Promise.resolve({ path: 'non-existent-path' })
472+
const params = Promise.resolve({ path: 'pending-verification-path' })
432473

433474
const response = await POST(req as any, { params })
434475

@@ -439,6 +480,19 @@ describe('Webhook Trigger API Route', () => {
439480
})
440481
})
441482

483+
it('should return 404 for POST requests without type on unknown webhook paths', async () => {
484+
const req = createMockRequest('POST', { event: 'test' })
485+
486+
const params = Promise.resolve({ path: 'non-existent-path' })
487+
488+
const response = await POST(req as any, { params })
489+
490+
expect(response.status).toBe(404)
491+
492+
const text = await response.text()
493+
expect(text).toMatch(/not found/i)
494+
})
495+
442496
describe('Generic Webhook Authentication', () => {
443497
it('should process generic webhook without authentication', async () => {
444498
testData.webhooks.push({

apps/sim/app/api/webhooks/trigger/[path]/route.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,10 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{
3131
return challengeResponse
3232
}
3333

34-
return handlePreLookupWebhookVerification(
35-
request.method,
36-
undefined,
37-
requestId,
38-
path
39-
) as NextResponse
34+
return (
35+
(await handlePreLookupWebhookVerification(request.method, undefined, requestId, path)) ||
36+
new NextResponse('Method not allowed', { status: 405 })
37+
)
4038
}
4139

4240
export async function POST(
@@ -70,7 +68,7 @@ export async function POST(
7068
const webhooksForPath = await findAllWebhooksForPath({ requestId, path })
7169

7270
if (webhooksForPath.length === 0) {
73-
const verificationResponse = handlePreLookupWebhookVerification(
71+
const verificationResponse = await handlePreLookupWebhookVerification(
7472
request.method,
7573
body,
7674
requestId,

apps/sim/lib/webhooks/deploy.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { and, eq, inArray } from 'drizzle-orm'
55
import { nanoid } from 'nanoid'
66
import type { NextRequest } from 'next/server'
77
import { getProviderIdFromServiceId } from '@/lib/oauth'
8+
import { PendingWebhookVerificationTracker } from '@/lib/webhooks/pending-verification'
89
import {
910
cleanupExternalWebhook,
1011
createExternalWebhookSubscription,
@@ -580,6 +581,7 @@ export async function saveTriggerWebhooksForDeploy({
580581
updatedProviderConfig: Record<string, unknown>
581582
externalSubscriptionCreated: boolean
582583
}> = []
584+
const pendingVerificationTracker = new PendingWebhookVerificationTracker()
583585

584586
for (const block of blocksNeedingWebhook) {
585587
const config = webhookConfigs.get(block.id)
@@ -595,6 +597,13 @@ export async function saveTriggerWebhooksForDeploy({
595597
}
596598

597599
try {
600+
await pendingVerificationTracker.register({
601+
path: triggerPath,
602+
provider,
603+
workflowId,
604+
blockId: block.id,
605+
})
606+
598607
const result = await createExternalWebhookSubscription(
599608
request,
600609
createPayload,
@@ -613,6 +622,7 @@ export async function saveTriggerWebhooksForDeploy({
613622
})
614623
} catch (error: any) {
615624
logger.error(`[${requestId}] Failed to create external subscription for ${block.id}`, error)
625+
await pendingVerificationTracker.clearAll()
616626
for (const sub of createdSubscriptions) {
617627
if (sub.externalSubscriptionCreated) {
618628
try {
@@ -666,6 +676,8 @@ export async function saveTriggerWebhooksForDeploy({
666676
}
667677
})
668678

679+
await pendingVerificationTracker.clearAll()
680+
669681
for (const sub of createdSubscriptions) {
670682
const pollingError = await configurePollingIfNeeded(
671683
sub.provider,
@@ -710,6 +722,7 @@ export async function saveTriggerWebhooksForDeploy({
710722
}
711723
}
712724
} catch (error: any) {
725+
await pendingVerificationTracker.clearAll()
713726
logger.error(`[${requestId}] Failed to insert webhook records`, error)
714727
for (const sub of createdSubscriptions) {
715728
if (sub.externalSubscriptionCreated) {
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { afterEach, describe, expect, it, vi } from 'vitest'
5+
6+
vi.mock('@/lib/core/config/redis', () => ({
7+
getRedisClient: vi.fn().mockReturnValue(null),
8+
}))
9+
10+
vi.mock('@sim/logger', () => ({
11+
createLogger: vi.fn().mockReturnValue({
12+
info: vi.fn(),
13+
warn: vi.fn(),
14+
error: vi.fn(),
15+
}),
16+
}))
17+
18+
import {
19+
clearPendingWebhookVerification,
20+
getPendingWebhookVerification,
21+
matchesPendingWebhookVerificationProbe,
22+
PendingWebhookVerificationTracker,
23+
registerPendingWebhookVerification,
24+
} from '@/lib/webhooks/pending-verification'
25+
26+
describe('pending webhook verification', () => {
27+
afterEach(async () => {
28+
await clearPendingWebhookVerification('grain-path-1')
29+
await clearPendingWebhookVerification('grain-path-2')
30+
await clearPendingWebhookVerification('grain-path-3')
31+
await clearPendingWebhookVerification('grain-path-4')
32+
})
33+
34+
it('stores and retrieves pending Grain verification entries', async () => {
35+
await registerPendingWebhookVerification({
36+
path: 'grain-path-1',
37+
provider: 'grain',
38+
workflowId: 'workflow-1',
39+
blockId: 'block-1',
40+
})
41+
42+
const entry = await getPendingWebhookVerification('grain-path-1')
43+
44+
expect(entry).toMatchObject({
45+
path: 'grain-path-1',
46+
provider: 'grain',
47+
workflowId: 'workflow-1',
48+
blockId: 'block-1',
49+
})
50+
})
51+
52+
it('matches Grain verification probe shapes only for registered paths', async () => {
53+
await registerPendingWebhookVerification({
54+
path: 'grain-path-2',
55+
provider: 'grain',
56+
})
57+
58+
const entry = await getPendingWebhookVerification('grain-path-2')
59+
60+
expect(entry).not.toBeNull()
61+
expect(
62+
matchesPendingWebhookVerificationProbe(entry!, {
63+
method: 'POST',
64+
body: {},
65+
})
66+
).toBe(true)
67+
expect(
68+
matchesPendingWebhookVerificationProbe(entry!, {
69+
method: 'POST',
70+
body: { type: 'recording_added' },
71+
})
72+
).toBe(false)
73+
})
74+
75+
it('clears tracked pending verifications after a successful lifecycle', async () => {
76+
const tracker = new PendingWebhookVerificationTracker()
77+
78+
await tracker.register({
79+
path: 'grain-path-3',
80+
provider: 'grain',
81+
})
82+
83+
expect(await getPendingWebhookVerification('grain-path-3')).not.toBeNull()
84+
85+
await tracker.clearAll()
86+
87+
expect(await getPendingWebhookVerification('grain-path-3')).toBeNull()
88+
})
89+
90+
it('clears tracked pending verifications after a failed lifecycle', async () => {
91+
const tracker = new PendingWebhookVerificationTracker()
92+
93+
await tracker.register({
94+
path: 'grain-path-4',
95+
provider: 'grain',
96+
})
97+
98+
expect(await getPendingWebhookVerification('grain-path-4')).not.toBeNull()
99+
100+
await tracker.clear('grain-path-4')
101+
102+
expect(await getPendingWebhookVerification('grain-path-4')).toBeNull()
103+
})
104+
})

0 commit comments

Comments
 (0)