security: fix high-severity findings #50, #52, #53, #63, #64

- Default non-interactive policy is now deny (was allow).
- Strip project-level backend redirects: embedding, memory, sessions,
  skills.dirs/skills.embedding, telegram, web_search.
- Protect all ~/.odek trust anchors from generic file-tool writes:
  sessions/, audit/, plans/, schedules.json, schedule-state.json,
  mcp_approvals.json, mcp_tool_approvals.json, restart.json,
  telegram.lock, telegram.pid, schedule.pid, schedule.log.
- Add field-by-field skills overlay so project skills settings do not
  clobber global dirs/embedding.
- Update docs (SECURITY.md, CONFIG.md, CLI.md, AGENTS.md) and add
  regression tests.

Author: jkyberneees

AGENTS.md

@@ -133,14 +133,16 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
 - **Serve sandbox default-on** — `odek serve` enables `--sandbox` automatically unless `--no-sandbox` is passed.
 - **Sandbox volume confinement** (`internal/sandbox/sandbox.go`) — extra `--sandbox-volume` host paths must resolve to a location under the working directory, cannot contain `..` or symlink escapes, and cannot match sensitive prefixes such as `/etc`, `/proc`, `/sys`, `/dev`, `/root`, `/home`, `/var`, `/run`, or `/var/run/docker.sock`.
 - **Sandbox read-only enforcement** (`cmd/odek/sandbox_file.go` + `cmd/odek/file_tool.go` + `cmd/odek/perf_tools.go`) — when a sandbox container is active, `write_file`, `patch`, and `batch_patch` translate host paths to `/workspace/...` and copy data into the container with `docker cp`, so a read-only workspace mount (`--sandbox-readonly`) is enforced for the agent's own file tools.
-- **Project config sensitive-field rejection** (`internal/config/loader.go`) — `./odek.json` is untrusted, so `base_url`, `api_key`, `system`, and the `dangerous` section set there are ignored (with stderr warnings). These can only be configured from operator-controlled sources: `~/.odek/config.json`, `ODEK_*` env vars, or CLI flags.
+- **Project config sensitive-field rejection** (`internal/config/loader.go`) — `./odek.json` is untrusted, so `base_url`, `api_key`, `system`, the `dangerous` section, `embedding`, `memory`, `sessions`, `skills.dirs`/`skills.embedding`, `telegram`, and `web_search` set there are ignored (with stderr warnings). These can only be configured from operator-controlled sources: `~/.odek/config.json`, `ODEK_*` env vars, or CLI flags.
 - **MCP subprocess environment sanitisation** (`internal/mcpclient/client.go`) — MCP server children receive only a minimal allowlist of safe environment variables plus explicit `env` overrides. Keys matching secret patterns (`*_API_KEY`, `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, etc.) are stripped, preventing a compromised or malicious MCP server from reading parent secrets.
 - **MCP `tools/list` metadata hardening** (`internal/mcpclient/client.go`, `cmd/odek/mcp_approval.go`, `cmd/odek/main.go`) — tool names from MCP servers are validated (ASCII letters/digits/`-`/`_`, ≤ 64 chars) and descriptions are scanned for injection patterns. Tools whose raw names collide with odek built-ins are rejected. Each tool from a project-level server requires per-tool approval (interactive, `ODEK_APPROVE_MCP=1`, or persisted in `~/.odek/mcp_tool_approvals.json`); global servers from `~/.odek/config.json` are operator-trusted.
 - **Read-only perf-tool file-size cap** (`cmd/odek/perf_tools.go`) — `count_lines`, `checksum`, `head_tail`, and `word_count` reject files larger than 10 MiB before scanning/hashing, consistent with other perf tools.
 - **Inline content size cap** (`cmd/odek/perf_tools.go`) — `base64` and `tr` reject inline `string`/`content` arguments larger than 10 MiB, preventing prompt-injected multi-hundred-megabyte payloads from OOMing the process.
 - **Schedule atomic-write hardening** (`internal/schedule/store.go` + `internal/fsatomic`) — schedule file writes now use `fsatomic.WriteFile`, which creates a random temp file with `O_EXCL`, fsyncs data and directory, and renames over the target. A swapped-in symlink is replaced rather than followed, closing the symlink-override attack on `schedules.json` / `schedule-state.json`.
 - **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.
 - **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.

cmd/odek/browser_tool_test.go

@@ -6,8 +6,18 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+
+	"github.com/BackendStack21/odek/internal/danger"
 )
 
+// newTestBrowserTool returns a browserTool configured with non_interactive=allow
+// so unit tests that hit local httptest servers are not blocked by the default
+// deny policy.
+func newTestBrowserTool() *browserTool {
+	allow := "allow"
+	return newBrowserTool(danger.DangerousConfig{NonInteractive: &allow})
+}
+
 // ── Browser Navigate ──────────────────────────────────────────────────
 
 func TestBrowser_Navigate(t *testing.T) {
@@ -21,7 +31,7 @@ func TestBrowser_Navigate(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	result := callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)
 	var r struct {
 		Title   string `json:"title"`
@@ -49,7 +59,7 @@ func TestBrowser_Navigate(t *testing.T) {
 }
 
 func TestBrowser_Navigate_InvalidURL(t *testing.T) {
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	result := callJSON(t, b, `{"action":"navigate","url":"not-a-valid-url"}`)
 	var r struct {
 		Error string `json:"error"`
@@ -61,7 +71,7 @@ func TestBrowser_Navigate_InvalidURL(t *testing.T) {
 }
 
 func TestBrowser_Navigate_MissingURL(t *testing.T) {
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	result := callJSON(t, b, `{"action":"navigate"}`)
 	var r struct {
 		Error string `json:"error"`
@@ -79,7 +89,7 @@ func TestBrowser_Navigate_HTTPError(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	result := callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)
 	var r struct {
 		Content string `json:"content"`
@@ -105,7 +115,7 @@ func TestBrowser_Snapshot(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)
 
 	result := callJSON(t, b, `{"action":"snapshot"}`)
@@ -127,7 +137,7 @@ func TestBrowser_Snapshot(t *testing.T) {
 }
 
 func TestBrowser_Snapshot_NoPage(t *testing.T) {
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	result := callJSON(t, b, `{"action":"snapshot"}`)
 	var r struct {
 		Error string `json:"error"`
@@ -156,7 +166,7 @@ func TestBrowser_Click(t *testing.T) {
 		"/page2": `<html><body><h1>Page 2 Content</h1></body></html>`,
 	}
 
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)
 
 	// Click on the link
@@ -188,7 +198,7 @@ func TestBrowser_Click_InvalidRef(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)
 
 	result := callJSON(t, b, `{"action":"click","ref":"nonexistent"}`)
@@ -202,7 +212,7 @@ func TestBrowser_Click_InvalidRef(t *testing.T) {
 }
 
 func TestBrowser_Click_MissingRef(t *testing.T) {
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	result := callJSON(t, b, `{"action":"click"}`)
 	var r struct {
 		Error string `json:"error"`
@@ -219,7 +229,7 @@ func TestBrowser_Click_ButtonAcknowledges(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)
 
 	result := callJSON(t, b, `{"action":"click","ref":"e1"}`)
@@ -242,7 +252,7 @@ func TestBrowser_Click_InputSubmitAcknowledges(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)
 
 	result := callJSON(t, b, `{"action":"click","ref":"e1"}`)
@@ -275,7 +285,7 @@ func TestBrowser_Back(t *testing.T) {
 		"/page2": `<html><body><h1>Page 2</h1></body></html>`,
 	}
 
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)
 	callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`/page2"}`)
 
@@ -306,7 +316,7 @@ func TestBrowser_Back_NoHistory(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)
 
 	result := callJSON(t, b, `{"action":"back"}`)
@@ -322,7 +332,7 @@ func TestBrowser_Back_NoHistory(t *testing.T) {
 // ── Browser Unknown Action ────────────────────────────────────────────
 
 func TestBrowser_UnknownAction(t *testing.T) {
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	result := callJSON(t, b, `{"action":"fly"}`)
 	var r struct {
 		Error string `json:"error"`
@@ -336,7 +346,7 @@ func TestBrowser_UnknownAction(t *testing.T) {
 // ── Browser Schema ───────────────────────────────────────────────────
 
 func TestBrowser_Schema(t *testing.T) {
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	schema := b.Schema().(map[string]any)
 	props := schema["properties"].(map[string]any)
 
@@ -364,7 +374,7 @@ func TestBrowser_ExtractsInteractiveElements(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)
 
 	result := callJSON(t, b, `{"action":"snapshot"}`)
@@ -447,7 +457,7 @@ func TestResolveURL_SameDirRelative(t *testing.T) {
 // ── Browser Bad Action Parameters ─────────────────────────────────────
 
 func TestBrowser_Navigate_BadJSON(t *testing.T) {
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	result, err := b.Call(`{"action":"navigate","url":123}`)
 	if err != nil {
 		t.Fatalf("Call() error: %v", err)
@@ -467,7 +477,7 @@ func TestBrowser_WrapsLinkURL(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	b := &browserTool{}
+	b := newTestBrowserTool()
 	result := callJSON(t, b, `{"action":"navigate","url":"`+ts.URL+`"}`)
 
 	var r struct {

cmd/odek/file_tool.go

@@ -1091,9 +1091,44 @@ func confineToCWD(path string) (string, error) {
 // must not be writable through the generic file tools. Keep in sync with
 // the SystemWrite escalation in danger.ClassifyPath.
 func isProtectedOdekPath(rel string) bool {
-	return rel == "config.json" || rel == "secrets.env" ||
-		rel == "IDENTITY.md" ||
-		rel == "skills" || strings.HasPrefix(rel, "skills"+string(filepath.Separator))
+	rel = filepath.Clean(rel)
+
+	// Exact-file trust anchors. Rewriting any of these can disable safety
+	// policy, exfiltrate secrets, or persist attacker control.
+	protectedExact := []string{
+		"config.json",
+		"secrets.env",
+		"IDENTITY.md",
+		"schedules.json",
+		"schedule-state.json",
+		"schedules.lock",
+		"mcp_approvals.json",
+		"mcp_tool_approvals.json",
+		"restart.json",
+		"telegram.lock",
+		"telegram.pid",
+		"schedule.pid",
+		"schedule.log",
+	}
+	for _, p := range protectedExact {
+		if rel == p {
+			return true
+		}
+	}
+
+	// Directory trust anchors. Anything inside these directories is protected.
+	protectedDirs := []string{
+		"skills",   // auto-loaded into future prompts
+		"sessions", // conversation history & auth tokens
+		"audit",    // forensic audit log
+		"plans",    // persisted task plans
+	}
+	for _, d := range protectedDirs {
+		if rel == d || strings.HasPrefix(rel, d+string(filepath.Separator)) {
+			return true
+		}
+	}
+	return false
 }
 
 func truncateDiff(s string, maxLen int) string {

cmd/odek/file_tool_test.go

@@ -786,8 +786,8 @@ func TestConfineToCWD_AllowsOdekDir(t *testing.T) {
 	}
 	for _, p := range []string{
 		home + "/.odek/memory/episodes.json",
-		home + "/.odek/sessions/abc.json",
 		home + "/.odek/notes.md",
+		home + "/.odek/media/photo.jpg",
 	} {
 		if _, err := confineToCWD(p); err != nil {
 			t.Errorf("non-sensitive ~/.odek/ path %q should be allowed, got: %v", p, err)
@@ -812,9 +812,27 @@ func TestConfineToCWD_RejectsProtectedOdekPaths(t *testing.T) {
 		home + "/.odek/IDENTITY.md",
 		home + "/.odek/skills/evil/SKILL.md",
 		home + "/.odek/skills",
+		home + "/.odek/sessions/abc.json",
+		home + "/.odek/sessions",
+		home + "/.odek/audit/turn-1.json",
+		home + "/.odek/audit",
+		home + "/.odek/plans/evil.md",
+		home + "/.odek/plans",
+		home + "/.odek/schedules.json",
+		home + "/.odek/schedule-state.json",
+		home + "/.odek/schedules.lock",
+		home + "/.odek/mcp_approvals.json",
+		home + "/.odek/mcp_tool_approvals.json",
+		home + "/.odek/restart.json",
+		home + "/.odek/telegram.lock",
+		home + "/.odek/telegram.pid",
+		home + "/.odek/schedule.pid",
+		home + "/.odek/schedule.log",
 		// ".." traversal inside the carve-out must not reach the anchors
 		home + "/.odek/memory/../config.json",
 		home + "/.odek/memory/../IDENTITY.md",
+		home + "/.odek/memory/../schedules.json",
+		home + "/.odek/memory/../sessions/abc.json",
 	} {
 		if _, err := confineToCWD(p); err == nil {
 			t.Errorf("protected odek path %q should be rejected by confineToCWD", p)

cmd/odek/injection_hardening_test.go

@@ -186,7 +186,7 @@ func TestHTTPBatch_Redirect_BlockedTargetIsNotFetched(t *testing.T) {
 func TestBrowserClients_HaveCheckRedirectInstalled(t *testing.T) {
 	// Guards against a future refactor dropping the redirect guard from a
 	// client constructor (the original gap that motivated this fix).
-	bt := newBrowserTool(danger.DangerousConfig{})
+	bt := newTestBrowserTool()
 	if bt.client.CheckRedirect == nil {
 		t.Error("browser client is missing CheckRedirect — redirects would not be re-classified")
 	}
@@ -197,7 +197,7 @@ func TestBrowserClients_HaveCheckRedirectInstalled(t *testing.T) {
 }
 
 func TestCheckRedirect_EnforcesHopLimit(t *testing.T) {
-	bt := newBrowserTool(danger.DangerousConfig{}) // allow-all (no class overrides)
+	bt := newTestBrowserTool() // policy not exercised by this test
 	req, _ := http.NewRequest("GET", "http://example.com/", nil)
 	via := make([]*http.Request, 10)
 	if err := bt.checkRedirect(req, via); err == nil {

cmd/odek/main.go

@@ -656,7 +656,7 @@ const defaultConfigTemplate = `{
   "sandbox_volumes": [],
   "dangerous": {
     "action": "prompt",
-    "non_interactive": "allow",
+    "non_interactive": "deny",
     "classes": {
       "destructive": "deny",
       "network_egress": "prompt",

cmd/odek/next_security_vulnerabilities_test.go

@@ -27,7 +27,7 @@ func TestBrowser_HistoryCap(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	tool := newBrowserTool(danger.DangerousConfig{})
+	tool := newTestBrowserTool()
 	for i := 0; i < 55; i++ {
 		callJSON(t, tool, fmt.Sprintf(`{"action":"navigate","url":%q}`, srv.URL))
 	}
@@ -331,7 +331,8 @@ func TestShell_CapsOutputSize(t *testing.T) {
 	path := filepath.Join(dir, "huge.txt")
 	os.WriteFile(path, []byte(strings.Repeat("x", 15*1024*1024)), 0644)
 
-	tool := &shellTool{}
+	allow := "allow"
+	tool := &shellTool{dangerousConfig: danger.DangerousConfig{NonInteractive: &allow}}
 	tool.SetContext(context.Background())
 	result, err := tool.Call(fmt.Sprintf(`{"command":"cat %s","description":"read huge file"}`, path))
 	if err != nil {
@@ -349,7 +350,8 @@ func TestParallelShell_CapsOutputSize(t *testing.T) {
 	path := filepath.Join(dir, "huge.txt")
 	os.WriteFile(path, []byte(strings.Repeat("x", 15*1024*1024)), 0644)
 
-	tool := &parallelShellTool{}
+	allow := "allow"
+	tool := &parallelShellTool{dangerousConfig: danger.DangerousConfig{NonInteractive: &allow}}
 	result := callJSON(t, tool, fmt.Sprintf(`{"commands":[{"command":"cat %s"}]}`, path))
 	var r struct {
 		Results []struct {
@@ -381,7 +383,7 @@ func TestBrowser_NavigateTimeout(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	tool := newBrowserTool(danger.DangerousConfig{})
+	tool := newTestBrowserTool()
 	result := callJSON(t, tool, fmt.Sprintf(`{"action":"navigate","url":%q}`, srv.URL))
 	var r struct {
 		Error string `json:"error,omitempty"`
@@ -928,7 +930,7 @@ func TestBrowser_WrapsTitleAndElementText(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	tool := newBrowserTool(danger.DangerousConfig{})
+	tool := newTestBrowserTool()
 	result := callJSON(t, tool, fmt.Sprintf(`{"action":"navigate","url":%q}`, srv.URL))
 	var r struct {
 		Title    string `json:"title"`
@@ -961,7 +963,7 @@ func TestBrowser_CapsElementCount(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	tool := newBrowserTool(danger.DangerousConfig{})
+	tool := newTestBrowserTool()
 	result := callJSON(t, tool, fmt.Sprintf(`{"action":"navigate","url":%q}`, srv.URL))
 	var r struct {
 		Elements []any `json:"elements"`

cmd/odek/security_report_validation_test.go

@@ -92,7 +92,8 @@ func TestReport_BrowserWrapsUntrustedContent(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	bt := newBrowserTool(danger.DangerousConfig{})
+	allow := "allow"
+	bt := newBrowserTool(danger.DangerousConfig{NonInteractive: &allow})
 	resJSON, err := bt.doNavigate(ts.URL)
 	if err != nil {
 		t.Fatalf("doNavigate: %v", err)