Skip to content

Commit 4dc8004

Browse files
NiveditJainclaude
andauthored
[luv-338] fix: canonicalize OpenCode + Pi tool-input arg keys (#337)
* [luv-338] fix: canonicalize OpenCode + Pi tool-input arg keys so path-checking builtins fire OpenCode's `read` slipped past `block-read-outside-cwd`: the shim canonicalized the tool name (`read` → `Read`) but forwarded `output.args` verbatim. OpenCode delivers `{ filePath }`; the policy reads `ctx.toolInput.file_path`, so `getFilePath()` returned "" and the target-empty short-circuit at builtin-policies.ts:799 allowed the read. Same family of bug on Pi with a different mismatch shape: Pi's read/write/edit use `path`. `block-read-outside-cwd` happens to work on Pi via the existing `tool_input.path` fallback at builtin-policies.ts:796, but `block-env-files` and `block-secrets-write` only check `file_path` and were silently no-op'd. Mirrors the TOOL_NAME_MAP pattern from #293 with two new per-tool input-key maps keyed by canonical PascalCase tool name: • OPENCODE_TOOL_INPUT_MAP — Read/Write/Edit: filePath→file_path, plus oldString/newString/replaceAll for Edit • PI_TOOL_INPUT_MAP — Read/Write/Edit: path→file_path (Pi's nested edits[{oldText,newText}] array doesn't flat-rename, no current builtin reads it) Both maps are mirrored inline in their shims so .opencode/plugins/ failproofai.mjs and pi-extension/index.ts stay self-contained. MCP mcp_* and any unmapped tool pass through unchanged. Existing users must re-run `failproofai policies --install --cli opencode` / `--cli pi` to regenerate their shims and pick up the fix. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * [luv-338] docs: document OPENCODE_TOOL_INPUT_MAP and PI_TOOL_INPUT_MAP in configuration.mdx Mirror the existing tool-name canonicalization wording in the OpenCode and Pi sections. Calls out which builtin policies start firing as a result (block-read-outside-cwd / block-env-files / block-secrets-write on OpenCode; block-env-files / block-secrets-write on Pi). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * [luv-338] chore: stamp PR number in CHANGELOG entry Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 9f0b14b commit 4dc8004

8 files changed

Lines changed: 299 additions & 12 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
### Fixes
66
- Read full session UUID from each Gemini JSONL's metadata header at project-page session-listing time (`lib/gemini-projects.ts`), so links route to a valid `[sessionId]` segment instead of the 8-hex filename prefix that the session detail route's `UUID_RE` check rejects (404). Hooks-section links were already correct because hook stdin carries the full UUID; this aligns the projects-section with that path (#336).
7+
- Canonicalize OpenCode and Pi tool-input arg keys so the path-checking builtin policies actually fire on `read` / `write` / `edit` tool calls. OpenCode delivers args as `filePath` / `oldString` / `newString` / `replaceAll`; Pi delivers `path`. The failproofai builtins read `ctx.toolInput.file_path`, so the shape mismatch silently no-op'd `block-read-outside-cwd` (OpenCode), `block-env-files`, and `block-secrets-write` for both CLIs — letting an OpenCode session read paths outside its CWD without any deny, and letting Pi sessions write to `.env` / SSH-key paths unchecked. Note: `block-read-outside-cwd` already worked on Pi via an existing `tool_input.path` fallback at `src/hooks/builtin-policies.ts:796`, so only `block-env-files` and `block-secrets-write` were affected on Pi. Mirrors the `OPENCODE_TOOL_MAP` / `PI_TOOL_MAP` pattern from PR #293 with two new per-tool maps keyed by canonical PascalCase tool name: `OPENCODE_TOOL_INPUT_MAP` (Read / Write / Edit) and `PI_TOOL_INPUT_MAP` (Read / Write / Edit, top-level `path` only — Pi's nested `edits[{oldText,newText}]` array isn't a flat key rename). Both maps are mirrored inline in their respective shims so `.opencode/plugins/failproofai.mjs` and `pi-extension/index.ts` stay self-contained; MCP `mcp_*` and any unmapped tool pass through unchanged. Existing OpenCode users must regenerate their shim via `failproofai policies --install --cli opencode` to pick up the fix; Pi users must reinstall via `failproofai policies --install --cli pi` (#337).
78
- Route OpenCode project pages by encoded cwd (`encodeFolderName(worktree)`) instead of opencode's project name / basename, fixing the dashboard `/project/<slug>` 404 for OpenCode-only sessions and merging same-cwd OpenCode + other-CLI rows on the Projects page (#335).
89
- `.failproofai/policies/workflow-policies.mjs`: drop the `## Unreleased` section; new `release-prep-check` policy + updated `changelog-check` instruct the agent to put entries under a dated `## <version> — <YYYY-MM-DD>` heading so each PR ships release-ready, and all four workflow policies now anchor command-phrase matches to shell boundaries to avoid false-positives from HEREDOC bodies (#335).
910

__tests__/e2e/hooks/opencode-integration.e2e.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,28 @@ describe("E2E: OpenCode integration — hook protocol", () => {
208208
}
209209
});
210210

211+
it("Read of a path outside cwd is denied by block-read-outside-cwd (regression for arg-key canonicalization gap)", () => {
212+
// Regression for the bug where opencode's `read` tool delivered
213+
// `tool_input.filePath` (camelCase) but the policy reads
214+
// `ctx.toolInput.file_path`, letting a read outside cwd slip through.
215+
// The shim now canonicalizes the arg keys; this test asserts the policy
216+
// fires when the binary sees the post-canonicalized Claude-shape payload.
217+
const env = createOpenCodeEnv();
218+
try {
219+
writeConfig(env.cwd, ["block-read-outside-cwd"]);
220+
const result = runHook(
221+
"PreToolUse",
222+
OpenCodePayloads.preToolUse.read("/etc/passwd", env.cwd),
223+
{ homeDir: env.home, cli: "opencode" },
224+
);
225+
assertPreToolUseDeny(result);
226+
const out = result.parsed?.hookSpecificOutput as Record<string, unknown> | undefined;
227+
expect(out?.permissionDecisionReason).toMatch(/outside project directory/i);
228+
} finally {
229+
env.cleanup();
230+
}
231+
});
232+
211233
it("Bash read of .opencode/plugins/failproofai.mjs is denied by the agent-settings guard", () => {
212234
const env = createOpenCodeEnv();
213235
try {

__tests__/hooks/opencode-plugin-shim.test.ts

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,96 @@ describe("OpenCode plugin shim — translation of plugin events to binary stdin"
169169
expect(stdin.tool_name).toBe("mcp_github_create_issue");
170170
});
171171

172+
it("tool.execute.before translates OpenCode camelCase arg keys to Claude snake_case for Read", async () => {
173+
// Regression for the missing arg-key canonicalization that let a `read`
174+
// tool call slip past `block-read-outside-cwd`: opencode delivers
175+
// `{ filePath, offset, limit }` but the policy reads `ctx.toolInput.file_path`.
176+
responses.push({ status: 0, stdout: "", stderr: "" });
177+
const { plugin } = await setup();
178+
const hooks = await plugin({ client: fakeClient(), directory: "/repo" });
179+
await hooks["tool.execute.before"]!(
180+
{ tool: "read", sessionID: "ses_1", callID: "c1" },
181+
{ args: { filePath: "/etc/passwd", offset: 0, limit: 2000 } },
182+
);
183+
const stdin = JSON.parse(calls[0].opts.input!);
184+
expect(stdin.tool_name).toBe("Read");
185+
// filePath is renamed; unmapped keys (offset, limit) pass through unchanged.
186+
expect(stdin.tool_input).toEqual({ file_path: "/etc/passwd", offset: 0, limit: 2000 });
187+
});
188+
189+
it("tool.execute.before translates all four Edit arg keys", async () => {
190+
responses.push({ status: 0, stdout: "", stderr: "" });
191+
const { plugin } = await setup();
192+
const hooks = await plugin({ client: fakeClient(), directory: "/repo" });
193+
await hooks["tool.execute.before"]!(
194+
{ tool: "edit", sessionID: "ses_1", callID: "c1" },
195+
{ args: { filePath: "/repo/x", oldString: "a", newString: "b", replaceAll: true } },
196+
);
197+
const stdin = JSON.parse(calls[0].opts.input!);
198+
expect(stdin.tool_name).toBe("Edit");
199+
expect(stdin.tool_input).toEqual({
200+
file_path: "/repo/x",
201+
old_string: "a",
202+
new_string: "b",
203+
replace_all: true,
204+
});
205+
});
206+
207+
it("tool.execute.before passes unknown-tool args through unchanged (MCP / extensions)", async () => {
208+
responses.push({ status: 0, stdout: "", stderr: "" });
209+
const { plugin } = await setup();
210+
const hooks = await plugin({ client: fakeClient(), directory: "/repo" });
211+
await hooks["tool.execute.before"]!(
212+
{ tool: "mcp_github_create_issue", sessionID: "ses_1", callID: "c1" },
213+
{ args: { title: "x", filePath: "/literal/key/name" } },
214+
);
215+
const stdin = JSON.parse(calls[0].opts.input!);
216+
// Unknown tools pass through with the OpenCode camelCase shape — we don't
217+
// rewrite keys we can't claim to understand.
218+
expect(stdin.tool_input).toEqual({ title: "x", filePath: "/literal/key/name" });
219+
});
220+
221+
it("tool.execute.after also canonicalizes input args", async () => {
222+
responses.push({ status: 0, stdout: "", stderr: "" });
223+
const { plugin } = await setup();
224+
const hooks = await plugin({ client: fakeClient(), directory: "/repo" });
225+
await hooks["tool.execute.after"]!(
226+
{ tool: "write", sessionID: "ses_1", callID: "c1", args: { filePath: "/repo/x", content: "hi" } },
227+
{ title: "ok", output: "", metadata: {} },
228+
);
229+
const stdin = JSON.parse(calls[0].opts.input!);
230+
expect(stdin.tool_name).toBe("Write");
231+
expect(stdin.tool_input).toEqual({ file_path: "/repo/x", content: "hi" });
232+
});
233+
234+
it("tool.execute.before canonicalizes every OPENCODE_TOOL_INPUT_MAP entry", async () => {
235+
// Parity with the OPENCODE_TOOL_MAP coverage test below — keeps the
236+
// shim's inline map in sync with the exported map in src/hooks/types.ts.
237+
const { OPENCODE_TOOL_INPUT_MAP } = await import("../../src/hooks/types");
238+
const { plugin } = await setup();
239+
const hooks = await plugin({ client: fakeClient(), directory: "/repo" });
240+
// Reverse-canonical-name → opencode raw name lookup so we can drive each
241+
// case via the lowercase tool ID the plugin actually receives.
242+
const rawByCanonical: Record<string, string> = {
243+
Read: "read",
244+
Write: "write",
245+
Edit: "edit",
246+
};
247+
for (const canonical of Object.keys(OPENCODE_TOOL_INPUT_MAP)) {
248+
const raw = rawByCanonical[canonical];
249+
const map = OPENCODE_TOOL_INPUT_MAP[canonical];
250+
const args: Record<string, unknown> = {};
251+
for (const camel of Object.keys(map)) args[camel] = `v_${camel}`;
252+
responses.push({ status: 0, stdout: "", stderr: "" });
253+
await hooks["tool.execute.before"]!({ tool: raw, sessionID: "s", callID: "c" }, { args });
254+
const stdin = JSON.parse(calls[calls.length - 1].opts.input!);
255+
for (const camel of Object.keys(map)) {
256+
expect(stdin.tool_input[map[camel]]).toBe(`v_${camel}`);
257+
expect(stdin.tool_input[camel]).toBeUndefined();
258+
}
259+
}
260+
});
261+
172262
it("tool.execute.before canonicalizes every OPENCODE_TOOL_MAP entry", async () => {
173263
const { plugin } = await setup();
174264
const hooks = await plugin({ client: fakeClient(), directory: "/repo" });

__tests__/hooks/pi-extension-shim.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,71 @@ describe("pi-extension shim — sessionId resolution via on-disk discovery", ()
167167
expect(captured.at(-1)?.payload.session_id).toBe(b);
168168
});
169169

170+
it("translates Pi `path` arg to Claude `file_path` for read tool", () => {
171+
// Regression for the same arg-key canonicalization gap that bit OpenCode:
172+
// Pi's read tool delivers `{ path }`, but block-env-files /
173+
// block-secrets-write only check `tool_input.file_path`. Without the
174+
// PI_TOOL_INPUT_MAP, those policies silently no-op on Pi.
175+
handlers.tool_call({ type: "tool_call", toolName: "read", input: { path: "/etc/passwd", offset: 0, limit: 100 }, cwd: "/proj" });
176+
const payload = captured.at(-1)?.payload as { tool_name: string; tool_input: Record<string, unknown> };
177+
expect(payload.tool_name).toBe("Read");
178+
// path → file_path; unmapped offset/limit pass through unchanged.
179+
expect(payload.tool_input).toEqual({ file_path: "/etc/passwd", offset: 0, limit: 100 });
180+
});
181+
182+
it("translates Pi `path` arg for write tool", () => {
183+
handlers.tool_call({ type: "tool_call", toolName: "write", input: { path: "/proj/.env", content: "SECRET=1" }, cwd: "/proj" });
184+
const payload = captured.at(-1)?.payload as { tool_name: string; tool_input: Record<string, unknown> };
185+
expect(payload.tool_name).toBe("Write");
186+
expect(payload.tool_input).toEqual({ file_path: "/proj/.env", content: "SECRET=1" });
187+
});
188+
189+
it("translates Pi `path` for edit tool top-level only — nested edits[] stays Pi-shape", () => {
190+
// Pi's edit tool: { path, edits: [{oldText, newText}, …] } — different
191+
// structurally from Claude's flat { file_path, old_string, new_string }.
192+
// We can only translate the top-level `path` key; the nested array stays
193+
// as-is because it isn't a flat key→key rename.
194+
const edits = [{ oldText: "a", newText: "b" }];
195+
handlers.tool_call({ type: "tool_call", toolName: "edit", input: { path: "/proj/x", edits }, cwd: "/proj" });
196+
const payload = captured.at(-1)?.payload as { tool_name: string; tool_input: Record<string, unknown> };
197+
expect(payload.tool_name).toBe("Edit");
198+
expect(payload.tool_input).toEqual({ file_path: "/proj/x", edits });
199+
});
200+
201+
it("passes unknown-tool args through unchanged", () => {
202+
handlers.tool_call({ type: "tool_call", toolName: "mcp_github_create_issue", input: { title: "x", path: "/literal/key" }, cwd: "/proj" });
203+
const payload = captured.at(-1)?.payload as { tool_name: string; tool_input: Record<string, unknown> };
204+
// Unknown tools (MCP / extensions) keep their raw arg shape — we don't
205+
// rewrite keys we can't claim to understand.
206+
expect(payload.tool_input).toEqual({ title: "x", path: "/literal/key" });
207+
});
208+
209+
it("tool_result also canonicalizes input args", () => {
210+
handlers.tool_result({ type: "tool_result", toolName: "write", input: { path: "/proj/x", content: "hi" }, content: [], isError: false, cwd: "/proj" });
211+
const payload = captured.at(-1)?.payload as { tool_name: string; tool_input: Record<string, unknown> };
212+
expect(payload.tool_name).toBe("Write");
213+
expect(payload.tool_input).toEqual({ file_path: "/proj/x", content: "hi" });
214+
});
215+
216+
it("PI_TOOL_INPUT_MAP coverage parity with the inline shim map", async () => {
217+
// Drives every entry in the exported map through the shim to keep the
218+
// two copies in sync — same pattern as the OPENCODE_TOOL_INPUT_MAP test.
219+
const { PI_TOOL_INPUT_MAP } = await import("../../src/hooks/types");
220+
const rawByCanonical: Record<string, string> = { Read: "read", Write: "write", Edit: "edit" };
221+
for (const canonical of Object.keys(PI_TOOL_INPUT_MAP)) {
222+
const raw = rawByCanonical[canonical];
223+
const map = PI_TOOL_INPUT_MAP[canonical];
224+
const args: Record<string, unknown> = {};
225+
for (const camel of Object.keys(map)) args[camel] = `v_${camel}`;
226+
handlers.tool_call({ type: "tool_call", toolName: raw, input: args, cwd: "/proj" });
227+
const payload = captured.at(-1)?.payload as { tool_input: Record<string, unknown> };
228+
for (const camel of Object.keys(map)) {
229+
expect(payload.tool_input[map[camel]]).toBe(`v_${camel}`);
230+
expect(payload.tool_input[camel]).toBeUndefined();
231+
}
232+
}
233+
});
234+
170235
// Cleanup
171236
afterEach(() => {
172237
if (originalEnv === undefined) delete process.env.PI_SESSIONS_DIR;

0 commit comments

Comments
 (0)