Merge pull request #954 from bbassie/feature/redact-mod-portal-token

Redact credentials from URLs in download errors

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);

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) === " ") {

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(

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) {