fix(security): medium findings #65, #66, #70, #56

- #66: shell reads of ~/.odek trust anchors now classify as system_write
- #65: bare env/printenv environment dumps classify as system_write
- #70: Web UI file attachments wrapped as untrusted before prompt injection
- #56: per-instance random CSRF token required for /ws upgrades
  - token injected into index.html meta tag and HttpOnly SameSite=Strict cookie
  - accepted via cookie, X-Odek-Ws-Token header, or odek.<token> subprotocol
- added regression tests and updated SECURITY.md / AGENTS.md

Author: jkyberneees

AGENTS.md

@@ -99,7 +99,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/<pid>/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`.
-- **WS Origin allowlist** (`cmd/odek/serve.go::checkLocalOrigin`) — rejects non-localhost upgrades. Closes CSRF-on-localhost.
+- **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.
 - **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.
 - **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.
@@ -142,7 +142,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
 - **Schedule cross-process lock hard error** (`internal/schedule/store.go`) — `fileLock` now returns an error instead of silently falling back to a no-op releaser when `~/.odek/schedules.lock` cannot be opened or locked. Mutating schedule operations abort rather than risk two concurrent processes loading the same baseline and overwriting each other.
 - **Schedule JSON file-size cap** (`internal/schedule/store.go`) — `schedules.json` and `schedule-state.json` are rejected if larger than 10 MiB before being read into memory, preventing a tampered multi-gigabyte file from OOMing the scheduler.
 - **Default non-interactive policy is deny** (`internal/danger/classifier.go`, `cmd/odek/main.go`) — headless/CI/piped runs no longer auto-approve dangerous operations. Operators must explicitly set `non_interactive: "allow"` for unattended execution.
-- **`~/.odek` trust-anchor protection** (`internal/danger/classifier.go`, `cmd/odek/file_tool.go`) — generic file tools reject writes to `~/.odek/config.json`, `secrets.env`, `IDENTITY.md`, `skills/`, `sessions/`, `audit/`, `plans/`, `schedules.json`, `schedule-state.json`, `mcp_approvals.json`, `mcp_tool_approvals.json`, `restart.json`, `telegram.lock`, and related state files. These paths classify as `system_write` and must be modified through their dedicated subsystems.
+- **`~/.odek` trust-anchor protection** (`internal/danger/classifier.go`, `cmd/odek/file_tool.go`) — generic file tools reject writes to `~/.odek/config.json`, `secrets.env`, `IDENTITY.md`, `skills/`, `sessions/`, `audit/`, `plans/`, `schedules.json`, `schedule-state.json`, `mcp_approvals.json`, `mcp_tool_approvals.json`, `restart.json`, `telegram.lock`, and related state files. These paths classify as `system_write` and must be modified through their dedicated subsystems. Shell reads of these trust anchors are also escalated to `system_write`.
 - **Nonce'd tool-result delimiter** (`internal/loop/loop.go`) — the static `┌── TOOL RESULT: ... └── END TOOL RESULT: ...` delimiter is now unique per tool call via a random hex nonce embedded in both the opening and closing lines. A tool or MCP server can no longer forge the closing delimiter to break out of the data framing and inject instructions.
 - **`parallel_shell` context + process-group kill** (`cmd/odek/perf_tools.go`) — commands now run via `exec.CommandContext` bound to the agent context, in their own process group. Cancellation or timeout kills the whole group (negative PID), so `sh -c 'sleep 3600 &'` cannot leave orphaned children. Per-command timeouts are also capped at 30 minutes.
 - **`batch_patch` trusted-class propagation** (`cmd/odek/perf_tools.go`) — `batch_patch` now passes its cached `trustedClasses` to `CheckOperation`, matching `write_file` and `patch`. A trusted `local_write` class is honored across all patches in the batch instead of re-prompting per patch.
@@ -155,6 +155,8 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
 - **Telegram outbound media path allowlist** (`internal/telegram/media_path.go` + `internal/telegram/handler.go` + `internal/tool/send_message.go` + `cmd/odek/telegram.go`) — paths supplied to `MEDIA:...` prefixes or `send_message(file=...)` are resolved to an absolute path and verified against an allowlist (cwd, `~/.odek/media/`, system temp dir). `os.Lstat` rejects symlink final components and `filepath.EvalSymlinks` ensures the resolved path does not escape the allowlist, preventing prompt-injection-driven exfiltration of arbitrary files.
 - **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/<id>` 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.
 - **Skill/episode untrusted wrapper** (`internal/loop/loop.go` + `odek.go`) — skill context and retrieved session-episode context are passed through the caller-provided untrusted wrapper (the same nonce'd `<untrusted_content_*>` 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 <cmd>` still classifies `<cmd>` 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 `<untrusted_content_*>` boundary (`source="attachment:<filename>"`) before injection into the prompt.
 - **Episode index session ID validation** (`internal/memory/episode_index.go` + `internal/session/session.go`) — `readAllSummaries` treats `index.json` as untrusted input and validates every `session_id` with `session.ValidateSessionID` before building the `filepath.Join(dir, sessionID+".md")` path. Invalid / traversal / separator-containing IDs are skipped with a warning, preventing a tampered episode index from pulling arbitrary files (e.g. `~/.odek/config.json`, `IDENTITY.md`) into the embedding space.
 - **Secret redaction** (`internal/redact/redact.go`) — 20+ patterns: OpenAI, Anthropic, GitHub PAT, AWS, PEM, JWT, Vault, Google OAuth, SendGrid, Discord, DB URLs, etc.
 

cmd/odek/next_security_vulnerabilities_test.go

@@ -214,7 +214,7 @@ func TestServe_CSRF_AllowsLocalhostOrigin(t *testing.T) {
 func TestServe_StaticSecurityHeaders(t *testing.T) {
 	req := httptest.NewRequest(http.MethodGet, "/", nil)
 	rr := httptest.NewRecorder()
-	handleStatic().ServeHTTP(rr, req)
+	handleStatic("").ServeHTTP(rr, req)
 
 	if rr.Code != http.StatusOK {
 		t.Fatalf("static handler returned %d", rr.Code)

cmd/odek/serve.go

@@ -2,8 +2,10 @@ package main
 
 import (
 	"context"
+	"crypto/rand"
 	"crypto/subtle"
 	"embed"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"net"
@@ -256,10 +258,23 @@ func serveCmd(args []string) error {
 		resource.NewSessionResolver(filepath.Join(home, ".odek", "sessions")),
 	)
 
+	// Per-instance CSRF token for browser-driven WebSocket connections. A
+	// random token is issued at server start, injected into the served HTML,
+	// and also delivered as a SameSite=Strict HttpOnly cookie. The /ws
+	// handshake requires the token via cookie, header, or WebSocket
+	// subprotocol, so a page served by another localhost port cannot open
+	// an agent-controlling WebSocket.
+	wsToken, err := newServeToken()
+	if err != nil {
+		return fmt.Errorf("CSRF token: %w", err)
+	}
+
 	mux := http.NewServeMux()
-	mux.HandleFunc("/", handleStatic())
+	mux.HandleFunc("/", handleStatic(wsToken))
 	mux.Handle("/ws", &golangws.Server{
-		Handshake: wsHandshakeWithLimits,
+		Handshake: func(cfg *golangws.Config, req *http.Request) error {
+			return wsHandshakeWithLimits(cfg, req, wsToken)
+		},
 		Handler: func(conn *golangws.Conn) {
 			handleWS(store, resourceReg, resolved, systemMessage, conn)
 		},
@@ -550,13 +565,19 @@ func newServeAgent(resolved config.ResolvedConfig, system string, sendFn func(v
 
 // ── WebSocket Types ────────────────────────────────────────────────────
 
+type wsAttachment struct {
+	Name    string `json:"name"`
+	Content string `json:"content"`
+}
+
 type wsClientMsg struct {
-	Type      string `json:"type"`
-	Content   string `json:"content"`
-	SessionID string `json:"session_id"`
-	AuthToken string `json:"auth_token,omitempty"`
-	Model     string `json:"model,omitempty"`
-	Thinking  string `json:"thinking,omitempty"` // "enabled" | "" — per-query toggle
+	Type        string         `json:"type"`
+	Content     string         `json:"content"`
+	SessionID   string         `json:"session_id"`
+	AuthToken   string         `json:"auth_token,omitempty"`
+	Model       string         `json:"model,omitempty"`
+	Thinking    string         `json:"thinking,omitempty"` // "enabled" | "" — per-query toggle
+	Attachments []wsAttachment `json:"attachments,omitempty"`
 }
 
 // ── WebSocket Handler ──────────────────────────────────────────────────
@@ -773,7 +794,7 @@ func handleWS(store *session.Store, resources *resource.Registry, resolved confi
 		// Derived from connCtx so a disconnect also aborts the running prompt.
 		promptCtx, promptCancel := context.WithCancel(connCtx)
 
-		currentSession = handlePrompt(promptCtx, conn, store, resources, resolved, agent, currentSession, msg.Content, msg.SessionID, &sessionInputTokens, &sessionOutputTokens, promptCancel)
+		currentSession = handlePrompt(promptCtx, conn, store, resources, resolved, agent, currentSession, msg, &sessionInputTokens, &sessionOutputTokens, promptCancel)
 
 		// Cancel the prompt context once the run is complete.
 		promptCancel()
@@ -802,11 +823,13 @@ func handlePrompt(
 	resolved config.ResolvedConfig,
 	agent *odek.Agent,
 	currSess *session.Session,
-	prompt string,
-	sessionID string,
+	msg wsClientMsg,
 	sessionInputTokens, sessionOutputTokens *int,
 	promptCancel context.CancelFunc,
 ) *session.Session {
+	prompt := msg.Content
+	sessionID := msg.SessionID
+
 	// Resolve @ references
 	refs := resource.ParseRefs(prompt)
 	resolvedRefs := make(map[string]string)
@@ -819,6 +842,40 @@ func handlePrompt(
 	}
 	enrichedPrompt := resource.ReplaceRefs(prompt, resolvedRefs)
 
+	// Web UI file attachments cross the browser trust boundary. Wrap each one
+	// with the same nonce'd untrusted boundary used for tool output before
+	// injecting them into the prompt, so a malicious attachment cannot be
+	// mistaken for system instructions.
+	if len(msg.Attachments) > 0 {
+		const maxAttachmentBytes = 5 * 1024 * 1024
+		const maxTotalAttachmentBytes = 10 * 1024 * 1024
+		var total int
+		var wrapped []string
+		for _, att := range msg.Attachments {
+			if att.Name == "" || att.Content == "" {
+				continue
+			}
+			if len(att.Content) > maxAttachmentBytes {
+				writeWSError(conn, "attachment too large: "+att.Name)
+				return currSess
+			}
+			total += len(att.Content)
+			if total > maxTotalAttachmentBytes {
+				writeWSError(conn, "total attachment size exceeds 10 MB")
+				return currSess
+			}
+			header := "--- " + att.Name + " ---\n"
+			wrapped = append(wrapped, wrapUntrusted(ctx, "attachment:"+att.Name, header+att.Content))
+		}
+		if len(wrapped) > 0 {
+			if enrichedPrompt != "" {
+				enrichedPrompt = strings.Join(wrapped, "\n\n") + "\n\n" + enrichedPrompt
+			} else {
+				enrichedPrompt = strings.Join(wrapped, "\n\n")
+			}
+		}
+	}
+
 	// Load or create session
 	var sess *session.Session
 	var err error
@@ -1071,6 +1128,9 @@ func (w *wsStreamWriter) Write(p []byte) (int, error) {
 // approve dangerous tool calls. The default policy allows any port on
 // localhost / 127.0.0.1 / [::1] and an empty Origin (curl, native
 // clients). See IMPROVEMENTS_ROADMAP.md S-M1.
+//
+// Note: this check is now defense-in-depth. The primary CSRF protection is
+// the per-instance wsToken validated by validateServeToken.
 func checkLocalOrigin(_ *golangws.Config, req *http.Request) error {
 	origin := req.Header.Get("Origin")
 	if origin == "" {
@@ -1087,10 +1147,69 @@ func checkLocalOrigin(_ *golangws.Config, req *http.Request) error {
 	return fmt.Errorf("Origin %q not allowed (only localhost is accepted)", origin)
 }
 
-// wsHandshakeWithLimits wraps checkLocalOrigin with a per-IP upgrade rate limit
-// and a global concurrent-connection semaphore. The semaphore is acquired before
-// the WebSocket handshake completes and released when the handler exits.
-func wsHandshakeWithLimits(_ *golangws.Config, req *http.Request) error {
+const (
+	wsTokenCookieName     = "odek_ws_token"
+	wsTokenHeaderName     = "X-Odek-Ws-Token"
+	wsTokenProtocolPrefix = "odek."
+)
+
+// newServeToken generates a 256-bit random token used to authenticate
+// browser WebSocket upgrades. The token is issued once per server process.
+func newServeToken() (string, error) {
+	b := make([]byte, 32)
+	if _, err := rand.Read(b); err != nil {
+		return "", err
+	}
+	return hex.EncodeToString(b), nil
+}
+
+// validateServeToken verifies that the browser has the per-instance CSRF
+// token. It accepts the token via:
+//   - the HttpOnly SameSite=Strict cookie set when serving index.html
+//     (automatic for legitimate same-origin pages),
+//   - an X-Odek-Ws-Token header (for non-browser clients), or
+//   - a WebSocket subprotocol of the form "odek.<token>" (for clients that
+//     can set Sec-WebSocket-Protocol).
+func validateServeToken(cfg *golangws.Config, 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
+		}
+	}
+
+	// WebSocket subprotocol.
+	for _, p := range cfg.Protocol {
+		if strings.HasPrefix(p, wsTokenProtocolPrefix) {
+			got := strings.TrimPrefix(p, wsTokenProtocolPrefix)
+			if subtle.ConstantTimeCompare([]byte(got), []byte(token)) == 1 {
+				return nil
+			}
+		}
+	}
+
+	return fmt.Errorf("missing or invalid server token")
+}
+
+// wsHandshakeWithLimits validates the CSRF token and checks the origin, then
+// applies a per-IP upgrade rate limit and acquires the global
+// concurrent-connection semaphore. The semaphore is acquired before the
+// WebSocket handshake completes and released when the handler exits.
+func wsHandshakeWithLimits(cfg *golangws.Config, req *http.Request, token string) error {
+	if err := validateServeToken(cfg, req, token); err != nil {
+		return err
+	}
 	if err := checkLocalOrigin(nil, req); err != nil {
 		return err
 	}
@@ -1413,7 +1532,7 @@ var staticFiles = map[string][2]string{
 	"/app.js":    {"ui/app.js", "application/javascript; charset=utf-8"},
 }
 
-func handleStatic() http.HandlerFunc {
+func handleStatic(wsToken string) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		// Browsers auto-request favicon.ico — serve a minimal SVG inline.
 		if r.URL.Path == "/favicon.ico" {
@@ -1431,6 +1550,22 @@ func handleStatic() http.HandlerFunc {
 			http.Error(w, "not found", http.StatusNotFound)
 			return
 		}
+
+		// The HTML entry point gets the per-instance CSRF token both as a
+		// 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))
+		}
+
 		w.Header().Set("Content-Type", entry[1])
 		w.Header().Set("X-Content-Type-Options", "nosniff")
 		w.Header().Set("Referrer-Policy", "no-referrer")

cmd/odek/serve_api_test.go

@@ -582,11 +582,7 @@ func TestServe_E2E_NoRepetitiveResponses(t *testing.T) {
 	go serveOnListener(ln, mux)
 	waitForHTTP(t, ln.Addr().String())
 
-	wsURL := "ws://" + ln.Addr().String() + "/ws"
-	conn, err := golangws.Dial(wsURL, "", "http://localhost")
-	if err != nil {
-		t.Fatalf("Dial: %v", err)
-	}
+	conn := dialTestWS(t, ln.Addr().String())
 	defer conn.Close()
 
 	send := func(content string) {
@@ -665,11 +661,7 @@ func TestServe_E2E_SessionMessagesStoredWithoutSystemInjections(t *testing.T) {
 	go serveOnListener(ln, mux)
 	waitForHTTP(t, ln.Addr().String())
 
-	wsURL := "ws://" + ln.Addr().String() + "/ws"
-	conn, err := golangws.Dial(wsURL, "", "http://localhost")
-	if err != nil {
-		t.Fatalf("Dial: %v", err)
-	}
+	conn := dialTestWS(t, ln.Addr().String())
 	defer conn.Close()
 
 	// Send one prompt, collect session ID.

cmd/odek/serve_approval_test.go

@@ -44,10 +44,17 @@ func buildServeMuxPromptAll(t *testing.T, store *session.Store) (net.Listener, *
 		resource.NewSessionResolver(filepath.Join(home, ".odek", "sessions")),
 	)
 
+	wsToken, err := newServeToken()
+	if err != nil {
+		t.Fatalf("CSRF token: %v", err)
+	}
+
 	mux := http.NewServeMux()
-	mux.HandleFunc("/", handleStatic())
+	mux.HandleFunc("/", handleStatic(wsToken))
 	mux.Handle("/ws", &golangws.Server{
-		Handshake: func(*golangws.Config, *http.Request) error { return nil },
+		Handshake: func(cfg *golangws.Config, req *http.Request) error {
+			return wsHandshakeWithLimits(cfg, req, wsToken)
+		},
 		Handler: func(conn *golangws.Conn) {
 			handleWS(store, resourceReg, resolved, systemMessage, conn)
 		},
@@ -85,11 +92,7 @@ func TestServe_E2E_ApprovalRoundTrip(t *testing.T) {
 	go func() { _ = serveOnListener(ln, mux) }()
 	waitForHTTP(t, ln.Addr().String())
 
-	wsURL := "ws://" + ln.Addr().String() + "/ws"
-	conn, err := golangws.Dial(wsURL, "", "http://localhost")
-	if err != nil {
-		t.Fatalf("Dial(%q): %v", wsURL, err)
-	}
+	conn := dialTestWS(t, ln.Addr().String())
 	defer conn.Close()
 
 	prompt := map[string]string{"type": "prompt", "content": "run a command"}