From c7148538b8cbd76e80c3c3c8da36a5e20ccefb69 Mon Sep 17 00:00:00 2001
From: "Aaron K. Clark" <akclark@thenetwerk.net>
Date: Tue, 19 May 2026 01:05:58 -0500
Subject: [PATCH] fix(error-handler): redact sensitive query params in the 404
 echo path
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`notFound` returned `path: req.originalUrl` verbatim. An SDK bug
that puts the authKey on the query string instead of the
authKey header (`GET /v1/foo?authKey=...`) would 404 against an
unmatched path and receive its own secret echoed back in the
response body — at which point any upstream proxy logging
responses, or the client's own error logger capturing 4xx bodies,
would persist the secret far from where it was supposed to live.

pino-http's request logger already routes `req.url` through
`redactUrl` for exactly this reason. Use the same helper here so
the 404 echo gets the same treatment. Pin behavior with a test
asserting `?authKey=...` becomes `?authKey=<REDACTED>` in the
echoed path while non-sensitive query params (`?q=ok`) are
preserved.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 app/middleware/error-handler.js | 12 +++++++++++-
 tests/api/error-handler.test.js | 11 +++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/app/middleware/error-handler.js b/app/middleware/error-handler.js
index f817d76..f4e0266 100644
--- a/app/middleware/error-handler.js
+++ b/app/middleware/error-handler.js
@@ -3,6 +3,7 @@
 "use strict";
 
 const log = require('../config/logger.js');
+const { redactUrl } = require('./redact-url.js');
 
 /**
  * Global error handler. Mounted AFTER all routes in server.js.
@@ -108,12 +109,21 @@ function errorHandler(err, req, res, next) {
 /**
  * 404 fallthrough. Mounted right before the error handler so any
  * unmatched route emits a structured 404 instead of Express's HTML.
+ *
+ * `path` is redacted through `redactUrl` to strip sensitive query
+ * parameters (authKey, token, password, …) before echoing the request
+ * back to the client. The pino-http request logger already redacts the
+ * same way; without this step, an SDK bug that puts the authKey on the
+ * query string (`GET /v1/foo?authKey=...` instead of in the header)
+ * would have its secret echoed back in the response body — and any
+ * upstream proxy / client-side error logger capturing the body
+ * would persist the secret.
  */
 function notFound(req, res) {
     return res.status(404).json({
         message: 'Not found.',
         method: req.method,
-        path: req.originalUrl,
+        path: redactUrl(req.originalUrl),
     });
 }
 
diff --git a/tests/api/error-handler.test.js b/tests/api/error-handler.test.js
index 03ce02f..021d8f4 100644
--- a/tests/api/error-handler.test.js
+++ b/tests/api/error-handler.test.js
@@ -130,4 +130,15 @@ describe('404 fallthrough', () => {
         const res = await request(app).post('/explode/500');
         expect(res.status).toBe(404);
     });
+
+    test('redacts sensitive query params in the echoed path', async () => {
+        // SDK bugs that put authKey in the query string instead of the
+        // header would otherwise have their secret echoed back in the
+        // 404 response body. Pin the redaction so any proxy / client
+        // error-logger capturing the body cannot persist the secret.
+        const res = await request(app).get('/no/such/path?authKey=super-secret&q=ok');
+        expect(res.status).toBe(404);
+        expect(res.body.path).toBe('/no/such/path?authKey=<REDACTED>&q=ok');
+        expect(res.body.path).not.toContain('super-secret');
+    });
 });