Skip to content

Commit 00761e3

Browse files
fix(security): backstop public OTP requests
1 parent 46c4a14 commit 00761e3

6 files changed

Lines changed: 129 additions & 4 deletions

File tree

apps/sim/app/api/chat/[identifier]/otp/route.test.ts

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,48 @@ describe('Chat OTP API Route', () => {
283283
})
284284

285285
it('returns 429 with Retry-After when email rate limit is exceeded', async () => {
286+
mockCheckRateLimitDirect
287+
.mockResolvedValueOnce({
288+
allowed: true,
289+
remaining: 9,
290+
resetAt: new Date(Date.now() + 60_000),
291+
})
292+
.mockResolvedValueOnce({
293+
allowed: true,
294+
remaining: 99,
295+
resetAt: new Date(Date.now() + 60_000),
296+
})
297+
.mockResolvedValueOnce({
298+
allowed: false,
299+
remaining: 0,
300+
resetAt: new Date(Date.now() + 900_000),
301+
retryAfterMs: 900_000,
302+
})
303+
304+
const headerSet = vi.fn()
305+
mockCreateErrorResponse.mockImplementationOnce((message: string, status: number) => ({
306+
json: () => Promise.resolve({ error: message }),
307+
status,
308+
headers: { set: headerSet },
309+
}))
310+
311+
queueDeployment(emailDeployment)
312+
313+
const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
314+
method: 'POST',
315+
body: JSON.stringify({ email: mockEmail }),
316+
})
317+
318+
const response = await POST(request, {
319+
params: Promise.resolve({ identifier: mockIdentifier }),
320+
})
321+
322+
expect(response.status).toBe(429)
323+
expect(headerSet).toHaveBeenCalledWith('Retry-After', '900')
324+
expect(mockSendEmail).not.toHaveBeenCalled()
325+
})
326+
327+
it('returns 429 with Retry-After when the chat resource rate limit is exceeded', async () => {
286328
mockCheckRateLimitDirect
287329
.mockResolvedValueOnce({
288330
allowed: true,
@@ -343,7 +385,7 @@ describe('Chat OTP API Route', () => {
343385
expect(headerSet).toHaveBeenCalledWith('Retry-After', '900')
344386
})
345387

346-
it('skips the IP bucket when the client IP cannot be resolved', async () => {
388+
it('retains resource and email backstops when the client IP cannot be resolved', async () => {
347389
requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce(null)
348390
queueDeployment(emailDeployment)
349391

@@ -354,8 +396,15 @@ describe('Chat OTP API Route', () => {
354396

355397
await POST(request, { params: Promise.resolve({ identifier: mockIdentifier }) })
356398

357-
expect(mockCheckRateLimitDirect).toHaveBeenCalledTimes(1)
358-
expect(mockCheckRateLimitDirect).toHaveBeenCalledWith(
399+
expect(mockCheckRateLimitDirect).toHaveBeenCalledTimes(2)
400+
expect(mockCheckRateLimitDirect).toHaveBeenNthCalledWith(
401+
1,
402+
'chat-otp:resource:chat-123',
403+
expect.any(Object),
404+
{ failClosed: true }
405+
)
406+
expect(mockCheckRateLimitDirect).toHaveBeenNthCalledWith(
407+
2,
359408
expect.stringContaining('chat-otp:email:'),
360409
expect.any(Object),
361410
{ failClosed: true }

apps/sim/app/api/chat/[identifier]/otp/route.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
MAX_OTP_ATTEMPTS,
1818
OTP_EMAIL_RATE_LIMIT,
1919
OTP_IP_RATE_LIMIT,
20+
OTP_RESOURCE_RATE_LIMIT,
2021
storeOTP,
2122
} from '@/lib/core/security/otp'
2223
import { generateRequestId, getClientIp } from '@/lib/core/utils/request'
@@ -88,6 +89,24 @@ export const POST = withRouteHandler(
8889
? deployment.allowedEmails
8990
: []
9091

92+
const resourceRateLimit = await rateLimiter.checkRateLimitDirect(
93+
`chat-otp:resource:${deployment.id}`,
94+
OTP_RESOURCE_RATE_LIMIT,
95+
{ failClosed: true }
96+
)
97+
if (!resourceRateLimit.allowed) {
98+
logger.warn(`[${requestId}] OTP resource rate limit exceeded for chat ${deployment.id}`)
99+
const retryAfter = Math.ceil(
100+
(resourceRateLimit.retryAfterMs ?? OTP_RESOURCE_RATE_LIMIT.refillIntervalMs) / 1000
101+
)
102+
const response = createErrorResponse(
103+
'Too many verification code requests. Please try again later.',
104+
429
105+
)
106+
response.headers.set('Retry-After', String(retryAfter))
107+
return response
108+
}
109+
91110
if (!isEmailAllowed(email, allowedEmails)) {
92111
return createErrorResponse('Email not authorized for this chat', 403)
93112
}

apps/sim/app/api/files/public/[token]/otp/route.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import { requestUtilsMockFns } from '@sim/testing'
45
import { NextRequest } from 'next/server'
56
import { beforeEach, describe, expect, it, vi } from 'vitest'
67

@@ -49,6 +50,7 @@ vi.mock('@/lib/core/security/otp', () => ({
4950
MAX_OTP_ATTEMPTS: 5,
5051
OTP_IP_RATE_LIMIT: { maxTokens: 10, refillRate: 10, refillIntervalMs: 1000 },
5152
OTP_EMAIL_RATE_LIMIT: { maxTokens: 3, refillRate: 3, refillIntervalMs: 1000 },
53+
OTP_RESOURCE_RATE_LIMIT: { maxTokens: 100, refillRate: 100, refillIntervalMs: 1000 },
5254
}))
5355
vi.mock('@/components/emails', () => ({
5456
getOtpSubject: (label: string) => `Verification code for ${label}`,
@@ -128,6 +130,40 @@ describe('POST /api/files/public/[token]/otp', () => {
128130
expect(res.status).toBe(429)
129131
expect(res.headers.get('Retry-After')).toBe('1')
130132
})
133+
134+
it('returns 429 when the share resource rate limit is exceeded', async () => {
135+
mockCheckRateLimitDirect
136+
.mockResolvedValueOnce({ allowed: true })
137+
.mockResolvedValueOnce({ allowed: false, retryAfterMs: 1000 })
138+
139+
const res = await POST(post('user@acme.com'), params())
140+
141+
expect(res.status).toBe(429)
142+
expect(res.headers.get('Retry-After')).toBe('1')
143+
expect(mockStoreOTP).not.toHaveBeenCalled()
144+
expect(mockSendEmail).not.toHaveBeenCalled()
145+
})
146+
147+
it('retains resource and email backstops when the client IP cannot be resolved', async () => {
148+
requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce(null)
149+
150+
const res = await POST(post('user@acme.com'), params())
151+
152+
expect(res.status).toBe(200)
153+
expect(mockCheckRateLimitDirect).toHaveBeenCalledTimes(2)
154+
expect(mockCheckRateLimitDirect).toHaveBeenNthCalledWith(
155+
1,
156+
'file-otp:resource:sh_1',
157+
expect.any(Object),
158+
{ failClosed: true }
159+
)
160+
expect(mockCheckRateLimitDirect).toHaveBeenNthCalledWith(
161+
2,
162+
'file-otp:email:sh_1:user@acme.com',
163+
expect.any(Object),
164+
{ failClosed: true }
165+
)
166+
})
131167
})
132168

133169
describe('PUT /api/files/public/[token]/otp', () => {

apps/sim/app/api/files/public/[token]/otp/route.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
MAX_OTP_ATTEMPTS,
2020
OTP_EMAIL_RATE_LIMIT,
2121
OTP_IP_RATE_LIMIT,
22+
OTP_RESOURCE_RATE_LIMIT,
2223
storeOTP,
2324
} from '@/lib/core/security/otp'
2425
import { generateRequestId, getClientIp } from '@/lib/core/utils/request'
@@ -88,6 +89,18 @@ export const POST = withRouteHandler(
8889
)
8990
}
9091

92+
const resourceRateLimit = await rateLimiter.checkRateLimitDirect(
93+
`file-otp:resource:${resolved.share.id}`,
94+
OTP_RESOURCE_RATE_LIMIT,
95+
{ failClosed: true }
96+
)
97+
if (!resourceRateLimit.allowed) {
98+
logger.warn(
99+
`[${requestId}] OTP resource rate limit exceeded for share ${resolved.share.id}`
100+
)
101+
return rateLimited(resourceRateLimit.retryAfterMs, OTP_RESOURCE_RATE_LIMIT.refillIntervalMs)
102+
}
103+
91104
if (!isEmailAllowed(email, shareAllowedEmails(resolved.share.allowedEmails))) {
92105
return NextResponse.json({ error: 'Email not authorized for this file' }, { status: 403 })
93106
}

apps/sim/lib/core/config/env.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,8 @@ export const env = createEnv({
612612
REACT_SCAN_ENABLED: z.boolean().optional(), // Enable React Scan for performance debugging (dev only)
613613

614614
// Network / proxy trust
615-
AUTH_TRUSTED_PROXIES: z.string().optional(), // Comma-separated reverse-proxy IPs or CIDR ranges. Sim and Better Auth walk the forwarded-IP chain right to left, skip these trusted hops, and use the first untrusted address. Invalid and catch-all entries fail at startup.
615+
/** Comma-separated proxy IPs/CIDRs skipped while resolving the forwarded client chain. */
616+
AUTH_TRUSTED_PROXIES: z.string().optional(),
616617

617618
// SSO Configuration (for script-based registration)
618619
SSO_ENABLED: z.boolean().optional(), // Enable SSO functionality

apps/sim/lib/core/security/otp.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ export const OTP_EMAIL_RATE_LIMIT: TokenBucketConfig = {
2828
refillIntervalMs: 15 * 60_000,
2929
}
3030

31+
/** Caps OTP requests against one deployment independently of client identity. */
32+
export const OTP_RESOURCE_RATE_LIMIT: TokenBucketConfig = {
33+
maxTokens: 100,
34+
refillRate: 100,
35+
refillIntervalMs: 15 * 60_000,
36+
}
37+
3138
/**
3239
* Key formats are kept per-kind to preserve any in-flight OTPs already issued
3340
* against existing chat deployments. The chat Redis key uses the legacy `otp:`

0 commit comments

Comments
 (0)