fix: validate non_interactive and reject invalid values as deny (L-1) (#73)

Author: jkyberneees

AGENTS.md

@@ -158,7 +158,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
 - **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.
+- **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. Invalid values such as `"prompt"` are rejected at load time with a warning and floored to `"deny"`, because a non-interactive environment cannot prompt.
 - **`~/.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. Matching is case-insensitive so variants such as `CONFIG.JSON` or `SECRETS.ENV` are also blocked on case-insensitive filesystems (e.g. macOS APFS). 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.

docs/SECURITY.md

@@ -721,7 +721,7 @@ Defaults: `FrictionThreshold=3`, `FrictionWindow=60s`. To opt out (TTYApprover o
 
 When odek cannot open a TTY (headless/CI/piped input), prompted operations used to fall back to the `non_interactive` action. The built-in default was `"allow"`, so a prompt-injected task such as `echo "task" | odek run "download and run attacker.sh"` could silently execute `curl … | sh`.
 
-The default is now `"deny"`. Unattended runs must explicitly opt in to auto-approval by setting `"non_interactive": "allow"` in `~/.odek/config.json`, `ODEK_DANGEROUS_NON_INTERACTIVE=allow`, or the CLI. This makes the safe behaviour the default and closes the headless prompt-injection auto-execution vector.
+The default is now `"deny"`. Unattended runs must explicitly opt in to auto-approval by setting `"non_interactive": "allow"` in `~/.odek/config.json` or the CLI. Any other value — including the previously accepted `"prompt"` — is rejected at load time with a warning and treated as `"deny"`, because a non-interactive environment cannot prompt. This makes the safe behaviour the default and closes the headless prompt-injection auto-execution vector.
 
 ### 26. Generic file tools cannot write `~/.odek` trust anchors
 

internal/config/loader.go

@@ -1427,7 +1427,7 @@ func LoadConfig(cli CLIFlags) ResolvedConfig {
 		SandboxEnv:            cfg.SandboxEnv,
 		SandboxVolumes:        cfg.SandboxVolumes,
 		Skills:                resolveSkills(cfg.Skills),
-		Dangerous:             resolveDangerous(cfg.Dangerous),
+		Dangerous:             resolveDangerous(cfg.Dangerous, true),
 		Memory:                resolveMemory(cfg.Memory),
 		Guard:                 resolveGuard(cfg.Guard),
 		Embedding:             cfg.Embedding,
@@ -1632,11 +1632,22 @@ func resolveSkills(cfg *SkillsConfig) skills.SkillsConfig {
 
 // resolveDangerous merges file-level and potential env-level dangerous config.
 // If no config is provided, returns an empty DangerousConfig (safe defaults).
-func resolveDangerous(cfg *danger.DangerousConfig) danger.DangerousConfig {
-	if cfg != nil {
-		return *cfg
+// When validate is true, invalid non_interactive values are rejected with a
+// warning and forced to "deny" so headless runs cannot accidentally
+// auto-approve dangerous ops.
+func resolveDangerous(cfg *danger.DangerousConfig, validate bool) danger.DangerousConfig {
+	if cfg == nil {
+		return danger.DangerousConfig{}
 	}
-	return danger.DangerousConfig{}
+	resolved := *cfg
+	if validate && resolved.NonInteractive != nil {
+		if _, ok := danger.ParseNonInteractiveAction(*resolved.NonInteractive); !ok {
+			fmt.Fprintf(os.Stderr, "odek: warning: invalid non_interactive value %q — must be 'allow' or 'deny'; using 'deny'\n", *resolved.NonInteractive)
+			deny := "deny"
+			resolved.NonInteractive = &deny
+		}
+	}
+	return resolved
 }
 
 // resolveMemory merges file-level memory config with defaults.
@@ -1955,7 +1966,7 @@ func resolveSchedules(cfg *SchedulesConfig) ScheduleConfig {
 	}
 	out.TelegramAdminChats = cfg.TelegramAdminChats
 	out.TelegramAdminUsers = cfg.TelegramAdminUsers
-	out.Dangerous = resolveDangerous(cfg.Dangerous)
+	out.Dangerous = resolveDangerous(cfg.Dangerous, false)
 	return out
 }
 

internal/config/loader_test.go

@@ -223,6 +223,48 @@ func TestLoadConfig_GlobalFile(t *testing.T) {
 	}
 }
 
+func TestLoadConfig_InvalidNonInteractiveFlooredToDeny(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("HOME", dir)
+
+	cfgDir := filepath.Join(dir, ".odek")
+	os.MkdirAll(cfgDir, 0755)
+	cfgPath := filepath.Join(cfgDir, "config.json")
+	if err := os.WriteFile(cfgPath, []byte(`{
+		"dangerous": {
+			"non_interactive": "prompt"
+		}
+	}`), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	cfg := LoadConfig(CLIFlags{})
+	if cfg.Dangerous.NonInteractive == nil || *cfg.Dangerous.NonInteractive != "deny" {
+		t.Errorf("invalid non_interactive should be floored to 'deny', got %v", cfg.Dangerous.NonInteractive)
+	}
+}
+
+func TestLoadConfig_ValidNonInteractivePreserved(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("HOME", dir)
+
+	cfgDir := filepath.Join(dir, ".odek")
+	os.MkdirAll(cfgDir, 0755)
+	cfgPath := filepath.Join(cfgDir, "config.json")
+	if err := os.WriteFile(cfgPath, []byte(`{
+		"dangerous": {
+			"non_interactive": "allow"
+		}
+	}`), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	cfg := LoadConfig(CLIFlags{})
+	if cfg.Dangerous.NonInteractive == nil || *cfg.Dangerous.NonInteractive != "allow" {
+		t.Errorf("valid non_interactive='allow' should be preserved, got %v", cfg.Dangerous.NonInteractive)
+	}
+}
+
 func TestLoadConfig_ProjectOverridesGlobal(t *testing.T) {
 	dir := t.TempDir()
 

internal/danger/classifier.go

@@ -589,13 +589,33 @@ func (c *DangerousConfig) ActionForCommand(cmd string) Action {
 // NonInteractiveAction returns the action to use when no TTY is available.
 // Defaults to Deny so unattended/headless runs fail closed rather than
 // auto-approving dangerous operations.
+//
+// Only "allow" and "deny" are accepted; any other value (including "prompt")
+// is treated as "deny" because a non-interactive environment cannot prompt.
 func (c *DangerousConfig) NonInteractiveAction() Action {
 	if c.NonInteractive != nil {
-		return parseAction(*c.NonInteractive)
+		action, ok := ParseNonInteractiveAction(*c.NonInteractive)
+		if ok {
+			return action
+		}
 	}
 	return Deny
 }
 
+// ParseNonInteractiveAction parses the non_interactive config value. It accepts
+// only "allow" and "deny"; "prompt" and any other value are rejected because
+// prompting is impossible without a TTY.
+func ParseNonInteractiveAction(s string) (Action, bool) {
+	switch strings.ToLower(strings.TrimSpace(s)) {
+	case "allow":
+		return Allow, true
+	case "deny":
+		return Deny, true
+	default:
+		return Deny, false
+	}
+}
+
 // CheckOperation checks whether a tool operation is allowed, denied,
 // or needs approval. Returns nil on allow, error on deny, and prompts
 // the user on prompt. Uses the configured Approver when set; falls back

internal/danger/classifier_test.go

@@ -961,6 +961,46 @@ func TestNonInteractiveAction_Deny(t *testing.T) {
 	}
 }
 
+func TestNonInteractiveAction_InvalidFailsClosed(t *testing.T) {
+	// Any value other than "allow" or "deny" (including the previously
+	// accepted "prompt") must fail closed to Deny, because a non-interactive
+	// environment cannot prompt.
+	for _, s := range []string{"prompt", "maybe", "yes", "", "  prompt  "} {
+		s := s
+		t.Run(s, func(t *testing.T) {
+			cfg := &DangerousConfig{NonInteractive: &s}
+			if got := cfg.NonInteractiveAction(); got != Deny {
+				t.Errorf("non-interactive %q = %s, want deny", s, got)
+			}
+		})
+	}
+}
+
+func TestParseNonInteractiveAction(t *testing.T) {
+	cases := []struct {
+		input   string
+		want    Action
+		wantOK  bool
+	}{
+		{"allow", Allow, true},
+		{"deny", Deny, true},
+		{"ALLOW", Allow, true},
+		{"Deny", Deny, true},
+		{"prompt", Deny, false},
+		{"", Deny, false},
+		{"maybe", Deny, false},
+	}
+	for _, tc := range cases {
+		got, ok := ParseNonInteractiveAction(tc.input)
+		if ok != tc.wantOK {
+			t.Errorf("ParseNonInteractiveAction(%q) ok=%v, want %v", tc.input, ok, tc.wantOK)
+		}
+		if got != tc.want {
+			t.Errorf("ParseNonInteractiveAction(%q) = %s, want %s", tc.input, got, tc.want)
+		}
+	}
+}
+
 func TestActionFor_UnknownClass(t *testing.T) {
 	cfg := &DangerousConfig{}
 	action := cfg.ActionFor(RiskClass("nonexistent"))