From b8ca71d309349147c160105d9c171b850b138f2e Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 9 May 2026 19:51:55 -0400 Subject: [PATCH] fix(task): subagent inherits parent agent's deny rules (Plan Mode security bypass) (#26597) Co-authored-by: Developer --- .../src/agent/subagent-permissions.ts | 33 ++++ packages/opencode/src/tool/task.ts | 37 ++--- .../agent/plan-mode-subagent-bypass.test.ts | 141 ++++++++++++++++++ 3 files changed, 185 insertions(+), 26 deletions(-) create mode 100644 packages/opencode/src/agent/subagent-permissions.ts create mode 100644 packages/opencode/test/agent/plan-mode-subagent-bypass.test.ts diff --git a/packages/opencode/src/agent/subagent-permissions.ts b/packages/opencode/src/agent/subagent-permissions.ts new file mode 100644 index 0000000000..1174ec31ad --- /dev/null +++ b/packages/opencode/src/agent/subagent-permissions.ts @@ -0,0 +1,33 @@ +import type { Permission } from "../permission" +import type { Agent } from "./agent" + +/** + * Build the `permission` ruleset for a subagent's session when it's spawned + * via the task tool. Combines: + * + * 1. The parent **agent's** deny rules — Plan Mode and other agent-level + * restrictions live on the agent ruleset, not on the session, so a + * subagent that only inherited the parent SESSION's permission would + * silently bypass them. (#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 + * doesn't already permit them. + */ +export function deriveSubagentSessionPermission(input: { + parentSessionPermission: Permission.Ruleset + parentAgent: Agent.Info | undefined + subagent: Agent.Info +}): Permission.Ruleset { + const canTask = input.subagent.permission.some((rule) => rule.permission === "task") + const canTodo = input.subagent.permission.some((rule) => rule.permission === "todowrite") + const parentAgentDenies = input.parentAgent?.permission.filter((rule) => rule.action === "deny") ?? [] + return [ + ...parentAgentDenies, + ...input.parentSessionPermission.filter( + (rule) => rule.permission === "external_directory" || rule.action === "deny", + ), + ...(canTodo ? [] : [{ permission: "todowrite" as const, pattern: "*" as const, action: "deny" as const }]), + ...(canTask ? [] : [{ permission: "task" as const, pattern: "*" as const, action: "deny" as const }]), + ] +} diff --git a/packages/opencode/src/tool/task.ts b/packages/opencode/src/tool/task.ts index 22e4e5671c..c4d5bf7f4a 100644 --- a/packages/opencode/src/tool/task.ts +++ b/packages/opencode/src/tool/task.ts @@ -4,6 +4,7 @@ import { Session } from "@/session/session" import { SessionID, MessageID } from "../session/schema" import { MessageV2 } from "../session/message-v2" import { Agent } from "../agent/agent" +import { deriveSubagentSessionPermission } from "../agent/subagent-permissions" import type { SessionPrompt } from "../session/prompt" import { Config } from "@/config/config" import { Effect, Exit, Schema } from "effect" @@ -58,41 +59,25 @@ export const TaskTool = Tool.define( return yield* Effect.fail(new Error(`Unknown agent type: ${params.subagent_type} is not a valid agent type`)) } - const canTask = next.permission.some((rule) => rule.permission === id) - const canTodo = next.permission.some((rule) => rule.permission === "todowrite") - const taskID = params.task_id const session = taskID ? yield* sessions.get(SessionID.make(taskID)).pipe(Effect.catchCause(() => Effect.succeed(undefined))) : undefined const parent = yield* sessions.get(ctx.sessionID) + const parentAgent = parent.agent + ? yield* agent.get(parent.agent).pipe(Effect.catchCause(() => Effect.succeed(undefined))) + : undefined const nextSession = session ?? (yield* sessions.create({ parentID: ctx.sessionID, title: params.description + ` (@${next.name} subagent)`, permission: [ - ...(parent.permission ?? []).filter( - (rule) => rule.permission === "external_directory" || rule.action === "deny", - ), - ...(canTodo - ? [] - : [ - { - permission: "todowrite" as const, - pattern: "*" as const, - action: "deny" as const, - }, - ]), - ...(canTask - ? [] - : [ - { - permission: id, - pattern: "*" as const, - action: "deny" as const, - }, - ]), + ...deriveSubagentSessionPermission({ + parentSessionPermission: parent.permission ?? [], + parentAgent, + subagent: next, + }), ...(cfg.experimental?.primary_tools?.map((item) => ({ pattern: "*", action: "allow" as const, @@ -144,8 +129,8 @@ export const TaskTool = Tool.define( }, agent: next.name, tools: { - ...(canTodo ? {} : { todowrite: false }), - ...(canTask ? {} : { task: false }), + ...(next.permission.some((rule) => rule.permission === "todowrite") ? {} : { todowrite: false }), + ...(next.permission.some((rule) => rule.permission === id) ? {} : { task: false }), ...Object.fromEntries((cfg.experimental?.primary_tools ?? []).map((item) => [item, false])), }, parts, diff --git a/packages/opencode/test/agent/plan-mode-subagent-bypass.test.ts b/packages/opencode/test/agent/plan-mode-subagent-bypass.test.ts new file mode 100644 index 0000000000..5ba6b54834 --- /dev/null +++ b/packages/opencode/test/agent/plan-mode-subagent-bypass.test.ts @@ -0,0 +1,141 @@ +/** + * Reproducer for opencode issue #26514: + * + * In Plan Mode (the `plan` agent), the main agent's edit/write tools are + * blocked by the plan agent's permission ruleset (`edit: { "*": "deny" }`). + * However, when the plan agent spawns a subagent via the `task` tool, the + * subagent retains full file modification capabilities — a security bypass. + * + * This test replicates the permission ruleset that would govern a + * `general` subagent when launched from a `plan` parent session, mirroring + * the logic in `src/tool/task.ts` (filtered parent permissions ++ runtime + * subagent agent permissions, evaluated as in `session/prompt.ts`). + * + * The expected (secure) behavior is that the subagent inherits the plan + * mode read-only restriction and `edit`/`write` resolve to `deny`. On + * origin/dev this assertion fails because the parent **agent** permissions + * are not propagated to the subagent — only the parent **session** + * permissions are passed through, and Plan Mode's restrictions live on the + * agent, not the session. + */ +import { test, expect, afterEach } from "bun:test" +import { Effect } from "effect" +import { disposeAllInstances, provideInstance, tmpdir } from "../fixture/fixture" +import { WithInstance } from "../../src/project/with-instance" +import { Agent } from "../../src/agent/agent" +import { deriveSubagentSessionPermission } from "../../src/agent/subagent-permissions" +import { Permission } from "../../src/permission" + +afterEach(async () => { + await disposeAllInstances() +}) + +function load(dir: string, fn: (svc: Agent.Interface) => Effect.Effect) { + return Effect.runPromise(provideInstance(dir)(Agent.Service.use(fn)).pipe(Effect.provide(Agent.defaultLayer))) +} + +// `deriveSubagentSessionPermission` is imported from production. The test +// exercises the actual helper that task.ts uses to build the subagent's +// session permission, so any regression in that helper trips this test. + +test("[#26514] subagent spawned from plan mode inherits read-only restriction (edit denied)", async () => { + await using tmp = await tmpdir() + await WithInstance.provide({ + directory: tmp.path, + fn: async () => { + const planAgent = await load(tmp.path, (svc) => svc.get("plan")) + const generalAgent = await load(tmp.path, (svc) => svc.get("general")) + + expect(planAgent).toBeDefined() + expect(generalAgent).toBeDefined() + // Sanity: the plan agent itself blocks edit. (Note: `write` and + // `apply_patch` route through the `edit` permission at the runtime + // tool layer — see Permission.disabled / EDIT_TOOLS.) + expect(Permission.evaluate("edit", "/some/file.ts", planAgent!.permission).action).toBe("deny") + + // Simulate the plan-mode parent session: in real flow the plan + // session's `permission` field is empty (Plan Mode lives on the agent + // ruleset, not the session). So we pass [] through as the parent + // session permission, exactly like the actual code path. + const parentSessionPermission: Permission.Ruleset = [] + + const subagentSessionPermission = deriveSubagentSessionPermission({ + parentSessionPermission, + parentAgent: planAgent, + subagent: generalAgent!, + }) + + // Mirror the runtime evaluation in session/prompt.ts (~line 410, 639): + // ruleset: Permission.merge(agent.permission, session.permission ?? []) + const effective = Permission.merge(generalAgent!.permission, subagentSessionPermission) + + expect(Permission.evaluate("edit", "/some/file.ts", effective).action).toBe("deny") + expect(Permission.evaluate("edit", "/another/path/index.tsx", effective).action).toBe("deny") + }, + }) +}) + +test("[#26514] explore subagent launched from plan mode also stays read-only", async () => { + // Sibling check: even though `explore` is intrinsically read-only, the + // bug surface is the same. Including this case to document that the fix + // should propagate the parent **agent** permissions, not just deny edit + // when the subagent happens to already deny it. + await using tmp = await tmpdir() + await WithInstance.provide({ + directory: tmp.path, + fn: async () => { + const planAgent = await load(tmp.path, (svc) => svc.get("plan")) + const explore = await load(tmp.path, (svc) => svc.get("explore")) + expect(planAgent).toBeDefined() + expect(explore).toBeDefined() + + const parentSessionPermission: Permission.Ruleset = [] + const subagentSessionPermission = deriveSubagentSessionPermission({ + parentSessionPermission, + parentAgent: planAgent, + subagent: explore!, + }) + const effective = Permission.merge(explore!.permission, subagentSessionPermission) + + // Already deny — sanity check. + expect(Permission.evaluate("edit", "/x.ts", effective).action).toBe("deny") + }, + }) +}) + +test("[#26514] custom user subagent launched from plan mode bypasses Plan Mode read-only", async () => { + // The most damaging case: a user-defined subagent with default + // permissions (allow-by-default, like `general`). The subagent must NOT + // be able to edit when the parent agent is `plan`. + await using tmp = await tmpdir({ + config: { + agent: { + my_subagent: { + description: "A user-defined subagent", + mode: "subagent", + }, + }, + }, + }) + await WithInstance.provide({ + directory: tmp.path, + fn: async () => { + const planAgent = await load(tmp.path, (svc) => svc.get("plan")) + const my = await load(tmp.path, (svc) => svc.get("my_subagent")) + expect(planAgent).toBeDefined() + expect(my).toBeDefined() + + const parentSessionPermission: Permission.Ruleset = [] + const subagentSessionPermission = deriveSubagentSessionPermission({ + parentSessionPermission, + parentAgent: planAgent, + subagent: my!, + }) + const effective = Permission.merge(my!.permission, subagentSessionPermission) + + // BUG: on origin/dev edit resolves to "allow" because the plan + // agent's `edit: deny *` rule never reaches the subagent. + expect(Permission.evaluate("edit", "/some/file.ts", effective).action).toBe("deny") + }, + }) +})