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") + }), +)