feat(decap-server): Allow the server to be run securely #7579
Conversation
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
There was a problem hiding this comment.
Pull Request Overview
This PR adds security configuration options to the decap-server to allow it to be run more securely by introducing host binding and CORS origin restrictions. The changes enable developers to restrict server access to localhost only and limit API requests to specific origins.
- Added
BIND_HOSTenvironment variable to control which IP address the server binds to - Added
ORIGINenvironment variable to configure CORS origin restrictions instead of allowing all origins - Fixed type issue with port parsing to ensure it's always a number
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/decap-server/src/index.ts | Added host binding configuration and port parsing fix |
| packages/decap-server/src/middlewares/common/index.ts | Updated CORS configuration to use ORIGIN environment variable |
| packages/decap-server/README.md | Added documentation for new environment variables |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@palant run |
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.
Summary
The documentation currently says:
This is fair but it is also unnecessary. The server can be restricted in such a way that it is safe to use.
I introduced two optional environment variables to configure
decap-server. TheBIND_HOSTvariable allows binding to127.0.0.1rather than all IP addresses. AndORIGINallows restricting CORS responses to a specific origin rather that allowing the API to be accessed from any server.Note that I would have preferred making
BIND_HOST=localhostthe default. I didn’t want to introduce backwards incompatible changes however.Test plan
Running
npm run startstill allows connecting to bothhttp://localhost:8081/andhttp://<public_ip>:8081>/. The responses contain the HTTP headerAccess-Content-Allow-Origin: *as before this change.Running
BIND_HOST=localhost npm run startallows connecting tohttp://localhost:8081/but connections tohttp://<public_ip>:8081>/are no longer possible. It’s a purely local server now.Running
ORIGIN=https://example.com npm run startchanges the HTTP header in responses toAccess-Content-Allow-Origin: https://example.com. The server can no longer be accessed by arbitrary websites.Tests complain about
portbeingnumber | stringwhereasnumberis expected. This is unrelated to my change but I’ve fixed it.Checklist
Please add a
xinside each checkbox:I’ve also noticed that the script required fornpm run formatdoesn’t actually exist. I guessnpm run lintis meant here which for me errors out due to two warnings unrelated to my changes – but for some reason it succeeds in the CI runs (are these running an older lint version?).🐈