Skip to content

Commit fbb9ced

Browse files
authored
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=<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.
1 parent 5e28ca9 commit fbb9ced

5 files changed

Lines changed: 154 additions & 40 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
114114
- **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/<pid>/environ`.
115115
- **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.
116116
- **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`.
117-
- **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.<token>` subprotocol. The localhost origin check remains as defense-in-depth.
117+
- **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.<token>` 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.
118118
- **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.
119119
- **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.
120120
- **Browser history cap** (`cmd/odek/browser_tool.go`) — navigation history is capped at 50 snapshots to prevent memory DoS from repeated `browser_navigate` calls.

cmd/odek/serve.go

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -322,12 +322,24 @@ func serveCmd(args []string) error {
322322
return fmt.Errorf("listen: %w", err)
323323
}
324324

325-
fmt.Fprintf(os.Stderr, "odek serve ⚡ http://%s\n", listener.Addr())
325+
// Print the WebSocket token to the console only (Jupyter-style). It is
326+
// never served over plain HTTP, so a network attacker who can only make
327+
// `GET /` cannot retrieve it. Browser clients get it via the token URL;
328+
// non-browser clients can supply it as a header or subprotocol.
329+
tokenURL := "http://" + listener.Addr().String() + "/?token=" + wsToken
330+
fmt.Fprintf(os.Stderr, "odek serve ⚡ %s\n", tokenURL)
326331
fmt.Fprintf(os.Stderr, " WebSocket: ws://%s/ws\n", listener.Addr())
332+
fmt.Fprintf(os.Stderr, " WS token: %s\n", wsToken)
327333
fmt.Fprintf(os.Stderr, " Type @ to reference files, drop or attach files inline.\n")
328334

335+
if !isLoopbackAddr(listener.Addr()) {
336+
fmt.Fprintf(os.Stderr, "\n⚠️ WARNING: odek serve is bound to a non-loopback address.\n")
337+
fmt.Fprintf(os.Stderr, " Anyone who can reach this port can control the agent once they have the token above.\n")
338+
fmt.Fprintf(os.Stderr, " Use --addr 127.0.0.1:<port> (or a firewall) to restrict access.\n\n")
339+
}
340+
329341
if openBrowser {
330-
openInBrowser("http://" + listener.Addr().String())
342+
openInBrowser(tokenURL)
331343
}
332344

333345
return serveOnListener(listener, mux)
@@ -539,11 +551,11 @@ func newServeAgent(resolved config.ResolvedConfig, system string, sendFn func(v
539551
SystemMessage: system,
540552
UntrustedWrapper: func(source, content string) string { return wrapUntrusted(context.Background(), source, content) },
541553
RuntimeContext: runtimeCtx,
542-
NoProjectFile: resolved.NoAgents,
543-
Thinking: resolved.Thinking,
544-
InteractionMode: resolved.InteractionMode,
545-
Tools: tools,
546-
ToolFilter: odek.ToolFilterConfig{Enabled: resolved.Tools.Enabled, Disabled: resolved.Tools.Disabled},
554+
NoProjectFile: resolved.NoAgents,
555+
Thinking: resolved.Thinking,
556+
InteractionMode: resolved.InteractionMode,
557+
Tools: tools,
558+
ToolFilter: odek.ToolFilterConfig{Enabled: resolved.Tools.Enabled, Disabled: resolved.Tools.Disabled},
547559
// SandboxCleanup is intentionally NOT passed here. In serve mode,
548560
// cleanup is the caller's responsibility (handleWS defers it).
549561
// 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
552564
Renderer: nil, // silent — we stream via WebSocket
553565
Skills: &resolved.Skills,
554566
SkillManager: sm,
555-
MemoryConfig: resolved.Memory,
567+
MemoryConfig: resolved.Memory,
556568
MemoryDir: expandHome("~/.odek/memory"),
557569
Guard: injectionGuard,
558570
GuardConfig: resolved.Guard,
@@ -1647,19 +1659,30 @@ func handleStatic(wsToken string) http.HandlerFunc {
16471659
return
16481660
}
16491661

1650-
// The HTML entry point gets the per-instance CSRF token both as a
1662+
// The HTML entry point only receives the per-instance CSRF token when
1663+
// the request presents the token in the `?token=` query parameter. This
1664+
// prevents a network attacker from retrieving the token with a simple
1665+
// `GET /`. The token is delivered both as a SameSite=Strict HttpOnly
16511666
// cookie (sent automatically on same-site WebSocket upgrades) and as a
16521667
// meta tag (read by app.js and sent as a WebSocket subprotocol).
16531668
if r.URL.Path == "/" && wsToken != "" {
1654-
http.SetCookie(w, &http.Cookie{
1655-
Name: wsTokenCookieName,
1656-
Value: wsToken,
1657-
Path: "/",
1658-
SameSite: http.SameSiteStrictMode,
1659-
HttpOnly: true,
1660-
// No explicit MaxAge/Expires so the cookie is a session cookie.
1661-
})
1662-
data = []byte(strings.Replace(string(data), "{{ODEK_WS_TOKEN}}", wsToken, 1))
1669+
if r.URL.Query().Get("token") == wsToken {
1670+
http.SetCookie(w, &http.Cookie{
1671+
Name: wsTokenCookieName,
1672+
Value: wsToken,
1673+
Path: "/",
1674+
SameSite: http.SameSiteStrictMode,
1675+
HttpOnly: true,
1676+
// No explicit MaxAge/Expires so the cookie is a session cookie.
1677+
})
1678+
data = []byte(strings.Replace(string(data), "{{ODEK_WS_TOKEN}}", wsToken, 1))
1679+
w.Header().Set("Cache-Control", "no-store")
1680+
} else {
1681+
// No valid token in the URL: serve the UI but leave the meta tag
1682+
// empty so the browser cannot connect until the user uses the
1683+
// token URL printed to the console.
1684+
data = []byte(strings.Replace(string(data), "{{ODEK_WS_TOKEN}}", "", 1))
1685+
}
16631686
}
16641687

16651688
w.Header().Set("Content-Type", entry[1])
@@ -1692,3 +1715,14 @@ func openInBrowser(url string) {
16921715
}
16931716
}
16941717
}
1718+
1719+
// isLoopbackAddr reports whether a TCP listener is bound to a loopback
1720+
// address. Unix sockets and non-TCP listeners are treated as non-loopback
1721+
// (fail safe) because we cannot reason about their exposure.
1722+
func isLoopbackAddr(a net.Addr) bool {
1723+
ta, ok := a.(*net.TCPAddr)
1724+
if !ok {
1725+
return false
1726+
}
1727+
return ta.IP.IsLoopback()
1728+
}

cmd/odek/serve_approval_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ func buildServeMuxPromptAll(t *testing.T, store *session.Store) (net.Listener, *
4848
if err != nil {
4949
t.Fatalf("CSRF token: %v", err)
5050
}
51+
testTokenMu.Lock()
52+
testLastToken = wsToken
53+
testTokenMu.Unlock()
5154

5255
mux := http.NewServeMux()
5356
mux.HandleFunc("/", handleStatic(wsToken))

cmd/odek/serve_test.go

Lines changed: 84 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"reflect"
1515
"regexp"
1616
"strings"
17+
"sync"
1718
"testing"
1819
"time"
1920

@@ -24,6 +25,15 @@ import (
2425
golangws "golang.org/x/net/websocket"
2526
)
2627

28+
// testLastToken lets dialTestWS discover the token generated by the most
29+
// recent buildServeMux call without changing the helper's signature (which
30+
// would require touching every test). Tests that need the token read it
31+
// immediately after buildServeMux, so a simple last-value variable is enough.
32+
var (
33+
testTokenMu sync.Mutex
34+
testLastToken string
35+
)
36+
2737
// ── Test Server ────────────────────────────────────────────────────────
2838

2939
// testServer wraps the odek serve HTTP server for testing.
@@ -158,10 +168,10 @@ func (s *testServer) handleWebSocket(conn *golangws.Conn) {
158168

159169
// Send session event
160170
writeJSON(conn, map[string]any{
161-
"type": "session",
162-
"session_id": "test-session-001",
163-
"auth_token": authToken,
164-
"model": "test-model",
171+
"type": "session",
172+
"session_id": "test-session-001",
173+
"auth_token": authToken,
174+
"model": "test-model",
165175
})
166176

167177
// Send thinking
@@ -553,6 +563,9 @@ func TestServe_E2E_WebSocketPipeline(t *testing.T) {
553563
if err != nil {
554564
t.Fatalf("CSRF token: %v", err)
555565
}
566+
testTokenMu.Lock()
567+
testLastToken = wsToken
568+
testTokenMu.Unlock()
556569

557570
mux := http.NewServeMux()
558571
mux.HandleFunc("/", handleStatic(wsToken))
@@ -888,6 +901,9 @@ func buildServeMux(t *testing.T, store *session.Store) (net.Listener, *http.Serv
888901
if err != nil {
889902
t.Fatalf("CSRF token: %v", err)
890903
}
904+
testTokenMu.Lock()
905+
testLastToken = wsToken
906+
testTokenMu.Unlock()
891907

892908
mux := http.NewServeMux()
893909
mux.HandleFunc("/", handleStatic(wsToken))
@@ -921,29 +937,37 @@ func waitForHTTP(t *testing.T, addr string) {
921937
}
922938

923939
// dialTestWS connects to the real /ws endpoint using the per-instance CSRF
924-
// token served in index.html. Tests that exercise the production handshake
925-
// must use this helper so the WebSocket upgrade is not rejected.
940+
// token. Tests that exercise the production handshake must use this helper so
941+
// the WebSocket upgrade is not rejected.
926942
func dialTestWS(t *testing.T, addr string) *golangws.Conn {
927943
t.Helper()
928944

929-
resp, err := http.Get("http://" + addr + "/")
945+
testTokenMu.Lock()
946+
token := testLastToken
947+
testTokenMu.Unlock()
948+
if token == "" {
949+
t.Fatal("dialTestWS called before buildServeMux set a token")
950+
}
951+
952+
// Verify the token is only exposed when the correct ?token= is supplied.
953+
resp, err := http.Get("http://" + addr + "/?token=" + token)
930954
if err != nil {
931-
t.Fatalf("GET /: %v", err)
955+
t.Fatalf("GET /?token=: %v", err)
932956
}
933957
body, err := io.ReadAll(resp.Body)
934958
resp.Body.Close()
935959
if err != nil {
936960
t.Fatalf("read index.html: %v", err)
937961
}
938962

939-
re := regexp.MustCompile(`<meta\s+name="odek-ws-token"\s+content="([^"]+)"`)
963+
re := regexp.MustCompile(`<meta\s+name="odek-ws-token"\s+content="([^"]*)"`)
940964
m := re.FindStringSubmatch(string(body))
941-
if m == nil {
942-
t.Fatalf("CSRF token not found in served index.html")
965+
if m == nil || m[1] != token {
966+
t.Fatalf("CSRF token not exposed with correct ?token= query")
943967
}
944968

945969
wsURL := "ws://" + addr + "/ws"
946-
conn, err := golangws.Dial(wsURL, wsTokenProtocolPrefix+m[1], "http://localhost")
970+
conn, err := golangws.Dial(wsURL, wsTokenProtocolPrefix+token, "http://localhost")
947971
if err != nil {
948972
t.Fatalf("Dial(%q): %v", wsURL, err)
949973
}
@@ -2215,9 +2239,10 @@ func TestServe_E2E_AttachmentsWrappedAsUntrusted(t *testing.T) {
22152239
}
22162240
}
22172241

2218-
// TestServe_CSRF_TokenRequired verifies that the WebSocket upgrade requires
2219-
// the per-instance CSRF token, and that the token is delivered both as an
2220-
// HttpOnly SameSite=Strict cookie and as a meta tag in index.html.
2242+
// TestServe_CSRF_TokenRequired verifies that the WebSocket endpoint requires
2243+
// the per-instance CSRF token, that the token is only exposed when the request
2244+
// includes the correct ?token= query parameter, and that the token is exposed
2245+
// both as an HttpOnly SameSite=Strict cookie and as a meta tag in index.html.
22212246
func TestServe_CSRF_TokenRequired(t *testing.T) {
22222247
store := newTestSessionStore(t)
22232248
ln, mux := buildServeMux(t, store)
@@ -2228,8 +2253,13 @@ func TestServe_CSRF_TokenRequired(t *testing.T) {
22282253
waitForHTTP(t, ln.Addr().String())
22292254

22302255
addr := ln.Addr().String()
2256+
testTokenMu.Lock()
2257+
token := testLastToken
2258+
testTokenMu.Unlock()
2259+
2260+
re := regexp.MustCompile(`<meta\s+name="odek-ws-token"\s+content="([^"]*)"`)
22312261

2232-
// 1. The HTML entry point must expose the token and set a cookie.
2262+
// 1. A plain GET / must NOT expose the token or set the cookie.
22332263
resp, err := http.Get("http://" + addr + "/")
22342264
if err != nil {
22352265
t.Fatalf("GET /: %v", err)
@@ -2239,10 +2269,28 @@ func TestServe_CSRF_TokenRequired(t *testing.T) {
22392269
if err != nil {
22402270
t.Fatalf("read index.html: %v", err)
22412271
}
2242-
re := regexp.MustCompile(`<meta\s+name="odek-ws-token"\s+content="([^"]+)"`)
2272+
if m := re.FindStringSubmatch(string(body)); m != nil && m[1] != "" {
2273+
t.Errorf("GET / leaked CSRF token in meta tag")
2274+
}
2275+
for _, c := range resp.Cookies() {
2276+
if c.Name == wsTokenCookieName {
2277+
t.Errorf("GET / set %s cookie without token", wsTokenCookieName)
2278+
}
2279+
}
2280+
2281+
// 2. GET /?token=<token> exposes the token and sets the cookie.
2282+
resp, err = http.Get("http://" + addr + "/?token=" + token)
2283+
if err != nil {
2284+
t.Fatalf("GET /?token=: %v", err)
2285+
}
2286+
body, err = io.ReadAll(resp.Body)
2287+
resp.Body.Close()
2288+
if err != nil {
2289+
t.Fatalf("read index.html: %v", err)
2290+
}
22432291
m := re.FindStringSubmatch(string(body))
2244-
if m == nil {
2245-
t.Fatalf("CSRF token meta tag missing from index.html")
2292+
if m == nil || m[1] != token {
2293+
t.Fatalf("CSRF token meta tag missing from index.html with correct token")
22462294
}
22472295
var cookieFound bool
22482296
for _, c := range resp.Cookies() {
@@ -2263,15 +2311,15 @@ func TestServe_CSRF_TokenRequired(t *testing.T) {
22632311
t.Errorf("%s cookie not set by index.html handler", wsTokenCookieName)
22642312
}
22652313

2266-
// 2. A WebSocket upgrade without the token must be rejected.
2314+
// 3. A WebSocket upgrade without the token must be rejected.
22672315
wsURL := "ws://" + addr + "/ws"
22682316
conn, err := golangws.Dial(wsURL, "", "http://localhost")
22692317
if err == nil {
22702318
conn.Close()
22712319
t.Fatalf("WebSocket upgrade without token should be rejected")
22722320
}
22732321

2274-
// 3. A WebSocket upgrade with the token subprotocol must succeed.
2322+
// 4. A WebSocket upgrade with the token subprotocol must succeed.
22752323
conn = dialTestWS(t, addr)
22762324
conn.Close()
22772325
}
@@ -2387,3 +2435,18 @@ func TestServe_E2E_InvalidModelIDRejected(t *testing.T) {
23872435
}
23882436
}
23892437
}
2438+
2439+
func TestIsLoopbackAddr(t *testing.T) {
2440+
if !isLoopbackAddr(&net.TCPAddr{IP: net.ParseIP("127.0.0.1")}) {
2441+
t.Error("127.0.0.1 should be loopback")
2442+
}
2443+
if !isLoopbackAddr(&net.TCPAddr{IP: net.ParseIP("::1")}) {
2444+
t.Error("::1 should be loopback")
2445+
}
2446+
if isLoopbackAddr(&net.TCPAddr{IP: net.ParseIP("0.0.0.0")}) {
2447+
t.Error("0.0.0.0 should not be loopback")
2448+
}
2449+
if isLoopbackAddr(&net.TCPAddr{IP: net.ParseIP("192.168.1.1")}) {
2450+
t.Error("192.168.1.1 should not be loopback")
2451+
}
2452+
}

docs/SECURITY.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,20 @@ The `/api/resources?q=...&limit=N` autocomplete endpoint previously accepted any
550550

551551
This bounds the memory/container blast radius if a local process or malicious page tries to spawn many agent sessions.
552552

553+
### 41a. WebSocket token no longer served over plain HTTP
554+
555+
`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.
556+
557+
The token is now treated like a Jupyter notebook token:
558+
559+
- It is printed to the console on startup, together with a URL of the form `http://<addr>/?token=<token>`.
560+
- The browser only receives the cookie/meta tag when it requests `/` with the correct `?token=` query parameter.
561+
- A plain `GET /` returns the UI but leaves the token field empty, so it cannot connect.
562+
- Non-browser clients can supply the token via the `X-Odek-Ws-Token` header or the `odek.<token>` WebSocket subprotocol.
563+
- A loud warning is printed when the server is bound to a non-loopback address.
564+
565+
This removes the unauthenticated token disclosure path while preserving the same browser experience for the operator who has access to the startup console output.
566+
553567
### 42. Sub-agent progress stream limits
554568

555569
`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.

0 commit comments

Comments
 (0)