diff --git a/apps/docs/app/api/chat/route.ts b/apps/docs/app/api/chat/route.ts index 915fe9a39c4..141808c03ea 100644 --- a/apps/docs/app/api/chat/route.ts +++ b/apps/docs/app/api/chat/route.ts @@ -1,4 +1,10 @@ import { openai } from '@ai-sdk/openai' +import { + parseTrustedProxies, + parseTrustForwardedHeaders, + resolveClientIp, + UNKNOWN_CLIENT_IP, +} from '@sim/security/client-ip' import { convertToModelMessages, jsonSchema, @@ -69,11 +75,35 @@ const RATE_LIMIT_MAX = 20 const RATE_LIMIT_WINDOW_MS = 60_000 const rateLimitHits = new Map() -/** Resolve the client IP from forwarding headers, falling back to a shared bucket. */ +/** + * Reverse-proxy hops trusted for forwarded-IP resolution, named after the main + * app's setting so the two behave alike where both are deployed. The docs site + * ships separately and does not normally set it, so this is usually empty — + * which is safe (the rightmost, proxy-written hop wins) but coarse: if the docs + * edge presents more than one hop, visitors share one bucket. Set it here too if + * that shows up as spurious 429s. + */ +const trustedProxies = parseTrustedProxies(process.env.AUTH_TRUSTED_PROXIES) + +/** + * Mirrors the main app's `TRUST_PROXY_HEADERS`. Every rule about which hop to + * read presumes a proxy wrote one of them; with nothing in front, the header is + * caller-authored and this limiter guards paid inference, so decline to guess + * and let all callers share one bucket. Defaults to true — the docs site is + * served behind an edge that sets the header. + */ +const trustForwardedHeaders = parseTrustForwardedHeaders(process.env.TRUST_PROXY_HEADERS) + +/** + * Resolve the client IP from forwarding headers, falling back to a shared + * bucket. Walks the chain right to left: the leftmost `X-Forwarded-For` entry is + * caller-supplied, so keying this limit on it would let anyone rotate the header + * to mint a fresh bucket per request — and, on this endpoint, unmetered model + * spend plus unbounded growth of `rateLimitHits`. See {@link resolveClientIp}. + */ function getClientIp(req: Request): string { - const forwarded = req.headers.get('x-forwarded-for') - if (forwarded) return forwarded.split(',')[0].trim() - return req.headers.get('x-real-ip') ?? 'unknown' + if (!trustForwardedHeaders) return UNKNOWN_CLIENT_IP + return resolveClientIp(req, trustedProxies) } /** Fixed-window check. Returns retry-after seconds when the caller is over the limit, else null. */ diff --git a/apps/docs/package.json b/apps/docs/package.json index a1f451235d1..682c35b0890 100644 --- a/apps/docs/package.json +++ b/apps/docs/package.json @@ -21,6 +21,7 @@ "@ai-sdk/react": "2.0.205", "@sim/db": "workspace:*", "@sim/emcn": "workspace:*", + "@sim/security": "workspace:*", "@sim/workflow-renderer": "workspace:*", "ai": "5.0.203", "class-variance-authority": "^0.7.1", diff --git a/apps/sim/.env.example b/apps/sim/.env.example index db177410995..e503a83202a 100644 --- a/apps/sim/.env.example +++ b/apps/sim/.env.example @@ -19,7 +19,8 @@ BETTER_AUTH_URL=http://localhost:3000 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. 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. +# 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 diff --git a/apps/sim/app/api/chat/[identifier]/otp/route.ts b/apps/sim/app/api/chat/[identifier]/otp/route.ts index 9f7fffd741f..09670f4a7ad 100644 --- a/apps/sim/app/api/chat/[identifier]/otp/route.ts +++ b/apps/sim/app/api/chat/[identifier]/otp/route.ts @@ -19,7 +19,8 @@ import { OTP_IP_RATE_LIMIT, storeOTP, } from '@/lib/core/security/otp' -import { generateRequestId, getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' +import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { sendEmail } from '@/lib/messaging/email/mailer' import { setChatAuthCookie } from '@/app/api/chat/utils' diff --git a/apps/sim/app/api/chat/[identifier]/sso/route.ts b/apps/sim/app/api/chat/[identifier]/sso/route.ts index c6ab98cfe94..08bebebf245 100644 --- a/apps/sim/app/api/chat/[identifier]/sso/route.ts +++ b/apps/sim/app/api/chat/[identifier]/sso/route.ts @@ -8,7 +8,8 @@ import { parseRequest } from '@/lib/api/server' import type { TokenBucketConfig } from '@/lib/core/rate-limiter' import { RateLimiter } from '@/lib/core/rate-limiter' import { isEmailAllowed } from '@/lib/core/security/deployment' -import { generateRequestId, getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' +import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' diff --git a/apps/sim/app/api/chat/utils.test.ts b/apps/sim/app/api/chat/utils.test.ts index 6c41eeb21cc..d843a1372c2 100644 --- a/apps/sim/app/api/chat/utils.test.ts +++ b/apps/sim/app/api/chat/utils.test.ts @@ -20,6 +20,7 @@ const { mockSetDeploymentAuthCookie, mockIsEmailAllowed, mockCheckRateLimitDirect, + mockResetRateLimitBucket, } = vi.hoisted(() => ({ mockMergeSubblockStateWithValues: vi.fn().mockReturnValue({}), mockMergeSubBlockValues: vi.fn().mockReturnValue({}), @@ -27,11 +28,13 @@ const { mockSetDeploymentAuthCookie: vi.fn(), mockIsEmailAllowed: vi.fn(), mockCheckRateLimitDirect: vi.fn().mockResolvedValue({ allowed: true }), + mockResetRateLimitBucket: vi.fn().mockResolvedValue(undefined), })) vi.mock('@/lib/core/rate-limiter', () => ({ RateLimiter: class { checkRateLimitDirect = mockCheckRateLimitDirect + resetRateLimitBucket = mockResetRateLimitBucket }, })) @@ -212,6 +215,98 @@ describe('Chat API Utils', () => { expect(result.authorized).toBe(true) }) + it('clears the per-resource failure counter once a password verifies', async () => { + const deployment = { + id: 'chat-id', + authType: 'password', + password: 'encrypted-password', + } + + const mockRequest = { + method: 'POST', + cookies: { get: vi.fn().mockReturnValue(null) }, + } as any + + await validateChatAuth('request-id', deployment, mockRequest, { + password: 'correct-password', + }) + + expect(mockResetRateLimitBucket).toHaveBeenCalledWith('chat-password:resource:chat-id') + }) + + it('leaves the per-resource failure counter consumed when the password is wrong', async () => { + const deployment = { + id: 'chat-id', + authType: 'password', + password: 'encrypted-password', + } + + const mockRequest = { + method: 'POST', + cookies: { get: vi.fn().mockReturnValue(null) }, + } as any + + const result = await validateChatAuth('request-id', deployment, mockRequest, { + password: 'wrong-password', + }) + + expect(result.authorized).toBe(false) + expect(mockCheckRateLimitDirect).toHaveBeenCalledWith( + 'chat-password:resource:chat-id', + expect.objectContaining({ maxTokens: 500 }), + { failClosed: true } + ) + expect(mockResetRateLimitBucket).not.toHaveBeenCalled() + }) + + it('checks the per-resource ceiling fail-closed so an outage cannot lift it', async () => { + // It is the only bound on distributed guessing at the secret; failing open + // would silently remove it during exactly the outage an attacker waits for. + const deployment = { + id: 'chat-id', + authType: 'password', + password: 'encrypted-password', + } + const mockRequest = { + method: 'POST', + cookies: { get: vi.fn().mockReturnValue(null) }, + } as any + + await validateChatAuth('request-id', deployment, mockRequest, { + password: 'correct-password', + }) + + const resourceCall = mockCheckRateLimitDirect.mock.calls.find((call: unknown[]) => + String(call[0]).includes(':resource:') + ) + expect(resourceCall?.[2]).toEqual({ failClosed: true }) + }) + + it('rejects guesses once the per-resource counter is exhausted, without decrypting', async () => { + const deployment = { + id: 'chat-id', + authType: 'password', + password: 'encrypted-password', + } + + const mockRequest = { + method: 'POST', + cookies: { get: vi.fn().mockReturnValue(null) }, + } as any + + mockCheckRateLimitDirect.mockImplementation(async (key: string) => + key.includes(':resource:') ? { allowed: false, retryAfterMs: 900_000 } : { allowed: true } + ) + + const result = await validateChatAuth('request-id', deployment, mockRequest, { + password: 'guess', + }) + + expect(result.authorized).toBe(false) + expect(result.status).toBe(429) + expect(decryptSecret).not.toHaveBeenCalled() + }) + it('should reject incorrect password', async () => { const deployment = { id: 'chat-id', diff --git a/apps/sim/app/api/contact/route.ts b/apps/sim/app/api/contact/route.ts index 2b610ec2114..a8738e63f67 100644 --- a/apps/sim/app/api/contact/route.ts +++ b/apps/sim/app/api/contact/route.ts @@ -11,7 +11,8 @@ import { env } from '@/lib/core/config/env' import type { TokenBucketConfig } from '@/lib/core/rate-limiter' import { RateLimiter } from '@/lib/core/rate-limiter' import { isTurnstileConfigured, verifyTurnstileToken } from '@/lib/core/security/turnstile' -import { generateRequestId, getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' +import { generateRequestId } from '@/lib/core/utils/request' import { getEmailDomain } from '@/lib/core/utils/urls' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { sendEmail } from '@/lib/messaging/email/mailer' diff --git a/apps/sim/app/api/demo-requests/route.ts b/apps/sim/app/api/demo-requests/route.ts index 7553239e7b2..013353b44bf 100644 --- a/apps/sim/app/api/demo-requests/route.ts +++ b/apps/sim/app/api/demo-requests/route.ts @@ -8,7 +8,8 @@ import { parseRequest } from '@/lib/api/server' import { env } from '@/lib/core/config/env' import type { TokenBucketConfig } from '@/lib/core/rate-limiter' import { RateLimiter } from '@/lib/core/rate-limiter' -import { generateRequestId, getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' +import { generateRequestId } from '@/lib/core/utils/request' import { getEmailDomain } from '@/lib/core/utils/urls' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { sendEmail } from '@/lib/messaging/email/mailer' diff --git a/apps/sim/app/api/files/public/[token]/otp/route.ts b/apps/sim/app/api/files/public/[token]/otp/route.ts index 0dd240788fd..4ea958eee9e 100644 --- a/apps/sim/app/api/files/public/[token]/otp/route.ts +++ b/apps/sim/app/api/files/public/[token]/otp/route.ts @@ -21,7 +21,8 @@ import { OTP_IP_RATE_LIMIT, storeOTP, } from '@/lib/core/security/otp' -import { generateRequestId, getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' +import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { sendEmail } from '@/lib/messaging/email/mailer' import { resolveActiveShareByToken } from '@/lib/public-shares/share-manager' diff --git a/apps/sim/app/api/files/public/[token]/sso/route.ts b/apps/sim/app/api/files/public/[token]/sso/route.ts index b5185149440..ed1f93e84df 100644 --- a/apps/sim/app/api/files/public/[token]/sso/route.ts +++ b/apps/sim/app/api/files/public/[token]/sso/route.ts @@ -7,7 +7,8 @@ import { parseRequest } from '@/lib/api/server' import type { TokenBucketConfig } from '@/lib/core/rate-limiter' import { RateLimiter } from '@/lib/core/rate-limiter' import { isEmailAllowed } from '@/lib/core/security/deployment' -import { generateRequestId, getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' +import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { resolveActiveShareByToken } from '@/lib/public-shares/share-manager' diff --git a/apps/sim/app/api/help/integration-request/route.ts b/apps/sim/app/api/help/integration-request/route.ts index 6a8faf682b6..f78141aa497 100644 --- a/apps/sim/app/api/help/integration-request/route.ts +++ b/apps/sim/app/api/help/integration-request/route.ts @@ -5,7 +5,8 @@ import { parseRequest, validationErrorResponse } from '@/lib/api/server' import { env } from '@/lib/core/config/env' import type { TokenBucketConfig } from '@/lib/core/rate-limiter' import { RateLimiter } from '@/lib/core/rate-limiter' -import { generateRequestId, getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' +import { generateRequestId } from '@/lib/core/utils/request' import { getEmailDomain } from '@/lib/core/utils/urls' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { sendEmail } from '@/lib/messaging/email/mailer' diff --git a/apps/sim/app/api/speech/token/route.ts b/apps/sim/app/api/speech/token/route.ts index aacadc145d5..4b520dc651a 100644 --- a/apps/sim/app/api/speech/token/route.ts +++ b/apps/sim/app/api/speech/token/route.ts @@ -22,7 +22,7 @@ import { env } from '@/lib/core/config/env' import { getCostMultiplier, isBillingEnabled } from '@/lib/core/config/env-flags' import { RateLimiter } from '@/lib/core/rate-limiter' import { validateAuthToken } from '@/lib/core/security/deployment' -import { getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { verifyWorkspaceMembership } from '@/app/api/workflows/utils' diff --git a/apps/sim/lib/analytics/profound.ts b/apps/sim/lib/analytics/profound.ts index ff8c568e14d..7c9507fbe7a 100644 --- a/apps/sim/lib/analytics/profound.ts +++ b/apps/sim/lib/analytics/profound.ts @@ -6,9 +6,10 @@ * @see https://docs.tryprofound.com/agent-analytics/custom */ import { createLogger } from '@sim/logger' +import { UNKNOWN_CLIENT_IP } from '@sim/security/client-ip' import { env } from '@/lib/core/config/env' import { isHosted } from '@/lib/core/config/env-flags' -import { getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' import { getBaseDomain } from '@/lib/core/utils/urls' const logger = createLogger('ProfoundAnalytics') @@ -104,7 +105,7 @@ export function sendToProfound(request: Request, statusCode: number): void { status_code: statusCode, ip: (() => { const resolved = getClientIp(request) - return resolved === 'unknown' ? '0.0.0.0' : resolved + return resolved === UNKNOWN_CLIENT_IP ? '0.0.0.0' : resolved })(), user_agent: request.headers.get('user-agent') || '', ...(Object.keys(queryParams).length > 0 && { query_params: queryParams }), diff --git a/apps/sim/lib/auth/internal.ts b/apps/sim/lib/auth/internal.ts index a94ee42011a..95d7ffb613a 100644 --- a/apps/sim/lib/auth/internal.ts +++ b/apps/sim/lib/auth/internal.ts @@ -3,7 +3,7 @@ import { safeCompare } from '@sim/security/compare' import { jwtVerify, SignJWT } from 'jose' import { type NextRequest, NextResponse } from 'next/server' import { env } from '@/lib/core/config/env' -import { getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' const logger = createLogger('CronAuth') diff --git a/apps/sim/lib/core/config/env.ts b/apps/sim/lib/core/config/env.ts index 9d16e13ab4f..2c94aa35f2e 100644 --- a/apps/sim/lib/core/config/env.ts +++ b/apps/sim/lib/core/config/env.ts @@ -507,7 +507,8 @@ export const env = createEnv({ REACT_SCAN_ENABLED: z.boolean().optional(), // Enable React Scan for performance debugging (dev only) // Network / proxy trust - AUTH_TRUSTED_PROXIES: z.string().optional(), // Comma-separated reverse-proxy IPs or CIDR ranges. When set, Better Auth walks the forwarded-IP chain right to left, skips these trusted hops, and uses the first untrusted address as the client IP. Leave unset to trust only single-value IP headers. + 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 diff --git a/apps/sim/lib/core/rate-limiter/rate-limiter.ts b/apps/sim/lib/core/rate-limiter/rate-limiter.ts index 9e274839d86..8230801dc35 100644 --- a/apps/sim/lib/core/rate-limiter/rate-limiter.ts +++ b/apps/sim/lib/core/rate-limiter/rate-limiter.ts @@ -204,6 +204,26 @@ export class RateLimiter { } } + /** + * Clears a single bucket addressed by its exact storage key — the counterpart + * to {@link checkRateLimitDirect}. Lets a caller keep a failure-only counter + * (consume on every attempt, reset once the attempt succeeds) so legitimate + * traffic never walks the bucket down. + * + * Never throws: a reset that fails leaves tokens consumed, which only makes + * the limit stricter. + */ + async resetRateLimitBucket(storageKey: string): Promise { + try { + await this.storage.resetBucket(storageKey) + } catch (error) { + logger.warn('Failed to reset rate limit bucket', { + storageKey, + error: toError(error).message, + }) + } + } + async resetRateLimit(rateLimitKey: string): Promise { try { await Promise.all([ diff --git a/apps/sim/lib/core/rate-limiter/route-helpers.test.ts b/apps/sim/lib/core/rate-limiter/route-helpers.test.ts index 0f895e81e1a..17b0af73a94 100644 --- a/apps/sim/lib/core/rate-limiter/route-helpers.test.ts +++ b/apps/sim/lib/core/rate-limiter/route-helpers.test.ts @@ -1,6 +1,7 @@ /** * @vitest-environment node */ +import { type ClientIpHeaderSource, resolveClientIp } from '@sim/security/client-ip' import { createMockRequest, requestUtilsMockFns } from '@sim/testing' import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest' @@ -22,12 +23,14 @@ vi.mock('@/lib/core/rate-limiter/storage', async () => { } }) +/** + * Route the globally-mocked `getClientIp` through the real resolver, so these + * assertions exercise the actual forwarded-header semantics rather than a + * hand-rolled restatement of them that could drift from the implementation. + */ function passThroughClientIp() { - requestUtilsMockFns.mockGetClientIp.mockImplementation( - (req: { headers: { get(name: string): string | null } }) => - req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() || - req.headers.get('x-real-ip')?.trim() || - 'unknown' + requestUtilsMockFns.mockGetClientIp.mockImplementation((req: ClientIpHeaderSource) => + resolveClientIp(req) ) } @@ -106,7 +109,7 @@ describe('route-helpers rate limiting', () => { passThroughClientIp() }) - it('uses the X-Forwarded-For client IP in the bucket key', async () => { + it('keys on the proxy-appended hop, not the caller-supplied leftmost one', async () => { consume.mockResolvedValueOnce({ allowed: true, tokensRemaining: 9, @@ -118,11 +121,7 @@ describe('route-helpers rate limiting', () => { await enforceIpRateLimit('public-bucket', request) - expect(consume).toHaveBeenCalledWith( - 'route:public-bucket:ip:203.0.113.7', - 1, - expect.any(Object) - ) + expect(consume).toHaveBeenCalledWith('route:public-bucket:ip:10.0.0.1', 1, expect.any(Object)) }) it('folds spoofed `X-Forwarded-For: unknown` into a single shared bucket', async () => { diff --git a/apps/sim/lib/core/rate-limiter/route-helpers.ts b/apps/sim/lib/core/rate-limiter/route-helpers.ts index f71115bf532..bb5b4e512cd 100644 --- a/apps/sim/lib/core/rate-limiter/route-helpers.ts +++ b/apps/sim/lib/core/rate-limiter/route-helpers.ts @@ -2,7 +2,7 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' import { RateLimiter } from '@/lib/core/rate-limiter/rate-limiter' import type { TokenBucketConfig } from '@/lib/core/rate-limiter/storage' -import { getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' const logger = createLogger('RouteRateLimit') const rateLimiter = new RateLimiter() diff --git a/apps/sim/lib/core/security/deployment-auth.ts b/apps/sim/lib/core/security/deployment-auth.ts index 69c842def61..d1777e990e8 100644 --- a/apps/sim/lib/core/security/deployment-auth.ts +++ b/apps/sim/lib/core/security/deployment-auth.ts @@ -10,7 +10,7 @@ import { validateAuthToken, } from '@/lib/core/security/deployment' import { decryptSecret } from '@/lib/core/security/encryption' -import { getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' const logger = createLogger('DeploymentAuth') @@ -26,6 +26,31 @@ const PASSWORD_IP_RATE_LIMIT: TokenBucketConfig = { refillIntervalMs: 15 * 60_000, } +/** + * Bounds *consecutive failed* guesses against one deployment secret, keyed on + * the resource rather than the caller. The IP bucket above cannot be the only + * defense: a distributed caller simply gets a fresh IP bucket per source, which + * leaves the secret itself with no ceiling at all. + * + * A token is consumed per attempt and the bucket is reset the moment a password + * verifies, so this counts *consecutive* failures — a resource that anyone is + * successfully signing into never drifts toward the limit. + * + * The ceiling is a deliberate trade, not a free win: because the check must run + * before the comparison to be worth anything, an exhausted bucket also rejects + * the correct password, so whoever burns it through locks out new visitors for + * the rest of the window (holders of an auth cookie are unaffected — that path + * returns before this one). It is sized so that only a genuinely distributed + * attack can get there: at 10 attempts per IP per window, tripping it takes ~50 + * distinct source addresses, while still capping blind guessing at 500 per 15 + * minutes instead of the unbounded rate a single spoofed header used to buy. + */ +const PASSWORD_RESOURCE_RATE_LIMIT: TokenBucketConfig = { + maxTokens: 500, + refillRate: 500, + refillIntervalMs: 15 * 60_000, +} + /** * A password/email-gated resource (a deployed chat or a public file share). Only * the fields the auth check needs — the `password` is the encrypted secret. @@ -122,11 +147,39 @@ export async function validateDeploymentAuth( } } + const resourceKey = `${cookiePrefix}-password:resource:${resource.id}` + /** + * `failClosed` because this is the only bound on distributed guessing at + * the secret: failing open would silently remove it during exactly the + * storage outage an attacker could wait for. The cost is bounded — the + * bucket store is Redis or the app database, and if the database is down + * the deployment is unreachable anyway. + */ + const resourceRateLimit = await rateLimiter.checkRateLimitDirect( + resourceKey, + PASSWORD_RESOURCE_RATE_LIMIT, + { failClosed: true } + ) + if (!resourceRateLimit.allowed) { + logger.warn( + `[${requestId}] Password attempt resource rate limit exceeded for ${resource.id}` + ) + return { + authorized: false, + error: 'Too many attempts. Please try again later.', + status: 429, + retryAfterMs: + resourceRateLimit.retryAfterMs ?? PASSWORD_RESOURCE_RATE_LIMIT.refillIntervalMs, + } + } + const { decrypted } = await decryptSecret(resource.password) if (!safeCompare(password, decrypted)) { return { authorized: false, error: 'Invalid password' } } + await rateLimiter.resetRateLimitBucket(resourceKey) + return { authorized: true } } catch (error) { logger.error(`[${requestId}] Error validating password:`, error) diff --git a/apps/sim/lib/core/utils/client-ip.test.ts b/apps/sim/lib/core/utils/client-ip.test.ts new file mode 100644 index 00000000000..61ce7d4f0c6 --- /dev/null +++ b/apps/sim/lib/core/utils/client-ip.test.ts @@ -0,0 +1,93 @@ +/** + * Covers the wiring between `AUTH_TRUSTED_PROXIES` and the shared resolver. + * `@/lib/core/utils/client-ip` is mocked globally in `vitest.setup.ts` (it is + * what every route consumes), so without the `vi.unmock` below the real module + * — and therefore the env read that makes trusted proxies take effect — would + * never execute anywhere in CI. + * + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockEnv } = vi.hoisted(() => ({ + 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.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 a handful of imports of a tiny module. + */ +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 +} + +function req(headers: Record) { + return { headers: new Headers(headers) } +} + +describe('getClientIp', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('keys on the proxy-appended hop, not the caller-supplied leftmost one', async () => { + const getClientIp = await loadGetClientIp(undefined) + + expect(getClientIp(req({ 'x-forwarded-for': '203.0.113.7, 10.0.0.1' }))).toBe('10.0.0.1') + }) + + it('honors AUTH_TRUSTED_PROXIES, resolving past the configured hop', async () => { + const getClientIp = await loadGetClientIp('10.0.0.0/24') + + expect(getClientIp(req({ 'x-forwarded-for': '203.0.113.7, 10.0.0.1' }))).toBe('203.0.113.7') + }) + + it('gives one bucket per caller no matter what they prepend', async () => { + const getClientIp = await loadGetClientIp(undefined) + const keys = ['9.9.9.9', 'unknown', '203.0.113.250'].map((spoof) => + getClientIp(req({ 'x-forwarded-for': `${spoof}, 10.0.0.1` })) + ) + + expect(new Set(keys)).toEqual(new Set(['10.0.0.1'])) + }) + + it('falls back to a shared bucket when no header yields an address', async () => { + const getClientIp = await loadGetClientIp(undefined) + + 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') + } + }) +}) diff --git a/apps/sim/lib/core/utils/client-ip.ts b/apps/sim/lib/core/utils/client-ip.ts new file mode 100644 index 00000000000..cd0ee003571 --- /dev/null +++ b/apps/sim/lib/core/utils/client-ip.ts @@ -0,0 +1,52 @@ +import { + type ClientIpHeaderSource, + parseTrustedProxies, + parseTrustForwardedHeaders, + resolveClientIp, + UNKNOWN_CLIENT_IP, +} from '@sim/security/client-ip' +import { env } from '@/lib/core/config/env' + +/** + * Reverse-proxy hops trusted for forwarded-IP resolution, read from the same + * `AUTH_TRUSTED_PROXIES` as Better Auth's `advanced.ipAddress.trustedProxies` + * (see `lib/auth/auth.ts`). Parsed once at module load. + * + * Configured, the two agree on who the caller is. Left unset they diverge by + * design: Better Auth trusts only a single-value header and records no IP for a + * longer chain, whereas a throttle cannot opt out of having a key, so this falls + * back to the rightmost — still proxy-written, never caller-authored. + */ +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 = parseTrustForwardedHeaders(env.TRUST_PROXY_HEADERS) + +/** + * Extract the client IP from a request for logging, audit trails, and — most + * importantly — per-IP rate-limit keys. + * + * Server-only: kept out of `@/lib/core/utils/request` so the `ipaddr.js` + * dependency never reaches a client bundle through that module's other exports. + * + * 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) +} diff --git a/apps/sim/lib/core/utils/request.ts b/apps/sim/lib/core/utils/request.ts index 3634c2f38c9..07b5f8ad814 100644 --- a/apps/sim/lib/core/utils/request.ts +++ b/apps/sim/lib/core/utils/request.ts @@ -10,17 +10,6 @@ export function generateRequestId(): string { return getRequestContext()?.requestId ?? generateId().slice(0, 8) } -/** - * Extract the client IP from a request, checking `x-forwarded-for` then `x-real-ip`. - */ -export function getClientIp(request: { headers: { get(name: string): string | null } }): string { - return ( - request.headers.get('x-forwarded-for')?.split(',')[0]?.trim() || - request.headers.get('x-real-ip')?.trim() || - 'unknown' - ) -} - /** * No-operation function for use as default callback */ diff --git a/apps/sim/lib/public-shares/rate-limit.ts b/apps/sim/lib/public-shares/rate-limit.ts index 60f7223a60d..696782afdb5 100644 --- a/apps/sim/lib/public-shares/rate-limit.ts +++ b/apps/sim/lib/public-shares/rate-limit.ts @@ -1,6 +1,6 @@ import { NextResponse } from 'next/server' import { RateLimiter, type TokenBucketConfig } from '@/lib/core/rate-limiter' -import { getClientIp } from '@/lib/core/utils/request' +import { getClientIp } from '@/lib/core/utils/client-ip' const rateLimiter = new RateLimiter() diff --git a/apps/sim/lib/webhooks/providers/generic.ts b/apps/sim/lib/webhooks/providers/generic.ts index 71372bebad6..1777fe21422 100644 --- a/apps/sim/lib/webhooks/providers/generic.ts +++ b/apps/sim/lib/webhooks/providers/generic.ts @@ -1,6 +1,6 @@ import { createLogger } from '@sim/logger' +import { canonicalizeIp, getAssertedOriginIp } from '@sim/security/client-ip' import { NextResponse } from 'next/server' -import { getClientIp } from '@/lib/core/utils/request' import type { AuthContext, EventFilterContext, @@ -31,9 +31,26 @@ export const genericHandler: WebhookProviderHandler = { const allowedIps = providerConfig.allowedIps if (allowedIps && Array.isArray(allowedIps) && allowedIps.length > 0) { - const clientIp = getClientIp(request) + /** + * Matches the *asserted* origin — the leftmost forwarded hop — because the + * operator's allowlist names the sending service (Stripe, GitHub, …), not + * the proxy in front of us. Rate-limit keys deliberately use the opposite + * end of the chain; see {@link getAssertedOriginIp} for why this value is + * a filter against honest senders rather than authentication. `requireAuth` + * is the control that actually authenticates. + * + * Both sides are canonicalized so an entry written as `::ffff:1.2.3.4` or + * `01.02.03.04` still matches the same address on the wire. + */ + const clientIp = getAssertedOriginIp(request) + const allowed = new Set( + allowedIps.flatMap((entry) => { + const canonical = typeof entry === 'string' ? canonicalizeIp(entry) : null + return canonical ? [canonical] : [] + }) + ) - if (clientIp === 'unknown' || !allowedIps.includes(clientIp)) { + if (!clientIp || !allowed.has(clientIp)) { logger.warn(`[${requestId}] Forbidden webhook access attempt - IP not allowed: ${clientIp}`) return new NextResponse('Forbidden - IP not allowed', { status: 403, diff --git a/apps/sim/proxy.ts b/apps/sim/proxy.ts index 73d03e9b797..adbffe8a528 100644 --- a/apps/sim/proxy.ts +++ b/apps/sim/proxy.ts @@ -5,7 +5,7 @@ import { sendToProfound } from './lib/analytics/profound' import { getEnv } from './lib/core/config/env' import { isAuthDisabled, isDev, isHosted } from './lib/core/config/env-flags' import { generateRuntimeCSP } from './lib/core/security/csp' -import { getClientIp } from './lib/core/utils/request' +import { getClientIp } from './lib/core/utils/client-ip' import { isNonCanonicalSimHost } from './lib/core/utils/urls' const logger = createLogger('Proxy') diff --git a/apps/sim/vitest.setup.ts b/apps/sim/vitest.setup.ts index 25c34639607..b0e7859ee9e 100644 --- a/apps/sim/vitest.setup.ts +++ b/apps/sim/vitest.setup.ts @@ -1,5 +1,6 @@ import { authMock, + clientIpMock, databaseMock, drizzleOrmMock, envFlagsMock, @@ -38,6 +39,7 @@ vi.mock('@sim/platform-authz/workflow', () => workflowAuthzMock) vi.mock('@/lib/auth', () => authMock) vi.mock('@/lib/auth/hybrid', () => hybridAuthMock) vi.mock('@/lib/core/utils/request', () => requestUtilsMock) +vi.mock('@/lib/core/utils/client-ip', () => clientIpMock) vi.mock('@/lib/core/config/env-flags', () => envFlagsMock) vi.mock('@/lib/core/config/env', () => envMock) vi.mock('@/lib/core/utils/urls', () => urlsMock) diff --git a/bun.lock b/bun.lock index 7670781748c..2166f729872 100644 --- a/bun.lock +++ b/bun.lock @@ -65,6 +65,7 @@ "@ai-sdk/react": "2.0.205", "@sim/db": "workspace:*", "@sim/emcn": "workspace:*", + "@sim/security": "workspace:*", "@sim/workflow-renderer": "workspace:*", "ai": "5.0.203", "class-variance-authority": "^0.7.1", @@ -384,6 +385,7 @@ "dependencies": { "@sim/db": "workspace:*", "@sim/logger": "workspace:*", + "@sim/security": "workspace:*", "@sim/utils": "workspace:*", "drizzle-orm": "^0.45.2", }, diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 363422c3013..3f3c7e7f75a 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -21,11 +21,31 @@ services: # (apex + www, alias hostnames, reverse-proxy IPs). Empty by default. - TRUSTED_ORIGINS=${TRUSTED_ORIGINS:-} # AUTH_TRUSTED_PROXIES: comma-separated reverse-proxy IPs or CIDR ranges in - # front of the app (ingress, load balancer). Better Auth walks - # x-forwarded-for right to left, skips these hops, and uses the first - # untrusted address as the client IP. Required for correct session IPs and - # rate-limit keying behind a multi-hop proxy chain. Empty by default. + # front of the app (ingress, load balancer). When set, 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 — never the + # leftmost entry, which the caller supplies and could otherwise be rotated + # to mint a fresh rate-limit bucket per request. Empty by default, and then + # the two differ: Better Auth trusts only a single-value header (recording + # no IP for a multi-hop chain), while Sim's throttles key on the rightmost + # (proxy-written) entry — not spoofable, but behind a multi-hop chain + # (e.g. CDN in front of ingress) it collapses callers onto the edge + # addresses. Set your real hops for per-client keying and correct session + # IPs. Use the proxies' actual addresses, NOT a broad private range that + # also covers clients: a caller inside a trusted range makes the whole + # 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:-} diff --git a/helm/sim/Chart.yaml b/helm/sim/Chart.yaml index d167b0cad4e..5f6ee8ff61e 100644 --- a/helm/sim/Chart.yaml +++ b/helm/sim/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: sim description: A Helm chart for Sim - the open-source AI workspace where teams build, deploy, and manage AI agents type: application -version: 1.4.0 +version: 1.4.1 appVersion: "v0.7.44" kubeVersion: ">=1.25.0-0" home: https://sim.ai diff --git a/helm/sim/templates/_helpers.tpl b/helm/sim/templates/_helpers.tpl index 4ecaf0d263b..6d040bde233 100644 --- a/helm/sim/templates/_helpers.tpl +++ b/helm/sim/templates/_helpers.tpl @@ -313,7 +313,7 @@ than enforced. {{- define "sim.validateExternalSecretCoverage" -}} {{- if and .Values.externalSecrets .Values.externalSecrets.enabled -}} {{- $remoteRefs := default (dict) (default (dict) .Values.externalSecrets.remoteRefs).app -}} -{{- $chartComputed := list "DATABASE_URL" "SOCKET_SERVER_URL" "OLLAMA_URL" "PII_URL" -}} +{{- $chartComputed := list "DATABASE_URL" "SOCKET_SERVER_URL" "OLLAMA_URL" "PII_URL" "TRUST_PROXY_HEADERS" -}} {{- $appEnv := default (dict) .Values.app.env -}} {{/* Required-key coverage: these are non-optional at runtime. With ESO enabled @@ -446,6 +446,31 @@ Ollama URL {{- end }} {{- end }} +{{/* +Whether the app may believe x-forwarded-for / x-real-ip. + +Derived from ingress.enabled rather than defaulted to "true": every rule about +which forwarded hop to read presumes a proxy wrote one of them. With the ingress +off, the Service is reached directly (ClusterIP port-forward, LoadBalancer, +NodePort) and the header is authored entirely by the caller — trusting it would +let anyone rotate it for a fresh per-IP rate-limit bucket per request. An +explicit app.env.TRUST_PROXY_HEADERS always wins, for edges the chart cannot see +(a Gateway API listener, a service mesh, an external LB that appends). +*/}} +{{- define "sim.trustProxyHeaders" -}} +{{- $explicit := toString ((default (dict) .Values.app.env).TRUST_PROXY_HEADERS) -}} +{{- /* + Compare the STRINGIFIED value, never the raw one: an explicit `false` is falsy + in Go templates, so `if $explicit` would silently discard the one override + that turns trust off and fall through to the ingress default. +*/ -}} +{{- if or (eq $explicit "") (eq $explicit "") -}} +{{- ternary "true" "false" .Values.ingress.enabled -}} +{{- else -}} +{{- $explicit -}} +{{- end -}} +{{- end }} + {{/* PII (Presidio) service URL */}} diff --git a/helm/sim/templates/deployment-app.yaml b/helm/sim/templates/deployment-app.yaml index f3506edf53c..86bc91c2704 100644 --- a/helm/sim/templates/deployment-app.yaml +++ b/helm/sim/templates/deployment-app.yaml @@ -91,6 +91,8 @@ spec: value: {{ include "sim.ollamaUrl" . | quote }} - name: PII_URL value: {{ include "sim.piiUrl" . | quote }} + - name: TRUST_PROXY_HEADERS + value: {{ include "sim.trustProxyHeaders" . | quote }} {{- /* Skip envDefaults keys that the user has explicitly overridden in app.env with a non-empty value. K8s `env` takes precedence over `envFrom`, so an @@ -119,7 +121,7 @@ spec: and in inline mode (values flow through the chart-managed Secret). */}} {{- if and .Values.app.secrets.existingSecret.enabled (not .Values.externalSecrets.enabled) }} - {{- $chartComputed := list "DATABASE_URL" "SOCKET_SERVER_URL" "OLLAMA_URL" "PII_URL" }} + {{- $chartComputed := list "DATABASE_URL" "SOCKET_SERVER_URL" "OLLAMA_URL" "PII_URL" "TRUST_PROXY_HEADERS" }} {{- range $key, $value := $appEnv }} {{- if and (ne (toString $value) "") (ne (toString $value) "") (not (has $key $chartComputed)) }} - name: {{ $key }} diff --git a/helm/sim/templates/deployment-realtime.yaml b/helm/sim/templates/deployment-realtime.yaml index 1e3487164bd..2db673eb020 100644 --- a/helm/sim/templates/deployment-realtime.yaml +++ b/helm/sim/templates/deployment-realtime.yaml @@ -62,6 +62,15 @@ spec: env: - name: DATABASE_URL value: {{ include "sim.databaseUrl" . | quote }} + {{- /* + Inlined for the same reason as on the app pod: @sim/audit runs here + too and reads this to decide whether a forwarded header may be + believed when stamping an audit row's ipAddress. Chart-computed, so + it is excluded from the shared Secret and must be set explicitly on + both deployments. + */}} + - name: TRUST_PROXY_HEADERS + value: {{ include "sim.trustProxyHeaders" . | quote }} {{- if .Values.telemetry.enabled }} {{- $nodeEnv := default (default "production" (index (.Values.realtime.envDefaults | default dict) "NODE_ENV")) (index (.Values.realtime.env | default dict) "NODE_ENV") }} # OpenTelemetry configuration @@ -112,7 +121,7 @@ spec: deployment. */}} {{- if and .Values.app.secrets.existingSecret.enabled (not .Values.externalSecrets.enabled) }} - {{- $chartComputed := list "DATABASE_URL" "SOCKET_SERVER_URL" "OLLAMA_URL" "PII_URL" }} + {{- $chartComputed := list "DATABASE_URL" "SOCKET_SERVER_URL" "OLLAMA_URL" "PII_URL" "TRUST_PROXY_HEADERS" }} {{- /* Build the effective realtime env from app.env as the base, then overlay non-empty realtime.env values. Sprig's `merge` keeps the diff --git a/helm/sim/templates/secrets-app.yaml b/helm/sim/templates/secrets-app.yaml index cf598096b5f..9385c206b63 100644 --- a/helm/sim/templates/secrets-app.yaml +++ b/helm/sim/templates/secrets-app.yaml @@ -18,7 +18,7 @@ metadata: {{- include "sim.app.labels" . | nindent 4 }} type: Opaque stringData: - {{- $chartComputed := list "DATABASE_URL" "SOCKET_SERVER_URL" "OLLAMA_URL" "PII_URL" }} + {{- $chartComputed := list "DATABASE_URL" "SOCKET_SERVER_URL" "OLLAMA_URL" "PII_URL" "TRUST_PROXY_HEADERS" }} {{- /* Intent: app.env is authoritative for shared keys (both pods envFrom this Secret, so the app container must not be silently overwritten by a diff --git a/helm/sim/values.schema.json b/helm/sim/values.schema.json index 9088f9bd6f1..79206dc8732 100644 --- a/helm/sim/values.schema.json +++ b/helm/sim/values.schema.json @@ -157,7 +157,11 @@ }, "AUTH_TRUSTED_PROXIES": { "type": "string", - "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." + "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", "boolean"], + "description": "Whether x-forwarded-for / x-real-ip may be believed at all. Leave empty to derive it from ingress.enabled: on with the ingress (which appends the peer address), off without it, since a directly-reached Service sees a header written entirely by the caller and trusting it makes per-IP rate limits bypassable per request. Set explicitly ('true'/'false', quoted or bare) only for an edge the chart cannot see, e.g. a Gateway API listener, a service mesh, or an external load balancer that appends." }, "SSO_TRUSTED_PROVIDER_IDS": { "type": "string", diff --git a/helm/sim/values.yaml b/helm/sim/values.yaml index 72f8cf52551..a9299ff0c01 100644 --- a/helm/sim/values.yaml +++ b/helm/sim/values.yaml @@ -80,10 +80,26 @@ app: # Merged into Better Auth `trustedOrigins` alongside NEXT_PUBLIC_APP_URL. Leave empty when serving from a single origin. TRUSTED_ORIGINS: "" # AUTH_TRUSTED_PROXIES: comma-separated reverse-proxy IPs or CIDR ranges in front of the app - # (ingress controller, load balancer). Better Auth walks x-forwarded-for right to left, skips - # these hops, and uses the first untrusted address as the client IP. Required for correct - # session IPs and rate-limit keying behind a multi-hop proxy chain (e.g. "10.0.0.0/16"). + # (ingress controller, load balancer). When set, 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 — never the leftmost entry, which the caller supplies and could otherwise be + # rotated to mint a fresh rate-limit bucket per request. Empty, the two differ: Better Auth + # trusts only a single-value header (recording no IP for a multi-hop chain), while Sim's + # throttles key on the rightmost (proxy-written) entry — not spoofable, but behind a multi-hop + # chain (e.g. CDN in front of ingress) it collapses callers onto the edge addresses. + # Set the ingress pods' actual addresses (e.g. "10.42.0.0/24"). Do NOT use a range broad enough + # 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 it is DERIVED from ingress.enabled — on with the ingress (which appends the peer + # address), off without it. That matters because ingress.enabled defaults to false: a Service + # reached directly (port-forward, LoadBalancer, NodePort) sees a header written entirely by the + # caller, and believing it lets anyone rotate a header for a fresh per-IP rate-limit bucket on + # every request. While off, per-IP limits collapse into one shared bucket — blunt, but it fails + # closed. Set it explicitly ("true"/"false") only for an edge the chart cannot see: a Gateway + # API listener, a service mesh, or an external load balancer that appends the peer address. + 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). diff --git a/packages/audit/package.json b/packages/audit/package.json index caaf323d8d5..dec8f4ae398 100644 --- a/packages/audit/package.json +++ b/packages/audit/package.json @@ -27,6 +27,7 @@ "dependencies": { "@sim/db": "workspace:*", "@sim/logger": "workspace:*", + "@sim/security": "workspace:*", "@sim/utils": "workspace:*", "drizzle-orm": "^0.45.2" }, diff --git a/packages/audit/src/log.test.ts b/packages/audit/src/log.test.ts index 98a71773e65..c95726ffc03 100644 --- a/packages/audit/src/log.test.ts +++ b/packages/audit/src/log.test.ts @@ -1,13 +1,7 @@ /** * @vitest-environment node */ -import { - auditMock, - dbChainMock, - dbChainMockFns, - requestUtilsMockFns, - resetDbChainMock, -} from '@sim/testing' +import { auditMock, dbChainMock, dbChainMockFns, resetDbChainMock } from '@sim/testing' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('@sim/db', () => ({ @@ -75,12 +69,6 @@ describe('recordAudit', () => { beforeEach(() => { vi.clearAllMocks() resetDbChainMock() - requestUtilsMockFns.mockGetClientIp.mockImplementation( - (request: { headers: { get(name: string): string | null } }) => - request.headers.get('x-forwarded-for')?.split(',')[0]?.trim() || - request.headers.get('x-real-ip')?.trim() || - 'unknown' - ) }) afterEach(() => { @@ -139,7 +127,7 @@ describe('recordAudit', () => { ) }) - it('extracts IP address from x-forwarded-for header', async () => { + it('records the proxy-supplied x-forwarded-for hop, not the caller-supplied one', async () => { const request = new Request('https://example.com', { headers: { 'x-forwarded-for': '1.2.3.4, 5.6.7.8', @@ -161,12 +149,34 @@ describe('recordAudit', () => { expect(dbChainMockFns.values).toHaveBeenCalledWith( expect.objectContaining({ - ipAddress: '1.2.3.4', + ipAddress: '5.6.7.8', userAgent: 'TestAgent/1.0', }) ) }) + it('does not let a caller forge the audited IP by prepending to x-forwarded-for', async () => { + const request = new Request('https://example.com', { + headers: { 'x-forwarded-for': '203.0.113.9, 5.6.7.8' }, + }) + + recordAudit({ + workspaceId: 'ws-1', + actorId: 'user-1', + actorName: 'Test', + actorEmail: 'test@test.com', + action: AuditAction.MEMBER_INVITED, + resourceType: AuditResourceType.WORKSPACE, + request, + }) + + await flush() + + expect(dbChainMockFns.values).toHaveBeenCalledWith( + expect.objectContaining({ ipAddress: '5.6.7.8' }) + ) + }) + it('falls back to x-real-ip when x-forwarded-for is absent', async () => { const request = new Request('https://example.com', { headers: { 'x-real-ip': '10.0.0.1' }, diff --git a/packages/audit/src/log.ts b/packages/audit/src/log.ts index 93381ae43b7..b38e422ebcc 100644 --- a/packages/audit/src/log.ts +++ b/packages/audit/src/log.ts @@ -1,5 +1,12 @@ import { auditLog, db, user } from '@sim/db' import { createLogger } from '@sim/logger' +import { + type ClientIpHeaderSource, + parseTrustedProxies, + parseTrustForwardedHeaders, + resolveClientIp, + UNKNOWN_CLIENT_IP, +} from '@sim/security/client-ip' import { generateShortId } from '@sim/utils/id' import { eq } from 'drizzle-orm' import type { AuditActionType, AuditResourceTypeValue } from './types' @@ -23,15 +30,32 @@ interface AuditLogParams { resourceName?: string description?: string metadata?: Record - request?: { headers: { get(name: string): string | null } } + request?: ClientIpHeaderSource } -function getClientIp(request: { headers: { get(name: string): string | null } }): string { - return ( - request.headers.get('x-forwarded-for')?.split(',')[0]?.trim() || - request.headers.get('x-real-ip')?.trim() || - 'unknown' - ) +/** + * Reverse-proxy hops trusted for forwarded-IP resolution. Read from the + * environment directly rather than the app's env module so this package stays + * free of `apps/*` imports; the value is the same `AUTH_TRUSTED_PROXIES` Better + * Auth and Sim's rate limiters use, so an audit row's IP matches the session's. + */ +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 = parseTrustForwardedHeaders(process.env.TRUST_PROXY_HEADERS) + +/** + * 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) } /** diff --git a/packages/security/package.json b/packages/security/package.json index 68b9e74dfb6..e839e44f229 100644 --- a/packages/security/package.json +++ b/packages/security/package.json @@ -10,6 +10,10 @@ "node": ">=20.0.0" }, "exports": { + "./client-ip": { + "types": "./src/client-ip.ts", + "default": "./src/client-ip.ts" + }, "./compare": { "types": "./src/compare.ts", "default": "./src/compare.ts" diff --git a/packages/security/src/client-ip.test.ts b/packages/security/src/client-ip.test.ts new file mode 100644 index 00000000000..f980a7e2d40 --- /dev/null +++ b/packages/security/src/client-ip.test.ts @@ -0,0 +1,300 @@ +import { describe, expect, it } from 'vitest' +import { + canonicalizeIp, + getAssertedOriginIp, + parseTrustedProxies, + parseTrustForwardedHeaders, + resolveClientIp, + UNKNOWN_CLIENT_IP, +} from './client-ip' + +function req(headers: Record) { + return { headers: new Headers(headers) } +} + +describe('resolveClientIp', () => { + describe('spoofing resistance', () => { + it('ignores a caller-supplied leftmost hop in favour of the proxy-appended one', () => { + expect(resolveClientIp(req({ 'x-forwarded-for': '203.0.113.7, 198.51.100.4' }))).toBe( + '198.51.100.4' + ) + }) + + it('returns the same address regardless of what the caller prepends', () => { + const a = resolveClientIp(req({ 'x-forwarded-for': '10.0.0.1, 198.51.100.4' })) + const b = resolveClientIp(req({ 'x-forwarded-for': '10.0.0.2, 198.51.100.4' })) + const c = resolveClientIp(req({ 'x-forwarded-for': 'unknown, 198.51.100.4' })) + expect(new Set([a, b, c])).toEqual(new Set(['198.51.100.4'])) + }) + + it('strips IPv6 zone ids so they cannot mint unbounded distinct keys', () => { + // `ipaddr.isValid` accepts an arbitrary-length zone and `process()` keeps + // it verbatim, so an unstripped zone would be attacker-chosen text in the + // key. The /64 mask happens to drop zones from v6 keys too — the + // getAssertedOriginIp cases below pin the stripping on its own, since + // that path is deliberately unmasked. + const zoned = ['fe80::1%eth0', 'fe80::1%evil', `fe80::1%${'x'.repeat(200)}`].map((value) => + resolveClientIp(req({ 'x-forwarded-for': value })) + ) + expect(new Set(zoned)).toEqual(new Set(['fe80::'])) + expect(resolveClientIp(req({ 'x-real-ip': 'fe80::1%evil' }))).toBe('fe80::') + expect(canonicalizeIp('fe80::1%evil')).toBe('fe80::1') + }) + + it('masks IPv6 to its routed /64 so one subscriber is one bucket', () => { + // A single IPv6 client is delegated a whole /64, so the proxy honestly + // writes a different address per request. Keying on the full /128 would + // leave per-IP throttles bypassable with no spoofing at all. + const withinOnePrefix = [ + '2001:db8:1:2::1', + '2001:db8:1:2::dead:beef', + '2001:db8:1:2:ffff:ffff:ffff:ffff', + ].map((value) => resolveClientIp(req({ 'x-forwarded-for': value }))) + expect(new Set(withinOnePrefix)).toEqual(new Set(['2001:db8:1:2::'])) + }) + + it('keeps distinct IPv6 /64s in distinct buckets', () => { + const a = resolveClientIp(req({ 'x-forwarded-for': '2001:db8:1:2::1' })) + const b = resolveClientIp(req({ 'x-forwarded-for': '2001:db8:1:3::1' })) + expect(a).not.toBe(b) + }) + + it('does not mask IPv4, which is already a single host', () => { + expect(resolveClientIp(req({ 'x-forwarded-for': '198.51.100.4' }))).toBe('198.51.100.4') + }) + + it('masks the x-real-ip fallback too', () => { + expect(resolveClientIp(req({ 'x-real-ip': '2001:db8:1:2::99' }))).toBe('2001:db8:1:2::') + }) + + it('matches a trusted range against the full address, not the masked key', () => { + // Masking before the trust check would compare a different address. + const trusted = parseTrustedProxies('2001:db8:1:2::abcd/128') + expect( + resolveClientIp(req({ 'x-forwarded-for': '203.0.113.7, 2001:db8:1:2::abcd' }), trusted) + ).toBe('203.0.113.7') + }) + + it('collapses equivalent spellings of one address onto a single value', () => { + const forms = ['198.51.100.4', '::ffff:198.51.100.4', '0xc6336404', '198.51.100.4:4444'] + const resolved = forms.map((form) => resolveClientIp(req({ 'x-forwarded-for': form }))) + expect(new Set(resolved)).toEqual(new Set(['198.51.100.4'])) + }) + }) + + describe('trusted proxy chain', () => { + it('skips trusted hops and returns the first untrusted address', () => { + const trusted = parseTrustedProxies('198.51.100.0/24') + expect( + resolveClientIp(req({ 'x-forwarded-for': '203.0.113.7, 198.51.100.4' }), trusted) + ).toBe('203.0.113.7') + }) + + it('accepts bare addresses as single-host trusted ranges', () => { + const trusted = parseTrustedProxies('198.51.100.4, 192.0.2.10') + expect( + resolveClientIp( + req({ 'x-forwarded-for': '203.0.113.7, 192.0.2.10, 198.51.100.4' }), + trusted + ) + ).toBe('203.0.113.7') + }) + + it('stops at the first untrusted hop rather than walking to the leftmost', () => { + const trusted = parseTrustedProxies('198.51.100.4') + expect( + resolveClientIp(req({ 'x-forwarded-for': '10.0.0.1, 203.0.113.7, 198.51.100.4' }), trusted) + ).toBe('203.0.113.7') + }) + + it('falls back to the rightmost hop when the whole chain is trusted', () => { + const trusted = parseTrustedProxies('198.51.100.0/24') + expect( + resolveClientIp(req({ 'x-forwarded-for': '198.51.100.1, 198.51.100.4' }), trusted) + ).toBe('198.51.100.4') + }) + + it('cannot be bypassed by forging a hop from inside a broad trusted range', () => { + // The docs recommend ranges like 10.0.0.0/16. A caller who forges an + // address from inside it makes every hop "trusted"; the resolver must + // still land on the proxy-written hop, not the forged one. + const trusted = parseTrustedProxies('10.0.0.0/16') + const forged = ['10.0.99.99', '10.0.7.7', '10.0.1.2'].map((spoof) => + resolveClientIp(req({ 'x-forwarded-for': `${spoof}, 10.0.0.1` }), trusted) + ) + expect(new Set(forged)).toEqual(new Set(['10.0.0.1'])) + }) + + it('does not treat an IPv6 hop as matching an IPv4 trusted range', () => { + // Two hops, so a wrongly-trusted rightmost entry would visibly shift the + // answer left rather than merely avoiding a kind-mismatch throw. + const trusted = parseTrustedProxies('0.0.0.0/0') + expect( + resolveClientIp(req({ 'x-forwarded-for': '2001:db8:1::1, 2001:db8:2::2' }), trusted) + ).toBe('2001:db8:2::') + }) + + it('matches an IPv4-mapped IPv6 trusted range against the unwrapped hop', () => { + const trusted = parseTrustedProxies('::ffff:10.0.0.0/104') + expect( + resolveClientIp(req({ 'x-forwarded-for': '203.0.113.5, ::ffff:10.0.0.1' }), trusted) + ).toBe('203.0.113.5') + }) + + it('matches IPv6 trusted ranges', () => { + const trusted = parseTrustedProxies('2001:db8::/32') + expect(resolveClientIp(req({ 'x-forwarded-for': '203.0.113.7, 2001:db8::1' }), trusted)).toBe( + '203.0.113.7' + ) + }) + }) + + describe('header parsing', () => { + it('handles a single-hop chain', () => { + expect(resolveClientIp(req({ 'x-forwarded-for': '198.51.100.4' }))).toBe('198.51.100.4') + }) + + it('strips brackets and ports from IPv6 hops', () => { + // Masked to /64 like every IPv6 key; unmasked forms are covered by + // getAssertedOriginIp below. + expect(resolveClientIp(req({ 'x-forwarded-for': '[2001:db8::1]:8080' }))).toBe('2001:db8::') + }) + + it('accepts a bare IPv6 literal', () => { + expect(resolveClientIp(req({ 'x-forwarded-for': '2001:db8::1' }))).toBe('2001:db8::') + }) + + it('skips unparseable hops while walking right to left', () => { + expect(resolveClientIp(req({ 'x-forwarded-for': '198.51.100.4, _hidden' }))).toBe( + '198.51.100.4' + ) + }) + + it('falls back to x-real-ip when x-forwarded-for holds no address', () => { + expect( + resolveClientIp(req({ 'x-forwarded-for': 'unknown', 'x-real-ip': '198.51.100.4' })) + ).toBe('198.51.100.4') + }) + + it('prefers x-forwarded-for over x-real-ip when both parse', () => { + expect( + resolveClientIp(req({ 'x-forwarded-for': '198.51.100.4', 'x-real-ip': '10.0.0.1' })) + ).toBe('198.51.100.4') + }) + + it('returns the unknown sentinel when no header yields an address', () => { + expect(resolveClientIp(req({}))).toBe(UNKNOWN_CLIENT_IP) + expect(resolveClientIp(req({ 'x-forwarded-for': 'unknown, garbage' }))).toBe( + UNKNOWN_CLIENT_IP + ) + }) + }) +}) + +describe('getAssertedOriginIp', () => { + it('returns the leftmost hop — the sender the delivery claims to be', () => { + expect(getAssertedOriginIp(req({ 'x-forwarded-for': '203.0.113.7, 10.0.0.1' }))).toBe( + '203.0.113.7' + ) + }) + + it('is the opposite end of the chain from the throttling key', () => { + const request = req({ 'x-forwarded-for': '203.0.113.7, 10.0.0.1' }) + expect(getAssertedOriginIp(request)).not.toBe(resolveClientIp(request)) + }) + + it('canonicalizes so an allowlist entry matches any spelling of the address', () => { + expect(getAssertedOriginIp(req({ 'x-forwarded-for': '::ffff:203.0.113.7' }))).toBe( + canonicalizeIp('203.0.113.7') + ) + expect(canonicalizeIp('01.02.03.04')).toBe('1.2.3.4') + expect(canonicalizeIp('2001:0db8:0000:0000:0000:0000:0000:0001')).toBe('2001:db8::1') + }) + + it('skips unparseable leading hops', () => { + expect(getAssertedOriginIp(req({ 'x-forwarded-for': 'unknown, 203.0.113.7' }))).toBe( + '203.0.113.7' + ) + }) + + it('falls back to x-real-ip for proxies that set it instead of a chain', () => { + expect(getAssertedOriginIp(req({ 'x-real-ip': '203.0.113.7' }))).toBe('203.0.113.7') + expect( + getAssertedOriginIp(req({ 'x-forwarded-for': 'unknown', 'x-real-ip': '203.0.113.7' })) + ).toBe('203.0.113.7') + }) + + it('prefers the forwarded chain over x-real-ip when both parse', () => { + expect( + getAssertedOriginIp(req({ 'x-forwarded-for': '203.0.113.7', 'x-real-ip': '10.0.0.1' })) + ).toBe('203.0.113.7') + }) + + it('does not mask IPv6 — an allowlist needs the exact address', () => { + expect(getAssertedOriginIp(req({ 'x-forwarded-for': '2001:db8:1:2::99' }))).toBe( + '2001:db8:1:2::99' + ) + }) + + it('strips brackets, ports, and zone ids like the resolver does', () => { + expect(getAssertedOriginIp(req({ 'x-forwarded-for': '[2001:db8::1%eth0]:8080' }))).toBe( + '2001:db8::1' + ) + expect(getAssertedOriginIp(req({ 'x-forwarded-for': '203.0.113.7:4444' }))).toBe('203.0.113.7') + }) + + it('returns null when no header yields an address', () => { + expect(getAssertedOriginIp(req({}))).toBeNull() + expect(getAssertedOriginIp(req({ 'x-forwarded-for': 'unknown' }))).toBeNull() + expect(canonicalizeIp('not-an-ip')).toBeNull() + }) +}) + +describe('parseTrustedProxies', () => { + it('treats empty, null, and undefined input as trusting nothing', () => { + expect(parseTrustedProxies('').cidrs).toHaveLength(0) + expect(parseTrustedProxies(null).cidrs).toHaveLength(0) + expect(parseTrustedProxies(undefined).cidrs).toHaveLength(0) + }) + + it('drops malformed entries instead of throwing, keeping the valid ones', () => { + const trusted = parseTrustedProxies('not-an-ip, 10.0.0.0/99, , 198.51.100.4') + expect(trusted.cidrs).toHaveLength(1) + expect(resolveClientIp(req({ 'x-forwarded-for': '203.0.113.7, 198.51.100.4' }), trusted)).toBe( + '203.0.113.7' + ) + }) + + it('ignores surrounding whitespace', () => { + const trusted = parseTrustedProxies(' 198.51.100.0/24 , 192.0.2.10 ') + expect(trusted.cidrs).toHaveLength(2) + }) +}) + +describe('parseTrustForwardedHeaders', () => { + it('defaults to trusting the headers when unset', () => { + // Unset must never silently disable IP resolution — that would turn every + // per-IP limit into one global bucket on an ordinary proxied deployment. + expect(parseTrustForwardedHeaders(undefined)).toBe(true) + expect(parseTrustForwardedHeaders(null)).toBe(true) + expect(parseTrustForwardedHeaders('')).toBe(true) + expect(parseTrustForwardedHeaders(' ')).toBe(true) + }) + + it('accepts the usual falsey spellings, case- and space-insensitively', () => { + for (const value of ['false', 'FALSE', ' False ', '0', 'no', 'off', 'OFF']) { + expect(parseTrustForwardedHeaders(value)).toBe(false) + } + }) + + it('accepts a real boolean, since one caller reads a parsed env', () => { + expect(parseTrustForwardedHeaders(false)).toBe(false) + expect(parseTrustForwardedHeaders(true)).toBe(true) + }) + + it('treats anything else as trusting', () => { + for (const value of ['true', 'yes', 'on', '1', 'anything']) { + expect(parseTrustForwardedHeaders(value)).toBe(true) + } + }) +}) diff --git a/packages/security/src/client-ip.ts b/packages/security/src/client-ip.ts new file mode 100644 index 00000000000..bf88879b69d --- /dev/null +++ b/packages/security/src/client-ip.ts @@ -0,0 +1,267 @@ +import * as ipaddr from 'ipaddr.js' + +type ParsedIp = ipaddr.IPv4 | ipaddr.IPv6 +type ParsedCidr = readonly [ParsedIp, number] + +/** Anything with a case-insensitive header getter — `Request`, `NextRequest`, `Headers`. */ +export interface ClientIpHeaderSource { + headers: { get(name: string): string | null } +} + +/** + * A prepared trusted-proxy list. Build it once at module scope with + * {@link parseTrustedProxies} and reuse it — parsing on every request would + * re-tokenize the CIDRs for no benefit. + */ +export interface TrustedProxyList { + readonly cidrs: readonly ParsedCidr[] +} + +/** The default list: no proxy hop is trusted to have vouched for the hop left of it. */ +const NO_TRUSTED_PROXIES: TrustedProxyList = { cidrs: [] } + +/** Returned when no header yields a parseable address. Callers share one bucket for it. */ +export const UNKNOWN_CLIENT_IP = 'unknown' + +/** + * Prefix an IPv6 address is masked to before it becomes a rate-limit key. + * + * A single IPv6 client is routinely delegated a whole /64 — that is the standard + * residential and cloud allocation — so every request can legitimately carry a + * different source address with no spoofing whatsoever. Keying on the full /128 + * would therefore leave per-IP throttles just as bypassable over IPv6 as the + * forwarded-header bug this module exists to close, except the proxy itself + * writes the varying value and nothing looks wrong. + * + * Masking to the routed prefix makes one subscriber one bucket. Matches Better + * Auth's `ipv6Subnet` default, so session and throttle keys agree. + */ +const IPV6_KEY_PREFIX_BITS = 64 + +/** + * Reduces a forwarded-hop token to a bare address, dropping brackets, a port, + * and any IPv6 zone id — `[::1]:8080`, `[::1]`, `1.2.3.4:5678`, `fe80::1%eth0`. + * A bare IPv6 literal (two or more colons, no brackets) keeps its colons; the + * single-colon rule only strips a port from `host:port`. + */ +function stripPortAndBrackets(value: string): string { + let host = value + if (host.startsWith('[')) { + const end = host.indexOf(']') + host = end === -1 ? host.slice(1) : host.slice(1, end) + } else { + const firstColon = host.indexOf(':') + if (firstColon !== -1 && host.indexOf(':', firstColon + 1) === -1) { + host = host.slice(0, firstColon) + } + } + // Drop any IPv6 zone id (`fe80::1%eth0`). `ipaddr.isValid` accepts an + // arbitrary-length zone and `process()` preserves it verbatim, so keeping it + // would hand a caller an unbounded supply of distinct-but-equivalent strings + // to use as rate-limit keys — and let them write arbitrary text into keys and + // audit rows. The zone is a local interface selector, never client identity. + const zone = host.indexOf('%') + return zone === -1 ? host : host.slice(0, zone) +} + +/** + * Parses one forwarded-hop token into a canonical address, or `null` when it is + * not an IP at all (`unknown`, `_hidden`, an injected junk value). `process()` + * collapses equivalent spellings — IPv4-mapped IPv6, octal and hex IPv4 — onto + * one representation, so a caller cannot multiply rate-limit buckets by varying + * the encoding of a single address. + */ +function parseHop(raw: string): ParsedIp | null { + const value = stripPortAndBrackets(raw.trim()) + if (!value || !ipaddr.isValid(value)) return null + try { + return ipaddr.process(value) + } catch { + return null + } +} + +/** + * Rewrites an IPv4-mapped IPv6 range (`::ffff:10.0.0.0/104`) to its IPv4 form so + * it can match hops, which {@link parseHop} always unwraps to IPv4. Without this + * the kinds never agree and the entry is silently inert. + */ +function normalizeCidr(cidr: ParsedCidr): ParsedCidr { + const [addr, bits] = cidr + if (addr.kind() !== 'ipv6') return cidr + const v6 = addr as ipaddr.IPv6 + if (!v6.isIPv4MappedAddress() || bits < 96) return cidr + return [v6.toIPv4Address(), bits - 96] +} + +/** + * Renders an address as a rate-limit key, masking IPv6 to + * {@link IPV6_KEY_PREFIX_BITS} so one delegated prefix is one bucket. IPv4 is + * returned exactly — a v4 address is a single host. + * + * Applied only at the point a key is produced, never before + * {@link isTrustedProxy}: a masked address would compare against trusted ranges + * as a different (and wrong) address. + */ +function toKey(addr: ParsedIp): string { + if (addr.kind() !== 'ipv6') return addr.toString() + const bytes = (addr as ipaddr.IPv6).toByteArray() + for (let i = IPV6_KEY_PREFIX_BITS / 8; i < bytes.length; i++) bytes[i] = 0 + return ipaddr.fromByteArray(bytes).toString() +} + +function isTrustedProxy(addr: ParsedIp, trustedProxies: TrustedProxyList): boolean { + for (const [range, bits] of trustedProxies.cidrs) { + if (addr.kind() !== range.kind()) continue + if (addr.kind() === 'ipv4') { + if ((addr as ipaddr.IPv4).match(range as ipaddr.IPv4, bits)) return true + } else if ((addr as ipaddr.IPv6).match(range as ipaddr.IPv6, bits)) return true + } + return false +} + +/** + * Parses a comma-separated trusted-proxy setting (`AUTH_TRUSTED_PROXIES`) into + * matchable ranges. Entries may be CIDRs (`10.0.0.0/24`) or bare addresses, + * which become single-host ranges. + * + * Unparseable entries are dropped rather than thrown on: a typo must not take + * the app down, and dropping an entry can only make IP resolution stricter + * (one fewer hop is skipped), never more permissive. + */ +export function parseTrustedProxies(raw: string | null | undefined): TrustedProxyList { + const cidrs: ParsedCidr[] = [] + for (const entry of (raw ?? '').split(',')) { + const value = entry.trim() + if (!value) continue + try { + if (value.includes('/')) { + cidrs.push(normalizeCidr(ipaddr.parseCIDR(value))) + continue + } + const addr = parseHop(value) + if (addr) cidrs.push([addr, addr.kind() === 'ipv4' ? 32 : 128]) + } catch { + // Malformed entry — skip it. + } + } + return { cidrs } +} + +/** + * Reads the `TRUST_PROXY_HEADERS` setting: may `X-Forwarded-For` / `X-Real-IP` + * be believed at all? + * + * Every rule about *which* hop to read presumes a proxy wrote one of them. An + * app reachable directly sees a header authored entirely by the caller, and no + * parsing strategy recovers a real address from that — so this is a deployment + * fact the operator has to state, not something the code can detect. + * + * Defaults to `true` (a proxy is assumed) so an unset value never silently + * disables IP resolution. Accepts a boolean or the usual string spellings, + * because the value arrives parsed from the app's env module in one caller and + * raw from `process.env` in others. + */ +export function parseTrustForwardedHeaders(raw: string | boolean | null | undefined): boolean { + if (typeof raw === 'boolean') return raw + return !/^(false|0|no|off)$/i.test((raw ?? '').trim()) +} + +/** + * Resolves the client IP behind a reverse proxy, safely enough to key a rate + * limit on. + * + * `X-Forwarded-For` is a chain that every hop *appends* to, so the **leftmost** + * entry is whatever the original caller sent — fully attacker-controlled — while + * the **rightmost** was written by the proxy directly in front of the app. This + * walks the chain right to left, skips hops that match {@link TrustedProxyList}, + * and returns the first address that is not a trusted proxy. That is the closest + * hop the infrastructure actually vouched for. + * + * With no trusted proxies configured the rightmost entry wins. Behind a longer + * chain (e.g. a CDN in front of an ingress) that collapses callers onto the edge + * addresses and throttles them together — coarse, but it fails closed. Listing + * the real hops in `AUTH_TRUSTED_PROXIES` restores per-client keying. + * + * When *every* entry is trusted the walk falls back to the **rightmost** hop, + * never the leftmost. This matters: operators are told to configure ranges like + * `10.0.0.0/16`, and a caller who forges `X-Forwarded-For: 10.0..` + * from inside that range would otherwise make the whole chain "trusted" and get + * their own forged value back — reinstating the very bucket-per-request bypass + * this function exists to close. The rightmost hop is the one the adjacent proxy + * wrote, so it is the only entry a caller can never author. + * + * `X-Real-IP` is the fallback when `X-Forwarded-For` carries no parseable + * address, and {@link UNKNOWN_CLIENT_IP} when neither header does. + * + * The result is a **bucket key, not an address**: IPv6 is masked to + * {@link IPV6_KEY_PREFIX_BITS} (see there for why a /128 key is bypassable). + * Use {@link getAssertedOriginIp} when an exact address is required. + * + * None of this helps if no proxy sets the header at all — a directly-exposed + * app sees only what the caller wrote, and no parsing rule can recover from + * that. Terminate at a proxy that appends the peer address. + * + * @param request Any object exposing a header getter. + * @param trustedProxies Prepared list from {@link parseTrustedProxies}. + */ +export function resolveClientIp( + request: ClientIpHeaderSource, + trustedProxies: TrustedProxyList = NO_TRUSTED_PROXIES +): string { + const forwarded = request.headers.get('x-forwarded-for') + if (forwarded) { + const hops = forwarded.split(',') + let rightmostParsed: string | null = null + for (let i = hops.length - 1; i >= 0; i--) { + const addr = parseHop(hops[i]) + if (!addr) continue + if (!isTrustedProxy(addr, trustedProxies)) return toKey(addr) + rightmostParsed ??= toKey(addr) + } + if (rightmostParsed) return rightmostParsed + } + + const realIp = parseHop(request.headers.get('x-real-ip') ?? '') + return realIp ? toKey(realIp) : UNKNOWN_CLIENT_IP +} + +/** + * The **leftmost** `X-Forwarded-For` hop — the origin address *asserted* by the + * caller — in canonical form, falling back to `X-Real-IP`, or `null` when + * neither header yields an address. + * + * Deliberately the opposite end of the chain from {@link resolveClientIp}, and + * usable for exactly one thing: comparing against an operator-configured + * allowlist of expected senders, where the question is "which address does this + * delivery claim to come from" rather than "who do I throttle". The `X-Real-IP` + * fallback matters because a proxy may set it *instead of* a forwarded chain, + * and it is then the only record of the sender. + * + * **Never key a rate limit, quota, or lockout on this.** Under any proxy that + * appends to `X-Forwarded-For` the value is caller-controlled and can be rotated + * per request. An allowlist built on it is a filter against honest senders, not + * an authentication mechanism — pair it with a shared secret or signature. + */ +export function getAssertedOriginIp(request: ClientIpHeaderSource): string | null { + const forwarded = request.headers.get('x-forwarded-for') + for (const hop of forwarded?.split(',') ?? []) { + const addr = parseHop(hop) + if (addr) return addr.toString() + } + const realIp = parseHop(request.headers.get('x-real-ip') ?? '') + return realIp ? realIp.toString() : null +} + +/** + * Canonicalizes an operator-supplied address so it can be compared against + * {@link getAssertedOriginIp}. Returns `null` when the entry is not an IP. + * + * Needed because both sides must agree on spelling: `::ffff:1.2.3.4`, + * `01.02.03.04`, and `1.2.3.4` are one address, and a config entry typed in a + * non-canonical form would otherwise never match. + */ +export function canonicalizeIp(value: string): string | null { + const addr = parseHop(value) + return addr ? addr.toString() : null +} diff --git a/packages/testing/src/mocks/index.ts b/packages/testing/src/mocks/index.ts index 7ef622c4d61..b4fc53e5b1e 100644 --- a/packages/testing/src/mocks/index.ts +++ b/packages/testing/src/mocks/index.ts @@ -134,6 +134,7 @@ export { } from './redis-config.mock' // Request mocks export { + clientIpMock, createMockFormDataRequest, createMockRequest, requestUtilsMock, diff --git a/packages/testing/src/mocks/request.mock.ts b/packages/testing/src/mocks/request.mock.ts index 614366ad938..1298b3e1527 100644 --- a/packages/testing/src/mocks/request.mock.ts +++ b/packages/testing/src/mocks/request.mock.ts @@ -67,7 +67,8 @@ export function createMockFormDataRequest( } /** - * Controllable mock functions for `@/lib/core/utils/request`. + * Controllable mock functions for `@/lib/core/utils/request` and + * `@/lib/core/utils/client-ip`. * * @example * ```ts @@ -92,6 +93,19 @@ export const requestUtilsMockFns = { */ export const requestUtilsMock = { generateRequestId: requestUtilsMockFns.mockGenerateRequestId, - getClientIp: requestUtilsMockFns.mockGetClientIp, noop: () => {}, } + +/** + * Static mock module for `@/lib/core/utils/client-ip`. Separate from + * {@link requestUtilsMock} because the real module pulls `ipaddr.js` and the + * app env, which is exactly why it is not part of `utils/request`. + * + * @example + * ```ts + * vi.mock('@/lib/core/utils/client-ip', () => clientIpMock) + * ``` + */ +export const clientIpMock = { + getClientIp: requestUtilsMockFns.mockGetClientIp, +}