Skip to content

fix(security): never echo err.message verbatim on 4xx#75

Merged
CryptoJones merged 1 commit into
masterfrom
fix/security-error-handler-no-leak
May 18, 2026
Merged

fix(security): never echo err.message verbatim on 4xx#75
CryptoJones merged 1 commit into
masterfrom
fix/security-error-handler-no-leak

Conversation

@CryptoJones
Copy link
Copy Markdown
Owner

Closes #73 P1-B.

Problem

error-handler.js echoed err.message verbatim on any status < 500. Future middleware that throws with detail (Sequelize: column names; pg driver: hostnames; third-party libs: stack frames) leaked to the client.

Fix

4xx defaults to per-status generic messages. Pass-through is opt-in via err.expose === true (http-errors convention) — body-parser, http-errors-derived libs, our own intentional client errors all keep working. Sequelize / pg-driver / un-annotated errors get the generic.

Test plan

  • vitest: 280 + 4 integration skipped (was 276 + 4 skipped)
  • New cases: 4xx without expose → generic; 4xx with expose=true → message echoed; PayloadTooLarge → echoed; Sequelize-style 4xx → generic ("Sequelize" / column name not in body)
  • Existing 500-doesn't-leak coverage preserved

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

Before: error-handler.js returned err.message verbatim for any
status < 500. A future middleware / library that threw with detail
(Sequelize validation error revealing column names, pg driver
hostname leak, third-party library stack frame) would surface that
to the client without filtering.

After: 4xx responses use generic per-status messages by default
(GENERIC_MESSAGES table). Echo the original err.message ONLY when
the error is explicitly marked safe via err.expose === true —
the http-errors / express body-parser convention. Body-parser's
PayloadTooLargeError, BadRequest from malformed JSON, etc. all
set expose=true, so legitimate client-side errors keep their
descriptive messages. Sequelize, pg, and most third-party libs
don't set expose, so their messages now stay in the log instead
of the response body.

5xx behavior unchanged — still always "Error!" with a requestId
for log correlation.

Tests:
  - Existing "doesn't leak 500 detail" coverage preserved.
  - New cases:
    * 4xx without expose → generic, no detail
    * 4xx with expose=true → message echoed
    * PayloadTooLargeError-style → message echoed
    * Sequelize-style 4xx → generic, no "Sequelize" / column name

Closes #73 P1-B.

Suite: 280 / 280 + 4 integration skipped (was 276 / 276).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@CryptoJones CryptoJones merged commit d2d208d into master May 18, 2026
2 of 3 checks passed
@CryptoJones CryptoJones deleted the fix/security-error-handler-no-leak branch May 18, 2026 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Architecture audit: prioritized list of security + functionality follow-ups

1 participant