mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 13:22:14 +00:00
feat(audit): cf-connecting-ip branch in getClientIp behind TRUST_CF_CONNECTING_IP
The Coolify-fronted HealthLog stack sits behind Cloudflare, which strips and re-sets `cf-connecting-ip` on every request that lands on its edge. Without consulting this header, `getClientIp` reads the Caddy loopback IP off the `X-Forwarded-For` chain and the admin sign-in geo lookup ends up with a null IP for every visitor — Marc's "Standort" column rendered a literal "—" on every row of the sign-in overview even after v1.4.36 closed the missing-ASN fallback. The new branch prefers `cf-connecting-ip` ahead of the XFF / x-real-ip chain, but only when the operator opts in via `TRUST_CF_CONNECTING_IP=1`. A self-hosted deployment without Cloudflare in front MUST leave the flag off — an attacker can set the header directly on the public internet, and the downstream geo resolver would happily report a forged location. The trust-violation accounting in `getClientIpOrTrustWarning` skips when the CF header satisfies the lookup because Cloudflare re-sets the value on every edge request — there is no chain to violate. The existing XFF + x-real-ip semantics are unchanged for every deployment that leaves the flag unset. `.env.example` documents the flag in a dedicated "Reverse-proxy trust" block right above the process-split section so the operator finds it next to the existing infra-trust controls.
This commit is contained in:
1 parent
00e2515849
commit
a43f704690
3 files changed
+143
-3
No files matched your search
@@ -110,6 +110,21 @@ API_TOKEN_HMAC_KEY="REPLACE-WITH-OUTPUT-OF-openssl-rand-hex-32"
|
||||
# LOKI_PASSWORD=""
|
||||
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
# Reverse-proxy trust (optional, defaults match a single-proxy stack)
|
||||
# -----------------------------------------------------------------------------
|
||||
# When HealthLog sits behind Cloudflare (Tunnel, Workers, classic proxy),
|
||||
# the visitor IP arrives in the `CF-Connecting-IP` header — the upstream
|
||||
# `X-Forwarded-For` chain stops at the Coolify / Caddy loopback and the
|
||||
# admin sign-in geo lookup has no signal to resolve from.
|
||||
#
|
||||
# Set this to "1" ONLY when every public request goes through Cloudflare.
|
||||
# A deployment without Cloudflare in front MUST leave this off — an
|
||||
# attacker can set the header directly on the public internet and the geo
|
||||
# resolver would happily report a forged location.
|
||||
# TRUST_CF_CONNECTING_IP=""
|
||||
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
# Process split (optional, single-container deploys ignore this)
|
||||
# -----------------------------------------------------------------------------
|
||||
|
||||
@@ -5,16 +5,20 @@ import {
|
||||
_resetTrustViolationWarningForTests,
|
||||
} from "../api-response";
|
||||
|
||||
const ORIGINAL_ENV = process.env.TRUST_PROXY_HOPS;
|
||||
const ORIGINAL_HOPS = process.env.TRUST_PROXY_HOPS;
|
||||
const ORIGINAL_CF_FLAG = process.env.TRUST_CF_CONNECTING_IP;
|
||||
|
||||
beforeEach(() => {
|
||||
delete process.env.TRUST_PROXY_HOPS;
|
||||
delete process.env.TRUST_CF_CONNECTING_IP;
|
||||
_resetTrustViolationWarningForTests();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (ORIGINAL_ENV === undefined) delete process.env.TRUST_PROXY_HOPS;
|
||||
else process.env.TRUST_PROXY_HOPS = ORIGINAL_ENV;
|
||||
if (ORIGINAL_HOPS === undefined) delete process.env.TRUST_PROXY_HOPS;
|
||||
else process.env.TRUST_PROXY_HOPS = ORIGINAL_HOPS;
|
||||
if (ORIGINAL_CF_FLAG === undefined) delete process.env.TRUST_CF_CONNECTING_IP;
|
||||
else process.env.TRUST_CF_CONNECTING_IP = ORIGINAL_CF_FLAG;
|
||||
});
|
||||
|
||||
function makeRequest(headers: Record<string, string>): Request {
|
||||
@@ -139,6 +143,83 @@ describe("getClientIp trust-violation warning (F-6, 2026-05-16)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* v1.4.37 — Cloudflare's `cf-connecting-ip` carries the visitor IP
|
||||
* for every request that lands on its edge. The Coolify-fronted
|
||||
* HealthLog stack sits behind Cloudflare; without consulting this
|
||||
* header, the admin sign-in geo lookup had no signal to resolve from
|
||||
* because XFF stops at the Caddy loopback. The header is honoured
|
||||
* only under `TRUST_CF_CONNECTING_IP=1` so a self-hosted deployment
|
||||
* without Cloudflare in front cannot be tricked by an attacker
|
||||
* setting the header directly.
|
||||
*/
|
||||
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";
|
||||
const ip = getClientIp(
|
||||
makeRequest({
|
||||
"cf-connecting-ip": "203.0.113.42",
|
||||
"x-forwarded-for": "1.1.1.1, 192.0.2.7",
|
||||
}),
|
||||
);
|
||||
expect(ip).toBe("203.0.113.42");
|
||||
});
|
||||
|
||||
it("ignores cf-connecting-ip when the env flag is off (default)", () => {
|
||||
// Default — header set by an attacker on a deployment without
|
||||
// Cloudflare in front must NOT be trusted; the helper falls
|
||||
// through to the XFF / x-real-ip chain.
|
||||
const ip = getClientIp(
|
||||
makeRequest({
|
||||
"cf-connecting-ip": "203.0.113.42",
|
||||
"x-forwarded-for": "9.9.9.9, 5.6.7.8",
|
||||
}),
|
||||
);
|
||||
expect(ip).toBe("5.6.7.8");
|
||||
});
|
||||
|
||||
it("ignores cf-connecting-ip when the env flag is any value other than '1'", () => {
|
||||
process.env.TRUST_CF_CONNECTING_IP = "true";
|
||||
const ip = getClientIp(
|
||||
makeRequest({
|
||||
"cf-connecting-ip": "203.0.113.42",
|
||||
"x-forwarded-for": "9.9.9.9, 5.6.7.8",
|
||||
}),
|
||||
);
|
||||
expect(ip).toBe("5.6.7.8");
|
||||
});
|
||||
|
||||
it("falls back to the XFF chain when the env flag is on but cf-connecting-ip is missing", () => {
|
||||
process.env.TRUST_CF_CONNECTING_IP = "1";
|
||||
const ip = getClientIp(
|
||||
makeRequest({ "x-forwarded-for": "1.1.1.1, 5.6.7.8" }),
|
||||
);
|
||||
expect(ip).toBe("5.6.7.8");
|
||||
});
|
||||
|
||||
it("rejects a malformed cf-connecting-ip when the env flag is on", () => {
|
||||
process.env.TRUST_CF_CONNECTING_IP = "1";
|
||||
const ip = getClientIp(
|
||||
makeRequest({
|
||||
"cf-connecting-ip": "<<not-an-ip>>",
|
||||
"x-forwarded-for": "1.1.1.1, 5.6.7.8",
|
||||
}),
|
||||
);
|
||||
expect(ip).toBe("5.6.7.8");
|
||||
});
|
||||
|
||||
it("getClientIpOrTrustWarning also prefers cf-connecting-ip when the flag is on", () => {
|
||||
process.env.TRUST_CF_CONNECTING_IP = "1";
|
||||
const result = getClientIpOrTrustWarning(
|
||||
makeRequest({
|
||||
"cf-connecting-ip": "203.0.113.42",
|
||||
"x-forwarded-for": "1.1.1.1",
|
||||
}),
|
||||
);
|
||||
expect(result).toEqual({ ip: "203.0.113.42", trustViolation: false });
|
||||
});
|
||||
});
|
||||
|
||||
describe("getClientIpOrTrustWarning shape (F-6, 2026-05-16)", () => {
|
||||
it("returns trustViolation=false on a well-formed chain", () => {
|
||||
const result = getClientIpOrTrustWarning(
|
||||
|
||||
@@ -75,12 +75,41 @@ export async function safeJson<T = unknown>(
|
||||
* observed when the request arrived.
|
||||
* - "N" (>1) → trust N hops; read the Nth-from-rightmost XFF entry.
|
||||
*
|
||||
* 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
|
||||
* request that lands on its edge, so it carries the real visitor IP
|
||||
* even when XFF/x-real-ip end up as the Coolify proxy's loopback
|
||||
* address. The flag is OFF by default: a self-hosted deployment
|
||||
* without Cloudflare in front would otherwise trust an attacker-set
|
||||
* header on the public internet.
|
||||
*
|
||||
* Returns the resolved IP or null when no trusted source is available.
|
||||
*/
|
||||
function looksLikeIp(s: string): boolean {
|
||||
return /^[0-9a-fA-F.:]+$/.test(s) && s.length >= 3 && s.length <= 45;
|
||||
}
|
||||
|
||||
/**
|
||||
* v1.4.37 — Cloudflare puts the visitor IP into `cf-connecting-ip` on
|
||||
* every request hitting the edge. The Coolify-fronted HealthLog stack
|
||||
* sits behind Cloudflare; without consulting this header, every
|
||||
* `getClientIp` caller landed with the Caddy loopback IP and the geo
|
||||
* resolver had no signal to backfill the admin sign-in overview from.
|
||||
*
|
||||
* The header is honoured only when `TRUST_CF_CONNECTING_IP=1`. A
|
||||
* self-hosted deployment without Cloudflare in front must NOT trust
|
||||
* the header — any attacker can set it on a direct request and the
|
||||
* downstream geo resolver would happily report a forged location.
|
||||
*/
|
||||
function readCfConnectingIp(request: Request): string | null {
|
||||
if (process.env.TRUST_CF_CONNECTING_IP !== "1") return null;
|
||||
const candidate = request.headers.get("cf-connecting-ip");
|
||||
if (!candidate) return null;
|
||||
const trimmed = candidate.trim();
|
||||
return looksLikeIp(trimmed) ? trimmed : null;
|
||||
}
|
||||
|
||||
function parseTrustProxyHops(raw: string | undefined): number {
|
||||
if (raw === undefined) return 1;
|
||||
const trimmed = raw.trim();
|
||||
@@ -124,6 +153,13 @@ function warnTrustViolationOnce(hops: number, chainLength: number): void {
|
||||
}
|
||||
|
||||
export function getClientIp(request: Request): string | null {
|
||||
// v1.4.37 — Cloudflare's `cf-connecting-ip` takes precedence when
|
||||
// the env flag opts in. The header carries the visitor IP the edge
|
||||
// observed; without consulting it first, every request behind
|
||||
// Cloudflare lands with the Caddy loopback as the resolved IP.
|
||||
const cfIp = readCfConnectingIp(request);
|
||||
if (cfIp) return cfIp;
|
||||
|
||||
const hops = parseTrustProxyHops(process.env.TRUST_PROXY_HOPS);
|
||||
|
||||
if (hops > 0) {
|
||||
@@ -167,6 +203,14 @@ export function getClientIpOrTrustWarning(request: Request): {
|
||||
ip: string | null;
|
||||
trustViolation: boolean;
|
||||
} {
|
||||
// v1.4.37 — same CF preference as `getClientIp`. The Cloudflare
|
||||
// header is the operator-controlled signal; trusting it bypasses
|
||||
// the XFF trust-violation accounting entirely because Cloudflare's
|
||||
// edge re-sets the value on every request, so there is no chain to
|
||||
// violate.
|
||||
const cfIp = readCfConnectingIp(request);
|
||||
if (cfIp) return { ip: cfIp, trustViolation: false };
|
||||
|
||||
const hops = parseTrustProxyHops(process.env.TRUST_PROXY_HOPS);
|
||||
|
||||
if (hops > 0) {
|
||||
|
||||
Reference in new issue
Block a user