fix(security): stop believing forwarded headers when nothing appends them

Walking the chain right to left only means anything if a proxy wrote part of
it. docker-compose.prod.yml publishes port 3000 directly and ships no reverse
proxy, so on that reference deployment the whole header is caller-authored and
every per-IP limit stayed bypassable no matter which hop we read.

No parsing rule can recover a real address from a header nobody vouched for, so
make it explicit: TRUST_PROXY_HEADERS declares whether a proxy is in front.
False, getClientIp reports 'unknown' and per-IP limits collapse into one shared
bucket — blunt, and it throttles unrelated callers together, but it fails closed
instead of handing out a fresh bucket per request.

Defaults to true, preserving behavior for the ingress-fronted chart and hosted
deployments. docker-compose.prod.yml defaults it to false, because that file
knows it has no proxy; operators flip it when they put one in front.

The audit package mirrors the flag: recording a caller-authored address as
forensic evidence is worse than recording none.

Author: waleedlatif1

apps/sim/.env.example

@@ -20,6 +20,7 @@ NEXT_PUBLIC_APP_URL=http://localhost:3000
 # INTERNAL_API_BASE_URL=http://sim-app.default.svc.cluster.local:3000  # Optional: internal URL for server-side /api self-calls; defaults to NEXT_PUBLIC_APP_URL
 # TRUSTED_ORIGINS=https://www.example.com,https://app.example.com  # Optional: comma-separated additional public origins to trust for auth (apex+www, alias domains). Merged into Better Auth trustedOrigins.
 # AUTH_TRUSTED_PROXIES=10.0.0.0/24,192.0.2.10  # Optional: reverse-proxy IPs/CIDRs in front of the app. When set, Better Auth and Sim's own per-IP throttles both walk x-forwarded-for right to left, skip these hops, and use the first untrusted address as the client IP (the leftmost entry is caller-supplied and would otherwise let anyone mint a fresh rate-limit bucket per request). Unset, the two differ: Better Auth trusts only single-value headers, while Sim's throttles key on the rightmost, proxy-written entry — never spoofable, but a multi-hop chain collapses callers onto the edge addresses. Use your proxies' actual addresses, NOT broad private ranges that also cover clients: a caller whose own address falls inside a trusted range makes the whole chain trusted.
+# TRUST_PROXY_HEADERS=false  # Optional: set false when the app is exposed directly with NO reverse proxy in front. With nothing appending the peer address, x-forwarded-for/x-real-ip are written entirely by the caller, so believing them lets anyone rotate a header for a fresh per-IP rate-limit bucket per request. While false, getClientIp reports 'unknown' and per-IP limits become one shared bucket (blunt, but fails closed). Defaults to true.
 
 # Chat (Optional)
 # COPILOT_API_KEY=                      # Mint one at https://sim.ai. Without it the Sim Chat block, prompt jobs, and Inbox cannot run

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

@@ -508,6 +508,7 @@ export const env = createEnv({
 
     // Network / proxy trust
     AUTH_TRUSTED_PROXIES:                  z.string().optional(),                  // Comma-separated reverse-proxy IPs or CIDR ranges. When set, Better Auth and getClientIp (per-IP rate-limit keys, audit rows) both walk the forwarded-IP chain right to left, skip these trusted hops, and use the first untrusted address as the client IP. Leave unset and the two differ: Better Auth trusts only single-value IP headers (recording no IP for a multi-hop chain), while getClientIp keys on the rightmost, proxy-written entry — never the caller-supplied leftmost one.
+    TRUST_PROXY_HEADERS:                   z.boolean().optional(),                 // Whether x-forwarded-for / x-real-ip may be believed at all. Default true: the app is assumed to sit behind a proxy that appends the peer address. Set false when it is exposed directly (no proxy), where those headers are written entirely by the caller — getClientIp then reports 'unknown' so per-IP limits become one shared bucket instead of a per-request bypass.
 
     // SSO Configuration (for script-based registration)
     SSO_ENABLED:                           z.boolean().optional(),                 // Enable SSO functionality

apps/sim/lib/core/utils/client-ip.test.ts

@@ -10,21 +10,32 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const { mockEnv } = vi.hoisted(() => ({
-  mockEnv: { AUTH_TRUSTED_PROXIES: undefined as string | undefined },
+  mockEnv: {
+    AUTH_TRUSTED_PROXIES: undefined as string | undefined,
+    TRUST_PROXY_HEADERS: undefined as string | boolean | undefined,
+  },
 }))
 
-vi.mock('@/lib/core/config/env', () => ({ env: mockEnv }))
+vi.mock('@/lib/core/config/env', () => ({
+  env: mockEnv,
+  isFalsy: (value: string | boolean | number | undefined) =>
+    value === false || value === 'false' || value === 0 || value === '0',
+}))
 vi.unmock('@/lib/core/utils/client-ip')
 
 /**
  * The module parses the env once at import — that is the behavior under test —
  * so each case needs a fresh module instance. This is the deliberate exception
  * to the repo's "no `vi.resetModules()` + dynamic import" performance rule
  * (`.cursor/rules/sim-testing.mdc`): module-init behavior cannot be observed any
- * other way, and the cost here is four imports of a six-line module.
+ * other way, and the cost here is a handful of imports of a tiny module.
  */
-async function loadGetClientIp(trustedProxies: string | undefined) {
+async function loadGetClientIp(
+  trustedProxies: string | undefined,
+  trustProxyHeaders?: string | boolean
+) {
   mockEnv.AUTH_TRUSTED_PROXIES = trustedProxies
+  mockEnv.TRUST_PROXY_HEADERS = trustProxyHeaders
   vi.resetModules()
   return (await import('@/lib/core/utils/client-ip')).getClientIp
 }
@@ -64,4 +75,23 @@ describe('getClientIp', () => {
 
     expect(getClientIp(req({}))).toBe('unknown')
   })
+
+  it('declines to read forwarded headers when TRUST_PROXY_HEADERS is false', async () => {
+    // No proxy in front: the whole header is caller-authored, so every caller
+    // shares one bucket rather than each minting their own.
+    const getClientIp = await loadGetClientIp(undefined, 'false')
+    const keys = ['203.0.113.7, 10.0.0.1', '9.9.9.9', '2001:db8::1'].map((value) =>
+      getClientIp(req({ 'x-forwarded-for': value }))
+    )
+
+    expect(new Set(keys)).toEqual(new Set(['unknown']))
+    expect(getClientIp(req({ 'x-real-ip': '203.0.113.7' }))).toBe('unknown')
+  })
+
+  it('still reads forwarded headers when TRUST_PROXY_HEADERS is unset or true', async () => {
+    for (const value of [undefined, 'true'] as const) {
+      const getClientIp = await loadGetClientIp(undefined, value)
+      expect(getClientIp(req({ 'x-forwarded-for': '203.0.113.7, 10.0.0.1' }))).toBe('10.0.0.1')
+    }
+  })
 })

apps/sim/lib/core/utils/client-ip.ts

@@ -2,8 +2,9 @@ import {
   type ClientIpHeaderSource,
   parseTrustedProxies,
   resolveClientIp,
+  UNKNOWN_CLIENT_IP,
 } from '@sim/security/client-ip'
-import { env } from '@/lib/core/config/env'
+import { env, isFalsy } from '@/lib/core/config/env'
 
 /**
  * Reverse-proxy hops trusted for forwarded-IP resolution, read from the same
@@ -17,6 +18,17 @@ import { env } from '@/lib/core/config/env'
  */
 const trustedProxies = parseTrustedProxies(env.AUTH_TRUSTED_PROXIES)
 
+/**
+ * Whether forwarded headers may be believed at all.
+ *
+ * Every rule about which hop to read presumes a proxy wrote at least one of
+ * them. Reachable directly — no proxy, port published straight to the internet —
+ * the entire header is caller-authored and no parsing strategy can recover a
+ * real address from it. Operators of such a deployment set
+ * `TRUST_PROXY_HEADERS=false`, which makes {@link getClientIp} decline to guess.
+ */
+const trustForwardedHeaders = !isFalsy(env.TRUST_PROXY_HEADERS)
+
 /**
  * Extract the client IP from a request for logging, audit trails, and — most
  * importantly — per-IP rate-limit keys.
@@ -27,7 +39,13 @@ const trustedProxies = parseTrustedProxies(env.AUTH_TRUSTED_PROXIES)
  * See {@link resolveClientIp} for why the chain is walked right to left. In
  * short: the leftmost `X-Forwarded-For` entry is supplied by the caller, so
  * keying a throttle on it lets anyone mint a fresh bucket per request.
+ *
+ * With `TRUST_PROXY_HEADERS=false` this returns {@link UNKNOWN_CLIENT_IP} for
+ * every caller, collapsing per-IP limits to a single shared bucket. That is
+ * deliberately blunt — it throttles unrelated callers together — but it fails
+ * closed, which a header nobody vouched for does not.
  */
 export function getClientIp(request: ClientIpHeaderSource): string {
+  if (!trustForwardedHeaders) return UNKNOWN_CLIENT_IP
   return resolveClientIp(request, trustedProxies)
 }

docker-compose.prod.yml

@@ -36,6 +36,16 @@ services:
       # chain trusted. This all assumes a proxy that appends the peer address —
       # an app exposed directly to the internet sees only what the caller wrote.
       - AUTH_TRUSTED_PROXIES=${AUTH_TRUSTED_PROXIES:-}
+      # TRUST_PROXY_HEADERS: whether x-forwarded-for / x-real-ip may be believed.
+      # Defaults to FALSE here because this file publishes port 3000 directly and
+      # ships no reverse proxy — with nothing in front, those headers are written
+      # entirely by the caller, and believing them would let anyone rotate a
+      # header to get a fresh per-IP rate-limit bucket on every request. While
+      # false, per-IP limits collapse into one shared bucket: blunt, but it fails
+      # closed. Set it to true once a proxy that APPENDS the peer address (nginx,
+      # Caddy, Traefik, an ALB, Cloudflare) terminates in front of the app, and
+      # set AUTH_TRUSTED_PROXIES to that proxy's address at the same time.
+      - TRUST_PROXY_HEADERS=${TRUST_PROXY_HEADERS:-false}
       - BETTER_AUTH_SECRET=${BETTER_AUTH_SECRET}
       - ENCRYPTION_KEY=${ENCRYPTION_KEY}
       - API_ENCRYPTION_KEY=${API_ENCRYPTION_KEY:-}

helm/sim/values.schema.json

@@ -159,6 +159,10 @@
               "type": "string",
               "description": "Comma-separated reverse-proxy IPs or CIDR ranges in front of the app (e.g. the ingress pods, '10.42.0.0/24'). When set, Better Auth and Sim's per-IP rate limits both walk x-forwarded-for right to left, skip these hops, and use the first untrusted address as the client IP. Leave empty and the two differ: Better Auth trusts only a single-value header, while Sim's throttles key on the rightmost, proxy-written entry. Do not use a range broad enough to also cover client traffic — a caller inside a trusted range makes the whole chain trusted."
             },
+            "TRUST_PROXY_HEADERS": {
+              "type": "string",
+              "description": "Whether x-forwarded-for / x-real-ip may be believed at all. Empty (the default) means yes, which is correct behind the chart's ingress. Set to 'false' only when the app is exposed with no proxy appending the peer address, where those headers are entirely caller-written; per-IP rate limits then collapse to one shared bucket rather than being bypassable per request."
+            },
             "SSO_TRUSTED_PROVIDER_IDS": {
               "type": "string",
               "description": "Comma-separated SSO provider IDs to trust for automatic account linking when an SSO sign-in matches an existing account's email. Only needed for IdPs that do not assert email_verified. Merged into Better Auth accountLinking.trustedProviders."

helm/sim/values.yaml

@@ -91,6 +91,13 @@ app:
     # to also cover client traffic: a caller whose own address falls inside a trusted range makes
     # the whole chain trusted and can then forge the value Sim keys on.
     AUTH_TRUSTED_PROXIES: ""
+    # TRUST_PROXY_HEADERS: whether x-forwarded-for / x-real-ip may be believed at all. Left empty
+    # (= true), which is correct here because the chart runs behind an ingress that appends the peer
+    # address. Set to "false" only if you expose the Service directly with no proxy in front: with
+    # nothing appending, those headers are written entirely by the caller and believing them lets
+    # anyone rotate a header for a fresh per-IP rate-limit bucket per request. While false, per-IP
+    # limits collapse into one shared bucket — blunt, but it fails closed.
+    TRUST_PROXY_HEADERS: ""
     # SOCKET_SERVER_URL: Auto-detected when realtime.enabled=true (uses internal service)
     # NEXT_PUBLIC_SOCKET_URL: public WebSocket URL for browsers. Leave empty to default to the
     # page's own origin (assumes the ingress/reverse proxy routes /socket.io to the realtime service).

packages/audit/src/log.ts

@@ -4,6 +4,7 @@ import {
   type ClientIpHeaderSource,
   parseTrustedProxies,
   resolveClientIp,
+  UNKNOWN_CLIENT_IP,
 } from '@sim/security/client-ip'
 import { generateShortId } from '@sim/utils/id'
 import { eq } from 'drizzle-orm'
@@ -39,12 +40,22 @@ interface AuditLogParams {
  */
 const trustedProxies = parseTrustedProxies(process.env.AUTH_TRUSTED_PROXIES)
 
+/**
+ * Mirrors the app's `TRUST_PROXY_HEADERS`. Recording a caller-authored address
+ * as forensic evidence is worse than recording none, so a deployment that
+ * declares it has no proxy in front gets `unknown` rather than a fabrication.
+ */
+const trustForwardedHeaders = !/^(false|0|no|off)$/i.test(
+  (process.env.TRUST_PROXY_HEADERS ?? '').trim()
+)
+
 /**
  * An audit row's `ipAddress` is forensic evidence, so it must not be whatever
  * the caller put in the leftmost `X-Forwarded-For` entry. See
  * {@link resolveClientIp}.
  */
 function getClientIp(request: ClientIpHeaderSource): string {
+  if (!trustForwardedHeaders) return UNKNOWN_CLIENT_IP
   return resolveClientIp(request, trustedProxies)
 }