From 8bad74777018ae9b02220e6d61e02924a4ef111d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= <mbombeck@gmail.com>
Date: Sun, 9 Aug 2026 22:25:48 +0200
Subject: [PATCH] Keep the MaxMind licence key out of the runtime-fetch error
 path

safeFetch embeds the full target URL in its thrown error, and the
GeoLite2 download URL carries license_key=<secret> in its query string.
The wide-event warning and job-failure paths do not redact it, so a
timeout or network error on the runtime fetch printed the key to stdout,
Loki and GlitchTip in clear. The download timeout is 120s for slow
self-host links, so that path is expected, not rare.

The fetch now re-throws with the edition and the error kind only, never
the URL. As defence in depth, the central query-param redactor also
learns license_key, so any other path that logs such a URL through
redactSecrets is covered too. A regression test asserts the returned
error carries neither the key nor the parameter name.
---
 src/lib/geo/__tests__/geolite2-fetch.test.ts | 20 +++++++++++
 src/lib/geo/geolite2-fetch.ts                | 38 +++++++++++++-------
 src/lib/logging/redact.ts                    |  2 +-
 3 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/src/lib/geo/__tests__/geolite2-fetch.test.ts b/src/lib/geo/__tests__/geolite2-fetch.test.ts
index c3958f911..8caed00e3 100644
--- a/src/lib/geo/__tests__/geolite2-fetch.test.ts
+++ b/src/lib/geo/__tests__/geolite2-fetch.test.ts
@@ -182,6 +182,26 @@ describe("fetchGeoLite2Databases — fail-soft", () => {
     expect(resetCacheSpy).not.toHaveBeenCalled();
   });
 
+  it("never leaks the licence key into the error on a fetch-layer throw", async () => {
+    // safeFetch embeds the full target URL — which carries `license_key=<secret>`
+    // — in its thrown message. The download timeout was raised to 120s for slow
+    // self-host links, so timeouts are an expected path; the reported error must
+    // name the failure without the URL, or the key lands on stdout / Loki.
+    safeFetchMock.mockRejectedValue(
+      new SafeFetchError(
+        "safeFetch aborted (TimeoutError): https://download.maxmind.com/app/geoip_download?edition_id=GeoLite2-City&license_key=test-licence-key&suffix=tar.gz",
+        "timeout",
+      ),
+    );
+
+    const result = await fetchGeoLite2Databases();
+
+    expect(result.status).toBe("failed");
+    expect(result.error).toBeDefined();
+    expect(result.error).not.toContain("test-licence-key");
+    expect(result.error).not.toContain("license_key");
+  });
+
   it("promotes nothing when the first edition succeeds but the second fails", async () => {
     // The atomicity contract: City downloads + extracts fine, ASN 401s. Since
     // not every edition was staged, NOTHING is promoted — no GeoLite2-City.mmdb
diff --git a/src/lib/geo/geolite2-fetch.ts b/src/lib/geo/geolite2-fetch.ts
index 25bd4f2f8..e447f1672 100644
--- a/src/lib/geo/geolite2-fetch.ts
+++ b/src/lib/geo/geolite2-fetch.ts
@@ -35,7 +35,7 @@ import zlib from "node:zlib";
 import fsp from "node:fs/promises";
 import path from "node:path";
 import { getEvent } from "@/lib/logging/context";
-import { safeFetch } from "@/lib/safe-fetch";
+import { safeFetch, SafeFetchError } from "@/lib/safe-fetch";
 import {
   geoLiteDir,
   offlineGeoReady,
@@ -173,18 +173,30 @@ async function downloadAndExtractEdition(
   destDir: string,
 ): Promise<string> {
   const url = buildMaxmindDownloadUrl(edition.editionId, licenseKey);
-  const res = await safeFetch(
-    url,
-    { headers: { Accept: "application/gzip" } },
-    {
-      // The permalink 302s to a signed CDN, so follow redirects; keep the
-      // connect-time public-host pin so a rebinding cannot swing the hop onto
-      // a private/metadata address.
-      followRedirects: true,
-      requirePublicHost: true,
-      timeoutMs: DOWNLOAD_TIMEOUT_MS,
-    },
-  );
+  // The URL carries `license_key=<secret>` in its query string. safeFetch
+  // embeds the full URL in its thrown error message, and the wide-event
+  // warning / job-failure paths do not redact it, so a bare re-throw would
+  // print the licence key to stdout, Loki and GlitchTip on any timeout or
+  // network error. Re-throw with the edition and the error KIND only, never
+  // the URL.
+  let res: Response;
+  try {
+    res = await safeFetch(
+      url,
+      { headers: { Accept: "application/gzip" } },
+      {
+        // The permalink 302s to a signed CDN, so follow redirects; keep the
+        // connect-time public-host pin so a rebinding cannot swing the hop onto
+        // a private/metadata address.
+        followRedirects: true,
+        requirePublicHost: true,
+        timeoutMs: DOWNLOAD_TIMEOUT_MS,
+      },
+    );
+  } catch (err) {
+    const kind = err instanceof SafeFetchError ? err.kind : "network";
+    throw new Error(`${edition.editionId} download failed (${kind})`);
+  }
   if (!res.ok) {
     // 401 = wrong key / unsigned EULA, 403 = throttle, 5xx = MaxMind-side.
     throw new Error(
diff --git a/src/lib/logging/redact.ts b/src/lib/logging/redact.ts
index 8c52e78ac..426417981 100644
--- a/src/lib/logging/redact.ts
+++ b/src/lib/logging/redact.ts
@@ -110,7 +110,7 @@ export function redactSecrets(input: string): string {
       // `passphrase` that derives the encrypted-export key must never land in
       // `http.path` / error strings reaching Loki / Glitchtip.
       .replace(
-        /([?&])(secret|code|token|ticket|api[_-]?key|insurance(?:number)?|kvnr|totp|mfa|backup[_-]?code|recovery[_-]?code|passphrase)=[^&\s]+/gi,
+        /([?&])(secret|code|token|ticket|api[_-]?key|license[_-]?key|insurance(?:number)?|kvnr|totp|mfa|backup[_-]?code|recovery[_-]?code|passphrase)=[^&\s]+/gi,
         "$1$2=[REDACTED]",
       )
   );