Skip to content

Commit 8266f0a

Browse files
authored
fix(security): rate limit chat OTP + validate mothership proxy endpoint (#4312)
* fix(security): rate limit chat OTP endpoint to prevent email bombing * fix(security): validate mothership proxy endpoint to block path traversal * fix(security): address greptile feedback on OTP rate limiting
1 parent 896a00a commit 8266f0a

3 files changed

Lines changed: 214 additions & 1 deletion

File tree

apps/sim/app/api/admin/mothership/route.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ function getMothershipUrl(environment: string): string | null {
1616
return ENV_URLS[environment] ?? null
1717
}
1818

19+
const ENDPOINT_PATTERN = /^[a-zA-Z0-9_-]+(?:\/[a-zA-Z0-9_-]+)*$/
20+
21+
function isValidEndpoint(endpoint: string): boolean {
22+
if (!endpoint) return false
23+
if (endpoint.includes('..')) return false
24+
return ENDPOINT_PATTERN.test(endpoint)
25+
}
26+
1927
async function isAdminRequestAuthorized() {
2028
const session = await getSession()
2129
if (!session?.user?.id) return false
@@ -57,6 +65,10 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
5765
return NextResponse.json({ error: 'endpoint query param required' }, { status: 400 })
5866
}
5967

68+
if (!isValidEndpoint(endpoint)) {
69+
return NextResponse.json({ error: 'invalid endpoint' }, { status: 400 })
70+
}
71+
6072
const baseUrl = getMothershipUrl(environment)
6173
if (!baseUrl) {
6274
return NextResponse.json(
@@ -108,6 +120,10 @@ export const GET = withRouteHandler(async (req: NextRequest) => {
108120
return NextResponse.json({ error: 'endpoint query param required' }, { status: 400 })
109121
}
110122

123+
if (!isValidEndpoint(endpoint)) {
124+
return NextResponse.json({ error: 'invalid endpoint' }, { status: 400 })
125+
}
126+
111127
const baseUrl = getMothershipUrl(environment)
112128
if (!baseUrl) {
113129
return NextResponse.json(

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

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,16 @@ vi.mock('@/lib/core/storage', () => ({
112112
getStorageMethod: mockGetStorageMethod,
113113
}))
114114

115+
const { mockCheckRateLimitDirect } = vi.hoisted(() => ({
116+
mockCheckRateLimitDirect: vi.fn(),
117+
}))
118+
119+
vi.mock('@/lib/core/rate-limiter', () => ({
120+
RateLimiter: class {
121+
checkRateLimitDirect = mockCheckRateLimitDirect
122+
},
123+
}))
124+
115125
vi.mock('@/lib/messaging/email/mailer', () => ({
116126
sendEmail: mockSendEmail,
117127
}))
@@ -234,6 +244,13 @@ describe('Chat OTP API Route', () => {
234244
}))
235245

236246
requestUtilsMockFns.mockGenerateRequestId.mockReturnValue('req-123')
247+
requestUtilsMockFns.mockGetClientIp.mockReturnValue('1.2.3.4')
248+
249+
mockCheckRateLimitDirect.mockResolvedValue({
250+
allowed: true,
251+
remaining: 10,
252+
resetAt: new Date(Date.now() + 60_000),
253+
})
237254

238255
mockZodParse.mockImplementation((data: unknown) => data)
239256

@@ -283,6 +300,134 @@ describe('Chat OTP API Route', () => {
283300
})
284301
})
285302

303+
describe('POST - Rate limiting', () => {
304+
const buildDeploymentSelect = () =>
305+
mockDbSelect.mockImplementationOnce(() => ({
306+
from: vi.fn().mockReturnValue({
307+
where: vi.fn().mockReturnValue({
308+
limit: vi.fn().mockResolvedValue([
309+
{
310+
id: mockChatId,
311+
authType: 'email',
312+
allowedEmails: [mockEmail],
313+
title: 'Test Chat',
314+
},
315+
]),
316+
}),
317+
}),
318+
}))
319+
320+
it('returns 429 with Retry-After when IP rate limit is exceeded', async () => {
321+
mockCheckRateLimitDirect.mockResolvedValueOnce({
322+
allowed: false,
323+
remaining: 0,
324+
resetAt: new Date(Date.now() + 900_000),
325+
retryAfterMs: 900_000,
326+
})
327+
328+
const headerSet = vi.fn()
329+
mockCreateErrorResponse.mockImplementationOnce((message: string, status: number) => ({
330+
json: () => Promise.resolve({ error: message }),
331+
status,
332+
headers: { set: headerSet },
333+
}))
334+
335+
const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
336+
method: 'POST',
337+
body: JSON.stringify({ email: mockEmail }),
338+
})
339+
340+
const response = await POST(request, {
341+
params: Promise.resolve({ identifier: mockIdentifier }),
342+
})
343+
344+
expect(response.status).toBe(429)
345+
expect(headerSet).toHaveBeenCalledWith('Retry-After', '900')
346+
expect(mockSendEmail).not.toHaveBeenCalled()
347+
expect(mockDbSelect).not.toHaveBeenCalled()
348+
})
349+
350+
it('returns 429 with Retry-After when email rate limit is exceeded', async () => {
351+
mockCheckRateLimitDirect
352+
.mockResolvedValueOnce({
353+
allowed: true,
354+
remaining: 9,
355+
resetAt: new Date(Date.now() + 60_000),
356+
})
357+
.mockResolvedValueOnce({
358+
allowed: false,
359+
remaining: 0,
360+
resetAt: new Date(Date.now() + 900_000),
361+
retryAfterMs: 900_000,
362+
})
363+
364+
const headerSet = vi.fn()
365+
mockCreateErrorResponse.mockImplementationOnce((message: string, status: number) => ({
366+
json: () => Promise.resolve({ error: message }),
367+
status,
368+
headers: { set: headerSet },
369+
}))
370+
371+
buildDeploymentSelect()
372+
373+
const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
374+
method: 'POST',
375+
body: JSON.stringify({ email: mockEmail }),
376+
})
377+
378+
const response = await POST(request, {
379+
params: Promise.resolve({ identifier: mockIdentifier }),
380+
})
381+
382+
expect(response.status).toBe(429)
383+
expect(headerSet).toHaveBeenCalledWith('Retry-After', '900')
384+
expect(mockSendEmail).not.toHaveBeenCalled()
385+
})
386+
387+
it('falls back to refill interval when retryAfterMs is missing', async () => {
388+
mockCheckRateLimitDirect.mockResolvedValueOnce({
389+
allowed: false,
390+
remaining: 0,
391+
resetAt: new Date(Date.now() + 900_000),
392+
})
393+
394+
const headerSet = vi.fn()
395+
mockCreateErrorResponse.mockImplementationOnce((message: string, status: number) => ({
396+
json: () => Promise.resolve({ error: message }),
397+
status,
398+
headers: { set: headerSet },
399+
}))
400+
401+
const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
402+
method: 'POST',
403+
body: JSON.stringify({ email: mockEmail }),
404+
})
405+
406+
await POST(request, { params: Promise.resolve({ identifier: mockIdentifier }) })
407+
408+
expect(headerSet).toHaveBeenCalledWith('Retry-After', '900')
409+
})
410+
411+
it('skips IP rate limit when client IP is unknown', async () => {
412+
requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce('unknown')
413+
buildDeploymentSelect()
414+
415+
const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
416+
method: 'POST',
417+
body: JSON.stringify({ email: mockEmail }),
418+
})
419+
420+
await POST(request, { params: Promise.resolve({ identifier: mockIdentifier }) })
421+
422+
// Only the email-scoped check should run, not the IP-scoped one
423+
expect(mockCheckRateLimitDirect).toHaveBeenCalledTimes(1)
424+
expect(mockCheckRateLimitDirect).toHaveBeenCalledWith(
425+
expect.stringContaining('chat-otp:email:'),
426+
expect.any(Object)
427+
)
428+
})
429+
})
430+
286431
describe('POST - Store OTP (Database path)', () => {
287432
beforeEach(() => {
288433
mockGetStorageMethod.mockReturnValue('database')

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

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,32 @@ import type { NextRequest } from 'next/server'
88
import { z } from 'zod'
99
import { renderOTPEmail } from '@/components/emails'
1010
import { getRedisClient } from '@/lib/core/config/redis'
11+
import type { TokenBucketConfig } from '@/lib/core/rate-limiter'
12+
import { RateLimiter } from '@/lib/core/rate-limiter'
1113
import { addCorsHeaders, isEmailAllowed } from '@/lib/core/security/deployment'
1214
import { getStorageMethod } from '@/lib/core/storage'
13-
import { generateRequestId } from '@/lib/core/utils/request'
15+
import { generateRequestId, getClientIp } from '@/lib/core/utils/request'
1416
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1517
import { sendEmail } from '@/lib/messaging/email/mailer'
1618
import { setChatAuthCookie } from '@/app/api/chat/utils'
1719
import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils'
1820

1921
const logger = createLogger('ChatOtpAPI')
2022

23+
const rateLimiter = new RateLimiter()
24+
25+
const OTP_IP_RATE_LIMIT: TokenBucketConfig = {
26+
maxTokens: 10,
27+
refillRate: 10,
28+
refillIntervalMs: 15 * 60_000,
29+
}
30+
31+
const OTP_EMAIL_RATE_LIMIT: TokenBucketConfig = {
32+
maxTokens: 3,
33+
refillRate: 3,
34+
refillIntervalMs: 15 * 60_000,
35+
}
36+
2137
function generateOTP(): string {
2238
return randomInt(100000, 1000000).toString()
2339
}
@@ -214,6 +230,23 @@ export const POST = withRouteHandler(
214230
const requestId = generateRequestId()
215231

216232
try {
233+
const ip = getClientIp(request)
234+
if (ip !== 'unknown') {
235+
const ipRateLimit = await rateLimiter.checkRateLimitDirect(
236+
`chat-otp:ip:${identifier}:${ip}`,
237+
OTP_IP_RATE_LIMIT
238+
)
239+
if (!ipRateLimit.allowed) {
240+
logger.warn(`[${requestId}] OTP IP rate limit exceeded for ${identifier} from ${ip}`)
241+
const retryAfter = Math.ceil(
242+
(ipRateLimit.retryAfterMs ?? OTP_IP_RATE_LIMIT.refillIntervalMs) / 1000
243+
)
244+
const response = createErrorResponse('Too many requests. Please try again later.', 429)
245+
response.headers.set('Retry-After', String(retryAfter))
246+
return addCorsHeaders(response, request)
247+
}
248+
}
249+
217250
const body = await request.json()
218251
const { email } = otpRequestSchema.parse(body)
219252

@@ -255,6 +288,25 @@ export const POST = withRouteHandler(
255288
)
256289
}
257290

291+
const emailRateLimit = await rateLimiter.checkRateLimitDirect(
292+
`chat-otp:email:${deployment.id}:${email.toLowerCase()}`,
293+
OTP_EMAIL_RATE_LIMIT
294+
)
295+
if (!emailRateLimit.allowed) {
296+
logger.warn(
297+
`[${requestId}] OTP email rate limit exceeded for ${email} on chat ${deployment.id}`
298+
)
299+
const retryAfter = Math.ceil(
300+
(emailRateLimit.retryAfterMs ?? OTP_EMAIL_RATE_LIMIT.refillIntervalMs) / 1000
301+
)
302+
const response = createErrorResponse(
303+
'Too many verification code requests. Please try again later.',
304+
429
305+
)
306+
response.headers.set('Retry-After', String(retryAfter))
307+
return addCorsHeaders(response, request)
308+
}
309+
258310
const otp = generateOTP()
259311
await storeOTP(email, deployment.id, otp)
260312

0 commit comments

Comments
 (0)