From 24586a4b7c4478c7680a7358c22485b6b8ff63d5 Mon Sep 17 00:00:00 2001 From: Bastiaan <17990055+bbassie@users.noreply.github.com> Date: Wed, 14 Oct 2026 16:58:00 +0200 Subject: [PATCH] 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 --- packages/controller/src/Controller.ts | 14 ++++++----- packages/lib/src/helpers.ts | 36 +++++++++++++++++++++++---- test/lib/helpers.js | 20 ++++++++++++++- 3 files changed, 58 insertions(+), 12 deletions(-) diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index ef82a15b..454b96ef 100644 --- a/packages/controller/src/Controller.ts +++ b/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), }, }); }); diff --git a/packages/lib/src/helpers.ts b/packages/lib/src/helpers.ts index dbbf2f26..289f978a 100644 --- a/packages/lib/src/helpers.ts +++ b/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) { + 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) { diff --git a/test/lib/helpers.js b/test/lib/helpers.js index 2517ab6a..8bd1466d 100644 --- a/test/lib/helpers.js +++ b/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) {