feat: security hardening pass for v6.0.0#121
Merged
jkyberneees merged 6 commits intomasterfrom Apr 25, 2026
Merged
Conversation
- Remove TRACE HTTP method by default; add enableTrace option for debugging - Block forbidden headers (transfer-encoding, content-length, connection, keep-alive, host, set-cookie) in res.send() headers parameter - Gracefully handle invalid header key characters (CRLF, newlines) - Suppress error details in production via parseErr() NODE_ENV check - Set default security headers: X-Content-Type-Options, X-Frame-Options, X-XSS-Protection, and Strict-Transport-Security (on HTTPS) - Add securityHeaders: false option to opt out of default headers - Extract security headers logic into reusable libs/security-headers.js - Deep-freeze nested plain objects in getConfigOptions() - Update APM base to use static methods list - Add comprehensive security review tests (38 tests) BREAKING CHANGE: TRACE method disabled by default, res.send() now validates headers, production error masking on res.send(err), and getConfigOptions() deep-freezes nested plain objects.
- Update package.json engines.node to >=24.x - Update GitHub Actions workflow matrix to 24.x - Update .travis.yml to test against 24.x - Update .devcontainer.json to typescript-node:1-24 - Update v6.0 changelog with new minimum requirement BREAKING CHANGE: Node.js >=24 is now required.
There was a problem hiding this comment.
Pull request overview
Security hardening release for Restana v6.0.0, adding safer defaults (headers, TRACE opt-in, production error masking) and introducing a new security regression test suite to codify the expected behavior.
Changes:
- Add default security headers (opt-out via
securityHeaders: false) and extract logic intolibs/security-headers.js. - Harden
res.send(..., headers)by blocking sensitive/hop-by-hop headers and skipping invalid header keys/values without crashing. - Disable
TRACEmethod registration by default (opt-in viaenableTrace: true) and update method plumbing/APM integration accordingly.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/security-review-2026.test.js | Adds a comprehensive security regression test suite for the new hardened behaviors. |
| package.json | Bumps package version to v6.0.0. |
| libs/security-headers.js | New helper applying default security headers (including conditional HSTS). |
| libs/response-extensions.js | Header validation/forbidden header blocking in res.send(), plus production masking for res.send(err). |
| libs/request-router.js | Switches to method list factory so TRACE can be conditionally exposed. |
| libs/methods.js | Replaces static array export with options-aware method list + exports BASE/TRACE constants. |
| libs/apm-base.js | Uses the static BASE methods list for patching. |
| index.js | Applies default security headers per-request and updates getConfigOptions() to freeze nested objects. |
| docs/README.md | Documents v6 security defaults and new options (enableTrace, securityHeaders), plus breaking changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
getConfigOptions() was freezing nested plain objects in place via Object.freeze(val), which also froze the user's original objects since the spread copy is shallow. Now deep-clones with JSON.parse(JSON.stringify()) before freezing, so the returned config is immutable but the caller's originals remain mutable.
getConfigOptions() only froze the first level of nested plain objects (e.g., customNested was frozen but customNested.nested was still mutable). Replaced Object.freeze() with a recursive deepFreezePlain() that walks all depths, matching the documented 'deep freeze' behavior.
Move the deep clone logic (JSON.parse/JSON.stringify) out of index.js and into a reusable deepObjectClone utility in libs/utils.js alongside forEachObject.
…prove prod error messages - Move recursive deep-freeze logic into libs/utils.js as deepFreezeObject() — deep-clones, recursively freezes all nesting levels, safe for non-plain values (pass-through) - Cache frozen config in getConfigOptions() after first call to avoid re-cloning and re-freezing on every invocation - Use http.STATUS_CODES in parseErr() production mode so error messages match the status code (e.g. 404 → 'Not Found', 502 → 'Bad Gateway') instead of always 'Internal Server Error' - Fix documented default errorHandler to match actual statusCode computation instead of undefined 'code' variable
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
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.
BREAKING CHANGE: TRACE method disabled by default, res.send() now validates headers, production error masking on res.send(err), and getConfigOptions() deep-freezes nested plain objects.