diff --git a/src/hooks/atlas/tool-execute-before.ts b/src/hooks/atlas/tool-execute-before.ts index f4e03ad65..d9ede7484 100644 --- a/src/hooks/atlas/tool-execute-before.ts +++ b/src/hooks/atlas/tool-execute-before.ts @@ -1,4 +1,5 @@ import { log } from "../../shared/logger" +import { replaceToolArgs } from "../../shared/replace-tool-args" import { SYSTEM_DIRECTIVE_PREFIX } from "../../shared/system-directive" import { isCallerOrchestrator } from "../../shared/session-utils" import type { PluginInput } from "@opencode-ai/plugin" @@ -179,7 +180,7 @@ export function createToolExecuteBeforeHandler(input: { const prompt = toolOutput.args.prompt as string | undefined if (prompt && !prompt.includes(SYSTEM_DIRECTIVE_PREFIX)) { - toolOutput.args.prompt = `${SINGLE_TASK_DIRECTIVE}\n` + prompt + replaceToolArgs(toolOutput, { prompt: `${SINGLE_TASK_DIRECTIVE}\n` + prompt }) log(`[${HOOK_NAME}] Injected single-task directive to task`, { sessionID: toolInput.sessionID, }) diff --git a/src/hooks/claude-code-hooks/handlers/tool-execute-before-handler.ts b/src/hooks/claude-code-hooks/handlers/tool-execute-before-handler.ts index 412cc2c23..612a63696 100644 --- a/src/hooks/claude-code-hooks/handlers/tool-execute-before-handler.ts +++ b/src/hooks/claude-code-hooks/handlers/tool-execute-before-handler.ts @@ -8,7 +8,7 @@ import { import { appendTranscriptEntry } from "../transcript" import { cacheToolInput } from "../tool-input-cache" import type { PluginConfig } from "../types" -import { isHookDisabled, log } from "../../../shared" +import { isHookDisabled, log, replaceToolArgs } from "../../../shared" export function createToolExecuteBeforeHandler(ctx: PluginInput, config: PluginConfig) { return async ( @@ -39,7 +39,7 @@ export function createToolExecuteBeforeHandler(ctx: PluginInput, config: PluginC ) } - output.args.todos = parsed + replaceToolArgs(output, { todos: parsed }) log("todowrite: parsed todos string to array", { sessionID: input.sessionID }) } @@ -87,7 +87,7 @@ export function createToolExecuteBeforeHandler(ctx: PluginInput, config: PluginC } if (result.modifiedInput) { - Object.assign(output.args, result.modifiedInput) + replaceToolArgs(output, result.modifiedInput as Record) } } } diff --git a/src/hooks/compaction-todo-preserver/hook.ts b/src/hooks/compaction-todo-preserver/hook.ts index 63e744c9c..dd2ea5c82 100644 --- a/src/hooks/compaction-todo-preserver/hook.ts +++ b/src/hooks/compaction-todo-preserver/hook.ts @@ -1,6 +1,7 @@ import type { PluginInput } from "@opencode-ai/plugin" import { resolveSessionEventID } from "../../shared/event-session-id" import { log } from "../../shared/logger" +import { replaceToolArgs } from "../../shared/replace-tool-args" interface TodoSnapshot { id?: string @@ -233,7 +234,7 @@ export function createCompactionTodoPreserverHook( return } - output.args.todos = snapshot + replaceToolArgs(output, { todos: snapshot }) log(`[${HOOK_NAME}] Replaced late Atlas bootstrap todowrite with restored snapshot`, { sessionID: input.sessionID, count: snapshot.length, diff --git a/src/hooks/non-interactive-env/non-interactive-env-hook.ts b/src/hooks/non-interactive-env/non-interactive-env-hook.ts index 6fc42aea9..42cb25122 100644 --- a/src/hooks/non-interactive-env/non-interactive-env-hook.ts +++ b/src/hooks/non-interactive-env/non-interactive-env-hook.ts @@ -1,6 +1,6 @@ import type { PluginInput } from "@opencode-ai/plugin" import { HOOK_NAME, NON_INTERACTIVE_ENV, SHELL_COMMAND_PATTERNS } from "./constants" -import { log, buildEnvPrefix } from "../../shared" +import { log, buildEnvPrefix, replaceToolArgs } from "../../shared" import { detectShellType, type ShellType } from "../../shared/shell-env" export * from "./constants" @@ -97,7 +97,7 @@ export function createNonInteractiveEnvHook(_ctx: PluginInput) { return } - output.args.command = `${envPrefix} ${command}` + replaceToolArgs(output, { command: `${envPrefix} ${command}` }) log(`[${HOOK_NAME}] Prepended non-interactive env vars to git command`, { sessionID: input.sessionID, diff --git a/src/hooks/prometheus-md-only/hook.ts b/src/hooks/prometheus-md-only/hook.ts index 96f5093bd..57ee04a3d 100644 --- a/src/hooks/prometheus-md-only/hook.ts +++ b/src/hooks/prometheus-md-only/hook.ts @@ -1,6 +1,7 @@ import type { PluginInput } from "@opencode-ai/plugin" import { HOOK_NAME, BLOCKED_TOOLS, PLANNING_CONSULT_WARNING, PROMETHEUS_WORKFLOW_REMINDER } from "./constants" import { log } from "../../shared/logger" +import { replaceToolArgs } from "../../shared/replace-tool-args" import { SYSTEM_DIRECTIVE_PREFIX } from "../../shared/system-directive" import { getAgentDisplayName } from "../../shared/agent-display-names" import { getAgentFromSession } from "./agent-resolution" @@ -27,7 +28,7 @@ export function createPrometheusMdOnlyHook(ctx: PluginInput) { if (TASK_TOOLS.includes(toolName)) { const prompt = output.args.prompt as string | undefined if (prompt && !prompt.includes(SYSTEM_DIRECTIVE_PREFIX)) { - output.args.prompt = PLANNING_CONSULT_WARNING + prompt + replaceToolArgs(output, { prompt: PLANNING_CONSULT_WARNING + prompt }) log(`[${HOOK_NAME}] Injected planning warning to ${toolName}`, { sessionID: input.sessionID, tool: toolName, diff --git a/src/hooks/question-label-truncator/hook.ts b/src/hooks/question-label-truncator/hook.ts index a43fa4fc7..ed28a394a 100644 --- a/src/hooks/question-label-truncator/hook.ts +++ b/src/hooks/question-label-truncator/hook.ts @@ -1,3 +1,5 @@ +import { replaceToolArgs } from "../../shared/replace-tool-args" + const MAX_LABEL_LENGTH = 30; interface QuestionOption { @@ -56,7 +58,7 @@ export function createQuestionLabelTruncatorHook() { if (toolName === "askuserquestion" || toolName === "ask_user_question") { if (hasQuestions(output.args)) { const truncatedArgs = truncateQuestionLabels(output.args); - Object.assign(output.args, truncatedArgs); + replaceToolArgs(output, { questions: truncatedArgs.questions }); } } }, diff --git a/src/hooks/sisyphus-junior-notepad/hook.ts b/src/hooks/sisyphus-junior-notepad/hook.ts index 28a284e6f..5e9c00456 100644 --- a/src/hooks/sisyphus-junior-notepad/hook.ts +++ b/src/hooks/sisyphus-junior-notepad/hook.ts @@ -3,6 +3,7 @@ import type { PluginInput } from "@opencode-ai/plugin" import { isCallerOrchestrator } from "../../shared/session-utils" import { SYSTEM_DIRECTIVE_PREFIX } from "../../shared/system-directive" import { log } from "../../shared/logger" +import { replaceToolArgs } from "../../shared/replace-tool-args" import { HOOK_NAME, NOTEPAD_DIRECTIVE } from "./constants" export function createSisyphusJuniorNotepadHook(ctx: PluginInput) { @@ -33,7 +34,7 @@ export function createSisyphusJuniorNotepadHook(ctx: PluginInput) { } // 5. Prepend directive - output.args.prompt = NOTEPAD_DIRECTIVE + prompt + replaceToolArgs(output, { prompt: NOTEPAD_DIRECTIVE + prompt }) // 6. Log injection log(`[${HOOK_NAME}] Injected notepad directive to task`, { diff --git a/src/hooks/webfetch-redirect-guard/hook.ts b/src/hooks/webfetch-redirect-guard/hook.ts index 1599bc1eb..055498fe6 100644 --- a/src/hooks/webfetch-redirect-guard/hook.ts +++ b/src/hooks/webfetch-redirect-guard/hook.ts @@ -1,5 +1,5 @@ import type { PluginInput } from "@opencode-ai/plugin" -import { log } from "../../shared" +import { log, replaceToolArgs } from "../../shared" import { MAX_WEBFETCH_REDIRECTS, WEBFETCH_REDIRECT_ERROR_PATTERNS, @@ -85,7 +85,7 @@ export function createWebFetchRedirectGuardHook(_ctx: PluginInput) { }) if (resolution.type === "resolved") { - output.args.url = resolution.url + replaceToolArgs(output, { url: resolution.url }) return } diff --git a/src/plugin/tool-execute-before.ts b/src/plugin/tool-execute-before.ts index 3b66aa2c9..508f40dde 100644 --- a/src/plugin/tool-execute-before.ts +++ b/src/plugin/tool-execute-before.ts @@ -3,7 +3,7 @@ import { randomUUID } from "node:crypto" import { getMainSessionID } from "../features/claude-code-session-state" import { clearBoulderState } from "../features/boulder-state" -import { log } from "../shared" +import { log, replaceToolArgs } from "../shared" import { stripInvisibleAgentCharacters } from "../shared/agent-display-names" import { resolveSessionAgent } from "./session-agent-resolver" import { parseRalphLoopArguments } from "../hooks/ralph-loop/command-arguments" @@ -54,7 +54,7 @@ export function createToolExecuteBeforeHandler(args: { return async (input, output): Promise => { if (input.tool.toLowerCase() === "bash" && typeof output.args.command === "string") { if (output.args.command.includes("\x00")) { - output.args.command = output.args.command.replace(/\x00/g, "") + replaceToolArgs(output, { command: output.args.command.replace(/\x00/g, "") }) log("[tool-execute-before] Stripped null bytes from bash command", { sessionID: input.sessionID, callID: input.callID, @@ -100,21 +100,20 @@ export function createToolExecuteBeforeHandler(args: { } if (input.tool === "task") { - const argsObject = output.args - const category = typeof argsObject.category === "string" ? argsObject.category : undefined - const subagentType = typeof argsObject.subagent_type === "string" ? argsObject.subagent_type : undefined - const taskId = typeof argsObject.task_id === "string" ? argsObject.task_id : undefined + const category = typeof output.args.category === "string" ? output.args.category : undefined + const subagentType = typeof output.args.subagent_type === "string" ? output.args.subagent_type : undefined + const taskId = typeof output.args.task_id === "string" ? output.args.task_id : undefined if (category) { - argsObject.subagent_type = "sisyphus-junior" + replaceToolArgs(output, { subagent_type: "sisyphus-junior" }) } else if (!subagentType && taskId) { const resolvedAgent = await resolveSessionAgent(ctx.client, taskId) - argsObject.subagent_type = resolvedAgent ?? "continue" + replaceToolArgs(output, { subagent_type: resolvedAgent ?? "continue" }) } const normalizedSubagentType = - typeof argsObject.subagent_type === "string" ? stripInvisibleAgentCharacters(argsObject.subagent_type) : undefined - const prompt = typeof argsObject.prompt === "string" ? argsObject.prompt : "" + typeof output.args.subagent_type === "string" ? stripInvisibleAgentCharacters(output.args.subagent_type) : undefined + const prompt = typeof output.args.prompt === "string" ? output.args.prompt : "" const loopState = typeof ctx.directory === "string" ? readState(ctx.directory) : null const shouldInjectOracleVerification = normalizedSubagentType === "oracle" @@ -136,12 +135,14 @@ export function createToolExecuteBeforeHandler(args: { verification_attempt_id: verificationAttemptId, verification_session_id: undefined, }) - argsObject.run_in_background = false - argsObject.prompt = buildUltraworkOracleVerificationPrompt( - prompt, - loopState.prompt, - verificationAttemptId, - ) + replaceToolArgs(output, { + run_in_background: false, + prompt: buildUltraworkOracleVerificationPrompt( + prompt, + loopState.prompt, + verificationAttemptId, + ), + }) } } diff --git a/src/shared/index.ts b/src/shared/index.ts index 07103d094..17ebfd0c9 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -84,3 +84,4 @@ export * from "./task-system-enabled" export * from "./parse-tools-config" export { parseModelString } from "./model-string-parser" export { EXCLUDED_DIRS } from "./excluded-dirs" +export * from "./replace-tool-args" diff --git a/src/shared/replace-tool-args.audit.test.ts b/src/shared/replace-tool-args.audit.test.ts new file mode 100644 index 000000000..1c85ee6d9 --- /dev/null +++ b/src/shared/replace-tool-args.audit.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect } from "bun:test" +import { readdir, readFile } from "node:fs/promises" +import { join, relative } from "node:path" + +const SRC_DIR = join(import.meta.dir, "..") + +async function collectTsFiles(dir: string): Promise { + const results: string[] = [] + const entries = await readdir(dir, { withFileTypes: true }) + + for (const entry of entries) { + const fullPath = join(dir, entry.name) + if (entry.isDirectory()) { + if (entry.name === "node_modules" || entry.name === "dist") continue + results.push(...(await collectTsFiles(fullPath))) + } else if (entry.name.endsWith(".ts") && !entry.name.endsWith(".test.ts") && !entry.name.endsWith(".audit.test.ts")) { + results.push(fullPath) + } + } + + return results +} + +const HELPER_FILE = "shared/replace-tool-args.ts" + +// Matches direct mutations like `output.args.foo =` or `toolOutput.args.foo =` +// but excludes comparisons (===, !==, ==) +const DIRECT_MUTATION_PATTERN = /\w*[Oo]utput\.args\.\w+\s*=[^=]/g +const OBJECT_ASSIGN_PATTERN = /Object\.assign\(\s*\w*[Oo]utput\.args/g + +describe("replace-tool-args audit", () => { + it("#given src/**/*.ts files #when scanning for direct output.args mutation #then no matches found outside the helper", async () => { + // given + const files = await collectTsFiles(SRC_DIR) + const violations: string[] = [] + + // when + for (const file of files) { + const relPath = relative(SRC_DIR, file) + if (relPath === HELPER_FILE) continue + + const content = await readFile(file, "utf-8") + const lines = content.split("\n") + + for (let i = 0; i < lines.length; i++) { + const line = lines[i] + if (DIRECT_MUTATION_PATTERN.test(line)) { + violations.push(`${relPath}:${i + 1}: ${line.trim()}`) + } + DIRECT_MUTATION_PATTERN.lastIndex = 0 + + if (OBJECT_ASSIGN_PATTERN.test(line)) { + violations.push(`${relPath}:${i + 1}: ${line.trim()}`) + } + OBJECT_ASSIGN_PATTERN.lastIndex = 0 + } + } + + // then + expect(violations).toEqual([]) + }) +}) diff --git a/src/shared/replace-tool-args.test.ts b/src/shared/replace-tool-args.test.ts new file mode 100644 index 000000000..113d8ab65 --- /dev/null +++ b/src/shared/replace-tool-args.test.ts @@ -0,0 +1,148 @@ +import { describe, it, expect } from "bun:test" +import { replaceToolArgs } from "./replace-tool-args" + +describe("replaceToolArgs", () => { + describe("#given a mutable output.args object", () => { + it("#when patching a single property #then the output.args contains the patched value", () => { + // given + const output = { args: { command: "git status", timeout: 30 } as Record } + + // when + replaceToolArgs(output, { command: "git log" }) + + // then + expect(output.args.command).toBe("git log") + expect(output.args.timeout).toBe(30) + }) + + it("#when patching multiple properties #then all patched values are present", () => { + // given + const output = { args: { url: "http://old.com", format: "text" } as Record } + + // when + replaceToolArgs(output, { url: "http://new.com", format: "markdown" }) + + // then + expect(output.args.url).toBe("http://new.com") + expect(output.args.format).toBe("markdown") + }) + + it("#when patching #then the original args object is not the same reference", () => { + // given + const originalArgs = { command: "echo hi" } as Record + const output = { args: originalArgs } + + // when + replaceToolArgs(output, { command: "echo bye" }) + + // then + expect(output.args).not.toBe(originalArgs) + expect(originalArgs.command).toBe("echo hi") + }) + }) + + describe("#given a frozen output.args object", () => { + it("#when patching a single property #then no TypeError is thrown and the value is updated", () => { + // given + const output = { args: Object.freeze({ command: "git status", timeout: 30 }) as Record } + + // when / then + expect(() => replaceToolArgs(output, { command: "git log" })).not.toThrow() + expect(output.args.command).toBe("git log") + expect(output.args.timeout).toBe(30) + }) + + it("#when patching with Object-typed value #then no TypeError is thrown", () => { + // given + const output = { args: Object.freeze({ todos: "[]" }) as Record } + const parsed = [{ id: "1", content: "test", status: "pending" }] + + // when / then + expect(() => replaceToolArgs(output, { todos: parsed })).not.toThrow() + expect(output.args.todos).toEqual(parsed) + }) + + it("#when patching url on frozen webfetch args #then no TypeError is thrown", () => { + // given + const output = { args: Object.freeze({ url: "http://old.com", format: "markdown" }) as Record } + + // when / then + expect(() => replaceToolArgs(output, { url: "http://redirected.com" })).not.toThrow() + expect(output.args.url).toBe("http://redirected.com") + expect(output.args.format).toBe("markdown") + }) + + it("#when patching command with env prefix on frozen bash args #then no TypeError is thrown", () => { + // given + const output = { args: Object.freeze({ command: "git rebase --continue" }) as Record } + + // when / then + expect(() => replaceToolArgs(output, { command: "GIT_EDITOR=: git rebase --continue" })).not.toThrow() + expect(output.args.command).toBe("GIT_EDITOR=: git rebase --continue") + }) + + it("#when patching prompt on frozen task args #then no TypeError is thrown", () => { + // given + const output = { args: Object.freeze({ prompt: "Do the thing", category: "quick" }) as Record } + + // when / then + expect(() => replaceToolArgs(output, { prompt: "[DIRECTIVE] Do the thing" })).not.toThrow() + expect(output.args.prompt).toBe("[DIRECTIVE] Do the thing") + expect(output.args.category).toBe("quick") + }) + + it("#when stripping null bytes from frozen command #then no TypeError is thrown", () => { + // given + const frozenCommand = "echo \x00hello" + const output = { args: Object.freeze({ command: frozenCommand }) as Record } + + // when / then + expect(() => replaceToolArgs(output, { command: "echo hello" })).not.toThrow() + expect(output.args.command).toBe("echo hello") + }) + + it("#when replacing truncated question labels on frozen args #then no TypeError is thrown", () => { + // given + const output = { + args: Object.freeze({ + questions: [{ question: "Pick", options: [{ label: "A very long label that should be truncated" }] }], + }) as Record, + } + const truncated = { + questions: [{ question: "Pick", options: [{ label: "A very long label that sho..." }] }], + } + + // when / then + expect(() => replaceToolArgs(output, truncated)).not.toThrow() + expect((output.args.questions as Array<{ options: Array<{ label: string }> }>)[0].options[0].label).toBe( + "A very long label that sho...", + ) + }) + + it("#when replacing modifiedInput from PreToolUse hook on frozen args #then no TypeError is thrown", () => { + // given + const output = { args: Object.freeze({ filePath: "/old/path.ts" }) as Record } + + // when / then + expect(() => replaceToolArgs(output, { filePath: "/new/path.ts" })).not.toThrow() + expect(output.args.filePath).toBe("/new/path.ts") + }) + + it("#when replacing todo snapshot on frozen args #then no TypeError is thrown", () => { + // given + const output = { + args: Object.freeze({ + todos: [{ content: "bootstrap", status: "pending" }], + }) as Record, + } + const snapshot = [ + { content: "Real task 1", status: "in_progress" }, + { content: "Real task 2", status: "pending" }, + ] + + // when / then + expect(() => replaceToolArgs(output, { todos: snapshot })).not.toThrow() + expect(output.args.todos).toEqual(snapshot) + }) + }) +}) diff --git a/src/shared/replace-tool-args.ts b/src/shared/replace-tool-args.ts new file mode 100644 index 000000000..0a0566ad3 --- /dev/null +++ b/src/shared/replace-tool-args.ts @@ -0,0 +1,16 @@ +/** + * Safely replace tool arguments without mutating frozen objects. + * + * opencode >=1.14 may freeze `output.args` via Immer before plugin hooks run. + * Direct property assignment (`output.args.key = value`) or `Object.assign(output.args, patch)` + * throws `TypeError: Attempted to assign to readonly property` on a frozen object. + * + * This helper replaces `output.args` with a shallow clone containing the patch, + * which works regardless of whether the original args object is frozen. + */ +export function replaceToolArgs( + output: { args: Record }, + patch: Record, +): void { + output.args = { ...output.args, ...patch } +}