From ce133d8c3f387c1af30960ae8b82e0a5ba18fb17 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Sat, 18 Jul 2026 14:10:16 +0200 Subject: [PATCH] fix(serve): stop serving WS token over HTTP and warn on non-loopback bind (H9) - The per-instance WebSocket token is now printed to the console only, like a Jupyter token. - handleStatic only injects the token cookie/meta tag when the request includes ?token=. - A plain GET / returns the UI with an empty token, so unauthenticated visitors cannot retrieve the credential. - A loud warning is printed when the bind address is not loopback. - Update tests, AGENTS.md, and docs/SECURITY.md. --- AGENTS.md | 2 +- cmd/odek/serve.go | 70 +++++++++++++++------ cmd/odek/serve_approval_test.go | 3 + cmd/odek/serve_test.go | 105 +++++++++++++++++++++++++------- docs/SECURITY.md | 14 +++++ 5 files changed, 154 insertions(+), 40 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 06a007c..940a120 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -114,7 +114,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU - **FD-based API key handoff** (`cmd/odek/subagent_key.go`) — parent writes key to a 0600 tempfile, immediately `unlink()`s, passes the FD via `cmd.ExtraFiles`. Sub-agent reads from `$ODEK_API_KEY_FD` and closes. Key never in `/proc//environ`. - **Approver friction** (`internal/danger/approver.go`, `cmd/odek/wsapprover.go`) — both TTYApprover and WSApprover engage friction mode after 3 approvals of the same class in 60s: require typing literal `approve`, 1.5s pause. Trust-class shortcut disabled for `destructive` + `blocked` regardless. - **Danger classifier bypass resistance** (`internal/danger/classifier.go`) — `normalize()` pre-processes: expand `$IFS` / `${IFS}`, extract `$(...)` / `` `...` `` substitutions, strip `command` / `exec` / `builtin` wrappers, collapse unquoted backslashes, basename absolute paths. `awk`/`gawk`, `sed` (`e` command / `-f`), and editors (`vi`/`vim`/`nvim`/`emacs`/`ed`/`ex`) are classified as `code_execution` when given a script or file operand, closing `awk 'BEGIN{system(...)}'`, `sed 's///e'`, and editor `!` shell escapes. Regression suite in `classifier_bypass_test.go`. -- **WebSocket CSRF token** (`cmd/odek/serve.go`) — `odek serve` issues a random 256-bit token at startup, injects it into `index.html`, sets an `HttpOnly` `SameSite=Strict` cookie, and requires it on `/ws` via cookie, `X-Odek-Ws-Token` header, or `odek.` subprotocol. The localhost origin check remains as defense-in-depth. +- **WebSocket CSRF token** (`cmd/odek/serve.go`) — `odek serve` issues a random 256-bit token at startup and requires it on `/ws` via cookie, `X-Odek-Ws-Token` header, or `odek.` subprotocol. The token is no longer embedded in every `GET /` response; it is only delivered when the request includes the correct `?token=` query parameter (Jupyter-style), and a non-loopback bind prints a loud warning. The localhost origin check remains as defense-in-depth. - **SSRF / DNS-rebinding dial guard** (`cmd/odek/ssrf_guard.go`) — `browser`, `http_batch`, and `web_search` resolve hostnames at dial time and refuse internal IPs, then pin the dial to the validated IP. Operator-configured backends (e.g. `web_search.base_url`) are added to an allowlist so container-internal services such as SearXNG remain reachable. - **REST API CSRF protection** (`cmd/odek/serve.go::requireLocalOrigin`) — state-changing HTTP endpoints (POST/PUT/PATCH/DELETE) require a localhost origin or no Origin header, and static responses set `X-Frame-Options: DENY` + `Content-Security-Policy: frame-ancestors 'none'` to block clickjacking. - **Browser history cap** (`cmd/odek/browser_tool.go`) — navigation history is capped at 50 snapshots to prevent memory DoS from repeated `browser_navigate` calls. diff --git a/cmd/odek/serve.go b/cmd/odek/serve.go index 14bcac2..4354516 100644 --- a/cmd/odek/serve.go +++ b/cmd/odek/serve.go @@ -322,12 +322,24 @@ func serveCmd(args []string) error { return fmt.Errorf("listen: %w", err) } - fmt.Fprintf(os.Stderr, "odek serve ⚡ http://%s\n", listener.Addr()) + // Print the WebSocket token to the console only (Jupyter-style). It is + // never served over plain HTTP, so a network attacker who can only make + // `GET /` cannot retrieve it. Browser clients get it via the token URL; + // non-browser clients can supply it as a header or subprotocol. + tokenURL := "http://" + listener.Addr().String() + "/?token=" + wsToken + fmt.Fprintf(os.Stderr, "odek serve ⚡ %s\n", tokenURL) fmt.Fprintf(os.Stderr, " WebSocket: ws://%s/ws\n", listener.Addr()) + fmt.Fprintf(os.Stderr, " WS token: %s\n", wsToken) fmt.Fprintf(os.Stderr, " Type @ to reference files, drop or attach files inline.\n") + if !isLoopbackAddr(listener.Addr()) { + fmt.Fprintf(os.Stderr, "\n⚠️ WARNING: odek serve is bound to a non-loopback address.\n") + fmt.Fprintf(os.Stderr, " Anyone who can reach this port can control the agent once they have the token above.\n") + fmt.Fprintf(os.Stderr, " Use --addr 127.0.0.1: (or a firewall) to restrict access.\n\n") + } + if openBrowser { - openInBrowser("http://" + listener.Addr().String()) + openInBrowser(tokenURL) } return serveOnListener(listener, mux) @@ -539,11 +551,11 @@ func newServeAgent(resolved config.ResolvedConfig, system string, sendFn func(v SystemMessage: system, UntrustedWrapper: func(source, content string) string { return wrapUntrusted(context.Background(), source, content) }, RuntimeContext: runtimeCtx, - NoProjectFile: resolved.NoAgents, - Thinking: resolved.Thinking, - InteractionMode: resolved.InteractionMode, - Tools: tools, - ToolFilter: odek.ToolFilterConfig{Enabled: resolved.Tools.Enabled, Disabled: resolved.Tools.Disabled}, + NoProjectFile: resolved.NoAgents, + Thinking: resolved.Thinking, + InteractionMode: resolved.InteractionMode, + Tools: tools, + ToolFilter: odek.ToolFilterConfig{Enabled: resolved.Tools.Enabled, Disabled: resolved.Tools.Disabled}, // SandboxCleanup is intentionally NOT passed here. In serve mode, // cleanup is the caller's responsibility (handleWS defers it). // Passing it here would cause agent.Close() to call docker rm -f, @@ -552,7 +564,7 @@ func newServeAgent(resolved config.ResolvedConfig, system string, sendFn func(v Renderer: nil, // silent — we stream via WebSocket Skills: &resolved.Skills, SkillManager: sm, - MemoryConfig: resolved.Memory, + MemoryConfig: resolved.Memory, MemoryDir: expandHome("~/.odek/memory"), Guard: injectionGuard, GuardConfig: resolved.Guard, @@ -1647,19 +1659,30 @@ func handleStatic(wsToken string) http.HandlerFunc { return } - // The HTML entry point gets the per-instance CSRF token both as a + // The HTML entry point only receives the per-instance CSRF token when + // the request presents the token in the `?token=` query parameter. This + // prevents a network attacker from retrieving the token with a simple + // `GET /`. The token is delivered both as a SameSite=Strict HttpOnly // cookie (sent automatically on same-site WebSocket upgrades) and as a // meta tag (read by app.js and sent as a WebSocket subprotocol). if r.URL.Path == "/" && wsToken != "" { - http.SetCookie(w, &http.Cookie{ - Name: wsTokenCookieName, - Value: wsToken, - Path: "/", - SameSite: http.SameSiteStrictMode, - HttpOnly: true, - // No explicit MaxAge/Expires so the cookie is a session cookie. - }) - data = []byte(strings.Replace(string(data), "{{ODEK_WS_TOKEN}}", wsToken, 1)) + if r.URL.Query().Get("token") == wsToken { + http.SetCookie(w, &http.Cookie{ + Name: wsTokenCookieName, + Value: wsToken, + Path: "/", + SameSite: http.SameSiteStrictMode, + HttpOnly: true, + // No explicit MaxAge/Expires so the cookie is a session cookie. + }) + data = []byte(strings.Replace(string(data), "{{ODEK_WS_TOKEN}}", wsToken, 1)) + w.Header().Set("Cache-Control", "no-store") + } else { + // No valid token in the URL: serve the UI but leave the meta tag + // empty so the browser cannot connect until the user uses the + // token URL printed to the console. + data = []byte(strings.Replace(string(data), "{{ODEK_WS_TOKEN}}", "", 1)) + } } w.Header().Set("Content-Type", entry[1]) @@ -1692,3 +1715,14 @@ func openInBrowser(url string) { } } } + +// isLoopbackAddr reports whether a TCP listener is bound to a loopback +// address. Unix sockets and non-TCP listeners are treated as non-loopback +// (fail safe) because we cannot reason about their exposure. +func isLoopbackAddr(a net.Addr) bool { + ta, ok := a.(*net.TCPAddr) + if !ok { + return false + } + return ta.IP.IsLoopback() +} diff --git a/cmd/odek/serve_approval_test.go b/cmd/odek/serve_approval_test.go index 7728973..9261910 100644 --- a/cmd/odek/serve_approval_test.go +++ b/cmd/odek/serve_approval_test.go @@ -48,6 +48,9 @@ func buildServeMuxPromptAll(t *testing.T, store *session.Store) (net.Listener, * if err != nil { t.Fatalf("CSRF token: %v", err) } + testTokenMu.Lock() + testLastToken = wsToken + testTokenMu.Unlock() mux := http.NewServeMux() mux.HandleFunc("/", handleStatic(wsToken)) diff --git a/cmd/odek/serve_test.go b/cmd/odek/serve_test.go index ecc2e91..94f57d5 100644 --- a/cmd/odek/serve_test.go +++ b/cmd/odek/serve_test.go @@ -14,6 +14,7 @@ import ( "reflect" "regexp" "strings" + "sync" "testing" "time" @@ -24,6 +25,15 @@ import ( golangws "golang.org/x/net/websocket" ) +// testLastToken lets dialTestWS discover the token generated by the most +// recent buildServeMux call without changing the helper's signature (which +// would require touching every test). Tests that need the token read it +// immediately after buildServeMux, so a simple last-value variable is enough. +var ( + testTokenMu sync.Mutex + testLastToken string +) + // ── Test Server ──────────────────────────────────────────────────────── // testServer wraps the odek serve HTTP server for testing. @@ -158,10 +168,10 @@ func (s *testServer) handleWebSocket(conn *golangws.Conn) { // Send session event writeJSON(conn, map[string]any{ - "type": "session", - "session_id": "test-session-001", - "auth_token": authToken, - "model": "test-model", + "type": "session", + "session_id": "test-session-001", + "auth_token": authToken, + "model": "test-model", }) // Send thinking @@ -553,6 +563,9 @@ func TestServe_E2E_WebSocketPipeline(t *testing.T) { if err != nil { t.Fatalf("CSRF token: %v", err) } + testTokenMu.Lock() + testLastToken = wsToken + testTokenMu.Unlock() mux := http.NewServeMux() mux.HandleFunc("/", handleStatic(wsToken)) @@ -888,6 +901,9 @@ func buildServeMux(t *testing.T, store *session.Store) (net.Listener, *http.Serv if err != nil { t.Fatalf("CSRF token: %v", err) } + testTokenMu.Lock() + testLastToken = wsToken + testTokenMu.Unlock() mux := http.NewServeMux() mux.HandleFunc("/", handleStatic(wsToken)) @@ -921,14 +937,22 @@ func waitForHTTP(t *testing.T, addr string) { } // dialTestWS connects to the real /ws endpoint using the per-instance CSRF -// token served in index.html. Tests that exercise the production handshake -// must use this helper so the WebSocket upgrade is not rejected. +// token. Tests that exercise the production handshake must use this helper so +// the WebSocket upgrade is not rejected. func dialTestWS(t *testing.T, addr string) *golangws.Conn { t.Helper() - resp, err := http.Get("http://" + addr + "/") + testTokenMu.Lock() + token := testLastToken + testTokenMu.Unlock() + if token == "" { + t.Fatal("dialTestWS called before buildServeMux set a token") + } + + // Verify the token is only exposed when the correct ?token= is supplied. + resp, err := http.Get("http://" + addr + "/?token=" + token) if err != nil { - t.Fatalf("GET /: %v", err) + t.Fatalf("GET /?token=: %v", err) } body, err := io.ReadAll(resp.Body) resp.Body.Close() @@ -936,14 +960,14 @@ func dialTestWS(t *testing.T, addr string) *golangws.Conn { t.Fatalf("read index.html: %v", err) } - re := regexp.MustCompile(` exposes the token and sets the cookie. + resp, err = http.Get("http://" + addr + "/?token=" + token) + if err != nil { + t.Fatalf("GET /?token=: %v", err) + } + body, err = io.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + t.Fatalf("read index.html: %v", err) + } m := re.FindStringSubmatch(string(body)) - if m == nil { - t.Fatalf("CSRF token meta tag missing from index.html") + if m == nil || m[1] != token { + t.Fatalf("CSRF token meta tag missing from index.html with correct token") } var cookieFound bool for _, c := range resp.Cookies() { @@ -2263,7 +2311,7 @@ func TestServe_CSRF_TokenRequired(t *testing.T) { t.Errorf("%s cookie not set by index.html handler", wsTokenCookieName) } - // 2. A WebSocket upgrade without the token must be rejected. + // 3. A WebSocket upgrade without the token must be rejected. wsURL := "ws://" + addr + "/ws" conn, err := golangws.Dial(wsURL, "", "http://localhost") if err == nil { @@ -2271,7 +2319,7 @@ func TestServe_CSRF_TokenRequired(t *testing.T) { t.Fatalf("WebSocket upgrade without token should be rejected") } - // 3. A WebSocket upgrade with the token subprotocol must succeed. + // 4. A WebSocket upgrade with the token subprotocol must succeed. conn = dialTestWS(t, addr) conn.Close() } @@ -2387,3 +2435,18 @@ func TestServe_E2E_InvalidModelIDRejected(t *testing.T) { } } } + +func TestIsLoopbackAddr(t *testing.T) { + if !isLoopbackAddr(&net.TCPAddr{IP: net.ParseIP("127.0.0.1")}) { + t.Error("127.0.0.1 should be loopback") + } + if !isLoopbackAddr(&net.TCPAddr{IP: net.ParseIP("::1")}) { + t.Error("::1 should be loopback") + } + if isLoopbackAddr(&net.TCPAddr{IP: net.ParseIP("0.0.0.0")}) { + t.Error("0.0.0.0 should not be loopback") + } + if isLoopbackAddr(&net.TCPAddr{IP: net.ParseIP("192.168.1.1")}) { + t.Error("192.168.1.1 should not be loopback") + } +} diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 9a656dc..d2fa8dd 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -550,6 +550,20 @@ The `/api/resources?q=...&limit=N` autocomplete endpoint previously accepted any This bounds the memory/container blast radius if a local process or malicious page tries to spawn many agent sessions. +### 41a. WebSocket token no longer served over plain HTTP + +`odek serve` previously embedded the per-instance WebSocket token in every `GET /` response (as an HTML meta tag and an HttpOnly cookie). Because `GET /` was not behind any authentication, any network attacker who could reach the port could fetch the token, upgrade to `/ws`, and drive the agent using the operator's model, tools, and API key. + +The token is now treated like a Jupyter notebook token: + +- It is printed to the console on startup, together with a URL of the form `http:///?token=`. +- The browser only receives the cookie/meta tag when it requests `/` with the correct `?token=` query parameter. +- A plain `GET /` returns the UI but leaves the token field empty, so it cannot connect. +- Non-browser clients can supply the token via the `X-Odek-Ws-Token` header or the `odek.` WebSocket subprotocol. +- A loud warning is printed when the server is bound to a non-loopback address. + +This removes the unauthenticated token disclosure path while preserving the same browser experience for the operator who has access to the startup console output. + ### 42. Sub-agent progress stream limits `delegate_tasks` streams NDJSON progress lines from each sub-agent. A runaway or malicious sub-agent could emit an unbounded number of `tool_call`/`tool_result` events, causing unbounded memory growth in the parent. `scanSubagentStream` now caps the total progress stream at 100 000 lines and 100 MiB of data; exceeding either limit aborts the scan and cancels the sub-agent context so the child process is killed instead of continuing to flood stdout.