Skip to content

Commit 4814ab3

Browse files
authored
fix(core): enforce V2 tool permissions (#31061)
1 parent 747b8da commit 4814ab3

10 files changed

Lines changed: 180 additions & 10 deletions

File tree

packages/core/src/session/runner/llm.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ export const layer = Layer.effect(
219219
.filter((part): part is string => part !== undefined && part.length > 0)
220220
.map(SystemPart.make),
221221
messages: toLLMMessages(context, model),
222-
tools: yield* tools.definitions(),
222+
tools: yield* tools.definitions(agent.info?.permissions),
223223
})
224224
if (yield* compaction.compactIfNeeded({ sessionID: session.id, entries, model, request }))
225225
return yield* Effect.die(rebuildPreparedTurn())

packages/core/src/tool/question.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
export * as QuestionTool from "./question"
22

3-
import { Tool, toolText } from "@opencode-ai/llm"
3+
import { Tool, ToolFailure, toolText } from "@opencode-ai/llm"
44
import { Effect, Layer, Schema } from "effect"
55
import { QuestionV2 } from "../question"
66
import { ToolRegistry } from "./registry"
@@ -57,6 +57,11 @@ export const layer = Layer.effectDiscard(
5757
yield* registry.contribute((editor) =>
5858
editor.set(name, {
5959
tool: definition,
60+
permission: { action: "question", resource: "*" },
61+
authorize: ({ assertPermission }) =>
62+
assertPermission({ action: "question", resources: ["*"] }).pipe(
63+
Effect.mapError(() => new ToolFailure({ message: "Permission denied: question" })),
64+
),
6065
execute: ({ parameters, sessionID, source }) =>
6166
question
6267
.ask({

packages/core/src/tool/registry.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type { SessionV2 } from "../session"
2020
import { ApplicationTools } from "./application-tools"
2121
import { ToolOutputStore } from "../tool-output-store"
2222
import { AgentV2 } from "../agent"
23+
import { Wildcard } from "../util/wildcard"
2324

2425
export type ExecuteInput = {
2526
readonly sessionID: SessionSchema.ID
@@ -54,6 +55,8 @@ export type Entry<
5455
Success extends ToolSchema<any> = ToolSchema<any>,
5556
> = {
5657
readonly tool: TypedTool<Parameters, Success>
58+
/** Catalog visibility only. Execution authorization remains leaf-owned. */
59+
readonly permission?: { readonly action: string; readonly resource: "*" }
5760
readonly authorize?: (input: AuthorizeInput<Schema.Schema.Type<Parameters>>) => Effect.Effect<void, ToolFailure>
5861
readonly execute?: (
5962
input: AuthorizeInput<Schema.Schema.Type<Parameters>>,
@@ -78,7 +81,9 @@ export type Editor = {
7881
export interface Interface {
7982
readonly transform: State.Interface<Data, Editor>["transform"]
8083
readonly contribute: (update: State.Transform<Editor>) => Effect.Effect<void, never, Scope.Scope>
81-
readonly definitions: () => Effect.Effect<ReadonlyArray<ReturnType<typeof Tool.toDefinitions>[number]>>
84+
readonly definitions: (
85+
permissions?: PermissionV2.Ruleset,
86+
) => Effect.Effect<ReadonlyArray<ReturnType<typeof Tool.toDefinitions>[number]>>
8287
readonly execute: (input: ExecuteInput) => Effect.Effect<ToolResultValue>
8388
readonly settle: (input: ExecuteInput) => Effect.Effect<Settlement>
8489
}
@@ -114,13 +119,19 @@ export const layer = Layer.effect(
114119
}),
115120
})
116121

117-
const definitions = Effect.fn("ToolRegistry.definitions")(function* () {
118-
const tools = new Map(Array.from(state.get().entries, ([name, entry]) => [name, entry.tool] as const))
122+
const definitions = Effect.fn("ToolRegistry.definitions")(function* (permissions: PermissionV2.Ruleset = []) {
123+
const tools = new Map(state.get().entries)
119124
// Location tools own their names. Application tools fill otherwise-unclaimed names.
120125
for (const [name, tool] of applications.entries()) {
121-
if (!tools.has(name)) tools.set(name, tool.definition)
126+
if (!tools.has(name)) tools.set(name, { tool: tool.definition })
122127
}
123-
return Tool.toDefinitions(Object.fromEntries(tools))
128+
return Tool.toDefinitions(
129+
Object.fromEntries(
130+
Array.from(tools)
131+
.filter(([name, entry]) => !whollyDisabled(entry.permission ?? defaultPermission(name), permissions))
132+
.map(([name, entry]) => [name, entry.tool]),
133+
),
134+
)
124135
})
125136

126137
const entry = (name: string): Entry | undefined => {
@@ -224,6 +235,15 @@ export const layer = Layer.effect(
224235
}),
225236
)
226237

238+
function defaultPermission(name: string) {
239+
return { action: ["edit", "write", "apply_patch"].includes(name) ? "edit" : name, resource: "*" as const }
240+
}
241+
242+
function whollyDisabled(permission: { readonly action: string; readonly resource: "*" }, rules: PermissionV2.Ruleset) {
243+
const rule = rules.findLast((rule) => Wildcard.match(permission.action, rule.action))
244+
return rule?.resource === "*" && rule.effect === "deny"
245+
}
246+
227247
export const defaultLayer = layer.pipe(
228248
Layer.provide(ApplicationTools.layer),
229249
Layer.provide(ToolOutputStore.defaultLayer),

packages/core/test/application-tools.test.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,26 @@ const contextual = (contexts: Tool.Context[]) =>
3737
})
3838

3939
describe("ApplicationTools", () => {
40+
it.effect("filters an application tool by its name without adding execution authorization", () =>
41+
Effect.gen(function* () {
42+
const applications = yield* ApplicationTools.Service
43+
const registry = yield* ToolRegistry.Service
44+
const contexts: Tool.Context[] = []
45+
yield* applications.attach({ application_context: contextual(contexts) })
46+
47+
expect(yield* registry.definitions([{ action: "application_context", resource: "*", effect: "deny" }])).toEqual(
48+
[],
49+
)
50+
expect(
51+
yield* registry.settle({
52+
sessionID,
53+
call: { type: "tool-call", id: "call-denied", name: "application_context", input: { query: "hello" } },
54+
}),
55+
).toMatchObject({ result: { type: "content" } })
56+
expect(contexts).toEqual([{ sessionID, id: "call-denied", name: "application_context" }])
57+
}),
58+
)
59+
4060
it.effect("advertises and executes a scoped application tool with Session context", () =>
4161
Effect.gen(function* () {
4262
const applications = yield* ApplicationTools.Service
@@ -169,13 +189,18 @@ describe("ApplicationTools", () => {
169189
yield* transform((editor) =>
170190
editor.set("shared", {
171191
tool: location.definition,
192+
permission: { action: "question", resource: "*" },
172193
execute: ({ parameters, sessionID, call }) =>
173194
location.execute(parameters, { sessionID, id: call.id, name: call.name }),
174195
}),
175196
)
176197
yield* applications.attach({ shared: contextual(applicationContexts) })
177198

178-
expect((yield* registry.definitions()).map((definition) => definition.name)).toEqual(["shared"])
199+
expect(
200+
(yield* registry.definitions([{ action: "question", resource: "*", effect: "deny" }])).map(
201+
(definition) => definition.name,
202+
),
203+
).toEqual([])
179204
expect(
180205
yield* registry.settle({
181206
sessionID,

packages/core/test/config/config.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,7 @@ describe("Config", () => {
449449
permission: {
450450
bash: "ask",
451451
edit: { "*.md": "allow", "*": "deny" },
452+
question: "deny",
452453
},
453454
agent: {
454455
reviewer: {
@@ -526,6 +527,7 @@ describe("Config", () => {
526527
{ action: "bash", resource: "*", effect: "ask" },
527528
{ action: "edit", resource: "*.md", effect: "allow" },
528529
{ action: "edit", resource: "*", effect: "deny" },
530+
{ action: "question", resource: "*", effect: "deny" },
529531
])
530532
expect(documents[0]?.info.agents?.reviewer).toMatchObject({
531533
system: "Review changes.",

packages/core/test/session-runner-tool-registry.test.ts

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,96 @@ const echo = Tool.make({
4545
})
4646

4747
describe("ToolRegistry", () => {
48+
it.effect("matches V1 whole-tool filtering, edit aliases, and ordered wildcard precedence", () =>
49+
Effect.gen(function* () {
50+
const registry = yield* ToolRegistry.Service
51+
const transform = yield* registry.transform()
52+
const sessionID = SessionV2.ID.make("ses_registry_filter")
53+
yield* transform((editor) => {
54+
editor.set("question", { tool: echo })
55+
editor.set("bash", { tool: echo })
56+
editor.set("edit", { tool: echo })
57+
editor.set("write", { tool: echo })
58+
editor.set("apply_patch", { tool: echo })
59+
})
60+
61+
const names = (rules: PermissionV2.Ruleset) =>
62+
registry.definitions(rules).pipe(Effect.map((definitions) => definitions.map((tool) => tool.name)))
63+
64+
expect(yield* names([{ action: "question", resource: "*", effect: "deny" }])).toEqual([
65+
"bash",
66+
"edit",
67+
"write",
68+
"apply_patch",
69+
])
70+
71+
expect(
72+
yield* names([
73+
{ action: "*", resource: "*", effect: "deny" },
74+
{ action: "question", resource: "private", effect: "allow" },
75+
]),
76+
).toEqual(["question"])
77+
78+
expect(
79+
yield* names([
80+
{ action: "question", resource: "private", effect: "allow" },
81+
{ action: "*", resource: "*", effect: "deny" },
82+
]),
83+
).toEqual([])
84+
85+
expect(yield* names([{ action: "question", resource: "*", effect: "ask" }])).toContain("question")
86+
expect(yield* names([{ action: "edit", resource: "*", effect: "deny" }])).toEqual(["question", "bash"])
87+
expect(
88+
yield* names([
89+
{ action: "edit", resource: "*", effect: "deny" },
90+
{ action: "edit", resource: "*.md", effect: "ask" },
91+
]),
92+
).toEqual(["question", "bash", "edit", "write", "apply_patch"])
93+
expect(
94+
yield* names([
95+
{ action: "edit", resource: "*.md", effect: "allow" },
96+
{ action: "edit", resource: "*", effect: "deny" },
97+
]),
98+
).toEqual(["question", "bash"])
99+
}),
100+
)
101+
102+
it.effect("settles only through concrete leaf authorization, not catalog visibility", () =>
103+
Effect.gen(function* () {
104+
const registry = yield* ToolRegistry.Service
105+
const transform = yield* registry.transform()
106+
const sessionID = SessionV2.ID.make("ses_registry_stale")
107+
let executed = false
108+
yield* transform((editor) =>
109+
editor.set("question", {
110+
tool: echo,
111+
permission: { action: "question", resource: "*" },
112+
authorize: ({ assertPermission }) =>
113+
assertPermission({ action: "question", resources: ["actual"] }).pipe(
114+
Effect.mapError(() => new ToolFailure({ message: "Denied" })),
115+
),
116+
execute: () =>
117+
Effect.sync(() => {
118+
executed = true
119+
return { text: "unexpected" }
120+
}),
121+
}),
122+
)
123+
124+
expect(
125+
(yield* registry.definitions([{ action: "question", resource: "*", effect: "deny" }])).map((tool) => tool.name),
126+
).toEqual([])
127+
expect(
128+
yield* registry.settle({
129+
sessionID,
130+
call: { type: "tool-call", id: "call-stale", name: "question", input: { text: "hello" } },
131+
}),
132+
).toMatchObject({ result: { type: "json", value: { text: "unexpected" } } })
133+
expect(assertions.at(-1)).toMatchObject({ action: "question", resources: ["actual"] })
134+
expect(executed).toBe(true)
135+
}),
136+
)
137+
48138
it.effect("rebuilds advertised definitions when a scoped transform closes", () =>
49139
Effect.gen(function* () {
50140
const registry = yield* ToolRegistry.Service

packages/core/test/tool-bash.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ describe("BashTool", () => {
149149
const definitions = yield* registry.definitions()
150150
expect(definitions.map((tool) => tool.name)).toEqual(["bash"])
151151
expect(definitions[0]?.inputSchema).not.toHaveProperty("properties.background")
152+
expect(yield* registry.definitions([{ action: "bash", resource: "*", effect: "deny" }])).toEqual([])
152153
expect(yield* registry.settle(call({ command: "pwd", description: "Print working directory" }))).toEqual({
153154
result: { type: "text", value: "hello\n\n\nCommand exited with code 0." },
154155
output: {

packages/core/test/tool-edit.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ describe("EditTool", () => {
110110
withTool(tmp.path, (registry) =>
111111
Effect.gen(function* () {
112112
expect((yield* registry.definitions()).map((tool) => tool.name)).toEqual(["edit"])
113+
expect(yield* registry.definitions([{ action: "edit", resource: "*", effect: "deny" }])).toEqual([])
113114
const settled = yield* registry.settle(
114115
call({ path: "hello.txt", oldString: "before", newString: "after" }),
115116
)

packages/core/test/tool-question.test.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,15 @@ const sessionID = SessionV2.ID.make("ses_question_tool_test")
1111
const assertions: PermissionV2.AssertInput[] = []
1212
let captured: QuestionV2.AskInput | undefined
1313
let reject = false
14+
let deny = false
1415
const capturedInput = () => captured
1516
const permission = Layer.succeed(
1617
PermissionV2.Service,
1718
PermissionV2.Service.of({
18-
assert: (input) => Effect.sync(() => assertions.push(input)),
19+
assert: (input) =>
20+
Effect.sync(() => assertions.push(input)).pipe(
21+
Effect.andThen(deny ? Effect.fail(new PermissionV2.DeniedError({ rules: [] })) : Effect.void),
22+
),
1923
ask: () => Effect.die("unused"),
2024
reply: () => Effect.die("unused"),
2125
get: () => Effect.die("unused"),
@@ -40,11 +44,30 @@ const tool = QuestionTool.layer.pipe(Layer.provide(registry), Layer.provide(ques
4044
const it = testEffect(Layer.mergeAll(permission, registry, question, tool))
4145

4246
describe("QuestionTool", () => {
47+
it.effect("omits a denied built-in question and terminally settles a stale call", () =>
48+
Effect.gen(function* () {
49+
captured = undefined
50+
deny = true
51+
const registry = yield* ToolRegistry.Service
52+
53+
expect(yield* registry.definitions([{ action: "question", resource: "*", effect: "deny" }])).toEqual([])
54+
expect(
55+
yield* registry.settle({
56+
sessionID,
57+
call: { type: "tool-call", id: "call-question-denied", name: "question", input: { questions: [] } },
58+
}),
59+
).toEqual({ result: { type: "error", value: "Permission denied: question" } })
60+
expect(capturedInput()).toBeUndefined()
61+
deny = false
62+
}),
63+
)
64+
4365
it.effect("registers question and projects user answers without a permission assertion", () =>
4466
Effect.gen(function* () {
4567
assertions.length = 0
4668
captured = undefined
4769
reject = false
70+
deny = false
4871
const registry = yield* ToolRegistry.Service
4972
const questions = [
5073
{
@@ -81,7 +104,7 @@ describe("QuestionTool", () => {
81104
],
82105
},
83106
})
84-
expect(assertions).toEqual([])
107+
expect(assertions).toEqual([{ sessionID, action: "question", resources: ["*"] }])
85108
expect(capturedInput()).toEqual({ sessionID, questions, tool: undefined })
86109
}),
87110
)
@@ -90,6 +113,7 @@ describe("QuestionTool", () => {
90113
Effect.gen(function* () {
91114
captured = undefined
92115
reject = false
116+
deny = false
93117
const registryService = yield* ToolRegistry.Service
94118

95119
yield* registryService.execute({
@@ -104,6 +128,7 @@ describe("QuestionTool", () => {
104128
Effect.gen(function* () {
105129
captured = undefined
106130
reject = true
131+
deny = false
107132
const registryService = yield* ToolRegistry.Service
108133
const fiber = yield* registryService
109134
.execute({

packages/core/test/tool-read.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ describe("ReadTool", () => {
101101
const registry = yield* ToolRegistry.Service
102102

103103
expect(yield* registry.definitions()).toMatchObject([{ name: "read" }])
104+
expect(yield* registry.definitions([{ action: "read", resource: "*", effect: "deny" }])).toEqual([])
104105
expect(
105106
yield* registry.execute({
106107
sessionID,

0 commit comments

Comments
 (0)