Skip to content

Commit aa5ed04

Browse files
committed
fix(webhooks): fail an Ashby webhook delete that returns success:false
Ashby returns what would be a 4XX elsewhere as HTTP 200 with `success: false` — its own docs state this explicitly. `deleteSubscription` branched on `ashbyResponse.ok`, so every rejected delete logged "Successfully deleted Ashby webhook subscription <id>" and never threw in strict mode. Sim then dropped its own row while the subscription stayed live in Ashby, and since there is no `webhook.list` endpoint the orphan cannot be enumerated afterwards. Check `success` the way `createSubscription` already does, and treat `webhook_not_found` as already-removed rather than an error — that is the shape an unknown id comes back in, not a 404. An absent `success` field stays a success here, unlike on create: teardown runs on the undeploy path, and failing closed on an undocumented response shape would wedge cleanup. Also corrects two trigger-surface details against the API reference: the setup text said the webhook is created when you save the trigger (it is created on deploy), and the jobCreate `employmentType` description omitted the documented `Temporary` value.
1 parent a37dc4d commit aa5ed04

3 files changed

Lines changed: 77 additions & 14 deletions

File tree

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,56 @@ describe('ashbyHandler', () => {
194194
})
195195
})
196196

197+
describe('deleteSubscription', () => {
198+
const realFetch = globalThis.fetch
199+
afterEach(() => {
200+
globalThis.fetch = realFetch
201+
})
202+
203+
const ctx = (strict: boolean) =>
204+
({
205+
requestId: 'req-1',
206+
strict,
207+
webhook: {
208+
id: 'wh-1',
209+
providerConfig: { apiKey: 'k', externalId: 'ext-1' },
210+
},
211+
}) as never
212+
213+
const respondWith = (body: unknown, status = 200) => {
214+
globalThis.fetch = vi.fn().mockResolvedValue(
215+
new Response(JSON.stringify(body), {
216+
status,
217+
headers: { 'content-type': 'application/json' },
218+
})
219+
) as never
220+
}
221+
222+
it('treats a 200 carrying success:false as a failed delete', async () => {
223+
// Ashby returns what would be a 4XX as HTTP 200. Branching on
224+
// response.ok alone reported the leak as a successful cleanup.
225+
respondWith({ success: false, errors: [{ message: 'missing_endpoint_permission' }] })
226+
await expect(ashbyHandler.deleteSubscription?.(ctx(true))).rejects.toThrow(
227+
/missing_endpoint_permission/
228+
)
229+
})
230+
231+
it('stays non-fatal for a failed delete when not strict', async () => {
232+
respondWith({ success: false, errors: [{ message: 'missing_endpoint_permission' }] })
233+
await expect(ashbyHandler.deleteSubscription?.(ctx(false))).resolves.toBeUndefined()
234+
})
235+
236+
it('treats an already-removed webhook as done even in strict mode', async () => {
237+
respondWith({ success: false, errors: ['webhook_not_found'] })
238+
await expect(ashbyHandler.deleteSubscription?.(ctx(true))).resolves.toBeUndefined()
239+
})
240+
241+
it('accepts a successful delete', async () => {
242+
respondWith({ success: true, results: { webhookId: 'ext-1' } })
243+
await expect(ashbyHandler.deleteSubscription?.(ctx(true))).resolves.toBeUndefined()
244+
})
245+
})
246+
197247
describe('extractIdempotencyId', () => {
198248
it('derives a stable key from application id + updatedAt', () => {
199249
const body = {

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

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -340,26 +340,39 @@ export const ashbyHandler: WebhookProviderHandler = {
340340
body: JSON.stringify({ webhookId: externalId }),
341341
})
342342

343-
if (ashbyResponse.ok) {
344-
await ashbyResponse.body?.cancel()
343+
const responseBody = (await ashbyResponse.json().catch(() => ({}))) as Record<string, unknown>
344+
345+
/**
346+
* Ashby returns what would be a 4XX elsewhere as HTTP 200 with
347+
* `success: false`, so the status alone cannot separate a completed
348+
* delete from a rejected one. Branching on `ashbyResponse.ok` reported
349+
* every rejection as a successful cleanup while Sim dropped its own row
350+
* — and with no `webhook.list` endpoint, an orphan left behind that way
351+
* cannot be enumerated afterwards.
352+
*
353+
* Unlike `createSubscription`, an absent `success` field is treated as
354+
* success rather than failure: teardown runs on the undeploy path, and
355+
* failing closed on an unparseable body would wedge cleanup on a
356+
* response shape Ashby does not document.
357+
*/
358+
const rejected = !ashbyResponse.ok || responseBody.success === false
359+
const errorMessage = ashbyErrorMessage(responseBody, `HTTP ${ashbyResponse.status}`)
360+
361+
if (!rejected) {
345362
logger.info(
346363
`[${ctx.requestId}] Successfully deleted Ashby webhook subscription ${externalId}`
347364
)
348-
} else if (ashbyResponse.status === 404) {
349-
await ashbyResponse.body?.cancel()
365+
} else if (ashbyResponse.status === 404 || /webhook_not_found/i.test(errorMessage)) {
350366
logger.info(
351367
`[${ctx.requestId}] Ashby webhook ${externalId} not found during deletion (already removed)`
352368
)
353369
} else {
354-
const responseBody = await ashbyResponse.json().catch(() => ({}))
355370
logger.warn(
356-
`[${ctx.requestId}] Failed to delete Ashby webhook (non-fatal): ${ashbyResponse.status}`,
357-
{ response: responseBody }
371+
`[${ctx.requestId}] Failed to delete Ashby webhook (non-fatal): ${errorMessage}`,
372+
{ status: ashbyResponse.status, response: responseBody }
358373
)
359374
if (ctx.strict) {
360-
throw new Error(
361-
`Failed to delete Ashby webhook: ${ashbyErrorMessage(responseBody, `HTTP ${ashbyResponse.status}`)}`
362-
)
375+
throw new Error(`Failed to delete Ashby webhook: ${errorMessage}`)
363376
}
364377
}
365378
} catch (error) {

apps/sim/triggers/ashby/utils.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ export function isAshbyEventMatch(triggerId: string, action: string): boolean {
4343
*/
4444
export function ashbySetupInstructions(eventType: string): string {
4545
const instructions = [
46-
'Enter your Ashby API Key above. You can find your API key in Ashby at <strong>Settings &gt; API Keys</strong>.',
47-
`The webhook for <strong>${eventType}</strong> events will be automatically created in Ashby when you save the trigger.`,
48-
'The webhook will be automatically deleted if you remove this trigger.',
46+
'Enter your Ashby API Key above. You can find your API key in Ashby at <strong>Settings &gt; API Keys</strong>. It needs the <strong>apiKeysWrite</strong> permission.',
47+
`The webhook for <strong>${eventType}</strong> events is created in Ashby when you deploy the workflow, not when you save the trigger.`,
48+
'The webhook is deleted from Ashby when you remove this trigger and redeploy.',
4949
]
5050

5151
return instructions
@@ -275,7 +275,7 @@ export function buildJobCreateOutputs(): Record<string, TriggerOutput> {
275275
status: { type: 'string', description: 'Job status (Open, Closed, Draft, Archived)' },
276276
employmentType: {
277277
type: 'string',
278-
description: 'Employment type (FullTime, PartTime, Intern, Contract)',
278+
description: 'Employment type (FullTime, PartTime, Intern, Contract, Temporary)',
279279
},
280280
},
281281
} as Record<string, TriggerOutput>

0 commit comments

Comments
 (0)