diff --git a/.env.example b/.env.example index 800b3f353..10bc7095e 100644 --- a/.env.example +++ b/.env.example @@ -202,10 +202,11 @@ API_TOKEN_HMAC_KEY="REPLACE-WITH-OUTPUT-OF-openssl-rand-hex-32" # operator — there is no way for the server to detect whether the flag # matches the real deployment topology. When in doubt, leave it off. # 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. Leave -# unset for a directly-exposed host; set to 1 behind a single reverse proxy. -# A wrong value makes the app refuse to read XFF and share one rate-limit -# bucket across all anonymous callers. +# read that many entries from the right of the X-Forwarded-For chain. Set 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 headers, log one warning, and share one rate-limit bucket across +# all anonymous callers. # TRUST_PROXY_HOPS="1" # TRUST_CF_CONNECTING_IP="" diff --git a/.env.production.example b/.env.production.example index 9e9a2a949..af58314bd 100644 --- a/.env.production.example +++ b/.env.production.example @@ -473,9 +473,10 @@ API_TOKEN_HMAC_KEY="" # ONLY when Cloudflare is in front -- a direct-exposed self-host must leave # 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. Unset -# for a directly-exposed host, 1 behind a single reverse proxy. A wrong -# value makes the app refuse XFF and collapse anonymous callers into one +# 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 +# headers, log one warning, and collapse anonymous callers into one # rate-limit bucket. # TRUST_PROXY_HOPS="1" # TRUST_CF_CONNECTING_IP="1" diff --git a/docs/self-hosting/reverse-proxy.md b/docs/self-hosting/reverse-proxy.md index 6ef02203b..5ce2db75f 100644 --- a/docs/self-hosting/reverse-proxy.md +++ b/docs/self-hosting/reverse-proxy.md @@ -52,11 +52,17 @@ every anonymous caller into one shared rate-limit bucket. Match the value to the actual hop count. `X-Real-IP` gets the same trust as `X-Forwarded-For`. It is read only -when `TRUST_PROXY_HOPS` is `1` or more and the request carries no -`X-Forwarded-For`, which covers a proxy that sets only `X-Real-IP`. With -`0` neither header is believed, because without a proxy in front both -come from the caller. If your proxy sets only `X-Real-IP` and you had -set `0` for it, set `1` instead. +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 +describes an `X-Forwarded-For` chain, so have your proxies send one. + +If requests arrive with a forwarding header the setting does not let +HealthLog read, the log shows one warning naming the header and the +setting. Every anonymous caller then shares one rate-limit bucket, so one +person mistyping a password can hold sign-in back for everyone. The usual +cause is `0` behind a proxy that sends only `X-Real-IP`: set `1`. ## Caddy diff --git a/src/__tests__/proxy-early-exit-headers.test.ts b/src/__tests__/proxy-early-exit-headers.test.ts index 9095f4105..bfa02f8c5 100644 --- a/src/__tests__/proxy-early-exit-headers.test.ts +++ b/src/__tests__/proxy-early-exit-headers.test.ts @@ -23,6 +23,7 @@ vi.mock("@/lib/process-type", () => ({ })); import { proxy } from "../proxy"; +import { applyBypassRouteHeaders } from "@/lib/http/proxy-bypass-routes"; function makeRequest( pathname: string, @@ -60,6 +61,10 @@ function expectBaselineHeaders(res: Response) { ); expect(res.headers.get("Permissions-Policy")).toContain("camera=()"); expect(res.headers.get("Cross-Origin-Opener-Policy")).toBe("same-origin"); + expect(res.headers.get("Cross-Origin-Resource-Policy")).toBe("same-origin"); + expect(res.headers.get("Cross-Origin-Embedder-Policy")).toBe( + "credentialless", + ); expect(res.headers.get("X-Permitted-Cross-Domain-Policies")).toBe("none"); expect(res.headers.get("Strict-Transport-Security")).toContain( "max-age=31536000", @@ -117,3 +122,22 @@ describe("proxy early exits carry the baseline security headers", () => { expect(res.headers.get("X-Frame-Options")).toBe("DENY"); }); }); + +describe("the routes that bypass the proxy", () => { + it("carry the same cross-origin headers the proxy sets on a full response", () => { + const viaProxy = proxy(makeRequest("/api/health")); + const headers = new Headers(); + applyBypassRouteHeaders(headers); + for (const name of [ + "Cross-Origin-Opener-Policy", + "Cross-Origin-Resource-Policy", + "Cross-Origin-Embedder-Policy", + "X-Content-Type-Options", + "X-Frame-Options", + "X-Permitted-Cross-Domain-Policies", + "Strict-Transport-Security", + ]) { + expect(headers.get(name), name).toBe(viaProxy.headers.get(name)); + } + }); +}); diff --git a/src/lib/__tests__/get-client-ip.test.ts b/src/lib/__tests__/get-client-ip.test.ts index 56b082589..56d4aec47 100644 --- a/src/lib/__tests__/get-client-ip.test.ts +++ b/src/lib/__tests__/get-client-ip.test.ts @@ -3,6 +3,7 @@ import { getClientIp, getClientIpOrTrustWarning, _resetTrustViolationWarningForTests, + _resetUnreadProxyHeadersWarningForTests, } from "../api-response"; const ORIGINAL_HOPS = process.env.TRUST_PROXY_HOPS; @@ -12,6 +13,7 @@ beforeEach(() => { delete process.env.TRUST_PROXY_HOPS; delete process.env.TRUST_CF_CONNECTING_IP; _resetTrustViolationWarningForTests(); + _resetUnreadProxyHeadersWarningForTests(); }); afterEach(() => { @@ -202,6 +204,49 @@ describe("getClientIp trust-violation warning (F-6, 2026-05-16)", () => { * without Cloudflare in front cannot be tricked by an attacker * setting the header directly. */ +describe("X-Real-IP with two or more hops, and headers the trust cannot read", () => { + it("does not read X-Real-IP when two hops are declared and XFF is missing", () => { + process.env.TRUST_PROXY_HOPS = "2"; + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + // A caller behind the outer proxy could have written it. + expect(getClientIp(makeRequest({ "x-real-ip": "9.9.9.9" }))).toBeNull(); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toMatch(/X-Real-IP/); + } finally { + warn.mockRestore(); + } + }); + + it("warns once when TRUST_PROXY_HOPS=0 and a proxy sends only X-Real-IP", () => { + process.env.TRUST_PROXY_HOPS = "0"; + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + for (let i = 0; i < 3; i++) { + expect( + getClientIp(makeRequest({ "x-real-ip": `10.0.0.${i}` })), + ).toBeNull(); + } + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toMatch(/TRUST_PROXY_HOPS=0/); + expect(warn.mock.calls[0][0]).toMatch(/TRUST_PROXY_HOPS=1/); + } finally { + warn.mockRestore(); + } + }); + + it("stays quiet with TRUST_PROXY_HOPS=0 and no forwarding header", () => { + process.env.TRUST_PROXY_HOPS = "0"; + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + expect(getClientIp(makeRequest({}))).toBeNull(); + expect(warn).not.toHaveBeenCalled(); + } finally { + warn.mockRestore(); + } + }); +}); + describe("getClientIp Cloudflare cf-connecting-ip branch (v1.4.37)", () => { it("returns cf-connecting-ip when the env flag is on and the header is present", () => { process.env.TRUST_CF_CONNECTING_IP = "1"; diff --git a/src/lib/api-response.ts b/src/lib/api-response.ts index adcbf113c..c2403c1dd 100644 --- a/src/lib/api-response.ts +++ b/src/lib/api-response.ts @@ -490,6 +490,33 @@ function warnTrustViolationOnce(hops: number, chainLength: number): void { ); } +/** + * Once per process: forwarding headers arrived that the configured trust + * does not let us read, so the client address resolves to nothing. The + * common cause is `TRUST_PROXY_HOPS=0` behind a proxy that sends only + * `X-Real-IP` (nginx's usual setup), or `TRUST_PROXY_HOPS` of 2 or more with + * no `X-Forwarded-For`. Every anonymous caller then shares one rate-limit + * bucket, so one person mistyping a password can hold the sign-in back for + * everybody, and the operator had no way to see why. + */ +let unreadProxyHeadersWarned = false; + +/** Test-only reset for {@link warnUnreadProxyHeadersOnce}. */ +export function _resetUnreadProxyHeadersWarningForTests(): void { + unreadProxyHeadersWarned = false; +} + +function warnUnreadProxyHeadersOnce(hops: number, header: string): void { + if (unreadProxyHeadersWarned) return; + unreadProxyHeadersWarned = true; + console.warn( + `[getClientIp] requests carry ${header} but TRUST_PROXY_HOPS=${hops} does not allow reading it, so client addresses resolve to nothing and every anonymous caller shares one rate-limit bucket. ` + + (hops === 0 + ? "If a proxy you control sits in front of HealthLog, set TRUST_PROXY_HOPS=1." + : "X-Real-IP is only read with TRUST_PROXY_HOPS=1; with more hops, have the proxies send X-Forwarded-For."), + ); +} + /** * Tagged return shape so a caller can apply a tighter universal * rate-limit when the trust chain is misconfigured. F-6 (mobile security @@ -546,17 +573,32 @@ export function getClientIpOrTrustWarning(request: Request): { return { ip: null, trustViolation: true }; } // x-real-ip is a forwarding header like XFF and gets the same trust: only - // when the operator declares a proxy in front (hops > 0), and only as the + // when the operator declares a proxy in front, and only as the // single-proxy stand-in for a missing XFF. It used to be read on every // path, including `TRUST_PROXY_HOPS=0` ("no proxy, trust no header") and // the broken-chain path above, so a caller sending a fresh x-real-ip per // request got a fresh per-IP rate-limit bucket each time. + // + // Exactly one hop, not "one or more". X-Real-IP holds a single address, + // set by whichever proxy wrote it last; with two or more hops declared, + // the outer proxy may pass through whatever the caller put there, and + // the chain the hop count describes is the XFF chain, which is absent. const realIp = request.headers.get("x-real-ip")?.trim(); - return { - ip: realIp && looksLikeIp(realIp) ? realIp : null, - trustViolation: false, - }; + if (hops === 1) { + return { + ip: realIp && looksLikeIp(realIp) ? realIp : null, + trustViolation: false, + }; + } + if (realIp) warnUnreadProxyHeadersOnce(hops, "X-Real-IP"); + return { ip: null, trustViolation: false }; } + const unread = request.headers.has("x-forwarded-for") + ? "X-Forwarded-For" + : request.headers.has("x-real-ip") + ? "X-Real-IP" + : null; + if (unread) warnUnreadProxyHeadersOnce(hops, unread); return { ip: null, trustViolation: false }; } diff --git a/src/lib/http/proxy-bypass-routes.ts b/src/lib/http/proxy-bypass-routes.ts index 83e0e1466..d5aca39c9 100644 --- a/src/lib/http/proxy-bypass-routes.ts +++ b/src/lib/http/proxy-bypass-routes.ts @@ -70,6 +70,12 @@ export function setBaselineSecurityHeaders(headers: Headers): void { headers.set("Referrer-Policy", "strict-origin-when-cross-origin"); headers.set("Permissions-Policy", "camera=(), microphone=(), geolocation=()"); headers.set("Cross-Origin-Opener-Policy", "same-origin"); + // The same pair the proxy sets on every page and API response. The bypass + // routes (uploads, imports, native batches) answered without them, so a + // cross-origin page could embed one of their JSON answers as a no-cors + // subresource where every other route refuses it. + headers.set("Cross-Origin-Resource-Policy", "same-origin"); + headers.set("Cross-Origin-Embedder-Policy", "credentialless"); headers.set("X-Permitted-Cross-Domain-Policies", "none"); if (process.env.NODE_ENV !== "development") { headers.set(