mirror of
https://github.com/clusterio/clusterio.git
synced 2026-11-13 23:14:53 +09:00
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
5e0ab3423f
commit
feb0e46aab
4 files changed
+103
-1
No files matched your search
@@ -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);
|
||||
|
||||
@@ -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) === " ") {
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in new issue
Block a user