mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 05:42:19 +00:00
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.
This commit is contained in:
1 parent
0a2cacb8b3
commit
8bad747770
3 files changed
+46
-14
No files matched your search
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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]",
|
||||
)
|
||||
);
|
||||
|
||||
Reference in new issue
Block a user