Skip to content

Commit 12513cb

Browse files
fix(slack): acknowledge filtered webhook deliveries
1 parent b7cfa65 commit 12513cb

6 files changed

Lines changed: 102 additions & 5 deletions

File tree

apps/sim/app/api/webhooks/slack/custom/[credentialId]/route.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,4 +146,18 @@ describe('Slack custom-bot webhook route', () => {
146146
expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(1)
147147
expect(res.status).toBe(200)
148148
})
149+
150+
it('returns the dispatch failure when no target is acknowledged', async () => {
151+
mockDispatchResolvedWebhookTarget.mockResolvedValue({
152+
outcome: 'failed',
153+
response: new Response('Preprocessing failed', { status: 500 }),
154+
reason: 'preprocessing',
155+
})
156+
157+
const res = await POST(makeRequest(), context)
158+
159+
expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(1)
160+
expect(res.status).toBe(500)
161+
await expect(res.text()).resolves.toBe('Preprocessing failed')
162+
})
149163
})

apps/sim/app/api/webhooks/slack/custom/[credentialId]/route.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,22 @@ async function handleSlackCustomBotWebhook(
6767
return authError
6868
}
6969

70-
await dispatchSlackCustomBotCredential({ credentialId, body, request, requestId, receivedAt })
70+
const dispatchResults = await dispatchSlackCustomBotCredential({
71+
credentialId,
72+
body,
73+
request,
74+
requestId,
75+
receivedAt,
76+
})
77+
const acknowledged = dispatchResults.some(
78+
(result) => result.outcome !== 'failed' && result.reason !== 'block-missing'
79+
)
80+
if (!acknowledged) {
81+
const failure = dispatchResults.find(
82+
(result) => result.outcome === 'failed' || result.reason === 'block-missing'
83+
)
84+
if (failure) return failure.response
85+
}
7186

7287
return new NextResponse(null, { status: 200 })
7388
}

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,37 @@ describe('Webhook Trigger API Route', () => {
878878
expect(response.status).toBe(500)
879879
expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled()
880880
})
881+
882+
it('acknowledges a legacy fan-out when every target filters the event', async () => {
883+
testData.webhooks.push({
884+
id: 'legacy-slack-webhook',
885+
provider: 'slack',
886+
path: 'legacy-slack-path',
887+
routingKey: 'credential-1',
888+
isActive: true,
889+
providerConfig: {
890+
triggerId: 'slack_webhook',
891+
credentialId: 'credential-1',
892+
ingressMode: 'legacy_custom_bot',
893+
},
894+
workflowId: 'test-workflow-id',
895+
})
896+
dispatchSlackCustomBotCredentialMock.mockResolvedValueOnce([
897+
{
898+
outcome: 'ignored',
899+
reason: 'filtered',
900+
response: NextResponse.json({ message: 'Webhook event ignored' }),
901+
},
902+
])
903+
904+
const response = await POST(createMockRequest('POST', { type: 'event_callback' }), {
905+
params: Promise.resolve({ path: 'legacy-slack-path' }),
906+
})
907+
908+
expect(response.status).toBe(200)
909+
await expect(response.json()).resolves.toEqual({ message: 'Webhook event ignored' })
910+
expect(dispatchResolvedWebhookTargetMock).not.toHaveBeenCalled()
911+
})
881912
})
882913

883914
describe('Reservation-free filtering', () => {

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,6 @@ async function handleWebhookPost(
187187
const responses: NextResponse[] = []
188188
const failures: NextResponse[] = []
189189
for (const dispatchResult of legacySlackDispatchResults) {
190-
if (dispatchResult.reason === 'filtered') continue
191190
if (dispatchResult.outcome === 'failed' || dispatchResult.reason === 'block-missing') {
192191
failures.push(dispatchResult.response)
193192
continue

packages/db/scripts/migrate-slack-custom-bots.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,39 @@ describe('planLegacySlackTriggerLink', () => {
296296
})
297297

298298
describe('resolveSlackSourceSecrets', () => {
299+
it('marks a trigger without a bot token as unresolved', () => {
300+
expect(
301+
resolveSlackSourceSecrets(
302+
source({
303+
sourceId: 'workflow-1:block-1:trigger',
304+
kind: 'trigger',
305+
rawBotToken: undefined,
306+
rawSigningSecret: 'signing-secret',
307+
}),
308+
environmentLookup()
309+
)
310+
).toEqual({
311+
status: 'unresolved',
312+
reason: 'Source workflow-1:block-1:trigger has no bot token',
313+
})
314+
})
315+
316+
it('marks a trigger without a signing secret as unresolved', () => {
317+
expect(
318+
resolveSlackSourceSecrets(
319+
source({
320+
sourceId: 'workflow-1:block-1:trigger',
321+
kind: 'trigger',
322+
rawSigningSecret: undefined,
323+
}),
324+
environmentLookup()
325+
)
326+
).toEqual({
327+
status: 'unresolved',
328+
reason: 'Trigger source workflow-1:block-1:trigger has no signing secret',
329+
})
330+
})
331+
299332
it('marks a missing environment variable as an unresolved source', () => {
300333
expect(
301334
resolveSlackSourceSecrets(source({ rawBotToken: '{{SLACK_BOT_TOKEN}}' }), environmentLookup())

packages/db/scripts/migrate-slack-custom-bots.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,9 @@ export function resolveSlackSourceSecrets(
482482
): SlackSourceSecretResolution {
483483
try {
484484
const botToken = resolveStoredSecret(source.rawBotToken, source, lookup, 'botToken')
485-
if (!botToken) throw new Error(`Source ${source.sourceId} has no bot token`)
485+
if (!botToken) {
486+
return { status: 'unresolved', reason: `Source ${source.sourceId} has no bot token` }
487+
}
486488

487489
const signingSecret = resolveStoredSecret(
488490
source.rawSigningSecret,
@@ -491,7 +493,10 @@ export function resolveSlackSourceSecrets(
491493
'signingSecret'
492494
)
493495
if (source.kind === 'trigger' && !signingSecret) {
494-
throw new Error(`Trigger source ${source.sourceId} has no signing secret`)
496+
return {
497+
status: 'unresolved',
498+
reason: `Trigger source ${source.sourceId} has no signing secret`,
499+
}
495500
}
496501

497502
return { status: 'ready', botToken, signingSecret }
@@ -716,7 +721,7 @@ async function prepareWorkspaceCredentials(params: {
716721
const resolution = resolveSlackSourceSecrets(source, environmentLookup)
717722
if (resolution.status === 'unresolved') {
718723
params.stats.skippedUnresolved++
719-
logger.warn('Skipping Slack bot credential source with an unresolved environment variable', {
724+
logger.warn('Skipping Slack bot credential source with unresolved secrets', {
720725
workspaceId: params.workspaceId,
721726
workflowId: source.workflowId,
722727
workflowName: source.workflowName,

0 commit comments

Comments
 (0)