Fix localhost bridge cross-origin file read and WebSocket hijack#44
Conversation
There was a problem hiding this comment.
Pull request overview
Addresses a localhost bridge security issue by restricting which browser origins can interact with Rose’s local HTTP/WebSocket server and by hardening path handling for asset/mod related flows.
Changes:
- Added loopback-only Origin validation helpers and used them to restrict HTTP bridge requests and WebSocket upgrades.
- Removed wildcard CORS behavior and restricted
/asset/resolution to Rose’s bundledassetsdirectory. - Hardened path validation in WebSocket message flows (UNC/absolute/traversal checks; URL-encoding for asset URLs; stricter League path validation).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/core/security.py | Introduces loopback-origin detection and CORS header construction helpers. |
| utils/core/paths.py | Adds get_assets_dir() and hardens get_asset_path() to prevent escaping the assets directory. |
| pengu/core/websocket_server.py | Blocks WebSocket upgrade requests when a non-loopback Origin is present. |
| pengu/core/http_handler.py | Enforces loopback-only Origin access for HTTP routes and removes Access-Control-Allow-Origin: *. |
| pengu/communication/message_handler.py | Tightens path validation for “other mod” selection, encodes asset URLs, and blocks unsafe League install paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _forbidden(self) -> tuple: | ||
| """Return a generic forbidden response.""" | ||
| return (403, {"Content-Type": "text/plain"}, b"Forbidden") |
There was a problem hiding this comment.
_forbidden() always returns a 403 without the computed cors_headers. For requests coming from an allowed loopback Origin, this results in a CORS-blocked response in the browser (the JS caller can’t read the 403/500 body/headers), even though the Origin itself is permitted. Consider threading cors_headers into _forbidden() (and the 500 handler) for the allowed-origin case, while still omitting CORS headers when the Origin is rejected.
| """ | ||
| Security helpers for Rose's localhost bridge. | ||
| """ |
There was a problem hiding this comment.
The PR description mentions adding regression tests for the new cross-origin / WebSocket / path validation behavior, but there don’t appear to be any test files added/updated in this change set. Either add the promised tests (or clarify where they live/how to run them), or update the PR description to avoid implying test coverage that isn’t present.
Summary
Fixes a localhost bridge security issue where arbitrary websites could interact with Rose's local HTTP/WebSocket server while Rose was running.
This patch:
Originheaders for HTTP bridge requestsOriginheaders during WebSocket upgrade/asset/resolution to files inside Rose's bundledassetsdirectorySecurity impact
Before this change, a page such as
https://attacker.examplecould read local files through:and could also open a WebSocket to Rose's local bridge and send unauthenticated commands.
After this change, those cross-origin browser requests are rejected.
Also, you should double check if nothing got broken since I can't compile the binary on my end