Skip to content

Commit 8062421

Browse files
authored
M5: classify MCP tool calls as unknown in batch gate (#62)
1 parent cb79f5a commit 8062421

4 files changed

Lines changed: 25 additions & 0 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
176176
- **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-<chatID>` prefix, `ResumeSession` rejects cross-chat IDs, and plans live under `~/.odek/plans/chat<chatID>/` so one chat cannot list, read, delete, or resume another chat's sessions/plans.
177177
- **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.
178178
- **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.
179+
- **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`.
179180
- **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.
180181
- **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.
181182
- **`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.

docs/SECURITY.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,12 @@ MCP servers supply not only tool descriptions but also `inputSchema` JSON schema
571571
- Caps the serialized schema JSON at 256 KiB per tool. Oversized schemas are rejected before they can be used for prompt stuffing.
572572
- Computes a SHA-256 hash of the canonical schema JSON and displays it in the interactive approval prompt (`schema: sha256:<hash> (<size> bytes)`), so the operator can notice when a previously-approved tool's schema has changed.
573573

574+
### 39f. MCP tool calls classified as `unknown` in the batch gate
575+
576+
MCP tool names are registered as `<server>__<tool>`. 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.
577+
578+
`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.
579+
574580
### 40. `/api/resources` result limit cap
575581

576582
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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,6 +1412,14 @@ func classifyToolCall(name, args string) (danger.RiskClass, string) {
14121412
case "http_batch":
14131413
return danger.NetworkEgress, args
14141414
default:
1415+
// MCP tools are registered with names of the form <server>__<tool>.
1416+
// They bypass the built-in danger classifier because the server, not
1417+
// odek, implements the Call() method. Treat them as Unknown so the
1418+
// batch gate shows them instead of auto-allowing, and so untrusted
1419+
// sub-agents force Deny for them.
1420+
if strings.Contains(name, "__") {
1421+
return danger.Unknown, name
1422+
}
14151423
// For unrecognized tools, return empty — they are handled by
14161424
// the tool's own Call() method individually. The batch gate
14171425
// will skip them (no pre-classification available).

internal/loop/loop_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1851,6 +1851,16 @@ func TestClassifyToolCall_UnknownTool(t *testing.T) {
18511851
}
18521852
}
18531853

1854+
func TestClassifyToolCall_MCPTool(t *testing.T) {
1855+
risk, resource := classifyToolCall("myserver__run_command", `{}`)
1856+
if risk != danger.Unknown {
1857+
t.Errorf("MCP tool risk = %q, want unknown", risk)
1858+
}
1859+
if resource != "myserver__run_command" {
1860+
t.Errorf("MCP tool resource = %q, want tool name", resource)
1861+
}
1862+
}
1863+
18541864
// ── Skills + Episode dedup regression tests ─────────────────────────
18551865
//
18561866
// TestEngine_SkillsAndEpisodesBothLoad verifies that when both skillLoader

0 commit comments

Comments
 (0)