feat: add reverse proxy support via trusted IP range config#1105
feat: add reverse proxy support via trusted IP range config#1105Simon2122 wants to merge 1 commit into
Conversation
Add support for running txAdmin behind a reverse proxy by configuring trusted proxy IP ranges. When a request comes from a trusted proxy IP, the real client IP is resolved from the X-Forwarded-For header. Configuration via convar: set txAdminProxyIpRange '10.0.0.0/8,172.16.0.0/12' Or via environment variable: TXHOST_PROXY_IP_RANGE=10.0.0.0/8,172.16.0.0/12 Changes: - New CIDR range parsing/matching utility (isIpInRanges) - New proxy IP resolver that walks XFF right-to-left - Updated ctxVarsMw, WebSocket, and rate limiter to use resolved IP - Added PROXY_IP_RANGE to host env var schemas - Added txAdminProxyIpRange convar support - Unit tests for CIDR matching
There was a problem hiding this comment.
Pull request overview
Adds reverse-proxy awareness to txAdmin by introducing a trusted-proxy IP range configuration and resolving the real client IP from X-Forwarded-For, then using that resolved IP for rate limiting and WebSocket/session-related checks.
Changes:
- Introduces IPv4 CIDR parsing/matching utilities (
parseIpRanges,isIpInRanges) plus schema validation for a proxy IP range config. - Adds
resolveProxyRealIpto derive the effective client IP fromX-Forwarded-Forwhen the connecting socket IP is a trusted proxy. - Wires the resolved IP into Koa ctx vars, WebSocket auth checks, and the global HTTP rate limiter; adds the new env var/convar plumbing and unit tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| core/modules/WebServer/webSocket.ts | Uses resolved client IP (via XFF + trusted proxy ranges) for WebSocket auth/IP checks. |
| core/modules/WebServer/middlewares/ctxVarsMw.ts | Sets ctx.txVars.realIP based on proxy-resolved IP instead of ctx.ip. |
| core/modules/WebServer/index.ts | Uses proxy-resolved IP for global HTTP rate limiting. |
| core/lib/host/resolveProxyRealIp.ts | New utility to resolve real client IP from XFF when the socket IP is a trusted proxy. |
| core/lib/host/isIpInRanges.ts | New CIDR parsing/matching utility plus Zod schema for proxy IP range config. |
| core/lib/host/isIpInRanges.test.ts | Adds unit tests for CIDR matching utility. |
| core/globalData.ts | Adds parsing/loading of proxy IP ranges into txHostConfig. |
| core/boot/getNativeVars.ts | Adds txAdminProxyIpRange convar support. |
| core/boot/getHostVars.ts | Adds TXHOST_PROXY_IP_RANGE env var schema support. |
| const xff = req?.headers?.['x-forwarded-for']; | ||
| const rateLimitIp = resolveProxyRealIp(socketIp, xff) ?? socketIp; |
There was a problem hiding this comment.
resolveProxyRealIp is typed to accept (socketIp: string, xffHeader: string | undefined), but here socketIp can be undefined and req.headers['x-forwarded-for'] can be string | string[] | undefined in Node. With strict: true this should fail type-checking; also it’s inconsistent with the other call sites that normalize XFF. Consider normalizing XFF to a single string and only calling the resolver when socketIp is a string (or widen the resolver signature to accept string | undefined / string | string[] | undefined).
| const xff = req?.headers?.['x-forwarded-for']; | |
| const rateLimitIp = resolveProxyRealIp(socketIp, xff) ?? socketIp; | |
| const rawXff = req?.headers?.['x-forwarded-for']; | |
| const xff = typeof rawXff === 'string' | |
| ? rawXff | |
| : Array.isArray(rawXff) | |
| ? rawXff.join(', ') | |
| : undefined; | |
| const rateLimitIp = typeof socketIp === 'string' | |
| ? resolveProxyRealIp(socketIp, xff) ?? socketIp | |
| : socketIp; |
| export const resolveProxyRealIp = (socketIp: string, xffHeader: string | undefined) => { | ||
| const { proxyIpRanges } = txHostConfig; | ||
| if (!proxyIpRanges || !isIpInRanges(socketIp, proxyIpRanges)) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
socketIp may not be a valid IPv4 string (e.g. 'unknown', IPv6, or IPv4-mapped IPv6 like ::ffff:10.0.0.1). Because isIpInRanges currently coerces invalid input into a number, a non-IP can be treated as 0.0.0.0 and accidentally match trusted ranges (especially 0.0.0.0/0), enabling client-IP spoofing via X-Forwarded-For. Add strict validation/normalization of socketIp before isIpInRanges (and ideally inside isIpInRanges as well).
| const getIP = (socket: SocketWithSession) => { | ||
| return socket?.request?.socket?.remoteAddress ?? 'unknown'; | ||
| const socketIp = socket?.request?.socket?.remoteAddress ?? 'unknown'; | ||
| const xff = socket?.request?.headers?.['x-forwarded-for']; | ||
| const xffStr = Array.isArray(xff) ? xff[0] : xff; | ||
| return resolveProxyRealIp(socketIp, xffStr) ?? socketIp; |
There was a problem hiding this comment.
getIP can produce 'unknown' when remoteAddress is missing, but that value is passed into resolveProxyRealIp. With the current isIpInRanges implementation, invalid IP strings can be coerced to 0.0.0.0 and mistakenly considered trusted, allowing X-Forwarded-For spoofing. Avoid calling the proxy resolver unless socketIp is a validated/normalized IP (or pass undefined when unavailable).
| const ipToNumber = (ip: string) => { | ||
| const parts = ip.split('.'); | ||
| return ( | ||
| (parseInt(parts[0]) << 24) | ||
| | (parseInt(parts[1]) << 16) | ||
| | (parseInt(parts[2]) << 8) | ||
| | parseInt(parts[3]) | ||
| ) >>> 0; | ||
| }; |
There was a problem hiding this comment.
ipToNumber assumes a dotted-quad IPv4 string and does no validation (parts length, numeric octets, 0–255). For invalid inputs, the bitwise operations coerce NaN to 0, which can cause incorrect range matches and undermines the trusted-proxy security boundary. Make this parsing reject/return a sentinel for invalid IPs and have isIpInRanges return false for invalid inputs.
| const parseCidr = (cidr: string) => { | ||
| const [ip, prefixStr] = cidr.split('/'); | ||
| const prefix = prefixStr !== undefined ? parseInt(prefixStr) : 32; | ||
| const mask = prefix === 0 ? 0 : (~0 << (32 - prefix)) >>> 0; | ||
| const network = ipToNumber(ip) & mask; | ||
| return { network, mask }; | ||
| }; |
There was a problem hiding this comment.
parseCidr doesn’t validate the prefix length; values >32 or NaN will produce incorrect masks due to JS shift semantics. Since these ranges gate trusted-proxy behavior, validate that the prefix is an integer in [0, 32] (and that the IP portion is valid) before computing the mask/network.
| const cidrRegex = /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}(\/\d{1,2})?$/; | ||
| export const proxyIpRangeSchema = z.string().transform((val: string) => { | ||
| return val.split(',').map((s: string) => s.trim()).filter((s: string) => s.length > 0); | ||
| }).pipe( | ||
| z.array(z.string().regex(cidrRegex, 'Each entry must be a valid IPv4 address or CIDR range (e.g. 10.0.0.0/8)')) | ||
| .min(1, 'At least one IP or CIDR range is required') |
There was a problem hiding this comment.
proxyIpRangeSchema currently accepts values like 999.999.999.999/99 (octets and prefix aren’t bounded). Those will be parsed into unintended networks/masks and could silently expand the trusted-proxy set. Tighten validation (e.g. zod IPv4 validation + explicit prefix 0–32) so invalid configs fail fast with a clear message.
| suite('parseIpRanges + isIpInRanges', () => { | ||
| it('should match a single IP (/32)', () => { | ||
| const ranges = parseIpRanges(['10.0.0.1']); | ||
| expect(isIpInRanges('10.0.0.1', ranges)).toBe(true); | ||
| expect(isIpInRanges('10.0.0.2', ranges)).toBe(false); | ||
| }); | ||
|
|
||
| it('should match a /24 range', () => { | ||
| const ranges = parseIpRanges(['192.168.1.0/24']); | ||
| expect(isIpInRanges('192.168.1.0', ranges)).toBe(true); | ||
| expect(isIpInRanges('192.168.1.255', ranges)).toBe(true); | ||
| expect(isIpInRanges('192.168.1.100', ranges)).toBe(true); |
There was a problem hiding this comment.
The new CIDR matcher is security-sensitive (trusted proxy boundary) but the tests only cover happy-path ranges. Add coverage for invalid inputs (bad octets, prefix >32/negative, empty segments) and for IPv4-mapped IPv6 strings (e.g. ::ffff:10.0.0.1) to ensure the matcher/resolver fails closed instead of matching unexpectedly.
|
This would be great! |
Add support for running txAdmin behind a reverse proxy by configuring trusted proxy IP ranges. When a request comes from a trusted proxy IP, the real client IP is resolved from the X-Forwarded-For header.
Configuration via convar:
set txAdminProxyIpRange '10.0.0.0/8,172.16.0.0/12'
Or via environment variable:
TXHOST_PROXY_IP_RANGE=10.0.0.0/8,172.16.0.0/12
Changes: