From 97acafe85e0efd8d39286ec507af2c5ba70097ab Mon Sep 17 00:00:00 2001 From: Elan Hasson <234704+ElanHasson@users.noreply.github.com> Date: Tue, 27 Jan 2026 20:33:55 +0000 Subject: [PATCH] feat(gateway): add CIDR support for trustedProxies configuration - Add CIDR subnet matching for trustedProxies (e.g., 188.114.96.0/20) - Support both IPv4 and IPv6 CIDR ranges - Add multi-proxy chain support for X-Forwarded-For header - Walk XFF chain right-to-left to find real client through proxy layers - Use ip-address npm package for robust IP/CIDR validation - Add comprehensive unit tests for CIDR matching and proxy chains - Add e2e test for CIDR range validation - Update documentation with CIDR examples and multi-proxy setup --- docs/gateway/configuration.md | 8 +- docs/gateway/security/index.md | 18 ++- package.json | 1 + pnpm-lock.yaml | 98 ++--------- src/gateway/net.test.ts | 242 +++++++++++++++++++++++++++- src/gateway/net.ts | 85 +++++++++- src/gateway/server.auth.e2e.test.ts | 44 +++++ 7 files changed, 399 insertions(+), 97 deletions(-) diff --git a/docs/gateway/configuration.md b/docs/gateway/configuration.md index f5438fb46..95c19ac37 100644 --- a/docs/gateway/configuration.md +++ b/docs/gateway/configuration.md @@ -2856,9 +2856,11 @@ Related docs: - [Remote access](/gateway/remote) Trusted proxies: -- `gateway.trustedProxies`: list of reverse proxy IPs that terminate TLS in front of the Gateway. -- When a connection comes from one of these IPs, Moltbot uses `x-forwarded-for` (or `x-real-ip`) to determine the client IP for local pairing checks and HTTP auth/local checks. -- Only list proxies you fully control, and ensure they **overwrite** incoming `x-forwarded-for`. +- `gateway.trustedProxies`: list of reverse proxy IPs or CIDR ranges that terminate TLS in front of the Gateway. +- Supports exact IPs (`127.0.0.1`), IPv4 CIDR (`188.114.96.0/20`), and IPv6 CIDR (`2400:cb00::/32`). +- When a connection comes from a trusted proxy, Moltbot uses `X-Forwarded-For` (or `X-Real-IP`) to determine the real client IP. +- For multi-proxy chains (e.g., Client to Cloudflare to nginx), Moltbot walks the `X-Forwarded-For` chain from right to left, skipping trusted proxies until it finds the real client. +- Only list proxies you fully control, and ensure they **overwrite** incoming `X-Forwarded-For`. Notes: - `moltbot gateway` refuses to start unless `gateway.mode` is set to `local` (or you pass the override flag). diff --git a/docs/gateway/security/index.md b/docs/gateway/security/index.md index e3c85af7f..1549589ee 100644 --- a/docs/gateway/security/index.md +++ b/docs/gateway/security/index.md @@ -87,16 +87,23 @@ If you run the Gateway behind a reverse proxy (nginx, Caddy, Traefik, etc.), you When the Gateway detects proxy headers (`X-Forwarded-For` or `X-Real-IP`) from an address that is **not** in `trustedProxies`, it will **not** treat connections as local clients. If gateway auth is disabled, those connections are rejected. This prevents authentication bypass where proxied connections would otherwise appear to come from localhost and receive automatic trust. +`trustedProxies` supports exact IPs and CIDR notation (both IPv4 and IPv6): + ```yaml gateway: trustedProxies: - - "127.0.0.1" # if your proxy runs on localhost + - "127.0.0.1" # exact IP (local proxy) + - "10.0.0.0/8" # IPv4 CIDR (private network) + - "188.114.96.0/20" # IPv4 CIDR (e.g., Cloudflare) + - "2400:cb00::/32" # IPv6 CIDR auth: mode: password password: ${CLAWDBOT_GATEWAY_PASSWORD} ``` -When `trustedProxies` is configured, the Gateway will use `X-Forwarded-For` headers to determine the real client IP for local client detection. Make sure your proxy overwrites (not appends to) incoming `X-Forwarded-For` headers to prevent spoofing. +**Multi-proxy chains:** When multiple proxies are involved (e.g., Client to Cloudflare to nginx to Gateway), Moltbot walks the `X-Forwarded-For` header from right to left, skipping trusted proxy IPs until it finds the first untrusted IP (the real client). This ensures correct client identification even through CDN and load balancer chains. + +Make sure your proxies overwrite (not append to) incoming `X-Forwarded-For` headers to prevent spoofing. ## Local session logs live on disk @@ -426,9 +433,10 @@ you terminate TLS or proxy in front of the gateway, disable `gateway.auth.allowTailscale` and use token/password auth instead. Trusted proxies: -- If you terminate TLS in front of the Gateway, set `gateway.trustedProxies` to your proxy IPs. -- Moltbot will trust `x-forwarded-for` (or `x-real-ip`) from those IPs to determine the client IP for local pairing checks and HTTP auth/local checks. -- Ensure your proxy **overwrites** `x-forwarded-for` and blocks direct access to the Gateway port. +- If you terminate TLS in front of the Gateway, set `gateway.trustedProxies` to your proxy IPs or CIDR ranges. +- Moltbot will trust `X-Forwarded-For` (or `X-Real-IP`) from those addresses to determine the client IP for local pairing checks and HTTP auth/local checks. +- For multi-proxy setups, include all proxy ranges (e.g., CDN + local proxy); Moltbot walks the chain to find the real client. +- Ensure your proxy **overwrites** `X-Forwarded-For` and blocks direct access to the Gateway port. See [Tailscale](/gateway/tailscale) and [Web overview](/web). diff --git a/package.json b/package.json index f91af8199..906abef68 100644 --- a/package.json +++ b/package.json @@ -186,6 +186,7 @@ "file-type": "^21.3.0", "grammy": "^1.39.3", "hono": "4.11.4", + "ip-address": "^10.0.1", "jiti": "^2.6.1", "json5": "^2.2.3", "jszip": "^3.10.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4f35c2612..4732f48ca 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -112,6 +112,9 @@ importers: hono: specifier: 4.11.4 version: 4.11.4 + ip-address: + specifier: ^10.0.1 + version: 10.1.0 jiti: specifier: ^2.6.1 version: 2.6.1 @@ -383,12 +386,12 @@ importers: '@microsoft/agents-hosting-extensions-teams': specifier: ^1.2.2 version: 1.2.2 - moltbot: - specifier: workspace:* - version: link:../.. express: specifier: ^5.2.1 version: 5.2.1 + moltbot: + specifier: workspace:* + version: link:../.. proper-lockfile: specifier: ^4.1.2 version: 4.1.2 @@ -3214,11 +3217,6 @@ packages: class-variance-authority@0.7.1: resolution: {integrity: sha512-Ka+9Trutv7G8M6WT6SeiRWz792K5qEqIGEGzXKhAE6xOWAY6pPH8U+9IY3oCMv6kqTmLsv7Xh/2w2RigkePMsg==} - clawdbot@2026.1.24-3: - resolution: {integrity: sha512-zt9BzhWXduq8ZZR4rfzQDurQWAgmijTTyPZCQGrn5ew6wCEwhxxEr2/NHG7IlCwcfRsKymsY4se9KMhoNz0JtQ==} - engines: {node: '>=22.12.0'} - hasBin: true - cli-cursor@5.0.0: resolution: {integrity: sha512-aCj4O5wKyszjMmDT4tZj93kxyydN/K5zPWSCe6/0AV/AA1pqe5ZBIw0a2ZfPQV7lL5/yb5HsUreJ6UFAF1tEQw==} engines: {node: '>=18'} @@ -3865,6 +3863,10 @@ packages: ini@1.3.8: resolution: {integrity: sha512-JV/yugV2uzW5iMRSiZAyDtQd+nxtUnjeLt0acNdw98kKLrvuRVyB80tsREOE7yvGVgalhZ6RNXCmEHkUKBKxew==} + ip-address@10.1.0: + resolution: {integrity: sha512-XXADHxXmvT9+CRxhXg56LJovE+bmWnEWB78LB83VZTprKTmaC5QfruXocxzTZ2Kl0DNwKuBdlIhjL8LeY8Sf8Q==} + engines: {node: '>= 12'} + ipaddr.js@1.9.1: resolution: {integrity: sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==} engines: {node: '>= 0.10'} @@ -9098,84 +9100,6 @@ snapshots: dependencies: clsx: 2.1.1 - clawdbot@2026.1.24-3(@types/express@5.0.6)(audio-decode@2.2.3)(devtools-protocol@0.0.1561482)(typescript@5.9.3): - dependencies: - '@agentclientprotocol/sdk': 0.13.1(zod@4.3.6) - '@aws-sdk/client-bedrock': 3.975.0 - '@buape/carbon': 0.14.0(hono@4.11.4) - '@clack/prompts': 0.11.0 - '@grammyjs/runner': 2.0.3(grammy@1.39.3) - '@grammyjs/transformer-throttler': 1.2.1(grammy@1.39.3) - '@homebridge/ciao': 1.3.4 - '@line/bot-sdk': 10.6.0 - '@lydell/node-pty': 1.2.0-beta.3 - '@mariozechner/pi-agent-core': 0.49.3(ws@8.19.0)(zod@4.3.6) - '@mariozechner/pi-ai': 0.49.3(ws@8.19.0)(zod@4.3.6) - '@mariozechner/pi-coding-agent': 0.49.3(ws@8.19.0)(zod@4.3.6) - '@mariozechner/pi-tui': 0.49.3 - '@mozilla/readability': 0.6.0 - '@sinclair/typebox': 0.34.47 - '@slack/bolt': 4.6.0(@types/express@5.0.6) - '@slack/web-api': 7.13.0 - '@whiskeysockets/baileys': 7.0.0-rc.9(audio-decode@2.2.3)(sharp@0.34.5) - ajv: 8.17.1 - body-parser: 2.2.2 - chalk: 5.6.2 - chokidar: 5.0.0 - chromium-bidi: 13.0.1(devtools-protocol@0.0.1561482) - cli-highlight: 2.1.11 - commander: 14.0.2 - croner: 9.1.0 - detect-libc: 2.1.2 - discord-api-types: 0.38.37 - dotenv: 17.2.3 - express: 5.2.1 - file-type: 21.3.0 - grammy: 1.39.3 - hono: 4.11.4 - jiti: 2.6.1 - json5: 2.2.3 - jszip: 3.10.1 - linkedom: 0.18.12 - long: 5.3.2 - markdown-it: 14.1.0 - node-edge-tts: 1.2.9 - osc-progress: 0.3.0 - pdfjs-dist: 5.4.530 - playwright-core: 1.58.0 - proper-lockfile: 4.1.2 - qrcode-terminal: 0.12.0 - sharp: 0.34.5 - sqlite-vec: 0.1.7-alpha.2 - tar: 7.5.4 - tslog: 4.10.2 - undici: 7.19.0 - ws: 8.19.0 - yaml: 2.8.2 - zod: 4.3.6 - optionalDependencies: - '@napi-rs/canvas': 0.1.88 - node-llama-cpp: 3.15.0(typescript@5.9.3) - transitivePeerDependencies: - - '@discordjs/opus' - - '@modelcontextprotocol/sdk' - - '@types/express' - - audio-decode - - aws-crt - - bufferutil - - canvas - - debug - - devtools-protocol - - encoding - - ffmpeg-static - - jimp - - link-preview-js - - node-opus - - opusscript - - supports-color - - typescript - - utf-8-validate - cli-cursor@5.0.0: dependencies: restore-cursor: 5.1.0 @@ -9940,6 +9864,8 @@ snapshots: ini@1.3.8: optional: true + ip-address@10.1.0: {} + ipaddr.js@1.9.1: {} ipull@3.9.3: diff --git a/src/gateway/net.test.ts b/src/gateway/net.test.ts index 46c426d63..ecf415d85 100644 --- a/src/gateway/net.test.ts +++ b/src/gateway/net.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { resolveGatewayListenHosts } from "./net.js"; +import { isTrustedProxyAddress, resolveGatewayClientIp, resolveGatewayListenHosts } from "./net.js"; describe("resolveGatewayListenHosts", () => { it("returns the input host when not loopback", async () => { @@ -26,3 +26,243 @@ describe("resolveGatewayListenHosts", () => { expect(hosts).toEqual(["127.0.0.1"]); }); }); + +describe("isTrustedProxyAddress", () => { + describe("exact IP matching", () => { + it("returns false for undefined IP", () => { + expect(isTrustedProxyAddress(undefined, ["1.2.3.4"])).toBe(false); + }); + + it("returns false for empty trusted proxies", () => { + expect(isTrustedProxyAddress("1.2.3.4", [])).toBe(false); + }); + + it("returns false for undefined trusted proxies", () => { + expect(isTrustedProxyAddress("1.2.3.4", undefined)).toBe(false); + }); + + it("matches exact IPv4 address", () => { + expect(isTrustedProxyAddress("192.168.1.1", ["192.168.1.1"])).toBe(true); + }); + + it("rejects non-matching IPv4 address", () => { + expect(isTrustedProxyAddress("192.168.1.2", ["192.168.1.1"])).toBe(false); + }); + + it("normalizes IPv4-mapped IPv6 addresses", () => { + expect(isTrustedProxyAddress("::ffff:192.168.1.1", ["192.168.1.1"])).toBe(true); + }); + }); + + describe("IPv4 CIDR matching", () => { + it("matches IP within /24 subnet", () => { + expect(isTrustedProxyAddress("192.168.1.50", ["192.168.1.0/24"])).toBe(true); + }); + + it("rejects IP outside /24 subnet", () => { + expect(isTrustedProxyAddress("192.168.2.50", ["192.168.1.0/24"])).toBe(false); + }); + + it("matches IP within /20 subnet", () => { + // 188.114.96.0/20 covers 188.114.96.0 - 188.114.111.255 + expect(isTrustedProxyAddress("188.114.96.1", ["188.114.96.0/20"])).toBe(true); + expect(isTrustedProxyAddress("188.114.111.255", ["188.114.96.0/20"])).toBe(true); + expect(isTrustedProxyAddress("188.114.100.50", ["188.114.96.0/20"])).toBe(true); + }); + + it("rejects IP outside /20 subnet", () => { + expect(isTrustedProxyAddress("188.114.112.1", ["188.114.96.0/20"])).toBe(false); + expect(isTrustedProxyAddress("188.114.95.255", ["188.114.96.0/20"])).toBe(false); + }); + + it("matches IP within /22 subnet", () => { + // 197.234.240.0/22 covers 197.234.240.0 - 197.234.243.255 + expect(isTrustedProxyAddress("197.234.240.1", ["197.234.240.0/22"])).toBe(true); + expect(isTrustedProxyAddress("197.234.243.255", ["197.234.240.0/22"])).toBe(true); + }); + + it("rejects IP outside /22 subnet", () => { + expect(isTrustedProxyAddress("197.234.244.0", ["197.234.240.0/22"])).toBe(false); + }); + + it("handles /32 as exact match", () => { + expect(isTrustedProxyAddress("10.0.0.1", ["10.0.0.1/32"])).toBe(true); + expect(isTrustedProxyAddress("10.0.0.2", ["10.0.0.1/32"])).toBe(false); + }); + + it("handles /0 as match all", () => { + expect(isTrustedProxyAddress("1.2.3.4", ["0.0.0.0/0"])).toBe(true); + }); + }); + + describe("IPv6 CIDR matching", () => { + it("matches IP within /64 subnet", () => { + expect(isTrustedProxyAddress("2001:db8::1", ["2001:db8::/64"])).toBe(true); + expect(isTrustedProxyAddress("2001:db8::ffff", ["2001:db8::/64"])).toBe(true); + }); + + it("rejects IP outside /64 subnet", () => { + expect(isTrustedProxyAddress("2001:db9::1", ["2001:db8::/64"])).toBe(false); + }); + + it("matches IP within /48 subnet", () => { + // /48 means first 48 bits (3 groups) must match + expect(isTrustedProxyAddress("2400:cb00:0:1::1", ["2400:cb00::/48"])).toBe(true); + expect(isTrustedProxyAddress("2400:cb00:0:ffff:eeee::1", ["2400:cb00::/48"])).toBe(true); + }); + + it("rejects IP outside /48 subnet", () => { + // Third group differs (0001 vs 0000) + expect(isTrustedProxyAddress("2400:cb00:1:2::1", ["2400:cb00::/48"])).toBe(false); + }); + + it("handles /128 as exact match", () => { + expect(isTrustedProxyAddress("::1", ["::1/128"])).toBe(true); + expect(isTrustedProxyAddress("::2", ["::1/128"])).toBe(false); + }); + }); + + describe("mixed trusted proxies list", () => { + it("matches against multiple CIDRs and exact IPs", () => { + const trustedProxies = ["188.114.96.0/20", "197.234.240.0/22", "10.0.0.1"]; + expect(isTrustedProxyAddress("188.114.100.50", trustedProxies)).toBe(true); + expect(isTrustedProxyAddress("197.234.241.1", trustedProxies)).toBe(true); + expect(isTrustedProxyAddress("10.0.0.1", trustedProxies)).toBe(true); + expect(isTrustedProxyAddress("1.2.3.4", trustedProxies)).toBe(false); + }); + }); +}); + +describe("resolveGatewayClientIp", () => { + describe("direct connection (no proxy)", () => { + it("returns remoteAddr when no trusted proxies configured", () => { + expect( + resolveGatewayClientIp({ + remoteAddr: "203.0.113.10", + forwardedFor: "192.168.1.1", + }), + ).toBe("203.0.113.10"); + }); + + it("returns remoteAddr when not from trusted proxy", () => { + expect( + resolveGatewayClientIp({ + remoteAddr: "203.0.113.10", + forwardedFor: "192.168.1.1", + trustedProxies: ["10.0.0.0/8"], + }), + ).toBe("203.0.113.10"); + }); + }); + + describe("single proxy", () => { + it("returns X-Forwarded-For client when direct connection is from trusted proxy", () => { + expect( + resolveGatewayClientIp({ + remoteAddr: "10.0.0.1", + forwardedFor: "203.0.113.10", + trustedProxies: ["10.0.0.0/8"], + }), + ).toBe("203.0.113.10"); + }); + + it("uses X-Real-IP as fallback", () => { + expect( + resolveGatewayClientIp({ + remoteAddr: "10.0.0.1", + realIp: "203.0.113.10", + trustedProxies: ["10.0.0.0/8"], + }), + ).toBe("203.0.113.10"); + }); + }); + + describe("multi-proxy chain (e.g., Client → Cloudflare → nginx)", () => { + // Cloudflare IPs: 188.114.96.0/20 + // Local nginx: 10.0.0.1 + const trustedProxies = ["188.114.96.0/20", "10.0.0.1"]; + + it("returns real client when all proxies are trusted", () => { + // Client → Cloudflare (188.114.100.1) → nginx (10.0.0.1) + // XFF: "203.0.113.10, 188.114.100.1" + expect( + resolveGatewayClientIp({ + remoteAddr: "10.0.0.1", + forwardedFor: "203.0.113.10, 188.114.100.1", + trustedProxies, + }), + ).toBe("203.0.113.10"); + }); + + it("stops at first untrusted proxy in chain", () => { + // Client → Unknown proxy (1.2.3.4) → Cloudflare → nginx + // XFF: "203.0.113.10, 1.2.3.4, 188.114.100.1" + // Should return 1.2.3.4 (first untrusted from right) + expect( + resolveGatewayClientIp({ + remoteAddr: "10.0.0.1", + forwardedFor: "203.0.113.10, 1.2.3.4, 188.114.100.1", + trustedProxies, + }), + ).toBe("1.2.3.4"); + }); + + it("handles single IP in X-Forwarded-For", () => { + expect( + resolveGatewayClientIp({ + remoteAddr: "10.0.0.1", + forwardedFor: "203.0.113.10", + trustedProxies, + }), + ).toBe("203.0.113.10"); + }); + + it("handles spaces in X-Forwarded-For", () => { + expect( + resolveGatewayClientIp({ + remoteAddr: "10.0.0.1", + forwardedFor: " 203.0.113.10 , 188.114.100.1 ", + trustedProxies, + }), + ).toBe("203.0.113.10"); + }); + + it("falls back to leftmost IP when all XFF entries are trusted", () => { + // All proxies trusted, return the leftmost (original client position) + expect( + resolveGatewayClientIp({ + remoteAddr: "10.0.0.1", + forwardedFor: "188.114.100.1, 188.114.100.2", + trustedProxies, + }), + ).toBe("188.114.100.1"); + }); + }); + + describe("CIDR-based proxy chain", () => { + // Example: trusting entire Cloudflare IPv4 ranges + const cloudflareRanges = [ + "173.245.48.0/20", + "103.21.244.0/22", + "103.22.200.0/22", + "103.31.4.0/22", + "141.101.64.0/18", + "108.162.192.0/18", + "190.93.240.0/20", + "188.114.96.0/20", + "197.234.240.0/22", + "198.41.128.0/17", + "127.0.0.0/8", // local proxy + ]; + + it("resolves client IP through Cloudflare proxy chain", () => { + expect( + resolveGatewayClientIp({ + remoteAddr: "127.0.0.1", + forwardedFor: "203.0.113.42, 188.114.100.50", + trustedProxies: cloudflareRanges, + }), + ).toBe("203.0.113.42"); + }); + }); +}); diff --git a/src/gateway/net.ts b/src/gateway/net.ts index 6702e0e8b..60308eb74 100644 --- a/src/gateway/net.ts +++ b/src/gateway/net.ts @@ -1,5 +1,7 @@ import net from "node:net"; +import { Address4, Address6 } from "ip-address"; + import { pickPrimaryTailnetIPv4, pickPrimaryTailnetIPv6 } from "../infra/tailnet.js"; export function isLoopbackAddress(ip: string | undefined): boolean { @@ -42,18 +44,78 @@ export function parseForwardedForClientIp(forwardedFor?: string): string | undef return normalizeIp(stripOptionalPort(raw)); } +/** + * Parse all IPs from X-Forwarded-For header. + * Returns array in original order: [client, proxy1, proxy2, ...] + */ +function parseForwardedForChain(forwardedFor?: string): string[] { + if (!forwardedFor) return []; + return forwardedFor + .split(",") + .map((ip) => normalizeIp(stripOptionalPort(ip.trim()))) + .filter((ip): ip is string => ip !== undefined); +} + function parseRealIp(realIp?: string): string | undefined { const raw = realIp?.trim(); if (!raw) return undefined; return normalizeIp(stripOptionalPort(raw)); } +/** + * Check if an IP address falls within a CIDR subnet. + * Supports both IPv4 (e.g., 188.114.96.0/20) and IPv6 (e.g., 2001:db8::/32). + * Uses the ip-address package for robust parsing and validation. + */ +function isIpInCidr(ip: string, cidr: string): boolean { + try { + // Detect IP version by presence of colon + const isIPv6 = ip.includes(":"); + const isCidrIPv6 = cidr.includes(":"); + + // IP and CIDR must be same version + if (isIPv6 !== isCidrIPv6) return false; + + if (isIPv6) { + const ipAddr = new Address6(ip); + const subnet = new Address6(cidr); + return ipAddr.isInSubnet(subnet); + } + + const ipAddr = new Address4(ip); + const subnet = new Address4(cidr); + return ipAddr.isInSubnet(subnet); + } catch { + // Invalid IP or CIDR format + return false; + } +} + export function isTrustedProxyAddress(ip: string | undefined, trustedProxies?: string[]): boolean { const normalized = normalizeIp(ip); if (!normalized || !trustedProxies || trustedProxies.length === 0) return false; - return trustedProxies.some((proxy) => normalizeIp(proxy) === normalized); + + return trustedProxies.some((proxy) => { + // Check for CIDR notation + if (proxy.includes("/")) { + return isIpInCidr(normalized, proxy); + } + // Exact IP match + return normalizeIp(proxy) === normalized; + }); } +/** + * Resolves the real client IP by walking the proxy chain. + * + * When the direct connection is from a trusted proxy, we examine the X-Forwarded-For + * header to find the real client. For multi-proxy setups (e.g., Client → Cloudflare → nginx → app), + * we walk the chain from right to left, skipping trusted proxies until we find the first + * untrusted IP (the real client). + * + * X-Forwarded-For format: "client, proxy1, proxy2" (leftmost is original client) + * We walk from right (most recent proxy) to left (original client), skipping trusted proxies. + */ export function resolveGatewayClientIp(params: { remoteAddr?: string; forwardedFor?: string; @@ -62,8 +124,27 @@ export function resolveGatewayClientIp(params: { }): string | undefined { const remote = normalizeIp(params.remoteAddr); if (!remote) return undefined; + + // If direct connection is not from a trusted proxy, that's our client if (!isTrustedProxyAddress(remote, params.trustedProxies)) return remote; - return parseForwardedForClientIp(params.forwardedFor) ?? parseRealIp(params.realIp) ?? remote; + + // Walk the X-Forwarded-For chain from right to left, skipping trusted proxies + const chain = parseForwardedForChain(params.forwardedFor); + for (let i = chain.length - 1; i >= 0; i--) { + const ip = chain[i]; + if (!isTrustedProxyAddress(ip, params.trustedProxies)) { + return ip; + } + } + + // Fall back to X-Real-IP if all XFF entries are trusted proxies + const realIp = parseRealIp(params.realIp); + if (realIp && !isTrustedProxyAddress(realIp, params.trustedProxies)) { + return realIp; + } + + // All proxies are trusted, return the leftmost (original) from chain or remote + return chain[0] ?? remote; } export function isLocalGatewayAddress(ip: string | undefined): boolean { diff --git a/src/gateway/server.auth.e2e.test.ts b/src/gateway/server.auth.e2e.test.ts index ee700ead6..76baaa1f8 100644 --- a/src/gateway/server.auth.e2e.test.ts +++ b/src/gateway/server.auth.e2e.test.ts @@ -566,5 +566,49 @@ describe("gateway server auth/connect", () => { } }); + test( + "trusts x-forwarded-for when proxy IP is within CIDR range", + { timeout: 60_000 }, + async () => { + testState.gatewayControlUi = { allowInsecureAuth: true }; + testState.gatewayAuth = { mode: "token", token: "secret" }; + const { writeConfigFile } = await import("../config/config.js"); + // Configure a CIDR range (127.0.0.0/8) that includes the proxy IP (127.0.0.1) + await writeConfigFile({ + gateway: { + trustedProxies: ["127.0.0.0/8"], + }, + } as Record); + const prevToken = process.env.CLAWDBOT_GATEWAY_TOKEN; + process.env.CLAWDBOT_GATEWAY_TOKEN = "secret"; + const port = await getFreePort(); + const server = await startGatewayServer(port); + // Connect through the trusted proxy (127.0.0.1 is within 127.0.0.0/8) + // with a forwarded client IP + const ws = new WebSocket(`ws://127.0.0.1:${port}`, { + headers: { "x-forwarded-for": "203.0.113.42" }, + }); + const challengePromise = onceMessage<{ payload?: unknown }>( + ws, + (o) => + (o as { type?: string }).type === "event" && + (o as { event?: string }).event === "connect.challenge", + ); + await new Promise((resolve) => ws.once("open", resolve)); + const challenge = await challengePromise; + const nonce = (challenge.payload as { nonce?: unknown } | undefined)?.nonce; + // A nonce is required when the resolved client IP is non-local + // Since x-forwarded-for is trusted (proxy in CIDR range), the client IP resolves to 203.0.113.42 + expect(typeof nonce).toBe("string"); + ws.close(); + await server.close(); + if (prevToken === undefined) { + delete process.env.CLAWDBOT_GATEWAY_TOKEN; + } else { + process.env.CLAWDBOT_GATEWAY_TOKEN = prevToken; + } + }, + ); + // Remaining tests require isolated gateway state. });