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..b14d8e2a 100644 --- a/packages/lib/src/helpers.ts +++ b/packages/lib/src/helpers.ts @@ -268,6 +268,51 @@ 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); + } + + 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(); +} + 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..27486882 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&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) {