From 9a88a37d2a80f574fb26d571ea0b2ffbd43811ae Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Sat, 18 Jul 2026 15:10:05 +0200 Subject: [PATCH] M5: classify MCP tool calls as unknown in batch gate --- AGENTS.md | 1 + docs/SECURITY.md | 6 ++++++ internal/loop/loop.go | 8 ++++++++ internal/loop/loop_test.go | 10 ++++++++++ 4 files changed, 25 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 3807381..dff18f1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -176,6 +176,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU - **Telegram chat-scoped sessions and plans** (`internal/telegram/session.go`, `internal/telegram/plan.go`, `cmd/odek/telegram.go`) — `/sessions`, `/resume`, `/prune`, `/plans`, `/plan_view`, `/plan_delete`, and `/plan_resume` are scoped to the requesting chat. Sessions are filtered by the `tg-` prefix, `ResumeSession` rejects cross-chat IDs, and plans live under `~/.odek/plans/chat/` so one chat cannot list, read, delete, or resume another chat's sessions/plans. - **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`. - **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/docs/SECURITY.md b/docs/SECURITY.md index 16aa5d2..d6d19ca 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -571,6 +571,12 @@ MCP servers supply not only tool descriptions but also `inputSchema` JSON schema - Caps the serialized schema JSON at 256 KiB per tool. Oversized schemas are rejected before they can be used for prompt stuffing. - Computes a SHA-256 hash of the canonical schema JSON and displays it in the interactive approval prompt (`schema: sha256: ( bytes)`), so the operator can notice when a previously-approved tool's schema has changed. +### 39f. MCP tool calls classified as `unknown` in the batch gate + +MCP tool names are registered as `__`. They were not handled by `internal/loop.classifyToolCall`, so the batch approval gate returned empty for them and the `SetTrustAll` path could silently approve whole batches that included MCP calls. In delegated (untrusted) sub-agents, the missing classification meant the documented forced-Deny damage cap never engaged for MCP tools. + +`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. + ### 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 7bcc854..2ac9049 100644 --- a/internal/loop/loop.go +++ b/internal/loop/loop.go @@ -1412,6 +1412,14 @@ func classifyToolCall(name, args string) (danger.RiskClass, string) { case "http_batch": return danger.NetworkEgress, args default: + // MCP tools are registered with names of the form __. + // They bypass the built-in danger classifier because the server, not + // odek, implements the Call() method. Treat them as Unknown so the + // batch gate shows them instead of auto-allowing, and so untrusted + // sub-agents force Deny for them. + if strings.Contains(name, "__") { + return danger.Unknown, name + } // For unrecognized tools, return empty — they are handled by // the tool's own Call() method individually. The batch gate // will skip them (no pre-classification available). diff --git a/internal/loop/loop_test.go b/internal/loop/loop_test.go index 88b298b..3f59eaf 100644 --- a/internal/loop/loop_test.go +++ b/internal/loop/loop_test.go @@ -1851,6 +1851,16 @@ func TestClassifyToolCall_UnknownTool(t *testing.T) { } } +func TestClassifyToolCall_MCPTool(t *testing.T) { + risk, resource := classifyToolCall("myserver__run_command", `{}`) + if risk != danger.Unknown { + t.Errorf("MCP tool risk = %q, want unknown", risk) + } + if resource != "myserver__run_command" { + t.Errorf("MCP tool resource = %q, want tool name", resource) + } +} + // ── Skills + Episode dedup regression tests ───────────────────────── // // TestEngine_SkillsAndEpisodesBothLoad verifies that when both skillLoader