fix: derive client IP from trusted proxy hops only

.env.example

@@ -64,6 +64,20 @@ BCRYPT_COST_FACTOR=12
 # -----------------------------------------------------------------------------
 # Proxy / Gateway
 # -----------------------------------------------------------------------------
+# Number of trusted reverse-proxy hops in front of the app, or a comma-separated
+# list of trusted proxy CIDRs. Controls how the client IP is derived from the
+# X-Forwarded-For header (Express "trust proxy" semantics).
+#
+#   TRUST_PROXY_HOPS=0   (default) — ignore X-Forwarded-For entirely and use the
+#                                    socket address. Safe when the app is directly
+#                                    exposed to clients.
+#   TRUST_PROXY_HOPS=1   — one trusted proxy; the client IP is taken one entry
+#                          from the right of X-Forwarded-For (the value appended
+#                          by that proxy). Leftmost entries are client-controlled
+#                          and are never trusted.
+#   TRUST_PROXY_HOPS=2   — two trusted proxies (e.g. CDN + load balancer), etc.
+#
+# TRUST_PROXY_HOPS=0
 UPSTREAM_URL=http://localhost:4000
 PROXY_TIMEOUT_MS=30000
 # REST API rate limiting (per-user with IP fallback for unauthenticated requests)

FORWARDED_HEADER_POLICY.md

@@ -40,7 +40,7 @@ All other headers not in the strip list are forwarded to upstream services, incl
 - `content-length` - Length of the request body
 - `accept` - Preferred response media types
 - `user-agent` - Client software identification
-- `accept-encoding` - Preferred content encodings
+- `accept-encoding` - Preferred response encodings
 - `accept-language` - Preferred response languages
 - Custom application headers (e.g., `x-custom-*`)
 
@@ -75,6 +75,22 @@ Header stripping is performed case-insensitively. All header name variations (e.
 - Request IDs are included in error responses for debugging
 - UUID v4 format ensures global uniqueness
 
+## Client IP Trust Boundary
+
+When the service sits behind one or more reverse proxies, client IP resolution follows Express's `trust proxy` semantics:
+
+- **No trust (default)**: all forwarded headers are ignored and the direct socket address is used. This is spoof-proof.
+- **Hop count**: the client address is taken N entries from the right of the forwarded chain. With one trusted hop, `X-Forwarded-For: 1.1.1.1, 2.2.2.2` yields `2.2.2.2`. The leftmost entry is fully client-controlled and must not be trusted.
+- **Trust all** (`true`):? legacy behaviour that trusts every hop. Only use this when every proxy in the chain is controlled by the operator.
+
+### Configuration
+
+- `TRUST_PROXY_HEADERS=true`: trust all hops (legacy).
+- `TRUST_PROXY_HOPS=N<number>`: trust the last N hops. Takes precedence over `TRUST_PROXY_HEADERS` when set to a positive integer.
+- Unset: no trust; the socket address is used.
+
+The IP-allowlist middleware and the request logger both call the same `helper in `src/lib/clientIp.ts`, so the trust boundary is applied consistently across the stack.
+
 ## Implementation Details
 
 The header policy is implemented in `src/routes/proxyRoutes.ts`:
@@ -92,6 +108,7 @@ const DEFAULT_STRIP_HEADERS = [
   'proxy-authorization',
   'proxy-connection',
 ];
+Labels: `x-forwarded-for` and `x-real-ip` are also stripped before forwarding to upstream services.
 ```
 
 Headers are processed case-insensitively using lowercase comparison:
@@ -113,5 +130,6 @@ Comprehensive tests verify:
 - Case-insensitive header stripping works
 - Response headers are filtered appropriately
 - Request ID correlation is maintained
+- Client IP resolution honours the trusted hop count and falls back to the socket address
 
-See `src/__tests__/proxy.integration.test.ts` for detailed test coverage.
+See `src/__tests__/proxy.integration.test.ts` and `src/lib/__tests__/clientIp.test.ts` for detailed test coverage.

src/config/env.ts

@@ -79,6 +79,30 @@ export const envSchema = z
     JWT_SECRET: z.string().min(1, "JWT_SECRET is required"),
     ADMIN_API_KEY: z.string().min(1, "ADMIN_API_KEY is required"),
     METRICS_API_KEY: z.string().min(1, "METRICS_API_KEY is required"),
+    /**
+     * TRUST_PROXY_HOPS — number of trusted reverse-proxy hops in front of the
+     * application. When greater than zero, the client IP is derived from the
+     * X-Forwarded-For header by selecting the entry that many positions from
+     * the right (matching Express `trust proxy` semantics). When zero (the
+     * default), the socket address is used and X-Forwarded-For is ignored.
+     *
+     * Example: TRUST_PROXY_HOPS=1 with "X-Forwarded-For: 1.1.1.1, 2.2.2.2"
+     * yields 2.2.2.2 (the rightmost entry, i.e. the address appended by the
+     * single trusted proxy). The leftmost entry is fully client-controlled
+     * and must never be trusted.
+     */
+    TRUST_PROXY_HOPS: z.coerce.number().int().min(0).default(0),
+    /**
+     * TRUST_PROXY_HEADERS — legacy boolean flag. Retained for backwards
+     * compatibility: when true and TRUST_PROXY_HOPS is unset/zero, it is
+     * treated as a single trusted hop. New deployments should prefer
+     * TRUST_PROXY_HOPS.
+     */
+    TRUST_PROXY_HEADERS: z
+      .string()
+      .optional()
+      .transform((v) => v === "true")
+      .default(false),
     TRUST_FORWARDED_USER_ID: z
       .string()
       .optional()

src/lib/__tests__/clientIp.test.ts

@@ -45,7 +45,36 @@ describe('getClientIp', () => {
     assert.equal(getClientIp(req, false), '1.2.3.4');
   });
 
-  test('uses x-forwarded-for leftmost IP when trustProxy is true', () => {
+  test('defaults to socket address when trustProxy is omitted', () => {
+    const req = makeReq({
+      headers: { 'x-forwarded-for': '9.9.9.9' },
+      socket: { remoteAddress: '1.2.3.4' } as never,
+    });
+    assert.equal(getClientIp(req), '1.2.3.4');
+  });
+
+  test('with one trusted hop, selects the rightmost forwarded entry', () => {
+    const req = makeReq({
+      headers: { 'x-forwarded-for': '1.1.1.1, 2.2.2.2' },
+    });
+    assert.equal(getClientIp(req, 1), '2.2.2.2');
+  });
+
+  test('with two trusted hops, selects the entry two from the right', () => {
+    const req = makeReq({
+      headers: { 'x-forwarded-for': '1.1.1.1, 2.2.2.2, 3.3.3.3' },
+    });
+    assert.equal(getClientIp(req, 2), '2.2.2.2');
+  });
+
+  test('spoofed leftmost entries cannot influence the result', () => {
+    const req = makeReq({
+      headers: { 'x-forwarded-for': '10.0.0.1, 10.0.0.2, 2.2.2.2' },
+    });
+    assert.equal(getClientIp(req, 1), '2.2.2.2');
+  });
+
+  test('trustProxy true trusts all hops and uses leftmost entry', () => {
     const req = makeReq({
       headers: { 'x-forwarded-for': '5.5.5.5, 10.0.0.1, 172.16.0.1' },
     });
@@ -57,7 +86,22 @@ describe('getClientIp', () => {
       headers: { 'x-forwarded-for': 'not-an-ip' },
       socket: { remoteAddress: '1.2.3.4' } as never,
     });
-    assert.equal(getClientIp(req, true), '1.2.3.4');
+    assert.equal(getClientIp(req, 1), '1.2.3.4');
+  });
+
+  test('falls back to socket when hop count exceeds chain length and leftmost is invalid', () => {
+    const req = makeReq({
+      headers: { 'x-forwarded-for': 'not-an-ip, 2.2.2.2' },
+      socket: { remoteAddress: '1.2.3.4' } as never,
+    });
+    assert.equal(getClientIp(req, 5), '1.2.3.4');
+  });
+
+  test('uses leftmost entry when hop count exceeds chain length', () => {
+    const req = makeReq({
+      headers: { 'x-forwarded-for': '2.2.2.2, 3.3.3.3' },
+    });
+    assert.equal(getClientIp(req, 5), '2.2.2.2');
   });
 
   test('falls back to req.ip when socket is absent', () => {
@@ -75,16 +119,16 @@ describe('getClientIp', () => {
     const reqBoth = makeReq({
       headers: { 'x-forwarded-for': '5.5.5.5', 'x-real-ip': '6.6.6.6' },
     });
-    assert.equal(getClientIp(reqBoth, true), '5.5.5.5');
+    assert.equal(getClientIp(reqBoth, 1), '5.5.5.5');
 
     // Only x-real-ip present
     const reqReal = makeReq({ headers: { 'x-real-ip': '6.6.6.6' } });
-    assert.equal(getClientIp(reqReal, true), '6.6.6.6');
+    assert.equal(getClientIp(reqReal, 1), '6.6.6.6');
   });
 
   test('accepts custom proxy header list', () => {
     const req = makeReq({ headers: { 'x-custom-ip': '7.7.7.7' } });
-    assert.equal(getClientIp(req, true, ['x-custom-ip']), '7.7.7.7');
+    assert.equal(getClientIp(req, 1, ['x-custom-ip']), '7.7.7.7');
   });
 
   test('DEFAULT_PROXY_HEADERS includes x-forwarded-for', () => {

src/lib/clientIp.ts

@@ -1,5 +1,7 @@
 import type { Request } from 'express';
 
+export type TrustProxyOption = boolean | number;
+
 /**
  * Proxy headers checked when trustProxy is enabled, ordered by reliability.
  * The same list is used by the IP-allowlist middleware and the request logger
@@ -18,40 +20,71 @@ export const DEFAULT_PROXY_HEADERS = [
 /** Returns true for a plausible IPv4 or IPv6 address string. */
 export function isValidIp(ip: string): boolean {
   const ipv4 = /^(\d{1,3}\.){3}\d{1,3}$/;
-  const ipv6 = /^([0-9a-fA-F]{0,4}:){2,7}[0-9a-fA-F]{0,4}$/;
+  const ipv6 = /^([0-9a-fA-F]{0,4}:){2}{2,7}[0-9a-fA-F]{0,4}$/;
   return ipv4.test(ip) || ipv6.test(ip) || ip.includes(':');
 }
 
 /**
  * Extracts the real client IP from an Express request.
  *
- * When `trustProxy` is false (the default) the direct socket address is
- * returned, making IP spoofing via headers impossible.
+ * Trust semantics follow Express' `trust proxy` model:
+ * - `false` (default): all forwarded headers are ignored and the direct
+ *   socket address is returned, making header spoofing impossible.
+ * - `true`: trust all hops (equivalent to a hop count of `Infinity`).
+ * - `number N >= 1`: trust the last N hops. The client address is taken
+ *   N entries from the right of the forwarded chain. With one trusted hop,
+ *   `'1.1.1.1, 2.2.2.2'` yields `2.2.2.2`. This prevents a client from
+ *   spoofing the leftmost entry to bypass the admin IP-allowlist or per-IP
+ *   rate limits.
  *
- * When `trustProxy` is true the proxy headers listed in `proxyHeaders` are
- * consulted in order; the first valid IP wins.  For `x-forwarded-for` only
- * the leftmost entry is used because that is the original client address —
- * subsequent entries are added by intermediary proxies and must not be trusted
- * as the client origin.
+ * Because the client address is selected from the right of the chain,
+ * spoofed leftmost entries cannot influence the result as long as the
+ * configured hop count matches the actual number of trusted proxies.
  *
  * @param req          Express request object
- * @param trustProxy   Whether to honour proxy forwarding headers
+ * @param trustProxy   False (no trust), true (trust all), or a hop count >= 1
  * @param proxyHeaders Ordered list of headers to inspect (defaults to {@link DEFAULT_PROXY_HEADERS})
  */
 export function getClientIp(
   req: Request,
-  trustProxy = false,
+  trustProxy: TrustProxyOption = false,
   proxyHeaders: readonly string[] = DEFAULT_PROXY_HEADERS,
 ): string {
-  if (trustProxy) {
+  const hops = normalizeTrustProxy(trustProxy);
+
+  if (hops > 0) {
     for (const header of proxyHeaders) {
       const value = req.headers[header.toLowerCase()];
-      if (typeof value === 'string' && value.trim()) {
-        const firstIp = value.split(',')[0].trim();
-        if (isValidIp(firstIp)) return firstIp;
-      }
+      if (typeof value !== 'string' || !value.trim()) continue;
+
+      const entries = value
+        .split(',')
+        .map((entry) => entry.trim())
+        .filter((entry) => entry.length > 0);
+
+      if (entries.length === 0) continue;
+
+      // Select the entry `hops` positions from the right. When the chain is
+      // shorter than the configured hop count, the leftmost entry is the
+      // best available candidate.
+      const index = Math.max(0, entries.length - hops);
+      const candidate = entries[index];
+      if (candidate && isValidIp(candidate)) return candidate;
     }
   }
 
   return req.ip ?? req.socket?.remoteAddress ?? '';
 }
+
+/**
+ * Normalises the `trustProxy` option into a non-negative hop count.
+ * `false` -> 0, `true` -> Infinity, and any number >= 1 -> that number.
+ */
+function normalizeTrustProxy(trustProxy: TrustProxyOption): number {
+  if (trustProxy === true) return Number.POSITIVE_INFINITY;
+  if (trustProxy === false) return 0;
+  if (typeof trustProxy === 'number' && Number.isFinite(trustProxy) && trustProxy >= 1) {
+    return Math.floor(trustProxy);
+  }
+  return 0;
+}