M6: default missing sub-agent trust_level to untrusted; classify delegate_tasks as system_write

Author: jkyberneees

AGENTS.md

@@ -177,6 +177,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
 - **odek self-invocation gate** (`internal/danger/classifier.go`) — any shell stage whose program basename is `odek` is classified as `system_write`. This prevents a prompt-injected agent from reaching the human-gated trust mutations (`odek memory promote`, `odek memory extended confirm`, `odek skill promote --force`) through the shell tool and flipping its own taint gates.
 - **MCP inputSchema hardening** (`cmd/odek/mcp_approval.go`) — every string in an MCP tool's `inputSchema` is recursively guard-scanned for injection patterns; schemas larger than 256 KiB are rejected; the interactive approval prompt shows a SHA-256 hash and byte size of the schema so operators can detect changes.
 - **MCP tool batch classification** (`internal/loop/loop.go`) — MCP tools (`<server>__<tool>`) are classified as `unknown` by `classifyToolCall`, so the batch approval gate shows them and untrusted sub-agents force them to `deny`.
+- **Sub-agent trust defaults + delegate_tasks gate** (`cmd/odek/subagent.go`, `internal/loop/loop.go`) — a missing `trust_level` in `delegate_tasks` defaults to `untrusted`; `delegate_tasks` itself is classified as `system_write` so it requires explicit approval before spawning child processes.
 - **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>`, `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.
 - **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.

cmd/odek/subagent.go

@@ -572,14 +572,18 @@ func applySubagentTrust(dc *danger.DangerousConfig, trustLevel, maxRisk string)
 	if dc == nil {
 		return
 	}
-	if trustLevel == "" && maxRisk == "" {
-		return
-	}
 
 	if dc.Classes == nil {
 		dc.Classes = make(map[danger.RiskClass]danger.Action)
 	}
 
+	// Default to untrusted: if a parent agent omits trust_level, the sub-agent
+	// must not inherit the parent's TTY/approval context. This closes the path
+	// where a prompt-injected parent silently spawns a full-trust child.
+	if trustLevel == "" {
+		trustLevel = "untrusted"
+	}
+
 	if trustLevel == "untrusted" {
 		deny := "deny"
 		dc.NonInteractive = &deny

cmd/odek/subagent_trust_test.go

@@ -6,16 +6,27 @@ import (
 	"github.com/BackendStack21/odek/internal/danger"
 )
 
-// TestApplySubagentTrust_Noop confirms that with neither trust_level nor
-// max_risk set, the DangerousConfig is unchanged.
-func TestApplySubagentTrust_Noop(t *testing.T) {
+// TestApplySubagentTrust_EmptyDefaultsToUntrusted confirms that a missing
+// trust_level is treated as untrusted, so a parent cannot spawn a full-trust
+// sub-agent simply by omitting the field.
+func TestApplySubagentTrust_EmptyDefaultsToUntrusted(t *testing.T) {
 	dc := danger.DangerousConfig{}
 	applySubagentTrust(&dc, "", "")
-	if len(dc.Classes) != 0 {
-		t.Errorf("Classes mutated for noop call: %+v", dc.Classes)
+	if dc.NonInteractive == nil || *dc.NonInteractive != "deny" {
+		t.Errorf("NonInteractive should default to 'deny' when trust_level is empty, got %v", dc.NonInteractive)
 	}
-	if dc.NonInteractive != nil {
-		t.Errorf("NonInteractive mutated for noop call: %v", *dc.NonInteractive)
+	for _, cls := range []danger.RiskClass{
+		danger.Destructive,
+		danger.CodeExecution,
+		danger.Install,
+		danger.SystemWrite,
+		danger.NetworkEgress,
+		danger.Unknown,
+		danger.Blocked,
+	} {
+		if got := dc.Classes[cls]; got != danger.Deny {
+			t.Errorf("Class %s = %q, want %q when trust_level is empty", cls, got, danger.Deny)
+		}
 	}
 }
 

docs/SECURITY.md

@@ -577,6 +577,18 @@ MCP tool names are registered as `<server>__<tool>`. They were not handled by `i
 
 `classifyToolCall` now detects the `__` naming convention and returns `unknown` for any MCP tool. This makes the batch gate display the tool instead of hiding it, and `applySubagentTrust` forces `unknown` to `deny` for untrusted sub-agents.
 
+### 39g. Sub-agent trust defaults + `delegate_tasks` gate
+
+The `delegate_tasks` tool spawns child `odek subagent` processes. Previously:
+
+- `trust_level` was optional and `applySubagentTrust` no-oped when it was empty. A prompt-injected parent could omit the field and the child inherited the parent's TTY, so its approval prompts appeared indistinguishable from the parent's and its dangerous classes were not clamped.
+- `delegate_tasks` itself had no classification, so the batch approval gate did not surface it.
+
+Now:
+
+- `cmd/odek/subagent.go::applySubagentTrust` treats a missing `trust_level` as `untrusted`, forcing `non_interactive: deny` and denying Destructive/CodeExecution/Install/SystemWrite/NetworkEgress/Unknown/Blocked.
+- `internal/loop/loop.go::classifyToolCall` classifies `delegate_tasks` as `system_write`, so spawning sub-agents requires explicit operator approval and cannot be used to escape the parent's approval gate.
+
 ### 40. `/api/resources` result limit cap
 
 The `/api/resources?q=...&limit=N` autocomplete endpoint previously accepted any positive `limit` value. It is now capped to 100 results both in the HTTP handler and in `Registry.Search`. This prevents a prompt-injected or attacker-forged request from forcing an unbounded directory walk and returning a multi-megabyte JSON response.

internal/loop/loop.go

@@ -1411,6 +1411,12 @@ func classifyToolCall(name, args string) (danger.RiskClass, string) {
 		}
 	case "http_batch":
 		return danger.NetworkEgress, args
+	case "delegate_tasks":
+		// Spawning sub-agents is a trust-mutating operation: a compromised or
+		// prompt-injected parent can use it to escape its own approval gate by
+		// running commands in a child that shares the parent's terminal. Treat
+		// the call itself as system_write so it requires explicit approval.
+		return danger.SystemWrite, args
 	default:
 		// MCP tools are registered with names of the form <server>__<tool>.
 		// They bypass the built-in danger classifier because the server, not

internal/loop/loop_test.go

@@ -1861,6 +1861,16 @@ func TestClassifyToolCall_MCPTool(t *testing.T) {
 	}
 }
 
+func TestClassifyToolCall_DelegateTasks(t *testing.T) {
+	risk, resource := classifyToolCall("delegate_tasks", `{"tasks":[{"goal":"x"}]}`)
+	if risk != danger.SystemWrite {
+		t.Errorf("delegate_tasks risk = %q, want system_write", risk)
+	}
+	if resource == "" {
+		t.Error("delegate_tasks resource should not be empty")
+	}
+}
+
 // ── Skills + Episode dedup regression tests ─────────────────────────
 //
 // TestEngine_SkillsAndEpisodesBothLoad verifies that when both skillLoader