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
2026-09-24 15:45:35 +08:00 · 2026-04-27 14:51:58 -07:00
parent 896a00ae31
commit 8266f0afdb
3 changed files with 214 additions and 1 deletions
@@ -16,6 +16,14 @@ function getMothershipUrl(environment: string): string | null {
  return ENV_URLS[environment] ?? null
 }

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

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

+  if (!isValidEndpoint(endpoint)) {
+    return NextResponse.json({ error: 'invalid endpoint' }, { status: 400 })
+  }
+
  const baseUrl = getMothershipUrl(environment)
  if (!baseUrl) {
    return NextResponse.json(
@@ -112,6 +112,16 @@ vi.mock('@/lib/core/storage', () => ({
  getStorageMethod: mockGetStorageMethod,
 }))

+const { mockCheckRateLimitDirect } = vi.hoisted(() => ({
+  mockCheckRateLimitDirect: vi.fn(),
+}))
+
+vi.mock('@/lib/core/rate-limiter', () => ({
+  RateLimiter: class {
+    checkRateLimitDirect = mockCheckRateLimitDirect
+  },
+}))
+
 vi.mock('@/lib/messaging/email/mailer', () => ({
  sendEmail: mockSendEmail,
 }))
@@ -234,6 +244,13 @@ describe('Chat OTP API Route', () => {
    }))

    requestUtilsMockFns.mockGenerateRequestId.mockReturnValue('req-123')
+    requestUtilsMockFns.mockGetClientIp.mockReturnValue('1.2.3.4')
+
+    mockCheckRateLimitDirect.mockResolvedValue({
+      allowed: true,
+      remaining: 10,
+      resetAt: new Date(Date.now() + 60_000),
+    })

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

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

+  describe('POST - Rate limiting', () => {
+    const buildDeploymentSelect = () =>
+      mockDbSelect.mockImplementationOnce(() => ({
+        from: vi.fn().mockReturnValue({
+          where: vi.fn().mockReturnValue({
+            limit: vi.fn().mockResolvedValue([
+              {
+                id: mockChatId,
+                authType: 'email',
+                allowedEmails: [mockEmail],
+                title: 'Test Chat',
+              },
+            ]),
+          }),
+        }),
+      }))
+
+    it('returns 429 with Retry-After when IP rate limit is exceeded', async () => {
+      mockCheckRateLimitDirect.mockResolvedValueOnce({
+        allowed: false,
+        remaining: 0,
+        resetAt: new Date(Date.now() + 900_000),
+        retryAfterMs: 900_000,
+      })
+
+      const headerSet = vi.fn()
+      mockCreateErrorResponse.mockImplementationOnce((message: string, status: number) => ({
+        json: () => Promise.resolve({ error: message }),
+        status,
+        headers: { set: headerSet },
+      }))
+
+      const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
+        method: 'POST',
+        body: JSON.stringify({ email: mockEmail }),
+      })
+
+      const response = await POST(request, {
+        params: Promise.resolve({ identifier: mockIdentifier }),
+      })
+
+      expect(response.status).toBe(429)
+      expect(headerSet).toHaveBeenCalledWith('Retry-After', '900')
+      expect(mockSendEmail).not.toHaveBeenCalled()
+      expect(mockDbSelect).not.toHaveBeenCalled()
+    })
+
+    it('returns 429 with Retry-After when email rate limit is exceeded', async () => {
+      mockCheckRateLimitDirect
+        .mockResolvedValueOnce({
+          allowed: true,
+          remaining: 9,
+          resetAt: new Date(Date.now() + 60_000),
+        })
+        .mockResolvedValueOnce({
+          allowed: false,
+          remaining: 0,
+          resetAt: new Date(Date.now() + 900_000),
+          retryAfterMs: 900_000,
+        })
+
+      const headerSet = vi.fn()
+      mockCreateErrorResponse.mockImplementationOnce((message: string, status: number) => ({
+        json: () => Promise.resolve({ error: message }),
+        status,
+        headers: { set: headerSet },
+      }))
+
+      buildDeploymentSelect()
+
+      const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
+        method: 'POST',
+        body: JSON.stringify({ email: mockEmail }),
+      })
+
+      const response = await POST(request, {
+        params: Promise.resolve({ identifier: mockIdentifier }),
+      })
+
+      expect(response.status).toBe(429)
+      expect(headerSet).toHaveBeenCalledWith('Retry-After', '900')
+      expect(mockSendEmail).not.toHaveBeenCalled()
+    })
+
+    it('falls back to refill interval when retryAfterMs is missing', async () => {
+      mockCheckRateLimitDirect.mockResolvedValueOnce({
+        allowed: false,
+        remaining: 0,
+        resetAt: new Date(Date.now() + 900_000),
+      })
+
+      const headerSet = vi.fn()
+      mockCreateErrorResponse.mockImplementationOnce((message: string, status: number) => ({
+        json: () => Promise.resolve({ error: message }),
+        status,
+        headers: { set: headerSet },
+      }))
+
+      const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
+        method: 'POST',
+        body: JSON.stringify({ email: mockEmail }),
+      })
+
+      await POST(request, { params: Promise.resolve({ identifier: mockIdentifier }) })
+
+      expect(headerSet).toHaveBeenCalledWith('Retry-After', '900')
+    })
+
+    it('skips IP rate limit when client IP is unknown', async () => {
+      requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce('unknown')
+      buildDeploymentSelect()
+
+      const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
+        method: 'POST',
+        body: JSON.stringify({ email: mockEmail }),
+      })
+
+      await POST(request, { params: Promise.resolve({ identifier: mockIdentifier }) })
+
+      // Only the email-scoped check should run, not the IP-scoped one
+      expect(mockCheckRateLimitDirect).toHaveBeenCalledTimes(1)
+      expect(mockCheckRateLimitDirect).toHaveBeenCalledWith(
+        expect.stringContaining('chat-otp:email:'),
+        expect.any(Object)
+      )
+    })
+  })
+
  describe('POST - Store OTP (Database path)', () => {
    beforeEach(() => {
      mockGetStorageMethod.mockReturnValue('database')
@@ -8,9 +8,11 @@ import type { NextRequest } from 'next/server'
 import { z } from 'zod'
 import { renderOTPEmail } from '@/components/emails'
 import { getRedisClient } from '@/lib/core/config/redis'
+import type { TokenBucketConfig } from '@/lib/core/rate-limiter'
+import { RateLimiter } from '@/lib/core/rate-limiter'
 import { addCorsHeaders, isEmailAllowed } from '@/lib/core/security/deployment'
 import { getStorageMethod } from '@/lib/core/storage'
-import { generateRequestId } from '@/lib/core/utils/request'
+import { generateRequestId, getClientIp } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { sendEmail } from '@/lib/messaging/email/mailer'
 import { setChatAuthCookie } from '@/app/api/chat/utils'
@@ -18,6 +20,20 @@ import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/

 const logger = createLogger('ChatOtpAPI')

+const rateLimiter = new RateLimiter()
+
+const OTP_IP_RATE_LIMIT: TokenBucketConfig = {
+  maxTokens: 10,
+  refillRate: 10,
+  refillIntervalMs: 15 * 60_000,
+}
+
+const OTP_EMAIL_RATE_LIMIT: TokenBucketConfig = {
+  maxTokens: 3,
+  refillRate: 3,
+  refillIntervalMs: 15 * 60_000,
+}
+
 function generateOTP(): string {
  return randomInt(100000, 1000000).toString()
 }
@@ -214,6 +230,23 @@ export const POST = withRouteHandler(
    const requestId = generateRequestId()

    try {
+      const ip = getClientIp(request)
+      if (ip !== 'unknown') {
+        const ipRateLimit = await rateLimiter.checkRateLimitDirect(
+          `chat-otp:ip:${identifier}:${ip}`,
+          OTP_IP_RATE_LIMIT
+        )
+        if (!ipRateLimit.allowed) {
+          logger.warn(`[${requestId}] OTP IP rate limit exceeded for ${identifier} from ${ip}`)
+          const retryAfter = Math.ceil(
+            (ipRateLimit.retryAfterMs ?? OTP_IP_RATE_LIMIT.refillIntervalMs) / 1000
+          )
+          const response = createErrorResponse('Too many requests. Please try again later.', 429)
+          response.headers.set('Retry-After', String(retryAfter))
+          return addCorsHeaders(response, request)
+        }
+      }
+
      const body = await request.json()
      const { email } = otpRequestSchema.parse(body)

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

+      const emailRateLimit = await rateLimiter.checkRateLimitDirect(
+        `chat-otp:email:${deployment.id}:${email.toLowerCase()}`,
+        OTP_EMAIL_RATE_LIMIT
+      )
+      if (!emailRateLimit.allowed) {
+        logger.warn(
+          `[${requestId}] OTP email rate limit exceeded for ${email} on chat ${deployment.id}`
+        )
+        const retryAfter = Math.ceil(
+          (emailRateLimit.retryAfterMs ?? OTP_EMAIL_RATE_LIMIT.refillIntervalMs) / 1000
+        )
+        const response = createErrorResponse(
+          'Too many verification code requests. Please try again later.',
+          429
+        )
+        response.headers.set('Retry-After', String(retryAfter))
+        return addCorsHeaders(response, request)
+      }
+
      const otp = generateOTP()
      await storeOTP(email, deployment.id, otp)