Merge pull request #120 from BackendStack21/fix/security-audit

Security hardening: safe error defaults, stream handling, immutable config, and promise depth cap

Author: jkyberneees

.github/workflows/tests.yaml

@@ -4,12 +4,15 @@ on: [push, pull_request]
 jobs:
   testing:
     runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        node-version: [20.x, 22.x]
     steps:
-      - uses: actions/checkout@v1
-      - name: Setup Environment (Using NodeJS 16.x)
-        uses: actions/setup-node@v1
+      - uses: actions/checkout@v4
+      - name: Setup Environment (Using NodeJS ${{ matrix.node-version }})
+        uses: actions/setup-node@v4
         with:
-          node-version: 16.x
+          node-version: ${{ matrix.node-version }}
 
       - name: Install dependencies
         run: npm install

README.md

@@ -57,5 +57,11 @@ service.get('/hi', (req, res) => res.send('Hello World!'))
 http.createServer(service).listen(3000, '0.0.0.0')
 ```
 
+# Security Defaults
+Restana ships with secure defaults out of the box:
+- **Error handling**: The default error handler returns a generic `Internal Server Error` message, preventing internal details (stack traces, database errors, file paths) from leaking to clients. Provide a custom `errorHandler` to control what gets exposed.
+- **Stream safety**: Stream errors are handled gracefully, preventing connection leaks.
+- **Immutable config**: `getConfigOptions()` returns a frozen copy, preventing middleware from mutating internal framework options.
+
 # More
 - Website and documentation: https://restana.21no.de

docs/README.md

@@ -80,7 +80,7 @@ Optionally, learn through examples:
 - `server`: Allows to optionally override the HTTP server instance to be used.
 - `prioRequestsProcessing`: If `TRUE`, HTTP requests processing/handling is prioritized using `setImmediate`. Default value: `TRUE`
 - `defaultRoute`: Optional route handler when no route match occurs. Default value: `((req, res) => res.send(404))`
-- `errorHandler`: Optional global error handler function. Default value: `(err, req, res) => res.send(err)`
+- `errorHandler`: Optional global error handler function. Default value: `(err, req, res) => res.send({ code, message: 'Internal Server Error' }, code)`. The default handler returns a generic error message to prevent leaking sensitive internal details (e.g. database connection strings, file paths, stack traces). The appropriate HTTP status code is still preserved from `err.status`, `err.code`, or `err.statusCode`.
 - `routerCacheSize`: The router matching cache size, indicates how many request matches will be kept in memory. Default value: `2000`
 
 # Full service example
@@ -136,6 +136,12 @@ service.start(3000).then((server) => {})
 service.close().then(()=> {})
 ```
 
+## Accessing configuration options
+```js
+const opts = service.getConfigOptions()
+```
+> `getConfigOptions()` returns a frozen shallow copy of the configuration options. This prevents third-party middleware from accidentally or maliciously modifying internal framework options at runtime.
+
 ## Async / Await support
 ```js
 service.post('/star/:username', async (req, res) => {
@@ -163,8 +169,8 @@ Supported datatypes are:
 - String
 - Buffer
 - Object
-- Stream
-- Promise
+- Stream (errors on the stream are handled gracefully, terminating the response instead of leaving the connection hanging)
+- Promise (recursive promise resolution is capped at a depth of 3 to prevent event loop starvation)
 
 Example usage:
 ```js
@@ -192,6 +198,9 @@ res.send(
 > `res.send(401)`
 
 ## Global error handling
+By default, restana returns a generic `Internal Server Error` message to the client, preventing internal details from being leaked. The HTTP status code is preserved from `err.status`, `err.code`, or `err.statusCode` (defaults to `500`).
+
+To customize error responses, provide your own `errorHandler`:
 ```js
 const service = require('restana')({
   errorHandler (err, req, res) {
@@ -204,6 +213,7 @@ service.get('/throw', (req, res) => {
   throw new Error('Upps!')
 })
 ```
+> **Note:** When using `res.send(err)` in a custom error handler, the error's `message` and `data` properties will be serialized and sent to the client. Make sure your custom handler only exposes information you intend to be public.
 ### errorHandler not being called?
 > Issue: https://github.com/jkyberneees/ana/issues/81  
 
@@ -457,6 +467,15 @@ service.get('/hello', (req, res) => {
 https://goo.gl/forms/qlBwrf5raqfQwteH3
 
 # Breaking changes
+## 5.2
+> Restana version 5.2 includes important security hardening while remaining backward compatible for most users.  
+
+Changed:
+ - The default `errorHandler` no longer sends `err.message` or `err.data` to clients. It now returns a generic `{ code, message: 'Internal Server Error' }` response. If you need the previous behavior, provide a custom `errorHandler`.
+ - `getConfigOptions()` now returns a frozen shallow copy of the options object instead of a direct mutable reference.
+ - Stream responses (`res.send(stream)`) now handle stream errors gracefully, terminating the response instead of leaving the connection hanging.
+ - Promise resolution in `res.send()` is now capped at a depth of 3 to prevent event loop starvation from deeply nested promise chains.
+
 ## 4.x
 > Restana version 4.x is much more simple to maintain, mature and faster!  
 

index.js

@@ -16,7 +16,10 @@ module.exports = (options = {}) => {
   options.errorHandler =
     options.errorHandler ||
     ((err, req, res) => {
-      res.send(err)
+      const statusCode = typeof (err.status || err.code || err.statusCode) === 'number'
+        ? (err.status || err.code || err.statusCode)
+        : 500
+      res.send({ code: statusCode, message: 'Internal Server Error' }, statusCode)
     })
 
   const server = options.server || require('http').createServer()
@@ -52,7 +55,7 @@ module.exports = (options = {}) => {
     },
 
     getConfigOptions () {
-      return options
+      return Object.freeze({ ...options })
     },
 
     handle,

libs/response-extensions.js

@@ -38,8 +38,10 @@ const parseErr = error => {
  * The friendly 'res.send' method
  * No comments needed ;)
  */
+const MAX_PROMISE_DEPTH = 3
+
 module.exports.send = (options, req, res) => {
-  const send = (data = res.statusCode, code = res.statusCode, headers = null, cb = NOOP) => {
+  const send = (data = res.statusCode, code = res.statusCode, headers = null, cb = NOOP, _promiseDepth = 0) => {
     let contentType
 
     if (data instanceof Error) {
@@ -76,13 +78,22 @@ module.exports.send = (options, req, res) => {
 
             data.pipe(res)
             data.on('end', cb)
+            data.on('error', () => {
+              res.end(cb)
+            })
 
             return
           } else if (Promise.resolve(data) === data) { // http://www.ecma-international.org/ecma-262/6.0/#sec-promise.resolve
-            headers = null
-            return data
-              .then(resolved => send(resolved, code, headers, cb))
-              .catch(err => send(err, code, headers, cb))
+            if (_promiseDepth >= MAX_PROMISE_DEPTH) {
+              data = stringify({ code: 500, message: 'Internal Server Error' })
+              contentType = TYPE_JSON
+              code = 500
+            } else {
+              headers = null
+              return data
+                .then(resolved => send(resolved, code, headers, cb, _promiseDepth + 1))
+                .catch(err => send(err, code, headers, cb, _promiseDepth + 1))
+            }
           } else {
             if (!contentType) contentType = TYPE_JSON
             data = stringify(data)