Skip to content

Commit d117b81

Browse files
committed
fix(webhooks): name the methods switch for what it actually accepts
"Accept All HTTP Methods" was not true: HEAD and OPTIONS still answer 405, so a browser preflight or a HEAD probe fails against a webhook whose panel says every method is accepted. Overstating what the endpoint does is the bug this whole change set exists to remove, so the switch should not reintroduce it. Renames it to "Accept Other HTTP Methods", says "no others" in the description, and renames the flag to match. Nothing has shipped under the old key, so no saved state is orphaned.
1 parent ad663a9 commit d117b81

5 files changed

Lines changed: 39 additions & 24 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,7 @@ describe('Webhook Trigger API Route', () => {
748748
provider: 'generic',
749749
path: 'get-path',
750750
isActive: true,
751-
providerConfig: { requireAuth: false, acceptAllMethods: true },
751+
providerConfig: { requireAuth: false, acceptOtherMethods: true },
752752
workflowId: 'test-workflow-id',
753753
})
754754

@@ -803,7 +803,7 @@ describe('Webhook Trigger API Route', () => {
803803
provider: 'generic',
804804
path: 'head-path',
805805
isActive: true,
806-
providerConfig: { requireAuth: false, acceptAllMethods: true },
806+
providerConfig: { requireAuth: false, acceptOtherMethods: true },
807807
workflowId: 'test-workflow-id',
808808
})
809809

@@ -855,7 +855,7 @@ describe('Webhook Trigger API Route', () => {
855855
provider: 'generic',
856856
path: 'any-method-path',
857857
isActive: true,
858-
providerConfig: { requireAuth: false, acceptAllMethods: true },
858+
providerConfig: { requireAuth: false, acceptOtherMethods: true },
859859
workflowId: 'test-workflow-id',
860860
})
861861

apps/sim/lib/webhooks/providers/generic.test.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ interface ContextOptions {
1010
secretHeaderName?: string
1111
token?: string
1212
method?: string
13-
/** `acceptAllMethods` — gates the `method` key and the non-POST route methods. */
14-
acceptAllMethods?: boolean
13+
/** `acceptOtherMethods` — gates the `method` key and the non-POST route methods. */
14+
acceptOtherMethods?: boolean
1515
/** `exposeRequestHeaders` — gates the `headers` key. */
1616
exposeRequestHeaders?: boolean
1717
}
@@ -28,7 +28,7 @@ function context(
2828
providerConfig: {
2929
...(options.secretHeaderName ? { secretHeaderName: options.secretHeaderName } : {}),
3030
...(options.token ? { token: options.token } : {}),
31-
...(options.acceptAllMethods ? { acceptAllMethods: true } : {}),
31+
...(options.acceptOtherMethods ? { acceptOtherMethods: true } : {}),
3232
...(options.exposeRequestHeaders ? { exposeRequestHeaders: true } : {}),
3333
},
3434
},
@@ -103,7 +103,7 @@ describe('genericHandler.formatInput body precedence', () => {
103103
it.each([
104104
['query', { query: 'user typed this' }, { srcId: '123' }, {}],
105105
['headers', { headers: 'user typed this' }, {}, { exposeRequestHeaders: true }],
106-
['method', { method: 'user typed this' }, {}, { acceptAllMethods: true, method: 'PUT' }],
106+
['method', { method: 'user typed this' }, {}, { acceptOtherMethods: true, method: 'PUT' }],
107107
])('keeps a body field named "%s" instead of overwriting it', async (_key, body, query, opts) => {
108108
const result = await format(body, query, {
109109
...(opts as ContextOptions),
@@ -217,18 +217,22 @@ describe('genericHandler delivery methods', () => {
217217
it('declares the extra methods and the flag that unlocks them', () => {
218218
expect(genericHandler.extraDeliveryMethods).toEqual({
219219
methods: ['GET', 'PUT', 'PATCH', 'DELETE'],
220-
enabledBy: 'acceptAllMethods',
220+
enabledBy: 'acceptOtherMethods',
221221
})
222222
})
223223

224224
it('exposes the request method once the webhook accepts more than POST', async () => {
225-
const result = await format({ event: 'test' }, {}, { method: 'DELETE', acceptAllMethods: true })
225+
const result = await format(
226+
{ event: 'test' },
227+
{},
228+
{ method: 'DELETE', acceptOtherMethods: true }
229+
)
226230

227231
expect(result.input).toEqual({ event: 'test', method: 'DELETE' })
228232
})
229233

230234
it('omits "method" for legacy queued jobs that carry none', async () => {
231-
const result = await format({ event: 'test' }, {}, { method: '', acceptAllMethods: true })
235+
const result = await format({ event: 'test' }, {}, { method: '', acceptOtherMethods: true })
232236

233237
expect(result.input).not.toHaveProperty('method')
234238
})
@@ -240,7 +244,7 @@ describe('genericHandler delivery methods', () => {
240244
*/
241245
it('treats a stringified "false" flag as off', async () => {
242246
const ctx = context({ event: 'test' }, {}, { method: 'DELETE' })
243-
;(ctx.webhook.providerConfig as Record<string, unknown>).acceptAllMethods = 'false'
247+
;(ctx.webhook.providerConfig as Record<string, unknown>).acceptOtherMethods = 'false'
244248

245249
const result = await genericHandler.formatInput!(ctx)
246250

@@ -249,7 +253,7 @@ describe('genericHandler delivery methods', () => {
249253

250254
it('treats a stringified "true" flag as on', async () => {
251255
const ctx = context({ event: 'test' }, {}, { method: 'DELETE' })
252-
;(ctx.webhook.providerConfig as Record<string, unknown>).acceptAllMethods = 'true'
256+
;(ctx.webhook.providerConfig as Record<string, unknown>).acceptOtherMethods = 'true'
253257

254258
const result = await genericHandler.formatInput!(ctx)
255259

apps/sim/lib/webhooks/providers/generic.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const logger = createLogger('WebhookProvider:Generic')
2020
* Both default to off, so every webhook deployed before these existed keeps its current behavior:
2121
* `POST` only, and a workflow input that is exactly the request body.
2222
*/
23-
const ACCEPT_ALL_METHODS_FLAG = 'acceptAllMethods'
23+
const ACCEPT_OTHER_METHODS_FLAG = 'acceptOtherMethods'
2424
const EXPOSE_REQUEST_HEADERS_FLAG = 'exposeRequestHeaders'
2525

2626
/**
@@ -140,7 +140,7 @@ function mergeRequestData(
140140
export const genericHandler: WebhookProviderHandler = {
141141
extraDeliveryMethods: {
142142
methods: ['GET', 'PUT', 'PATCH', 'DELETE'],
143-
enabledBy: ACCEPT_ALL_METHODS_FLAG,
143+
enabledBy: ACCEPT_OTHER_METHODS_FLAG,
144144
},
145145

146146
verifyAuth({ request, requestId, providerConfig }: AuthContext) {
@@ -235,7 +235,7 @@ export const genericHandler: WebhookProviderHandler = {
235235
}: FormatInputContext): Promise<FormatInputResult> {
236236
const providerConfig = (webhook.providerConfig as Record<string, unknown> | null) ?? {}
237237

238-
const exposesMethod = isProviderConfigFlagEnabled(providerConfig[ACCEPT_ALL_METHODS_FLAG])
238+
const exposesMethod = isProviderConfigFlagEnabled(providerConfig[ACCEPT_OTHER_METHODS_FLAG])
239239
const exposesHeaders = isProviderConfigFlagEnabled(providerConfig[EXPOSE_REQUEST_HEADERS_FLAG])
240240

241241
return {

apps/sim/triggers/generic/webhook.test.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe('genericWebhookTrigger', () => {
2525
* `providerConfig`, and a newly created one must start in the same state rather than silently
2626
* opting every new webhook into replayable GET deliveries and headers in execution logs.
2727
*/
28-
it.each(['acceptAllMethods', 'exposeRequestHeaders'])('ships %s off by default', (id) => {
28+
it.each(['acceptOtherMethods', 'exposeRequestHeaders'])('ships %s off by default', (id) => {
2929
const field = subBlock(id)
3030

3131
expect(field?.type).toBe('switch')
@@ -36,7 +36,7 @@ describe('genericWebhookTrigger', () => {
3636
const instructions = setupInstructions()
3737

3838
expect(instructions).toContain('The webhook accepts POST.')
39-
expect(instructions).toContain('"Accept All HTTP Methods"')
39+
expect(instructions).toContain('"Accept Other HTTP Methods"')
4040
expect(instructions).toContain('GET, PUT, PATCH and DELETE')
4141
})
4242

@@ -57,7 +57,7 @@ describe('genericWebhookTrigger', () => {
5757
* reference dropdown must not offer a field the running webhook will not send.
5858
*/
5959
it.each([
60-
['method', 'acceptAllMethods'],
60+
['method', 'acceptOtherMethods'],
6161
['headers', 'exposeRequestHeaders'],
6262
])('gates the %s output on the switch that produces it', (key, field) => {
6363
expect(genericWebhookTrigger.outputs[key].condition).toEqual({
@@ -81,4 +81,15 @@ describe('genericWebhookTrigger', () => {
8181
it('warns that authentication cannot be used with a plain link', () => {
8282
expect(setupInstructions()).toContain('cannot be used with a plain link')
8383
})
84+
85+
/**
86+
* The switch accepts four named methods, not every method — HEAD and OPTIONS still answer 405.
87+
* A title claiming "all" would be the same kind of overstatement this trigger exists to remove.
88+
*/
89+
it('does not claim to accept methods it rejects', () => {
90+
const field = subBlock('acceptOtherMethods')
91+
92+
expect(field?.title).not.toContain('All')
93+
expect(field?.description).toContain('GET, PUT, PATCH and DELETE')
94+
})
8495
})

apps/sim/triggers/generic/webhook.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,11 @@ export const genericWebhookTrigger: TriggerConfig = {
5151
mode: 'trigger',
5252
},
5353
{
54-
id: 'acceptAllMethods',
55-
title: 'Accept All HTTP Methods',
54+
id: 'acceptOtherMethods',
55+
title: 'Accept Other HTTP Methods',
5656
type: 'switch',
5757
description:
58-
'Also accept GET, PUT, PATCH and DELETE, 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.',
58+
'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.',
5959
defaultValue: false,
6060
mode: 'trigger',
6161
},
@@ -136,8 +136,8 @@ export const genericWebhookTrigger: TriggerConfig = {
136136
defaultValue: [
137137
'Copy the webhook URL and use it in your external service or API.',
138138
'Configure your service to send webhooks to this URL.',
139-
'The webhook accepts POST. Turn on "Accept All HTTP Methods" to also accept GET, PUT, PATCH and DELETE — for example to trigger the workflow from a link in an email.',
140-
'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 All HTTP Methods" to also get "method".',
139+
'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.',
140+
'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".',
141141
'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.',
142142
'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.',
143143
'Enable "Verify Test Events" only if the sending service needs a temporary 200 response while validating the webhook URL.',
@@ -165,7 +165,7 @@ export const genericWebhookTrigger: TriggerConfig = {
165165
type: 'string',
166166
description:
167167
'HTTP method of the request. Yields to a body field of the same name if the caller sends one.',
168-
condition: { field: 'acceptAllMethods', value: [true, 'true'] },
168+
condition: { field: 'acceptOtherMethods', value: [true, 'true'] },
169169
},
170170
query: {
171171
type: 'object',

0 commit comments

Comments
 (0)