Skip to content

Commit bb6950b

Browse files
committed
fix(security): mask IPv6 rate-limit keys to the routed /64
A single IPv6 client is delegated a whole /64 — the standard residential and cloud allocation — so it can legitimately source every request from a different address. Keying a per-IP throttle on the full /128 therefore left the exact bypass this fix exists to close wide open over IPv6, with no header spoofing at all: the proxy itself writes the varying value and nothing looks wrong. Mask IPv6 to its routed prefix when producing a key, so one subscriber is one bucket. Matches Better Auth's `ipv6Subnet` default, so session and throttle keys agree. IPv4 is untouched — a v4 address is already a single host. Masking happens only where a key is produced, never before the trusted-proxy comparison, which must see the full address. `getAssertedOriginIp` stays unmasked: the webhook allowlist needs the exact sender. Also corrects what the docs claim about Better Auth. With no trusted proxies configured it does not walk the chain — `getIPFromHeader` returns null for any multi-value header — so the previous wording (and the env.ts line this replaces, which had been accurate) overstated the agreement between the two. Each surface now states where they align and where they deliberately differ, warns against a trusted range broad enough to cover clients, and notes that none of it helps an app exposed without a proxy. - values.schema.json carried the same stale claim as values.yaml - profound.ts compares against UNKNOWN_CLIENT_IP instead of a bare literal - cover the env -> parseTrustedProxies wiring, which was globally mocked and so never executed in CI, and de-vacuum the IPv6/IPv4 kind-mismatch test
1 parent 8cf99e9 commit bb6950b

11 files changed

Lines changed: 221 additions & 41 deletions

File tree

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,12 @@ const RATE_LIMIT_WINDOW_MS = 60_000
7171
const rateLimitHits = new Map<string, { count: number; resetAt: number }>()
7272

7373
/**
74-
* Reverse-proxy hops trusted for forwarded-IP resolution — the same
75-
* `AUTH_TRUSTED_PROXIES` the main app reads. Parsed once at module load.
74+
* Reverse-proxy hops trusted for forwarded-IP resolution, named after the main
75+
* app's setting so the two behave alike where both are deployed. The docs site
76+
* ships separately and does not normally set it, so this is usually empty —
77+
* which is safe (the rightmost, proxy-written hop wins) but coarse: if the docs
78+
* edge presents more than one hop, visitors share one bucket. Set it here too if
79+
* that shows up as spurious 429s.
7680
*/
7781
const trustedProxies = parseTrustedProxies(process.env.AUTH_TRUSTED_PROXIES)
7882

apps/sim/.env.example

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ BETTER_AUTH_URL=http://localhost:3000
1919
NEXT_PUBLIC_APP_URL=http://localhost:3000
2020
# 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
2121
# 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.
22-
# AUTH_TRUSTED_PROXIES=10.0.0.0/24,192.0.2.10 # Optional: reverse-proxy IPs/CIDRs in front of the app. Better Auth and Sim's own per-IP throttles 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 trusts no hop and keys on the rightmost entry — safe, but a multi-hop chain collapses callers onto the edge addresses. Use your proxies' actual addresses, not broad private ranges that also cover clients.
22+
# 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.
2323

2424
# Chat (Optional)
2525
# COPILOT_API_KEY= # Mint one at https://sim.ai. Without it the Sim Chat block, prompt jobs, and Inbox cannot run

apps/sim/lib/analytics/profound.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* @see https://docs.tryprofound.com/agent-analytics/custom
77
*/
88
import { createLogger } from '@sim/logger'
9+
import { UNKNOWN_CLIENT_IP } from '@sim/security/client-ip'
910
import { env } from '@/lib/core/config/env'
1011
import { isHosted } from '@/lib/core/config/env-flags'
1112
import { getClientIp } from '@/lib/core/utils/client-ip'
@@ -104,7 +105,7 @@ export function sendToProfound(request: Request, statusCode: number): void {
104105
status_code: statusCode,
105106
ip: (() => {
106107
const resolved = getClientIp(request)
107-
return resolved === 'unknown' ? '0.0.0.0' : resolved
108+
return resolved === UNKNOWN_CLIENT_IP ? '0.0.0.0' : resolved
108109
})(),
109110
user_agent: request.headers.get('user-agent') || '',
110111
...(Object.keys(queryParams).length > 0 && { query_params: queryParams }),

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ export const env = createEnv({
507507
REACT_SCAN_ENABLED: z.boolean().optional(), // Enable React Scan for performance debugging (dev only)
508508

509509
// Network / proxy trust
510-
AUTH_TRUSTED_PROXIES: z.string().optional(), // Comma-separated reverse-proxy IPs or CIDR ranges. Better Auth and getClientIp (per-IP rate-limit keys, audit rows) walk the forwarded-IP chain right to left, skip these trusted hops, and use the first untrusted address as the client IP. Unset trusts no hop and keys on the rightmost, proxy-written entry — never the caller-supplied leftmost one.
510+
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.
511511

512512
// SSO Configuration (for script-based registration)
513513
SSO_ENABLED: z.boolean().optional(), // Enable SSO functionality
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* Covers the wiring between `AUTH_TRUSTED_PROXIES` and the shared resolver.
3+
* `@/lib/core/utils/client-ip` is mocked globally in `vitest.setup.ts` (it is
4+
* what every route consumes), so without the `vi.unmock` below the real module
5+
* — and therefore the env read that makes trusted proxies take effect — would
6+
* never execute anywhere in CI.
7+
*
8+
* @vitest-environment node
9+
*/
10+
import { beforeEach, describe, expect, it, vi } from 'vitest'
11+
12+
const { mockEnv } = vi.hoisted(() => ({
13+
mockEnv: { AUTH_TRUSTED_PROXIES: undefined as string | undefined },
14+
}))
15+
16+
vi.mock('@/lib/core/config/env', () => ({ env: mockEnv }))
17+
vi.unmock('@/lib/core/utils/client-ip')
18+
19+
/**
20+
* The module parses the env once at import — that is the behavior under test —
21+
* so each case needs a fresh module instance. This is the deliberate exception
22+
* to the repo's "no `vi.resetModules()` + dynamic import" performance rule
23+
* (`.cursor/rules/sim-testing.mdc`): module-init behavior cannot be observed any
24+
* other way, and the cost here is four imports of a six-line module.
25+
*/
26+
async function loadGetClientIp(trustedProxies: string | undefined) {
27+
mockEnv.AUTH_TRUSTED_PROXIES = trustedProxies
28+
vi.resetModules()
29+
return (await import('@/lib/core/utils/client-ip')).getClientIp
30+
}
31+
32+
function req(headers: Record<string, string>) {
33+
return { headers: new Headers(headers) }
34+
}
35+
36+
describe('getClientIp', () => {
37+
beforeEach(() => {
38+
vi.clearAllMocks()
39+
})
40+
41+
it('keys on the proxy-appended hop, not the caller-supplied leftmost one', async () => {
42+
const getClientIp = await loadGetClientIp(undefined)
43+
44+
expect(getClientIp(req({ 'x-forwarded-for': '203.0.113.7, 10.0.0.1' }))).toBe('10.0.0.1')
45+
})
46+
47+
it('honors AUTH_TRUSTED_PROXIES, resolving past the configured hop', async () => {
48+
const getClientIp = await loadGetClientIp('10.0.0.0/24')
49+
50+
expect(getClientIp(req({ 'x-forwarded-for': '203.0.113.7, 10.0.0.1' }))).toBe('203.0.113.7')
51+
})
52+
53+
it('gives one bucket per caller no matter what they prepend', async () => {
54+
const getClientIp = await loadGetClientIp(undefined)
55+
const keys = ['9.9.9.9', 'unknown', '203.0.113.250'].map((spoof) =>
56+
getClientIp(req({ 'x-forwarded-for': `${spoof}, 10.0.0.1` }))
57+
)
58+
59+
expect(new Set(keys)).toEqual(new Set(['10.0.0.1']))
60+
})
61+
62+
it('falls back to a shared bucket when no header yields an address', async () => {
63+
const getClientIp = await loadGetClientIp(undefined)
64+
65+
expect(getClientIp(req({}))).toBe('unknown')
66+
})
67+
})

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,14 @@ import {
66
import { env } from '@/lib/core/config/env'
77

88
/**
9-
* Reverse-proxy hops trusted for forwarded-IP resolution, shared with Better
10-
* Auth's `advanced.ipAddress.trustedProxies` so session IPs and Sim's own
11-
* rate-limit keys agree on who the caller is. Parsed once at module load.
9+
* Reverse-proxy hops trusted for forwarded-IP resolution, read from the same
10+
* `AUTH_TRUSTED_PROXIES` as Better Auth's `advanced.ipAddress.trustedProxies`
11+
* (see `lib/auth/auth.ts`). Parsed once at module load.
12+
*
13+
* Configured, the two agree on who the caller is. Left unset they diverge by
14+
* design: Better Auth trusts only a single-value header and records no IP for a
15+
* longer chain, whereas a throttle cannot opt out of having a key, so this falls
16+
* back to the rightmost — still proxy-written, never caller-authored.
1217
*/
1318
const trustedProxies = parseTrustedProxies(env.AUTH_TRUSTED_PROXIES)
1419

docker-compose.prod.yml

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,20 @@ services:
2121
# (apex + www, alias hostnames, reverse-proxy IPs). Empty by default.
2222
- TRUSTED_ORIGINS=${TRUSTED_ORIGINS:-}
2323
# AUTH_TRUSTED_PROXIES: comma-separated reverse-proxy IPs or CIDR ranges in
24-
# front of the app (ingress, load balancer). Better Auth AND Sim's own
25-
# per-IP throttles walk x-forwarded-for right to left, skip these hops, and
26-
# use the first untrusted address as the client IP — never the leftmost
27-
# entry, which the caller supplies and could otherwise rotate to mint a
28-
# fresh rate-limit bucket per request. Empty by default, which trusts no
29-
# hop and keys on the rightmost (proxy-written) entry: safe, but behind a
30-
# multi-hop chain (e.g. CDN in front of ingress) it collapses callers onto
31-
# the edge addresses. Set it to your real hops for per-client keying and
32-
# correct session IPs.
24+
# front of the app (ingress, load balancer). When set, Better Auth AND
25+
# Sim's own per-IP throttles walk x-forwarded-for right to left, skip these
26+
# hops, and use the first untrusted address as the client IP — never the
27+
# leftmost entry, which the caller supplies and could otherwise be rotated
28+
# to mint a fresh rate-limit bucket per request. Empty by default, and then
29+
# the two differ: Better Auth trusts only a single-value header (recording
30+
# no IP for a multi-hop chain), while Sim's throttles key on the rightmost
31+
# (proxy-written) entry — not spoofable, but behind a multi-hop chain
32+
# (e.g. CDN in front of ingress) it collapses callers onto the edge
33+
# addresses. Set your real hops for per-client keying and correct session
34+
# IPs. Use the proxies' actual addresses, NOT a broad private range that
35+
# also covers clients: a caller inside a trusted range makes the whole
36+
# chain trusted. This all assumes a proxy that appends the peer address —
37+
# an app exposed directly to the internet sees only what the caller wrote.
3338
- AUTH_TRUSTED_PROXIES=${AUTH_TRUSTED_PROXIES:-}
3439
- BETTER_AUTH_SECRET=${BETTER_AUTH_SECRET}
3540
- ENCRYPTION_KEY=${ENCRYPTION_KEY}

helm/sim/values.schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@
157157
},
158158
"AUTH_TRUSTED_PROXIES": {
159159
"type": "string",
160-
"description": "Comma-separated reverse-proxy IPs or CIDR ranges in front of the app (e.g. '10.0.0.0/16'). Better Auth walks x-forwarded-for right to left, skips these hops, and uses the first untrusted address as the client IP."
160+
"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."
161161
},
162162
"SSO_TRUSTED_PROVIDER_IDS": {
163163
"type": "string",

helm/sim/values.yaml

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,16 @@ app:
8080
# Merged into Better Auth `trustedOrigins` alongside NEXT_PUBLIC_APP_URL. Leave empty when serving from a single origin.
8181
TRUSTED_ORIGINS: ""
8282
# AUTH_TRUSTED_PROXIES: comma-separated reverse-proxy IPs or CIDR ranges in front of the app
83-
# (ingress controller, load balancer). Better Auth AND Sim's own per-IP throttles walk
84-
# x-forwarded-for right to left, skip these hops, and use the first untrusted address as the
85-
# client IP — never the leftmost entry, which the caller supplies and could otherwise be
86-
# rotated to mint a fresh rate-limit bucket per request. Empty trusts no hop and keys on the
87-
# rightmost (proxy-written) entry: safe, but behind a multi-hop chain (e.g. CDN in front of
88-
# ingress) it collapses callers onto the edge addresses. Set your real hops (e.g. "10.0.0.0/16")
89-
# for per-client keying and correct session IPs.
83+
# (ingress controller, load balancer). When set, Better Auth AND Sim's own per-IP throttles
84+
# walk x-forwarded-for right to left, skip these hops, and use the first untrusted address as
85+
# the client IP — never the leftmost entry, which the caller supplies and could otherwise be
86+
# rotated to mint a fresh rate-limit bucket per request. Empty, the two differ: Better Auth
87+
# trusts only a single-value header (recording no IP for a multi-hop chain), while Sim's
88+
# throttles key on the rightmost (proxy-written) entry — not spoofable, but behind a multi-hop
89+
# chain (e.g. CDN in front of ingress) it collapses callers onto the edge addresses.
90+
# Set the ingress pods' actual addresses (e.g. "10.42.0.0/24"). Do NOT use a range broad enough
91+
# to also cover client traffic: a caller whose own address falls inside a trusted range makes
92+
# the whole chain trusted and can then forge the value Sim keys on.
9093
AUTH_TRUSTED_PROXIES: ""
9194
# SOCKET_SERVER_URL: Auto-detected when realtime.enabled=true (uses internal service)
9295
# NEXT_PUBLIC_SOCKET_URL: public WebSocket URL for browsers. Leave empty to default to the

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

Lines changed: 65 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,51 @@ describe('resolveClientIp', () => {
2727
})
2828

2929
it('strips IPv6 zone ids so they cannot mint unbounded distinct keys', () => {
30+
// `ipaddr.isValid` accepts an arbitrary-length zone and `process()` keeps
31+
// it verbatim, so an unstripped zone would be attacker-chosen text in the
32+
// key. The /64 mask happens to drop zones from v6 keys too — the
33+
// getAssertedOriginIp cases below pin the stripping on its own, since
34+
// that path is deliberately unmasked.
3035
const zoned = ['fe80::1%eth0', 'fe80::1%evil', `fe80::1%${'x'.repeat(200)}`].map((value) =>
3136
resolveClientIp(req({ 'x-forwarded-for': value }))
3237
)
33-
expect(new Set(zoned)).toEqual(new Set(['fe80::1']))
34-
expect(resolveClientIp(req({ 'x-real-ip': 'fe80::1%evil' }))).toBe('fe80::1')
35-
expect(resolveClientIp(req({ 'x-forwarded-for': '[fe80::1%eth0]:8080' }))).toBe('fe80::1')
38+
expect(new Set(zoned)).toEqual(new Set(['fe80::']))
39+
expect(resolveClientIp(req({ 'x-real-ip': 'fe80::1%evil' }))).toBe('fe80::')
40+
expect(canonicalizeIp('fe80::1%evil')).toBe('fe80::1')
41+
})
42+
43+
it('masks IPv6 to its routed /64 so one subscriber is one bucket', () => {
44+
// A single IPv6 client is delegated a whole /64, so the proxy honestly
45+
// writes a different address per request. Keying on the full /128 would
46+
// leave per-IP throttles bypassable with no spoofing at all.
47+
const withinOnePrefix = [
48+
'2001:db8:1:2::1',
49+
'2001:db8:1:2::dead:beef',
50+
'2001:db8:1:2:ffff:ffff:ffff:ffff',
51+
].map((value) => resolveClientIp(req({ 'x-forwarded-for': value })))
52+
expect(new Set(withinOnePrefix)).toEqual(new Set(['2001:db8:1:2::']))
53+
})
54+
55+
it('keeps distinct IPv6 /64s in distinct buckets', () => {
56+
const a = resolveClientIp(req({ 'x-forwarded-for': '2001:db8:1:2::1' }))
57+
const b = resolveClientIp(req({ 'x-forwarded-for': '2001:db8:1:3::1' }))
58+
expect(a).not.toBe(b)
59+
})
60+
61+
it('does not mask IPv4, which is already a single host', () => {
62+
expect(resolveClientIp(req({ 'x-forwarded-for': '198.51.100.4' }))).toBe('198.51.100.4')
63+
})
64+
65+
it('masks the x-real-ip fallback too', () => {
66+
expect(resolveClientIp(req({ 'x-real-ip': '2001:db8:1:2::99' }))).toBe('2001:db8:1:2::')
67+
})
68+
69+
it('matches a trusted range against the full address, not the masked key', () => {
70+
// Masking before the trust check would compare a different address.
71+
const trusted = parseTrustedProxies('2001:db8:1:2::abcd/128')
72+
expect(
73+
resolveClientIp(req({ 'x-forwarded-for': '203.0.113.7, 2001:db8:1:2::abcd' }), trusted)
74+
).toBe('203.0.113.7')
3675
})
3776

3877
it('collapses equivalent spellings of one address onto a single value', () => {
@@ -86,10 +125,12 @@ describe('resolveClientIp', () => {
86125
})
87126

88127
it('does not treat an IPv6 hop as matching an IPv4 trusted range', () => {
128+
// Two hops, so a wrongly-trusted rightmost entry would visibly shift the
129+
// answer left rather than merely avoiding a kind-mismatch throw.
89130
const trusted = parseTrustedProxies('0.0.0.0/0')
90-
expect(resolveClientIp(req({ 'x-forwarded-for': '2001:db8::1' }), trusted)).toBe(
91-
'2001:db8::1'
92-
)
131+
expect(
132+
resolveClientIp(req({ 'x-forwarded-for': '2001:db8:1::1, 2001:db8:2::2' }), trusted)
133+
).toBe('2001:db8:2::')
93134
})
94135

95136
it('matches an IPv4-mapped IPv6 trusted range against the unwrapped hop', () => {
@@ -113,11 +154,13 @@ describe('resolveClientIp', () => {
113154
})
114155

115156
it('strips brackets and ports from IPv6 hops', () => {
116-
expect(resolveClientIp(req({ 'x-forwarded-for': '[2001:db8::1]:8080' }))).toBe('2001:db8::1')
157+
// Masked to /64 like every IPv6 key; unmasked forms are covered by
158+
// getAssertedOriginIp below.
159+
expect(resolveClientIp(req({ 'x-forwarded-for': '[2001:db8::1]:8080' }))).toBe('2001:db8::')
117160
})
118161

119-
it('preserves a bare IPv6 literal', () => {
120-
expect(resolveClientIp(req({ 'x-forwarded-for': '2001:db8::1' }))).toBe('2001:db8::1')
162+
it('accepts a bare IPv6 literal', () => {
163+
expect(resolveClientIp(req({ 'x-forwarded-for': '2001:db8::1' }))).toBe('2001:db8::')
121164
})
122165

123166
it('skips unparseable hops while walking right to left', () => {
@@ -186,6 +229,19 @@ describe('getAssertedOriginIp', () => {
186229
).toBe('203.0.113.7')
187230
})
188231

232+
it('does not mask IPv6 — an allowlist needs the exact address', () => {
233+
expect(getAssertedOriginIp(req({ 'x-forwarded-for': '2001:db8:1:2::99' }))).toBe(
234+
'2001:db8:1:2::99'
235+
)
236+
})
237+
238+
it('strips brackets, ports, and zone ids like the resolver does', () => {
239+
expect(getAssertedOriginIp(req({ 'x-forwarded-for': '[2001:db8::1%eth0]:8080' }))).toBe(
240+
'2001:db8::1'
241+
)
242+
expect(getAssertedOriginIp(req({ 'x-forwarded-for': '203.0.113.7:4444' }))).toBe('203.0.113.7')
243+
})
244+
189245
it('returns null when no header yields an address', () => {
190246
expect(getAssertedOriginIp(req({}))).toBeNull()
191247
expect(getAssertedOriginIp(req({ 'x-forwarded-for': 'unknown' }))).toBeNull()

0 commit comments

Comments
 (0)