Skip to content

Commit 1c6ca3e

Browse files
committed
fix(triggers): address review — fail-closed auth, Twilio event filtering, user-only secrets
- Twilio: add matchEvent so inbound-SMS and status-callback triggers don't cross-fire on a shared webhook URL (inbound = SmsStatus 'received') - Rootly + RevenueCat: verifyAuth now fails closed (401) when the signing secret is absent from provider config (both auto-register, so it is always present after deploy) instead of skipping verification - Mark every user-provided credential field paramVisibility 'user-only' (Clerk, incident.io, RevenueCat, Sentry, Twilio) so secrets are never exposed as LLM-visible trigger params - Sentry: correct metric_alert web_url description per docs
1 parent 9abb8d5 commit 1c6ca3e

11 files changed

Lines changed: 83 additions & 11 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ describe('RevenueCat webhook provider', () => {
4747
describe('verifyAuth', () => {
4848
const secret = 'super-secret-header-value'
4949

50-
it('allows requests when no secret is configured', async () => {
50+
it('rejects requests when no secret is configured (fail-closed)', async () => {
5151
const res = await revenueCatHandler.verifyAuth!({
5252
request: requestWithAuth(),
5353
rawBody: '{}',
@@ -56,7 +56,7 @@ describe('RevenueCat webhook provider', () => {
5656
webhook: {},
5757
workflow: {},
5858
})
59-
expect(res).toBeNull()
59+
expect(res?.status).toBe(401)
6060
})
6161

6262
it('rejects requests missing the Authorization header', async () => {

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,11 @@ const REVENUECAT_API_BASE = 'https://api.revenuecat.com/v2'
2222
/**
2323
* RevenueCat webhook handler.
2424
*
25-
* RevenueCat does not sign payloads. Instead, the user configures an Authorization
26-
* header value in the RevenueCat dashboard that is sent verbatim on every request.
27-
* We verify the incoming `Authorization` header against the configured secret using
28-
* a timing-safe comparison.
25+
* RevenueCat does not sign payloads. Instead, an Authorization header value is sent
26+
* verbatim on every request. Sim generates this secret and sets it when it creates the
27+
* webhook integration (see `createSubscription`), so it is always present once the
28+
* trigger is deployed. We verify the incoming `Authorization` header against it using a
29+
* timing-safe comparison and fail closed if the secret is missing from config.
2930
*
3031
* @see https://www.revenuecat.com/docs/integrations/webhooks
3132
*/
@@ -34,10 +35,12 @@ export const revenueCatHandler: WebhookProviderHandler = {
3435
const secret = providerConfig.authHeaderSecret as string | undefined
3536

3637
if (!secret) {
37-
logger.debug(
38-
`[${requestId}] RevenueCat webhook has no Authorization secret configured, skipping verification`
38+
logger.warn(
39+
`[${requestId}] RevenueCat webhook missing Authorization secret in provider configuration`
3940
)
40-
return null
41+
return new NextResponse('Unauthorized - RevenueCat Authorization secret is required', {
42+
status: 401,
43+
})
4144
}
4245

4346
const authHeader = request.headers.get('authorization')

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,21 @@ describe('Rootly webhook provider', () => {
4141
expect(res).toBeNull()
4242
})
4343

44+
it('rejects when the signing secret is missing from config (fail-closed)', async () => {
45+
const rawBody = JSON.stringify({ event: { id: 'evt-1', type: 'incident.created' }, data: {} })
46+
47+
const res = await rootlyHandler.verifyAuth!({
48+
request: new NextRequest('http://localhost/test'),
49+
rawBody,
50+
requestId: 'rootly-t1b',
51+
providerConfig: {},
52+
webhook: {},
53+
workflow: {},
54+
})
55+
56+
expect(res?.status).toBe(401)
57+
})
58+
4459
it('rejects a request with an invalid signature', async () => {
4560
const secret = 'rootly-secret'
4661
const timestamp = Math.floor(Date.now() / 1000).toString()

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ export const rootlyHandler: WebhookProviderHandler = {
6767
}: AuthContext): Promise<NextResponse | null> {
6868
const secret = providerConfig.webhookSecret as string | undefined
6969
if (!secret) {
70-
return null
70+
logger.warn(`[${requestId}] Rootly webhook missing signing secret in provider configuration`)
71+
return new NextResponse('Unauthorized - Rootly signing secret is required', { status: 401 })
7172
}
7273

7374
const header = request.headers.get('X-Rootly-Signature')

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,36 @@ describe('twilioHandler', () => {
134134
})
135135
})
136136

137+
describe('matchEvent', () => {
138+
const match = (triggerId: string, body: Record<string, unknown>) =>
139+
twilioHandler.matchEvent!({
140+
body,
141+
request: new Request('http://localhost/test') as never,
142+
rawBody: '',
143+
requestId: 'r1',
144+
providerConfig: { triggerId },
145+
webhook: {},
146+
workflow: {},
147+
})
148+
149+
const inbound = { MessageSid: 'SM1', From: '+1', Body: 'hi', SmsStatus: 'received' }
150+
const status = { MessageSid: 'SM1', MessageStatus: 'delivered', SmsStatus: 'delivered' }
151+
152+
it('routes inbound messages only to the received trigger', () => {
153+
expect(match('twilio_sms_received', inbound)).toBe(true)
154+
expect(match('twilio_sms_status', inbound)).toBe(false)
155+
})
156+
157+
it('routes delivery callbacks only to the status trigger', () => {
158+
expect(match('twilio_sms_status', status)).toBe(true)
159+
expect(match('twilio_sms_received', status)).toBe(false)
160+
})
161+
162+
it('passes through when no triggerId is configured', () => {
163+
expect(match('', inbound)).toBe(true)
164+
})
165+
})
166+
137167
describe('formatInput', () => {
138168
const ctx = (body: Record<string, unknown>) => ({
139169
webhook: {},

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { verifyTwilioAuth } from '@/lib/webhooks/providers/twilio-signature'
22
import type {
33
AuthContext,
4+
EventMatchContext,
45
FormatInputContext,
56
FormatInputResult,
67
WebhookProviderHandler,
@@ -24,6 +25,23 @@ export const twilioHandler: WebhookProviderHandler = {
2425
return verifyTwilioAuth(ctx, 'Twilio SMS')
2526
},
2627

28+
/**
29+
* Distinguish an inbound SMS from a delivery status callback so the two
30+
* triggers don't fire on each other's deliveries when they share a URL.
31+
* Twilio reports `SmsStatus: 'received'` for inbound messages; status
32+
* callbacks carry a delivery `MessageStatus` (queued/sent/delivered/…).
33+
*/
34+
matchEvent({ body, providerConfig }: EventMatchContext) {
35+
const triggerId = providerConfig.triggerId as string | undefined
36+
if (!triggerId) return true
37+
const b = body as Record<string, unknown>
38+
const status = (((b.MessageStatus as string) || (b.SmsStatus as string)) ?? '').toLowerCase()
39+
const isInbound = status === 'received'
40+
if (triggerId === 'twilio_sms_received') return isInbound
41+
if (triggerId === 'twilio_sms_status') return !isInbound
42+
return true
43+
},
44+
2745
extractIdempotencyId(body: unknown) {
2846
const obj = body as Record<string, unknown>
2947
return (obj.MessageSid as string) || (obj.CallSid as string) || null

apps/sim/triggers/clerk/utils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export function buildClerkExtraFields(triggerId: string): SubBlockConfig[] {
6767
placeholder: 'whsec_...',
6868
description: 'Copy this from your Clerk webhook endpoint to verify event signatures.',
6969
password: true,
70+
paramVisibility: 'user-only',
7071
required: true,
7172
mode: 'trigger',
7273
condition: { field: 'selectedTriggerId', value: triggerId },

apps/sim/triggers/incidentio/utils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ export function buildIncidentioExtraFields(triggerId: string): SubBlockConfig[]
6262
description:
6363
'The signing secret from your incident.io webhook endpoint. Used to verify events.',
6464
password: true,
65+
paramVisibility: 'user-only',
6566
required: true,
6667
mode: 'trigger',
6768
condition: { field: 'selectedTriggerId', value: triggerId },

apps/sim/triggers/revenuecat/utils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ export function buildRevenueCatExtraFields(triggerId: string): SubBlockConfig[]
8686
title: 'API Key',
8787
type: 'short-input',
8888
password: true,
89+
paramVisibility: 'user-only',
8990
placeholder: 'RevenueCat v2 Secret API key (sk_...)',
9091
description:
9192
'Secret API key with the project_configuration:integrations:read_write permission. Sim uses it to create and remove the webhook in RevenueCat.',

apps/sim/triggers/sentry/utils.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export function buildSentryExtraFields(triggerId: string): SubBlockConfig[] {
4848
type: 'short-input',
4949
placeholder: "Paste your Sentry Internal Integration's Client Secret",
5050
password: true,
51+
paramVisibility: 'user-only',
5152
required: true,
5253
mode: 'trigger',
5354
condition: { field: 'selectedTriggerId', value: triggerId },
@@ -199,7 +200,7 @@ export function buildMetricAlertOutputs(): Record<string, TriggerOutput> {
199200
},
200201
description_text: { type: 'string', description: 'Human-friendly description of the alert' },
201202
description_title: { type: 'string', description: 'Human-friendly title of the alert' },
202-
web_url: { type: 'string', description: 'Browser URL for the incident' },
203+
web_url: { type: 'string', description: 'API URL for the incident' },
203204
}
204205
}
205206

0 commit comments

Comments
 (0)