Repository navigation
fix(proxy): stop a "*" origin setting from waiving the WebSocket Origin check - #4846
chopratejas wants to merge 6 commits into
Conversation
…in check The WebSocket routes (/v1/responses, /v1/live) accept an Origin from HEADROOM_WS_ORIGINS, falling back to HEADROOM_CORS_ORIGINS. A "*" entry in either made every Origin acceptable, so loosening CORS for a remote dashboard also let any page in the user's browser open these sockets. "*" now matches nothing; a list that holds only "*" keeps the loopback default, and explicit origins keep working. Assay-Kind: bugfix Assay-Ask: Setting HEADROOM_CORS_ORIGINS to "*" silently lets any browser page open the proxy's credentialed WebSocket routes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR governanceThis PR does not yet satisfy the required template fields:
Please update the PR body, or move the PR back to draft while it is still in progress. |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Picks up the dependency updates from #4835 so the audit checks pass. Assay-Kind: deps Assay-Ask: The dependency audit checks failed on this PR because the branch predates the dependency updates on main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
JerrettDavis
left a comment
There was a problem hiding this comment.
The origin-policy change looks correct: isolated checks pass 13 cases, including rejecting foreign origins under either wildcard setting, preserving loopback defaults, and retaining explicit-origin matching. The same checks reproduce five wildcard-policy mismatches on recorded main. No changed-code defect found. Full handler/live validation was not independently run. Review qualification remains held by the inherited shared workflow audit. CI also reports commitlint failing; its diagnostic is needed before proposing a concrete repair.
JerrettDavis
left a comment
There was a problem hiding this comment.
The origin-policy source change looks correct, and the prior 13 policy checks remain applicable at this unchanged head. The outstanding CI failure is now diagnosed: commitlint rejects the merge commit c14e46a because its Assay-Ask footer is 116 characters (limit 100). The original fix commit 190eabf also has a 126-character Assay-Ask footer. Reproduce by inspecting git log main..HEAD --format=%B and counting those footer lines. Please wrap both footers to at most 100 characters, preserving their content, then obtain a passing commitlint check. These commits are unique to the PR, so this is separate from inherited workflow provenance issues. Full handler/live validation was not independently rerun.
Sync PR #4846 with latest main.
Sync PR #4846 with latest main.
JerrettDavis
left a comment
There was a problem hiding this comment.
The origin-policy delta still looks correct against updated main. The previously requested commit-message repair remains unresolved: current CI run 43547199764 reports footer-max-line-length. Reproduce with git log main..HEAD --format=%B: Assay-Ask lines in c14e46a and 190eabf are 116 and 126 characters, exceeding 100. Please wrap both footers while preserving their content and obtain a passing commitlint check. The latest merge retains both offending commits. No new source defect found; earlier-head policy tests are context only, and full handler/live validation was not independently rerun.
|
Jerrett, the commitlint blocker is cleared. The footer-length rule fix (#4868) is merged into main, and I merged main into this branch, so commitlint now passes on this head. No code changes in this update. |
## Description The provider routes (`/v1/messages`, `/anthropic/v1/messages`, `/v1/chat/completions`, `/v1/responses`, Bedrock/Vertex routes and the catch-all passthrough) add operator-held credentials to what they forward: `*_extra_headers`, the Copilot token (`apply_copilot_api_auth`), and the provider env key on the litellm/anyllm backends when the request carries none. Loopback callers skip `HEADROOM_PROXY_TOKEN`, and a page open in the user's browser reaches the proxy as a loopback caller. It can send a JSON body as `Content-Type: text/plain`, which needs no CORS preflight, and the proxy parses and forwards it. CORS only stops the page from reading the answer, not the proxy from sending the request. Only the WebSocket routes checked `Origin`. The security gate now applies the WebSocket Origin rule to every HTTP request, before routing. A request is refused with 403 when it carries an `Origin` that is not: - a loopback page (`http(s)://127.0.0.1|localhost|[::1]` on any port; this is the dashboard), - an origin listed in `HEADROOM_WS_ORIGINS` / `HEADROOM_CORS_ORIGINS` (the same list `_is_allowed_websocket_origin` honours), - for a non-loopback caller only, the proxy's own address (a remote dashboard behind `HEADROOM_PROXY_TRUSTED_DASHBOARD_CLIENT_CIDRS`). A loopback caller gets no same-origin pass, since a non-loopback `Host` with a loopback peer means a page's hostname was pointed at 127.0.0.1. A cross-site subresource load (`<img>`, `<script>`) sends no `Origin` on GET, so `Sec-Fetch-Site: cross-site` is refused too, except for a top-level navigation. Requests without `Origin` (Claude Code, Codex, Copilot's extension host, SDKs, curl) are unaffected. Because the check sits in the gate, it covers every backend, including the litellm/anyllm env-key fallback, without changing how a keyless request from a local tool is authenticated. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [x] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/proxy/server.py`: `_is_refused_browser_request(request)` and a step 0 in `_security_gate` that answers 403 before the token check and routing. The 403 comes with the usual security headers and a hint naming `HEADROOM_WS_ORIGINS`, and an `event=proxy_browser_origin_rejected` log line records the escaped Origin. An origin listed in either `HEADROOM_WS_ORIGINS` or `HEADROOM_CORS_ORIGINS` is accepted, even when both are set. - `tests/test_proxy_browser_origin.py`: text/plain POSTs to `/v1/messages` and `/v1/chat/completions` from a loopback peer with a foreign / `null` / look-alike Origin never reach the upstream; without Origin, from a loopback page, and from a configured origin they still forward with the operator header; cross-site subresource GET refused; cross-site navigation still served; loopback peer with a re-pointed hostname refused. - `tests/test_proxy_loopback_gating.py`: two existing tests that sent a foreign `Origin` to `/stats` and `/metrics` now expect the gate's 403 (they already asserted refusal / no metadata; the gate now refuses earlier). - `docs/content/docs/security-model.mdx`: describes the rule, and lists the setups that need an explicit origin: a reverse proxy on the same host, and app-scheme desktop shells. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text # new test file on origin/main (before the fix) $ pytest tests/test_proxy_browser_origin.py FAILED ...test_foreign_page_request_is_refused_before_the_upstream[browser_headers0-/v1/chat/completions] FAILED ...test_foreign_page_request_is_refused_before_the_upstream[browser_headers0-/v1/messages] FAILED ...test_foreign_page_request_is_refused_before_the_upstream[browser_headers1-/v1/chat/completions] FAILED ...test_foreign_page_request_is_refused_before_the_upstream[browser_headers1-/v1/messages] FAILED ...test_foreign_page_request_is_refused_before_the_upstream[browser_headers2-/v1/chat/completions] FAILED ...test_foreign_page_request_is_refused_before_the_upstream[browser_headers2-/v1/messages] FAILED ...test_configured_origin_still_forwards FAILED ...test_cross_site_subresource_without_origin_is_refused ========================= 8 failed, 5 passed in 11.06s ========================= # with the fix $ pytest tests/test_proxy_browser_origin.py tests/test_proxy_loopback_gating.py tests/test_proxy_cors.py ======================= 175 passed, 5 warnings in 34.36s ======================= # wider sweep: tests/test_proxy*.py tests/test_proxy/ tests/test_header_isolation.py # tests/test_upstream_credential_scoping.py tests/test_openai_codex_routing.py # tests/test_dashboard_* tests/test_copilot* (-k "not playwright") = 12 failed, 2368 passed, 114 skipped, 30 deselected in 483.68s = # the 12 failures (tests/test_proxy_uds.py x11, one test_proxy_compress_endpoint lossy-inline case) # fail identically on origin/main in this environment $ ruff check headroom/proxy/server.py tests/test_proxy_browser_origin.py tests/test_proxy_loopback_gating.py All checks passed! $ mypy headroom/proxy/server.py --ignore-missing-imports Success: no issues found in 1 source file ``` ## Real Behavior Proof - Environment: macOS, Python 3.12. `headroom proxy --host 127.0.0.1 --no-optimize --anthropic-api-url http://127.0.0.1:<L> --anthropic-extra-headers '{"x-api-key":"OPERATOR_KEY_SENTINEL"}'`, no proxy token, with a small HTTP listener on `<L>` that logs each request's `x-api-key` and prompt. One run on origin/main, one on this branch. - Exact command / steps: three `curl -X POST http://127.0.0.1:<port>/v1/messages -H 'Content-Type: text/plain' --data '<messages JSON>'`, with `Origin: http://page.example`, with `Origin: http://127.0.0.1:<port>`, and with no Origin. - Observed result: ```text == origin/main Origin: http://page.example -> HTTP 200 Origin: http://127.0.0.1:18961 -> HTTP 200 (no Origin) -> HTTP 200 listener saw: {"path": "/v1/messages", "x-api-key": "OPERATOR_KEY_SENTINEL", "prompt": "request-1 (Origin: http://page.example)"} {"path": "/v1/messages", "x-api-key": "OPERATOR_KEY_SENTINEL", "prompt": "request-2 (Origin: http://127.0.0.1:18961)"} {"path": "/v1/messages", "x-api-key": "OPERATOR_KEY_SENTINEL", "prompt": "request-3 (X-No-Origin: 1)"} == this branch Origin: http://page.example -> HTTP 403 Origin: http://127.0.0.1:18962 -> HTTP 200 (no Origin) -> HTTP 200 listener saw: {"path": "/v1/messages", "x-api-key": "OPERATOR_KEY_SENTINEL", "prompt": "request-2 (Origin: http://127.0.0.1:18962)"} {"path": "/v1/messages", "x-api-key": "OPERATOR_KEY_SENTINEL", "prompt": "request-3 (X-No-Origin: 1)"} ``` Dashboard on the same branch: `GET /dashboard` as a cross-site navigation -> 200; same-origin `POST /settings` -> 200; same-origin `GET /stats` -> 200. - Not tested: a real Copilot upstream (the refusal happens before any handler, so it is the same for every route). ## Runtime Rollout Safety - Rollout-managed feature(s): none - Minimum rollout channel: n/a - Stable/default behavior changed: HTTP requests carrying an `Origin` that is neither loopback, configured, nor (for a remote caller) the proxy's own address now get 403. A browser app on another origin that calls the proxy directly must be listed in `HEADROOM_WS_ORIGINS` (or `HEADROOM_CORS_ORIGINS`, which also governs CORS). Electron/webview renderers that send `Origin: null` or an app scheme are refused unless listed. - Kill switch / disable path: list the origin in `HEADROOM_WS_ORIGINS`. - Unsafe override required: no - Qualification impact: none - Rollback path: revert this commit ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I did **not** edit `CHANGELOG.md` — it is generated by release-please from my Conventional Commit PR title (a CI guard enforces this) ## Additional Notes A companion PR (#4846) stops a `*` entry in `HEADROOM_WS_ORIGINS` / `HEADROOM_CORS_ORIGINS` from accepting every Origin. Both PRs share `_is_allowed_websocket_origin`, so once both land `*` is ignored on HTTP too; until then, `HEADROOM_CORS_ORIGINS=*` keeps HTTP open to every Origin, as it keeps the WebSocket routes today. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
66a90fb to
ac6bd70
Compare
|
Applied the requested mechanical fix: rewrote the PR-range commits to wrap the oversized Assay-Ask footer(s) to under the 100-character commitlint limit (content preserved, wrapped at word boundaries). Branch force-pushed; commitlint and CI are re-running at the new head. |
With the HTTP browser-origin check on main and "*" no longer matching any Origin, a foreign page's preflight under HEADROOM_CORS_ORIGINS=* is refused by the security gate (403) instead of getting the wildcard answer. That is the intended combined behaviour: "*" widens which pages may read a response, not which pages may drive the proxy. The test now pins that, and that a loopback page still gets "*" without credentials. Assay-Kind: tests Assay-Ask: The CORS wildcard test still expects a foreign page to be answered after "*" stopped waiving the browser-origin check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Jerrett, after main was merged in, CI failed on I merged origin/main into this branch (aaf8648), then updated the test in c5d935e. It now pins three things under |
Description
The WebSocket routes (
/v1/responses,/v1/live) check the browserOriginagainstHEADROOM_WS_ORIGINS, falling back toHEADROOM_CORS_ORIGINS. A*entry in either made every Origin acceptable.HEADROOM_CORS_ORIGINS=*is a documented value for CORS (which pages may read a response), so loosening CORS for a remote dashboard also let any page open in the user's browser open these sockets, and the proxy then adds its own credentials upstream.*now matches nothing in the Origin check. A list that holds only*keeps the loopback default; explicit origins in either variable keep working. CORS itself is unchanged.Type of Change
Changes Made
headroom/proxy/handlers/openai.py:_allowed_ws_origins_from_envdrops*entries (empty result means the loopback default);_is_allowed_websocket_originno longer short-circuits toTrueon*.providers/codex/live.pyuses the same helper, so/v1/liveis covered too.tests/test_openai_codex_routing.py: policy test for*via either variable and mixed with an explicit origin, plus a handler test that a foreign Origin is closed with 1008 underHEADROOM_CORS_ORIGINS=*.Testing
pytest)ruff check .)mypy headroom)Test Output
Real Behavior Proof
headroom proxy --host 127.0.0.1 --no-optimizestarted withHEADROOM_CORS_ORIGINS='*', one run on origin/main and one on this branch.ws://127.0.0.1:<port>/v1/responseswith thewebsocketsclient, once withOrigin: https://page.exampleand once withOrigin: http://127.0.0.1:<port>./v1/liveend to end (same helper, covered by the unit tests' shared code path).Runtime Rollout Safety
*inHEADROOM_WS_ORIGINSorHEADROOM_CORS_ORIGINS. Native clients (Codex, SDKs) send no Origin and are unaffected; loopback pages still connect.HEADROOM_WS_ORIGINS.Review Readiness
Checklist
CHANGELOG.md— it is generated by release-please from my Conventional Commit PR title (a CI guard enforces this)Additional Notes
HEADROOM_WS_ORIGINSis not documented anywhere, so there was no doc to update. A companion PR applies the same Origin rule to the HTTP routes; once both land,*is ignored there too because they share_is_allowed_websocket_origin.🤖 Generated with Claude Code
Reported by @JoaoMarcos44. Thank you for your contribution! 🩵