From 3e05f3f2697412034252232041bc4bc826c3be8a Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Sat, 18 Jul 2026 16:04:24 +0200 Subject: [PATCH] fix: require serve token + loopback Host on all /api/* endpoints (M-15) --- AGENTS.md | 2 +- .../next_security_vulnerabilities_test.go | 93 +++++++++++++++++++ cmd/odek/serve.go | 73 ++++++++++++++- cmd/odek/serve_api_test.go | 15 ++- cmd/odek/serve_test.go | 22 ++++- cmd/odek/ui/app.js | 40 +++++--- docs/SECURITY.md | 6 +- 7 files changed, 227 insertions(+), 24 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 50e1be3..31ff3c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -116,7 +116,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU - **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. `rm -rf ./` / `rm -rf ./..` are normalised to `.` / `..` for wipe-target matching, and `${VAR:--rf}` default-value flag substitutions are treated as fail-closed. Regression suite in `classifier_bypass_test.go`. - **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` + `internal/danger/classifier.go`) — `browser`, `http_batch`, and `web_search` resolve hostnames at dial time and refuse internal IPs (loopback, RFC1918, RFC4193, link-local, RFC 6598 CGNAT `100.64.0.0/10`, RFC 2544 benchmark `198.18.0.0/15`, and unspecified), 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. +- **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. Every `/api/*` endpoint additionally requires the per-instance `odek_ws_token` (cookie or `X-Odek-Ws-Token` header) and rejects non-loopback `Host` headers, closing DNS-rebinding reads of `/api/sessions`, `/api/resources`, and `/api/models`. - **Browser history cap** (`cmd/odek/browser_tool.go`) — navigation history is capped at 50 snapshots to prevent memory DoS from repeated `browser_navigate` calls. - **Browser element cap** (`cmd/odek/browser_tool.go`) — the number of interactive elements extracted per page is capped at 500 so a hostile page cannot OOM the agent with thousands of links or buttons. - **Search path classification** (`cmd/odek/file_tool.go`, `cmd/odek/perf_tools.go`) — `search_files` and `multi_grep` classify every descended directory and every discovered file the same way the search root is classified. Sensitive paths that would require approval (or are denied) are skipped and reported in the `skipped` field rather than returned silently, closing the gap where a broad search root auto-approved sensitive files. diff --git a/cmd/odek/next_security_vulnerabilities_test.go b/cmd/odek/next_security_vulnerabilities_test.go index d56fdb3..88fa712 100644 --- a/cmd/odek/next_security_vulnerabilities_test.go +++ b/cmd/odek/next_security_vulnerabilities_test.go @@ -211,6 +211,99 @@ func TestServe_CSRF_AllowsLocalhostOrigin(t *testing.T) { } } +// ── 4b. serve API endpoints must require the serve token and loopback Host ─ + +func TestServe_API_RequiresServeToken(t *testing.T) { + store := newTestSessionStore(t) + ln, mux := buildServeMux(t, store) + defer ln.Close() + + go serveOnListener(ln, mux) + waitForHTTP(t, ln.Addr().String()) + + resp, err := http.Get("http://" + ln.Addr().String() + "/api/sessions") + if err != nil { + t.Fatalf("GET /api/sessions: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("expected 403 without serve token, got %d", resp.StatusCode) + } +} + +func TestServe_API_RequiresLocalHost(t *testing.T) { + store := newTestSessionStore(t) + ln, mux := buildServeMux(t, store) + defer ln.Close() + + testTokenMu.Lock() + token := testLastToken + testTokenMu.Unlock() + + go serveOnListener(ln, mux) + waitForHTTP(t, ln.Addr().String()) + + req, _ := http.NewRequest(http.MethodGet, "http://"+ln.Addr().String()+"/api/sessions", nil) + req.Header.Set(wsTokenHeaderName, token) + req.Host = "attacker.example.com" + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("GET /api/sessions: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("expected 403 for non-loopback Host, got %d", resp.StatusCode) + } +} + +func TestServe_API_AcceptsServeTokenHeader(t *testing.T) { + store := newTestSessionStore(t) + ln, mux := buildServeMux(t, store) + defer ln.Close() + + testTokenMu.Lock() + token := testLastToken + testTokenMu.Unlock() + + go serveOnListener(ln, mux) + waitForHTTP(t, ln.Addr().String()) + + req, _ := http.NewRequest(http.MethodGet, "http://"+ln.Addr().String()+"/api/sessions", nil) + req.Header.Set(wsTokenHeaderName, token) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("GET /api/sessions: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected 200 with serve token header, got %d", resp.StatusCode) + } +} + +func TestServe_API_AcceptsServeTokenCookie(t *testing.T) { + store := newTestSessionStore(t) + ln, mux := buildServeMux(t, store) + defer ln.Close() + + testTokenMu.Lock() + token := testLastToken + testTokenMu.Unlock() + + go serveOnListener(ln, mux) + waitForHTTP(t, ln.Addr().String()) + + req, _ := http.NewRequest(http.MethodGet, "http://"+ln.Addr().String()+"/api/sessions", nil) + req.AddCookie(&http.Cookie{Name: wsTokenCookieName, Value: token}) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("GET /api/sessions: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected 200 with serve token cookie, got %d", resp.StatusCode) + } +} + func TestServe_StaticSecurityHeaders(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", nil) rr := httptest.NewRecorder() diff --git a/cmd/odek/serve.go b/cmd/odek/serve.go index 6235a58..69b5523 100644 --- a/cmd/odek/serve.go +++ b/cmd/odek/serve.go @@ -311,11 +311,17 @@ func serveCmd(args []string) error { handleWS(store, resourceReg, resolved, systemMessage, conn) }, }) - mux.Handle("/api/resources", requireLocalOrigin(handleResourceSearch(resourceReg))) - mux.Handle("/api/sessions", requireLocalOrigin(handleSessionList(store))) - mux.Handle("/api/sessions/", requireLocalOrigin(handleSessionByID(store))) - mux.Handle("/api/models", requireLocalOrigin(handleModelList(resolved.Model))) - mux.Handle("/api/cancel", requireLocalOrigin(handleCancel(store))) + // All API endpoints require the per-instance CSRF token, a loopback Host, + // and (for state-changing methods) a local Origin. This blocks DNS-rebinding + // and cross-site reads of sessions/resources/models. + apiAuth := func(h http.Handler) http.Handler { + return requireServeToken(wsToken)(requireLocalHost(requireLocalOrigin(h))) + } + mux.Handle("/api/resources", apiAuth(handleResourceSearch(resourceReg))) + mux.Handle("/api/sessions", apiAuth(handleSessionList(store))) + mux.Handle("/api/sessions/", apiAuth(handleSessionByID(store))) + mux.Handle("/api/models", apiAuth(handleModelList(resolved.Model))) + mux.Handle("/api/cancel", apiAuth(handleCancel(store))) listener, err := net.Listen("tcp", addr) if err != nil { @@ -1355,6 +1361,63 @@ func isStateChangingMethod(method string) bool { return true } +// requireLocalHost rejects requests whose Host header does not name a +// loopback interface. This closes DNS-rebinding attacks that point an external +// domain at 127.0.0.1 and then drive the local API from a malicious web page. +func requireLocalHost(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + host := r.Host + if h, _, err := net.SplitHostPort(host); err == nil { + host = h + } + if host != "localhost" && host != "127.0.0.1" && host != "::1" { + http.Error(w, "Host not allowed", http.StatusForbidden) + return + } + next.ServeHTTP(w, r) + }) +} + +// validateServeTokenHTTP verifies the per-instance CSRF token on an HTTP +// request. It accepts the same cookie and header transports as the WebSocket +// handshake, but not the WebSocket subprotocol. +func validateServeTokenHTTP(req *http.Request, token string) error { + if token == "" { + return fmt.Errorf("server token not configured") + } + + // Cookie (browser same-origin / same-site requests). + if c, err := req.Cookie(wsTokenCookieName); err == nil && c.Value != "" { + if subtle.ConstantTimeCompare([]byte(c.Value), []byte(token)) == 1 { + return nil + } + } + + // Explicit header (non-browser clients). + if h := req.Header.Get(wsTokenHeaderName); h != "" { + if subtle.ConstantTimeCompare([]byte(h), []byte(token)) == 1 { + return nil + } + } + + return fmt.Errorf("missing or invalid server token") +} + +// requireServeToken requires the per-instance CSRF token on every request. +// This blocks DNS-rebinding and cross-site driven reads of API endpoints that +// were previously unauthenticated on GET. +func requireServeToken(token string) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := validateServeTokenHTTP(r, token); err != nil { + http.Error(w, "Forbidden", http.StatusForbidden) + return + } + next.ServeHTTP(w, r) + }) + } +} + // sessionTokenFromRequest returns the session auth token from the // X-Session-Token header or the session_token cookie, in that order. func sessionTokenFromRequest(r *http.Request) string { diff --git a/cmd/odek/serve_api_test.go b/cmd/odek/serve_api_test.go index f5bda63..5e82b84 100644 --- a/cmd/odek/serve_api_test.go +++ b/cmd/odek/serve_api_test.go @@ -694,8 +694,14 @@ func TestServe_E2E_SessionMessagesStoredWithoutSystemInjections(t *testing.T) { } // Fetch the stored session and verify no system messages are stored. + // The production API auth middleware requires the per-instance serve token. + testTokenMu.Lock() + serveToken := testLastToken + testTokenMu.Unlock() + req, _ := http.NewRequest(http.MethodGet, "http://"+ln.Addr().String()+"/api/sessions/"+sid, nil) req.Header.Set("X-Session-Token", authToken) + req.Header.Set(wsTokenHeaderName, serveToken) resp, err := http.DefaultClient.Do(req) if err != nil { t.Fatalf("GET session: %v", err) @@ -728,7 +734,14 @@ func TestServe_E2E_SessionMessagesStoredWithoutSystemInjections(t *testing.T) { func buildServeMuxWithSessionByID(t *testing.T, store *session.Store) (net.Listener, *http.ServeMux) { t.Helper() ln, mux := buildServeMux(t, store) - mux.HandleFunc("/api/sessions/", handleSessionByID(store)) + // Mirror production authentication stack for session-by-ID endpoint. + testTokenMu.Lock() + token := testLastToken + testTokenMu.Unlock() + apiAuth := func(h http.Handler) http.Handler { + return requireServeToken(token)(requireLocalHost(requireLocalOrigin(h))) + } + mux.Handle("/api/sessions/", apiAuth(handleSessionByID(store))) return ln, mux } diff --git a/cmd/odek/serve_test.go b/cmd/odek/serve_test.go index 94f57d5..fcc3fb9 100644 --- a/cmd/odek/serve_test.go +++ b/cmd/odek/serve_test.go @@ -915,9 +915,14 @@ func buildServeMux(t *testing.T, store *session.Store) (net.Listener, *http.Serv handleWS(store, resourceReg, resolved, systemMessage, conn) }, }) - mux.HandleFunc("/api/resources", handleResourceSearch(resourceReg)) - mux.HandleFunc("/api/sessions", handleSessionList(store)) - mux.HandleFunc("/api/cancel", handleCancel(store)) + // Mirror the production API authentication stack so E2E tests exercise the + // real security controls (token + loopback Host + local Origin). + apiAuth := func(h http.Handler) http.Handler { + return requireServeToken(wsToken)(requireLocalHost(requireLocalOrigin(h))) + } + mux.Handle("/api/resources", apiAuth(handleResourceSearch(resourceReg))) + mux.Handle("/api/sessions", apiAuth(handleSessionList(store))) + mux.Handle("/api/cancel", apiAuth(handleCancel(store))) return ln, mux } @@ -2056,7 +2061,9 @@ func readSessionID(t *testing.T, conn *golangws.Conn) string { return sid } -// postCancel makes an authenticated POST /api/cancel request. +// postCancel makes an authenticated POST /api/cancel request. It sends both +// the per-instance CSRF token (required by the API auth middleware) and the +// per-session auth token (required by handleCancel). func postCancel(t *testing.T, url, sessionID, token string) *http.Response { t.Helper() req, err := http.NewRequest(http.MethodPost, url+"/api/cancel?session_id="+sessionID, nil) @@ -2066,6 +2073,13 @@ func postCancel(t *testing.T, url, sessionID, token string) *http.Response { if token != "" { req.Header.Set("X-Session-Token", token) } + // Tests that exercise the production mux must supply the serve token. + testTokenMu.Lock() + serveToken := testLastToken + testTokenMu.Unlock() + if serveToken != "" { + req.Header.Set(wsTokenHeaderName, serveToken) + } resp, err := http.DefaultClient.Do(req) if err != nil { t.Fatalf("POST /api/cancel: %v", err) diff --git a/cmd/odek/ui/app.js b/cmd/odek/ui/app.js index 00990a9..c2bd321 100644 --- a/cmd/odek/ui/app.js +++ b/cmd/odek/ui/app.js @@ -208,6 +208,17 @@ function getWsToken() { return meta ? meta.getAttribute('content') : ''; } +// Build API request headers that include the per-instance CSRF token. The +// server requires this token on every /api/* endpoint to block DNS-rebinding +// and cross-site driven reads. Browser same-origin requests also send the +// cookie, but the header defends-in-depth and works when cookies are blocked. +function apiHeaders(extra) { + const token = getWsToken(); + const headers = token ? { 'X-Odek-Ws-Token': token } : {}; + if (extra) Object.assign(headers, extra); + return headers; +} + function connect() { const proto = location.protocol === 'https:' ? 'wss:' : 'ws:'; const token = getWsToken(); @@ -954,7 +965,9 @@ window.executeDeleteSession = async function() { let token = getSessionToken(sid); if (!token) { try { - const bootstrap = await fetch('/api/sessions/' + encodeURIComponent(sid)); + const bootstrap = await fetch('/api/sessions/' + encodeURIComponent(sid), { + headers: apiHeaders() + }); if (bootstrap.ok) { const bs = await bootstrap.json(); token = bootstrap.headers.get('X-Session-Token') || bs.auth_token; @@ -965,7 +978,7 @@ window.executeDeleteSession = async function() { fetch('/api/sessions/' + encodeURIComponent(sid), { method: 'DELETE', - headers: token ? { 'X-Session-Token': token } : {} + headers: apiHeaders(token ? { 'X-Session-Token': token } : {}) }) .then(() => { clearSessionToken(sid); @@ -995,7 +1008,7 @@ async function fetchModels() { const picker = document.getElementById('model-picker'); try { picker.disabled = true; - const resp = await fetch('/api/models'); + const resp = await fetch('/api/models', { headers: apiHeaders() }); if (!resp.ok) { picker.innerHTML = ''; return; } const models = await resp.json(); availableModels = models; @@ -1098,7 +1111,9 @@ window.renameSession = async function(sid, el) { let token = getSessionToken(sid); if (!token) { try { - const bootstrap = await fetch('/api/sessions/' + encodeURIComponent(sid)); + const bootstrap = await fetch('/api/sessions/' + encodeURIComponent(sid), { + headers: apiHeaders() + }); if (bootstrap.ok) { const bs = await bootstrap.json(); token = bootstrap.headers.get('X-Session-Token') || bs.auth_token; @@ -1109,10 +1124,10 @@ window.renameSession = async function(sid, el) { fetch('/api/sessions/' + encodeURIComponent(sid), { method: 'POST', - headers: { + headers: apiHeaders({ 'Content-Type': 'application/json', ...(token ? { 'X-Session-Token': token } : {}) - }, + }), body: JSON.stringify({ name: newName }) }) .then(resp => { @@ -1630,7 +1645,9 @@ async function checkCompletion() { compQuery = query; try { - const resp = await fetch('/api/resources?q=' + encodeURIComponent(query) + '&limit=8'); + const resp = await fetch('/api/resources?q=' + encodeURIComponent(query) + '&limit=8', { + headers: apiHeaders() + }); const results = await resp.json(); if (!results || results.length === 0) { completionEl.classList.remove('visible'); @@ -1714,8 +1731,9 @@ sessionListEl.addEventListener('click', (e) => { async function loadAndRenderSession(sid) { try { let token = getSessionToken(sid); - const headers = token ? { 'X-Session-Token': token } : {}; - const resp = await fetch('/api/sessions/' + encodeURIComponent(sid), { headers }); + const resp = await fetch('/api/sessions/' + encodeURIComponent(sid), { + headers: apiHeaders(token ? { 'X-Session-Token': token } : {}) + }); if (!resp.ok) { showToast('Failed to load session'); return; } const sess = await resp.json(); @@ -1801,7 +1819,7 @@ sidebarSearch.addEventListener('input', () => { async function loadSessions() { try { - const resp = await fetch('/api/sessions'); + const resp = await fetch('/api/sessions', { headers: apiHeaders() }); const sessions = await resp.json(); if (!sessions || !Array.isArray(sessions)) return; allSessions = sessions; @@ -1893,7 +1911,7 @@ window.cancelAgent = function() { } fetch('/api/cancel?session_id=' + encodeURIComponent(sessionId), { method: 'POST', - headers: { 'X-Session-Token': getSessionToken(sessionId) || '' } + headers: apiHeaders({ 'X-Session-Token': getSessionToken(sessionId) || '' }) }).catch(function(){}); hideCancel(); addSystemMessage('⏹ Canceled'); diff --git a/docs/SECURITY.md b/docs/SECURITY.md index dddad15..50acbf1 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -239,9 +239,11 @@ On Windows, where you cannot `unlink` an open file, a 0600 temp file is used and - injected into the served `index.html` as ``, - delivered as an `HttpOnly` `SameSite=Strict` cookie named `odek_ws_token`, and -- required by the `/ws` handshake via the cookie, an `X-Odek-Ws-Token` header, or a WebSocket subprotocol of the form `odek.`. +- required by the `/ws` handshake and by every `/api/*` endpoint via the cookie, an `X-Odek-Ws-Token` header, or a WebSocket subprotocol of the form `odek.`. -The origin allowlist (`localhost`, `127.0.0.1`, `[::1]`, and empty Origin for non-browser clients) remains as defense-in-depth, but the token is the primary protection against cross-port localhost CSRF: a malicious page served by another local port cannot obtain the token and therefore cannot open an agent-controlling WebSocket. +The origin allowlist (`localhost`, `127.0.0.1`, `[::1]`, and empty Origin for non-browser clients) remains as defense-in-depth, but the token is the primary protection against cross-port localhost CSRF: a malicious page served by another local port cannot obtain the token and therefore cannot open an agent-controlling WebSocket or read from `/api/sessions`, `/api/resources`, `/api/models`, etc. + +On top of the token, all `/api/*` handlers validate the `Host` header and reject any request whose host is not `localhost`, `127.0.0.1`, or `[::1]`. This closes DNS-rebinding attacks that point an external domain at the loopback interface and then drive the local API from a malicious web page. ### 9a. Web UI file attachments