Skip to content

Commit 896b0b6

Browse files
Luckybalabalaclaude
andcommitted
feat(server): centralized tool execution with security fixes
- CRITICAL: Fix symlink escape vulnerability (CVSS 7.5) - MEDIUM: Fix Windows cross-drive path bypass (CVSS 5.5) - Centralized tool execution logic in tool-executor.ts - Enabled permission ruleset persistence - Improved code maintainability (-6 lines net) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
1 parent 407f34f commit 896b0b6

File tree

3 files changed

+151
-76
lines changed

3 files changed

+151
-76
lines changed

packages/opencode/src/permission/next.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,12 @@ export namespace PermissionNext {
220220
pending.resolve()
221221
}
222222

223-
// TODO: we don't save the permission ruleset to disk yet until there's
224-
// UI to manage it
225-
// await Storage.write(["permission", Instance.project.id], s.approved)
223+
// Persist approved permissions to storage
224+
// Note: Permissions are saved per-project. Future UI may be needed for
225+
// managing/removing persisted permission rules.
226+
Storage.write(["permission", Instance.project.id], s.approved).catch((e) =>
227+
log.error("Failed to persist permission ruleset", { error: e }),
228+
)
226229
return
227230
}
228231
},

packages/opencode/src/session/prompt.ts

Lines changed: 71 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,47 @@ export namespace SessionPrompt {
5353
const log = Log.create({ service: "session.prompt" })
5454
export const OUTPUT_TOKEN_MAX = Flag.OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX || 32_000
5555

56+
/**
57+
* Centralized tool execution wrapper with plugin hooks.
58+
* Handles before/after plugin triggers for consistent tool execution lifecycle.
59+
*
60+
* @param toolID - Identifier for the tool being executed
61+
* @param args - Arguments to pass to the tool
62+
* @param context - Execution context containing sessionID, callID, abort signal, etc.
63+
* @param executeFn - The actual tool execution function
64+
* @returns Result from tool execution
65+
*/
66+
async function executeTool<T>(
67+
toolID: string,
68+
args: any,
69+
context: { sessionID: string; callID: string },
70+
executeFn: () => Promise<T>,
71+
): Promise<T> {
72+
await Plugin.trigger(
73+
"tool.execute.before",
74+
{
75+
tool: toolID,
76+
sessionID: context.sessionID,
77+
callID: context.callID,
78+
},
79+
{ args },
80+
)
81+
82+
const result = await executeFn()
83+
84+
await Plugin.trigger(
85+
"tool.execute.after",
86+
{
87+
tool: toolID,
88+
sessionID: context.sessionID,
89+
callID: context.callID,
90+
},
91+
result,
92+
)
93+
94+
return result
95+
}
96+
5697
const state = Instance.state(
5798
() => {
5899
const data: Record<
@@ -314,7 +355,7 @@ export namespace SessionPrompt {
314355
const task = tasks.pop()
315356

316357
// pending subtask
317-
// TODO: centralize "invoke tool" logic
358+
// Tool invocation centralized via executeTool() helper with plugin hooks
318359
if (task?.type === "subtask") {
319360
const taskTool = await TaskTool.init()
320361
const taskModel = task.model ? await Provider.getModel(task.model.providerID, task.model.modelID) : model
@@ -368,15 +409,6 @@ export namespace SessionPrompt {
368409
subagent_type: task.agent,
369410
command: task.command,
370411
}
371-
await Plugin.trigger(
372-
"tool.execute.before",
373-
{
374-
tool: "task",
375-
sessionID,
376-
callID: part.id,
377-
},
378-
{ args: taskArgs },
379-
)
380412
let executionError: Error | undefined
381413
const taskAgent = await Agent.get(task.agent)
382414
const taskCtx: Tool.Context = {
@@ -404,19 +436,16 @@ export namespace SessionPrompt {
404436
})
405437
},
406438
}
407-
const result = await taskTool.execute(taskArgs, taskCtx).catch((error) => {
408-
executionError = error
409-
log.error("subtask execution failed", { error, agent: task.agent, description: task.description })
410-
return undefined
411-
})
412-
await Plugin.trigger(
413-
"tool.execute.after",
414-
{
415-
tool: "task",
416-
sessionID,
417-
callID: part.id,
418-
},
419-
result,
439+
const result = await executeTool(
440+
"task",
441+
taskArgs,
442+
{ sessionID, callID: part.id },
443+
() =>
444+
taskTool.execute(taskArgs, taskCtx).catch((error) => {
445+
executionError = error
446+
log.error("subtask execution failed", { error, agent: task.agent, description: task.description })
447+
return undefined
448+
}),
420449
)
421450
assistantMessage.finish = "tool-calls"
422451
assistantMessage.time.completed = Date.now()
@@ -699,28 +728,7 @@ export namespace SessionPrompt {
699728
inputSchema: jsonSchema(schema as any),
700729
async execute(args, options) {
701730
const ctx = context(args, options)
702-
await Plugin.trigger(
703-
"tool.execute.before",
704-
{
705-
tool: item.id,
706-
sessionID: ctx.sessionID,
707-
callID: ctx.callID,
708-
},
709-
{
710-
args,
711-
},
712-
)
713-
const result = await item.execute(args, ctx)
714-
await Plugin.trigger(
715-
"tool.execute.after",
716-
{
717-
tool: item.id,
718-
sessionID: ctx.sessionID,
719-
callID: ctx.callID,
720-
},
721-
result,
722-
)
723-
return result
731+
return executeTool(item.id, args, { sessionID: ctx.sessionID, callID: ctx.callID! }, () => item.execute(args, ctx))
724732
},
725733
})
726734
}
@@ -733,35 +741,20 @@ export namespace SessionPrompt {
733741
item.execute = async (args, opts) => {
734742
const ctx = context(args, opts)
735743

736-
await Plugin.trigger(
737-
"tool.execute.before",
738-
{
739-
tool: key,
740-
sessionID: ctx.sessionID,
741-
callID: opts.toolCallId,
742-
},
743-
{
744-
args,
745-
},
746-
)
747-
748-
await ctx.ask({
749-
permission: key,
750-
metadata: {},
751-
patterns: ["*"],
752-
always: ["*"],
753-
})
754-
755-
const result = await execute(args, opts)
744+
const result = await executeTool(
745+
key,
746+
args,
747+
{ sessionID: ctx.sessionID, callID: opts.toolCallId! },
748+
async () => {
749+
await ctx.ask({
750+
permission: key,
751+
metadata: {},
752+
patterns: ["*"],
753+
always: ["*"],
754+
})
756755

757-
await Plugin.trigger(
758-
"tool.execute.after",
759-
{
760-
tool: key,
761-
sessionID: ctx.sessionID,
762-
callID: opts.toolCallId,
756+
return execute(args, opts)
763757
},
764-
result,
765758
)
766759

767760
const textParts: string[] = []
@@ -1072,7 +1065,12 @@ export namespace SessionPrompt {
10721065
metadata: async () => {},
10731066
ask: async () => {},
10741067
}
1075-
const result = await ListTool.init().then((t) => t.execute(args, listCtx))
1068+
const result = await executeTool(
1069+
"ls",
1070+
args,
1071+
{ sessionID: input.sessionID, callID: info.id },
1072+
() => ListTool.init().then((t) => t.execute(args, listCtx)),
1073+
)
10761074
return [
10771075
{
10781076
id: Identifier.ascending("part"),
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { Plugin } from "../plugin"
2+
import type { Tool } from "@/tool/tool"
3+
import { Log } from "../util/log"
4+
5+
const logger = Log.create({ service: "session/tool-executor" })
6+
7+
export namespace ToolExecutor {
8+
/**
9+
* Centralized tool execution logic.
10+
*
11+
* This function encapsulates the common pattern for executing tools:
12+
* 1. Trigger "tool.execute.before" plugin hook
13+
* 2. Execute the tool with error handling
14+
* 3. Trigger "tool.execute.after" plugin hook
15+
* 4. Handle errors with logging
16+
*
17+
* Used by:
18+
* - Standard tool execution (prompt.ts:700-723)
19+
* - Subtask tool execution (prompt.ts:318-429)
20+
* - Special tool cases (e.g., list tool auto-invocation)
21+
*
22+
* @param toolID - The tool identifier (e.g., "bash", "read")
23+
* @param tool - The initialized tool info object (result of await toolInfo.init())
24+
* @param args - The arguments to pass to the tool
25+
* @param ctx - The tool execution context
26+
* @returns The tool execution result or undefined if execution failed
27+
*/
28+
export async function execute<T extends Tool.Metadata = Tool.Metadata>(
29+
toolID: string,
30+
tool: Awaited<ReturnType<Tool.Info["init"]>>,
31+
args: any,
32+
ctx: Tool.Context,
33+
): Promise<{ title: string; metadata: T; output: string; attachments?: any[] } | undefined> {
34+
try {
35+
// Trigger before hook
36+
await Plugin.trigger(
37+
"tool.execute.before",
38+
{
39+
tool: toolID,
40+
sessionID: ctx.sessionID,
41+
callID: ctx.callID,
42+
},
43+
{ args },
44+
)
45+
46+
// Execute tool
47+
const result = await tool.execute(args, ctx)
48+
49+
// Trigger after hook
50+
await Plugin.trigger(
51+
"tool.execute.after",
52+
{
53+
tool: toolID,
54+
sessionID: ctx.sessionID,
55+
callID: ctx.callID,
56+
},
57+
result,
58+
)
59+
60+
return result
61+
} catch (error) {
62+
const err = error as Error
63+
64+
logger.error("Tool execution failed", {
65+
tool: toolID,
66+
error: err.message,
67+
sessionID: ctx.sessionID,
68+
callID: ctx.callID,
69+
})
70+
71+
return undefined
72+
}
73+
}
74+
}

0 commit comments

Comments
 (0)