fix(security): isolate rejected OTP attempts

Author: TheodoreSpeaks

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

@@ -252,6 +252,59 @@ describe('Chat OTP API Route', () => {
   })
 
   describe('POST - Rate limiting', () => {
+    it('isolates rejected emails from the OTP send bucket without a client IP', async () => {
+      requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce(null)
+      queueDeployment(emailDeployment)
+
+      const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
+        method: 'POST',
+        body: JSON.stringify({ email: 'not-allowed@example.com' }),
+      })
+
+      const response = await POST(request, {
+        params: Promise.resolve({ identifier: mockIdentifier }),
+      })
+
+      expect(response.status).toBe(403)
+      expect(mockCheckRateLimitDirect).toHaveBeenCalledTimes(1)
+      expect(mockCheckRateLimitDirect).toHaveBeenCalledWith(
+        'chat-otp:rejected:chat-123',
+        expect.any(Object),
+        { failClosed: true }
+      )
+      expect(mockSendEmail).not.toHaveBeenCalled()
+    })
+
+    it('rate limits rejected emails independently without a client IP', async () => {
+      requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce(null)
+      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 },
+      }))
+      queueDeployment(emailDeployment)
+
+      const request = new NextRequest('http://localhost:3000/api/chat/test/otp', {
+        method: 'POST',
+        body: JSON.stringify({ email: 'not-allowed@example.com' }),
+      })
+
+      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('returns 429 with Retry-After when IP rate limit is exceeded', async () => {
       mockCheckRateLimitDirect.mockResolvedValueOnce({
         allowed: false,

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

@@ -89,6 +89,26 @@ export const POST = withRouteHandler(
         ? deployment.allowedEmails
         : []
 
+      if (!isEmailAllowed(email, allowedEmails)) {
+        const rejectedRateLimit = await rateLimiter.checkRateLimitDirect(
+          `chat-otp:rejected:${deployment.id}`,
+          OTP_RESOURCE_RATE_LIMIT,
+          { failClosed: true }
+        )
+        if (!rejectedRateLimit.allowed) {
+          logger.warn(
+            `[${requestId}] OTP rejected-email rate limit exceeded for chat ${deployment.id}`
+          )
+          const retryAfter = Math.ceil(
+            (rejectedRateLimit.retryAfterMs ?? OTP_RESOURCE_RATE_LIMIT.refillIntervalMs) / 1000
+          )
+          const response = createErrorResponse('Too many requests. Please try again later.', 429)
+          response.headers.set('Retry-After', String(retryAfter))
+          return response
+        }
+        return createErrorResponse('Email not authorized for this chat', 403)
+      }
+
       const resourceRateLimit = await rateLimiter.checkRateLimitDirect(
         `chat-otp:resource:${deployment.id}`,
         OTP_RESOURCE_RATE_LIMIT,
@@ -107,10 +127,6 @@ export const POST = withRouteHandler(
         return response
       }
 
-      if (!isEmailAllowed(email, allowedEmails)) {
-        return createErrorResponse('Email not authorized for this chat', 403)
-      }
-
       const emailRateLimit = await rateLimiter.checkRateLimitDirect(
         `chat-otp:email:${deployment.id}:${email.toLowerCase()}`,
         OTP_EMAIL_RATE_LIMIT,

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

@@ -106,9 +106,46 @@ describe('POST /api/files/public/[token]/otp', () => {
     mockIsEmailAllowed.mockReturnValueOnce(false)
     const res = await POST(post('user@evil.com'), params())
     expect(res.status).toBe(403)
+    expect(mockCheckRateLimitDirect).toHaveBeenCalledTimes(2)
+    expect(mockCheckRateLimitDirect).toHaveBeenNthCalledWith(
+      2,
+      'file-otp:rejected:sh_1',
+      expect.any(Object),
+      { failClosed: true }
+    )
     expect(mockStoreOTP).not.toHaveBeenCalled()
   })
 
+  it('isolates rejected emails from the OTP send bucket without a client IP', async () => {
+    requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce(null)
+    mockIsEmailAllowed.mockReturnValueOnce(false)
+
+    const res = await POST(post('user@evil.com'), params())
+
+    expect(res.status).toBe(403)
+    expect(mockCheckRateLimitDirect).toHaveBeenCalledTimes(1)
+    expect(mockCheckRateLimitDirect).toHaveBeenCalledWith(
+      'file-otp:rejected:sh_1',
+      expect.any(Object),
+      { failClosed: true }
+    )
+    expect(mockStoreOTP).not.toHaveBeenCalled()
+    expect(mockSendEmail).not.toHaveBeenCalled()
+  })
+
+  it('rate limits rejected emails independently without a client IP', async () => {
+    requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce(null)
+    mockIsEmailAllowed.mockReturnValueOnce(false)
+    mockCheckRateLimitDirect.mockResolvedValueOnce({ allowed: false, retryAfterMs: 1000 })
+
+    const res = await POST(post('user@evil.com'), params())
+
+    expect(res.status).toBe(429)
+    expect(res.headers.get('Retry-After')).toBe('1')
+    expect(mockStoreOTP).not.toHaveBeenCalled()
+    expect(mockSendEmail).not.toHaveBeenCalled()
+  })
+
   it('lowercases the email for allow-list matching and OTP storage', async () => {
     await POST(post('User@ACME.com'), params())
     expect(mockIsEmailAllowed).toHaveBeenCalledWith('user@acme.com', expect.anything())

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

@@ -89,6 +89,24 @@ export const POST = withRouteHandler(
         )
       }
 
+      if (!isEmailAllowed(email, shareAllowedEmails(resolved.share.allowedEmails))) {
+        const rejectedRateLimit = await rateLimiter.checkRateLimitDirect(
+          `file-otp:rejected:${resolved.share.id}`,
+          OTP_RESOURCE_RATE_LIMIT,
+          { failClosed: true }
+        )
+        if (!rejectedRateLimit.allowed) {
+          logger.warn(
+            `[${requestId}] OTP rejected-email rate limit exceeded for share ${resolved.share.id}`
+          )
+          return rateLimited(
+            rejectedRateLimit.retryAfterMs,
+            OTP_RESOURCE_RATE_LIMIT.refillIntervalMs
+          )
+        }
+        return NextResponse.json({ error: 'Email not authorized for this file' }, { status: 403 })
+      }
+
       const resourceRateLimit = await rateLimiter.checkRateLimitDirect(
         `file-otp:resource:${resolved.share.id}`,
         OTP_RESOURCE_RATE_LIMIT,
@@ -101,10 +119,6 @@ export const POST = withRouteHandler(
         return rateLimited(resourceRateLimit.retryAfterMs, OTP_RESOURCE_RATE_LIMIT.refillIntervalMs)
       }
 
-      if (!isEmailAllowed(email, shareAllowedEmails(resolved.share.allowedEmails))) {
-        return NextResponse.json({ error: 'Email not authorized for this file' }, { status: 403 })
-      }
-
       const emailRateLimit = await rateLimiter.checkRateLimitDirect(
         `file-otp:email:${resolved.share.id}:${email}`,
         OTP_EMAIL_RATE_LIMIT,