Files
meshcore-analyzer/cmd/server/websocket_checkorigin_test.go
Kpa-clawbot ec0ebeda2f fix(#1793): WebSocket CheckOrigin allowlist (block cross-origin scrapers) (#1795)
## Summary

Closes the wide-open `/ws` WebSocket upgrader (`CheckOrigin: return
true`) that lets any browser origin scrape live packet data. Replaces it
with an explicit allowlist consulted from `cfg.CORSAllowedOrigins`, plus
an implicit same-origin allowance and an empty-Origin (non-browser
client) allowance.

Fixes #1793.

## Rules (`Hub.checkOrigin`)

- Empty `Origin` header → **allow** (non-browser clients; per-IP
rate/deny gating tracked separately in #1794).
- `Origin` host == request `Host` (case-insensitive) → **allow**
(same-origin).
- `Origin` matches an entry in `cfg.CORSAllowedOrigins` by exact
case-insensitive match → **allow**.
- `"*"` in `cfg.CORSAllowedOrigins` is **deliberately ignored** for
`/ws`. A startup `[ws] WARNING:` is logged once when present.
- Anything else → **reject** (gorilla returns 403).

### Deliberate divergence from CORS XHR

CORS XHR (`corsMiddleware`) still honors `"*"` for read-only
cross-origin GETs. The `/ws` upgrade does NOT, per OWASP's WebSocket
Security Cheat Sheet:

> Use an allowlist, not a denylist. Avoid wildcards or substring
matching.

—
https://cheatsheetseries.owasp.org/cheatsheets/WebSocket_Security_Cheat_Sheet.html

`"*"` on the WS path would re-open the exact CSWSH/scraping vector this
PR closes, so it is rejected with a startup warning rather than silently
honored. This intentional asymmetry is documented in the updated
`_comment_corsAllowedOrigins` in `config.example.json`.

## TDD red → green

- `e5974c6a` **RED** — adds `cmd/server/websocket_checkorigin_test.go`
with five cases; `SetAllowedOrigins` introduced as an enforcement stub
so the test compiles and fails on the assertion (CI fails on this commit
by design).
- `a4791dc3` **GREEN** — implements `Hub.checkOrigin`, wires
`SetAllowedOrigins` from `main.go`, updates the config example. All
tests pass.

## Tests added (`cmd/server/websocket_checkorigin_test.go`)

- `TestCheckOriginRejectsForeignOrigin` — foreign Origin → 403
- `TestCheckOriginAllowsEmptyOrigin` — non-browser client → 101
- `TestCheckOriginAllowsSameHost` — same-origin → 101
- `TestCheckOriginAllowsAllowlistedOrigin` — exact allowlist match → 101
- `TestCheckOriginWildcardDoesNotAllowForeignOrigin` — `"*"` in
allowlist still rejects foreign origin → 403

## Files changed

- `cmd/server/websocket.go` — `Hub.allowedOrigins`, `SetAllowedOrigins`,
`checkOrigin`, wired into `Upgrader.CheckOrigin`.
- `cmd/server/main.go` — `hub.SetAllowedOrigins(cfg.CORSAllowedOrigins)`
at the single call site.
- `cmd/server/websocket_checkorigin_test.go` — new test file.
- `config.example.json` — updated `_comment_corsAllowedOrigins` to
document `/ws` gating and the `"*"` divergence.

## Out of scope (follow-up)

- **#1794** — per-IP rate limit / deny list / connection cap for
non-browser clients (which still bypass Origin because they don't send
one). Layered defense; not in this PR.

## Verification

- `go test ./cmd/server/...` — all server tests pass locally (574s).
- Preflight clean (`bash
~/.openclaw/skills/pr-preflight/scripts/run-all.sh origin/master`).

---------

Co-authored-by: openclaw-bot <bot@openclaw.local>
2026-06-29 05:48:58 -07:00

121 lines
3.8 KiB
Go

package main
import (
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/gorilla/websocket"
)
// dialWS attempts a WebSocket upgrade against srv with the given Origin
// header. It returns the HTTP status code received (101 on success, 403 on
// rejection) and any dial error. Connection is closed if it succeeds.
func dialWS(t *testing.T, srv *httptest.Server, origin string) (int, error) {
t.Helper()
wsURL := "ws" + srv.URL[4:]
headers := http.Header{}
if origin != "" {
headers.Set("Origin", origin)
}
conn, resp, err := websocket.DefaultDialer.Dial(wsURL, headers)
if conn != nil {
defer conn.Close()
}
status := 0
if resp != nil {
status = resp.StatusCode
}
return status, err
}
func newCheckOriginServer(t *testing.T, allowed []string) (*httptest.Server, *Hub) {
t.Helper()
hub := NewHub()
hub.SetAllowedOrigins(allowed)
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
hub.ServeWS(w, r)
}))
return srv, hub
}
// TestCheckOriginRejectsForeignOrigin: the RED test that proves a
// cross-origin browser cannot upgrade /ws when not allowlisted.
// (#1793)
func TestCheckOriginRejectsForeignOrigin(t *testing.T) {
srv, _ := newCheckOriginServer(t, nil)
defer srv.Close()
status, err := dialWS(t, srv, "https://evil.example.com")
if err == nil {
t.Fatalf("expected upgrade rejection for foreign origin, got success (status=%d)", status)
}
if status != http.StatusForbidden {
t.Fatalf("expected 403 Forbidden for foreign origin, got status=%d err=%v", status, err)
}
}
func TestCheckOriginAllowsEmptyOrigin(t *testing.T) {
srv, _ := newCheckOriginServer(t, nil)
defer srv.Close()
status, err := dialWS(t, srv, "")
if err != nil {
t.Fatalf("expected upgrade success for empty Origin (non-browser client), got status=%d err=%v", status, err)
}
if status != http.StatusSwitchingProtocols {
t.Fatalf("expected 101 Switching Protocols, got %d", status)
}
}
func TestCheckOriginAllowsSameHost(t *testing.T) {
srv, _ := newCheckOriginServer(t, nil)
defer srv.Close()
// httptest.NewServer uses srv.URL host, e.g. 127.0.0.1:PORT.
hostURL := "http" + srv.URL[4:] // strip "ws" was N/A — srv.URL is http://
if !strings.HasPrefix(srv.URL, "http://") {
t.Fatalf("unexpected srv URL: %s", srv.URL)
}
sameOrigin := hostURL[:len(hostURL)] // same scheme+host as the dial target
status, err := dialWS(t, srv, sameOrigin)
if err != nil {
t.Fatalf("expected upgrade success for same-host origin %s, got status=%d err=%v", sameOrigin, status, err)
}
if status != http.StatusSwitchingProtocols {
t.Fatalf("expected 101 Switching Protocols, got %d", status)
}
}
func TestCheckOriginAllowsAllowlistedOrigin(t *testing.T) {
allow := []string{"https://embed.example.com"}
srv, _ := newCheckOriginServer(t, allow)
defer srv.Close()
status, err := dialWS(t, srv, "https://embed.example.com")
if err != nil {
t.Fatalf("expected upgrade success for allowlisted origin, got status=%d err=%v", status, err)
}
if status != http.StatusSwitchingProtocols {
t.Fatalf("expected 101 Switching Protocols, got %d", status)
}
}
// TestCheckOriginWildcardDoesNotAllowForeignOrigin asserts the deliberate
// divergence from CORS: "*" in the allowlist does NOT permit cross-origin
// WebSocket upgrades. OWASP WebSocket Security Cheat Sheet — avoid wildcards
// in CSWSH defense. See PR for citation.
func TestCheckOriginWildcardDoesNotAllowForeignOrigin(t *testing.T) {
srv, _ := newCheckOriginServer(t, []string{"*"})
defer srv.Close()
status, err := dialWS(t, srv, "https://evil.example.com")
if err == nil {
t.Fatalf("expected upgrade rejection for foreign origin even when allowlist=[\"*\"], got success (status=%d)", status)
}
if status != http.StatusForbidden {
t.Fatalf("expected 403 Forbidden, got status=%d err=%v", status, err)
}
}