← Back to PRs

#10930: fix: validate WebSocket Origin for all client types, not just browser UIs

by OneZeroEight-ai open 2026-02-07 06:06 View on GitHub →
gateway stale
## Summary - **Origin validation now keys on the `Origin` header presence**, not the self-reported `client.id` - Browsers always send `Origin` on WebSocket upgrades (cannot be suppressed by JS); non-browser clients (CLI, native apps) typically omit it - Closes a client-type spoofing vector where a malicious webpage could claim to be a CLI client to bypass origin checks ## The vulnerability In `message-handler.ts`, origin validation only ran for connections where `connectParams.client.id` matched Control UI or WebChat: ```typescript // Before (vulnerable) const isControlUi = connectParams.client.id === GATEWAY_CLIENT_IDS.CONTROL_UI; const isWebchat = isWebchatConnect(connectParams); if (isControlUi || isWebchat) { // origin check } ``` Since `client.id` is self-reported in the connect params, a malicious webpage could: 1. Open a WebSocket to `ws://127.0.0.1:18789` (the local gateway) 2. Send a connect message with `client.id: "cli"` and `client.mode: "cli"` 3. Bypass origin validation entirely ## The fix ```typescript // After (fixed) if (requestOrigin) { // origin check — applies to ALL connections with an Origin header } ``` If a connection includes an `Origin` header, it's validated regardless of client type. The existing `checkBrowserOrigin()` function handles all the right cases: - Same-origin → allowed - Loopback-to-loopback → allowed (preserves local dev with Vite/webpack dev servers) - In `allowedOrigins` allowlist → allowed - Everything else → rejected ## Tests added **Unit tests** (`origin-check.test.ts`): - Cross-origin attack on localhost gateway (evil.com → 127.0.0.1) → rejected - `null` origin string (sandboxed iframes) → rejected **E2E tests** (`server.auth.e2e.test.ts`): - CLI-spoofing connection with cross-origin header → rejected - Loopback origin with non-browser client type → still allowed (dev workflow preserved) ## Test plan - [x] `vitest run src/gateway/origin-check.test.ts` — 7/7 passing - [x] `vitest run src/gateway/auth.test.ts` — 7/7 passing - [ ] Full E2E suite (`vitest run --config vitest.e2e.config.ts`) - [ ] Manual: open browser console on `https://evil.com`, try `new WebSocket("ws://localhost:18789")` — should be rejected after connect 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> - Changes WebSocket Origin enforcement to trigger on `Origin` header presence instead of self-reported `client.id`, closing a spoofing bypass for browser-initiated connections. - Keeps existing `checkBrowserOrigin()` logic (same-origin, loopback-to-loopback dev allowance, allowlist) but applies it to any connection with an Origin header. - Adds unit tests for localhost cross-origin and `Origin: null` rejection. - Adds E2E coverage for CLI-spoofing with cross-origin Origin and for allowing loopback origins across client types. <h3>Confidence Score: 4/5</h3> - This PR is mostly safe to merge, but the new E2E tests can hang indefinitely in failure cases and should be made deterministic. - Core logic change is small and aligns with the stated security intent, and unit tests cover new origin cases. The main concern is test reliability: both new E2E tests await a WebSocket `open` event without handling `error`/`close`, so a correct rejection path can stall CI. - src/gateway/server.auth.e2e.test.ts <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs