diff --git a/apps/sim/app/api/webhooks/trigger/[path]/route.test.ts b/apps/sim/app/api/webhooks/trigger/[path]/route.test.ts index f54e18337fd..5634cdddaf3 100644 --- a/apps/sim/app/api/webhooks/trigger/[path]/route.test.ts +++ b/apps/sim/app/api/webhooks/trigger/[path]/route.test.ts @@ -462,7 +462,7 @@ vi.mock('postgres', () => vi.fn().mockReturnValue({})) process.env.DATABASE_URL = 'postgresql://test:test@localhost:5432/test' -import { GET, POST } from '@/app/api/webhooks/trigger/[path]/route' +import { DELETE, GET, PATCH, POST, PUT } from '@/app/api/webhooks/trigger/[path]/route' describe('Webhook Trigger API Route', () => { beforeEach(() => { @@ -683,6 +683,123 @@ describe('Webhook Trigger API Route', () => { }) }) + describe('GET deliveries', () => { + it('dispatches a GET delivery to a generic webhook', async () => { + testData.webhooks.push({ + id: 'generic-webhook-id', + provider: 'generic', + path: 'get-path', + isActive: true, + providerConfig: { requireAuth: false }, + workflowId: 'test-workflow-id', + }) + + const req = createMockRequest( + 'GET', + undefined, + {}, + 'http://localhost:3000/api/webhooks/trigger/get-path?srcId=123' + ) + + const response = await GET(req, { params: Promise.resolve({ path: 'get-path' }) }) + + expect(response.status).toBe(200) + expect(dispatchResolvedWebhookTargetMock).toHaveBeenCalledOnce() + }) + + it('rejects a GET delivery to a provider that only accepts POST', async () => { + testData.webhooks.push({ + id: 'stripe-webhook-id', + provider: 'stripe', + path: 'post-only-path', + isActive: true, + providerConfig: {}, + workflowId: 'test-workflow-id', + }) + + const req = createMockRequest( + 'GET', + undefined, + {}, + 'http://localhost:3000/api/webhooks/trigger/post-only-path' + ) + + const response = await GET(req, { params: Promise.resolve({ path: 'post-only-path' }) }) + + expect(response.status).toBe(405) + expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled() + }) + }) + + describe('PUT, PATCH and DELETE deliveries', () => { + const handlers = { PUT, PATCH, DELETE } + + it.each(Object.keys(handlers) as Array)( + 'dispatches a %s delivery to a generic webhook', + async (method) => { + testData.webhooks.push({ + id: 'generic-webhook-id', + provider: 'generic', + path: 'any-method-path', + isActive: true, + providerConfig: { requireAuth: false }, + workflowId: 'test-workflow-id', + }) + + const req = createMockRequest( + method, + { event: 'test' }, + {}, + 'http://localhost:3000/api/webhooks/trigger/any-method-path?srcId=123' + ) + + const response = await handlers[method](req, { + params: Promise.resolve({ path: 'any-method-path' }), + }) + + expect(response.status).toBe(200) + expect(dispatchResolvedWebhookTargetMock).toHaveBeenCalledOnce() + } + ) + + it('rejects a PUT delivery to a provider that only accepts POST', async () => { + testData.webhooks.push({ + id: 'stripe-webhook-id', + provider: 'stripe', + path: 'post-only-path', + isActive: true, + providerConfig: {}, + workflowId: 'test-workflow-id', + }) + + const req = createMockRequest( + 'PUT', + { event: 'test' }, + {}, + 'http://localhost:3000/api/webhooks/trigger/post-only-path' + ) + + const response = await PUT(req, { params: Promise.resolve({ path: 'post-only-path' }) }) + + expect(response.status).toBe(405) + expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled() + }) + + it('returns 405 for a DELETE to an unknown path', async () => { + const req = createMockRequest( + 'DELETE', + undefined, + {}, + 'http://localhost:3000/api/webhooks/trigger/unknown-path' + ) + + const response = await DELETE(req, { params: Promise.resolve({ path: 'unknown-path' }) }) + + expect(response.status).toBe(405) + expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled() + }) + }) + describe('Reservation-free filtering', () => { it('skips filtered webhook events before preprocessing reserves a slot', async () => { testData.webhooks.push({ diff --git a/apps/sim/app/api/webhooks/trigger/[path]/route.ts b/apps/sim/app/api/webhooks/trigger/[path]/route.ts index 31b37a6edbe..9d3e272e9d2 100644 --- a/apps/sim/app/api/webhooks/trigger/[path]/route.ts +++ b/apps/sim/app/api/webhooks/trigger/[path]/route.ts @@ -14,7 +14,7 @@ import { parseWebhookBody, verifyProviderAuth, } from '@/lib/webhooks/processor' -import { acceptsPathWebhookDelivery } from '@/lib/webhooks/providers' +import { acceptsPathWebhookDelivery, acceptsWebhookDeliveryMethod } from '@/lib/webhooks/providers' const logger = createLogger('WebhookTriggerAPI') @@ -35,14 +35,30 @@ export const GET = withRouteHandler( return challengeResponse } - return ( - (await handlePreLookupWebhookVerification(request.method, undefined, requestId, path)) || - new NextResponse('Method not allowed', { status: 405 }) + const verificationResponse = await handlePreLookupWebhookVerification( + request.method, + undefined, + requestId, + path ) + if (verificationResponse) { + return verificationResponse + } + + const ticket = tryAdmit() + if (!ticket) { + return admissionRejectedResponse() + } + + try { + return await handleWebhookDelivery(request, context, webhookTriggerGetContract) + } finally { + ticket.release() + } } ) -export const POST = withRouteHandler( +const handleBodyDelivery = withRouteHandler( async (request: NextRequest, context: { params: Promise<{ path: string }> }) => { const ticket = tryAdmit() if (!ticket) { @@ -50,16 +66,27 @@ export const POST = withRouteHandler( } try { - return await handleWebhookPost(request, context) + return await handleWebhookDelivery(request, context, webhookTriggerPostContract) } finally { ticket.release() } } ) -async function handleWebhookPost( +export const POST = handleBodyDelivery + +/** + * Methods a provider must opt into via `extraDeliveryMethods`. A delivery to a path whose + * triggers have not opted in gets a 405 from `handleWebhookDelivery`. + */ +export const PUT = handleBodyDelivery +export const PATCH = handleBodyDelivery +export const DELETE = handleBodyDelivery + +async function handleWebhookDelivery( request: NextRequest, - context: { params: Promise<{ path: string }> } + context: { params: Promise<{ path: string }> }, + contract: typeof webhookTriggerGetContract | typeof webhookTriggerPostContract ): Promise { const receivedAt = Date.now() /** @@ -73,7 +100,7 @@ async function handleWebhookPost( : undefined const requestId = generateRequestId() - const parsed = await parseRequest(webhookTriggerPostContract, request, context) + const parsed = await parseRequest(contract, request, context) if (!parsed.success) return parsed.response const { path } = parsed.data.params @@ -99,15 +126,26 @@ async function handleWebhookPost( // Find all webhooks for this path (multiple webhooks in one workflow may share a path) const allWebhooksForPath = await findAllWebhooksForPath({ requestId, path }) - const webhooksForPath = allWebhooksForPath.filter(({ webhook: foundWebhook }) => + const pathWebhooks = allWebhooksForPath.filter(({ webhook: foundWebhook }) => acceptsPathWebhookDelivery(foundWebhook.provider) ) - if (allWebhooksForPath.length > 0 && webhooksForPath.length === 0) { + if (allWebhooksForPath.length > 0 && pathWebhooks.length === 0) { logger.warn(`[${requestId}] Rejected HTTP delivery to non-path trigger: ${path}`) return new NextResponse('Not Found', { status: 404 }) } + const webhooksForPath = pathWebhooks.filter(({ webhook: foundWebhook }) => + acceptsWebhookDeliveryMethod(foundWebhook.provider, request.method) + ) + + if (pathWebhooks.length > 0 && webhooksForPath.length === 0) { + logger.warn( + `[${requestId}] Rejected ${request.method} delivery to path ${path}: no trigger on this path accepts that method` + ) + return new NextResponse('Method not allowed', { status: 405 }) + } + if (webhooksForPath.length === 0) { const verificationResponse = await handlePreLookupWebhookVerification( request.method, @@ -120,7 +158,11 @@ async function handleWebhookPost( } logger.warn(`[${requestId}] Webhook or workflow not found for path: ${path}`) - return new NextResponse('Not Found', { status: 404 }) + // Unknown paths keep answering 405 on GET so probes cannot tell an unknown path + // from one whose trigger only accepts POST. + return request.method === 'POST' + ? new NextResponse('Not Found', { status: 404 }) + : new NextResponse('Method not allowed', { status: 405 }) } // Process each webhook matched on this path diff --git a/apps/sim/background/webhook-execution.ts b/apps/sim/background/webhook-execution.ts index e27d6edf3fd..5e6bb7d6617 100644 --- a/apps/sim/background/webhook-execution.ts +++ b/apps/sim/background/webhook-execution.ts @@ -269,6 +269,10 @@ export type WebhookExecutionPayload = { provider: string body: unknown headers: Record + /** Request URL query parameters; absent when the request had none or on legacy queued jobs. */ + query?: Record + /** HTTP method the delivery arrived with; absent on legacy queued jobs. */ + method?: string path: string blockId?: string /** Immutable deployment admitted by webhook ingress; absent on legacy queued jobs. */ @@ -622,6 +626,8 @@ async function executeWebhookJobInternal( workflow: { id: payload.workflowId, userId: payload.userId }, body: payload.body, headers: payload.headers, + query: payload.query ?? {}, + method: payload.method ?? '', requestId, }) input = result.input as Record | null diff --git a/apps/sim/lib/webhooks/processor.test.ts b/apps/sim/lib/webhooks/processor.test.ts index 3904473599b..f13125af10f 100644 --- a/apps/sim/lib/webhooks/processor.test.ts +++ b/apps/sim/lib/webhooks/processor.test.ts @@ -477,6 +477,44 @@ describe('webhook processor execution identity', () => { expect(mockReleaseExecutionSlot).not.toHaveBeenCalled() }) + it('carries request query parameters into the queued payload', async () => { + await dispatchResolvedWebhookTarget( + makeWebhookRecord({ path: 'incoming/hook', provider: 'generic' }), + makeWorkflowRecord({}), + {}, + createMockRequest( + 'GET', + undefined, + {}, + 'http://localhost:3000/api/webhooks/trigger/incoming/hook?srcId=123&title=Hello%20World' + ) as NextRequest, + { requestId: 'request-1', path: 'incoming/hook' } + ) + + expect(mockEnqueue).toHaveBeenCalledWith( + 'webhook-execution', + expect.objectContaining({ query: { srcId: '123', title: 'Hello World' } }), + expect.anything() + ) + }) + + it('omits query from the queued payload when the request has none', async () => { + await dispatchResolvedWebhookTarget( + makeWebhookRecord({ path: 'incoming/hook', provider: 'generic' }), + makeWorkflowRecord({}), + { event: 'test' }, + createMockRequest( + 'POST', + { event: 'test' }, + {}, + 'http://localhost:3000/api/webhooks/trigger/incoming/hook' + ) as NextRequest, + { requestId: 'request-1', path: 'incoming/hook' } + ) + + expect(mockEnqueue.mock.calls[0]?.[1]).not.toHaveProperty('query') + }) + it('runs database-inline webhook jobs through the queue cancellation signal', async () => { mockShouldExecuteInline.mockReturnValue(true) const result = await dispatchResolvedWebhookTarget( diff --git a/apps/sim/lib/webhooks/processor.ts b/apps/sim/lib/webhooks/processor.ts index 06933871349..d11e9a03e85 100644 --- a/apps/sim/lib/webhooks/processor.ts +++ b/apps/sim/lib/webhooks/processor.ts @@ -681,6 +681,7 @@ async function queueWebhookExecutionWithResult( } const credentialId = getCredentialId(providerConfig) + const query = Object.fromEntries(new URL(request.url).searchParams) const actorUserId = options.actorUserId const billingAttribution = options.billingAttribution @@ -722,6 +723,8 @@ async function queueWebhookExecutionWithResult( provider: foundWebhook.provider, body, headers, + method: request.method, + ...(Object.keys(query).length > 0 ? { query } : {}), path: options.path || foundWebhook.path || '', blockId: foundWebhook.blockId ?? undefined, ...(foundWebhook.deploymentVersionId diff --git a/apps/sim/lib/webhooks/providers/generic.test.ts b/apps/sim/lib/webhooks/providers/generic.test.ts new file mode 100644 index 00000000000..1a6b25da6a4 --- /dev/null +++ b/apps/sim/lib/webhooks/providers/generic.test.ts @@ -0,0 +1,151 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { genericHandler } from '@/lib/webhooks/providers/generic' +import type { FormatInputContext } from '@/lib/webhooks/providers/types' + +function context( + body: unknown, + query: Record, + options: { headers?: Record; secretHeaderName?: string; method?: string } = {} +): FormatInputContext { + return { + webhook: { + id: 'webhook-id', + provider: 'generic', + providerConfig: options.secretHeaderName + ? { secretHeaderName: options.secretHeaderName } + : {}, + }, + workflow: { id: 'workflow-id', userId: 'user-id' }, + body, + headers: options.headers ?? {}, + query, + method: options.method ?? '', + requestId: 'req-1', + } +} + +describe('genericHandler.formatInput', () => { + it('exposes query parameters under "query" alongside body fields', async () => { + const result = await genericHandler.formatInput?.( + context({ event: 'test' }, { srcId: '123', title: 'Hello' }) + ) + + expect(result?.input).toEqual({ + event: 'test', + query: { srcId: '123', title: 'Hello' }, + }) + }) + + it('exposes query parameters when the request has no body', async () => { + const result = await genericHandler.formatInput?.(context({}, { srcId: '123' })) + + expect(result?.input).toEqual({ query: { srcId: '123' } }) + }) + + it('passes the body through unchanged when there are no query parameters', async () => { + const body = { event: 'test' } + const result = await genericHandler.formatInput?.(context(body, {})) + + expect(result?.input).toEqual(body) + expect(result?.input).not.toHaveProperty('query') + }) + + it('keeps a body field named "query" instead of overwriting it', async () => { + const body = { query: 'user typed this' } + const result = await genericHandler.formatInput?.(context(body, { srcId: '123' })) + + expect(result?.input).toEqual(body) + }) + + it('leaves non-object bodies untouched', async () => { + const body = [{ event: 'a' }] + const result = await genericHandler.formatInput?.(context(body, { srcId: '123' })) + + expect(result?.input).toEqual(body) + }) + + it('exposes request headers under "headers" with lowercased names', async () => { + const result = await genericHandler.formatInput?.( + context({ event: 'test' }, {}, { headers: { 'X-Event-Name': 'created' } }) + ) + + expect(result?.input).toEqual({ + event: 'test', + headers: { 'x-event-name': 'created' }, + }) + }) + + it('withholds headers that carry credentials', async () => { + const result = await genericHandler.formatInput?.( + context( + {}, + {}, + { + headers: { + authorization: 'Bearer secret', + cookie: 'session=secret', + 'x-api-key': 'secret', + 'x-sim-idempotency-key': 'abc', + 'x-event-name': 'created', + }, + } + ) + ) + + expect(result?.input).toEqual({ headers: { 'x-event-name': 'created' } }) + }) + + it("withholds the webhook's own configured secret header", async () => { + const result = await genericHandler.formatInput?.( + context( + {}, + {}, + { + headers: { 'X-Secret-Key': 'secret', 'x-event-name': 'created' }, + secretHeaderName: 'X-Secret-Key', + } + ) + ) + + expect(result?.input).toEqual({ headers: { 'x-event-name': 'created' } }) + }) + + it('keeps a body field named "headers" instead of overwriting it', async () => { + const body = { headers: 'user typed this' } + const result = await genericHandler.formatInput?.( + context(body, {}, { headers: { 'x-event-name': 'created' } }) + ) + + expect(result?.input).toEqual(body) + }) +}) + +describe('genericHandler delivery methods', () => { + it('opts into GET, PUT, PATCH and DELETE deliveries', () => { + expect(genericHandler.extraDeliveryMethods).toEqual(['GET', 'PUT', 'PATCH', 'DELETE']) + }) + + it('exposes the request method under "method"', async () => { + const result = await genericHandler.formatInput?.( + context({ event: 'test' }, {}, { method: 'DELETE' }) + ) + + expect(result?.input).toEqual({ event: 'test', method: 'DELETE' }) + }) + + it('omits "method" for legacy queued jobs that carry none', async () => { + const result = await genericHandler.formatInput?.(context({ event: 'test' }, {})) + + expect(result?.input).not.toHaveProperty('method') + }) + + it('keeps a body field named "method" instead of overwriting it', async () => { + const body = { method: 'user typed this' } + const result = await genericHandler.formatInput?.(context(body, {}, { method: 'PUT' })) + + expect(result?.input).toEqual(body) + }) +}) diff --git a/apps/sim/lib/webhooks/providers/generic.ts b/apps/sim/lib/webhooks/providers/generic.ts index 71372bebad6..487b8478c7e 100644 --- a/apps/sim/lib/webhooks/providers/generic.ts +++ b/apps/sim/lib/webhooks/providers/generic.ts @@ -1,4 +1,5 @@ import { createLogger } from '@sim/logger' +import { isRecordLike } from '@sim/utils/object' import { NextResponse } from 'next/server' import { getClientIp } from '@/lib/core/utils/request' import type { @@ -13,7 +14,84 @@ import { verifyTokenAuth } from '@/lib/webhooks/providers/utils' const logger = createLogger('WebhookProvider:Generic') +/** + * Headers withheld from the workflow input because they carry credentials. Exposing one would + * copy the secret into execution logs and trace spans, where it outlives the request. The + * webhook's own `secretHeaderName` is withheld on top of this list, per webhook. + * + * A denylist rather than an allowlist, because arbitrary custom headers being usable is the + * point of the feature. + */ +const CREDENTIAL_HEADER_NAMES = new Set([ + 'authorization', + 'proxy-authorization', + 'cookie', + 'set-cookie', + 'x-api-key', + 'x-auth-token', + 'x-sim-idempotency-key', +]) + +/** Request headers for the workflow input, minus the ones that carry credentials. */ +function exposedHeaders( + headers: Record, + secretHeaderName?: string +): Record { + const withheld = secretHeaderName?.toLowerCase() + const exposed: Record = {} + + for (const [name, value] of Object.entries(headers)) { + const lowerName = name.toLowerCase() + if (CREDENTIAL_HEADER_NAMES.has(lowerName) || lowerName === withheld) continue + exposed[lowerName] = value + } + + return exposed +} + +/** + * Merge request metadata into the body under reserved keys. The body keeps precedence per key, + * so a payload that already carries a field of that name resolves exactly as it did before. + */ +function mergeRequestData( + body: unknown, + requestData: Record>, + requestId: string +): unknown { + const entries = Object.entries(requestData).filter(([, value]) => + typeof value === 'string' ? value.length > 0 : Object.keys(value).length > 0 + ) + + if (entries.length === 0) { + return body + } + + if (!isRecordLike(body)) { + logger.warn( + `[${requestId}] Dropping webhook request metadata: the body is not an object, so there is no field to merge it into`, + { keys: entries.map(([key]) => key) } + ) + return body + } + + const merged: Record = { ...body } + + for (const [key, value] of entries) { + if (key in body) { + logger.warn( + `[${requestId}] Dropping webhook ${key}: the body already defines a "${key}" field` + ) + continue + } + merged[key] = value + } + + return merged +} + export const genericHandler: WebhookProviderHandler = { + extraDeliveryMethods: ['GET', 'PUT', 'PATCH', 'DELETE'], + verifyAuth({ request, requestId, providerConfig }: AuthContext) { if (providerConfig.requireAuth) { const configToken = providerConfig.token as string | undefined @@ -84,8 +162,31 @@ export const genericHandler: WebhookProviderHandler = { return null }, - async formatInput({ body }: FormatInputContext): Promise { - return { input: body } + /** + * Expose the request method, query parameters and headers under reserved `method`, `query` and + * `headers` keys alongside the body fields. + */ + async formatInput({ + body, + headers, + query, + method, + webhook, + requestId, + }: FormatInputContext): Promise { + const providerConfig = (webhook.providerConfig as Record | null) ?? {} + + return { + input: mergeRequestData( + body, + { + method, + query, + headers: exposedHeaders(headers, providerConfig.secretHeaderName as string | undefined), + }, + requestId + ), + } }, async processInputFiles({ diff --git a/apps/sim/lib/webhooks/providers/index.ts b/apps/sim/lib/webhooks/providers/index.ts index e3f4adfd48c..733d8b9b00a 100644 --- a/apps/sim/lib/webhooks/providers/index.ts +++ b/apps/sim/lib/webhooks/providers/index.ts @@ -28,3 +28,15 @@ export function acceptsPathWebhookDelivery(provider: string | null): boolean { if (isInternalTriggerProvider(provider) || isPollingWebhookProvider(provider)) return false return getProviderHandler(provider).ingressMode !== 'provider' } + +/** + * Whether a provider accepts a delivery arriving with this HTTP method. + * + * Every provider accepts `POST`. Anything else is opt-in per provider because such a delivery may + * carry no body, and because a `GET` in particular is not idempotent-safe against link prefetchers. + */ +export function acceptsWebhookDeliveryMethod(provider: string | null, method: string): boolean { + if (method === 'POST') return true + if (!provider) return false + return getProviderHandler(provider).extraDeliveryMethods?.includes(method) === true +} diff --git a/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts b/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts index 93bf5642662..19c7c424f84 100644 --- a/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts +++ b/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts @@ -219,4 +219,39 @@ describe('microsoftTeamsHandler formatInput (outgoing webhook channelData)', () teamsChannelId: 'channel-1', }) }) + + describe('handleChallenge', () => { + function challengeRequest(method: string): NextRequest { + return new NextRequest( + 'https://app.example.com/api/webhooks/trigger/abc?validationToken=token-123', + { method } + ) + } + + it('echoes the validation token for the POST Microsoft Graph sends', async () => { + const response = microsoftTeamsHandler.handleChallenge!( + {}, + challengeRequest('POST'), + 'teams-challenge-post', + 'abc' + ) + + expect(response?.status).toBe(200) + await expect(response?.text()).resolves.toBe('token-123') + }) + + it.each(['GET', 'PUT', 'PATCH', 'DELETE'])( + 'ignores a validationToken query parameter on a %s delivery', + (method) => { + expect( + microsoftTeamsHandler.handleChallenge!( + {}, + challengeRequest(method), + 'teams-challenge-other-method', + 'abc' + ) + ).toBeNull() + } + ) + }) }) diff --git a/apps/sim/lib/webhooks/providers/microsoft-teams.ts b/apps/sim/lib/webhooks/providers/microsoft-teams.ts index ccaa56eb12c..178c3963939 100644 --- a/apps/sim/lib/webhooks/providers/microsoft-teams.ts +++ b/apps/sim/lib/webhooks/providers/microsoft-teams.ts @@ -479,6 +479,15 @@ async function formatTeamsGraphNotification( export const microsoftTeamsHandler: WebhookProviderHandler = { handleChallenge(_body: unknown, request: NextRequest, requestId: string, path: string) { + /** + * Microsoft Graph sends the subscription validation as a POST. Answering it for any method + * would let a `validationToken` query parameter on a GET, PUT, PATCH or DELETE delivery to + * another provider's path be echoed back instead of triggering that workflow. + */ + if (request.method !== 'POST') { + return null + } + const url = new URL(request.url) const validationToken = url.searchParams.get('validationToken') if (validationToken) { diff --git a/apps/sim/lib/webhooks/providers/types.ts b/apps/sim/lib/webhooks/providers/types.ts index 45a6dd5b03e..2d69e901c92 100644 --- a/apps/sim/lib/webhooks/providers/types.ts +++ b/apps/sim/lib/webhooks/providers/types.ts @@ -33,6 +33,10 @@ export interface FormatInputContext { workflow: { id: string; userId: string } body: unknown headers: Record + /** Request URL query parameters. Repeated keys collapse to the last value. */ + query: Record + /** HTTP method of the delivering request. Empty on legacy queued jobs. */ + method: string requestId: string } @@ -99,6 +103,16 @@ export interface WebhookProviderHandler { */ ingressMode?: 'path' | 'provider' + /** + * Methods accepted in addition to `POST`. Use for providers whose events can originate from a + * plain HTTP call (an email link, a REST-style client) rather than a signed callback. Such a + * delivery may carry no body at all, so the provider must be able to trigger on the query + * parameters alone, and with `GET` the caller must tolerate the request being replayed by link + * prefetchers and scanners. Workflows tell the methods apart through the trigger input's + * `method` field. + */ + extraDeliveryMethods?: readonly string[] + /** * Queue workflow execution through the configured durable backend instead of the low-latency * in-process path. Use for providers whose ingress is acknowledged before target processing. diff --git a/apps/sim/lib/webhooks/providers/whatsapp.test.ts b/apps/sim/lib/webhooks/providers/whatsapp.test.ts index c26763dea02..e6f373e3727 100644 --- a/apps/sim/lib/webhooks/providers/whatsapp.test.ts +++ b/apps/sim/lib/webhooks/providers/whatsapp.test.ts @@ -2,7 +2,7 @@ * @vitest-environment node */ import { createHmac } from 'node:crypto' -import { dbChainMock, schemaMock } from '@sim/testing' +import { dbChainMock, queueTableRows, schemaMock } from '@sim/testing' import { NextRequest } from 'next/server' import { describe, expect, it, vi } from 'vitest' @@ -265,6 +265,42 @@ describe('WhatsApp webhook provider', () => { expect(input.caption).toBeUndefined() }) + describe('handleChallenge', () => { + function verificationRequest(): NextRequest { + return new NextRequest( + 'http://localhost/api/webhooks/trigger/abc?hub.mode=subscribe&hub.verify_token=t&hub.challenge=c' + ) + } + + it('falls through when no WhatsApp webhook on the path expects a token', async () => { + queueTableRows(schemaMock.webhook, []) + + const response = await whatsappHandler.handleChallenge!( + {}, + verificationRequest(), + 'wa-challenge-no-webhook', + 'abc' + ) + + expect(response).toBeNull() + }) + + it('still fails verification when a WhatsApp webhook expects a different token', async () => { + queueTableRows(schemaMock.webhook, [ + { webhook: { id: 'wh_1', providerConfig: { verificationToken: 'other' } } }, + ]) + + const response = await whatsappHandler.handleChallenge!( + {}, + verificationRequest(), + 'wa-challenge-token-mismatch', + 'abc' + ) + + expect(response?.status).toBe(403) + }) + }) + it('ignores a media type whose payload object is missing', async () => { const input = await formatMediaMessage({ id: 'wamid.image.2', diff --git a/apps/sim/lib/webhooks/providers/whatsapp.ts b/apps/sim/lib/webhooks/providers/whatsapp.ts index a62fec720c4..a2402d69590 100644 --- a/apps/sim/lib/webhooks/providers/whatsapp.ts +++ b/apps/sim/lib/webhooks/providers/whatsapp.ts @@ -202,6 +202,8 @@ async function handleWhatsAppVerification( ) ) + let candidates = 0 + for (const row of webhooks) { const wh = row.webhook const providerConfig = (wh.providerConfig as Record) || {} @@ -211,6 +213,8 @@ async function handleWhatsAppVerification( continue } + candidates++ + if (safeCompare(token, verificationToken as string)) { logger.info(`[${requestId}] WhatsApp verification successful for webhook ${wh.id}`) return new NextResponse(challenge, { @@ -222,6 +226,15 @@ async function handleWhatsAppVerification( } } + /** + * A path with no WhatsApp webhook expecting a token is not a failed verification: the + * `hub.*` parameters belong to whoever owns that path. Fall through so the delivery is + * routed normally instead of answering 403 for someone else's query parameters. + */ + if (candidates === 0) { + return null + } + logger.warn(`[${requestId}] No matching WhatsApp verification token found`) return new NextResponse('Verification failed', { status: 403 }) } diff --git a/apps/sim/triggers/generic/webhook.test.ts b/apps/sim/triggers/generic/webhook.test.ts new file mode 100644 index 00000000000..61df54095bf --- /dev/null +++ b/apps/sim/triggers/generic/webhook.test.ts @@ -0,0 +1,34 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { genericWebhookTrigger } from '@/triggers/generic/webhook' + +function setupInstructions(): string { + return String( + genericWebhookTrigger.subBlocks.find((subBlock) => subBlock.id === 'triggerInstructions') + ?.defaultValue + ) +} + +describe('genericWebhookTrigger', () => { + it('declares the request metadata so it can be referenced from later blocks', () => { + expect(Object.keys(genericWebhookTrigger.outputs)).toEqual(['method', 'query', 'headers']) + expect(genericWebhookTrigger.outputs.method.type).toBe('string') + expect(genericWebhookTrigger.outputs.query.type).toBe('object') + expect(genericWebhookTrigger.outputs.headers.type).toBe('object') + }) + + it('names the methods the endpoint actually accepts', () => { + expect(setupInstructions()).toContain('GET, POST, PUT, PATCH and DELETE requests') + }) + + it('names every reserved key the input carries', () => { + const instructions = setupInstructions() + + for (const key of Object.keys(genericWebhookTrigger.outputs)) { + expect(instructions).toContain(`"${key}"`) + } + expect(instructions).toContain('Headers that carry credentials are withheld.') + }) +}) diff --git a/apps/sim/triggers/generic/webhook.ts b/apps/sim/triggers/generic/webhook.ts index c546c4d4f5c..e34d20f0e6f 100644 --- a/apps/sim/triggers/generic/webhook.ts +++ b/apps/sim/triggers/generic/webhook.ts @@ -118,9 +118,9 @@ export const genericWebhookTrigger: TriggerConfig = { defaultValue: [ 'Copy the webhook URL and use it in your external service or API.', 'Configure your service to send webhooks to this URL.', - 'The webhook will receive any HTTP method (GET, POST, PUT, DELETE, etc.).', - 'All request data (headers, body, query parameters) will be available in your workflow.', - 'If authentication is enabled, include the token in requests using either the custom header or "Authorization: Bearer TOKEN".', + 'The webhook accepts GET, POST, PUT, PATCH and DELETE requests. Use GET to trigger the workflow from a plain URL, such as a link in an email.', + 'Body fields are available in your workflow, and the request method, headers and query parameters are available under "method", "headers" and "query" (for example "headers.x-event-name", "query.id"). Headers that carry credentials are withheld.', + 'If authentication is enabled, include the token in the Secret Header Name you configured, or in "Authorization: Bearer TOKEN" if you left it blank. Only the configured method is accepted.', 'To deduplicate incoming events, set the Deduplication Field to the dot-notation path of a unique identifier in the payload (e.g. "event.id"). Duplicate values within 7 days will be skipped.', 'Enable "Verify Test Events" only if the sending service needs a temporary 200 response while validating the webhook URL.', ] @@ -133,7 +133,24 @@ export const genericWebhookTrigger: TriggerConfig = { }, ], - outputs: {}, + /** + * Body fields stay undeclared because a generic webhook receives whatever JSON the caller + * sends. The request metadata below is known ahead of time, so it can be offered for reference. + */ + outputs: { + method: { + type: 'string', + description: 'HTTP method of the request (GET, POST, PUT, PATCH or DELETE)', + }, + query: { + type: 'object', + description: 'Query parameters from the request URL', + }, + headers: { + type: 'object', + description: 'Request headers, excluding the ones that carry credentials', + }, + }, webhook: { method: 'POST',