Skip to content

Commit 9f0cd26

Browse files
committed
fix(security): apply the proxy-trust gate in the docs app too
The docs Ask-AI limiter honored the trusted-proxy list but not TRUST_PROXY_HEADERS, so on a direct exposure it still keyed on a caller-authored header — leaving paid inference unmetered on the one endpoint where that costs real money. Same gate as the app and audit package now. Consolidate the predicate into parseTrustForwardedHeaders rather than keep a third copy of the spelling check. Three hand-rolled copies of a security predicate drifting apart is the exact failure this PR started as.
1 parent e42c49c commit 9f0cd26

6 files changed

Lines changed: 70 additions & 11 deletions

File tree

apps/docs/app/api/chat/route.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { openai } from '@ai-sdk/openai'
2-
import { parseTrustedProxies, resolveClientIp } from '@sim/security/client-ip'
2+
import {
3+
parseTrustedProxies,
4+
parseTrustForwardedHeaders,
5+
resolveClientIp,
6+
UNKNOWN_CLIENT_IP,
7+
} from '@sim/security/client-ip'
38
import {
49
convertToModelMessages,
510
jsonSchema,
@@ -80,6 +85,15 @@ const rateLimitHits = new Map<string, { count: number; resetAt: number }>()
8085
*/
8186
const trustedProxies = parseTrustedProxies(process.env.AUTH_TRUSTED_PROXIES)
8287

88+
/**
89+
* Mirrors the main app's `TRUST_PROXY_HEADERS`. Every rule about which hop to
90+
* read presumes a proxy wrote one of them; with nothing in front, the header is
91+
* caller-authored and this limiter guards paid inference, so decline to guess
92+
* and let all callers share one bucket. Defaults to true — the docs site is
93+
* served behind an edge that sets the header.
94+
*/
95+
const trustForwardedHeaders = parseTrustForwardedHeaders(process.env.TRUST_PROXY_HEADERS)
96+
8397
/**
8498
* Resolve the client IP from forwarding headers, falling back to a shared
8599
* bucket. Walks the chain right to left: the leftmost `X-Forwarded-For` entry is
@@ -88,6 +102,7 @@ const trustedProxies = parseTrustedProxies(process.env.AUTH_TRUSTED_PROXIES)
88102
* spend plus unbounded growth of `rateLimitHits`. See {@link resolveClientIp}.
89103
*/
90104
function getClientIp(req: Request): string {
105+
if (!trustForwardedHeaders) return UNKNOWN_CLIENT_IP
91106
return resolveClientIp(req, trustedProxies)
92107
}
93108

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,7 @@ const { mockEnv } = vi.hoisted(() => ({
1616
},
1717
}))
1818

19-
vi.mock('@/lib/core/config/env', () => ({
20-
env: mockEnv,
21-
isFalsy: (value: string | boolean | number | undefined) =>
22-
value === false || value === 'false' || value === 0 || value === '0',
23-
}))
19+
vi.mock('@/lib/core/config/env', () => ({ env: mockEnv }))
2420
vi.unmock('@/lib/core/utils/client-ip')
2521

2622
/**

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import {
22
type ClientIpHeaderSource,
33
parseTrustedProxies,
4+
parseTrustForwardedHeaders,
45
resolveClientIp,
56
UNKNOWN_CLIENT_IP,
67
} from '@sim/security/client-ip'
7-
import { env, isFalsy } from '@/lib/core/config/env'
8+
import { env } from '@/lib/core/config/env'
89

910
/**
1011
* Reverse-proxy hops trusted for forwarded-IP resolution, read from the same
@@ -27,7 +28,7 @@ const trustedProxies = parseTrustedProxies(env.AUTH_TRUSTED_PROXIES)
2728
* real address from it. Operators of such a deployment set
2829
* `TRUST_PROXY_HEADERS=false`, which makes {@link getClientIp} decline to guess.
2930
*/
30-
const trustForwardedHeaders = !isFalsy(env.TRUST_PROXY_HEADERS)
31+
const trustForwardedHeaders = parseTrustForwardedHeaders(env.TRUST_PROXY_HEADERS)
3132

3233
/**
3334
* Extract the client IP from a request for logging, audit trails, and — most

packages/audit/src/log.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { createLogger } from '@sim/logger'
33
import {
44
type ClientIpHeaderSource,
55
parseTrustedProxies,
6+
parseTrustForwardedHeaders,
67
resolveClientIp,
78
UNKNOWN_CLIENT_IP,
89
} from '@sim/security/client-ip'
@@ -45,9 +46,7 @@ const trustedProxies = parseTrustedProxies(process.env.AUTH_TRUSTED_PROXIES)
4546
* as forensic evidence is worse than recording none, so a deployment that
4647
* declares it has no proxy in front gets `unknown` rather than a fabrication.
4748
*/
48-
const trustForwardedHeaders = !/^(false|0|no|off)$/i.test(
49-
(process.env.TRUST_PROXY_HEADERS ?? '').trim()
50-
)
49+
const trustForwardedHeaders = parseTrustForwardedHeaders(process.env.TRUST_PROXY_HEADERS)
5150

5251
/**
5352
* An audit row's `ipAddress` is forensic evidence, so it must not be whatever

packages/security/src/client-ip.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
canonicalizeIp,
44
getAssertedOriginIp,
55
parseTrustedProxies,
6+
parseTrustForwardedHeaders,
67
resolveClientIp,
78
UNKNOWN_CLIENT_IP,
89
} from './client-ip'
@@ -269,3 +270,31 @@ describe('parseTrustedProxies', () => {
269270
expect(trusted.cidrs).toHaveLength(2)
270271
})
271272
})
273+
274+
describe('parseTrustForwardedHeaders', () => {
275+
it('defaults to trusting the headers when unset', () => {
276+
// Unset must never silently disable IP resolution — that would turn every
277+
// per-IP limit into one global bucket on an ordinary proxied deployment.
278+
expect(parseTrustForwardedHeaders(undefined)).toBe(true)
279+
expect(parseTrustForwardedHeaders(null)).toBe(true)
280+
expect(parseTrustForwardedHeaders('')).toBe(true)
281+
expect(parseTrustForwardedHeaders(' ')).toBe(true)
282+
})
283+
284+
it('accepts the usual falsey spellings, case- and space-insensitively', () => {
285+
for (const value of ['false', 'FALSE', ' False ', '0', 'no', 'off', 'OFF']) {
286+
expect(parseTrustForwardedHeaders(value)).toBe(false)
287+
}
288+
})
289+
290+
it('accepts a real boolean, since one caller reads a parsed env', () => {
291+
expect(parseTrustForwardedHeaders(false)).toBe(false)
292+
expect(parseTrustForwardedHeaders(true)).toBe(true)
293+
})
294+
295+
it('treats anything else as trusting', () => {
296+
for (const value of ['true', 'yes', 'on', '1', 'anything']) {
297+
expect(parseTrustForwardedHeaders(value)).toBe(true)
298+
}
299+
})
300+
})

packages/security/src/client-ip.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,25 @@ export function parseTrustedProxies(raw: string | null | undefined): TrustedProx
148148
return { cidrs }
149149
}
150150

151+
/**
152+
* Reads the `TRUST_PROXY_HEADERS` setting: may `X-Forwarded-For` / `X-Real-IP`
153+
* be believed at all?
154+
*
155+
* Every rule about *which* hop to read presumes a proxy wrote one of them. An
156+
* app reachable directly sees a header authored entirely by the caller, and no
157+
* parsing strategy recovers a real address from that — so this is a deployment
158+
* fact the operator has to state, not something the code can detect.
159+
*
160+
* Defaults to `true` (a proxy is assumed) so an unset value never silently
161+
* disables IP resolution. Accepts a boolean or the usual string spellings,
162+
* because the value arrives parsed from the app's env module in one caller and
163+
* raw from `process.env` in others.
164+
*/
165+
export function parseTrustForwardedHeaders(raw: string | boolean | null | undefined): boolean {
166+
if (typeof raw === 'boolean') return raw
167+
return !/^(false|0|no|off)$/i.test((raw ?? '').trim())
168+
}
169+
151170
/**
152171
* Resolves the client IP behind a reverse proxy, safely enough to key a rate
153172
* limit on.

0 commit comments

Comments
 (0)