Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 94 additions & 2 deletions apps/sim/app/api/webhooks/trigger/[path]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,32 @@ vi.mock('@/lib/webhooks/processor', () => ({
}
}),
handleProviderChallenges: vi.fn().mockResolvedValue(null),
handlePreLookupWebhookVerification: vi
.fn()
.mockImplementation(
async (
method: string,
body: Record<string, unknown> | undefined,
_requestId: string,
path: string
) => {
if (path !== 'pending-verification-path') {
return null
}

const isVerificationProbe =
method === 'GET' ||
method === 'HEAD' ||
(method === 'POST' && (!body || Object.keys(body).length === 0 || !body.type))

if (!isVerificationProbe) {
return null
}

const { NextResponse } = require('next/server')
return NextResponse.json({ status: 'ok', message: 'Webhook endpoint verified' })
}
),
handleProviderReachabilityTest: vi.fn().mockReturnValue(null),
verifyProviderAuth: vi
.fn()
Expand Down Expand Up @@ -353,7 +379,7 @@ vi.mock('@/lib/core/utils/request', () => requestUtilsMock)

process.env.DATABASE_URL = 'postgresql://test:test@localhost:5432/test'

import { POST } from '@/app/api/webhooks/trigger/[path]/route'
import { GET, POST } from '@/app/api/webhooks/trigger/[path]/route'

describe('Webhook Trigger API Route', () => {
beforeEach(() => {
Expand Down Expand Up @@ -389,7 +415,7 @@ describe('Webhook Trigger API Route', () => {
})

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

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

Expand All @@ -401,6 +427,72 @@ describe('Webhook Trigger API Route', () => {
expect(text).toMatch(/not found/i)
})

it('should return 405 for GET requests on unknown webhook paths', async () => {
const req = createMockRequest(
'GET',
undefined,
{},
'http://localhost:3000/api/webhooks/trigger/non-existent-path'
)

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

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

expect(response.status).toBe(405)
})

it('should return 200 for GET verification probes on registered pending paths', async () => {
const req = createMockRequest(
'GET',
undefined,
{},
'http://localhost:3000/api/webhooks/trigger/pending-verification-path'
)

const params = Promise.resolve({ path: 'pending-verification-path' })

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

expect(response.status).toBe(200)
await expect(response.json()).resolves.toMatchObject({
status: 'ok',
message: 'Webhook endpoint verified',
})
})

it('should return 200 for empty POST verification probes on registered pending paths', async () => {
const req = createMockRequest(
'POST',
undefined,
{},
'http://localhost:3000/api/webhooks/trigger/pending-verification-path'
)

const params = Promise.resolve({ path: 'pending-verification-path' })

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

expect(response.status).toBe(200)
await expect(response.json()).resolves.toMatchObject({
status: 'ok',
message: 'Webhook endpoint verified',
})
})

it('should return 404 for POST requests without type on unknown webhook paths', async () => {
const req = createMockRequest('POST', { event: 'test' })

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

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

expect(response.status).toBe(404)

const text = await response.text()
expect(text).toMatch(/not found/i)
})

describe('Generic Webhook Authentication', () => {
it('should process generic webhook without authentication', async () => {
testData.webhooks.push({
Expand Down
16 changes: 15 additions & 1 deletion apps/sim/app/api/webhooks/trigger/[path]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
checkWebhookPreprocessing,
findAllWebhooksForPath,
handlePreDeploymentVerification,
handlePreLookupWebhookVerification,
handleProviderChallenges,
handleProviderReachabilityTest,
parseWebhookBody,
Expand All @@ -30,7 +31,10 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{
return challengeResponse
}

return new NextResponse('Method not allowed', { status: 405 })
return (
(await handlePreLookupWebhookVerification(request.method, undefined, requestId, path)) ||
new NextResponse('Method not allowed', { status: 405 })
)
}

export async function POST(
Expand Down Expand Up @@ -64,6 +68,16 @@ export async function POST(
const webhooksForPath = await findAllWebhooksForPath({ requestId, path })

if (webhooksForPath.length === 0) {
const verificationResponse = await handlePreLookupWebhookVerification(
request.method,
body,
requestId,
path
)
if (verificationResponse) {
return verificationResponse
}

logger.warn(`[${requestId}] Webhook or workflow not found for path: ${path}`)
return new NextResponse('Not Found', { status: 404 })
}
Expand Down
44 changes: 28 additions & 16 deletions apps/sim/blocks/blocks/grain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const GrainBlock: BlockConfig = {
{ label: 'List Recordings', id: 'grain_list_recordings' },
{ label: 'Get Recording', id: 'grain_get_recording' },
{ label: 'Get Transcript', id: 'grain_get_transcript' },
{ label: 'List Views', id: 'grain_list_views' },
{ label: 'List Teams', id: 'grain_list_teams' },
{ label: 'List Meeting Types', id: 'grain_list_meeting_types' },
{ label: 'Create Webhook', id: 'grain_create_hook' },
Expand Down Expand Up @@ -72,7 +73,7 @@ export const GrainBlock: BlockConfig = {
placeholder: 'ISO8601 timestamp (e.g., 2024-01-01T00:00:00Z)',
condition: {
field: 'operation',
value: ['grain_list_recordings', 'grain_create_hook'],
value: ['grain_list_recordings'],
},
wandConfig: {
enabled: true,
Expand All @@ -96,7 +97,7 @@ Return ONLY the timestamp string - no explanations, no quotes, no extra text.`,
placeholder: 'ISO8601 timestamp (e.g., 2024-01-01T00:00:00Z)',
condition: {
field: 'operation',
value: ['grain_list_recordings', 'grain_create_hook'],
value: ['grain_list_recordings'],
},
wandConfig: {
enabled: true,
Expand Down Expand Up @@ -125,7 +126,7 @@ Return ONLY the timestamp string - no explanations, no quotes, no extra text.`,
value: () => '',
condition: {
field: 'operation',
value: ['grain_list_recordings', 'grain_create_hook'],
value: ['grain_list_recordings'],
},
},
// Title search
Expand Down Expand Up @@ -162,7 +163,7 @@ Return ONLY the search term - no explanations, no quotes, no extra text.`,
placeholder: 'Filter by team UUID (optional)',
condition: {
field: 'operation',
value: ['grain_list_recordings', 'grain_create_hook'],
value: ['grain_list_recordings'],
},
},
// Meeting type ID filter
Expand All @@ -173,7 +174,7 @@ Return ONLY the search term - no explanations, no quotes, no extra text.`,
placeholder: 'Filter by meeting type UUID (optional)',
condition: {
field: 'operation',
value: ['grain_list_recordings', 'grain_create_hook'],
value: ['grain_list_recordings'],
},
},
// Include highlights
Expand All @@ -183,7 +184,7 @@ Return ONLY the search term - no explanations, no quotes, no extra text.`,
type: 'switch',
condition: {
field: 'operation',
value: ['grain_list_recordings', 'grain_get_recording', 'grain_create_hook'],
value: ['grain_list_recordings', 'grain_get_recording'],
},
},
// Include participants
Expand All @@ -193,7 +194,7 @@ Return ONLY the search term - no explanations, no quotes, no extra text.`,
type: 'switch',
condition: {
field: 'operation',
value: ['grain_list_recordings', 'grain_get_recording', 'grain_create_hook'],
value: ['grain_list_recordings', 'grain_get_recording'],
},
},
// Include AI summary
Expand All @@ -203,7 +204,18 @@ Return ONLY the search term - no explanations, no quotes, no extra text.`,
type: 'switch',
condition: {
field: 'operation',
value: ['grain_list_recordings', 'grain_get_recording', 'grain_create_hook'],
value: ['grain_list_recordings', 'grain_get_recording'],
},
},
{
id: 'viewId',
title: 'View ID',
type: 'short-input',
placeholder: 'Enter Grain view UUID',
required: true,
condition: {
field: 'operation',
value: ['grain_create_hook'],
},
},
// Include calendar event (get_recording only)
Expand Down Expand Up @@ -271,6 +283,7 @@ Return ONLY the search term - no explanations, no quotes, no extra text.`,
'grain_list_recordings',
'grain_get_recording',
'grain_get_transcript',
'grain_list_views',
'grain_list_teams',
'grain_list_meeting_types',
'grain_create_hook',
Expand Down Expand Up @@ -327,24 +340,21 @@ Return ONLY the search term - no explanations, no quotes, no extra text.`,

case 'grain_list_teams':
case 'grain_list_meeting_types':
case 'grain_list_views':
case 'grain_list_hooks':
return baseParams

case 'grain_create_hook':
if (!params.hookUrl?.trim()) {
throw new Error('Webhook URL is required.')
}
if (!params.viewId?.trim()) {
throw new Error('View ID is required.')
}
return {
...baseParams,
hookUrl: params.hookUrl.trim(),
filterBeforeDatetime: params.beforeDatetime || undefined,
filterAfterDatetime: params.afterDatetime || undefined,
filterParticipantScope: params.participantScope || undefined,
filterTeamId: params.teamId || undefined,
filterMeetingTypeId: params.meetingTypeId || undefined,
includeHighlights: params.includeHighlights || false,
includeParticipants: params.includeParticipants || false,
includeAiSummary: params.includeAiSummary || false,
viewId: params.viewId.trim(),
}

case 'grain_delete_hook':
Expand All @@ -367,6 +377,7 @@ Return ONLY the search term - no explanations, no quotes, no extra text.`,
apiKey: { type: 'string', description: 'Grain API key (Personal Access Token)' },
recordingId: { type: 'string', description: 'Recording UUID' },
cursor: { type: 'string', description: 'Pagination cursor' },
viewId: { type: 'string', description: 'Grain view UUID for webhook subscriptions' },
beforeDatetime: {
type: 'string',
description: 'Filter recordings before this ISO8601 timestamp',
Expand Down Expand Up @@ -416,6 +427,7 @@ Return ONLY the search term - no explanations, no quotes, no extra text.`,
teamsList: { type: 'json', description: 'Array of team objects' },
// Meeting type outputs
meetingTypes: { type: 'json', description: 'Array of meeting type objects' },
views: { type: 'json', description: 'Array of Grain views' },
// Hook outputs
hooks: { type: 'json', description: 'Array of webhook objects' },
hook: { type: 'json', description: 'Created webhook data' },
Expand Down
14 changes: 14 additions & 0 deletions apps/sim/lib/webhooks/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { and, eq, inArray } from 'drizzle-orm'
import { nanoid } from 'nanoid'
import type { NextRequest } from 'next/server'
import { getProviderIdFromServiceId } from '@/lib/oauth'
import { PendingWebhookVerificationTracker } from '@/lib/webhooks/pending-verification'
import {
cleanupExternalWebhook,
createExternalWebhookSubscription,
Expand Down Expand Up @@ -580,6 +581,7 @@ export async function saveTriggerWebhooksForDeploy({
updatedProviderConfig: Record<string, unknown>
externalSubscriptionCreated: boolean
}> = []
const pendingVerificationTracker = new PendingWebhookVerificationTracker()

for (const block of blocksNeedingWebhook) {
const config = webhookConfigs.get(block.id)
Expand All @@ -595,6 +597,14 @@ export async function saveTriggerWebhooksForDeploy({
}

try {
await pendingVerificationTracker.register({
path: triggerPath,
provider,
workflowId,
blockId: block.id,
metadata: providerConfig,
})

const result = await createExternalWebhookSubscription(
request,
createPayload,
Expand All @@ -613,6 +623,7 @@ export async function saveTriggerWebhooksForDeploy({
})
} catch (error: any) {
logger.error(`[${requestId}] Failed to create external subscription for ${block.id}`, error)
await pendingVerificationTracker.clearAll()
for (const sub of createdSubscriptions) {
if (sub.externalSubscriptionCreated) {
try {
Expand Down Expand Up @@ -666,6 +677,8 @@ export async function saveTriggerWebhooksForDeploy({
}
})

await pendingVerificationTracker.clearAll()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clearAll() called before configurePollingIfNeeded return paths

pendingVerificationTracker.clearAll() is called at line 680 (inside the try block, after the DB transaction but before the polling loop). If configurePollingIfNeeded triggers the early-return path (return { success: false, error: pollingError }), the function exits without a finally-guarded cleanup — but since clearAll() already ran at this point, the pending entries are correctly cleared.

The concern is the catch branch lower in the function also calls clearAll() as a separate statement. If a new early-return path is ever introduced between line 680 and the end of the try block (e.g., a new pollingError cleanup that throws instead of returning), the double-call is harmless but the absence of a finally block means the intent isn't self-documenting. Consider wrapping the tracker cleanup in a finally block for robustness:

try {
  await db.transaction(...)
  for (const sub of createdSubscriptions) { ... }
} catch (error: any) {
  // cleanup
} finally {
  await pendingVerificationTracker.clearAll()
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


for (const sub of createdSubscriptions) {
const pollingError = await configurePollingIfNeeded(
sub.provider,
Expand Down Expand Up @@ -710,6 +723,7 @@ export async function saveTriggerWebhooksForDeploy({
}
}
} catch (error: any) {
await pendingVerificationTracker.clearAll()
logger.error(`[${requestId}] Failed to insert webhook records`, error)
for (const sub of createdSubscriptions) {
if (sub.externalSubscriptionCreated) {
Expand Down
Loading
Loading