Redact credentials from logged request urls (#1074)

The controller logs the url and query of every HTTP request. Plugins
doing OAuth get the authorization code as a query parameter on their
callback route, so it ended up in the log. redactUrl now accepts a
path, code joins the redacted parameter names, and redactQuery covers
the parsed query in the log metadata.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

packages/controller/src/Controller.ts

@@ -403,16 +403,17 @@ export default class Controller {
 
 		// Log all express errors from middleware or routes (app.use is missing the type overload)
 		this.app.use((err: Error, req: express.Request, res: express.Response, next: express.NextFunction): void => {
+			const url = lib.redactUrl(req.originalUrl);
 			logger.log({
 				level: "error",
 				message: "Error while handling HTTP" +
-					`${req.httpVersion} ${req.method} ${req.originalUrl}:\n${err.stack}`,
+					`${req.httpVersion} ${req.method} ${url}:\n${err.stack}`,
 				meta: {
 					method: req.method,
-					url: req.originalUrl,
+					url,
 					httpVersion: req.httpVersion,
 					headers: Controller.loggableHeaders(req.headers),
-					query: req.query,
+					query: lib.redactQuery(req.query),
 				},
 			});
 
@@ -889,18 +890,19 @@ export default class Controller {
 				const endNs = process.hrtime.bigint();
 				const durationMs = (Number(endNs - startNs) / 1e6);
 				endpointDurationSummary.labels(routePath).observe(durationMs / 1e3);
+				const url = lib.redactUrl(req.originalUrl);
 				logger.log({
 					level: "http",
-					message: `HTTP${req.httpVersion} ${req.method} ${req.originalUrl}` +
+					message: `HTTP${req.httpVersion} ${req.method} ${url}` +
 						` (Status ${res.statusCode} in ${durationMs}ms)`,
 					meta: {
 						method: req.method,
-						url: req.originalUrl,
+						url,
 						statusCode: res.statusCode,
 						responseTime: durationMs,
 						httpVersion: req.httpVersion,
 						headers: Controller.loggableHeaders(req.headers),
-						query: req.query,
+						query: lib.redactQuery(req.query),
 					},
 				});
 			});

packages/lib/src/helpers.ts

@@ -277,6 +277,8 @@ const sensitiveSearchParams = new Set([
 	"access_token",
 	"api_key",
 	"apikey",
+	// OAuth2 authorization code, plugins doing OAuth receive it on their callback route
+	"code",
 	"password",
 	"secret",
 	"token",
@@ -290,19 +292,25 @@ const sensitiveSearchParams = new Set([
  * 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.
+ * Accepts a path with a query string, like the url of an incoming HTTP
+ * request, and returns a path in that case.  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.
+ * @param url - URL or path to redact credentials from.
  * @returns url with credentials replaced by `REDACTED`.
  */
 export function redactUrl(url: URL | string) {
+	const isPath = typeof url === "string" && url.startsWith("/") && !url.startsWith("//");
 	let parsed;
 	try {
-		parsed = new URL(url);
+		parsed = isPath ? new URL(url, "http://localhost") : new URL(url);
 	} catch {
 		return String(url);
 	}
+	if (![...parsed.searchParams.keys()].some(name => sensitiveSearchParams.has(name.toLowerCase()))) {
+		return String(url);
+	}
 
 	const searchParams = new URLSearchParams();
 	for (const [name, value] of parsed.searchParams) {
@@ -310,7 +318,25 @@ export function redactUrl(url: URL | string) {
 	}
 
 	parsed.search = searchParams.toString();
-	return parsed.toString();
+	return isPath ? parsed.pathname + parsed.search + parsed.hash : parsed.toString();
+}
+
+/**
+ * Copy of parsed query parameters with credentials replaced by `REDACTED`
+ *
+ * Uses the same parameter names as {@link redactUrl}.
+ *
+ * @param query - query parameters, for example `req.query` from express.
+ * @returns query safe to write to the log.
+ */
+export function redactQuery(query: Record<string, unknown>) {
+	const result = { ...query };
+	for (const name of Object.keys(result)) {
+		if (sensitiveSearchParams.has(name.toLowerCase())) {
+			result[name] = "REDACTED";
+		}
+	}
+	return result;
 }
 
 function skipWhitespace(pos: number, input: string) {

test/lib/helpers.js

@@ -241,7 +241,7 @@ describe("lib/helpers", function() {
 				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"]) {
+			for (const name of ["access_token", "api_key", "apikey", "code", "password", "secret", "token"]) {
 				assert.equal(
 					lib.redactUrl(`https://example.com/?${name}=secret`),
 					`https://example.com/?${name}=REDACTED`
@@ -272,6 +272,24 @@ describe("lib/helpers", function() {
 			assert.equal(lib.redactUrl("not a url?token=secret"), "not a url?token=secret");
 			assert.equal(lib.redactUrl(""), "");
 		});
+		it("should redact a path with a query string and return a path", function() {
+			assert.equal(
+				lib.redactUrl("/api/plugin/callback?code=secret&state=abc"),
+				"/api/plugin/callback?code=REDACTED&state=abc"
+			);
+			assert.equal(lib.redactUrl("/api/plugin?page=2"), "/api/plugin?page=2");
+		});
+		it("should keep the original encoding when there is nothing to redact", function() {
+			assert.equal(lib.redactUrl("/search?q=a%20b"), "/search?q=a%20b");
+		});
+	});
+
+	describe("redactQuery()", function() {
+		it("should redact credential parameters and keep the rest", function() {
+			const query = { code: "secret", Token: "secret", state: "abc" };
+			assert.deepEqual(lib.redactQuery(query), { code: "REDACTED", Token: "REDACTED", state: "abc" });
+			assert.equal(query.code, "secret");
+		});
 	});
 
 	function parse(input, attributes) {