fix(http): read forwarding headers only from a connection a proxy could make

The client-IP resolver trusted one forwarding hop by default and read the rightmost X-Forwarded-For entry. Next fills that header with the socket's address only when the request carries none, so a caller reaching the published port directly could send its own and choose a fresh address per request: every per-IP limit (sign-in, second factor, passkey, share unlock, webhooks) became per-request, and sessions, the audit log and sign-in notices recorded the chosen address. The server now writes each connection's own peer address into an internal header from Node's http.server.request.start diagnostics channel, installed at boot and overwriting anything a caller sent under that name. The resolver reads forwarding headers only when that peer is loopback, private, unique-local or CGNAT, where a reverse proxy sits; a public peer is its own client address, and a log line says once that its forwarding headers were ignored. With TRUST_PROXY_HOPS=0 the peer is used instead of no address, so a directly exposed host no longer shares one bucket across all anonymous callers. Proxies on the same host, Docker network or tailnet need no change.
2026-11-07 04:12:04 +00:00 · 2026-10-25 13:52:04 +01:00
1 parent ba22932101
commit d6d0f7e73f
@@ -525,8 +525,10 @@ API_TOKEN_HMAC_KEY="<openssl rand -hex 32>"
 # it unset, or an attacker could forge the header. Off by default.
 # Number of trusted reverse-proxy hops in front of the app; the client IP is
 # read that many entries from the right of the X-Forwarded-For chain. 0 for a
-# directly-exposed host, 1 (the default when unset) behind a single reverse
-# proxy; X-Real-IP is read only at 1. A wrong value makes the app refuse the
+# directly-exposed host (the connection's own address is used), 1 (the
+# default when unset) behind a single reverse proxy; X-Real-IP is read only
+# at 1. Forwarding headers are never read from a connection that comes from a
+# public address: only a proxy on loopback or a private network is believed. A wrong value makes the app refuse the
 # headers, log one warning, and collapse anonymous callers into one
 # rate-limit bucket.
 # TRUST_PROXY_HOPS="1"
@@ -40,22 +40,35 @@ chain from the **right**, counting back `TRUST_PROXY_HOPS` entries.
 This guards against a client rotating `X-Forwarded-For` per request
 to defeat IP-based rate limits.

-| Topology                                                  | `TRUST_PROXY_HOPS`                    |
-| --------------------------------------------------------- | ------------------------------------- |
-| App is internet-facing with no proxy                      | `0` (no forwarding header is trusted) |
-| Single proxy in front (Caddy / Traefik / Nginx / Coolify) | `1` (default)                         |
-| Cloudflare → your proxy → app                             | `2`                                   |
-| Cloudflare → Coolify-Tunnel → Coolify → app               | `3`                                   |
+| Topology                                                  | `TRUST_PROXY_HOPS`                 |
+| --------------------------------------------------------- | ---------------------------------- |
+| App is internet-facing with no proxy                      | `0` (the connection's own address) |
+| Single proxy in front (Caddy / Traefik / Nginx / Coolify) | `1` (default)                      |
+| Cloudflare → your proxy → app                             | `2`                                |
+| Cloudflare → Coolify-Tunnel → Coolify → app               | `3`                                |

 A misconfigured count logs a one-shot warning to stderr and collapses
 every anonymous caller into one shared rate-limit bucket. Match the
 value to the actual hop count.

+Forwarding headers are believed only from a connection that could come
+from a proxy: loopback, a private or unique-local network, or CGNAT (a
+Tailscale address). A request that reaches HealthLog straight from a public
+address carries headers its sender wrote, so HealthLog ignores them and
+uses the connection's own address, and says so once in the log. A proxy on
+the same host or Docker network connects from a private address, so the
+usual setups above need nothing. If your proxy reaches HealthLog over a
+public address, put it on a private network with HealthLog instead (or, for
+Cloudflare straight to the origin, use `TRUST_CF_CONNECTING_IP=1`). The log
+line also shows up when the published port is open to the internet beside
+the proxy; bind it to the proxy's network or to `127.0.0.1` then.
+
 `X-Real-IP` gets the same trust as `X-Forwarded-For`. It is read only
 when `TRUST_PROXY_HOPS` is exactly `1` and the request carries no
 `X-Forwarded-For`, which covers a single proxy that sets only
 `X-Real-IP`. With `0` neither header is believed, because without a proxy
-in front both come from the caller. With `2` or more, the hop count
+in front both come from the caller; the address the connection came from
+is used instead. With `2` or more, the hop count
 describes an `X-Forwarded-For` chain, so have your proxies send one.

 If requests arrive with a forwarding header the setting does not let
@@ -18,6 +18,9 @@ async function registerOfflineGeoNoticeAtBoot(): Promise<void> {
 export async function register() {
  // Only start the worker on the Node.js server runtime (not Edge, not build)
  if (process.env.NEXT_RUNTIME === "nodejs") {
+    // First: every request then carries its socket's address (peer-address).
+    (await import("@/lib/http/peer-address")).installPeerAddressStamp();
+
    // The event-loop monitor comes first, so the loading of the database
    // client and of the worker graph is inside its view, and every stall it
    // reports names the boot step that was running (`boot-steps.ts`).
@@ -1,4 +1,12 @@
-import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
+import {
+  describe,
+  it,
+  expect,
+  beforeEach,
+  afterEach,
+  onTestFinished,
+  vi,
+} from "vitest";
 import {
  getClientIp,
  getClientIpOrTrustWarning,
@@ -363,3 +371,70 @@ describe("getClientIpOrTrustWarning shape (F-6, 2026-05-16)", () => {
    }
  });
 });
+
+/**
+ * A caller that reaches the server directly used to choose its own address:
+ * one trusted hop is the default, and the rightmost X-Forwarded-For entry is
+ * whatever the caller wrote when no proxy appended to it. Every per-IP limit
+ * became per-request. The server now stamps each request with its
+ * connection's peer (`@/lib/http/peer-address`), and forwarding headers are
+ * read only from a peer that could be a proxy.
+ */
+describe("getClientIp against the connection's own peer", () => {
+  const stamped = (peer: string, headers: Record<string, string> = {}) =>
+    makeRequest({ ...headers, "x-healthlog-peer-address": peer });
+
+  it("ignores forwarding headers from a public peer, however they rotate", () => {
+    const ips = ["203.0.113.1", "203.0.113.2", "203.0.113.3"].map((forged) =>
+      getClientIp(
+        stamped("198.51.100.7", {
+          "x-forwarded-for": forged,
+          "x-real-ip": forged,
+        }),
+      ),
+    );
+    expect(ips).toEqual(["198.51.100.7", "198.51.100.7", "198.51.100.7"]);
+  });
+
+  it("reads forwarding headers from a proxy on a private network or loopback", () => {
+    for (const proxy of ["172.18.0.2", "127.0.0.1", "::1", "100.101.102.103"]) {
+      expect(
+        getClientIp(stamped(proxy, { "x-forwarded-for": "203.0.113.5" })),
+      ).toBe("203.0.113.5");
+    }
+    expect(
+      getClientIp(stamped("10.0.0.2", { "x-real-ip": "203.0.113.6" })),
+    ).toBe("203.0.113.6");
+  });
+
+  it("reads an IPv4 peer that the socket reports in IPv6 form", () => {
+    expect(
+      getClientIp(
+        stamped("::ffff:198.51.100.7", { "x-forwarded-for": "203.0.113.1" }),
+      ),
+    ).toBe("198.51.100.7");
+  });
+
+  it("uses the peer itself with no proxy declared, instead of nothing", () => {
+    process.env.TRUST_PROXY_HOPS = "0";
+    expect(
+      getClientIp(
+        stamped("198.51.100.7", { "x-forwarded-for": "203.0.113.1" }),
+      ),
+    ).toBe("198.51.100.7");
+  });
+
+  it("trusts no forwarding header on a request the server did not stamp, once it stamps", async () => {
+    const { installPeerAddressStamp, __uninstallPeerAddressStampForTests } =
+      await import("../http/peer-address");
+    installPeerAddressStamp();
+    onTestFinished(__uninstallPeerAddressStampForTests);
+    expect(
+      getClientIp(makeRequest({ "x-forwarded-for": "203.0.113.1" })),
+    ).toBeNull();
+    // A stamp that is not an address (the socket was already gone).
+    expect(
+      getClientIp(stamped("", { "x-forwarded-for": "203.0.113.1" })),
+    ).toBeNull();
+  });
+});
@@ -23,7 +23,7 @@ import { ConsentRequiredError } from "./ai/consent-guard";
 import { attachRecordContextEcho } from "./sharing/record-session-fence";
 import { refuseOnKeyMismatch } from "./boot/key-mismatch-refusal";
 import { acceptIncomingRequestId } from "./http/request-id";
-import { withErrorEnvelopeDefaults } from "./api-response";
+import { getClientIp, withErrorEnvelopeDefaults } from "./api-response";
 import {
  capturedRateLimit,
  rateLimitResponseHeaders,
@@ -190,14 +190,9 @@ export function apiHandler<T extends (...args: any[]) => Promise<Response>>(
      user_agent:
        safeRequestProp(request, (r) => r.headers.get("user-agent"), null) ??
        undefined,
-      ip:
-        safeRequestProp(
-          request,
-          (r) => r.headers.get("x-forwarded-for")?.split(",")[0]?.trim(),
-          null,
-        ) ||
-        safeRequestProp(request, (r) => r.headers.get("x-real-ip"), null) ||
-        undefined,
+      // The resolved client address, not the raw leftmost forwarding entry
+      // a direct caller could have written itself.
+      ip: safeRequestProp(request, (r) => getClientIp(r), null) ?? undefined,
    });

    return runWithRateLimitCapture(() =>
@@ -3,6 +3,11 @@ import { NextResponse } from "next/server";
 import type { ZodError, ZodIssue } from "zod/v4";

 import { defaultErrorCodeForStatus } from "@/lib/api/default-error-codes";
+import {
+  peerAddressStampInstalled,
+  readPeerAddress,
+} from "@/lib/http/peer-address";
+import { isPublicIp } from "@/lib/validations/notifications";

 export function apiSuccess<T>(data: T, status = 200) {
  return NextResponse.json({ data, error: null }, { status });
@@ -485,13 +490,22 @@ export async function safeJson<T = unknown>(
 * Trust model (`TRUST_PROXY_HOPS` env):
 *   - "0"          → trust no forwarding header at all, XFF or x-real-ip
 *                    (use this if HealthLog is internet-facing without a
- *                    reverse proxy you control); the IP resolves to null
+ *                    reverse proxy you control); the IP is the connection's
+ *                    own peer address
 *   - "1" (default)→ trust exactly one hop (typical Coolify / Caddy /
 *                    Cloudflare-Tunnel single-proxy deployment); read the
 *                    rightmost XFF entry which is the IP your proxy
 *                    observed when the request arrived.
 *   - "N" (>1)     → trust N hops; read the Nth-from-rightmost XFF entry.
 *
+ * Forwarding headers are read only from a connection whose peer could be a
+ * proxy: loopback, a private or unique-local network, CGNAT (Tailscale). A
+ * connection from a public address reached the server directly, so whatever
+ * forwarding headers it sends are its own, and its own address is the client
+ * IP. The peer comes from `@/lib/http/peer-address`. Without that check, the
+ * default of one trusted hop let any direct caller name a fresh address per
+ * request and step around every per-IP limit.
+ *
 * Cloudflare opt-in (`TRUST_CF_CONNECTING_IP=1`): when the env flag is
 * set, the helper prefers the `cf-connecting-ip` header before walking
 * the XFF chain. Cloudflare strips and re-sets this header on every
@@ -610,6 +624,29 @@ function warnUnreadProxyHeadersOnce(hops: number, header: string): void {
  );
 }

+/**
+ * Once per process: a connection from a public address carried forwarding
+ * headers, which are ignored. Either HealthLog is reachable without its
+ * proxy (the published port is open to the internet), or the proxy in front
+ * connects from a public address and needs `TRUST_CF_CONNECTING_IP` or a
+ * private network between it and HealthLog.
+ */
+let publicPeerForwardingWarned = false;
+
+/** Test-only reset for {@link warnPublicPeerForwardingOnce}. */
+export function _resetPublicPeerForwardingWarningForTests(): void {
+  publicPeerForwardingWarned = false;
+}
+
+function warnPublicPeerForwardingOnce(): void {
+  if (publicPeerForwardingWarned) return;
+  publicPeerForwardingWarned = true;
+  console.warn(
+    "[getClientIp] a connection from a public address sent X-Forwarded-For or X-Real-IP; those headers are ignored and the connection's own address is used. " +
+      "If HealthLog is meant to be reached only through a reverse proxy, the published port is open beyond it.",
+  );
+}
+
 /**
 * Tagged return shape so a caller can apply a tighter universal
 * rate-limit when the trust chain is misconfigured. F-6 (mobile security
@@ -640,6 +677,30 @@ export function getClientIpOrTrustWarning(request: Request): {

  const hops = parseTrustProxyHops(process.env.TRUST_PROXY_HOPS);

+  // The connection's own peer, stamped by the server for every request it
+  // receives. `undefined` only for a request the server did not receive (a
+  // suite's hand-built request) — or, once the stamp is installed, one that
+  // arrived before it was, whose forwarding headers nothing vouches for.
+  const peer = readPeerAddress(request);
+  if (peer === null || (peer === undefined && peerAddressStampInstalled())) {
+    return { ip: null, trustViolation: false };
+  }
+  if (peer !== undefined) {
+    // No proxy declared: the peer is the client.
+    if (hops === 0) return { ip: peer, trustViolation: false };
+    // A peer on a public address reached the server directly; its
+    // forwarding headers are its own choice, not a proxy's report.
+    if (isPublicIp(peer)) {
+      if (
+        request.headers.has("x-real-ip") ||
+        request.headers.get("x-forwarded-for")?.trim() !== peer
+      ) {
+        warnPublicPeerForwardingOnce();
+      }
+      return { ip: peer, trustViolation: false };
+    }
+  }
+
  if (hops > 0) {
    const forwarded = request.headers.get("x-forwarded-for");
    if (forwarded) {
@@ -0,0 +1,68 @@
+import { request as httpRequest, createServer } from "node:http";
+import type { AddressInfo } from "node:net";
+
+import { describe, expect, it, onTestFinished } from "vitest";
+
+import {
+  __uninstallPeerAddressStampForTests,
+  installPeerAddressStamp,
+  PEER_ADDRESS_HEADER,
+  readPeerAddress,
+} from "../peer-address";
+
+/**
+ * The stamp is what the client-IP resolver trusts, so a caller must not be
+ * able to set it: the server writes the socket's address over whatever the
+ * request carried under that name.
+ */
+describe("installPeerAddressStamp", () => {
+  it("writes the socket's address over a header the caller sent", async () => {
+    installPeerAddressStamp();
+    installPeerAddressStamp(); // once per process, however often it is called
+    onTestFinished(__uninstallPeerAddressStampForTests);
+
+    const seen: Array<string | string[] | undefined> = [];
+    const server = createServer((req, res) => {
+      seen.push(req.headers[PEER_ADDRESS_HEADER]);
+      res.end("ok");
+    });
+    await new Promise<void>((resolve) =>
+      server.listen(0, "127.0.0.1", resolve),
+    );
+    const { port } = server.address() as AddressInfo;
+    try {
+      await new Promise<void>((resolve, reject) => {
+        const req = httpRequest(
+          {
+            host: "127.0.0.1",
+            port,
+            path: "/",
+            headers: { [PEER_ADDRESS_HEADER]: "203.0.113.9" },
+          },
+          (res) => {
+            res.resume();
+            res.on("end", resolve);
+          },
+        );
+        req.on("error", reject);
+        req.end();
+      });
+    } finally {
+      await new Promise((resolve) => server.close(resolve));
+    }
+    expect(seen).toEqual(["127.0.0.1"]);
+  });
+
+  it("reads the stamp back, normalised", () => {
+    const read = (value?: string) =>
+      readPeerAddress(
+        new Request("http://x/", {
+          headers: value === undefined ? {} : { [PEER_ADDRESS_HEADER]: value },
+        }),
+      );
+    expect(read()).toBeUndefined();
+    expect(read("::ffff:10.0.0.1")).toBe("10.0.0.1");
+    expect(read("2001:db8::1")).toBe("2001:db8::1");
+    expect(read("not-an-ip")).toBeNull();
+  });
+});
@@ -0,0 +1,72 @@
+/**
+ * The address a request's TCP connection came from, as a header the route
+ * handlers can read.
+ *
+ * Next does not hand a route the socket. It does fill `X-Forwarded-For` with
+ * the socket's address, but only when the request carries none, so a caller
+ * that reaches the server directly and sends its own `X-Forwarded-For`
+ * chooses the address every per-IP limit, the audit log and the sign-in
+ * notices see. To tell a forwarded request from a forged one, the client-IP
+ * resolver (`getClientIpOrTrustWarning` in `@/lib/api-response`) needs the
+ * connection's own peer, and this provides it.
+ *
+ * `installPeerAddressStamp` subscribes to Node's `http.server.request.start`
+ * diagnostics channel, which fires for every request before the server's
+ * request handler runs, and writes the socket's remote address into
+ * `PEER_ADDRESS_HEADER`, overwriting whatever the caller sent under that
+ * name. It is installed once, at boot, from `src/instrumentation.ts`.
+ */
+import { subscribe, unsubscribe } from "node:diagnostics_channel";
+import type { IncomingMessage } from "node:http";
+import { isIP } from "node:net";
+import type { Socket } from "node:net";
+
+/** The header the socket's address is written to. Never trusted from a caller. */
+export const PEER_ADDRESS_HEADER = "x-healthlog-peer-address";
+
+const INSTALLED = Symbol.for("healthlog.peerAddressStamp");
+
+type StampGlobal = typeof globalThis & { [INSTALLED]?: boolean };
+
+const CHANNEL = "http.server.request.start";
+
+function stamp(message: unknown): void {
+  const { request, socket } = message as {
+    request: IncomingMessage;
+    socket: Socket;
+  };
+  request.headers[PEER_ADDRESS_HEADER] = socket.remoteAddress ?? "";
+}
+
+/** Start writing each request's socket address into `PEER_ADDRESS_HEADER`. */
+export function installPeerAddressStamp(): void {
+  const g = globalThis as StampGlobal;
+  if (g[INSTALLED]) return;
+  subscribe(CHANNEL, stamp);
+  g[INSTALLED] = true;
+}
+
+/** Test hook: a suite that installs the stamp takes it out again. */
+export function __uninstallPeerAddressStampForTests(): void {
+  unsubscribe(CHANNEL, stamp);
+  (globalThis as StampGlobal)[INSTALLED] = false;
+}
+
+/** Whether this process stamps requests (it does once booted). */
+export function peerAddressStampInstalled(): boolean {
+  return (globalThis as StampGlobal)[INSTALLED] === true;
+}
+
+/**
+ * The connection's peer, normalised (an IPv4-mapped IPv6 address reads as
+ * IPv4). `undefined` when the request carries no stamp, `null` when it
+ * carries one that is not an address (a socket already gone).
+ */
+export function readPeerAddress(request: Request): string | null | undefined {
+  const raw = request.headers.get(PEER_ADDRESS_HEADER);
+  if (raw === null) return undefined;
+  const trimmed = raw.trim();
+  const mapped = /^::ffff:(\d+\.\d+\.\d+\.\d+)$/i.exec(trimmed);
+  const address = mapped ? mapped[1]! : trimmed;
+  return isIP(address) !== 0 ? address : null;
+}