From 8d73d9721fc5e176918a30718e2d8a8aa4a198f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 20 Jun 2026 19:00:27 +0200 Subject: [PATCH] fix(geo): default to ip-api.com and surface non-ok lookup failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ipwho.is default rejected server-side egress with a 403 on the free plan, which the resolver swallowed as a clean miss, so the admin login/audit overview rendered "—" for every session. Switch the default online provider to ip-api.com (the parser already accepts its status/countryCode shape) and surface a non-ok HTTP status (403/429/5xx) as a wide-event warning instead of returning null silently, so a future provider rejection is visible. Whitelist TRUST_CF_CONNECTING_IP, IP_GEO_LOOKUP_URL and IP_GEO_LOOKUP_DISABLED in the compose environment block and document them in .env.production.example so a Cloudflare-fronted deployment can store the real visitor IP rather than the edge address. --- .env.production.example | 28 +++++++++++++++ docker-compose.yml | 11 ++++++ src/lib/__tests__/geo.test.ts | 67 +++++++++++++++++++++++++++++++++-- src/lib/geo.ts | 38 ++++++++++++++++---- 4 files changed, 135 insertions(+), 9 deletions(-) diff --git a/.env.production.example b/.env.production.example index f2a0396d5..c07d9e292 100644 --- a/.env.production.example +++ b/.env.production.example @@ -187,3 +187,31 @@ API_TOKEN_HMAC_KEY="" # $ NEXT_PUBLIC_DASHBOARD_SNAPSHOT=false pnpm build # or a Dockerfile ARG # # NEXT_PUBLIC_DASHBOARD_SNAPSHOT="false" + + +# ----------------------------------------------------------------------------- +# Client-IP resolution -- optional (required behind Cloudflare) +# ----------------------------------------------------------------------------- +# When the stack sits behind Cloudflare, the real visitor IP arrives in the +# `cf-connecting-ip` header; the X-Forwarded-For chain otherwise yields the +# Cloudflare edge IP, so the audit-log overview stores the CF datacentre +# address and the geo lookup resolves the wrong location. Set this to "1" +# 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. +# TRUST_CF_CONNECTING_IP="1" + +# ----------------------------------------------------------------------------- +# IP geolocation lookup -- optional +# ----------------------------------------------------------------------------- +# The admin login/audit overview enriches auth events with a coarse +# "City, CC" location + carrier. The default online provider is ip-api.com. +# Override the base URL to point at a different provider; the response must +# match either the ipwho.is shape (`success` + `country_code`) or the +# ip-api.com shape (`status` + `countryCode`). HTTPS only -- a non-HTTPS URL +# is refused (audit-event IPs must never egress over plaintext HTTP). The +# `/` segment is appended automatically. +# IP_GEO_LOOKUP_URL="https://ip-api.com/json" +# +# Set to "1" to disable all third-party IP egress (GDPR Art. 44). With egress +# off the resolver uses only the offline GeoLite2 MMDB tier, if present. +# IP_GEO_LOOKUP_DISABLED="1" diff --git a/docker-compose.yml b/docker-compose.yml index f3c518c9c..8a22bb5b8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -52,6 +52,17 @@ services: # the container, even though docker-compose reads .env for # `${VAR}` substitution at compose-up time). SESSION_COOKIE_SECURE: "${SESSION_COOKIE_SECURE:-}" + # v1.18.11 (W3) — client-IP resolution + geo lookup. Behind Cloudflare + # the real visitor IP is in `cf-connecting-ip`; set + # TRUST_CF_CONNECTING_IP=1 ONLY when Cloudflare is in front (off by + # default — a direct-exposed host must not trust a forgeable header). + # IP_GEO_LOOKUP_URL overrides the default ip-api.com provider (HTTPS + # only); IP_GEO_LOOKUP_DISABLED=1 stops all third-party IP egress. + # Listed here so operator values in .env reach the container (the + # `environment:` block is a whitelist — vars not listed never propagate). + TRUST_CF_CONNECTING_IP: "${TRUST_CF_CONNECTING_IP:-}" + IP_GEO_LOOKUP_URL: "${IP_GEO_LOOKUP_URL:-}" + IP_GEO_LOOKUP_DISABLED: "${IP_GEO_LOOKUP_DISABLED:-}" # all = bundle web + worker in this container (default, single-host) # web = serve HTTP only; pair with a separate app-worker service HEALTHLOG_PROCESS_TYPE: "${HEALTHLOG_PROCESS_TYPE:-all}" diff --git a/src/lib/__tests__/geo.test.ts b/src/lib/__tests__/geo.test.ts index 642f4a7b4..dd231e4fb 100644 --- a/src/lib/__tests__/geo.test.ts +++ b/src/lib/__tests__/geo.test.ts @@ -1,11 +1,25 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +// v1.18.11 (W3): capture wide-event warnings so the non-ok-status path can +// be asserted. `getEvent()` returns null outside a request context, so +// without this mock the `getEvent()?.addWarning(...)` call is a silent +// no-op and there is nothing to assert against. +const addWarningSpy = vi.fn(); +vi.mock("@/lib/logging/context", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getEvent: () => ({ addWarning: addWarningSpy }), + }; +}); + const ORIGINAL_ENV = { ...process.env }; beforeEach(() => { process.env = { ...ORIGINAL_ENV }; delete process.env.IP_GEO_LOOKUP_URL; delete process.env.IP_GEO_LOOKUP_DISABLED; + addWarningSpy.mockClear(); vi.resetModules(); }); @@ -55,11 +69,14 @@ describe("lookupIpLocation IP-geolocation HTTPS guard", () => { ); } - it("uses HTTPS by default (ipwho.is)", async () => { + it("uses the ip-api.com HTTPS endpoint by default", async () => { + // v1.18.11 (W3): default provider is ip-api.com (ipwho.is 403s on + // datacentre egress). The wire URL must be the ip-api.com JSON path + // and the parser must accept its `status`/`countryCode` shape. const fetchSpy = vi .fn() .mockResolvedValue( - jsonOk({ success: true, city: "Berlin", country_code: "DE" }), + jsonOk({ status: "success", city: "Berlin", countryCode: "DE" }), ); vi.stubGlobal("fetch", fetchSpy); const { lookupIpLocation } = await import("../geo"); @@ -67,6 +84,7 @@ describe("lookupIpLocation IP-geolocation HTTPS guard", () => { expect(await lookupIpLocation("8.8.8.8")).toBe("Berlin, DE"); const url = fetchSpy.mock.calls[0]?.[0] as string; expect(url.startsWith("https://")).toBe(true); + expect(url).toBe("https://ip-api.com/json/8.8.8.8"); }); it("refuses to call non-HTTPS configured providers", async () => { @@ -104,6 +122,51 @@ describe("lookupIpLocation IP-geolocation HTTPS guard", () => { }); }); +// v1.18.11 (W3): a non-ok HTTP status (403 free-plan/CORS rejection, 429 +// rate-limit, 5xx) must surface a wide-event warning instead of being +// swallowed as a clean miss. The ipwho.is 403 that produced the prod "—" +// never emitted a single signal; this guards against that regression. +describe("lookupIpLocation non-ok status warning (v1.18.11 W3)", () => { + function nonOk(status: number): Response { + return new Response("", { + status, + headers: { "content-type": "application/json" }, + }); + } + + for (const status of [403, 429, 503]) { + it(`returns null and warns on HTTP ${status}`, async () => { + const fetchSpy = vi.fn().mockResolvedValue(nonOk(status)); + vi.stubGlobal("fetch", fetchSpy); + const { lookupIpLocation } = await import("../geo"); + + expect(await lookupIpLocation("8.8.8.8")).toBeNull(); + expect(addWarningSpy).toHaveBeenCalledTimes(1); + expect(addWarningSpy.mock.calls[0]?.[0]).toContain(String(status)); + }); + } + + it("does not warn on a clean 200 success", async () => { + const fetchSpy = vi.fn().mockResolvedValue( + new Response( + new TextEncoder().encode( + JSON.stringify({ + status: "success", + city: "Berlin", + countryCode: "DE", + }), + ).buffer as ArrayBuffer, + { status: 200, headers: { "content-type": "application/json" } }, + ), + ); + vi.stubGlobal("fetch", fetchSpy); + const { lookupIpLocation } = await import("../geo"); + + expect(await lookupIpLocation("8.8.8.8")).toBe("Berlin, DE"); + expect(addWarningSpy).not.toHaveBeenCalled(); + }); +}); + // v1.4.16 A8a: the maintainer spotted "Nrnberg" rendered in /admin/login-overview // for an audit row that should have read "Nürnberg, DE". The geo source // (ipwho.is) returns valid UTF-8, but `Response.json()` defers its diff --git a/src/lib/geo.ts b/src/lib/geo.ts index d7326b0e0..c3bfc95a9 100644 --- a/src/lib/geo.ts +++ b/src/lib/geo.ts @@ -30,11 +30,14 @@ * skip the offline tier — local dev without the MMDBs still works and * falls straight back to the online provider. * - * Default provider for the online lookup is ipwho.is (HTTPS, free, - * no key). Both the response shape from ipwho.is and the fallback - * ip-api.com pro endpoint are accepted, so swapping providers via - * `IP_GEO_LOOKUP_URL` only requires matching one of those response - * shapes. + * Default provider for the online lookup is ip-api.com (HTTPS JSON + * endpoint). ipwho.is was the v1.18.10 default but rejects datacentre / + * free-plan server egress with a 403, so it never resolved on prod. Both + * the ipwho.is shape (`success`/`country_code`) and the ip-api.com shape + * (`status`/`countryCode`) are accepted, so swapping providers via + * `IP_GEO_LOOKUP_URL` only requires matching one of those response shapes. + * A non-ok HTTP status (403/429/5xx) is surfaced on the wide event rather + * than swallowed, so a future provider rejection is visible. * * Setting `IP_GEO_LOOKUP_DISABLED=1` disables the online lookup * entirely — used by deployments that do not want any IP egress to a @@ -87,7 +90,16 @@ type GeoResponse = IpwhoIsResponse & IpApiProResponse; const PRIVATE_IP = /^(127\.|10\.|172\.(1[6-9]|2\d|3[01])\.|192\.168\.|::1|fc|fd|fe80|localhost|unknown)/; -const DEFAULT_GEO_URL = "https://ipwho.is"; +// v1.18.11 (W3) — the default online provider is ip-api.com, not ipwho.is. +// ipwho.is rejects datacentre / free-plan server egress with a 403 +// ("CORS is not supported on the Free plan"), which silently produced the +// "—" location on prod (every server-side lookup failed). The base is +// `https://ip-api.com/json`; `buildLookupUrl` appends `/` so the wire +// URL is `https://ip-api.com/json/`. The parser already accepts the +// ip-api.com shape (`status:"success"` + `countryCode`). Operators who need +// a different / keyed provider override via `IP_GEO_LOOKUP_URL`; the HTTPS +// egress guard in `buildLookupUrl` still applies to any override. +const DEFAULT_GEO_URL = "https://ip-api.com/json"; function geoLiteDir(): string { return process.env.GEOLITE2_DIR ?? "/opt/geolite2"; @@ -341,7 +353,19 @@ async function lookupIpLocationOnline(ip: string): Promise { // resolves null and drops the location (the "—" symptom on apps01). { timeoutMs: 2_500, requirePublicHost: true }, ); - if (!res.ok) return null; + if (!res.ok) { + // v1.18.11 (W3) — a non-ok response (403 free-plan/CORS rejection, + // 429 rate-limit, 5xx) is not a clean "this IP has no location": it is + // a provider-level failure that, left silent, renders as "—" with no + // signal anywhere. Surface it on the wide event so the next provider + // rejection is visible instead of swallowed (the ipwho.is 403 that + // caused the prod "—" never produced a single log line). Still return + // null so the offline fallback / negative-cache path runs unchanged. + getEvent()?.addWarning( + `geo: online lookup returned HTTP ${res.status} — location not resolved`, + ); + return null; + } const data = (await readUtf8Json(res)) as GeoResponse; const ok =