Skip to content

Commit 802b53d

Browse files
committed
fix(security): key per-IP rate limits on the proxy-written forwarded hop
getClientIp derived the client IP from the leftmost X-Forwarded-For entry. Under any proxy that appends to that header — nginx-ingress, HAProxy, Cloudflare, and both reference deployments in this repo — the leftmost entry is supplied by the caller, so rotating it minted a fresh token bucket per request and every per-IP throttle became a no-op: the contact and demo-request mailers, telemetry, the docs Ask-AI endpoint, and public-deployment password attempts. The repo already treated that hop as untrusted for Better Auth via AUTH_TRUSTED_PROXIES; Sim's own helper never consulted it. packages/audit carried a second copy of the same function, forging audit-row IPs. Resolve the chain right to left instead, skipping configured trusted hops and returning the first untrusted address — the closest hop the infrastructure actually vouched for. Shared from @sim/security/client-ip so the app, the docs app, and the audit package cannot drift again. - fall back to the rightmost hop, never the leftmost, when every hop is trusted, so forging an address inside a broad configured range (the docs recommend 10.0.0.0/16) cannot reinstate the bypass - strip IPv6 zone ids, which ipaddr accepts at arbitrary length and would otherwise hand a caller unlimited distinct bucket keys - canonicalize addresses so equivalent spellings share one bucket - bound consecutive failed password guesses per deployment, not just per IP, since a distributed caller gets a fresh IP bucket per source The generic webhook allowlist keeps leftmost semantics via getAssertedOriginIp: it names the sending service, not the proxy, so resolving it like a throttle key would have 403'd every allowlisted delivery. Both sides are now canonicalized. Operators behind a multi-hop chain should set AUTH_TRUSTED_PROXIES to their real hops; unset is safe but collapses callers onto the edge address.
1 parent 1821415 commit 802b53d

36 files changed

Lines changed: 729 additions & 76 deletions

File tree

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { openai } from '@ai-sdk/openai'
2+
import { parseTrustedProxies, resolveClientIp } from '@sim/security/client-ip'
23
import {
34
convertToModelMessages,
45
jsonSchema,
@@ -69,11 +70,21 @@ const RATE_LIMIT_MAX = 20
6970
const RATE_LIMIT_WINDOW_MS = 60_000
7071
const rateLimitHits = new Map<string, { count: number; resetAt: number }>()
7172

72-
/** Resolve the client IP from forwarding headers, falling back to a shared bucket. */
73+
/**
74+
* Reverse-proxy hops trusted for forwarded-IP resolution — the same
75+
* `AUTH_TRUSTED_PROXIES` the main app reads. Parsed once at module load.
76+
*/
77+
const trustedProxies = parseTrustedProxies(process.env.AUTH_TRUSTED_PROXIES)
78+
79+
/**
80+
* Resolve the client IP from forwarding headers, falling back to a shared
81+
* bucket. Walks the chain right to left: the leftmost `X-Forwarded-For` entry is
82+
* caller-supplied, so keying this limit on it would let anyone rotate the header
83+
* to mint a fresh bucket per request — and, on this endpoint, unmetered model
84+
* spend plus unbounded growth of `rateLimitHits`. See {@link resolveClientIp}.
85+
*/
7386
function getClientIp(req: Request): string {
74-
const forwarded = req.headers.get('x-forwarded-for')
75-
if (forwarded) return forwarded.split(',')[0].trim()
76-
return req.headers.get('x-real-ip') ?? 'unknown'
87+
return resolveClientIp(req, trustedProxies)
7788
}
7889

7990
/** Fixed-window check. Returns retry-after seconds when the caller is over the limit, else null. */

apps/docs/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
"@ai-sdk/react": "2.0.205",
2222
"@sim/db": "workspace:*",
2323
"@sim/emcn": "workspace:*",
24+
"@sim/security": "workspace:*",
2425
"@sim/workflow-renderer": "workspace:*",
2526
"ai": "5.0.203",
2627
"class-variance-authority": "^0.7.1",

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 walks x-forwarded-for right to left, skips these hops, and uses the first untrusted address as the client IP (prevents forwarded-header spoofing). 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. 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.
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/app/api/chat/[identifier]/otp/route.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ import {
1919
OTP_IP_RATE_LIMIT,
2020
storeOTP,
2121
} from '@/lib/core/security/otp'
22-
import { generateRequestId, getClientIp } from '@/lib/core/utils/request'
22+
import { getClientIp } from '@/lib/core/utils/client-ip'
23+
import { generateRequestId } from '@/lib/core/utils/request'
2324
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
2425
import { sendEmail } from '@/lib/messaging/email/mailer'
2526
import { setChatAuthCookie } from '@/app/api/chat/utils'

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import { parseRequest } from '@/lib/api/server'
88
import type { TokenBucketConfig } from '@/lib/core/rate-limiter'
99
import { RateLimiter } from '@/lib/core/rate-limiter'
1010
import { isEmailAllowed } from '@/lib/core/security/deployment'
11-
import { generateRequestId, getClientIp } from '@/lib/core/utils/request'
11+
import { getClientIp } from '@/lib/core/utils/client-ip'
12+
import { generateRequestId } from '@/lib/core/utils/request'
1213
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1314
import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils'
1415

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,21 @@ const {
2020
mockSetDeploymentAuthCookie,
2121
mockIsEmailAllowed,
2222
mockCheckRateLimitDirect,
23+
mockResetRateLimitBucket,
2324
} = vi.hoisted(() => ({
2425
mockMergeSubblockStateWithValues: vi.fn().mockReturnValue({}),
2526
mockMergeSubBlockValues: vi.fn().mockReturnValue({}),
2627
mockValidateAuthToken: vi.fn().mockReturnValue(false),
2728
mockSetDeploymentAuthCookie: vi.fn(),
2829
mockIsEmailAllowed: vi.fn(),
2930
mockCheckRateLimitDirect: vi.fn().mockResolvedValue({ allowed: true }),
31+
mockResetRateLimitBucket: vi.fn().mockResolvedValue(undefined),
3032
}))
3133

3234
vi.mock('@/lib/core/rate-limiter', () => ({
3335
RateLimiter: class {
3436
checkRateLimitDirect = mockCheckRateLimitDirect
37+
resetRateLimitBucket = mockResetRateLimitBucket
3538
},
3639
}))
3740

@@ -212,6 +215,74 @@ describe('Chat API Utils', () => {
212215
expect(result.authorized).toBe(true)
213216
})
214217

218+
it('clears the per-resource failure counter once a password verifies', async () => {
219+
const deployment = {
220+
id: 'chat-id',
221+
authType: 'password',
222+
password: 'encrypted-password',
223+
}
224+
225+
const mockRequest = {
226+
method: 'POST',
227+
cookies: { get: vi.fn().mockReturnValue(null) },
228+
} as any
229+
230+
await validateChatAuth('request-id', deployment, mockRequest, {
231+
password: 'correct-password',
232+
})
233+
234+
expect(mockResetRateLimitBucket).toHaveBeenCalledWith('chat-password:resource:chat-id')
235+
})
236+
237+
it('leaves the per-resource failure counter consumed when the password is wrong', async () => {
238+
const deployment = {
239+
id: 'chat-id',
240+
authType: 'password',
241+
password: 'encrypted-password',
242+
}
243+
244+
const mockRequest = {
245+
method: 'POST',
246+
cookies: { get: vi.fn().mockReturnValue(null) },
247+
} as any
248+
249+
const result = await validateChatAuth('request-id', deployment, mockRequest, {
250+
password: 'wrong-password',
251+
})
252+
253+
expect(result.authorized).toBe(false)
254+
expect(mockCheckRateLimitDirect).toHaveBeenCalledWith(
255+
'chat-password:resource:chat-id',
256+
expect.objectContaining({ maxTokens: 500 })
257+
)
258+
expect(mockResetRateLimitBucket).not.toHaveBeenCalled()
259+
})
260+
261+
it('rejects guesses once the per-resource counter is exhausted, without decrypting', async () => {
262+
const deployment = {
263+
id: 'chat-id',
264+
authType: 'password',
265+
password: 'encrypted-password',
266+
}
267+
268+
const mockRequest = {
269+
method: 'POST',
270+
cookies: { get: vi.fn().mockReturnValue(null) },
271+
} as any
272+
273+
mockCheckRateLimitDirect.mockImplementation(async (key: string) =>
274+
key.includes(':resource:') ? { allowed: false, retryAfterMs: 900_000 } : { allowed: true }
275+
)
276+
277+
const result = await validateChatAuth('request-id', deployment, mockRequest, {
278+
password: 'guess',
279+
})
280+
281+
expect(result.authorized).toBe(false)
282+
expect(result.status).toBe(429)
283+
expect(decryptSecret).not.toHaveBeenCalled()
284+
})
285+
215286
it('should reject incorrect password', async () => {
216287
const deployment = {
217288
id: 'chat-id',

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ import { env } from '@/lib/core/config/env'
1111
import type { TokenBucketConfig } from '@/lib/core/rate-limiter'
1212
import { RateLimiter } from '@/lib/core/rate-limiter'
1313
import { isTurnstileConfigured, verifyTurnstileToken } from '@/lib/core/security/turnstile'
14-
import { generateRequestId, getClientIp } from '@/lib/core/utils/request'
14+
import { getClientIp } from '@/lib/core/utils/client-ip'
15+
import { generateRequestId } from '@/lib/core/utils/request'
1516
import { getEmailDomain } from '@/lib/core/utils/urls'
1617
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1718
import { sendEmail } from '@/lib/messaging/email/mailer'

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import { parseRequest } from '@/lib/api/server'
88
import { env } from '@/lib/core/config/env'
99
import type { TokenBucketConfig } from '@/lib/core/rate-limiter'
1010
import { RateLimiter } from '@/lib/core/rate-limiter'
11-
import { generateRequestId, getClientIp } from '@/lib/core/utils/request'
11+
import { getClientIp } from '@/lib/core/utils/client-ip'
12+
import { generateRequestId } from '@/lib/core/utils/request'
1213
import { getEmailDomain } from '@/lib/core/utils/urls'
1314
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1415
import { sendEmail } from '@/lib/messaging/email/mailer'

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ import {
2121
OTP_IP_RATE_LIMIT,
2222
storeOTP,
2323
} from '@/lib/core/security/otp'
24-
import { generateRequestId, getClientIp } from '@/lib/core/utils/request'
24+
import { getClientIp } from '@/lib/core/utils/client-ip'
25+
import { generateRequestId } from '@/lib/core/utils/request'
2526
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
2627
import { sendEmail } from '@/lib/messaging/email/mailer'
2728
import { resolveActiveShareByToken } from '@/lib/public-shares/share-manager'

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ import { parseRequest } from '@/lib/api/server'
77
import type { TokenBucketConfig } from '@/lib/core/rate-limiter'
88
import { RateLimiter } from '@/lib/core/rate-limiter'
99
import { isEmailAllowed } from '@/lib/core/security/deployment'
10-
import { generateRequestId, getClientIp } from '@/lib/core/utils/request'
10+
import { getClientIp } from '@/lib/core/utils/client-ip'
11+
import { generateRequestId } from '@/lib/core/utils/request'
1112
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1213
import { resolveActiveShareByToken } from '@/lib/public-shares/share-manager'
1314

0 commit comments

Comments
 (0)