From 517b5904db280a3fd9cf072047fb14e65990b6ba Mon Sep 17 00:00:00 2001 From: "mini.jeong" Date: Wed, 19 Aug 2026 19:39:50 +0900 Subject: [PATCH 1/6] feat(webhooks): support query parameters and GET deliveries on generic webhooks The generic webhook Setup Instructions promised that query parameters would be available in the workflow and that any HTTP method would be accepted, but neither was true: query parameters were never carried past the route, and every GET that was not a provider challenge got a 405. Carry the request query string into the execution payload and expose it to providers through FormatInputContext. The generic provider merges it into the workflow input under a reserved `query` key, leaving the body's own fields untouched so existing payloads resolve exactly as before. Add an opt-in `acceptsGetDelivery` provider capability and enable it for the generic provider, so a workflow can be triggered by a plain URL fetch such as a link in an email. Providers that have not opted in still answer 405, and unknown paths keep answering 405 on GET so probes cannot distinguish them. Update the Setup Instructions to describe what the endpoint actually accepts. Signed-off-by: mini.jeong --- .../api/webhooks/trigger/[path]/route.test.ts | 48 ++++++++++++++ .../app/api/webhooks/trigger/[path]/route.ts | 54 ++++++++++++---- apps/sim/background/webhook-execution.ts | 3 + apps/sim/lib/webhooks/processor.test.ts | 38 +++++++++++ apps/sim/lib/webhooks/processor.ts | 2 + .../lib/webhooks/providers/generic.test.ts | 64 +++++++++++++++++++ apps/sim/lib/webhooks/providers/generic.ts | 30 ++++++++- apps/sim/lib/webhooks/providers/index.ts | 12 ++++ apps/sim/lib/webhooks/providers/types.ts | 10 +++ apps/sim/triggers/generic/webhook.ts | 4 +- 10 files changed, 250 insertions(+), 15 deletions(-) create mode 100644 apps/sim/lib/webhooks/providers/generic.test.ts 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..51d359e277c 100644 --- a/apps/sim/app/api/webhooks/trigger/[path]/route.test.ts +++ b/apps/sim/app/api/webhooks/trigger/[path]/route.test.ts @@ -683,6 +683,54 @@ 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('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..93d90335a83 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,10 +35,26 @@ 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() + } } ) @@ -50,16 +66,17 @@ export const POST = withRouteHandler( } try { - return await handleWebhookPost(request, context) + return await handleWebhookDelivery(request, context, webhookTriggerPostContract) } finally { ticket.release() } } ) -async function handleWebhookPost( +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 +90,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 +116,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 +148,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..dfd0d755c99 100644 --- a/apps/sim/background/webhook-execution.ts +++ b/apps/sim/background/webhook-execution.ts @@ -269,6 +269,8 @@ 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 path: string blockId?: string /** Immutable deployment admitted by webhook ingress; absent on legacy queued jobs. */ @@ -622,6 +624,7 @@ async function executeWebhookJobInternal( workflow: { id: payload.workflowId, userId: payload.userId }, body: payload.body, headers: payload.headers, + query: payload.query ?? {}, 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..494faadd192 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,7 @@ async function queueWebhookExecutionWithResult( provider: foundWebhook.provider, body, headers, + ...(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..631e647ff39 --- /dev/null +++ b/apps/sim/lib/webhooks/providers/generic.test.ts @@ -0,0 +1,64 @@ +/** + * @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): FormatInputContext { + return { + webhook: { id: 'webhook-id', provider: 'generic' }, + workflow: { id: 'workflow-id', userId: 'user-id' }, + body, + headers: {}, + query, + 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) + }) +}) + +describe('genericHandler delivery methods', () => { + it('opts into GET deliveries', () => { + expect(genericHandler.acceptsGetDelivery).toBe(true) + }) +}) diff --git a/apps/sim/lib/webhooks/providers/generic.ts b/apps/sim/lib/webhooks/providers/generic.ts index 71372bebad6..67269cbdc69 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 { @@ -14,6 +15,8 @@ import { verifyTokenAuth } from '@/lib/webhooks/providers/utils' const logger = createLogger('WebhookProvider:Generic') export const genericHandler: WebhookProviderHandler = { + acceptsGetDelivery: true, + verifyAuth({ request, requestId, providerConfig }: AuthContext) { if (providerConfig.requireAuth) { const configToken = providerConfig.token as string | undefined @@ -84,8 +87,31 @@ export const genericHandler: WebhookProviderHandler = { return null }, - async formatInput({ body }: FormatInputContext): Promise { - return { input: body } + /** + * Expose query parameters under a reserved `query` key alongside the body fields. + * The body keeps precedence so payloads that already carry their own `query` field + * resolve exactly as they did before. + */ + async formatInput({ body, query, requestId }: FormatInputContext): Promise { + if (Object.keys(query).length === 0) { + return { input: body } + } + + if (!isRecordLike(body)) { + logger.warn( + `[${requestId}] Dropping query parameters: webhook body is not an object, so there is no field to merge them into` + ) + return { input: body } + } + + if ('query' in body) { + logger.warn( + `[${requestId}] Dropping query parameters: webhook body already defines a "query" field` + ) + return { input: body } + } + + return { input: { ...body, query } } }, async processInputFiles({ diff --git a/apps/sim/lib/webhooks/providers/index.ts b/apps/sim/lib/webhooks/providers/index.ts index e3f4adfd48c..a3d7bb190f0 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`; `GET` is opt-in per provider because a GET delivery has no body + * and is not idempotent-safe against link prefetchers. Other methods are never accepted. + */ +export function acceptsWebhookDeliveryMethod(provider: string | null, method: string): boolean { + if (method === 'POST') return true + if (method !== 'GET' || !provider) return false + return getProviderHandler(provider).acceptsGetDelivery === true +} diff --git a/apps/sim/lib/webhooks/providers/types.ts b/apps/sim/lib/webhooks/providers/types.ts index 45a6dd5b03e..f45530e73d4 100644 --- a/apps/sim/lib/webhooks/providers/types.ts +++ b/apps/sim/lib/webhooks/providers/types.ts @@ -33,6 +33,8 @@ 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 requestId: string } @@ -99,6 +101,14 @@ export interface WebhookProviderHandler { */ ingressMode?: 'path' | 'provider' + /** + * Accept `GET` deliveries in addition to `POST`. Use for providers whose events can originate + * from a plain URL fetch (an email link, a browser navigation) rather than a signed callback. + * `GET` deliveries carry no body, so such providers must be able to trigger on query parameters + * alone, and callers must tolerate the request being replayed by link prefetchers and scanners. + */ + acceptsGetDelivery?: boolean + /** * 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/triggers/generic/webhook.ts b/apps/sim/triggers/generic/webhook.ts index c546c4d4f5c..839baac75b8 100644 --- a/apps/sim/triggers/generic/webhook.ts +++ b/apps/sim/triggers/generic/webhook.ts @@ -118,8 +118,8 @@ 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.', + 'The webhook accepts GET and POST requests. Use GET to trigger the workflow from a plain URL, such as a link in an email.', + 'Request headers and body fields are available in your workflow, and query parameters are available under "query" (for example "query.id").', 'If authentication is enabled, include the token in requests using either the custom header or "Authorization: Bearer TOKEN".', '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.', From 00eb9228e3fa87842b6d2d36a0e435cd320bb6e9 Mon Sep 17 00:00:00 2001 From: "mini.jeong" Date: Thu, 20 Aug 2026 18:57:34 +0900 Subject: [PATCH 2/6] feat(webhooks): expose generic webhook request headers The generic webhook's Setup Instructions promised that request headers would be available in the workflow, but formatInput returned only the body: headers were used solely for the idempotency key and provider signature checks. Expose them under a reserved `headers` key, withholding the ones that carry credentials. Exposing a credential would copy it into execution logs and trace spans, where it outlives the request, so a fixed denylist (authorization, cookie, x-api-key, ...) is combined with the webhook's own configured secretHeaderName. A denylist rather than an allowlist keeps arbitrary custom headers usable, which is the point of the feature. Generalize the query-parameter merge so query and headers share the same key-wise body-precedence rule. Also correct the authentication instruction: only the configured method is accepted, not either one. Refs #6888 Signed-off-by: mini.jeong --- .../lib/webhooks/providers/generic.test.ts | 71 ++++++++++- apps/sim/lib/webhooks/providers/generic.ts | 115 ++++++++++++++---- apps/sim/triggers/generic/webhook.ts | 4 +- 3 files changed, 163 insertions(+), 27 deletions(-) diff --git a/apps/sim/lib/webhooks/providers/generic.test.ts b/apps/sim/lib/webhooks/providers/generic.test.ts index 631e647ff39..303eb4c2496 100644 --- a/apps/sim/lib/webhooks/providers/generic.test.ts +++ b/apps/sim/lib/webhooks/providers/generic.test.ts @@ -5,12 +5,22 @@ 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): FormatInputContext { +function context( + body: unknown, + query: Record, + options: { headers?: Record; secretHeaderName?: string } = {} +): FormatInputContext { return { - webhook: { id: 'webhook-id', provider: 'generic' }, + webhook: { + id: 'webhook-id', + provider: 'generic', + providerConfig: options.secretHeaderName + ? { secretHeaderName: options.secretHeaderName } + : {}, + }, workflow: { id: 'workflow-id', userId: 'user-id' }, body, - headers: {}, + headers: options.headers ?? {}, query, requestId: 'req-1', } @@ -55,6 +65,61 @@ describe('genericHandler.formatInput', () => { 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', () => { diff --git a/apps/sim/lib/webhooks/providers/generic.ts b/apps/sim/lib/webhooks/providers/generic.ts index 67269cbdc69..d0a85a4e696 100644 --- a/apps/sim/lib/webhooks/providers/generic.ts +++ b/apps/sim/lib/webhooks/providers/generic.ts @@ -14,6 +14,79 @@ 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]) => 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 = { acceptsGetDelivery: true, @@ -88,30 +161,28 @@ export const genericHandler: WebhookProviderHandler = { }, /** - * Expose query parameters under a reserved `query` key alongside the body fields. - * The body keeps precedence so payloads that already carry their own `query` field - * resolve exactly as they did before. + * Expose query parameters and request headers under reserved `query` and `headers` keys + * alongside the body fields. */ - async formatInput({ body, query, requestId }: FormatInputContext): Promise { - if (Object.keys(query).length === 0) { - return { input: body } - } - - if (!isRecordLike(body)) { - logger.warn( - `[${requestId}] Dropping query parameters: webhook body is not an object, so there is no field to merge them into` - ) - return { input: body } - } - - if ('query' in body) { - logger.warn( - `[${requestId}] Dropping query parameters: webhook body already defines a "query" field` - ) - return { input: body } + async formatInput({ + body, + headers, + query, + webhook, + requestId, + }: FormatInputContext): Promise { + const providerConfig = (webhook.providerConfig as Record | null) ?? {} + + return { + input: mergeRequestData( + body, + { + query, + headers: exposedHeaders(headers, providerConfig.secretHeaderName as string | undefined), + }, + requestId + ), } - - return { input: { ...body, query } } }, async processInputFiles({ diff --git a/apps/sim/triggers/generic/webhook.ts b/apps/sim/triggers/generic/webhook.ts index 839baac75b8..a484c771795 100644 --- a/apps/sim/triggers/generic/webhook.ts +++ b/apps/sim/triggers/generic/webhook.ts @@ -119,8 +119,8 @@ export const genericWebhookTrigger: TriggerConfig = { 'Copy the webhook URL and use it in your external service or API.', 'Configure your service to send webhooks to this URL.', 'The webhook accepts GET and POST requests. Use GET to trigger the workflow from a plain URL, such as a link in an email.', - 'Request headers and body fields are available in your workflow, and query parameters are available under "query" (for example "query.id").', - 'If authentication is enabled, include the token in requests using either the custom header or "Authorization: Bearer TOKEN".', + 'Body fields are available in your workflow, and request headers and query parameters are available under "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.', ] From e3d1c7446b8218e9cc5ae8c8845c2bddc115cc89 Mon Sep 17 00:00:00 2001 From: "mini.jeong" Date: Thu, 20 Aug 2026 19:00:55 +0900 Subject: [PATCH 3/6] feat(webhooks): accept PUT, PATCH and DELETE deliveries and expose the request method The generic webhook's Setup Instructions promised any HTTP method, and the /api CORS policy already advertises PUT, PATCH and DELETE, yet the route answered 405 for everything except POST and GET. Open the remaining methods for providers that opt in, which today is only the generic webhook. Expose the method on the trigger input as well. Without it a workflow behind one URL cannot tell a create from a delete, which makes multi-method delivery half a feature. The payload field is optional so jobs already queued at deploy time keep executing. Turn the GET-only opt-in into a per-provider method set, and let the request metadata merge carry scalar values so `method` follows the same key-wise body-precedence rule as query and headers. Refs #6888 Signed-off-by: mini.jeong --- .../api/webhooks/trigger/[path]/route.test.ts | 71 ++++++++++++++++++- .../app/api/webhooks/trigger/[path]/route.ts | 12 +++- apps/sim/background/webhook-execution.ts | 3 + apps/sim/lib/webhooks/processor.ts | 1 + .../lib/webhooks/providers/generic.test.ts | 28 +++++++- apps/sim/lib/webhooks/providers/generic.ts | 14 ++-- apps/sim/lib/webhooks/providers/index.ts | 8 +-- apps/sim/lib/webhooks/providers/types.ts | 14 ++-- apps/sim/triggers/generic/webhook.ts | 4 +- 9 files changed, 134 insertions(+), 21 deletions(-) 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 51d359e277c..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(() => { @@ -731,6 +731,75 @@ describe('Webhook Trigger API Route', () => { }) }) + 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 93d90335a83..9d3e272e9d2 100644 --- a/apps/sim/app/api/webhooks/trigger/[path]/route.ts +++ b/apps/sim/app/api/webhooks/trigger/[path]/route.ts @@ -58,7 +58,7 @@ export const GET = withRouteHandler( } ) -export const POST = withRouteHandler( +const handleBodyDelivery = withRouteHandler( async (request: NextRequest, context: { params: Promise<{ path: string }> }) => { const ticket = tryAdmit() if (!ticket) { @@ -73,6 +73,16 @@ export const POST = withRouteHandler( } ) +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 }> }, diff --git a/apps/sim/background/webhook-execution.ts b/apps/sim/background/webhook-execution.ts index dfd0d755c99..5e6bb7d6617 100644 --- a/apps/sim/background/webhook-execution.ts +++ b/apps/sim/background/webhook-execution.ts @@ -271,6 +271,8 @@ export type WebhookExecutionPayload = { 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. */ @@ -625,6 +627,7 @@ async function executeWebhookJobInternal( 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.ts b/apps/sim/lib/webhooks/processor.ts index 494faadd192..d11e9a03e85 100644 --- a/apps/sim/lib/webhooks/processor.ts +++ b/apps/sim/lib/webhooks/processor.ts @@ -723,6 +723,7 @@ 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, diff --git a/apps/sim/lib/webhooks/providers/generic.test.ts b/apps/sim/lib/webhooks/providers/generic.test.ts index 303eb4c2496..1a6b25da6a4 100644 --- a/apps/sim/lib/webhooks/providers/generic.test.ts +++ b/apps/sim/lib/webhooks/providers/generic.test.ts @@ -8,7 +8,7 @@ import type { FormatInputContext } from '@/lib/webhooks/providers/types' function context( body: unknown, query: Record, - options: { headers?: Record; secretHeaderName?: string } = {} + options: { headers?: Record; secretHeaderName?: string; method?: string } = {} ): FormatInputContext { return { webhook: { @@ -22,6 +22,7 @@ function context( body, headers: options.headers ?? {}, query, + method: options.method ?? '', requestId: 'req-1', } } @@ -123,7 +124,28 @@ describe('genericHandler.formatInput', () => { }) describe('genericHandler delivery methods', () => { - it('opts into GET deliveries', () => { - expect(genericHandler.acceptsGetDelivery).toBe(true) + 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 d0a85a4e696..487b8478c7e 100644 --- a/apps/sim/lib/webhooks/providers/generic.ts +++ b/apps/sim/lib/webhooks/providers/generic.ts @@ -55,10 +55,12 @@ function exposedHeaders( */ function mergeRequestData( body: unknown, - requestData: Record>, + requestData: Record>, requestId: string ): unknown { - const entries = Object.entries(requestData).filter(([, value]) => Object.keys(value).length > 0) + const entries = Object.entries(requestData).filter(([, value]) => + typeof value === 'string' ? value.length > 0 : Object.keys(value).length > 0 + ) if (entries.length === 0) { return body @@ -88,7 +90,7 @@ function mergeRequestData( } export const genericHandler: WebhookProviderHandler = { - acceptsGetDelivery: true, + extraDeliveryMethods: ['GET', 'PUT', 'PATCH', 'DELETE'], verifyAuth({ request, requestId, providerConfig }: AuthContext) { if (providerConfig.requireAuth) { @@ -161,13 +163,14 @@ export const genericHandler: WebhookProviderHandler = { }, /** - * Expose query parameters and request headers under reserved `query` and `headers` keys - * alongside the body fields. + * 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 { @@ -177,6 +180,7 @@ export const genericHandler: WebhookProviderHandler = { input: mergeRequestData( body, { + method, query, headers: exposedHeaders(headers, providerConfig.secretHeaderName as string | undefined), }, diff --git a/apps/sim/lib/webhooks/providers/index.ts b/apps/sim/lib/webhooks/providers/index.ts index a3d7bb190f0..733d8b9b00a 100644 --- a/apps/sim/lib/webhooks/providers/index.ts +++ b/apps/sim/lib/webhooks/providers/index.ts @@ -32,11 +32,11 @@ export function acceptsPathWebhookDelivery(provider: string | null): boolean { /** * Whether a provider accepts a delivery arriving with this HTTP method. * - * Every provider accepts `POST`; `GET` is opt-in per provider because a GET delivery has no body - * and is not idempotent-safe against link prefetchers. Other methods are never accepted. + * 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 (method !== 'GET' || !provider) return false - return getProviderHandler(provider).acceptsGetDelivery === true + if (!provider) return false + return getProviderHandler(provider).extraDeliveryMethods?.includes(method) === true } diff --git a/apps/sim/lib/webhooks/providers/types.ts b/apps/sim/lib/webhooks/providers/types.ts index f45530e73d4..2d69e901c92 100644 --- a/apps/sim/lib/webhooks/providers/types.ts +++ b/apps/sim/lib/webhooks/providers/types.ts @@ -35,6 +35,8 @@ export interface FormatInputContext { 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 } @@ -102,12 +104,14 @@ export interface WebhookProviderHandler { ingressMode?: 'path' | 'provider' /** - * Accept `GET` deliveries in addition to `POST`. Use for providers whose events can originate - * from a plain URL fetch (an email link, a browser navigation) rather than a signed callback. - * `GET` deliveries carry no body, so such providers must be able to trigger on query parameters - * alone, and callers must tolerate the request being replayed by link prefetchers and scanners. + * 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. */ - acceptsGetDelivery?: boolean + extraDeliveryMethods?: readonly string[] /** * Queue workflow execution through the configured durable backend instead of the low-latency diff --git a/apps/sim/triggers/generic/webhook.ts b/apps/sim/triggers/generic/webhook.ts index a484c771795..fd9312e9752 100644 --- a/apps/sim/triggers/generic/webhook.ts +++ b/apps/sim/triggers/generic/webhook.ts @@ -118,8 +118,8 @@ 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 accepts GET and POST 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 request headers and query parameters are available under "headers" and "query" (for example "headers.x-event-name", "query.id"). Headers that carry credentials are withheld.', + '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.', From 6914b8ee64a977c684a9f51b45e385927afd1d24 Mon Sep 17 00:00:00 2001 From: "mini.jeong" Date: Thu, 20 Aug 2026 19:02:11 +0900 Subject: [PATCH 4/6] feat(webhooks): declare the generic webhook trigger outputs The trigger declared no outputs, so the reference dropdown in the editor offered no completions for it and users had to type paths like `query.id` by hand after reading the setup instructions. Declare the request metadata that is known ahead of time. Body fields stay undeclared because a generic webhook receives whatever JSON the caller sends. Refs #6888 Signed-off-by: mini.jeong --- apps/sim/triggers/generic/webhook.test.ts | 34 +++++++++++++++++++++++ apps/sim/triggers/generic/webhook.ts | 19 ++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 apps/sim/triggers/generic/webhook.test.ts 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 fd9312e9752..e34d20f0e6f 100644 --- a/apps/sim/triggers/generic/webhook.ts +++ b/apps/sim/triggers/generic/webhook.ts @@ -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', From ab13e20fce02e87a9cb988b5e0a4d8660e9435d0 Mon Sep 17 00:00:00 2001 From: "mini.jeong" Date: Thu, 20 Aug 2026 19:17:40 +0900 Subject: [PATCH 5/6] 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 --- .../providers/microsoft-teams.test.ts | 35 +++++++++++++++++ .../lib/webhooks/providers/microsoft-teams.ts | 9 +++++ .../lib/webhooks/providers/whatsapp.test.ts | 38 ++++++++++++++++++- apps/sim/lib/webhooks/providers/whatsapp.ts | 13 +++++++ 4 files changed, 94 insertions(+), 1 deletion(-) 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/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 }) } From 6d00f10503267d58b5f803e26a632536a36d8169 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 20 Aug 2026 14:27:42 -0700 Subject: [PATCH 6/6] fix(webhooks): make the request metadata opt-in per webhook The four commits below make the generic webhook do what its Setup Instructions promise. They do it through a provider-level capability, which applies to every generic webhook row the moment it deploys: each one begins accepting GET, PUT, PATCH and DELETE, and each one's workflow input gains `method` and `headers`, on POST deliveries too. No webhook owner chose either. Gate both behind `providerConfig` flags written by two switches, off by default. A webhook deployed before these existed has neither flag, so it answers POST only and its input is exactly the body, as before. `query` stays ungated: it is dropped today, only appears when the caller's own URL carries it, and yields to a body field of the same name. Generalize the Microsoft Teams challenge fix. Every challenge handler runs before the webhook lookup and matches on payload shape alone, so any of them will answer a delivery addressed to another provider on the same path. Gate them centrally to POST via `challengeMethods`, which WhatsApp widens to GET for Meta's handshake, rather than guarding one handler inline. Also: - Widen the credential header denylist to 24 names and withhold the webhook's own token by value as well as by name, since a denylist is leaky by construction. - Condition the `method` and `headers` trigger outputs on their switches, so the reference dropdown cannot offer a field the webhook will not send. - Give PUT, PATCH and DELETE their own contracts instead of reusing the POST one, whose `method: 'POST'` had become untrue. - Parse, challenge and generate a request ID once per delivery rather than twice on GET, which was logging one request under two IDs. - Offer the challenge handlers the request before admission, so Meta's GET handshake cannot be answered with a 429 by a busy instance. - Answer every non-POST rejection with the same 405 plus `Allow`, whether the path is unknown, holds only non-path triggers, or holds a trigger that has not opted in. - Read flags through a helper treating only `true`/`'true'` as on: the editor writes booleans, but a YAML- or Copilot-authored workflow can write the string `'false'`, which is truthy. - Name the methods switch "Accept Other HTTP Methods": HEAD and OPTIONS still answer 405, so claiming "all" would reintroduce the overstatement this whole change set exists to remove. - Drop the per-delivery metadata warn logs to debug. --- .../api/webhooks/trigger/[path]/route.test.ts | 169 +++++++++++- .../app/api/webhooks/trigger/[path]/route.ts | 150 ++++++---- apps/sim/lib/api/contracts/webhooks.ts | 38 +++ apps/sim/lib/webhooks/processor.test.ts | 63 ++++- apps/sim/lib/webhooks/processor.ts | 20 +- .../lib/webhooks/providers/generic.test.ts | 257 +++++++++++++----- apps/sim/lib/webhooks/providers/generic.ts | 96 +++++-- apps/sim/lib/webhooks/providers/index.ts | 22 +- .../providers/microsoft-teams.test.ts | 21 +- .../lib/webhooks/providers/microsoft-teams.ts | 9 - apps/sim/lib/webhooks/providers/types.ts | 33 ++- apps/sim/lib/webhooks/providers/utils.ts | 11 + .../lib/webhooks/providers/whatsapp.test.ts | 11 + apps/sim/lib/webhooks/providers/whatsapp.ts | 6 + apps/sim/triggers/generic/webhook.test.ts | 77 +++++- apps/sim/triggers/generic/webhook.ts | 40 ++- 16 files changed, 818 insertions(+), 205 deletions(-) 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 5634cdddaf3..0aec54129cd 100644 --- a/apps/sim/app/api/webhooks/trigger/[path]/route.test.ts +++ b/apps/sim/app/api/webhooks/trigger/[path]/route.test.ts @@ -462,6 +462,10 @@ vi.mock('postgres', () => vi.fn().mockReturnValue({})) process.env.DATABASE_URL = 'postgresql://test:test@localhost:5432/test' +import { + handlePreLookupWebhookVerification, + handleProviderChallenges, +} from '@/lib/webhooks/processor' import { DELETE, GET, PATCH, POST, PUT } from '@/app/api/webhooks/trigger/[path]/route' describe('Webhook Trigger API Route', () => { @@ -683,6 +687,60 @@ describe('Webhook Trigger API Route', () => { }) }) + /** + * Both handshakes are answered from the request alone, before any webhook lookup, so their + * order relative to each other and to the load-shed gate is the behavior — and it is invisible + * to every other test here, which is how an earlier refactor inverted it unnoticed. + */ + describe('pre-lookup handshake ordering', () => { + /** + * Meta verifies a WhatsApp URL with a GET challenge. Answering it behind the load-shed gate + * means a busy instance returns 429 and the webhook silently fails to verify, at setup time + * only — so the challenge must be answered without taking a ticket at all. + */ + it('answers a provider challenge without taking an admission ticket', async () => { + vi.mocked(handleProviderChallenges).mockResolvedValueOnce( + new NextResponse('hub-challenge-123', { status: 200 }) + ) + + const req = createMockRequest( + 'GET', + undefined, + {}, + 'http://localhost:3000/api/webhooks/trigger/verify-path?hub.challenge=hub-challenge-123' + ) + + const response = await GET(req, { params: Promise.resolve({ path: 'verify-path' }) }) + + expect(response.status).toBe(200) + await expect(response.text()).resolves.toBe('hub-challenge-123') + expect(tryAdmitMock).not.toHaveBeenCalled() + }) + + /** + * A challenge is the more specific answer: the provider is echoing a token it chose, where a + * pending verification only claims the URL is reachable. Answering the generic 200 first + * fails the handshake that actually had a token to return. + */ + it('prefers a provider challenge over a pending setup verification', async () => { + vi.mocked(handleProviderChallenges).mockResolvedValueOnce( + new NextResponse('hub-challenge-123', { status: 200 }) + ) + + const req = createMockRequest( + 'GET', + undefined, + {}, + 'http://localhost:3000/api/webhooks/trigger/verify-path?hub.challenge=hub-challenge-123' + ) + + const response = await GET(req, { params: Promise.resolve({ path: 'verify-path' }) }) + + await expect(response.text()).resolves.toBe('hub-challenge-123') + expect(handlePreLookupWebhookVerification).not.toHaveBeenCalled() + }) + }) + describe('GET deliveries', () => { it('dispatches a GET delivery to a generic webhook', async () => { testData.webhooks.push({ @@ -690,7 +748,7 @@ describe('Webhook Trigger API Route', () => { provider: 'generic', path: 'get-path', isActive: true, - providerConfig: { requireAuth: false }, + providerConfig: { requireAuth: false, acceptOtherMethods: true }, workflowId: 'test-workflow-id', }) @@ -707,6 +765,61 @@ describe('Webhook Trigger API Route', () => { expect(dispatchResolvedWebhookTargetMock).toHaveBeenCalledOnce() }) + /** + * The compatibility guarantee for the route: a generic webhook deployed before the flag + * existed has no flag, so it answers exactly as it did before — 405, no execution. + */ + it('rejects a GET delivery to a generic webhook that has not opted in', async () => { + testData.webhooks.push({ + id: 'generic-webhook-id', + provider: 'generic', + path: 'opt-out-path', + isActive: true, + providerConfig: { requireAuth: false }, + workflowId: 'test-workflow-id', + }) + + const req = createMockRequest( + 'GET', + undefined, + {}, + 'http://localhost:3000/api/webhooks/trigger/opt-out-path?srcId=123' + ) + + const response = await GET(req, { params: Promise.resolve({ path: 'opt-out-path' }) }) + + expect(response.status).toBe(405) + expect(response.headers.get('Allow')).toBe('POST') + expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled() + }) + + /** + * Next derives HEAD from the exported GET, so a HEAD probe reaches the same handler. It must + * not execute a workflow: scanners and prefetchers send HEAD unprompted. + */ + it('rejects a HEAD probe to a webhook that accepts every declared method', async () => { + testData.webhooks.push({ + id: 'generic-webhook-id', + provider: 'generic', + path: 'head-path', + isActive: true, + providerConfig: { requireAuth: false, acceptOtherMethods: true }, + workflowId: 'test-workflow-id', + }) + + const req = createMockRequest( + 'HEAD', + undefined, + {}, + 'http://localhost:3000/api/webhooks/trigger/head-path' + ) + + const response = await GET(req, { params: Promise.resolve({ path: 'head-path' }) }) + + expect(response.status).toBe(405) + expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled() + }) + it('rejects a GET delivery to a provider that only accepts POST', async () => { testData.webhooks.push({ id: 'stripe-webhook-id', @@ -742,7 +855,7 @@ describe('Webhook Trigger API Route', () => { provider: 'generic', path: 'any-method-path', isActive: true, - providerConfig: { requireAuth: false }, + providerConfig: { requireAuth: false, acceptOtherMethods: true }, workflowId: 'test-workflow-id', }) @@ -762,6 +875,29 @@ describe('Webhook Trigger API Route', () => { } ) + it('rejects a PUT delivery to a generic webhook that has not opted in', async () => { + testData.webhooks.push({ + id: 'generic-webhook-id', + provider: 'generic', + path: 'opt-out-path', + isActive: true, + providerConfig: { requireAuth: false }, + workflowId: 'test-workflow-id', + }) + + const req = createMockRequest( + 'PUT', + { event: 'test' }, + {}, + 'http://localhost:3000/api/webhooks/trigger/opt-out-path' + ) + + const response = await PUT(req, { params: Promise.resolve({ path: 'opt-out-path' }) }) + + expect(response.status).toBe(405) + expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled() + }) + it('rejects a PUT delivery to a provider that only accepts POST', async () => { testData.webhooks.push({ id: 'stripe-webhook-id', @@ -785,6 +921,35 @@ describe('Webhook Trigger API Route', () => { expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled() }) + /** + * Every non-POST rejection is the same 405, whether the path is unknown, holds only + * non-path triggers, or holds a trigger that has not opted in — so a probe cannot tell + * a configured path from an unused one. + */ + it('returns the same 405 for a DELETE to a non-path trigger as to an unknown path', async () => { + testData.webhooks.push({ + id: 'internal-webhook-id', + provider: 'sim', + path: 'internal-path', + isActive: true, + providerConfig: {}, + workflowId: 'test-workflow-id', + }) + + const req = createMockRequest( + 'DELETE', + undefined, + {}, + 'http://localhost:3000/api/webhooks/trigger/internal-path' + ) + + const response = await DELETE(req, { params: Promise.resolve({ path: 'internal-path' }) }) + + expect(response.status).toBe(405) + expect(response.headers.get('Allow')).toBe('POST') + expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled() + }) + it('returns 405 for a DELETE to an unknown path', async () => { const req = createMockRequest( 'DELETE', diff --git a/apps/sim/app/api/webhooks/trigger/[path]/route.ts b/apps/sim/app/api/webhooks/trigger/[path]/route.ts index 9d3e272e9d2..0a8a90ca774 100644 --- a/apps/sim/app/api/webhooks/trigger/[path]/route.ts +++ b/apps/sim/app/api/webhooks/trigger/[path]/route.ts @@ -1,6 +1,12 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' -import { webhookTriggerGetContract, webhookTriggerPostContract } from '@/lib/api/contracts/webhooks' +import { + webhookTriggerDeleteContract, + webhookTriggerGetContract, + webhookTriggerPatchContract, + webhookTriggerPostContract, + webhookTriggerPutContract, +} from '@/lib/api/contracts/webhooks' import { parseRequest } from '@/lib/api/server' import { admissionRejectedResponse, tryAdmit } from '@/lib/core/admission/gate' import { generateRequestId } from '@/lib/core/utils/request' @@ -22,27 +28,49 @@ export const dynamic = 'force-dynamic' export const runtime = 'nodejs' export const maxDuration = 60 -export const GET = withRouteHandler( - async (request: NextRequest, context: { params: Promise<{ path: string }> }) => { +type RouteContext = { params: Promise<{ path: string }> } + +type WebhookTriggerContract = + | typeof webhookTriggerGetContract + | typeof webhookTriggerPostContract + | typeof webhookTriggerPutContract + | typeof webhookTriggerPatchContract + | typeof webhookTriggerDeleteContract + +/** + * Shared delivery entry point, running the steps that need no body — and therefore no load-shed + * ticket — before admission, in the order the `GET` route has always run them: + * + * 1. resolve the path, + * 2. offer the request to the provider challenge handlers, + * 3. on `GET`, answer a setup-time verification probe for a path that has no webhook row yet, + * 4. take a ticket and hand off to the delivery path. + * + * Steps 2 and 3 keep their relative order because a challenge is the more specific answer: a + * provider echoing a token it chose beats a generic "reachable" 200 when a path somehow has both + * pending at once. + */ +function defineDeliveryRoute( + contract: WebhookTriggerContract, + options: { probeBeforeLookup?: boolean } = {} +) { + return withRouteHandler(async (request: NextRequest, context: RouteContext) => { const requestId = generateRequestId() - const parsed = await parseRequest(webhookTriggerGetContract, request, context) + const parsed = await parseRequest(contract, request, context) if (!parsed.success) return parsed.response const { path } = parsed.data.params - // Handle provider-specific GET verifications (Microsoft Graph, WhatsApp, etc.) - const challengeResponse = await handleProviderChallenges({}, request, requestId, path) - if (challengeResponse) { - return challengeResponse - } + const challenge = await handleProviderChallenges({}, request, requestId, path) + if (challenge) return challenge - const verificationResponse = await handlePreLookupWebhookVerification( - request.method, - undefined, - requestId, - path - ) - if (verificationResponse) { - return verificationResponse + if (options.probeBeforeLookup) { + const verification = await handlePreLookupWebhookVerification( + request.method, + undefined, + requestId, + path + ) + if (verification) return verification } const ticket = tryAdmit() @@ -51,42 +79,56 @@ export const GET = withRouteHandler( } try { - return await handleWebhookDelivery(request, context, webhookTriggerGetContract) + return await handleWebhookDelivery(request, requestId, path) } finally { ticket.release() } - } -) + }) +} -const handleBodyDelivery = withRouteHandler( - async (request: NextRequest, context: { params: Promise<{ path: string }> }) => { - const ticket = tryAdmit() - if (!ticket) { - return admissionRejectedResponse() - } +/** + * `GET` alone probes before the lookup, because a provider validating a URL it has not been given + * a webhook for can only be answered there. `handleWebhookDelivery` runs the same check for every + * method once the lookup comes back empty. + */ +export const GET = defineDeliveryRoute(webhookTriggerGetContract, { probeBeforeLookup: true }) - try { - return await handleWebhookDelivery(request, context, webhookTriggerPostContract) - } finally { - ticket.release() - } - } -) +export const POST = defineDeliveryRoute(webhookTriggerPostContract) + +/** + * Accepted only by a webhook whose provider declares the method AND whose owner has opted in. + * Everything else gets a 405 from `handleWebhookDelivery`. + */ +export const PUT = defineDeliveryRoute(webhookTriggerPutContract) +export const PATCH = defineDeliveryRoute(webhookTriggerPatchContract) +export const DELETE = defineDeliveryRoute(webhookTriggerDeleteContract) -export const POST = handleBodyDelivery +/** + * A 405 response carries `Allow` per RFC 9110. Every rejection here allows exactly `POST`: a + * webhook + * that accepts more never reaches this branch, so the header cannot be used to tell an unknown + * path from a configured one. + */ +function methodNotAllowedResponse(): NextResponse { + return new NextResponse('Method not allowed', { status: 405, headers: { Allow: 'POST' } }) +} /** - * Methods a provider must opt into via `extraDeliveryMethods`. A delivery to a path whose - * triggers have not opted in gets a 405 from `handleWebhookDelivery`. + * The answer for a path that will not accept this delivery. `POST` keeps its historical 404 so + * existing callers see no change; anything else answers 405 uniformly, whether the path is + * unknown, holds only non-path triggers, or holds a trigger that has not opted into the method — + * so a probe cannot tell those apart. */ -export const PUT = handleBodyDelivery -export const PATCH = handleBodyDelivery -export const DELETE = handleBodyDelivery +function notDeliverableResponse(method: string): NextResponse { + return method === 'POST' + ? new NextResponse('Not Found', { status: 404 }) + : methodNotAllowedResponse() +} async function handleWebhookDelivery( request: NextRequest, - context: { params: Promise<{ path: string }> }, - contract: typeof webhookTriggerGetContract | typeof webhookTriggerPostContract + requestId: string, + path: string ): Promise { const receivedAt = Date.now() /** @@ -99,16 +141,6 @@ async function handleWebhookDelivery( ? Number(slackRequestTimestamp) * 1000 : undefined - const requestId = generateRequestId() - const parsed = await parseRequest(contract, request, context) - if (!parsed.success) return parsed.response - const { path } = parsed.data.params - - const earlyChallenge = await handleProviderChallenges({}, request, requestId, path) - if (earlyChallenge) { - return earlyChallenge - } - const parseResult = await parseWebhookBody(request, requestId) // Check if parseWebhookBody returned an error response @@ -118,6 +150,10 @@ async function handleWebhookDelivery( const { body, rawBody } = parseResult + /** + * Offered a second time, now with the parsed body: the pre-admission pass answers only the + * handshakes readable from the URL, and the rest match on body shape. + */ const challengeResponse = await handleProviderChallenges(body, request, requestId, path, rawBody) if (challengeResponse) { return challengeResponse @@ -132,18 +168,18 @@ async function handleWebhookDelivery( 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 }) + return notDeliverableResponse(request.method) } const webhooksForPath = pathWebhooks.filter(({ webhook: foundWebhook }) => - acceptsWebhookDeliveryMethod(foundWebhook.provider, request.method) + acceptsWebhookDeliveryMethod(foundWebhook.provider, request.method, foundWebhook.providerConfig) ) 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 }) + return methodNotAllowedResponse() } if (webhooksForPath.length === 0) { @@ -158,11 +194,7 @@ async function handleWebhookDelivery( } logger.warn(`[${requestId}] Webhook or workflow not found for path: ${path}`) - // 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 }) + return notDeliverableResponse(request.method) } // Process each webhook matched on this path diff --git a/apps/sim/lib/api/contracts/webhooks.ts b/apps/sim/lib/api/contracts/webhooks.ts index 5363c77f9c1..47bd6ea4c9e 100644 --- a/apps/sim/lib/api/contracts/webhooks.ts +++ b/apps/sim/lib/api/contracts/webhooks.ts @@ -273,6 +273,44 @@ export const webhookTriggerPostContract = defineRouteContract({ }, }) +/** + * `PUT`, `PATCH` and `DELETE` deliveries. Same shape as the `POST` contract — they exist as + * separate declarations rather than reusing it so each route method is described by a contract + * that states its own method, which is what the boundary audit and any future client read. + */ +export const webhookTriggerPutContract = defineRouteContract({ + method: 'PUT', + path: '/api/webhooks/trigger/[path]', + params: webhookTriggerParamsSchema, + response: { + mode: 'json', + // untyped-response: webhook trigger forwards arbitrary provider challenge or workflow execution payloads + schema: z.unknown(), + }, +}) + +export const webhookTriggerPatchContract = defineRouteContract({ + method: 'PATCH', + path: '/api/webhooks/trigger/[path]', + params: webhookTriggerParamsSchema, + response: { + mode: 'json', + // untyped-response: webhook trigger forwards arbitrary provider challenge or workflow execution payloads + schema: z.unknown(), + }, +}) + +export const webhookTriggerDeleteContract = defineRouteContract({ + method: 'DELETE', + path: '/api/webhooks/trigger/[path]', + params: webhookTriggerParamsSchema, + response: { + mode: 'json', + // untyped-response: webhook trigger forwards arbitrary provider challenge or workflow execution payloads + schema: z.unknown(), + }, +}) + /** * TikTok app-level webhook ingress. Signature is verified from the raw body * before this schema runs; `content` remains a JSON string per TikTok docs. diff --git a/apps/sim/lib/webhooks/processor.test.ts b/apps/sim/lib/webhooks/processor.test.ts index f13125af10f..a3257514ac1 100644 --- a/apps/sim/lib/webhooks/processor.test.ts +++ b/apps/sim/lib/webhooks/processor.test.ts @@ -14,7 +14,7 @@ import { workflowsPersistenceUtilsMock, workflowsPersistenceUtilsMockFns, } from '@sim/testing' -import { NextRequest } from 'next/server' +import { NextRequest, NextResponse } from 'next/server' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' import { ADMISSION_ERROR_CODE, @@ -135,6 +135,7 @@ import { checkWebhookPreprocessing, dispatchResolvedWebhookTarget, findAllWebhooksForPath, + handleProviderChallenges, handleWebhookEventFilter, parseWebhookBody, processPolledWebhookEvent, @@ -773,3 +774,63 @@ describe('parseWebhookBody', () => { expect((response as Response).status).toBe(400) }) }) + +describe('handleProviderChallenges method gating', () => { + /** + * `getProviderHandler` is mocked module-wide here, so every provider in the challenge list + * resolves to the same stub. That is what this test wants: the behavior under test is the gate + * in `handleProviderChallenges`, not any one provider's matching logic. + */ + const challenge = (method: string, challengeMethods?: readonly string[]) => { + const handleChallenge = vi.fn(() => new NextResponse('answered', { status: 200 })) + mockProviderHandler.current = challengeMethods + ? { handleChallenge, challengeMethods } + : { handleChallenge } + + return { + handleChallenge, + response: handleProviderChallenges( + {}, + new NextRequest('http://localhost:3000/api/webhooks/trigger/abc', { method }), + 'req-1', + 'abc' + ), + } + } + + it('runs a handler that declares nothing on POST', async () => { + const { handleChallenge, response } = challenge('POST') + + expect((await response)?.status).toBe(200) + expect(handleChallenge).toHaveBeenCalledOnce() + }) + + /** + * The regression this gate exists for: a challenge handler matching on payload shape alone runs + * before the webhook lookup, so on a method its provider never uses it would answer a delivery + * addressed to whoever actually owns the path. + */ + it.each(['GET', 'PUT', 'PATCH', 'DELETE'])( + 'does not run a handler that declares nothing on a %s delivery', + async (method) => { + const { handleChallenge, response } = challenge(method) + + await expect(response).resolves.toBeNull() + expect(handleChallenge).not.toHaveBeenCalled() + } + ) + + it('runs a handler on a method it declares', async () => { + const { handleChallenge, response } = challenge('GET', ['GET', 'POST']) + + expect((await response)?.status).toBe(200) + expect(handleChallenge).toHaveBeenCalledOnce() + }) + + it('does not run a declaring handler on a method outside its list', async () => { + const { handleChallenge, response } = challenge('DELETE', ['GET', 'POST']) + + await expect(response).resolves.toBeNull() + expect(handleChallenge).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/webhooks/processor.ts b/apps/sim/lib/webhooks/processor.ts index d11e9a03e85..ca8c6694404 100644 --- a/apps/sim/lib/webhooks/processor.ts +++ b/apps/sim/lib/webhooks/processor.ts @@ -166,6 +166,8 @@ export async function parseWebhookBody( /** Providers that implement challenge/verification handling, checked before webhook lookup. */ const CHALLENGE_PROVIDERS = ['monday', 'slack', 'microsoft-teams', 'whatsapp', 'zoom'] as const +const DEFAULT_CHALLENGE_METHODS = ['POST'] as const + export async function handleProviderChallenges( body: unknown, request: NextRequest, @@ -175,11 +177,19 @@ export async function handleProviderChallenges( ): Promise { for (const provider of CHALLENGE_PROVIDERS) { const handler = getProviderHandler(provider) - if (handler.handleChallenge) { - const response = await handler.handleChallenge(body, request, requestId, path, rawBody) - if (response) { - return response - } + if (!handler.handleChallenge) continue + + /** + * Challenge handlers run before the webhook lookup and match on payload shape alone, so one + * that answers on a method its provider never uses will intercept another provider's + * delivery to the same path. `POST` is the default because every handshake but Meta's is one. + */ + const allowedMethods = handler.challengeMethods ?? DEFAULT_CHALLENGE_METHODS + if (!allowedMethods.includes(request.method)) continue + + const response = await handler.handleChallenge(body, request, requestId, path, rawBody) + if (response) { + return response } } return null diff --git a/apps/sim/lib/webhooks/providers/generic.test.ts b/apps/sim/lib/webhooks/providers/generic.test.ts index 1a6b25da6a4..2dbc6d45750 100644 --- a/apps/sim/lib/webhooks/providers/generic.test.ts +++ b/apps/sim/lib/webhooks/providers/generic.test.ts @@ -5,147 +5,258 @@ import { describe, expect, it } from 'vitest' import { genericHandler } from '@/lib/webhooks/providers/generic' import type { FormatInputContext } from '@/lib/webhooks/providers/types' +interface ContextOptions { + headers?: Record + secretHeaderName?: string + token?: string + method?: string + /** `acceptOtherMethods` — gates the `method` key and the non-POST route methods. */ + acceptOtherMethods?: boolean + /** `exposeRequestHeaders` — gates the `headers` key. */ + exposeRequestHeaders?: boolean +} + function context( body: unknown, query: Record, - options: { headers?: Record; secretHeaderName?: string; method?: string } = {} + options: ContextOptions = {} ): FormatInputContext { return { webhook: { id: 'webhook-id', provider: 'generic', - providerConfig: options.secretHeaderName - ? { secretHeaderName: options.secretHeaderName } - : {}, + providerConfig: { + ...(options.secretHeaderName ? { secretHeaderName: options.secretHeaderName } : {}), + ...(options.token ? { token: options.token } : {}), + ...(options.acceptOtherMethods ? { acceptOtherMethods: true } : {}), + ...(options.exposeRequestHeaders ? { exposeRequestHeaders: true } : {}), + }, }, workflow: { id: 'workflow-id', userId: 'user-id' }, body, headers: options.headers ?? {}, query, - method: options.method ?? '', + method: options.method ?? 'POST', 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' }) +const format = (...args: Parameters) => + genericHandler.formatInput!(context(...args)) + +describe('genericHandler.formatInput defaults', () => { + /** + * The compatibility guarantee for every webhook deployed before this feature existed: no flags + * in `providerConfig`, so a POST resolves to exactly the body it always did. + */ + it('passes a POST body through untouched when no flag is set', async () => { + const result = await format( + { event: 'test' }, + {}, + { headers: { 'x-event-name': 'created', authorization: 'Bearer secret' } } ) - expect(result?.input).toEqual({ + expect(result.input).toEqual({ event: 'test' }) + }) + + it('withholds "method" until the webhook accepts more than POST', async () => { + const result = await format({ event: 'test' }, {}, { method: 'POST' }) + + expect(result.input).not.toHaveProperty('method') + }) + + it('withholds "headers" until the webhook opts in', async () => { + const result = await format({}, {}, { headers: { 'x-event-name': 'created' } }) + + expect(result.input).not.toHaveProperty('headers') + }) +}) + +describe('genericHandler.formatInput query parameters', () => { + /** + * Query parameters are the one key that is not gated: they are dropped today, they only appear + * when the caller's own URL carries them, and they add nothing to a request without them. + */ + it('exposes query parameters under "query" alongside body fields', async () => { + const result = await format({ 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' })) + const result = await format({}, { srcId: '123' }) - expect(result?.input).toEqual({ query: { 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, {})) + const result = await format({ event: 'test' }, {}) + + expect(result.input).toEqual({ event: 'test' }) + }) +}) + +describe('genericHandler.formatInput body precedence', () => { + it.each([ + ['query', { query: 'user typed this' }, { srcId: '123' }, {}], + ['headers', { headers: 'user typed this' }, {}, { exposeRequestHeaders: true }], + ['method', { method: 'user typed this' }, {}, { acceptOtherMethods: true, method: 'PUT' }], + ])('keeps a body field named "%s" instead of overwriting it', async (_key, body, query, opts) => { + const result = await format(body, query, { + ...(opts as ContextOptions), + headers: { 'x-event-name': 'created' }, + }) - expect(result?.input).toEqual(body) - expect(result?.input).not.toHaveProperty('query') + expect(result.input).toEqual(body) }) - 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' })) + /** + * `Object.hasOwn`, not `in` — otherwise an inherited key would read as a collision and the + * metadata would silently vanish. + */ + it('does not treat an inherited property name as a body field', async () => { + const body = Object.create({ query: 'from the prototype' }) + body.event = 'test' - expect(result?.input).toEqual(body) + const result = await format(body, { srcId: '123' }) + + expect(result.input).toMatchObject({ event: 'test', query: { srcId: '123' } }) }) it('leaves non-object bodies untouched', async () => { const body = [{ event: 'a' }] - const result = await genericHandler.formatInput?.(context(body, { srcId: '123' })) + const result = await format(body, { srcId: '123' }) - expect(result?.input).toEqual(body) + expect(result.input).toEqual(body) }) +}) + +describe('genericHandler.formatInput exposed headers', () => { + const withHeaders = (headers: Record, options: ContextOptions = {}) => + format({}, {}, { ...options, headers, exposeRequestHeaders: true }) it('exposes request headers under "headers" with lowercased names', async () => { - const result = await genericHandler.formatInput?.( - context({ event: 'test' }, {}, { headers: { 'X-Event-Name': 'created' } }) - ) + const result = await withHeaders({ 'X-Event-Name': 'created' }) - expect(result?.input).toEqual({ - event: 'test', - headers: { 'x-event-name': 'created' }, - }) + expect(result.input).toEqual({ 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', - }, - } - ) - ) + it.each([ + 'authorization', + 'authentication', + 'proxy-authorization', + 'cookie', + 'set-cookie', + 'api-key', + 'apikey', + 'x-api-key', + 'x-api-token', + 'x-auth-token', + 'x-auth-key', + 'x-access-token', + 'x-secret-key', + 'x-functions-key', + 'x-amz-security-token', + 'x-goog-api-key', + 'x-csrf-token', + 'x-sim-idempotency-key', + ])('withholds the credential header %s', async (name) => { + const result = await withHeaders({ [name]: 'secret', 'x-event-name': 'created' }) - expect(result?.input).toEqual({ headers: { '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', - } - ) + const result = await withHeaders( + { 'X-Secret-Key': 'secret', 'x-event-name': 'created' }, + { secretHeaderName: 'X-Secret-Key' } ) - expect(result?.input).toEqual({ headers: { 'x-event-name': 'created' } }) + 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' } }) + /** + * The denylist is leaky by construction, so the token is withheld by value too: a sender that + * repeats it under a header name nobody anticipated is the case a name list cannot cover. + */ + it('withholds any header carrying the configured token, whatever it is named', async () => { + const result = await withHeaders( + { 'x-vendor-signature': 'not-a-real-token-fixture', 'x-event-name': 'created' }, + { token: 'not-a-real-token-fixture' } ) - expect(result?.input).toEqual(body) + expect(result.input).toEqual({ headers: { 'x-event-name': 'created' } }) + }) + + it('withholds a header that merely embeds the token', async () => { + const result = await withHeaders( + { 'x-vendor-auth': 'Token not-a-real-token-fixture' }, + { + token: 'not-a-real-token-fixture', + } + ) + + expect(result.input).not.toHaveProperty('headers') + }) + + /** + * A short token would match unrelated header values and quietly strip useful headers, so value + * matching only applies above a length where a collision stops being plausible. + */ + it('does not value-match a token too short to be distinctive', async () => { + const result = await withHeaders({ 'x-region': 'us' }, { token: 'us' }) + + expect(result.input).toEqual({ headers: { 'x-region': 'us' } }) }) }) describe('genericHandler delivery methods', () => { - it('opts into GET, PUT, PATCH and DELETE deliveries', () => { - expect(genericHandler.extraDeliveryMethods).toEqual(['GET', 'PUT', 'PATCH', 'DELETE']) + it('declares the extra methods and the flag that unlocks them', () => { + expect(genericHandler.extraDeliveryMethods).toEqual({ + methods: ['GET', 'PUT', 'PATCH', 'DELETE'], + enabledBy: 'acceptOtherMethods', + }) }) - it('exposes the request method under "method"', async () => { - const result = await genericHandler.formatInput?.( - context({ event: 'test' }, {}, { method: 'DELETE' }) + it('exposes the request method once the webhook accepts more than POST', async () => { + const result = await format( + { event: 'test' }, + {}, + { method: 'DELETE', acceptOtherMethods: true } ) - expect(result?.input).toEqual({ 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' }, {})) + const result = await format({ event: 'test' }, {}, { method: '', acceptOtherMethods: true }) - expect(result?.input).not.toHaveProperty('method') + 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' })) + /** + * The editor writes booleans, but a YAML- or Copilot-authored workflow can write the string + * `'false'`, which is truthy. Reading that as "on" would silently ship the opposite of the + * setting the user sees. + */ + it('treats a stringified "false" flag as off', async () => { + const ctx = context({ event: 'test' }, {}, { method: 'DELETE' }) + ;(ctx.webhook.providerConfig as Record).acceptOtherMethods = 'false' + + const result = await genericHandler.formatInput!(ctx) + + expect(result.input).not.toHaveProperty('method') + }) + + it('treats a stringified "true" flag as on', async () => { + const ctx = context({ event: 'test' }, {}, { method: 'DELETE' }) + ;(ctx.webhook.providerConfig as Record).acceptOtherMethods = 'true' + + const result = await genericHandler.formatInput!(ctx) - expect(result?.input).toEqual(body) + expect(result.input).toEqual({ event: 'test', method: 'DELETE' }) }) }) diff --git a/apps/sim/lib/webhooks/providers/generic.ts b/apps/sim/lib/webhooks/providers/generic.ts index 487b8478c7e..a97317d6fbe 100644 --- a/apps/sim/lib/webhooks/providers/generic.ts +++ b/apps/sim/lib/webhooks/providers/generic.ts @@ -10,39 +10,83 @@ import type { ProcessFilesContext, WebhookProviderHandler, } from '@/lib/webhooks/providers/types' -import { verifyTokenAuth } from '@/lib/webhooks/providers/utils' +import { isProviderConfigFlagEnabled, verifyTokenAuth } from '@/lib/webhooks/providers/utils' const logger = createLogger('WebhookProvider:Generic') +/** + * `providerConfig` flags, set by the matching `switch` subBlocks on the generic webhook trigger. + * + * Both default to off, so every webhook deployed before these existed keeps its current behavior: + * `POST` only, and a workflow input that is exactly the request body. + */ +const ACCEPT_OTHER_METHODS_FLAG = 'acceptOtherMethods' +const EXPOSE_REQUEST_HEADERS_FLAG = 'exposeRequestHeaders' + /** * 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. + * copy the secret into execution logs and trace spans, where it outlives the request. * - * A denylist rather than an allowlist, because arbitrary custom headers being usable is the - * point of the feature. + * A denylist rather than an allowlist, because arbitrary custom headers being usable is the point + * of the feature. A denylist is leaky by construction, so it is not the only defense: the + * webhook's own token is withheld by value as well as by name, and the whole feature is off + * unless the webhook owner turns it on. */ const CREDENTIAL_HEADER_NAMES = new Set([ 'authorization', + 'authentication', 'proxy-authorization', + 'www-authenticate', + 'proxy-authenticate', 'cookie', 'set-cookie', + 'api-key', + 'apikey', 'x-api-key', + 'x-apikey', + 'x-api-token', 'x-auth-token', + 'x-auth-key', + 'x-access-token', + 'x-secret', + 'x-secret-key', + 'x-token', + 'x-functions-key', + 'x-amz-security-token', + 'x-goog-api-key', + 'x-csrf-token', + 'x-xsrf-token', 'x-sim-idempotency-key', ]) -/** Request headers for the workflow input, minus the ones that carry credentials. */ +/** Shortest token still worth matching header values against; below this, collisions dominate. */ +const MIN_TOKEN_MATCH_LENGTH = 8 + +/** + * Request headers for the workflow input, minus the ones that carry credentials. + * + * Names are matched against a fixed denylist plus the webhook's own `secretHeaderName`. Values + * are matched against the webhook's own token, which catches a sender that repeats the token in + * a header this list has never heard of — the failure mode a denylist cannot avoid on its own. + */ function exposedHeaders( headers: Record, - secretHeaderName?: string + providerConfig: Record ): Record { - const withheld = secretHeaderName?.toLowerCase() + const secretHeaderName = providerConfig.secretHeaderName + const withheldName = + typeof secretHeaderName === 'string' ? secretHeaderName.toLowerCase() : undefined + + const token = providerConfig.token + const withheldValue = + typeof token === 'string' && token.length >= MIN_TOKEN_MATCH_LENGTH ? token : undefined + const exposed: Record = {} for (const [name, value] of Object.entries(headers)) { const lowerName = name.toLowerCase() - if (CREDENTIAL_HEADER_NAMES.has(lowerName) || lowerName === withheld) continue + if (CREDENTIAL_HEADER_NAMES.has(lowerName) || lowerName === withheldName) continue + if (withheldValue !== undefined && value.includes(withheldValue)) continue exposed[lowerName] = value } @@ -52,6 +96,10 @@ function exposedHeaders( /** * 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. + * + * Both drop paths log at debug: each fires once per delivery for a webhook whose shape simply is + * that way (an array body, a body with its own `headers` field), so a warning would be a + * per-request stream about a steady state rather than a signal. */ function mergeRequestData( body: unknown, @@ -67,7 +115,7 @@ function mergeRequestData( } if (!isRecordLike(body)) { - logger.warn( + logger.debug( `[${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) } ) @@ -77,8 +125,8 @@ function mergeRequestData( const merged: Record = { ...body } for (const [key, value] of entries) { - if (key in body) { - logger.warn( + if (Object.hasOwn(body, key)) { + logger.debug( `[${requestId}] Dropping webhook ${key}: the body already defines a "${key}" field` ) continue @@ -90,7 +138,10 @@ function mergeRequestData( } export const genericHandler: WebhookProviderHandler = { - extraDeliveryMethods: ['GET', 'PUT', 'PATCH', 'DELETE'], + extraDeliveryMethods: { + methods: ['GET', 'PUT', 'PATCH', 'DELETE'], + enabledBy: ACCEPT_OTHER_METHODS_FLAG, + }, verifyAuth({ request, requestId, providerConfig }: AuthContext) { if (providerConfig.requireAuth) { @@ -163,8 +214,16 @@ export const genericHandler: WebhookProviderHandler = { }, /** - * Expose the request method, query parameters and headers under reserved `method`, `query` and - * `headers` keys alongside the body fields. + * Expose request metadata under reserved `method`, `query` and `headers` keys alongside the + * body fields. Each key appears only when it carries information the webhook owner asked for: + * + * - `query` whenever the URL has parameters, which are otherwise silently dropped — the bug + * this exists to fix. It is not gated, because it is the caller's own URL and adds nothing + * to a request that has no query string. + * - `method` only once the webhook accepts more than `POST`; before that it is the constant + * `"POST"`, so emitting it would change every existing payload to say nothing. + * - `headers` only once the webhook opts in, because they land in execution logs and trace + * spans, where they outlive the request. */ async formatInput({ body, @@ -176,13 +235,16 @@ export const genericHandler: WebhookProviderHandler = { }: FormatInputContext): Promise { const providerConfig = (webhook.providerConfig as Record | null) ?? {} + const exposesMethod = isProviderConfigFlagEnabled(providerConfig[ACCEPT_OTHER_METHODS_FLAG]) + const exposesHeaders = isProviderConfigFlagEnabled(providerConfig[EXPOSE_REQUEST_HEADERS_FLAG]) + return { input: mergeRequestData( body, { - method, + ...(exposesMethod ? { method } : {}), query, - headers: exposedHeaders(headers, providerConfig.secretHeaderName as string | undefined), + ...(exposesHeaders ? { headers: exposedHeaders(headers, providerConfig) } : {}), }, requestId ), diff --git a/apps/sim/lib/webhooks/providers/index.ts b/apps/sim/lib/webhooks/providers/index.ts index 733d8b9b00a..b39abc5e00e 100644 --- a/apps/sim/lib/webhooks/providers/index.ts +++ b/apps/sim/lib/webhooks/providers/index.ts @@ -1,6 +1,8 @@ export { getProviderHandler } from '@/lib/webhooks/providers/registry' +import { toRecord } from '@sim/utils/object' import { getProviderHandler } from '@/lib/webhooks/providers/registry' +import { isProviderConfigFlagEnabled } from '@/lib/webhooks/providers/utils' import { isInternalTriggerProvider, isPollingWebhookProvider } from '@/triggers/constants' /** @@ -30,13 +32,23 @@ export function acceptsPathWebhookDelivery(provider: string | null): boolean { } /** - * Whether a provider accepts a delivery arriving with this HTTP method. + * Whether this webhook 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. + * Every webhook accepts `POST`. Anything else needs both a provider that declares the method in + * `extraDeliveryMethods` and the webhook itself opting in through the flag that declaration + * names. Requiring both is what keeps the capability from changing the behavior of a webhook + * deployed before it existed: such a row has no flag, so it keeps answering `405`. */ -export function acceptsWebhookDeliveryMethod(provider: string | null, method: string): boolean { +export function acceptsWebhookDeliveryMethod( + provider: string | null, + method: string, + providerConfig: unknown +): boolean { if (method === 'POST') return true if (!provider) return false - return getProviderHandler(provider).extraDeliveryMethods?.includes(method) === true + + const extra = getProviderHandler(provider).extraDeliveryMethods + if (!extra?.methods.includes(method)) return false + + return isProviderConfigFlagEnabled(toRecord(providerConfig)[extra.enabledBy]) } diff --git a/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts b/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts index 19c7c424f84..30dfcf2ba20 100644 --- a/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts +++ b/apps/sim/lib/webhooks/providers/microsoft-teams.test.ts @@ -240,18 +240,13 @@ describe('microsoftTeamsHandler formatInput (outgoing webhook channelData)', () 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() - } - ) + /** + * Non-POST deliveries never reach this handler: `handleProviderChallenges` gates it to the + * default `POST`, which is asserted in `lib/webhooks/processor.test.ts`. Declaring no + * `challengeMethods` is what buys that, so pin it here. + */ + it('claims no challenge method, so it is gated to POST by default', () => { + expect(microsoftTeamsHandler.challengeMethods).toBeUndefined() + }) }) }) diff --git a/apps/sim/lib/webhooks/providers/microsoft-teams.ts b/apps/sim/lib/webhooks/providers/microsoft-teams.ts index 178c3963939..ccaa56eb12c 100644 --- a/apps/sim/lib/webhooks/providers/microsoft-teams.ts +++ b/apps/sim/lib/webhooks/providers/microsoft-teams.ts @@ -479,15 +479,6 @@ 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 2d69e901c92..64da7506f3c 100644 --- a/apps/sim/lib/webhooks/providers/types.ts +++ b/apps/sim/lib/webhooks/providers/types.ts @@ -104,14 +104,33 @@ 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. + * Methods this provider is *able* to accept in addition to `POST`, and the `providerConfig` + * flag each webhook must set to actually accept them. Declaring the capability is not enough: + * `acceptsWebhookDeliveryMethod` still requires the per-webhook flag, so a webhook deployed + * before the capability existed keeps answering `405` exactly as it did. + * + * 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 — which is + * why it is the webhook owner's decision rather than a platform default. */ - extraDeliveryMethods?: readonly string[] + extraDeliveryMethods?: { + methods: readonly string[] + /** `providerConfig` key whose truthy value opts this webhook into {@link methods}. */ + enabledBy: string + } + + /** + * Methods on which {@link handleChallenge} may answer. Defaults to `POST` only, because a + * challenge is a provider handshake and every provider that sends one sends it as a `POST`. + * + * The default matters for correctness, not just tidiness: challenge handlers run before the + * webhook lookup and match on shape alone, so an unrestricted handler will answer a delivery + * addressed to a different provider on the same path. Widen this only for a provider that + * genuinely handshakes on another method (Meta sends the WhatsApp verification as a `GET`). + */ + challengeMethods?: readonly string[] /** * Queue workflow execution through the configured durable backend instead of the low-latency diff --git a/apps/sim/lib/webhooks/providers/utils.ts b/apps/sim/lib/webhooks/providers/utils.ts index 1afe2ce3196..5207a79ec36 100644 --- a/apps/sim/lib/webhooks/providers/utils.ts +++ b/apps/sim/lib/webhooks/providers/utils.ts @@ -96,6 +96,17 @@ export function createHmacVerifier({ } } +/** + * Read a boolean `providerConfig` flag written by a `switch` subBlock. + * + * The editor writes real booleans, but workflows authored through YAML or the Copilot can put + * the string `'false'` there, which is truthy. Anything other than an explicit on-value reads as + * off, so a flag that gates new behavior stays off for every webhook deployed before it existed. + */ +export function isProviderConfigFlagEnabled(value: unknown): boolean { + return value === true || value === 'true' +} + /** * Verify a bearer token or custom header token using timing-safe comparison. * Used by generic webhooks, Google Forms, and the default handler. diff --git a/apps/sim/lib/webhooks/providers/whatsapp.test.ts b/apps/sim/lib/webhooks/providers/whatsapp.test.ts index e6f373e3727..67a50c98d9f 100644 --- a/apps/sim/lib/webhooks/providers/whatsapp.test.ts +++ b/apps/sim/lib/webhooks/providers/whatsapp.test.ts @@ -313,3 +313,14 @@ describe('WhatsApp webhook provider', () => { expect(input.mediaId).toBeUndefined() }) }) + +describe('whatsappHandler challenge methods', () => { + /** + * Meta sends the verification handshake as a GET, so WhatsApp is the one provider that must + * widen past the POST-only default in `handleProviderChallenges`. Losing this declaration + * would break verification silently, at setup time only. + */ + it('declares GET so Meta can verify the URL', () => { + expect(whatsappHandler.challengeMethods).toEqual(['GET', 'POST']) + }) +}) diff --git a/apps/sim/lib/webhooks/providers/whatsapp.ts b/apps/sim/lib/webhooks/providers/whatsapp.ts index a2402d69590..5ab60964565 100644 --- a/apps/sim/lib/webhooks/providers/whatsapp.ts +++ b/apps/sim/lib/webhooks/providers/whatsapp.ts @@ -243,6 +243,12 @@ async function handleWhatsAppVerification( } export const whatsappHandler: WebhookProviderHandler = { + /** + * Meta sends the WhatsApp verification handshake as a `GET` with `hub.*` query parameters, so + * this is the one challenge handler that must answer outside `POST`. + */ + challengeMethods: ['GET', 'POST'], + verifyAuth({ request, rawBody, requestId, providerConfig }) { const appSecret = providerConfig.appSecret as string | undefined if (!appSecret) { diff --git a/apps/sim/triggers/generic/webhook.test.ts b/apps/sim/triggers/generic/webhook.test.ts index 61df54095bf..4bc93792e7f 100644 --- a/apps/sim/triggers/generic/webhook.test.ts +++ b/apps/sim/triggers/generic/webhook.test.ts @@ -4,11 +4,12 @@ import { describe, expect, it } from 'vitest' import { genericWebhookTrigger } from '@/triggers/generic/webhook' +function subBlock(id: string) { + return genericWebhookTrigger.subBlocks.find((entry) => entry.id === id) +} + function setupInstructions(): string { - return String( - genericWebhookTrigger.subBlocks.find((subBlock) => subBlock.id === 'triggerInstructions') - ?.defaultValue - ) + return String(subBlock('triggerInstructions')?.defaultValue) } describe('genericWebhookTrigger', () => { @@ -19,16 +20,76 @@ describe('genericWebhookTrigger', () => { 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') + /** + * The default is the compatibility contract: an existing webhook has neither key in its + * `providerConfig`, and a newly created one must start in the same state rather than silently + * opting every new webhook into replayable GET deliveries and headers in execution logs. + */ + it.each(['acceptOtherMethods', 'exposeRequestHeaders'])('ships %s off by default', (id) => { + const field = subBlock(id) + + expect(field?.type).toBe('switch') + expect(field?.defaultValue).toBe(false) }) - it('names every reserved key the input carries', () => { + it('describes POST as the accepted method and names the switch that widens it', () => { + const instructions = setupInstructions() + + expect(instructions).toContain('The webhook accepts POST.') + expect(instructions).toContain('"Accept Other HTTP Methods"') + expect(instructions).toContain('GET, PUT, PATCH and DELETE') + }) + + it('names every reserved key the input can carry', () => { const instructions = setupInstructions() for (const key of Object.keys(genericWebhookTrigger.outputs)) { expect(instructions).toContain(`"${key}"`) } - expect(instructions).toContain('Headers that carry credentials are withheld.') + }) + + it('names the switch that exposes headers rather than promising them', () => { + expect(setupInstructions()).toContain('"Expose Request Headers"') + }) + + /** + * Two of the three outputs only exist once a switch is on, so they are conditioned on it: the + * reference dropdown must not offer a field the running webhook will not send. + */ + it.each([ + ['method', 'acceptOtherMethods'], + ['headers', 'exposeRequestHeaders'], + ])('gates the %s output on the switch that produces it', (key, field) => { + expect(genericWebhookTrigger.outputs[key].condition).toEqual({ + field, + value: [true, 'true'], + }) + }) + + /** + * Query parameters are the one key that is not opt-in, so offering them unconditionally is + * correct — gating them on a switch that does not exist would hide them entirely. + */ + it('offers query unconditionally', () => { + expect(genericWebhookTrigger.outputs.query.condition).toBeUndefined() + }) + + /** + * Auth is header-based, so a plain link cannot carry it. Saying so is the difference between a + * user disabling auth knowingly and discovering it after publishing an open trigger URL. + */ + it('warns that authentication cannot be used with a plain link', () => { + expect(setupInstructions()).toContain('cannot be used with a plain link') + }) + + /** + * The switch accepts four named methods, not every method — HEAD and OPTIONS still answer 405. + * A title claiming "all" would be the same kind of overstatement this trigger exists to remove. + */ + it('does not claim to accept methods it rejects', () => { + const field = subBlock('acceptOtherMethods') + + expect(field?.title).not.toContain('All') + expect(field?.description).toContain('GET, PUT, PATCH and DELETE') }) }) diff --git a/apps/sim/triggers/generic/webhook.ts b/apps/sim/triggers/generic/webhook.ts index e34d20f0e6f..8136e36bb2f 100644 --- a/apps/sim/triggers/generic/webhook.ts +++ b/apps/sim/triggers/generic/webhook.ts @@ -50,6 +50,24 @@ export const genericWebhookTrigger: TriggerConfig = { required: false, mode: 'trigger', }, + { + id: 'acceptOtherMethods', + title: 'Accept Other HTTP Methods', + type: 'switch', + description: + 'Also accept GET, PUT, PATCH and DELETE — no others — and expose the method under "method". Leave off unless you need it: a GET URL can be replayed by link prefetchers and scanners, and a request with no body cannot be deduplicated.', + defaultValue: false, + mode: 'trigger', + }, + { + id: 'exposeRequestHeaders', + title: 'Expose Request Headers', + type: 'switch', + description: + 'Make the request headers available under "headers". Headers that carry credentials are withheld. Leave off unless you need it: exposed headers are stored in execution logs and trace spans, where they outlive the request.', + defaultValue: false, + mode: 'trigger', + }, { id: 'idempotencyField', title: 'Deduplication Field (Optional)', @@ -118,9 +136,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 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.', + 'The webhook accepts POST. Turn on "Accept Other HTTP Methods" to also accept GET, PUT, PATCH and DELETE — for example to trigger the workflow from a link in an email.', + 'Body fields are available in your workflow, and URL query parameters under "query" (for example "query.id"). Turn on "Expose Request Headers" to also get "headers" (for example "headers.x-event-name"), and "Accept Other HTTP Methods" to also get "method".', + 'Authentication is header-based, so it cannot be used with a plain link. 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 one is accepted, not either.', '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.', ] @@ -136,19 +154,29 @@ export const genericWebhookTrigger: TriggerConfig = { /** * 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. + * + * `method` and `headers` are conditioned on the switch that produces them, so the reference + * dropdown never offers a field the running webhook will not send. Both truthy forms are + * matched because a YAML- or Copilot-authored workflow can write the string rather than the + * boolean — the same tolerance `isProviderConfigFlagEnabled` applies at delivery time. */ outputs: { method: { type: 'string', - description: 'HTTP method of the request (GET, POST, PUT, PATCH or DELETE)', + description: + 'HTTP method of the request. Yields to a body field of the same name if the caller sends one.', + condition: { field: 'acceptOtherMethods', value: [true, 'true'] }, }, query: { type: 'object', - description: 'Query parameters from the request URL', + description: + 'Query parameters from the request URL, when it has any. Yields to a body field of the same name if the caller sends one.', }, headers: { type: 'object', - description: 'Request headers, excluding the ones that carry credentials', + description: + 'Request headers, excluding the ones that carry credentials. Yields to a body field of the same name if the caller sends one.', + condition: { field: 'exposeRequestHeaders', value: [true, 'true'] }, }, },