Skip to content

Commit f71612e

Browse files
fix: let VALIDATION_ERROR messages reach the client in production
The errorHandler middleware was unconditionally rewriting every VALIDATION_ERROR to the generic 'Invalid request' string in production, which silently neutralized the whole field-aware validation work from the previous commit. Live curl showed "Invalid request" on the wire even after the controllers were wired to formatZodError. Added PASS_THROUGH_CODES — a small allow-list of error codes whose real message is safe to expose. VALIDATION_ERROR is the only entry: its messages are written by formatZodError and contain only the field name, expected format, and the shape (not content) of what the client just submitted. Nothing the client doesn't already know; no internal state leaked. All other codes keep the existing sanitize-in-production behavior (generic message from the map, or 'An error occurred' for unknown codes). 2 new middleware tests lock both sides in: VALIDATION_ERROR propagates verbatim in production, NOT_FOUND still masks.
1 parent dc600f4 commit f71612e

2 files changed

Lines changed: 71 additions & 1 deletion

File tree

src/middleware/errorHandler.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import { AppError } from '../errors';
55
import { config } from '../config';
66
import { logger } from '../logger';
77

8+
// In production, messages for known error codes are replaced with sanitized
9+
// versions so internal detail never reaches the client; unknown codes fall
10+
// back to 'An error occurred'. See PASS_THROUGH_CODES below for the explicit
11+
// whitelist of error codes that propagate the real message verbatim.
812
const GENERIC_MESSAGES: Record<string, string> = {
913
NOT_FOUND: 'Resource not found',
1014
VALIDATION_ERROR: 'Invalid request',
@@ -13,9 +17,16 @@ const GENERIC_MESSAGES: Record<string, string> = {
1317
PAYMENT_REQUIRED: 'Payment required',
1418
};
1519

20+
// Codes whose real message is SAFE to expose to the client in production.
21+
// VALIDATION_ERROR messages are written by formatZodError and contain only
22+
// the field name, expected format, and the shape (not content) of the value
23+
// the client just submitted — nothing the client doesn't already know.
24+
const PASS_THROUGH_CODES = new Set(['VALIDATION_ERROR']);
25+
1626
export function errorHandler(err: Error, req: Request, res: Response, _next: NextFunction): void {
1727
if (err instanceof AppError) {
18-
const message = config.NODE_ENV === 'production'
28+
const shouldSanitize = config.NODE_ENV === 'production' && !PASS_THROUGH_CODES.has(err.code);
29+
const message = shouldSanitize
1930
? (GENERIC_MESSAGES[err.code] ?? 'An error occurred')
2031
: err.message;
2132

src/tests/middleware.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,65 @@ describe('errorHandler', () => {
165165
const body = res._body as { error: { code: string; message: string } };
166166
expect(body.error.message).toBe('Detailed dev error info');
167167
});
168+
169+
it('propagates the real VALIDATION_ERROR message in production', async () => {
170+
// VALIDATION_ERROR is on the pass-through allow-list: messages come from
171+
// formatZodError and are explicitly written for the client, so the
172+
// generic "Invalid request" must NOT mask them in production.
173+
vi.doMock('../config', () => ({
174+
config: { NODE_ENV: 'production' },
175+
}));
176+
vi.doMock('../logger', () => ({
177+
logger: { error: vi.fn() },
178+
}));
179+
180+
const { errorHandler } = await import('../middleware/errorHandler');
181+
const { ValidationError } = await import('../errors');
182+
const res = mockRes();
183+
184+
errorHandler(
185+
new ValidationError('caller must be a 64-char SHA256 hash or 66-char Lightning pubkey (02/03 prefix), got 11 chars'),
186+
mockReq(),
187+
res as unknown as Response,
188+
vi.fn() as unknown as NextFunction,
189+
);
190+
191+
expect(res._status).toBe(400);
192+
const body = res._body as { error: { code: string; message: string } };
193+
expect(body.error.code).toBe('VALIDATION_ERROR');
194+
expect(body.error.message).toContain('caller');
195+
expect(body.error.message).toContain('got 11 chars');
196+
expect(body.error.message).not.toBe('Invalid request');
197+
});
198+
199+
it('still masks NOT_FOUND messages in production', async () => {
200+
// Spot-check that the generic-messages behavior still applies to codes
201+
// NOT on the pass-through list, so the pass-through doesn't leak to
202+
// other error types.
203+
vi.doMock('../config', () => ({
204+
config: { NODE_ENV: 'production' },
205+
}));
206+
vi.doMock('../logger', () => ({
207+
logger: { error: vi.fn() },
208+
}));
209+
210+
const { errorHandler } = await import('../middleware/errorHandler');
211+
const { NotFoundError } = await import('../errors');
212+
const res = mockRes();
213+
214+
errorHandler(
215+
new NotFoundError('SecretInternalResource', 'hidden-id'),
216+
mockReq(),
217+
res as unknown as Response,
218+
vi.fn() as unknown as NextFunction,
219+
);
220+
221+
expect(res._status).toBe(404);
222+
const body = res._body as { error: { code: string; message: string } };
223+
expect(body.error.code).toBe('NOT_FOUND');
224+
expect(body.error.message).toBe('Resource not found');
225+
expect(body.error.message).not.toContain('SecretInternal');
226+
});
168227
});
169228

170229
describe('OpenAPI spec auth alignment', () => {

0 commit comments

Comments
 (0)