fix(validate): stop leaking raw error details in the non-ZodError fallback (+ extend regression test to middleware/)#154
Merged
Conversation
…lback (+ extend regression test to middleware/) The `fmt(err)` helper in `app/middleware/validate.js` had the same `error: String(err)` pattern I scrubbed from all 17 controllers in #140 — but the regression test from that PR only walked `app/controllers/`, so the middleware copy of the leak survived. The fallback branch fires when `schema.safeParse` somehow surfaces a non-ZodError (defensive — zod normally always wraps). When it does, the raw stringified error landed in the 400 response body. A future zod release or a quirky schema mutation could trip it, leaking internal text the global error-handler policy explicitly suppresses. Two changes: 1. validate.js: drop `error: String(err)` from the fallback shape. The client gets a generic `{ message: 'Validation failed.' }`; the original error is whatever caller logging propagates. 2. tests/unit/controller-error-shape.test.js: scan `app/middleware/` too, not just `app/controllers/`. Future drift in either directory now fails CI. Also broadens the pattern set to catch `String(err)` (singular variable) and `error.message` / `err.message`, plus widens the status-code match from 5xx to 4xx-or-5xx so this validation 400 case is in scope. Net: 604 tests pass (was 595 + 9 new = 8 per-middleware-file checks + 1 sanity floor for the middleware directory). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #153.
Summary
fmt(err)inapp/middleware/validate.jshad the sameerror: String(err)shape #140 scrubbed from every controller — but #140's regression test (tests/unit/controller-error-shape.test.js) only walkedapp/controllers/, so this middleware copy survived. The non-ZodError fallback branch fires defensively whenschema.safeParsesurfaces something zod didn't wrap; when it does, the raw stringified error lands in the 400 response body.Two changes:
error: String(err)from the validate fallback. Clients see{ message: 'Validation failed.' }; the original error stays in operator-side logs.app/middleware/in addition toapp/controllers/, broaden the pattern set (catchesString(err)/err.message/error.messagevariants), and widen the matched status-code range from 5xx to 4xx-or-5xx so this validation-400 case is in scope.Test plan
npm run lint— cleannpm test— 604 passed (was 595 + 9 new = 8 per-middleware-file assertions + 1 middleware-dir sanity-floor), 15 skippedProudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/