From 0d421b861c4c594d64c68cf706d17ac029fdad6f Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Sat, 18 Jul 2026 15:14:02 +0200 Subject: [PATCH] M6: default missing sub-agent trust_level to untrusted; classify delegate_tasks as system_write --- AGENTS.md | 1 + cmd/odek/subagent.go | 10 +++++++--- cmd/odek/subagent_trust_test.go | 25 ++++++++++++++++++------- docs/SECURITY.md | 12 ++++++++++++ internal/loop/loop.go | 6 ++++++ internal/loop/loop_test.go | 10 ++++++++++ 6 files changed, 54 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index dff18f1..f36da85 100644 --- a/AGENTS.md +++ b/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 (`__`) 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/`, `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 `` 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. diff --git a/cmd/odek/subagent.go b/cmd/odek/subagent.go index 4f3d684..b9ebd1a 100644 --- a/cmd/odek/subagent.go +++ b/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 diff --git a/cmd/odek/subagent_trust_test.go b/cmd/odek/subagent_trust_test.go index 87345f3..a93bb39 100644 --- a/cmd/odek/subagent_trust_test.go +++ b/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) + } } } diff --git a/docs/SECURITY.md b/docs/SECURITY.md index d6d19ca..fe16062 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -577,6 +577,18 @@ MCP tool names are registered as `__`. 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. diff --git a/internal/loop/loop.go b/internal/loop/loop.go index 2ac9049..18f14a9 100644 --- a/internal/loop/loop.go +++ b/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 __. // They bypass the built-in danger classifier because the server, not diff --git a/internal/loop/loop_test.go b/internal/loop/loop_test.go index 3f59eaf..c3c8935 100644 --- a/internal/loop/loop_test.go +++ b/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