Skip to content

Commit b46787b

Browse files
committed
fix(granola): never recover an orphaned endpoint by callback URL
The previous commit's URL-based recovery was unsafe. A redeploy reuses the live registration's `path`, so the candidate and the currently serving endpoint share a callback URL — listing by that URL and deleting every match would remove the live deployment's endpoint and silently stop a working trigger, which is worse than the leak it was trying to prevent. Cleanup is now keyed solely on the id Granola returned. When the success body carries no id there is no way to tell the candidate's endpoint from the live one, so it is left in place: a leaked endpoint produces unverifiable deliveries that Granola disables on its own, whereas deleting the wrong one takes down live traffic with no signal. The 2xx-missing-signing-secret case this originally fixed still cleans up, since that response does carry an id. Adds a test asserting no lookup or delete is attempted when the response has no id, so URL matching cannot be reintroduced unnoticed.
1 parent 7f6c4e6 commit b46787b

2 files changed

Lines changed: 39 additions & 130 deletions

File tree

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

Lines changed: 10 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import crypto from 'node:crypto'
22
import { NextRequest } from 'next/server'
33
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
4-
import { getNotificationUrl } from '@/lib/webhooks/provider-subscription-utils'
54
import { granolaHandler } from '@/lib/webhooks/providers/granola'
65

76
const SECRET_BYTES = Buffer.from('granola-test-secret-key-padding!!!!!')
@@ -314,29 +313,19 @@ describe('Granola webhook provider', () => {
314313
expect(url).toBe('https://public-api.granola.ai/v1/webhook-endpoints/whe_orphan')
315314
})
316315

317-
it('recovers the endpoint by URL when the response body is unusable', async () => {
316+
it('leaves the endpoint alone when the response carries no id', async () => {
317+
/**
318+
* A redeploy reuses the live registration's path, so the candidate and the currently
319+
* serving endpoint share a callback URL. Recovering by URL would delete the live
320+
* deployment's endpoint and kill a working trigger, so with no id there is nothing safe
321+
* to do — leaking beats taking down live traffic. This asserts no lookup or delete is
322+
* attempted, guarding against reintroducing URL matching.
323+
*/
318324
fetchMock.mockResolvedValueOnce({
319325
ok: true,
320326
status: 201,
321-
json: async () => {
322-
throw new Error('invalid json')
323-
},
324-
})
325-
fetchMock.mockResolvedValueOnce({
326-
ok: true,
327-
status: 200,
328-
json: async () => ({
329-
webhook_endpoints: [
330-
{ id: 'whe_other', url: 'https://other.test/hook', url_redacted: false },
331-
{
332-
id: 'whe_mine',
333-
url: getNotificationUrl({ path: 'p9' }),
334-
url_redacted: false,
335-
},
336-
],
337-
}),
327+
json: async () => ({}),
338328
})
339-
fetchMock.mockResolvedValueOnce({ ok: true, status: 200 })
340329

341330
await expect(
342331
granolaHandler.createSubscription!({
@@ -345,41 +334,7 @@ describe('Granola webhook provider', () => {
345334
} as never)
346335
).rejects.toThrow()
347336

348-
const deleteCall = fetchMock.mock.calls.find(([, init]) => init?.method === 'DELETE')
349-
expect(deleteCall?.[0]).toContain('whe_mine')
350-
expect(deleteCall?.[0]).not.toContain('whe_other')
351-
})
352-
353-
it('never deletes an endpoint whose URL was redacted to its origin', async () => {
354-
/* A redacted url is only an origin, so matching on it could delete another workflow's
355-
endpoint that happens to share the host. */
356-
fetchMock.mockResolvedValueOnce({
357-
ok: true,
358-
status: 201,
359-
json: async () => ({}),
360-
})
361-
fetchMock.mockResolvedValueOnce({
362-
ok: true,
363-
status: 200,
364-
json: async () => ({
365-
webhook_endpoints: [
366-
{
367-
id: 'whe_redacted',
368-
url: getNotificationUrl({ path: 'p10' }),
369-
url_redacted: true,
370-
},
371-
],
372-
}),
373-
})
374-
375-
await expect(
376-
granolaHandler.createSubscription!({
377-
webhook: { id: 'wh_10', path: 'p10', providerConfig: { apiKey: 'grn_key' } },
378-
requestId: 'granola-orphan3',
379-
} as never)
380-
).rejects.toThrow()
381-
382-
expect(fetchMock.mock.calls.some(([, init]) => init?.method === 'DELETE')).toBe(false)
337+
expect(fetchMock).toHaveBeenCalledTimes(1)
383338
})
384339

385340
it('does not attempt cleanup when Granola rejected the request outright', async () => {

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

Lines changed: 29 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -130,69 +130,40 @@ async function deleteGranolaEndpoint(apiKey: string, endpointId: string): Promis
130130
}
131131
}
132132

133-
/**
134-
* Find endpoints Granola is delivering to `url`.
135-
*
136-
* Used only to recover an endpoint whose id we never learned. Endpoints whose
137-
* `url` was redacted to its origin are skipped: the comparison would match any
138-
* endpoint sharing that origin, and deleting one of those could take down an
139-
* unrelated workflow's trigger.
140-
*/
141-
async function findGranolaEndpointIdsByUrl(apiKey: string, url: string): Promise<string[]> {
142-
const response = await fetch(GRANOLA_WEBHOOK_ENDPOINTS_URL, {
143-
method: 'GET',
144-
headers: {
145-
Authorization: `Bearer ${apiKey}`,
146-
'Content-Type': 'application/json',
147-
},
148-
})
149-
150-
if (!response.ok) return []
151-
152-
const body = (await response.json().catch(() => ({}))) as {
153-
webhook_endpoints?: { id?: string; url?: string; url_redacted?: boolean }[]
154-
}
155-
156-
return (body.webhook_endpoints ?? [])
157-
.filter(
158-
(endpoint) =>
159-
endpoint.url_redacted !== true && endpoint.url === url && typeof endpoint.id === 'string'
160-
)
161-
.map((endpoint) => endpoint.id as string)
162-
}
163-
164133
/**
165134
* Remove an endpoint Granola created for a registration that then failed.
166135
*
167136
* The registration service only rolls external state back when
168137
* `createSubscription` *returns*; a handler that throws is assumed to have left
169-
* nothing behind (`registration-service.ts` guards its rollback on
138+
* nothing behind (`prepareStableWebhookCandidate` guards its rollback on
170139
* `preparedProviderConfig`). So anything already created here has to be undone
171-
* here, or the endpoint stays live with no external id recorded — delivering to
172-
* a path whose signature can never be verified, and duplicating on every retry.
140+
* here, or the endpoint stays live with no external id recorded and keeps
141+
* delivering to a callback whose signature can never be verified.
142+
*
143+
* Deliberately keyed on the id Granola returned, and nothing else. A redeploy
144+
* reuses the live registration's `path`, so the candidate and the currently
145+
* serving endpoint share a callback URL — recovering "our" endpoint by matching
146+
* that URL would delete the live deployment's endpoint and silently kill a
147+
* working trigger. When Granola's response carries no id there is no way to
148+
* tell the two apart, so the endpoint is left in place: a leaked endpoint
149+
* produces unverifiable deliveries that Granola eventually disables, which is
150+
* far less harmful than taking down live traffic.
173151
*
174152
* Best effort by design: it never throws, because the caller is already
175153
* throwing the failure that matters.
176154
*/
177155
async function cleanupOrphanedGranolaEndpoint(params: {
178156
apiKey: string
179-
endpointId: string | undefined
180-
notificationUrl: string
157+
endpointId: string
181158
requestId: string
182159
}): Promise<void> {
183-
const { apiKey, endpointId, notificationUrl, requestId } = params
160+
const { apiKey, endpointId, requestId } = params
184161
try {
185-
const endpointIds = endpointId
186-
? [endpointId]
187-
: await findGranolaEndpointIdsByUrl(apiKey, notificationUrl)
188-
189-
for (const id of endpointIds) {
190-
await deleteGranolaEndpoint(apiKey, id)
191-
logger.info(`[${requestId}] Removed orphaned Granola webhook endpoint ${id}`)
192-
}
162+
await deleteGranolaEndpoint(apiKey, endpointId)
163+
logger.info(`[${requestId}] Removed orphaned Granola webhook endpoint ${endpointId}`)
193164
} catch (error) {
194165
logger.error(
195-
`[${requestId}] Failed to remove an orphaned Granola webhook endpoint; it may still be active`,
166+
`[${requestId}] Failed to remove orphaned Granola webhook endpoint ${endpointId}; it may still be active`,
196167
error
197168
)
198169
}
@@ -303,9 +274,8 @@ export const granolaHandler: WebhookProviderHandler = {
303274
const folderIds = parseList(providerConfig.folderIds)
304275
const events = [...(GRANOLA_TRIGGER_TO_EVENT_TYPES[triggerId ?? ''] ?? [])]
305276

306-
const notificationUrl = getNotificationUrl(webhook)
307277
const requestBody: Record<string, unknown> = {
308-
url: notificationUrl,
278+
url: getNotificationUrl(webhook),
309279
scopes: scopes.length > 0 ? scopes : DEFAULT_GRANOLA_SCOPES,
310280
}
311281
if (events.length > 0) requestBody.events = events
@@ -319,27 +289,14 @@ export const granolaHandler: WebhookProviderHandler = {
319289
webhookId: webhook.id,
320290
})
321291

322-
let response: Response
323-
try {
324-
response = await fetch(GRANOLA_WEBHOOK_ENDPOINTS_URL, {
325-
method: 'POST',
326-
headers: {
327-
Authorization: `Bearer ${apiKey}`,
328-
'Content-Type': 'application/json',
329-
},
330-
body: JSON.stringify(requestBody),
331-
})
332-
} catch (error) {
333-
/* The request may have reached Granola and created an endpoint before the
334-
connection failed, so look one up by URL rather than assume it did not. */
335-
await cleanupOrphanedGranolaEndpoint({
336-
apiKey,
337-
endpointId: undefined,
338-
notificationUrl,
339-
requestId,
340-
})
341-
throw error
342-
}
292+
const response = await fetch(GRANOLA_WEBHOOK_ENDPOINTS_URL, {
293+
method: 'POST',
294+
headers: {
295+
Authorization: `Bearer ${apiKey}`,
296+
'Content-Type': 'application/json',
297+
},
298+
body: JSON.stringify(requestBody),
299+
})
343300

344301
/* Granola rejected the request outright, so no endpoint exists to clean up. */
345302
if (!response.ok) {
@@ -360,12 +317,9 @@ export const granolaHandler: WebhookProviderHandler = {
360317
logger.error(
361318
`[${requestId}] Granola webhook endpoint response missing id or signing secret for webhook ${webhook.id}.`
362319
)
363-
await cleanupOrphanedGranolaEndpoint({
364-
apiKey,
365-
endpointId: created.id,
366-
notificationUrl,
367-
requestId,
368-
})
320+
if (created.id) {
321+
await cleanupOrphanedGranolaEndpoint({ apiKey, endpointId: created.id, requestId })
322+
}
369323
throw new Error(
370324
'Granola created the webhook endpoint but did not return an ID and signing secret, so deliveries could not be verified.'
371325
)

0 commit comments

Comments
 (0)