From b4bcf1385cd1a866b0d1041f9e2fada0b0876187 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Tue, 19 May 2026 14:31:55 -0500 Subject: [PATCH] test(error-shape): also catch \`message:\` field leakage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tests/unit/controller-error-shape.test.js` scanned for variants of `error: String(err) / err.message / error.message` to catch the original 137-occurrence leak that #140 cleaned up. But the runtime-shipped response body uses `message:` as its key — and a future contributor who writes `res.status(500).json({ message: err.message })` would route around the global error-handler's policy without the test catching the regression. Extend the regex set to cover the parallel `message:` patterns: - `message: String(error)` / `message: String(err)` - `message: err.message` - `message: error.message` A grep of `app/` confirms zero existing occurrences — this is a pure-tightening change. The global error-handler in `app/middleware/error-handler.js` is the ONE place allowed to echo err.message (under err.expose === true); the test specifically scans `app/controllers` and `app/middleware` so the handler's own file is covered too — its `message = err.message` assignment is NOT a `message: err.message` object-literal shape, so the regex doesn't false-positive there. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/unit/controller-error-shape.test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/unit/controller-error-shape.test.js b/tests/unit/controller-error-shape.test.js index 6a75bc0..6691575 100644 --- a/tests/unit/controller-error-shape.test.js +++ b/tests/unit/controller-error-shape.test.js @@ -56,10 +56,28 @@ describe('server code does not leak raw error details in error responses', () => // surface error detail emerges, prefer `next(err)` with // `err.expose = true` and let the global error-handler do it // under controlled conditions. + // + // Two field names to defend: `error:` (the original leak + // location pre-#140) and `message:` (the standard + // response-shape key — controllers MUST emit a hardcoded + // generic string here, never the raw error.message or + // String(err)). The global error-handler is the ONE place + // allowed to echo err.message when err.expose === true; + // controllers and middleware route errors through it via + // `next(err)` instead of building the body themselves. expect(content).not.toMatch(/error:\s*String\(\s*error\s*\)/); expect(content).not.toMatch(/error:\s*String\(\s*err\s*\)/); expect(content).not.toMatch(/error:\s*err\.message/); expect(content).not.toMatch(/error:\s*error\.message/); + // Same patterns guarded against on the `message:` key. + // Controllers emit fixed strings (`"Error!"`, `"Not found."`, + // `"Authorization key not sent."`); raw err / error + // references slipping in would route around the global + // handler. + expect(content).not.toMatch(/message:\s*String\(\s*error\s*\)/); + expect(content).not.toMatch(/message:\s*String\(\s*err\s*\)/); + expect(content).not.toMatch(/message:\s*err\.message/); + expect(content).not.toMatch(/message:\s*error\.message/); // And the static-string variant the customer controller used to // have: `error: "Sequelize Op not available"`. Same principle — // anything hardcoded into an error body bypasses the global