fix(security): fail closed without client IP

Author: TheodoreSpeaks

apps/sim/app/api/chat/utils.test.ts

@@ -5,6 +5,7 @@
  */
 import {
   authMockFns,
+  createMockRequest,
   encryptionMock,
   encryptionMockFns,
   loggingSessionMock,
@@ -290,10 +291,7 @@ describe('Chat API Utils', () => {
         authType: 'password',
         password: 'encrypted-password',
       }
-      const mockRequest = {
-        method: 'POST',
-        cookies: { get: vi.fn().mockReturnValue(null) },
-      } as any
+      const mockRequest = createMockRequest('POST')
       const candidate = 'password-attempt-fixture'
 
       const result = await validateChatAuth('request-id', deployment, mockRequest, {
@@ -319,13 +317,11 @@ describe('Chat API Utils', () => {
         authType: 'password',
         password: 'encrypted-password',
       }
-      const mockRequest = {
-        method: 'POST',
-        cookies: { get: vi.fn().mockReturnValue(null) },
-      } as any
+      const mockRequest = createMockRequest('POST')
+      const candidate = 'correct-password'
 
       const result = await validateChatAuth('request-id', deployment, mockRequest, {
-        password: 'correct-password',
+        password: candidate,
       })
 
       expect(result.authorized).toBe(true)

apps/sim/app/api/contact/route.ts

@@ -54,20 +54,22 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
 
   try {
     const ip = getClientIp(req)
-    if (ip) {
-      const storageKey = `public:contact:${ip}`
-      const { allowed, remaining, resetAt } = await rateLimiter.checkRateLimitDirect(
-        storageKey,
-        PUBLIC_ENDPOINT_RATE_LIMIT
-      )
-
-      if (!allowed) {
-        logger.warn(`[${requestId}] Rate limit exceeded for IP ${ip}`, { remaining, resetAt })
-        return NextResponse.json(TOO_MANY_REQUESTS_RESPONSE, {
-          status: 429,
-          headers: { 'Retry-After': String(Math.ceil((resetAt.getTime() - Date.now()) / 1000)) },
-        })
-      }
+    if (!ip) {
+      logger.warn(`[${requestId}] Unable to resolve client IP for public rate limit`)
+      return NextResponse.json(TOO_MANY_REQUESTS_RESPONSE, { status: 429 })
+    }
+    const storageKey = `public:contact:${ip}`
+    const { allowed, remaining, resetAt } = await rateLimiter.checkRateLimitDirect(
+      storageKey,
+      PUBLIC_ENDPOINT_RATE_LIMIT
+    )
+
+    if (!allowed) {
+      logger.warn(`[${requestId}] Rate limit exceeded for IP ${ip}`, { remaining, resetAt })
+      return NextResponse.json(TOO_MANY_REQUESTS_RESPONSE, {
+        status: 429,
+        headers: { 'Retry-After': String(Math.ceil((resetAt.getTime() - Date.now()) / 1000)) },
+      })
     }
 
     const parsed = await parseRequest(submitContactContract, req, {})
@@ -90,7 +92,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
         typeof captchaToken === 'string' && captchaToken.length > 0 ? captchaToken : null
 
       if (token) {
-        const verification = await verifyTurnstileToken({ token, remoteIp: ip ?? undefined })
+        const verification = await verifyTurnstileToken({ token, remoteIp: ip })
         if (verification.success) {
           captchaVerified = true
         } else if (!verification.transportError) {
@@ -111,10 +113,6 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
       }
 
       if (!captchaVerified) {
-        if (!ip) {
-          logger.warn(`[${requestId}] Cannot enforce no-captcha rate limit without a client IP`)
-          return NextResponse.json(TOO_MANY_REQUESTS_RESPONSE, { status: 429 })
-        }
         const nocaptchaKey = `public:contact:nocaptcha:${ip}`
         const { allowed: nocaptchaAllowed } = await rateLimiter.checkRateLimitDirect(
           nocaptchaKey,

apps/sim/app/api/demo-requests/route.ts

@@ -28,23 +28,28 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
 
   try {
     const ip = getClientIp(req)
-    if (ip) {
-      const storageKey = `public:demo-request:${ip}`
-      const { allowed, remaining, resetAt } = await rateLimiter.checkRateLimitDirect(
-        storageKey,
-        PUBLIC_ENDPOINT_RATE_LIMIT
+    if (!ip) {
+      logger.warn(`[${requestId}] Unable to resolve client IP for public rate limit`)
+      return NextResponse.json(
+        { error: 'Too many requests. Please try again later.' },
+        { status: 429 }
       )
+    }
+    const storageKey = `public:demo-request:${ip}`
+    const { allowed, remaining, resetAt } = await rateLimiter.checkRateLimitDirect(
+      storageKey,
+      PUBLIC_ENDPOINT_RATE_LIMIT
+    )
 
-      if (!allowed) {
-        logger.warn(`[${requestId}] Rate limit exceeded for IP ${ip}`, { remaining, resetAt })
-        return NextResponse.json(
-          { error: 'Too many requests. Please try again later.' },
-          {
-            status: 429,
-            headers: { 'Retry-After': String(Math.ceil((resetAt.getTime() - Date.now()) / 1000)) },
-          }
-        )
-      }
+    if (!allowed) {
+      logger.warn(`[${requestId}] Rate limit exceeded for IP ${ip}`, { remaining, resetAt })
+      return NextResponse.json(
+        { error: 'Too many requests. Please try again later.' },
+        {
+          status: 429,
+          headers: { 'Retry-After': String(Math.ceil((resetAt.getTime() - Date.now()) / 1000)) },
+        }
+      )
     }
 
     const parsed = await parseRequest(submitDemoRequestContract, req, {})

apps/sim/app/api/help/integration-request/route.ts

@@ -26,23 +26,28 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
 
   try {
     const ip = getClientIp(req)
-    if (ip) {
-      const storageKey = `public:integration-request:${ip}`
-      const { allowed, remaining, resetAt } = await rateLimiter.checkRateLimitDirect(
-        storageKey,
-        PUBLIC_ENDPOINT_RATE_LIMIT
+    if (!ip) {
+      logger.warn(`[${requestId}] Unable to resolve client IP for public rate limit`)
+      return NextResponse.json(
+        { error: 'Too many requests. Please try again later.' },
+        { status: 429 }
       )
+    }
+    const storageKey = `public:integration-request:${ip}`
+    const { allowed, remaining, resetAt } = await rateLimiter.checkRateLimitDirect(
+      storageKey,
+      PUBLIC_ENDPOINT_RATE_LIMIT
+    )
 
-      if (!allowed) {
-        logger.warn(`[${requestId}] Rate limit exceeded for IP ${ip}`, { remaining, resetAt })
-        return NextResponse.json(
-          { error: 'Too many requests. Please try again later.' },
-          {
-            status: 429,
-            headers: { 'Retry-After': String(Math.ceil((resetAt.getTime() - Date.now()) / 1000)) },
-          }
-        )
-      }
+    if (!allowed) {
+      logger.warn(`[${requestId}] Rate limit exceeded for IP ${ip}`, { remaining, resetAt })
+      return NextResponse.json(
+        { error: 'Too many requests. Please try again later.' },
+        {
+          status: 429,
+          headers: { 'Retry-After': String(Math.ceil((resetAt.getTime() - Date.now()) / 1000)) },
+        }
+      )
     }
 
     const parsed = await parseRequest(

apps/sim/lib/api/server/routes/v2-json-route.ts

@@ -277,7 +277,16 @@ export const v2OrchestrationErrorPolicy = {
 
 async function enforceV2PreAuthIpLimit(request: NextRequest): Promise<NextResponse | null> {
   const ip = getClientIp(request)
-  if (!ip) return null
+  if (!ip) {
+    const resetAt = new Date(Date.now() + V2_PREAUTH_IP_LIMIT.refillIntervalMs)
+    return v2RateLimitError({
+      allowed: false,
+      limit: V2_PREAUTH_IP_LIMIT.maxTokens,
+      remaining: 0,
+      resetAt,
+      retryAfterMs: V2_PREAUTH_IP_LIMIT.refillIntervalMs,
+    })
+  }
   const abuseLimit = await rateLimiter.checkRateLimitDirect(
     `v2:preauth:ip:${ip}`,
     V2_PREAUTH_IP_LIMIT,

apps/sim/lib/core/rate-limiter/route-helpers.test.ts

@@ -116,13 +116,14 @@ describe('route-helpers rate limiting', () => {
       )
     })
 
-    it('does not create a shared bucket when the client IP cannot be resolved', async () => {
+    it('fails closed without creating a shared bucket when the client IP cannot be resolved', async () => {
       requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce(null)
       const request = createMockRequest('POST')
 
       const result = await enforceIpRateLimit('otp', request)
 
-      expect(result).toBeNull()
+      expect(result?.status).toBe(429)
+      expect(result?.headers.get('Retry-After')).toBe('60')
       expect(consume).not.toHaveBeenCalled()
     })
 

apps/sim/lib/core/rate-limiter/route-helpers.ts

@@ -56,14 +56,17 @@ export async function enforceUserRateLimit(
   return buildRateLimitResponse(resetAt)
 }
 
-/** Apply a per-IP token bucket when the forwarded chain resolves safely. */
+/** Apply a per-IP token bucket and fail closed when the client cannot be resolved safely. */
 export async function enforceIpRateLimit(
   bucketName: string,
   request: NextRequest,
   config: TokenBucketConfig = DEFAULT_PUBLIC_IP_ROUTE_LIMIT
 ): Promise<NextResponse | null> {
   const ip = getClientIp(request)
-  if (!ip) return null
+  if (!ip) {
+    logger.warn('Unable to resolve client IP for public rate limit', { bucket: bucketName })
+    return buildRateLimitResponse(new Date(Date.now() + config.refillIntervalMs))
+  }
   const key = `route:${bucketName}:ip:${ip}`
   const { allowed, resetAt } = await rateLimiter.checkRateLimitDirect(key, config)
   if (allowed) return null

apps/sim/lib/credential-groups/rate-limit.ts

@@ -65,7 +65,7 @@ export async function enforcePublicCredentialGroupIpRateLimit(
 ): Promise<NextResponse | null> {
   const config = configForPublicScope(scope)
   const ip = getClientIp(request)
-  if (!ip) return null
+  if (!ip) return rateLimitResponse(undefined, config.refillIntervalMs)
   const result = await rateLimiter.checkRateLimitDirect(
     `public-credential-group:${scope}:ip:${ip}`,
     config,

apps/sim/lib/public-shares/rate-limit.test.ts

@@ -0,0 +1,33 @@
+/**
+ * @vitest-environment node
+ */
+import { requestUtilsMockFns } from '@sim/testing'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockCheckRateLimitDirect } = vi.hoisted(() => ({
+  mockCheckRateLimitDirect: vi.fn(),
+}))
+
+vi.mock('@/lib/core/rate-limiter', () => ({
+  RateLimiter: class {
+    checkRateLimitDirect = mockCheckRateLimitDirect
+  },
+}))
+
+import { enforcePublicFileRateLimit } from '@/lib/public-shares/rate-limit'
+
+describe('enforcePublicFileRateLimit', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('fails closed without creating a shared bucket when the client IP cannot be resolved', async () => {
+    requestUtilsMockFns.mockGetClientIp.mockReturnValueOnce(null)
+
+    const response = await enforcePublicFileRateLimit(new Request('http://localhost'), 'content')
+
+    expect(response?.status).toBe(429)
+    expect(response?.headers.get('Retry-After')).toBe('60')
+    expect(mockCheckRateLimitDirect).not.toHaveBeenCalled()
+  })
+})

apps/sim/lib/public-shares/rate-limit.ts

@@ -22,16 +22,21 @@ const CONTENT_RATE_LIMIT: TokenBucketConfig = {
  * Per-IP rate limit for the unauthenticated public share endpoints, returning a
  * `429` response when exceeded (or `null` to proceed). The token is unguessable,
  * so this defends a *known* link against hammering (DoS / S3 egress) rather than
- * enumeration. Fails open on storage errors (availability over strictness),
- * matching the chat public route.
+ * enumeration. Fails open on storage errors (availability over strictness), but
+ * fails closed when the forwarded chain cannot identify a safe client key.
  */
 export async function enforcePublicFileRateLimit(
   request: { headers: { get(name: string): string | null } },
   scope: 'metadata' | 'content'
 ): Promise<NextResponse | null> {
-  const ip = getClientIp(request)
-  if (!ip) return null
   const config = scope === 'content' ? CONTENT_RATE_LIMIT : METADATA_RATE_LIMIT
+  const ip = getClientIp(request)
+  if (!ip) {
+    return NextResponse.json(
+      { error: 'Too many requests. Please try again later.' },
+      { status: 429, headers: { 'Retry-After': String(config.refillIntervalMs / 1000) } }
+    )
+  }
   const result = await rateLimiter.checkRateLimitDirect(`public-file:${scope}:${ip}`, config)
   if (result.allowed) return null