diff --git a/packages/opencode/src/agent/subagent-permissions.ts b/packages/opencode/src/agent/subagent-permissions.ts index b1b99b484e76..16742d7cfec4 100644 --- a/packages/opencode/src/agent/subagent-permissions.ts +++ b/packages/opencode/src/agent/subagent-permissions.ts @@ -5,11 +5,19 @@ 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 session's deny rules and external_directory rules. + * 1. The parent session's effective deny rules and external_directory rules. * Parent agent restrictions only govern that agent; the subagent's own * permissions determine its capabilities. * 2. Default `todowrite` and `task` denies if the subagent's own ruleset * doesn't already permit them. + * + * Session permission rules are append-only and evaluated last-match-wins, so + * a deny that a later rule supersedes for the same permission and pattern is + * no longer part of the parent's effective ceiling and must not be copied. + * A parent whose history is `[bash deny, bash allow]` is effectively allowed; + * copying the stale deny alone would leave every new subagent permanently + * denied. Denies are still inherited whenever the superseding rule targets a + * different pattern, keeping partial overrides conservative. */ export function deriveSubagentSessionPermission(input: { parentSessionPermission: PermissionV1.Ruleset @@ -17,9 +25,16 @@ export function deriveSubagentSessionPermission(input: { }): PermissionV1.Ruleset { const canTask = input.subagent.permission.some((rule) => rule.permission === "task") const canTodo = input.subagent.permission.some((rule) => rule.permission === "todowrite") + const rules = input.parentSessionPermission return [ - ...input.parentSessionPermission.filter( - (rule) => rule.permission === "external_directory" || rule.action === "deny", + ...rules.filter( + (rule, index) => + rule.permission === "external_directory" || + (rule.action === "deny" && + !rules.some( + (later, laterIndex) => + laterIndex > index && later.permission === rule.permission && later.pattern === rule.pattern, + )), ), ...(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/test/agent/plan-mode-subagent-bypass.test.ts b/packages/opencode/test/agent/plan-mode-subagent-bypass.test.ts index 6fc9197d9ae9..72f267cefd87 100644 --- a/packages/opencode/test/agent/plan-mode-subagent-bypass.test.ts +++ b/packages/opencode/test/agent/plan-mode-subagent-bypass.test.ts @@ -158,3 +158,87 @@ it.effect("subagent inherits parent session deny rules as hard runtime ceilings" expect(Permission.evaluate("bash", "git status", effective).action).toBe("deny") }), ) + +it.effect("subagent does not inherit a parent session deny that a later rule re-allowed", () => + Effect.sync(() => { + const executor = testAgent({ + name: "executor", + mode: "subagent", + permission: { + bash: "allow", + }, + }) + // Session rules are append-only with last-match-wins evaluation. A parent + // that appended `bash deny` (e.g. a plan-style restriction) and later + // `bash allow` is effectively allowed, so its new subagents must not be + // pinned to the stale deny. + const parentSessionPermission: PermissionV1.Ruleset = [ + { permission: "bash", pattern: "*", action: "deny" }, + { permission: "bash", pattern: "*", action: "allow" }, + ] + const derived = deriveSubagentSessionPermission({ + parentSessionPermission, + subagent: executor, + }) + const effective = Permission.merge(executor.permission, derived) + + expect(Permission.evaluate("bash", "git status", parentSessionPermission).action).toBe("allow") + expect(derived.filter((rule) => rule.permission === "bash")).toEqual([]) + expect(Permission.evaluate("bash", "git status", effective).action).toBe("allow") + }), +) + +it.effect("subagent keeps a parent session deny that a later rule only partially overrides", () => + Effect.sync(() => { + const executor = testAgent({ + name: "executor", + mode: "subagent", + permission: { + bash: "allow", + }, + }) + // The later allow targets a narrower pattern, so the broad deny is still + // part of the parent's ceiling for everything else. Inheriting it stays + // conservative: the subagent is denied even the narrower pattern. + const effective = Permission.merge( + executor.permission, + deriveSubagentSessionPermission({ + parentSessionPermission: [ + { permission: "bash", pattern: "*", action: "deny" }, + { permission: "bash", pattern: "git *", action: "allow" }, + ], + subagent: executor, + }), + ) + + expect(Permission.evaluate("bash", "rm -rf /tmp/x", effective).action).toBe("deny") + expect(Permission.evaluate("bash", "git status", effective).action).toBe("deny") + }), +) + +it.effect("re-allowing one permission does not drop unrelated parent session denies", () => + Effect.sync(() => { + const executor = testAgent({ + name: "executor", + mode: "subagent", + permission: { + bash: "allow", + edit: "allow", + }, + }) + const effective = Permission.merge( + executor.permission, + deriveSubagentSessionPermission({ + parentSessionPermission: [ + { permission: "bash", pattern: "*", action: "deny" }, + { permission: "edit", pattern: "*", action: "deny" }, + { permission: "bash", pattern: "*", action: "allow" }, + ], + subagent: executor, + }), + ) + + expect(Permission.evaluate("bash", "git status", effective).action).toBe("allow") + expect(Permission.evaluate("edit", "/some/file.ts", effective).action).toBe("deny") + }), +)