diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0e753ced7..3c2926fa4 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the `write` approval gate misclassifying `xd://` device writes as `exec` when the mounted tool declared a function-valued (argument-dependent) `approval`: the gate discarded the function and never decoded the device JSON payload, so read/write device operations prompted in non-yolo modes their approval mode permits. It now parses valid object payloads and evaluates the mounted tool's normal approval decision, while malformed JSON, non-object payloads, and unknown devices still fall back to `exec` and prompt ([#5727](https://github.com/can1357/oh-my-pi/issues/5727)). + ## [17.0.1] - 2026-07-16 ### Changed diff --git a/packages/coding-agent/src/tools/approval.ts b/packages/coding-agent/src/tools/approval.ts index c1d53fbe8..9b39eb0a7 100644 --- a/packages/coding-agent/src/tools/approval.ts +++ b/packages/coding-agent/src/tools/approval.ts @@ -75,6 +75,17 @@ function getToolDecision(tool: ApprovalSubject, args: unknown): Omit).content; + if (typeof rawContent !== "string") return "exec"; + let parsed: unknown; + try { + parsed = JSON.parse(rawContent); + } catch { + return "exec"; + } + if (!isRecord(parsed)) return "exec"; + return resolveToolTier(inst, parsed); } // Remote SSH writes open an outbound connection and run a remote shell — // gate them like the exec-tier `ssh` tool, ahead of the handler-write diff --git a/packages/coding-agent/test/write-xdev-dispatch.test.ts b/packages/coding-agent/test/write-xdev-dispatch.test.ts index 4d127ef1e..969fa3621 100644 --- a/packages/coding-agent/test/write-xdev-dispatch.test.ts +++ b/packages/coding-agent/test/write-xdev-dispatch.test.ts @@ -59,12 +59,12 @@ describe("read and write route xd:// device URLs", () => { paths: [filePath], }); - // Approval resolves a tier instead of throwing. A mounted tool whose own - // approval is a function (unresolvable statically) falls back to exec. + // The write gate decodes the device payload and evaluates the mounted + // tool's own approval. ast_edit is write-tier for a filesystem path. const approval = write!.approval; expect(typeof approval).toBe("function"); if (typeof approval === "function") { - expect(approval({ path: "xd://ast_edit", content })).toBe("exec"); + expect(approval({ path: "xd://ast_edit", content })).toBe("write"); } // Execute dispatches through the xdev registry to the mounted ast_edit, @@ -86,6 +86,47 @@ describe("read and write route xd:// device URLs", () => { } }); + it("resolves function-valued device approvals per payload and fails closed on bad content", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "write-xdev-approval-")); + try { + const filePath = path.join(tempDir, "target.ts"); + await Bun.write(filePath, "legacyWrap(x, value)\n"); + const tools = await createTools(xdevSession(tempDir)); + const write = tools.find(entry => entry.name === "write"); + expect(write).toBeDefined(); + const approval = write!.approval; + expect(typeof approval).toBe("function"); + if (typeof approval !== "function") throw new Error("expected a function approval"); + const tier = (path: string, content: string) => approval({ path, content }); + + // ast_edit on a filesystem path → write; on internal URLs only → read. + const astFsPath = JSON.stringify({ + ops: [{ pat: "legacyWrap($A, $B)", out: "modernWrap($A, $B)" }], + paths: [filePath], + }); + const astInternalPath = JSON.stringify({ + ops: [{ pat: "a", out: "b" }], + paths: ["artifact://abc"], + }); + expect(tier("xd://ast_edit", astFsPath)).toBe("write"); + expect(tier("xd://ast_edit", astInternalPath)).toBe("read"); + + // debug: inspection action → read; a real launch → exec (control). + expect(tier("xd://debug", JSON.stringify({ action: "sessions" }))).toBe("read"); + expect(tier("xd://debug", JSON.stringify({ action: "launch", program: "./app" }))).toBe("exec"); + + // Fail closed: malformed JSON, non-object payloads, missing content, + // and unknown devices all stay exec so the gate never under-prompts. + expect(tier("xd://ast_edit", "{ not json")).toBe("exec"); + expect(tier("xd://ast_edit", "[1,2,3]")).toBe("exec"); + expect(tier("xd://ast_edit", '"a string"')).toBe("exec"); + expect(approval({ path: "xd://ast_edit" })).toBe("exec"); + expect(tier("xd://no_such_device", "{}")).toBe("exec"); + } finally { + await removeWithRetries(tempDir); + } + }); + it("renderCall withholds a partial xd:// URL, then delegates once settled", async () => { await themeModule.initTheme(); const uiTheme = (await themeModule.getThemeByName("dark")) ?? (await themeModule.getThemeByName("light"));