From feb0e46aab3c2b84c96cac0a670d38b072548157 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Thu, 23 Jul 2026 20:38:09 +0000 Subject: [PATCH 1/2] Redact credentials from URLs in download errors The mod portal takes the Factorio account token as a query parameter on the download URL. When a download failed, downloadFile put the whole URL into the error message, which ModStore then logged verbatim, printing the token into the log at error level on every failed mod download. Add a redactUrl helper that replaces the value of credential-carrying query parameters with REDACTED, and use it when downloadFile builds its error. Parameter names and the rest of the URL are kept so the message stays useful for diagnosing which request failed; username is deliberately left intact as it is not a secret and identifies the account in use. Fixes #795 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/lib/src/file_ops.ts | 4 +++- packages/lib/src/helpers.ts | 44 ++++++++++++++++++++++++++++++++++++ test/lib/file_ops.js | 17 ++++++++++++++ test/lib/helpers.js | 39 ++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 1 deletion(-) diff --git a/packages/lib/src/file_ops.ts b/packages/lib/src/file_ops.ts index d1dbf11f..6b54548d 100644 --- a/packages/lib/src/file_ops.ts +++ b/packages/lib/src/file_ops.ts @@ -7,6 +7,8 @@ import fs from "node:fs/promises"; import path from "path"; import stream from "stream"; +import { redactUrl } from "./helpers"; + /** * Returns the newest file in a directory @@ -311,7 +313,7 @@ export async function downloadFile( writeStream.end(); await events.once(writeStream, "close"); await fs.unlink(tempFilePath); - throw new Error(`Failed to download ${url}: ${response.status} ${response.statusText}`); + throw new Error(`Failed to download ${redactUrl(url)}: ${response.status} ${response.statusText}`); } const writer = stream.Writable.toWeb(writeStream); await response.body.pipeTo(writer); diff --git a/packages/lib/src/helpers.ts b/packages/lib/src/helpers.ts index 4d98d7a1..051339c7 100644 --- a/packages/lib/src/helpers.ts +++ b/packages/lib/src/helpers.ts @@ -268,6 +268,50 @@ export function formatBytes(bytes: number, prefixes: "si" | "binary" = "si") { return significant.toFixed(fractionDigits) + units[exponent]; } +/** + * Query parameters whose values are credentials and must never be logged. + * + * Matched case insensitively against the full parameter name. + */ +const sensitiveSearchParams = new Set([ + "access_token", + "api_key", + "apikey", + "password", + "secret", + "token", +]); + +/** + * Replace credentials in a URL with a placeholder + * + * Returns the URL as a string with the values of any query parameter + * carrying a credential replaced by `REDACTED`, so that the URL can + * safely be included in log output and error messages. The parameter + * names are kept intact to preserve the diagnostic value of the URL. + * + * Returns the input unchanged if it is a string which does not parse as + * a URL, as there are then no query parameters to redact. + * + * @param url - URL to redact credentials from. + * @returns url with credentials replaced by `REDACTED`. + */ +export function redactUrl(url: URL | string) { + let parsed; + try { + parsed = new URL(url); + } catch { + return String(url); + } + + for (const name of [...parsed.searchParams.keys()]) { + if (sensitiveSearchParams.has(name.toLowerCase())) { + parsed.searchParams.set(name, "REDACTED"); + } + } + return parsed.toString(); +} + function skipWhitespace(pos: number, input: string) { // whitespace = 1*" " while (pos < input.length && input.charAt(pos) === " ") { diff --git a/test/lib/file_ops.js b/test/lib/file_ops.js index 7e503326..428fd493 100644 --- a/test/lib/file_ops.js +++ b/test/lib/file_ops.js @@ -291,6 +291,23 @@ describe("lib/file_ops", function() { ); }); + it("should not leak credentials from the url when throwing", async function () { + // The mod portal takes the Factorio token as a query parameter, see #795. + const downloadPath = path.join(downloadDir, "credentials.txt"); + const url = new URL("/not-found", baseUrl); + url.searchParams.set("username", "user"); + url.searchParams.set("token", "secret"); + await assert.rejects( + lib.downloadFile(url, downloadPath, "overwrite"), + err => { + assert.ok(!err.message.includes("secret"), `token leaked in: ${err.message}`); + assert.match(err.message, /token=REDACTED/); + assert.match(err.message, /username=user/); + return true; + } + ); + }); + it("should throw if fetch response body is missing", async function () { const downloadPath = path.join(downloadDir, "not-content.txt"); await assert.rejects( diff --git a/test/lib/helpers.js b/test/lib/helpers.js index b38e50f7..f8632cf0 100644 --- a/test/lib/helpers.js +++ b/test/lib/helpers.js @@ -236,6 +236,45 @@ describe("lib/helpers", function() { }); }); + describe("redactUrl()", function() { + it("should redact the value of credential query parameters", function() { + assert.equal( + lib.redactUrl("https://mods.factorio.com/download/mod?username=user&token=secret"), + "https://mods.factorio.com/download/mod?username=user&token=REDACTED" + ); + for (const name of ["access_token", "api_key", "apikey", "password", "secret", "token"]) { + assert.equal( + lib.redactUrl(`https://example.com/?${name}=secret`), + `https://example.com/?${name}=REDACTED` + ); + } + }); + it("should match credential parameter names case insensitively", function() { + assert.equal(lib.redactUrl("https://example.com/?Token=secret"), "https://example.com/?Token=REDACTED"); + assert.equal(lib.redactUrl("https://example.com/?API_KEY=secret"), "https://example.com/?API_KEY=REDACTED"); + }); + it("should redact every occurrence of a repeated credential parameter", function() { + assert.equal( + lib.redactUrl("https://example.com/?token=first&token=second"), + "https://example.com/?token=REDACTED" + ); + }); + it("should accept a URL object", function() { + assert.equal( + lib.redactUrl(new URL("https://example.com/path?token=secret")), + "https://example.com/path?token=REDACTED" + ); + }); + it("should leave URLs without credentials untouched", function() { + assert.equal(lib.redactUrl("https://example.com/path"), "https://example.com/path"); + assert.equal(lib.redactUrl("https://example.com/?page=2"), "https://example.com/?page=2"); + }); + it("should return unparsable input unchanged", function() { + assert.equal(lib.redactUrl("not a url?token=secret"), "not a url?token=secret"); + assert.equal(lib.redactUrl(""), ""); + }); + }); + function parse(input, attributes) { const result = lib.parseSearchString(input, attributes); if (!result.issues.length) { From 417ee874229fa36a423519344a0f14e067dfc805 Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Wed, 29 Jul 2026 22:56:19 +0100 Subject: [PATCH 2/2] Maintain repeated credential parameters --- packages/lib/src/helpers.ts | 9 +++++---- test/lib/helpers.js | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/lib/src/helpers.ts b/packages/lib/src/helpers.ts index 051339c7..b14d8e2a 100644 --- a/packages/lib/src/helpers.ts +++ b/packages/lib/src/helpers.ts @@ -304,11 +304,12 @@ export function redactUrl(url: URL | string) { return String(url); } - for (const name of [...parsed.searchParams.keys()]) { - if (sensitiveSearchParams.has(name.toLowerCase())) { - parsed.searchParams.set(name, "REDACTED"); - } + const searchParams = new URLSearchParams(); + for (const [name, value] of parsed.searchParams) { + searchParams.append(name, sensitiveSearchParams.has(name.toLowerCase()) ? "REDACTED" : value); } + + parsed.search = searchParams.toString(); return parsed.toString(); } diff --git a/test/lib/helpers.js b/test/lib/helpers.js index f8632cf0..27486882 100644 --- a/test/lib/helpers.js +++ b/test/lib/helpers.js @@ -256,7 +256,7 @@ describe("lib/helpers", function() { it("should redact every occurrence of a repeated credential parameter", function() { assert.equal( lib.redactUrl("https://example.com/?token=first&token=second"), - "https://example.com/?token=REDACTED" + "https://example.com/?token=REDACTED&token=REDACTED" ); }); it("should accept a URL object", function() {