Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions packages/opencode/src/session/prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,9 @@ NOTE: At any point in time through this workflow you should feel free to ask the
sessionID,
abort: taskAbort.signal,
callID: part.callID,
extra: { bypassAgentCheck: true, promptOps },
// #26597: ctx.agent here is the subtask's (child) agent, not the dispatcher.
// Pass the real caller so the agent tool can honor its edit restriction.
extra: { bypassAgentCheck: true, promptOps, callerAgent: lastUser.agent },
messages: msgs,
metadata: (val: { title?: string; metadata?: Record<string, any> }) =>
Effect.gen(function* () {
Expand Down Expand Up @@ -1877,8 +1879,30 @@ NOTE: At any point in time through this workflow you should feel free to ask the
permissions.push({ permission: t, action: enabled ? "allow" : "deny", pattern: "*" })
}
if (permissions.length > 0) {
session.permission = permissions
yield* sessions.setPermission({ sessionID: session.id, permission: permissions })
// #26597: the boolean tools map is availability-only — it lists the subagent's structural
// denies (agent, worktree, todowrite, primary_tools), not what it inherited from its
// caller. The caller's deny rules are the single source of truth for inheritance and live
// on session.permission, forwarded at dispatch (tool/agent.ts). Rebuilding from the map
// alone would drop them, letting a caller regain access through the child. For agent-tool
// children, carry forward external_directory rules plus every caller deny the map does NOT
// regenerate: scoped (non-"*") denies (e.g. edit on one path) and whole-tool denies for
// keys absent from the map — the wildcard "*" and any tool not listed (automate, MCP,
// custom). Per-tool "*" denies for keys the map lists are regenerated each turn, so
// dropping them keeps this stable instead of accumulating.
// NOTE: like upstream #26597 this is forward-deny only — a caller's allow exception (e.g.
// a read-only "*": deny agent that also allows read) is not preserved, so its subagent
// loses those tools too. Matching upstream's deriveSubagentSessionPermission; toward deny.
const toolKeys = new Set(Object.keys(input.tools ?? {}))
const preserved = session.createdByAgentTool
? (session.permission ?? []).filter(
(rule) =>
rule.permission === "external_directory" ||
(rule.action === "deny" && (rule.pattern !== "*" || !toolKeys.has(rule.permission))),
)
: []
const next = [...preserved, ...permissions]
session.permission = next
yield* sessions.setPermission({ sessionID: session.id, permission: next })
}

yield* throwIfAborted(options)
Expand Down
91 changes: 65 additions & 26 deletions packages/opencode/src/tool/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Session } from "../session"
import { SessionID, MessageID } from "../session/schema"
import { MessageV2 } from "../session/message-v2"
import { Agent } from "../agent/agent"
import type { Permission } from "../permission"
import type { SessionPrompt } from "../session/prompt"
import { Config } from "../config"
import { SubagentRun } from "../session/subagent-run"
Expand Down Expand Up @@ -299,41 +300,75 @@ export const AgentTool = Tool.define(
const parent = yield* sessions.get(ctx.sessionID)
const parentExec = parent.executionContext

// #26597: a subagent must not use a tool its caller is denied, otherwise a restricted
// agent (Plan Mode's edit-deny, or a read-only "*": deny agent) could escalate by
// spawning a more-capable subagent. Resolve the caller's agent so its deny rules can be
// forwarded onto the child session below. The caller is ctx.agent on a normal LLM
// dispatch; for a subtask command SessionPrompt.handleSubtask runs the agent tool as the
// child and passes the real caller via ctx.extra.callerAgent (PawWork sessions don't
// store their agent). agent.get returns undefined for an unknown name (handled by the
// optional chaining below); do NOT catch a genuine resolution failure — letting it
// propagate fails the dispatch closed rather than silently dropping the caller's deny
// rules, which would re-open the escalation this fix closes.
const callerAgentName = (ctx.extra?.callerAgent as string | undefined) ?? ctx.agent
const callerAgent = yield* agent.get(callerAgentName)

// #26597: the subagent's inherited permission — the single source of truth for what it
// may do. Forward the caller's deny rules with patterns intact so they bind the child
// the same way they bind the caller: the caller agent's restrictions (scoped denies
// like edit on one path, or a wildcard "*" deny) live on its agent ruleset, not the
// session, so they're forwarded explicitly alongside the caller session's denies +
// external_directory. The rebuild in SessionPrompt.prompt carries these forward
// verbatim — it only regenerates the per-tool "*" denies the tools map below lists. The
// trailing rules are the subagent's own structural shape (no nested dispatch, no todos
// unless its agent allows them, primary-tool allows).
const inheritedPermission: Permission.Ruleset = [
...(parent.permission ?? []).filter(
(rule) => rule.permission === "external_directory" || rule.action === "deny",
),
...(callerAgent?.permission ?? []).filter((rule) => rule.action === "deny"),
// v1 nested-deny: agent is denied unconditionally so a subagent cannot recursively
// dispatch its own subagents (#283 non-goal: nested subagents).
{
permission: id,
pattern: "*" as const,
action: "deny" as const,
},
...(canTodo
? []
: [
{
permission: "todowrite" as const,
pattern: "*" as const,
action: "deny" as const,
},
]),
...(cfg.experimental?.primary_tools?.map((item) => ({
pattern: "*",
action: "allow" as const,
permission: item,
})) ?? []),
]

const nextSession =
session ??
(yield* sessions.create({
parentID: ctx.sessionID,
title: params.description + ` (@${next.name} subagent)`,
createdByAgentTool: true,
subagentType: params.subagent_type,
permission: [
...(parent.permission ?? []).filter(
(rule) => rule.permission === "external_directory" || rule.action === "deny",
),
// v1 nested-deny: agent is denied unconditionally so a subagent cannot
// recursively dispatch its own subagents (#283 non-goal: nested subagents).
{
permission: id,
pattern: "*" as const,
action: "deny" as const,
},
...(canTodo
? []
: [
{
permission: "todowrite" as const,
pattern: "*" as const,
action: "deny" as const,
},
]),
...(cfg.experimental?.primary_tools?.map((item) => ({
pattern: "*",
action: "allow" as const,
permission: item,
})) ?? []),
],
permission: inheritedPermission,
}))

// #26597: resume (subagent_session_id) skips sessions.create, so re-forward the CURRENT
// caller's inherited permission onto the existing child. Otherwise a caller that became
// more restrictive after the child was created — e.g. switched to Plan Mode — could
// resume it and regain the denied tools, since the child still carried its original
// creator's permission. The rebuild then carries this forward as on a fresh dispatch.
if (session) {
yield* sessions.setPermission({ sessionID: nextSession.id, permission: inheritedPermission })
}

const childExec = nextSession.executionContext
const sameWorktree =
parentExec.activeWorktree?.directory === childExec.activeWorktree?.directory &&
Expand Down Expand Up @@ -441,6 +476,10 @@ export const AgentTool = Tool.define(
sessionID: nextSession.id,
model: { modelID: model.modelID, providerID: model.providerID },
agent: next.name,
// Availability-only: structural constraints on the subagent (no nested
// dispatch, no worktree switching, no todos unless its agent allows them, no
// primary-only tools). Caller-inherited denies ride on session.permission
// (forwarded above), not this map. See #26597.
tools: {
agent: false,
"enter-worktree": false,
Expand Down
168 changes: 168 additions & 0 deletions packages/opencode/test/session/prompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { NamedError } from "@opencode-ai/util/error"
import { fileURLToPath, pathToFileURL } from "url"
import { Effect, Layer } from "effect"
import { Instance } from "../../src/project/instance"
import { Permission } from "../../src/permission"
import { ModelID, ProviderID } from "../../src/provider/schema"
import { Session } from "../../src/session"
import { MessageV2 } from "../../src/session/message-v2"
Expand Down Expand Up @@ -1002,3 +1003,170 @@ describe("session.agent-resolution", () => {
}
}, 30000)
})

// #26597: the prompt rebuilds session.permission from the boolean tools map, which can only
// regenerate whole-tool ("*") rules for the keys it lists. For an agent-tool subagent it must
// carry forward the caller's inherited rules the map can't regenerate — scoped denies,
// external_directory rules, and whole-tool denies for keys the map doesn't list (the wildcard
// "*", MCP/custom tools) — otherwise a caller denied e.g. edit on one path, an external dir, or a
// whole tool regains it through the child. Whole-tool denies for keys the map DOES list are
// regenerated from the map instead, so the ruleset doesn't accumulate across turns.
describe("session.prompt subagent permission rebuild (#26597)", () => {
test("carries scoped denies and external_directory forward for an agent-tool subagent", async () => {
await using tmp = await tmpdir({
config: { agent: { build: { model: "openai/gpt-5.2" } } },
})
await Instance.provide({
directory: tmp.path,
fn: () =>
run(
Effect.gen(function* () {
const prompt = yield* SessionPrompt.Service
const sessions = yield* Session.Service
const parent = yield* sessions.create({})
const child = yield* sessions.create({
parentID: parent.id,
createdByAgentTool: true,
subagentType: "general",
permission: [
{ permission: "external_directory", pattern: "/tmp/project/*", action: "allow" },
{ permission: "edit", pattern: "/secret/**", action: "deny" },
{ permission: "edit", pattern: "*", action: "deny" },
],
})

yield* prompt.prompt({
sessionID: child.id,
agent: "build",
noReply: true,
tools: { agent: false, "enter-worktree": false },
parts: [{ type: "text", text: "x" }],
})

const after = yield* sessions.get(child.id)
// Scoped deny + external_directory survive (the boolean tools map can't express them).
expect(after.permission).toContainEqual({
permission: "external_directory",
pattern: "/tmp/project/*",
action: "allow",
})
expect(after.permission).toContainEqual({ permission: "edit", pattern: "/secret/**", action: "deny" })
// The structural denies the boolean tools map lists are regenerated from it.
expect(after.permission).toContainEqual({ permission: "agent", pattern: "*", action: "deny" })
// The whole-tool ("*") edit deny is ALSO carried forward: "edit" is absent from the
// tools map (which lists only agent/enter-worktree here), so the map can't regenerate
// it — dropping it would let the caller's edit deny vanish through the child. A
// whole-tool deny for a key the map DOES list is regenerated instead (next test).
expect(after.permission).toContainEqual({ permission: "edit", pattern: "*", action: "deny" })
}),
),
})
}, 30000)

test("regenerates a whole-tool deny the map lists instead of double-carrying it", async () => {
await using tmp = await tmpdir({
config: { agent: { build: { model: "openai/gpt-5.2" } } },
})
await Instance.provide({
directory: tmp.path,
fn: () =>
run(
Effect.gen(function* () {
const prompt = yield* SessionPrompt.Service
const sessions = yield* Session.Service
const parent = yield* sessions.create({})
const child = yield* sessions.create({
parentID: parent.id,
createdByAgentTool: true,
subagentType: "general",
permission: [{ permission: "edit", pattern: "*", action: "deny" }],
})

yield* prompt.prompt({
sessionID: child.id,
agent: "build",
noReply: true,
// "edit" is in the map, so its "*" deny is regenerated from the map — the forwarded
// copy is dropped from the carry-forward so the ruleset doesn't accumulate.
tools: { agent: false, edit: false },
parts: [{ type: "text", text: "x" }],
})

const after = yield* sessions.get(child.id)
expect((after.permission ?? []).filter((r) => r.permission === "edit" && r.pattern === "*")).toHaveLength(1)
expect(Permission.evaluate("edit", "*", after.permission ?? []).action).toBe("deny")
}),
),
})
}, 30000)

test("carries the caller's wildcard deny forward so tools absent from the map stay denied", async () => {
await using tmp = await tmpdir({
config: { agent: { build: { model: "openai/gpt-5.2" } } },
})
await Instance.provide({
directory: tmp.path,
fn: () =>
run(
Effect.gen(function* () {
const prompt = yield* SessionPrompt.Service
const sessions = yield* Session.Service
const parent = yield* sessions.create({})
// A read-only-style caller forwards a wildcard ("*") deny onto the child.
const child = yield* sessions.create({
parentID: parent.id,
createdByAgentTool: true,
subagentType: "general",
permission: [{ permission: "*", pattern: "*", action: "deny" }],
})

yield* prompt.prompt({
sessionID: child.id,
agent: "build",
noReply: true,
tools: { agent: false, edit: false },
parts: [{ type: "text", text: "x" }],
})

const after = yield* sessions.get(child.id)
// The wildcard deny is preserved, so a tool absent from the boolean tools map
// (automate, MCP, custom) still evaluates to deny for the subagent.
expect(after.permission).toContainEqual({ permission: "*", pattern: "*", action: "deny" })
expect(Permission.evaluate("automate", "*", after.permission ?? []).action).toBe("deny")
}),
),
})
}, 30000)

test("replaces permission wholesale for a non-agent-tool session", async () => {
await using tmp = await tmpdir({
config: { agent: { build: { model: "openai/gpt-5.2" } } },
})
await Instance.provide({
directory: tmp.path,
fn: () =>
run(
Effect.gen(function* () {
const prompt = yield* SessionPrompt.Service
const sessions = yield* Session.Service
const session = yield* sessions.create({
permission: [{ permission: "edit", pattern: "/secret/**", action: "deny" }],
})

yield* prompt.prompt({
sessionID: session.id,
agent: "build",
noReply: true,
tools: { agent: false },
parts: [{ type: "text", text: "x" }],
})

const after = yield* sessions.get(session.id)
// Not an agent-tool child → the rebuild replaces wholesale (pre-existing behavior).
expect(after.permission).not.toContainEqual({ permission: "edit", pattern: "/secret/**", action: "deny" })
expect(after.permission).toContainEqual({ permission: "agent", pattern: "*", action: "deny" })
}),
),
})
}, 30000)
})
Loading
Loading