Skip to content

Commit f7ad8bd

Browse files
jkyberneeesmolty3000
andauthored
security: harden default error handler — safe-by-default (#51)
* security: harden default error handler — safe-by-default BREAKING: DEFAULT_ERROR_HANDLER now only exposes error details in NODE_ENV=development (opt-in). All other modes — production, staging, testing, and unset NODE_ENV — return sanitized "Internal Server Error". Previously, any mode except production leaked err.message to clients, which could expose DB queries, file paths, or internal state. Changes: - lib/router/sequential.js: flip condition from === 'production' to === 'development', add Content-Type header - tests/nested-routers.test.js: expect sanitized response in test mode - tests/router-coverage.test.js: expect sanitized response in test mode - tests/v4.4.test.js: add NODE_ENV-unset test - tooling/pentest.js: comprehensive 48-vector security test suite Pen test results: 48/48 passed, 0 findings (post-fix) Test suite: 64/64 passed, 97.7% coverage * fix: standard lint compliance in pentest.js - Remove unused requires (http, url) - Remove unused variables (checkPrototypePollution, timeout, nested→const) - Fix trailing commas (standard style) - Fix quotes (single quotes for strings) - Fix dot notation (res.getHeader('server') not ['server']) - Fix _body getter (captures mock response body correctly) - Convert template literals to string concatenation for standard --------- Co-authored-by: molty3000 <molty@21no.de>
1 parent 586f4e2 commit f7ad8bd

5 files changed

Lines changed: 661 additions & 5 deletions

File tree

lib/router/sequential.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@ const DEFAULT_ROUTE = (req, res) => {
1515

1616
const DEFAULT_ERROR_HANDLER = (err, req, res) => {
1717
res.statusCode = 500
18-
if (process.env.NODE_ENV === 'production') {
19-
res.end('Internal Server Error')
20-
} else {
18+
res.setHeader('Content-Type', 'text/plain')
19+
// Safe by default: only expose error details in explicit development mode.
20+
// Production, staging, testing, and unset NODE_ENV all receive sanitized response.
21+
if (process.env.NODE_ENV === 'development') {
2122
res.end(err.message)
23+
} else {
24+
res.end('Internal Server Error')
2225
}
2326
}
2427

tests/nested-routers.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ describe('0http - Nested Routers', () => {
9898
.get('/r2/rolando/throw')
9999
.expect(500)
100100
.then((response) => {
101-
expect(response.text).to.equals('nested error')
101+
expect(response.text).to.equals('Internal Server Error')
102102
})
103103
})
104104

tests/router-coverage.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ describe('0http - Router Coverage', () => {
161161
.get('/error')
162162
.expect(500)
163163
.then((response) => {
164-
expect(response.text).to.equal('Intentional error')
164+
expect(response.text).to.equal('Internal Server Error')
165165
})
166166
})
167167

tests/v4.4.test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,20 @@ describe('v4.4 Improvements', () => {
2828
.expect('Internal Server Error')
2929
})
3030

31+
it('should hide error message when NODE_ENV is unset', async () => {
32+
delete process.env.NODE_ENV
33+
const { router, server } = cero()
34+
35+
router.get('/error', (req, res, next) => {
36+
next(new Error('Sensitive Info'))
37+
})
38+
39+
await request(server)
40+
.get('/error')
41+
.expect(500)
42+
.expect('Internal Server Error')
43+
})
44+
3145
it('should show error message in development', async () => {
3246
process.env.NODE_ENV = 'development'
3347
const { router, server } = cero()

0 commit comments

Comments
 (0)