Skip to content

Commit 8a2994a

Browse files
committed
fix(webhooks): make generic webhook request metadata opt-in per webhook
The capabilities added on top of this branch changed behavior for every generic webhook already deployed: each one began accepting GET, PUT, PATCH and DELETE, and each one's workflow input gained `method` and `headers`. The provider declared the capability, so no webhook owner chose it. Gate both behind `providerConfig` flags written by two new 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 adds nothing to a request without one. Also 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 verification handshake, rather than guarding one handler inline. Alongside: widen the credential header denylist and withhold the webhook's own token by value as well as by name; give PUT/PATCH/DELETE their own contracts instead of reusing the POST one; parse, challenge and generate a request ID once per delivery rather than twice on GET; answer every non-POST rejection with the same 405 plus `Allow`; and drop the per-delivery warn logs to debug.
1 parent ab13e20 commit 8a2994a

16 files changed

Lines changed: 715 additions & 202 deletions

File tree

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

Lines changed: 109 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ describe('Webhook Trigger API Route', () => {
690690
provider: 'generic',
691691
path: 'get-path',
692692
isActive: true,
693-
providerConfig: { requireAuth: false },
693+
providerConfig: { requireAuth: false, acceptAllMethods: true },
694694
workflowId: 'test-workflow-id',
695695
})
696696

@@ -707,6 +707,61 @@ describe('Webhook Trigger API Route', () => {
707707
expect(dispatchResolvedWebhookTargetMock).toHaveBeenCalledOnce()
708708
})
709709

710+
/**
711+
* The compatibility guarantee for the route: a generic webhook deployed before the flag
712+
* existed has no flag, so it answers exactly as it did before — 405, no execution.
713+
*/
714+
it('rejects a GET delivery to a generic webhook that has not opted in', async () => {
715+
testData.webhooks.push({
716+
id: 'generic-webhook-id',
717+
provider: 'generic',
718+
path: 'opt-out-path',
719+
isActive: true,
720+
providerConfig: { requireAuth: false },
721+
workflowId: 'test-workflow-id',
722+
})
723+
724+
const req = createMockRequest(
725+
'GET',
726+
undefined,
727+
{},
728+
'http://localhost:3000/api/webhooks/trigger/opt-out-path?srcId=123'
729+
)
730+
731+
const response = await GET(req, { params: Promise.resolve({ path: 'opt-out-path' }) })
732+
733+
expect(response.status).toBe(405)
734+
expect(response.headers.get('Allow')).toBe('POST')
735+
expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled()
736+
})
737+
738+
/**
739+
* Next derives HEAD from the exported GET, so a HEAD probe reaches the same handler. It must
740+
* not execute a workflow: scanners and prefetchers send HEAD unprompted.
741+
*/
742+
it('rejects a HEAD probe to a webhook that accepts every declared method', async () => {
743+
testData.webhooks.push({
744+
id: 'generic-webhook-id',
745+
provider: 'generic',
746+
path: 'head-path',
747+
isActive: true,
748+
providerConfig: { requireAuth: false, acceptAllMethods: true },
749+
workflowId: 'test-workflow-id',
750+
})
751+
752+
const req = createMockRequest(
753+
'HEAD',
754+
undefined,
755+
{},
756+
'http://localhost:3000/api/webhooks/trigger/head-path'
757+
)
758+
759+
const response = await GET(req, { params: Promise.resolve({ path: 'head-path' }) })
760+
761+
expect(response.status).toBe(405)
762+
expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled()
763+
})
764+
710765
it('rejects a GET delivery to a provider that only accepts POST', async () => {
711766
testData.webhooks.push({
712767
id: 'stripe-webhook-id',
@@ -742,7 +797,7 @@ describe('Webhook Trigger API Route', () => {
742797
provider: 'generic',
743798
path: 'any-method-path',
744799
isActive: true,
745-
providerConfig: { requireAuth: false },
800+
providerConfig: { requireAuth: false, acceptAllMethods: true },
746801
workflowId: 'test-workflow-id',
747802
})
748803

@@ -762,6 +817,29 @@ describe('Webhook Trigger API Route', () => {
762817
}
763818
)
764819

820+
it('rejects a PUT delivery to a generic webhook that has not opted in', async () => {
821+
testData.webhooks.push({
822+
id: 'generic-webhook-id',
823+
provider: 'generic',
824+
path: 'opt-out-path',
825+
isActive: true,
826+
providerConfig: { requireAuth: false },
827+
workflowId: 'test-workflow-id',
828+
})
829+
830+
const req = createMockRequest(
831+
'PUT',
832+
{ event: 'test' },
833+
{},
834+
'http://localhost:3000/api/webhooks/trigger/opt-out-path'
835+
)
836+
837+
const response = await PUT(req, { params: Promise.resolve({ path: 'opt-out-path' }) })
838+
839+
expect(response.status).toBe(405)
840+
expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled()
841+
})
842+
765843
it('rejects a PUT delivery to a provider that only accepts POST', async () => {
766844
testData.webhooks.push({
767845
id: 'stripe-webhook-id',
@@ -785,6 +863,35 @@ describe('Webhook Trigger API Route', () => {
785863
expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled()
786864
})
787865

866+
/**
867+
* Every non-POST rejection is the same 405, whether the path is unknown, holds only
868+
* non-path triggers, or holds a trigger that has not opted in — so a probe cannot tell
869+
* a configured path from an unused one.
870+
*/
871+
it('returns the same 405 for a DELETE to a non-path trigger as to an unknown path', async () => {
872+
testData.webhooks.push({
873+
id: 'internal-webhook-id',
874+
provider: 'sim',
875+
path: 'internal-path',
876+
isActive: true,
877+
providerConfig: {},
878+
workflowId: 'test-workflow-id',
879+
})
880+
881+
const req = createMockRequest(
882+
'DELETE',
883+
undefined,
884+
{},
885+
'http://localhost:3000/api/webhooks/trigger/internal-path'
886+
)
887+
888+
const response = await DELETE(req, { params: Promise.resolve({ path: 'internal-path' }) })
889+
890+
expect(response.status).toBe(405)
891+
expect(response.headers.get('Allow')).toBe('POST')
892+
expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled()
893+
})
894+
788895
it('returns 405 for a DELETE to an unknown path', async () => {
789896
const req = createMockRequest(
790897
'DELETE',

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

Lines changed: 74 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { createLogger } from '@sim/logger'
22
import { type NextRequest, NextResponse } from 'next/server'
3-
import { webhookTriggerGetContract, webhookTriggerPostContract } from '@/lib/api/contracts/webhooks'
3+
import {
4+
webhookTriggerDeleteContract,
5+
webhookTriggerGetContract,
6+
webhookTriggerPatchContract,
7+
webhookTriggerPostContract,
8+
webhookTriggerPutContract,
9+
} from '@/lib/api/contracts/webhooks'
410
import { parseRequest } from '@/lib/api/server'
511
import { admissionRejectedResponse, tryAdmit } from '@/lib/core/admission/gate'
612
import { generateRequestId } from '@/lib/core/utils/request'
@@ -22,71 +28,93 @@ export const dynamic = 'force-dynamic'
2228
export const runtime = 'nodejs'
2329
export const maxDuration = 60
2430

25-
export const GET = withRouteHandler(
26-
async (request: NextRequest, context: { params: Promise<{ path: string }> }) => {
31+
type RouteContext = { params: Promise<{ path: string }> }
32+
33+
type WebhookTriggerContract =
34+
| typeof webhookTriggerGetContract
35+
| typeof webhookTriggerPostContract
36+
| typeof webhookTriggerPutContract
37+
| typeof webhookTriggerPatchContract
38+
| typeof webhookTriggerDeleteContract
39+
40+
/**
41+
* Shared delivery entry point. Parses the path once, sheds load, and hands off — so a request is
42+
* parsed, challenged and looked up exactly once no matter which method it arrived on.
43+
*/
44+
function defineDeliveryRoute(
45+
contract: WebhookTriggerContract,
46+
beforeAdmission?: (
47+
request: NextRequest,
48+
requestId: string,
49+
path: string
50+
) => Promise<NextResponse | null>
51+
) {
52+
return withRouteHandler(async (request: NextRequest, context: RouteContext) => {
2753
const requestId = generateRequestId()
28-
const parsed = await parseRequest(webhookTriggerGetContract, request, context)
54+
const parsed = await parseRequest(contract, request, context)
2955
if (!parsed.success) return parsed.response
3056
const { path } = parsed.data.params
3157

32-
// Handle provider-specific GET verifications (Microsoft Graph, WhatsApp, etc.)
33-
const challengeResponse = await handleProviderChallenges({}, request, requestId, path)
34-
if (challengeResponse) {
35-
return challengeResponse
36-
}
37-
38-
const verificationResponse = await handlePreLookupWebhookVerification(
39-
request.method,
40-
undefined,
41-
requestId,
42-
path
43-
)
44-
if (verificationResponse) {
45-
return verificationResponse
46-
}
58+
const earlyResponse = await beforeAdmission?.(request, requestId, path)
59+
if (earlyResponse) return earlyResponse
4760

4861
const ticket = tryAdmit()
4962
if (!ticket) {
5063
return admissionRejectedResponse()
5164
}
5265

5366
try {
54-
return await handleWebhookDelivery(request, context, webhookTriggerGetContract)
67+
return await handleWebhookDelivery(request, requestId, path)
5568
} finally {
5669
ticket.release()
5770
}
58-
}
71+
})
72+
}
73+
74+
/**
75+
* `GET` alone runs the pre-lookup verification probe before the webhook lookup, because a
76+
* provider validating a URL it has not been given a webhook for can only be answered there.
77+
* `handleWebhookDelivery` runs the same check for every method once the lookup comes back empty.
78+
*/
79+
export const GET = defineDeliveryRoute(webhookTriggerGetContract, (request, requestId, path) =>
80+
handlePreLookupWebhookVerification(request.method, undefined, requestId, path)
5981
)
6082

61-
const handleBodyDelivery = withRouteHandler(
62-
async (request: NextRequest, context: { params: Promise<{ path: string }> }) => {
63-
const ticket = tryAdmit()
64-
if (!ticket) {
65-
return admissionRejectedResponse()
66-
}
83+
export const POST = defineDeliveryRoute(webhookTriggerPostContract)
6784

68-
try {
69-
return await handleWebhookDelivery(request, context, webhookTriggerPostContract)
70-
} finally {
71-
ticket.release()
72-
}
73-
}
74-
)
85+
/**
86+
* Accepted only by a webhook whose provider declares the method AND whose owner has opted in.
87+
* Everything else gets a 405 from `handleWebhookDelivery`.
88+
*/
89+
export const PUT = defineDeliveryRoute(webhookTriggerPutContract)
90+
export const PATCH = defineDeliveryRoute(webhookTriggerPatchContract)
91+
export const DELETE = defineDeliveryRoute(webhookTriggerDeleteContract)
7592

76-
export const POST = handleBodyDelivery
93+
/**
94+
* A 405 body carries `Allow` per RFC 9110. Every rejection here allows exactly `POST`: a webhook
95+
* that accepts more never reaches this path, so the header cannot be used to tell an unknown
96+
* path from a configured one.
97+
*/
98+
function methodNotAllowedResponse(): NextResponse {
99+
return new NextResponse('Method not allowed', { status: 405, headers: { Allow: 'POST' } })
100+
}
77101

78102
/**
79-
* Methods a provider must opt into via `extraDeliveryMethods`. A delivery to a path whose
80-
* triggers have not opted in gets a 405 from `handleWebhookDelivery`.
103+
* The answer for a path that will not accept this delivery. `POST` keeps its historical 404 so
104+
* existing callers see no change; anything else answers 405 uniformly, whether the path is
105+
* unknown, holds only non-path triggers, or holds a trigger that has not opted into the method —
106+
* so a probe cannot tell those apart.
81107
*/
82-
export const PUT = handleBodyDelivery
83-
export const PATCH = handleBodyDelivery
84-
export const DELETE = handleBodyDelivery
108+
function notDeliverableResponse(method: string): NextResponse {
109+
return method === 'POST'
110+
? new NextResponse('Not Found', { status: 404 })
111+
: methodNotAllowedResponse()
112+
}
85113

86114
async function handleWebhookDelivery(
87115
request: NextRequest,
88-
context: { params: Promise<{ path: string }> },
89-
contract: typeof webhookTriggerGetContract | typeof webhookTriggerPostContract
116+
requestId: string,
117+
path: string
90118
): Promise<NextResponse> {
91119
const receivedAt = Date.now()
92120
/**
@@ -99,11 +127,6 @@ async function handleWebhookDelivery(
99127
? Number(slackRequestTimestamp) * 1000
100128
: undefined
101129

102-
const requestId = generateRequestId()
103-
const parsed = await parseRequest(contract, request, context)
104-
if (!parsed.success) return parsed.response
105-
const { path } = parsed.data.params
106-
107130
const earlyChallenge = await handleProviderChallenges({}, request, requestId, path)
108131
if (earlyChallenge) {
109132
return earlyChallenge
@@ -132,18 +155,18 @@ async function handleWebhookDelivery(
132155

133156
if (allWebhooksForPath.length > 0 && pathWebhooks.length === 0) {
134157
logger.warn(`[${requestId}] Rejected HTTP delivery to non-path trigger: ${path}`)
135-
return new NextResponse('Not Found', { status: 404 })
158+
return notDeliverableResponse(request.method)
136159
}
137160

138161
const webhooksForPath = pathWebhooks.filter(({ webhook: foundWebhook }) =>
139-
acceptsWebhookDeliveryMethod(foundWebhook.provider, request.method)
162+
acceptsWebhookDeliveryMethod(foundWebhook.provider, request.method, foundWebhook.providerConfig)
140163
)
141164

142165
if (pathWebhooks.length > 0 && webhooksForPath.length === 0) {
143166
logger.warn(
144167
`[${requestId}] Rejected ${request.method} delivery to path ${path}: no trigger on this path accepts that method`
145168
)
146-
return new NextResponse('Method not allowed', { status: 405 })
169+
return methodNotAllowedResponse()
147170
}
148171

149172
if (webhooksForPath.length === 0) {
@@ -158,11 +181,7 @@ async function handleWebhookDelivery(
158181
}
159182

160183
logger.warn(`[${requestId}] Webhook or workflow not found for path: ${path}`)
161-
// Unknown paths keep answering 405 on GET so probes cannot tell an unknown path
162-
// from one whose trigger only accepts POST.
163-
return request.method === 'POST'
164-
? new NextResponse('Not Found', { status: 404 })
165-
: new NextResponse('Method not allowed', { status: 405 })
184+
return notDeliverableResponse(request.method)
166185
}
167186

168187
// Process each webhook matched on this path

apps/sim/lib/api/contracts/webhooks.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,44 @@ export const webhookTriggerPostContract = defineRouteContract({
273273
},
274274
})
275275

276+
/**
277+
* `PUT`, `PATCH` and `DELETE` deliveries. Same shape as the `POST` contract — they exist as
278+
* separate declarations rather than reusing it so each route method is described by a contract
279+
* that states its own method, which is what the boundary audit and any future client read.
280+
*/
281+
export const webhookTriggerPutContract = defineRouteContract({
282+
method: 'PUT',
283+
path: '/api/webhooks/trigger/[path]',
284+
params: webhookTriggerParamsSchema,
285+
response: {
286+
mode: 'json',
287+
// untyped-response: webhook trigger forwards arbitrary provider challenge or workflow execution payloads
288+
schema: z.unknown(),
289+
},
290+
})
291+
292+
export const webhookTriggerPatchContract = defineRouteContract({
293+
method: 'PATCH',
294+
path: '/api/webhooks/trigger/[path]',
295+
params: webhookTriggerParamsSchema,
296+
response: {
297+
mode: 'json',
298+
// untyped-response: webhook trigger forwards arbitrary provider challenge or workflow execution payloads
299+
schema: z.unknown(),
300+
},
301+
})
302+
303+
export const webhookTriggerDeleteContract = defineRouteContract({
304+
method: 'DELETE',
305+
path: '/api/webhooks/trigger/[path]',
306+
params: webhookTriggerParamsSchema,
307+
response: {
308+
mode: 'json',
309+
// untyped-response: webhook trigger forwards arbitrary provider challenge or workflow execution payloads
310+
schema: z.unknown(),
311+
},
312+
})
313+
276314
/**
277315
* TikTok app-level webhook ingress. Signature is verified from the raw body
278316
* before this schema runs; `content` remains a JSON string per TikTok docs.

0 commit comments

Comments
 (0)