From 676e0587fc7e516626766086ab90fb8a43d39624 Mon Sep 17 00:00:00 2001 From: ollikurki Date: Mon, 1 Jun 2026 23:26:43 +0300 Subject: [PATCH] fix(opencode): inherit MCP tool allow permissions in subagent sessions Subagents spawned via the Task tool could see MCP tools in their tool registry but got permission denied when trying to execute them. The root cause was that deriveSubagentSessionPermission only forwarded deny rules and external_directory rules from the parent session, never copying the allow rules that MCP tools require. MCP tool permission keys contain underscores (e.g. context7_resolve-library-id, matrix_matrix_read) as they follow the sanitize(clientName) + '_' + sanitize(toolName) naming pattern. This fix copies all allow rules whose permission key contains an underscore, which covers MCP tools while leaving native tools (todowrite, task, bash, edit, etc.) unaffected since they don't contain underscores in their permission names. Wildcard '*' allow rules are also forwarded. Co-authored-by: Olli Kurki Co-authored-by: AI --- .../src/agent/subagent-permissions.ts | 14 ++- .../agent/plan-mode-subagent-bypass.test.ts | 105 ++++++++++++++++++ 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/packages/opencode/src/agent/subagent-permissions.ts b/packages/opencode/src/agent/subagent-permissions.ts index 051f42e37bb3..228711eb19cc 100644 --- a/packages/opencode/src/agent/subagent-permissions.ts +++ b/packages/opencode/src/agent/subagent-permissions.ts @@ -6,12 +6,16 @@ import type { Agent } from "./agent" * via the task tool. Combines: * * 1. The parent **agent's** edit-class deny rules — Plan Mode's file-edit - * restriction lives on the agent ruleset, not on the session, so a + * restriction lives on the agent ruleset, not the session, so a * subagent that only inherited the parent SESSION's permission would * silently bypass it. (#26514) * 2. The parent **session's** deny rules and external_directory rules — * same forwarding the original code already did. - * 3. Default `todowrite` and `task` denies if the subagent's own ruleset + * 3. The parent **session's** allow rules for MCP tools — subagents need + * explicit allow permissions to execute MCP tools (context7_resolve-library-id, + * matrix_matrix_read, etc.). Without this, subagents can see MCP tools in + * their tool list but get permission denied on execution. (#16491, #3808) + * 4. Default `todowrite` and `task` denies if the subagent's own ruleset * doesn't already permit them. */ export function deriveSubagentSessionPermission(input: { @@ -23,12 +27,16 @@ export function deriveSubagentSessionPermission(input: { const canTodo = input.subagent.permission.some((rule) => rule.permission === "todowrite") const parentAgentDenies = input.parentAgent?.permission.filter((rule) => rule.action === "deny" && rule.permission === "edit") ?? [] + const parentSessionMcpAllows = input.parentSessionPermission.filter( + (rule) => rule.action === "allow" && (rule.permission.includes("_") || rule.permission === "*"), + ) return [ ...parentAgentDenies, ...input.parentSessionPermission.filter( (rule) => rule.permission === "external_directory" || rule.action === "deny", ), + ...parentSessionMcpAllows, ...(canTodo ? [] : [{ permission: "todowrite" as const, pattern: "*" as const, action: "deny" as const }]), ...(canTask ? [] : [{ permission: "task" as const, pattern: "*" as const, action: "deny" as const }]), ] -} +} \ No newline at end of file diff --git a/packages/opencode/test/agent/plan-mode-subagent-bypass.test.ts b/packages/opencode/test/agent/plan-mode-subagent-bypass.test.ts index 07fb9a64d596..ebc309d9a8d5 100644 --- a/packages/opencode/test/agent/plan-mode-subagent-bypass.test.ts +++ b/packages/opencode/test/agent/plan-mode-subagent-bypass.test.ts @@ -210,3 +210,108 @@ it.effect("subagent inherits parent session deny rules as hard runtime ceilings" expect(Permission.evaluate("bash", "git status", effective).action).toBe("deny") }), ) + +it.effect("[#16491] subagent inherits parent session MCP tool allow rules", () => + Effect.sync(() => { + const executor = testAgent({ + name: "executor", + mode: "subagent", + permission: { + read: "allow", + }, + }) + + // Simulate a parent session that has allowed MCP tools. + // MCP tool permission keys use an underscore pattern: + // sanitize(clientName) + '_' + sanitize(toolName) + const parentWithMcpAllows: Permission.Ruleset = Permission.fromConfig({ + "myserver_tool-one": "allow", + "myserver_tool-two": "allow", + "otherclient_resource-list": "allow", + bash: "allow", + read: "allow", + }) + + const subagentSessionPermission = deriveSubagentSessionPermission({ + parentSessionPermission: parentWithMcpAllows, + parentAgent: undefined, + subagent: executor, + }) + + const effective = Permission.merge(executor.permission, subagentSessionPermission) + + // MCP tools (with underscores) should be allowed in the subagent + expect(Permission.evaluate("myserver_tool-one", "*", effective).action).toBe("allow") + expect(Permission.evaluate("myserver_tool-two", "*", effective).action).toBe("allow") + expect(Permission.evaluate("otherclient_resource-list", "*", effective).action).toBe("allow") + + // Native tools (no underscore) should NOT be inherited through the + // MCP allow filter. The subagent itself only allowed "read", so + // bash resolves to "ask" (the default) — not "deny" and not "allow". + // The parent session's bash:allow doesn't leak through because + // "bash" has no underscore in its permission key. + expect(Permission.evaluate("bash", "ls", effective).action).toBe("ask") + }), +) + +it.effect("[#16491] wildcard allow in parent session is inherited by subagent", () => + Effect.sync(() => { + const executor = testAgent({ + name: "executor", + mode: "subagent", + permission: { + read: "allow", + }, + }) + + // Parent session with wildcard allow (user accepted all tools) + const parentWithWildcard: Permission.Ruleset = Permission.fromConfig({ + "*": "allow", + }) + + const subagentSessionPermission = deriveSubagentSessionPermission({ + parentSessionPermission: parentWithWildcard, + parentAgent: undefined, + subagent: executor, + }) + + const effective = Permission.merge(executor.permission, subagentSessionPermission) + + // Wildcard allow should be inherited + expect(Permission.evaluate("context7_resolve-library-id", "*", effective).action).toBe("allow") + expect(Permission.evaluate("matrix_matrix_read", "*", effective).action).toBe("allow") + }), +) + +it.effect("[#16491] native tool deny rules still propagate and are not overridden by MCP allow rules", () => + Effect.sync(() => { + const executor = testAgent({ + name: "executor", + mode: "subagent", + permission: { + read: "allow", + }, + }) + + // Parent session: MCP tools allowed, but edit denied (e.g. read-only session) + const parentSession: Permission.Ruleset = Permission.fromConfig({ + "context7_resolve-library-id": "allow", + "matrix_matrix_read": "allow", + edit: { "*": "deny" }, + read: "allow", + }) + + const subagentSessionPermission = deriveSubagentSessionPermission({ + parentSessionPermission: parentSession, + parentAgent: undefined, + subagent: executor, + }) + + const effective = Permission.merge(executor.permission, subagentSessionPermission) + + // MCP tools allowed + expect(Permission.evaluate("context7_resolve-library-id", "*", effective).action).toBe("allow") + // Edit still denied from parent session + expect(Permission.evaluate("edit", "/some/file.ts", effective).action).toBe("deny") + }), +)