Skip to content

Commit 289b6bd

Browse files
committed
fix(slack): harden native webhook configuration
1 parent 776f148 commit 289b6bd

8 files changed

Lines changed: 177 additions & 17 deletions

File tree

apps/docs/content/docs/platform/self-hosting/integrations-oauth.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,8 @@ Webhook triggers receive callbacks from the provider and must be able to verify
196196
| `SLACK_SIGNING_SECRET` | Verifying Slack event and slash-command signatures |
197197
| `SLACK_EXTENDED_SCOPES` / `NEXT_PUBLIC_SLACK_EXTENDED_SCOPES` | Enabling the native Sim-app trigger and its broader Slack scope set; set both to the same value |
198198

199+
When enabling the native Sim Slack trigger, configure all three variables together. Slack OAuth actions can use `SLACK_CLIENT_ID` and `SLACK_CLIENT_SECRET` without enabling the native trigger or supplying a signing secret.
200+
199201
Your deployment must also be reachable from the provider's servers for webhook triggers to fire — a Sim instance on a private network can use polling triggers but not webhook triggers. Polling triggers additionally require the scheduler; see [Background Jobs](/platform/self-hosting/background-jobs).
200202

201203
<FAQ items={[

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

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,19 @@
44
import { resetEnvMock, setEnv } from '@sim/testing'
55
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
66

7-
const { mockParseWebhookBody, mockFindWebhooksByRoutingKey, mockDispatchResolvedWebhookTarget } =
8-
vi.hoisted(() => ({
9-
mockParseWebhookBody: vi.fn(),
10-
mockFindWebhooksByRoutingKey: vi.fn(),
11-
mockDispatchResolvedWebhookTarget: vi.fn(),
12-
}))
7+
const {
8+
mockParseWebhookBody,
9+
mockFindWebhooksByRoutingKey,
10+
mockDispatchResolvedWebhookTarget,
11+
mockHandleSlackChallenge,
12+
mockVerifySlackRequestSignature,
13+
} = vi.hoisted(() => ({
14+
mockParseWebhookBody: vi.fn(),
15+
mockFindWebhooksByRoutingKey: vi.fn(),
16+
mockDispatchResolvedWebhookTarget: vi.fn(),
17+
mockHandleSlackChallenge: vi.fn(),
18+
mockVerifySlackRequestSignature: vi.fn(),
19+
}))
1320

1421
vi.mock('@/lib/core/admission/gate', () => ({
1522
tryAdmit: () => ({ release: vi.fn() }),
@@ -23,8 +30,8 @@ vi.mock('@/lib/webhooks/processor', () => ({
2330
}))
2431

2532
vi.mock('@/lib/webhooks/providers/slack', () => ({
26-
handleSlackChallenge: () => null,
27-
verifySlackRequestSignature: () => null,
33+
handleSlackChallenge: mockHandleSlackChallenge,
34+
verifySlackRequestSignature: mockVerifySlackRequestSignature,
2835
resolveSlackEventKey: () => null,
2936
}))
3037

@@ -60,6 +67,8 @@ describe('Slack app webhook route', () => {
6067
beforeEach(() => {
6168
vi.clearAllMocks()
6269
setEnv({ SLACK_SIGNING_SECRET: 'test-secret' })
70+
mockHandleSlackChallenge.mockReturnValue(null)
71+
mockVerifySlackRequestSignature.mockReturnValue(null)
6372
mockFindWebhooksByRoutingKey.mockResolvedValue([webhook('wh1')])
6473
mockDispatchResolvedWebhookTarget.mockResolvedValue({
6574
outcome: 'queued',
@@ -70,9 +79,53 @@ describe('Slack app webhook route', () => {
7079

7180
it('dispatches each webhook resolved for the event team', async () => {
7281
await run(messageBody)
82+
expect(mockVerifySlackRequestSignature).toHaveBeenCalledWith(
83+
'test-secret',
84+
expect.anything(),
85+
JSON.stringify(messageBody),
86+
expect.any(String)
87+
)
7388
expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(1)
7489
})
7590

91+
it('rejects a verification challenge when the native app is not configured', async () => {
92+
setEnv({ SLACK_SIGNING_SECRET: undefined })
93+
mockHandleSlackChallenge.mockReturnValue(new Response('challenge', { status: 200 }))
94+
95+
const response = await run({ type: 'url_verification', challenge: 'challenge' })
96+
97+
expect(response.status).toBe(500)
98+
expect(mockVerifySlackRequestSignature).not.toHaveBeenCalled()
99+
expect(mockHandleSlackChallenge).not.toHaveBeenCalled()
100+
})
101+
102+
it('verifies a signed request before answering the verification challenge', async () => {
103+
const body = { type: 'url_verification', challenge: 'challenge' }
104+
mockHandleSlackChallenge.mockReturnValue(new Response('challenge', { status: 200 }))
105+
106+
const response = await run(body)
107+
108+
expect(mockVerifySlackRequestSignature).toHaveBeenCalledWith(
109+
'test-secret',
110+
expect.anything(),
111+
JSON.stringify(body),
112+
expect.any(String)
113+
)
114+
expect(mockHandleSlackChallenge).toHaveBeenCalledWith(body)
115+
expect(response.status).toBe(200)
116+
expect(mockFindWebhooksByRoutingKey).not.toHaveBeenCalled()
117+
})
118+
119+
it('does not answer a verification challenge with an invalid signature', async () => {
120+
mockVerifySlackRequestSignature.mockReturnValue(new Response(null, { status: 401 }))
121+
mockHandleSlackChallenge.mockReturnValue(new Response('challenge', { status: 200 }))
122+
123+
const response = await run({ type: 'url_verification', challenge: 'challenge' })
124+
125+
expect(response.status).toBe(401)
126+
expect(mockHandleSlackChallenge).not.toHaveBeenCalled()
127+
})
128+
76129
it('continues cleanly when the dispatcher filters the event', async () => {
77130
mockDispatchResolvedWebhookTarget.mockResolvedValue({
78131
outcome: 'ignored',

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { createLogger } from '@sim/logger'
22
import { type NextRequest, NextResponse } from 'next/server'
33
import { admissionRejectedResponse, tryAdmit } from '@/lib/core/admission/gate'
4-
import { env } from '@/lib/core/config/env'
54
import { generateRequestId } from '@/lib/core/utils/request'
65
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
76
import { findWebhooksByRoutingKey, parseWebhookBody } from '@/lib/webhooks/processor'
87
import { handleSlackChallenge, verifySlackRequestSignature } from '@/lib/webhooks/providers/slack'
98
import { dispatchSlackWebhooks, getSlackDispatchResponse } from '@/lib/webhooks/slack-dispatch'
9+
import { getSlackNativeSigningSecret } from '@/lib/webhooks/slack-native-config'
1010

1111
const logger = createLogger('SlackAppWebhookAPI')
1212

@@ -44,13 +44,7 @@ async function handleSlackAppWebhook(request: NextRequest): Promise<NextResponse
4444
}
4545
const { body, rawBody } = parseResult
4646

47-
// Slack's endpoint verification handshake — echo the challenge back.
48-
const challenge = handleSlackChallenge(body)
49-
if (challenge) {
50-
return challenge
51-
}
52-
53-
const signingSecret = env.SLACK_SIGNING_SECRET
47+
const signingSecret = getSlackNativeSigningSecret()
5448
if (!signingSecret) {
5549
logger.error(`[${requestId}] SLACK_SIGNING_SECRET is not configured`)
5650
return new NextResponse('Slack app not configured', { status: 500 })
@@ -61,6 +55,11 @@ async function handleSlackAppWebhook(request: NextRequest): Promise<NextResponse
6155
return authError
6256
}
6357

58+
const challenge = handleSlackChallenge(body)
59+
if (challenge) {
60+
return challenge
61+
}
62+
6463
const payload = body as Record<string, unknown>
6564

6665
// Route by the installed workspace(s). For Slack Connect the outer `team_id`

apps/sim/lib/webhooks/deploy.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import {
77
queueTableRows,
88
resetDbChainMock,
99
resetEnvFlagsMock,
10+
resetEnvMock,
11+
setEnv,
1012
setEnvFlags,
1113
} from '@sim/testing'
1214
import { eq, ne } from 'drizzle-orm'
@@ -84,6 +86,7 @@ import { getTrigger } from '@/triggers'
8486

8587
afterAll(() => {
8688
resetDbChainMock()
89+
resetEnvMock()
8790
resetEnvFlagsMock()
8891
})
8992

@@ -151,6 +154,7 @@ function makeBlock(
151154
beforeEach(() => {
152155
vi.clearAllMocks()
153156
resetDbChainMock()
157+
setEnv({ SLACK_SIGNING_SECRET: 'test-secret' })
154158
setEnvFlags({ isSlackExtendedScopesEnabled: true })
155159
;(getProviderHandler as unknown as Mock).mockImplementation((provider: string) =>
156160
provider === 'quickbooks' ? quickBooksHandler : {}
@@ -301,8 +305,9 @@ describe('resolveWebhookConfigForBlock — slack_oauth routing', () => {
301305
})
302306
}
303307

304-
it('routes a custom bot credential by credential id on the slack provider', async () => {
308+
it('routes a custom bot credential without the native app signing secret', async () => {
305309
setEnvFlags({ isSlackExtendedScopesEnabled: false })
310+
setEnv({ SLACK_SIGNING_SECRET: undefined })
306311
mockGetSlackBotCredential.mockResolvedValue({
307312
workspaceId: 'ws-1',
308313
botToken: 'xoxb-token',
@@ -403,6 +408,24 @@ describe('resolveWebhookConfigForBlock — slack_oauth routing', () => {
403408
expect(mockFetchSlackTeamId).not.toHaveBeenCalled()
404409
})
405410

411+
it('rejects a Sim-app credential when its signing secret is not configured', async () => {
412+
setEnv({ SLACK_SIGNING_SECRET: undefined })
413+
mockGetSlackBotCredential.mockResolvedValue(null)
414+
mockResolveOAuthAccountId.mockResolvedValue({ accountId: 'acct-1' })
415+
416+
const result = await resolveSlack({ eventType: 'message', customBotCredential: 'cred_oauth_1' })
417+
418+
expect(result?.success).toBe(false)
419+
if (result?.success) throw new Error('expected failure')
420+
expect(result?.error).toEqual({
421+
message:
422+
'The Sim Slack app trigger is not configured for this deployment. Configure its signing secret or select a custom bot.',
423+
status: 400,
424+
})
425+
expect(mockRefreshAccessTokenIfNeeded).not.toHaveBeenCalled()
426+
expect(mockFetchSlackTeamId).not.toHaveBeenCalled()
427+
})
428+
406429
it('rejects a custom bot credential from another workspace', async () => {
407430
mockGetSlackBotCredential.mockResolvedValue({
408431
workspaceId: 'other-ws',

apps/sim/lib/webhooks/deploy.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
type StableDesiredWebhookRegistration,
2929
} from '@/lib/webhooks/registration-service'
3030
import { LEGACY_SLACK_CUSTOM_BOT_INGRESS_MODE } from '@/lib/webhooks/slack-custom-ingress-constants'
31+
import { getSlackNativeSigningSecret } from '@/lib/webhooks/slack-native-config'
3132
import {
3233
isSlackStreamResponseRequested,
3334
normalizeSlackStreamResponseConfig,
@@ -517,6 +518,16 @@ export async function resolveWebhookConfigForBlock(input: {
517518
},
518519
}
519520
}
521+
if (!getSlackNativeSigningSecret()) {
522+
return {
523+
success: false,
524+
error: {
525+
message:
526+
'The Sim Slack app trigger is not configured for this deployment. Configure its signing secret or select a custom bot.',
527+
status: 400,
528+
},
529+
}
530+
}
520531
if (isSlackStreamResponseRequested(providerConfig)) {
521532
return {
522533
success: false,
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { env } from '@/lib/core/config/env'
2+
3+
/** Returns the signing secret for the native Sim Slack app when it is configured. */
4+
export function getSlackNativeSigningSecret(): string | null {
5+
const signingSecret = env.SLACK_SIGNING_SECRET?.trim()
6+
return signingSecret || null
7+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { type CheckContext, runChecks } from './checks'
3+
import type { EnvFile, EnvTarget } from './env-files'
4+
5+
function envFile(target: EnvTarget, values: Record<string, string> = {}): EnvFile {
6+
return {
7+
target,
8+
path: `${target}.env`,
9+
exists: target === 'root',
10+
content: '',
11+
vars: new Map(Object.entries(values)),
12+
}
13+
}
14+
15+
function rootContext(values: Record<string, string>): CheckContext {
16+
const root = envFile('root', values)
17+
return {
18+
layout: 'root',
19+
primary: root,
20+
live: false,
21+
env: {
22+
root,
23+
sim: envFile('sim'),
24+
realtime: envFile('realtime'),
25+
db: envFile('db'),
26+
},
27+
}
28+
}
29+
30+
describe('setup coherence checks', () => {
31+
it('requires a signing secret when native Slack triggers are enabled', async () => {
32+
const findings = await runChecks(
33+
rootContext({
34+
SLACK_EXTENDED_SCOPES: 'true',
35+
NEXT_PUBLIC_SLACK_EXTENDED_SCOPES: 'true',
36+
}),
37+
['coherence']
38+
)
39+
40+
expect(findings).toContainEqual({
41+
group: 'coherence',
42+
status: 'fail',
43+
message:
44+
'SLACK_EXTENDED_SCOPES is on but SLACK_SIGNING_SECRET is not set — native Slack triggers will fail at runtime',
45+
fix: 'set SLACK_SIGNING_SECRET or remove SLACK_EXTENDED_SCOPES',
46+
})
47+
})
48+
49+
it('does not require a signing secret for outbound-only Slack OAuth', async () => {
50+
const findings = await runChecks(
51+
rootContext({
52+
SLACK_CLIENT_ID: 'client-id',
53+
SLACK_CLIENT_SECRET: 'client-secret',
54+
}),
55+
['coherence']
56+
)
57+
58+
expect(findings.some((finding) => finding.message.includes('SLACK_SIGNING_SECRET'))).toBe(false)
59+
})
60+
})

packages/sim-setup/src/checks.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,11 @@ function checkCoherence(ctx: CheckContext): Finding[] {
467467
needs: ['STRIPE_SECRET_KEY'],
468468
label: 'billing',
469469
},
470+
{
471+
flag: 'SLACK_EXTENDED_SCOPES',
472+
needs: ['SLACK_SIGNING_SECRET'],
473+
label: 'native Slack triggers',
474+
},
470475
{ flag: 'SSO_ENABLED', needs: ['SSO_ISSUER'], label: 'SSO' },
471476
]
472477
for (const rule of featureRules) {

0 commit comments

Comments
 (0)