-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
doc(upgrade) ask our intern claude how they might write an upgrade guide #931
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a comprehensive upgrade guide for http-server version 15.0.0, documenting critical breaking changes and security considerations for users migrating from v14.1.1. The guide emphasizes security-first guidance and practical migration steps.
Key changes:
- Documents breaking IPv6 default binding change with security implications
- Provides detailed migration checklist and troubleshooting guidance
- Highlights new security risks with custom headers feature
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
**Document Version**: 1.0.0 (Matteo Collina synthesis) | ||
**Applies to**: http-server v15.0.0 | ||
**Last Updated**: 2025-10-10 | ||
**Philosophy**: Security first, pragmatic guidance, concise documentation that developers actually read. |
Copilot
AI
Oct 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The 'Philosophy' field in document metadata is unusual and subjective. Consider removing this line or incorporating the philosophy statement into the document introduction instead.
Copilot uses AI. Check for mistakes.
**Install without sudo** (recommended): | ||
```bash | ||
# Using nvm (best practice) | ||
nvm use 16.20.2 # or later |
Copilot
AI
Oct 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The specific Node.js version 16.20.2 is referenced multiple times throughout the document but is noted as 'unusually specific' and questioned. Consider using a more general version requirement like '>=16.20.2' consistently to reduce maintenance burden when the requirement changes.
Copilot uses AI. Check for mistakes.
Thank you for this @indexzero. I need to thoroughly test the instructions provided. Will get back to you soon. |
|
||
**SEVERITY: CRITICAL** | ||
|
||
The new `-H/--header` option has NO CRLF validation. This allows HTTP response splitting attacks if you pass untrusted input. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first statement, that that -H/--header
option has no CRLF validation, is true. The second statement that this allows injecting CRLF characters into headers, is not true. union
is already protecting against this:
TypeError [ERR_INVALID_HTTP_TOKEN]: Header name must be a valid HTTP token ["X-Custom: MyValue
X-Antoher: OtherValue"]
at ServerResponse.setHeader (node:_http_outgoing:701:3)
at new module.exports (/home/edube/repos/http-server/node_modules/union/lib/response-stream.js:31:21)
at new module.exports (/home/edube/repos/http-server/node_modules/union/lib/routing-stream.js:30:17)
at Server.requestHandler (/home/edube/repos/http-server/node_modules/union/lib/core.js:27:25)
at Server.emit (node:events:507:28)
at parserOnIncoming (node:_http_server:1153:12)
at HTTPParser.parserOnHeadersComplete (node:_http_common:117:17) {
code: 'ERR_INVALID_HTTP_TOKEN'
}
When #932 is merged http-server
will throw an error immediately when the configuration is invalid instead of waiting until attempting to send the first response.
### CORS Wildcard Warning | ||
|
||
`--cors` sets `Access-Control-Allow-Origin: *`, allowing ANY website to make JavaScript requests to your server and read responses. | ||
|
||
**DANGER**: | ||
- Any malicious website can access your API | ||
- Credentials in cookies will be sent | ||
- Private data becomes accessible to any origin | ||
- No CSRF protection | ||
|
||
**Only use --cors for**: | ||
- Public CDN resources (fonts, images, libraries) | ||
- Open APIs with no authentication | ||
- Development/testing (never production) | ||
|
||
**Never use --cors with**: | ||
- APIs with authentication | ||
- Any endpoint serving user-specific data | ||
- Internal resources |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an upgrade guide, should this be here? This won't cause a regression for anyone who isn't already using the flag. Maybe this belongs in the documentation for this flag.
@jelveh follow-up from our email chain with a decent (but very wordy and very
claude
sounding) Upgrade Guide for15.0.0
Excited to see this release go live π’ π β¨