Skip to content

Commit 9de9952

Browse files
fix(slack-trigger): apply event/channel/bot filters on custom-app path too
1 parent 65c1415 commit 9de9952

4 files changed

Lines changed: 505 additions & 371 deletions

File tree

apps/sim/app/api/webhooks/slack/route.test.ts

Lines changed: 38 additions & 166 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@ const {
99
mockCheckWebhookPreprocessing,
1010
mockQueueWebhookExecution,
1111
mockBlockExistsInDeployment,
12+
mockShouldSkip,
1213
} = vi.hoisted(() => ({
1314
mockParseWebhookBody: vi.fn(),
1415
mockFindWebhooksByRoutingKey: vi.fn(),
1516
mockCheckWebhookPreprocessing: vi.fn(),
1617
mockQueueWebhookExecution: vi.fn(),
1718
mockBlockExistsInDeployment: vi.fn(),
19+
mockShouldSkip: vi.fn(),
1820
}))
1921

2022
vi.mock('@/lib/core/admission/gate', () => ({
@@ -36,12 +38,7 @@ vi.mock('@/lib/webhooks/processor', () => ({
3638
vi.mock('@/lib/webhooks/providers/slack', () => ({
3739
handleSlackChallenge: () => null,
3840
verifySlackRequestSignature: () => null,
39-
resolveSlackEventChannel: (event: Record<string, unknown> | undefined) => {
40-
if (!event) return undefined
41-
if (typeof event.channel === 'string') return event.channel
42-
const item = event.item as Record<string, unknown> | undefined
43-
return typeof item?.channel === 'string' ? item.channel : undefined
44-
},
41+
shouldSkipSlackTriggerEvent: mockShouldSkip,
4542
}))
4643

4744
vi.mock('@/lib/workflows/persistence/utils', () => ({
@@ -50,196 +47,71 @@ vi.mock('@/lib/workflows/persistence/utils', () => ({
5047

5148
import { POST } from '@/app/api/webhooks/slack/route'
5249

53-
const API_APP_ID = 'A_SELF'
54-
5550
function makeRequest() {
5651
return new Request('https://sim.test/api/webhooks/slack', {
5752
method: 'POST',
5853
headers: { 'x-slack-request-timestamp': '1700000000' },
5954
}) as unknown as import('next/server').NextRequest
6055
}
6156

62-
function slackBody(event: Record<string, unknown>, extra: Record<string, unknown> = {}) {
63-
return { team_id: 'T1', api_app_id: API_APP_ID, event, ...extra }
57+
function webhook(id: string) {
58+
return { webhook: { id, blockId: `blk-${id}`, providerConfig: {} }, workflow: { id: `wf-${id}` } }
6459
}
6560

66-
/** Drive the route with a single webhook whose providerConfig is `config`. */
67-
async function fireWith(
68-
config: Record<string, unknown>,
69-
body: Record<string, unknown>
70-
): Promise<boolean> {
61+
async function run(body: Record<string, unknown>) {
7162
mockParseWebhookBody.mockResolvedValue({ body, rawBody: JSON.stringify(body) })
72-
mockFindWebhooksByRoutingKey.mockResolvedValue([
73-
{
74-
webhook: { id: 'wh1', blockId: 'blk1', providerConfig: config },
75-
workflow: { id: 'wf1' },
76-
},
77-
])
7863
mockCheckWebhookPreprocessing.mockResolvedValue({
7964
actorUserId: 'u1',
8065
executionId: 'e1',
8166
correlation: {},
8267
})
8368
mockBlockExistsInDeployment.mockResolvedValue(true)
84-
mockQueueWebhookExecution.mockClear()
85-
8669
await POST(makeRequest())
87-
return mockQueueWebhookExecution.mock.calls.length > 0
8870
}
8971

90-
describe('Slack app webhook route filtering', () => {
72+
const messageBody = {
73+
team_id: 'T1',
74+
api_app_id: 'A1',
75+
event: { type: 'message', channel_type: 'channel', channel: 'C1', ts: '1.1' },
76+
}
77+
78+
describe('Slack app webhook route', () => {
9179
beforeEach(() => {
9280
vi.clearAllMocks()
81+
mockFindWebhooksByRoutingKey.mockResolvedValue([webhook('wh1')])
9382
})
9483

95-
it('fires a message event matching source=channel in a public channel', async () => {
96-
const fired = await fireWith(
97-
{ eventType: 'message', source: ['channel'] },
98-
slackBody({ type: 'message', channel_type: 'channel', channel: 'C1', ts: '1.1' })
99-
)
100-
expect(fired).toBe(true)
101-
})
102-
103-
it('drops a DM when source is restricted to public channels', async () => {
104-
const fired = await fireWith(
105-
{ eventType: 'message', source: ['channel'] },
106-
slackBody({ type: 'message', channel_type: 'im', channel: 'D1', ts: '1.1' })
107-
)
108-
expect(fired).toBe(false)
109-
})
110-
111-
it('source=[public,private] fires on both channel types but drops DMs', async () => {
112-
const source = ['channel', 'group']
113-
const publicMsg = await fireWith(
114-
{ eventType: 'message', source },
115-
slackBody({ type: 'message', channel_type: 'channel', channel: 'C1', ts: '1.2' })
116-
)
117-
expect(publicMsg).toBe(true)
118-
119-
const privateMsg = await fireWith(
120-
{ eventType: 'message', source },
121-
slackBody({ type: 'message', channel_type: 'group', channel: 'G1', ts: '1.3' })
122-
)
123-
expect(privateMsg).toBe(true)
124-
125-
const dm = await fireWith(
126-
{ eventType: 'message', source },
127-
slackBody({ type: 'message', channel_type: 'im', channel: 'D1', ts: '1.4' })
128-
)
129-
expect(dm).toBe(false)
84+
it('queues execution when the shared filter does not skip', async () => {
85+
mockShouldSkip.mockReturnValue(false)
86+
await run(messageBody)
87+
expect(mockQueueWebhookExecution).toHaveBeenCalledTimes(1)
13088
})
13189

132-
it('empty source matches any channel type', async () => {
133-
const dm = await fireWith(
134-
{ eventType: 'message', source: [] },
135-
slackBody({ type: 'message', channel_type: 'im', channel: 'D1', ts: '1.5' })
136-
)
137-
expect(dm).toBe(true)
90+
it('does not queue when the shared filter skips the event', async () => {
91+
mockShouldSkip.mockReturnValue(true)
92+
await run(messageBody)
93+
expect(mockQueueWebhookExecution).not.toHaveBeenCalled()
13894
})
13995

140-
it('a channel filter never drops a DM allowed by Source', async () => {
141-
const config = { eventType: 'message', source: ['im', 'channel'], channelFilter: ['C1'] }
142-
// DM passes: the channel filter does not apply to DMs.
143-
const dm = await fireWith(
144-
config,
145-
slackBody({ type: 'message', channel_type: 'im', channel: 'D1', ts: '1.6' })
146-
)
147-
expect(dm).toBe(true)
148-
// Public message in the allowed channel fires.
149-
const inChannel = await fireWith(
150-
config,
151-
slackBody({ type: 'message', channel_type: 'channel', channel: 'C1', ts: '1.7' })
152-
)
153-
expect(inChannel).toBe(true)
154-
// Public message in another channel is dropped by the channel filter.
155-
const otherChannel = await fireWith(
156-
config,
157-
slackBody({ type: 'message', channel_type: 'channel', channel: 'C2', ts: '1.8' })
96+
it('routes via Slack Connect authorizations and dedups overlapping webhooks', async () => {
97+
mockShouldSkip.mockReturnValue(false)
98+
// Two candidate teams (outer + authorization) that resolve to overlapping webhooks.
99+
mockFindWebhooksByRoutingKey.mockImplementation(async (teamId: string) =>
100+
teamId === 'T1' ? [webhook('wh1')] : [webhook('wh1'), webhook('wh2')]
158101
)
159-
expect(otherChannel).toBe(false)
160-
})
161-
162-
it('app_mention Threads=Only fires only on threaded mentions', async () => {
163-
const topLevel = await fireWith(
164-
{ eventType: 'app_mention', threads: 'only' },
165-
slackBody({ type: 'app_mention', channel: 'C1', ts: '2.0' })
166-
)
167-
expect(topLevel).toBe(false)
168-
169-
const threaded = await fireWith(
170-
{ eventType: 'app_mention', threads: 'only' },
171-
slackBody({ type: 'app_mention', channel: 'C1', ts: '2.1', thread_ts: '2.0' })
172-
)
173-
expect(threaded).toBe(true)
174-
})
175-
176-
it('maps message_changed to message_edited and not to message', async () => {
177-
const editBody = slackBody({
178-
type: 'message',
179-
subtype: 'message_changed',
180-
channel_type: 'channel',
181-
channel: 'C1',
182-
ts: '3.1',
102+
await run({
103+
...messageBody,
104+
authorizations: [{ team_id: 'T2' }],
183105
})
184-
expect(await fireWith({ eventType: 'message_edited' }, editBody)).toBe(true)
185-
expect(await fireWith({ eventType: 'message' }, editBody)).toBe(false)
106+
expect(mockFindWebhooksByRoutingKey).toHaveBeenCalledTimes(2)
107+
// wh1 (in both) is queued once, wh2 once — dedup by webhook id.
108+
expect(mockQueueWebhookExecution).toHaveBeenCalledTimes(2)
186109
})
187110

188-
it("self-drops the app's own message unless includeOwnMessages is set", async () => {
189-
const ownBody = slackBody({
190-
type: 'message',
191-
channel_type: 'channel',
192-
channel: 'C1',
193-
ts: '4.1',
194-
app_id: API_APP_ID,
195-
bot_id: 'B1',
196-
})
197-
expect(await fireWith({ eventType: 'message' }, ownBody)).toBe(false)
198-
expect(await fireWith({ eventType: 'message', includeOwnMessages: true }, ownBody)).toBe(true)
199-
})
200-
201-
it("self-drops the app's own reaction via stored bot_user_id", async () => {
202-
const body = slackBody({
203-
type: 'reaction_added',
204-
reaction: 'thumbsup',
205-
user: 'U_BOT',
206-
item: { channel: 'C1', ts: '5.0' },
207-
})
208-
expect(await fireWith({ eventType: 'reaction_added', bot_user_id: 'U_BOT' }, body)).toBe(false)
209-
expect(await fireWith({ eventType: 'reaction_added', bot_user_id: 'U_OTHER' }, body)).toBe(true)
210-
})
211-
212-
it('applies the emoji filter to reaction events', async () => {
213-
const body = slackBody({
214-
type: 'reaction_added',
215-
reaction: 'eyes',
216-
user: 'U1',
217-
item: { channel: 'C1', ts: '6.0' },
218-
})
219-
expect(await fireWith({ eventType: 'reaction_added', emoji: 'thumbsup' }, body)).toBe(false)
220-
expect(await fireWith({ eventType: 'reaction_added', emoji: 'eyes, thumbsup' }, body)).toBe(
221-
true
222-
)
223-
})
224-
225-
it('honors the legacy events array for pre-redesign webhooks', async () => {
226-
const fired = await fireWith(
227-
{ events: ['message.channels'] },
228-
slackBody({ type: 'message', channel_type: 'channel', channel: 'C1', ts: '7.1' })
229-
)
230-
expect(fired).toBe(true)
231-
})
232-
233-
it('ignores other bots but not our own drop path when filterBotMessages is on', async () => {
234-
const otherBot = slackBody({
235-
type: 'message',
236-
channel_type: 'channel',
237-
channel: 'C1',
238-
ts: '8.1',
239-
bot_id: 'B_OTHER',
240-
app_id: 'A_OTHER',
241-
})
242-
expect(await fireWith({ eventType: 'message' }, otherBot)).toBe(false)
243-
expect(await fireWith({ eventType: 'message', filterBotMessages: false }, otherBot)).toBe(true)
111+
it('returns 200 with no team_id', async () => {
112+
mockShouldSkip.mockReturnValue(false)
113+
await run({ event: { type: 'message' } })
114+
expect(mockFindWebhooksByRoutingKey).not.toHaveBeenCalled()
115+
expect(mockQueueWebhookExecution).not.toHaveBeenCalled()
244116
})
245117
})

0 commit comments

Comments
 (0)