Fail closed on web server WebSocket origins#112
Conversation
|
Web preview: https://pr-112-charm-preview.justin-tech.workers.dev Built with |
There was a problem hiding this comment.
Pull request overview
This PR hardens charm-web-server’s WebSocket origin validation so misconfiguration of CHARM_WEB_SERVER_ALLOWED_ORIGIN no longer silently degrades to “allow any origin”, and updates tests/docs to reflect the new deployment contract.
Changes:
- Make
/api/wsfail closed whenCHARM_WEB_SERVER_ALLOWED_ORIGINis unset (reject upgrades rather than allowing any origin). - Adjust the raw-body origin guard so HTTP requests without an
Originheader can still work, while requests with anOriginheader are allowlist-checked. - Update
http_apiintegration tests and the crate README to document the new required configuration for real deployments.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/charm-web-server/src/routes.rs | Switch WS origin behavior to fail-closed when the allowlist env var is unset; relax raw-body guard to permit requests that truly lack Origin. |
| crates/charm-web-server/tests/http_api.rs | Add/adjust WS integration tests around origin behavior; refactor WS request construction. |
| crates/charm-web-server/README.md | Document the stricter origin allowlist contract and explicitly require CHARM_WEB_SERVER_ALLOWED_ORIGIN for deployments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d05b71940
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
CHARM_WEB_SERVER_ALLOWED_ORIGINis unset instead of allowing any originOriginheader working while still rejecting disallowed originsCHARM_WEB_SERVER_ALLOWED_ORIGINexplicitly; this repo does not currently include the production compose/systemd config to verifyRelated
Tests
cargo fmt --checkCARGO_TARGET_DIR=.target-codex cargo test -p charm-web-server --test http_api -- --test-threads=1CARGO_TARGET_DIR=.target-codex cargo test -p charm-web-serverNote: I also attempted
CARGO_TARGET_DIR=.target-codex cargo clippy -p charm-web-server -- -D warnings, but invalidated that run by deleting the generated.target-codexdirectory while clippy was still using it. The completed package test above passed before that cleanup mistake.