From c1b72ac8f577bd1b1988fd9a02bfd274155b37cc Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Sun, 26 Jul 2026 18:44:36 +0200 Subject: [PATCH] fix(serve): bootstrap session tokens for instance-token-authenticated front-ends Sessions created by one front-end (e.g. bodek, or the WebUI in another browser profile) could not be loaded by another: every new session gets a 256-bit AuthToken that only reaches the creating client, and the token bootstrap only covered legacy sessions with no token at all - so GET /api/sessions/ returned 401 and the WebUI showed "Failed to load session". The GET handler now also bootstraps the session token when the caller proves knowledge of the per-instance CSRF token by presenting it in the X-Odek-Ws-Token header (constant-time compared). Header presentation is a knowledge proof a cross-origin page cannot forge: a DNS-rebinding page holds only the ambient SameSite=Strict cookie and can neither read the token value nor set the custom header (CORS preflight, which odek does not answer). Cookie-only callers therefore still get 401 on token-carrying sessions, while the operator's legitimate front-ends - which always send the header via apiHeaders - can load each other's sessions. The rebinding damage boundary is unchanged; DELETE/POST (rename) flows inherit the fix through the client's existing ensureSessionToken GET bootstrap. Regression tests cover: bootstrap with a valid instance header (200 + X-Session-Token), no bootstrap with a wrong instance header (401), and no bootstrap for cookie-only callers (401). Docs updated (SECURITY.md section 24, AGENTS.md). --- AGENTS.md | 2 +- cmd/odek/serve.go | 31 +++++++++++- cmd/odek/serve_api_test.go | 96 ++++++++++++++++++++++++++++++++------ docs/SECURITY.md | 2 + 4 files changed, 114 insertions(+), 17 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 43d6882..b73666f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -199,7 +199,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU - **Memory add/replace pipe-to-shell filter** (`internal/memory/memory.go`) — `AddFact` and `ReplaceFact` now run `FactLooksUnsafe` in addition to the general guard scan, blocking agent-driven planting of download-and-execute facts. - **Skill learn-loop provenance propagation** (`internal/skills/learnloop.go`) — conversation-extracted suggestions and LLM-enhanced suggestions both retain the session's `SkillProvenance`, so tainted sessions cannot produce clean-looking auto-saved skills. - **Audit ingest recording for @-refs, `--ctx`, and Web-UI attachments** (`cmd/odek/refs.go`, `cmd/odek/serve.go`, `cmd/odek/audit.go`) — the per-session ingest recorder is attached before `@`-reference/`--ctx`/attachment resolution, `recordTurnAudit` scans `user` messages for untrusted wrappers in addition to `tool` messages, and the divergence heuristic compares agent actions against the original pre-enrichment prompt so attacker-injected resources are not treated as user-mentioned. -- **Session ID entropy + session-scoped auth tokens** (`internal/session/session.go`, `cmd/odek/serve.go`) — session IDs now carry 128 bits of randomness (16 bytes / 32 hex chars); each session stores a 256-bit `AuthToken` required by `GET/DELETE/POST /api/sessions/`, `POST /api/cancel`, and WebSocket session-resume messages via `X-Session-Token` header, `session_token` cookie, or `auth_token` WS field. Per-IP rate limiting (60/min) on session lookups adds a brute-force backstop. `X-Forwarded-For` / `X-Real-Ip` headers are only honored when the direct remote address is in the configured `trusted_proxies` list, so spoofed headers cannot bypass the limiters. +- **Session ID entropy + session-scoped auth tokens** (`internal/session/session.go`, `cmd/odek/serve.go`) — session IDs now carry 128 bits of randomness (16 bytes / 32 hex chars); each session stores a 256-bit `AuthToken` required by `GET/DELETE/POST /api/sessions/`, `POST /api/cancel`, and WebSocket session-resume messages via `X-Session-Token` header, `session_token` cookie, or `auth_token` WS field. Per-IP rate limiting (60/min) on session lookups adds a brute-force backstop. `X-Forwarded-For` / `X-Real-Ip` headers are only honored when the direct remote address is in the configured `trusted_proxies` list, so spoofed headers cannot bypass the limiters. Callers that present the per-instance CSRF token in the `X-Odek-Ws-Token` header (a knowledge proof a cross-origin page cannot forge) are bootstrapped the session token on `GET`, so the operator's front-ends (WebUI, bodek) can load each other's sessions; cookie-only callers still get 401. - **Skill/episode untrusted wrapper** (`internal/loop/loop.go` + `odek.go`) — skill context (both auto-load and lazy trigger-matched), the `skill_load` tool output, and retrieved session-episode context are passed through the caller-provided untrusted wrapper (the same nonce'd `` boundary used for tool output) before being injected into the model's system context. This prevents a compromised or tainted skill/episode from being treated as trusted system instructions. - **`env` / `printenv` environment-dump gate** (`internal/danger/classifier.go`) — bare `env` and `printenv` invocations are classified as `system_write` because they can leak process-environment secrets that the redaction scanner does not recognise. `env VAR=value ` still classifies `` normally. - **Web UI attachment wrapping** (`cmd/odek/serve.go` + `cmd/odek/ui/app.js`) — files attached through the browser are sent separately from the user's text and wrapped with the nonce'd `` boundary (`source="attachment:"`) before injection into the prompt. diff --git a/cmd/odek/serve.go b/cmd/odek/serve.go index b23d3e2..0adb6fd 100644 --- a/cmd/odek/serve.go +++ b/cmd/odek/serve.go @@ -333,7 +333,7 @@ func serveCmd(args []string) error { } mux.Handle("/api/resources", apiAuth(handleResourceSearch(resourceReg))) mux.Handle("/api/sessions", apiAuth(handleSessionList(store))) - mux.Handle("/api/sessions/", apiAuth(handleSessionByID(store, resolved.TrustedProxies))) + mux.Handle("/api/sessions/", apiAuth(handleSessionByID(store, resolved.TrustedProxies, wsToken))) mux.Handle("/api/models", apiAuth(handleModelList(resolved.Model))) mux.Handle("/api/cancel", apiAuth(handleCancel(store))) @@ -1440,6 +1440,22 @@ func requireServeToken(token string) func(http.Handler) http.Handler { } } +// presentsInstanceTokenHeader reports whether the request carries the +// per-instance CSRF token in the X-Odek-Ws-Token header. Header presentation +// is a KNOWLEDGE proof — unlike the ambient SameSite=Strict cookie, a +// cross-origin (e.g. DNS-rebinding) page can neither read the token value +// nor set the header (the custom header triggers a CORS preflight, which +// odek does not answer). It therefore distinguishes legitimate front-ends +// (the WebUI, bodek, curl with the token URL) from a rebinding page that +// merely holds the cookie. +func presentsInstanceTokenHeader(r *http.Request, wsToken string) bool { + if wsToken == "" { + return false + } + h := r.Header.Get(wsTokenHeaderName) + return h != "" && subtle.ConstantTimeCompare([]byte(h), []byte(wsToken)) == 1 +} + // 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 { @@ -1541,7 +1557,7 @@ func handleSessionList(store *session.Store) http.HandlerFunc { } } -func handleSessionByID(store *session.Store, trustedProxies []string) http.HandlerFunc { +func handleSessionByID(store *session.Store, trustedProxies []string, wsToken string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { id := strings.TrimPrefix(r.URL.Path, "/api/sessions/") if id == "" { @@ -1564,6 +1580,17 @@ func handleSessionByID(store *session.Store, trustedProxies []string) http.Handl } token := sessionTokenFromRequest(r) effectiveToken, ok := validateSessionToken(store, sess, token) + if !ok && presentsInstanceTokenHeader(r, wsToken) { + // Bootstrap the session token for callers that prove + // knowledge of the per-instance CSRF token via the + // X-Odek-Ws-Token header. Without this, sessions created by + // one front-end (e.g. bodek, or a WebUI in another browser + // profile) can never be loaded by another: the per-session + // token only reaches the client that created the session, + // and the legacy bootstrap only covers sessions that have + // no token at all. + effectiveToken, ok = sess.AuthToken, true + } if !ok { http.Error(w, "invalid session token", http.StatusUnauthorized) return diff --git a/cmd/odek/serve_api_test.go b/cmd/odek/serve_api_test.go index cd7ff89..07b954b 100644 --- a/cmd/odek/serve_api_test.go +++ b/cmd/odek/serve_api_test.go @@ -70,7 +70,7 @@ func TestHandleSessionByID_GET_ReturnsSession(t *testing.T) { t.Fatalf("Create session: %v", err) } - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") req := httptest.NewRequest(http.MethodGet, "/api/sessions/"+sess.ID, nil) req.Header.Set("X-Session-Token", sess.AuthToken) w := httptest.NewRecorder() @@ -106,7 +106,7 @@ func TestHandleSessionByID_GET_ReturnsSession(t *testing.T) { func TestHandleSessionByID_GET_NotFound(t *testing.T) { store := newTestSessionStore(t) - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") // Valid ID format but doesn't exist on disk. req := httptest.NewRequest(http.MethodGet, "/api/sessions/20260101-aabbcc", nil) @@ -120,7 +120,7 @@ func TestHandleSessionByID_GET_NotFound(t *testing.T) { func TestHandleSessionByID_GET_MissingID(t *testing.T) { store := newTestSessionStore(t) - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") req := httptest.NewRequest(http.MethodGet, "/api/sessions/", nil) w := httptest.NewRecorder() @@ -145,7 +145,7 @@ func TestHandleSessionByID_GET_MessagesArePresent(t *testing.T) { t.Fatalf("Create: %v", err) } - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") req := httptest.NewRequest(http.MethodGet, "/api/sessions/"+sess.ID, nil) req.Header.Set("X-Session-Token", sess.AuthToken) w := httptest.NewRecorder() @@ -161,7 +161,7 @@ func TestHandleSessionByID_GET_MessagesArePresent(t *testing.T) { func TestHandleSessionByID_MethodNotAllowed(t *testing.T) { store := newTestSessionStore(t) - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") for _, method := range []string{http.MethodPatch, http.MethodPut, http.MethodHead} { req := httptest.NewRequest(method, "/api/sessions/20260101-aabbcc", nil) @@ -182,7 +182,7 @@ func TestHandleSessionByID_DELETE_StillWorks(t *testing.T) { t.Fatalf("Create: %v", err) } - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") req := httptest.NewRequest(http.MethodDelete, "/api/sessions/"+sess.ID, nil) req.Header.Set("X-Session-Token", sess.AuthToken) w := httptest.NewRecorder() @@ -209,7 +209,7 @@ func TestHandleSessionByID_POST_RenameStillWorks(t *testing.T) { t.Fatalf("Create: %v", err) } - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") body := strings.NewReader(`{"name":"renamed task"}`) req := httptest.NewRequest(http.MethodPost, "/api/sessions/"+sess.ID, body) req.Header.Set("Content-Type", "application/json") @@ -232,7 +232,7 @@ func TestHandleSessionByID_GET_InvalidToken(t *testing.T) { store := newTestSessionStore(t) sess, _ := store.Create([]llm.Message{{Role: "user", Content: "hi"}}, "m", "task") - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") req := httptest.NewRequest(http.MethodGet, "/api/sessions/"+sess.ID, nil) req.Header.Set("X-Session-Token", "wrong-token") w := httptest.NewRecorder() @@ -247,7 +247,7 @@ func TestHandleSessionByID_GET_MissingToken(t *testing.T) { store := newTestSessionStore(t) sess, _ := store.Create([]llm.Message{{Role: "user", Content: "hi"}}, "m", "task") - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") req := httptest.NewRequest(http.MethodGet, "/api/sessions/"+sess.ID, nil) w := httptest.NewRecorder() handler(w, req) @@ -268,7 +268,7 @@ func TestHandleSessionByID_GET_LazyTokenBootstrap(t *testing.T) { t.Fatalf("Save: %v", err) } - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") req := httptest.NewRequest(http.MethodGet, "/api/sessions/"+sess.ID, nil) w := httptest.NewRecorder() handler(w, req) @@ -297,11 +297,79 @@ func TestHandleSessionByID_GET_LazyTokenBootstrap(t *testing.T) { } } +func TestHandleSessionByID_GET_InstanceHeaderBootstrap(t *testing.T) { + // Cross-front-end interop: a session created by another client (e.g. + // bodek) carries an auth token the browser doesn't have. When the caller + // proves knowledge of the per-instance CSRF token via the + // X-Odek-Ws-Token header, the GET bootstraps the session token. + store := newTestSessionStore(t) + sess, _ := store.Create([]llm.Message{{Role: "user", Content: "hi"}}, "m", "task") + const wsToken = "test-instance-token" + + handler := handleSessionByID(store, nil, wsToken) + req := httptest.NewRequest(http.MethodGet, "/api/sessions/"+sess.ID, nil) + req.Header.Set(wsTokenHeaderName, wsToken) + w := httptest.NewRecorder() + handler(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String()) + } + if got := w.Header().Get("X-Session-Token"); got != sess.AuthToken { + t.Errorf("X-Session-Token = %q, want bootstrapped session token %q", got, sess.AuthToken) + } + + // The bootstrapped token works for subsequent requests (rename/delete). + req2 := httptest.NewRequest(http.MethodGet, "/api/sessions/"+sess.ID, nil) + req2.Header.Set("X-Session-Token", sess.AuthToken) + w2 := httptest.NewRecorder() + handler(w2, req2) + if w2.Code != http.StatusOK { + t.Errorf("second GET status = %d, want 200", w2.Code) + } +} + +func TestHandleSessionByID_GET_InstanceHeaderBootstrap_WrongInstanceToken(t *testing.T) { + // A wrong instance token proves nothing — no bootstrap. + store := newTestSessionStore(t) + sess, _ := store.Create([]llm.Message{{Role: "user", Content: "hi"}}, "m", "task") + + handler := handleSessionByID(store, nil, "real-token") + req := httptest.NewRequest(http.MethodGet, "/api/sessions/"+sess.ID, nil) + req.Header.Set(wsTokenHeaderName, "wrong-token") + w := httptest.NewRecorder() + handler(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("status = %d, want 401", w.Code) + } +} + +func TestHandleSessionByID_GET_NoBootstrapWithoutInstanceHeader(t *testing.T) { + // Ambient-cookie-only callers (e.g. a DNS-rebinding page that cannot set + // custom headers) must NOT get the bootstrap even with the instance + // token configured: without the X-Odek-Ws-Token header the request is + // indistinguishable from the pre-fix behavior. + store := newTestSessionStore(t) + sess, _ := store.Create([]llm.Message{{Role: "user", Content: "hi"}}, "m", "task") + + handler := handleSessionByID(store, nil, "real-token") + req := httptest.NewRequest(http.MethodGet, "/api/sessions/"+sess.ID, nil) + // No X-Session-Token, no X-Odek-Ws-Token — only a (valid) instance cookie. + req.AddCookie(&http.Cookie{Name: wsTokenCookieName, Value: "real-token"}) + w := httptest.NewRecorder() + handler(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("status = %d, want 401", w.Code) + } +} + func TestHandleSessionByID_DELETE_RequiresToken(t *testing.T) { store := newTestSessionStore(t) sess, _ := store.Create([]llm.Message{{Role: "user", Content: "hi"}}, "m", "task") - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") req := httptest.NewRequest(http.MethodDelete, "/api/sessions/"+sess.ID, nil) req.Header.Set("X-Session-Token", "wrong-token") w := httptest.NewRecorder() @@ -316,7 +384,7 @@ func TestHandleSessionByID_POST_RequiresToken(t *testing.T) { store := newTestSessionStore(t) sess, _ := store.Create([]llm.Message{{Role: "user", Content: "hi"}}, "m", "task") - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") body := strings.NewReader(`{"name":"renamed"}`) req := httptest.NewRequest(http.MethodPost, "/api/sessions/"+sess.ID, body) req.Header.Set("Content-Type", "application/json") @@ -336,7 +404,7 @@ func TestHandleSessionByID_GET_RateLimit(t *testing.T) { sessionLookupLimiter.reset() defer sessionLookupLimiter.reset() - handler := handleSessionByID(store, nil) + handler := handleSessionByID(store, nil, "") // Exhaust the 60/min allowance. for i := 0; i < 60; i++ { req := httptest.NewRequest(http.MethodGet, "/api/sessions/"+sess.ID, nil) @@ -741,7 +809,7 @@ func buildServeMuxWithSessionByID(t *testing.T, store *session.Store) (net.Liste apiAuth := func(h http.Handler) http.Handler { return requireServeToken(token)(requireLocalHost(requireLocalOrigin(h))) } - mux.Handle("/api/sessions/", apiAuth(handleSessionByID(store, nil))) + mux.Handle("/api/sessions/", apiAuth(handleSessionByID(store, nil, ""))) return ln, mux } diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 4d35926..154d4e5 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -467,6 +467,8 @@ The IP used for rate limiting is taken from `X-Forwarded-For` / `X-Real-Ip` only Legacy sessions created before this defense have no `AuthToken`; the first access bootstraps one and returns it to the client, preserving backward compatibility without weakening protection for newly created sessions. +**Cross-front-end bootstrap (header knowledge proof).** A session created by one front-end (e.g. bodek, or the WebUI in another browser profile) carries an `AuthToken` the other front-ends don't have, so it could not be loaded by them. `GET /api/sessions/` now also bootstraps the session token when the caller proves **knowledge of the per-instance CSRF token** by presenting it in the `X-Odek-Ws-Token` header (constant-time compared). Header presentation is a knowledge proof a cross-origin page cannot forge: a DNS-rebinding page holds only the ambient `SameSite=Strict` cookie and can neither read the token value nor set the custom header (it triggers a CORS preflight odek does not answer). Cookie-only callers therefore still get 401 on token-carrying sessions, while the operator's legitimate front-ends — which always send the header — can load each other's sessions. The rebinding damage boundary is unchanged. + ### 25. Skill and episode context wrapped as untrusted Skill content and retrieved session episodes are externally-sourced data that cross the trust boundary. Before injecting them as `system` messages, the auto-load path (`odek.go`) and the lazy-match path (the loop) pass them through the same nonce'd `` wrapper used for tool output; the `skill_load` tool output is wrapped the same way at registration. The skill manager already gates `NeedsReview`/tainted skills, and the memory manager filters tainted episodes from search, but the wrapper provides defense-in-depth so a compromised skill or episode cannot pose as trusted system instructions.