Skip to content

Commit 42e6774

Browse files
committed
fix(tool): subagent inherits the caller's tool denies (privilege escalation)
A restricted agent's tool restrictions live on its agent ruleset, but a dispatched subagent ran with its own (often broader) permissions — so a read-only or Plan-Mode caller could escalate by spawning a more-capable subagent that edits files, runs bash, or reaches the network. Semantic port of anomalyco/opencode#26597 ("subagent inherits parent agent's deny rules"), adapted to PawWork's permission model. The subagent's session.permission ruleset is the single source of truth for what it inherits. At dispatch the agent tool forwards the caller's deny rules onto the child session — patterns intact — so scoped denies (edit on one path), whole-tool denies, and wildcard ("*") denies all reach the child as real rules. Both the availability gate (Permission.disabled hides whole-tool denies from the model) and the ask gate evaluate merge(subagentAgent.permission, session.permission), so a forwarded deny binds the child the same way it bound the caller. The caller agent's restrictions live on its agent ruleset, not the session, so they are forwarded explicitly; session/per-prompt denies and external_directory rules carry over too. SessionPrompt.prompt still rebuilds session.permission from the boolean tools map the agent tool passes, but that map is now availability-only — it lists the subagent's structural constraints (no nested dispatch, no worktree switching, no todos/primary-only tools), not caller-inherited authorization. So for an agent-tool child the rebuild carries forward the caller's inherited rules the map can't regenerate: external_directory rules, scoped (non-"*") denies, and whole-tool denies for keys the map doesn't list (the wildcard "*" and unlisted tools — automate, MCP, custom). Whole-tool denies for keys the map DOES list are regenerated from the map each turn, so the ruleset stays stable instead of accumulating; non-agent sessions still replace wholesale (pre-existing behavior). Like upstream, forwarding is deny-only: a wildcard caller's allow-exceptions (e.g. a read-only "*": deny agent that also allows read) are not preserved, so its subagent loses those tools too — erring toward deny. The caller is resolved as ctx.extra.callerAgent ?? ctx.agent: on a normal LLM dispatch ctx.agent is the caller, but on a subtask command SessionPrompt.handleSubtask runs the agent tool as the child, so it threads the real caller through ctx.extra (PawWork sessions don't store their agent). edit covers edit/write/apply_patch (all ask under the single "edit" key). Resume (subagent_session_id) is covered too: it skips sessions.create, so the inherited permission is recomputed from the current caller and re-applied onto the existing child before it runs. 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.
1 parent 687865b commit 42e6774

4 files changed

Lines changed: 498 additions & 51 deletions

File tree

packages/opencode/src/session/prompt.ts

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,7 +1126,9 @@ NOTE: At any point in time through this workflow you should feel free to ask the
11261126
sessionID,
11271127
abort: taskAbort.signal,
11281128
callID: part.callID,
1129-
extra: { bypassAgentCheck: true, promptOps },
1129+
// #26597: ctx.agent here is the subtask's (child) agent, not the dispatcher.
1130+
// Pass the real caller so the agent tool can honor its edit restriction.
1131+
extra: { bypassAgentCheck: true, promptOps, callerAgent: lastUser.agent },
11301132
messages: msgs,
11311133
metadata: (val: { title?: string; metadata?: Record<string, any> }) =>
11321134
Effect.gen(function* () {
@@ -1877,8 +1879,30 @@ NOTE: At any point in time through this workflow you should feel free to ask the
18771879
permissions.push({ permission: t, action: enabled ? "allow" : "deny", pattern: "*" })
18781880
}
18791881
if (permissions.length > 0) {
1880-
session.permission = permissions
1881-
yield* sessions.setPermission({ sessionID: session.id, permission: permissions })
1882+
// #26597: the boolean tools map is availability-only — it lists the subagent's structural
1883+
// denies (agent, worktree, todowrite, primary_tools), not what it inherited from its
1884+
// caller. The caller's deny rules are the single source of truth for inheritance and live
1885+
// on session.permission, forwarded at dispatch (tool/agent.ts). Rebuilding from the map
1886+
// alone would drop them, letting a caller regain access through the child. For agent-tool
1887+
// children, carry forward external_directory rules plus every caller deny the map does NOT
1888+
// regenerate: scoped (non-"*") denies (e.g. edit on one path) and whole-tool denies for
1889+
// keys absent from the map — the wildcard "*" and any tool not listed (automate, MCP,
1890+
// custom). Per-tool "*" denies for keys the map lists are regenerated each turn, so
1891+
// dropping them keeps this stable instead of accumulating.
1892+
// NOTE: like upstream #26597 this is forward-deny only — a caller's allow exception (e.g.
1893+
// a read-only "*": deny agent that also allows read) is not preserved, so its subagent
1894+
// loses those tools too. Matching upstream's deriveSubagentSessionPermission; toward deny.
1895+
const toolKeys = new Set(Object.keys(input.tools ?? {}))
1896+
const preserved = session.createdByAgentTool
1897+
? (session.permission ?? []).filter(
1898+
(rule) =>
1899+
rule.permission === "external_directory" ||
1900+
(rule.action === "deny" && (rule.pattern !== "*" || !toolKeys.has(rule.permission))),
1901+
)
1902+
: []
1903+
const next = [...preserved, ...permissions]
1904+
session.permission = next
1905+
yield* sessions.setPermission({ sessionID: session.id, permission: next })
18821906
}
18831907

18841908
yield* throwIfAborted(options)

packages/opencode/src/tool/agent.ts

Lines changed: 62 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Session } from "../session"
44
import { SessionID, MessageID } from "../session/schema"
55
import { MessageV2 } from "../session/message-v2"
66
import { Agent } from "../agent/agent"
7+
import type { Permission } from "../permission"
78
import type { SessionPrompt } from "../session/prompt"
89
import { Config } from "../config"
910
import { SubagentRun } from "../session/subagent-run"
@@ -299,41 +300,72 @@ export const AgentTool = Tool.define(
299300
const parent = yield* sessions.get(ctx.sessionID)
300301
const parentExec = parent.executionContext
301302

303+
// #26597: a subagent must not use a tool its caller is denied, otherwise a restricted
304+
// agent (Plan Mode's edit-deny, or a read-only "*": deny agent) could escalate by
305+
// spawning a more-capable subagent. Resolve the caller's agent so its deny rules can be
306+
// forwarded onto the child session below. The caller is ctx.agent on a normal LLM
307+
// dispatch; for a subtask command SessionPrompt.handleSubtask runs the agent tool as the
308+
// child and passes the real caller via ctx.extra.callerAgent (PawWork sessions don't
309+
// store their agent).
310+
const callerAgentName = (ctx.extra?.callerAgent as string | undefined) ?? ctx.agent
311+
const callerAgent = yield* agent.get(callerAgentName).pipe(Effect.catchCause(() => Effect.succeed(undefined)))
312+
313+
// #26597: the subagent's inherited permission — the single source of truth for what it
314+
// may do. Forward the caller's deny rules with patterns intact so they bind the child
315+
// the same way they bind the caller: the caller agent's restrictions (scoped denies
316+
// like edit on one path, or a wildcard "*" deny) live on its agent ruleset, not the
317+
// session, so they're forwarded explicitly alongside the caller session's denies +
318+
// external_directory. The rebuild in SessionPrompt.prompt carries these forward
319+
// verbatim — it only regenerates the per-tool "*" denies the tools map below lists. The
320+
// trailing rules are the subagent's own structural shape (no nested dispatch, no todos
321+
// unless its agent allows them, primary-tool allows).
322+
const inheritedPermission: Permission.Ruleset = [
323+
...(parent.permission ?? []).filter(
324+
(rule) => rule.permission === "external_directory" || rule.action === "deny",
325+
),
326+
...(callerAgent?.permission ?? []).filter((rule) => rule.action === "deny"),
327+
// v1 nested-deny: agent is denied unconditionally so a subagent cannot recursively
328+
// dispatch its own subagents (#283 non-goal: nested subagents).
329+
{
330+
permission: id,
331+
pattern: "*" as const,
332+
action: "deny" as const,
333+
},
334+
...(canTodo
335+
? []
336+
: [
337+
{
338+
permission: "todowrite" as const,
339+
pattern: "*" as const,
340+
action: "deny" as const,
341+
},
342+
]),
343+
...(cfg.experimental?.primary_tools?.map((item) => ({
344+
pattern: "*",
345+
action: "allow" as const,
346+
permission: item,
347+
})) ?? []),
348+
]
349+
302350
const nextSession =
303351
session ??
304352
(yield* sessions.create({
305353
parentID: ctx.sessionID,
306354
title: params.description + ` (@${next.name} subagent)`,
307355
createdByAgentTool: true,
308356
subagentType: params.subagent_type,
309-
permission: [
310-
...(parent.permission ?? []).filter(
311-
(rule) => rule.permission === "external_directory" || rule.action === "deny",
312-
),
313-
// v1 nested-deny: agent is denied unconditionally so a subagent cannot
314-
// recursively dispatch its own subagents (#283 non-goal: nested subagents).
315-
{
316-
permission: id,
317-
pattern: "*" as const,
318-
action: "deny" as const,
319-
},
320-
...(canTodo
321-
? []
322-
: [
323-
{
324-
permission: "todowrite" as const,
325-
pattern: "*" as const,
326-
action: "deny" as const,
327-
},
328-
]),
329-
...(cfg.experimental?.primary_tools?.map((item) => ({
330-
pattern: "*",
331-
action: "allow" as const,
332-
permission: item,
333-
})) ?? []),
334-
],
357+
permission: inheritedPermission,
335358
}))
336359

360+
// #26597: resume (subagent_session_id) skips sessions.create, so re-forward the CURRENT
361+
// caller's inherited permission onto the existing child. Otherwise a caller that became
362+
// more restrictive after the child was created — e.g. switched to Plan Mode — could
363+
// resume it and regain the denied tools, since the child still carried its original
364+
// creator's permission. The rebuild then carries this forward as on a fresh dispatch.
365+
if (session) {
366+
yield* sessions.setPermission({ sessionID: nextSession.id, permission: inheritedPermission })
367+
}
368+
337369
const childExec = nextSession.executionContext
338370
const sameWorktree =
339371
parentExec.activeWorktree?.directory === childExec.activeWorktree?.directory &&
@@ -441,6 +473,10 @@ export const AgentTool = Tool.define(
441473
sessionID: nextSession.id,
442474
model: { modelID: model.modelID, providerID: model.providerID },
443475
agent: next.name,
476+
// Availability-only: structural constraints on the subagent (no nested
477+
// dispatch, no worktree switching, no todos unless its agent allows them, no
478+
// primary-only tools). Caller-inherited denies ride on session.permission
479+
// (forwarded above), not this map. See #26597.
444480
tools: {
445481
agent: false,
446482
"enter-worktree": false,

packages/opencode/test/session/prompt.test.ts

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { NamedError } from "@opencode-ai/util/error"
44
import { fileURLToPath, pathToFileURL } from "url"
55
import { Effect, Layer } from "effect"
66
import { Instance } from "../../src/project/instance"
7+
import { Permission } from "../../src/permission"
78
import { ModelID, ProviderID } from "../../src/provider/schema"
89
import { Session } from "../../src/session"
910
import { MessageV2 } from "../../src/session/message-v2"
@@ -1002,3 +1003,170 @@ describe("session.agent-resolution", () => {
10021003
}
10031004
}, 30000)
10041005
})
1006+
1007+
// #26597: the prompt rebuilds session.permission from the boolean tools map, which can only
1008+
// regenerate whole-tool ("*") rules for the keys it lists. For an agent-tool subagent it must
1009+
// carry forward the caller's inherited rules the map can't regenerate — scoped denies,
1010+
// external_directory rules, and whole-tool denies for keys the map doesn't list (the wildcard
1011+
// "*", MCP/custom tools) — otherwise a caller denied e.g. edit on one path, an external dir, or a
1012+
// whole tool regains it through the child. Whole-tool denies for keys the map DOES list are
1013+
// regenerated from the map instead, so the ruleset doesn't accumulate across turns.
1014+
describe("session.prompt subagent permission rebuild (#26597)", () => {
1015+
test("carries scoped denies and external_directory forward for an agent-tool subagent", async () => {
1016+
await using tmp = await tmpdir({
1017+
config: { agent: { build: { model: "openai/gpt-5.2" } } },
1018+
})
1019+
await Instance.provide({
1020+
directory: tmp.path,
1021+
fn: () =>
1022+
run(
1023+
Effect.gen(function* () {
1024+
const prompt = yield* SessionPrompt.Service
1025+
const sessions = yield* Session.Service
1026+
const parent = yield* sessions.create({})
1027+
const child = yield* sessions.create({
1028+
parentID: parent.id,
1029+
createdByAgentTool: true,
1030+
subagentType: "general",
1031+
permission: [
1032+
{ permission: "external_directory", pattern: "/tmp/project/*", action: "allow" },
1033+
{ permission: "edit", pattern: "/secret/**", action: "deny" },
1034+
{ permission: "edit", pattern: "*", action: "deny" },
1035+
],
1036+
})
1037+
1038+
yield* prompt.prompt({
1039+
sessionID: child.id,
1040+
agent: "build",
1041+
noReply: true,
1042+
tools: { agent: false, "enter-worktree": false },
1043+
parts: [{ type: "text", text: "x" }],
1044+
})
1045+
1046+
const after = yield* sessions.get(child.id)
1047+
// Scoped deny + external_directory survive (the boolean tools map can't express them).
1048+
expect(after.permission).toContainEqual({
1049+
permission: "external_directory",
1050+
pattern: "/tmp/project/*",
1051+
action: "allow",
1052+
})
1053+
expect(after.permission).toContainEqual({ permission: "edit", pattern: "/secret/**", action: "deny" })
1054+
// The structural denies the boolean tools map lists are regenerated from it.
1055+
expect(after.permission).toContainEqual({ permission: "agent", pattern: "*", action: "deny" })
1056+
// The whole-tool ("*") edit deny is ALSO carried forward: "edit" is absent from the
1057+
// tools map (which lists only agent/enter-worktree here), so the map can't regenerate
1058+
// it — dropping it would let the caller's edit deny vanish through the child. A
1059+
// whole-tool deny for a key the map DOES list is regenerated instead (next test).
1060+
expect(after.permission).toContainEqual({ permission: "edit", pattern: "*", action: "deny" })
1061+
}),
1062+
),
1063+
})
1064+
}, 30000)
1065+
1066+
test("regenerates a whole-tool deny the map lists instead of double-carrying it", async () => {
1067+
await using tmp = await tmpdir({
1068+
config: { agent: { build: { model: "openai/gpt-5.2" } } },
1069+
})
1070+
await Instance.provide({
1071+
directory: tmp.path,
1072+
fn: () =>
1073+
run(
1074+
Effect.gen(function* () {
1075+
const prompt = yield* SessionPrompt.Service
1076+
const sessions = yield* Session.Service
1077+
const parent = yield* sessions.create({})
1078+
const child = yield* sessions.create({
1079+
parentID: parent.id,
1080+
createdByAgentTool: true,
1081+
subagentType: "general",
1082+
permission: [{ permission: "edit", pattern: "*", action: "deny" }],
1083+
})
1084+
1085+
yield* prompt.prompt({
1086+
sessionID: child.id,
1087+
agent: "build",
1088+
noReply: true,
1089+
// "edit" is in the map, so its "*" deny is regenerated from the map — the forwarded
1090+
// copy is dropped from the carry-forward so the ruleset doesn't accumulate.
1091+
tools: { agent: false, edit: false },
1092+
parts: [{ type: "text", text: "x" }],
1093+
})
1094+
1095+
const after = yield* sessions.get(child.id)
1096+
expect((after.permission ?? []).filter((r) => r.permission === "edit" && r.pattern === "*")).toHaveLength(1)
1097+
expect(Permission.evaluate("edit", "*", after.permission ?? []).action).toBe("deny")
1098+
}),
1099+
),
1100+
})
1101+
}, 30000)
1102+
1103+
test("carries the caller's wildcard deny forward so tools absent from the map stay denied", async () => {
1104+
await using tmp = await tmpdir({
1105+
config: { agent: { build: { model: "openai/gpt-5.2" } } },
1106+
})
1107+
await Instance.provide({
1108+
directory: tmp.path,
1109+
fn: () =>
1110+
run(
1111+
Effect.gen(function* () {
1112+
const prompt = yield* SessionPrompt.Service
1113+
const sessions = yield* Session.Service
1114+
const parent = yield* sessions.create({})
1115+
// A read-only-style caller forwards a wildcard ("*") deny onto the child.
1116+
const child = yield* sessions.create({
1117+
parentID: parent.id,
1118+
createdByAgentTool: true,
1119+
subagentType: "general",
1120+
permission: [{ permission: "*", pattern: "*", action: "deny" }],
1121+
})
1122+
1123+
yield* prompt.prompt({
1124+
sessionID: child.id,
1125+
agent: "build",
1126+
noReply: true,
1127+
tools: { agent: false, edit: false },
1128+
parts: [{ type: "text", text: "x" }],
1129+
})
1130+
1131+
const after = yield* sessions.get(child.id)
1132+
// The wildcard deny is preserved, so a tool absent from the boolean tools map
1133+
// (automate, MCP, custom) still evaluates to deny for the subagent.
1134+
expect(after.permission).toContainEqual({ permission: "*", pattern: "*", action: "deny" })
1135+
expect(Permission.evaluate("automate", "*", after.permission ?? []).action).toBe("deny")
1136+
}),
1137+
),
1138+
})
1139+
}, 30000)
1140+
1141+
test("replaces permission wholesale for a non-agent-tool session", async () => {
1142+
await using tmp = await tmpdir({
1143+
config: { agent: { build: { model: "openai/gpt-5.2" } } },
1144+
})
1145+
await Instance.provide({
1146+
directory: tmp.path,
1147+
fn: () =>
1148+
run(
1149+
Effect.gen(function* () {
1150+
const prompt = yield* SessionPrompt.Service
1151+
const sessions = yield* Session.Service
1152+
const session = yield* sessions.create({
1153+
permission: [{ permission: "edit", pattern: "/secret/**", action: "deny" }],
1154+
})
1155+
1156+
yield* prompt.prompt({
1157+
sessionID: session.id,
1158+
agent: "build",
1159+
noReply: true,
1160+
tools: { agent: false },
1161+
parts: [{ type: "text", text: "x" }],
1162+
})
1163+
1164+
const after = yield* sessions.get(session.id)
1165+
// Not an agent-tool child → the rebuild replaces wholesale (pre-existing behavior).
1166+
expect(after.permission).not.toContainEqual({ permission: "edit", pattern: "/secret/**", action: "deny" })
1167+
expect(after.permission).toContainEqual({ permission: "agent", pattern: "*", action: "deny" })
1168+
}),
1169+
),
1170+
})
1171+
}, 30000)
1172+
})

0 commit comments

Comments
 (0)