From 4ac322db9309c827e685a618cd04158f441231cb Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 27 Jul 2026 14:20:33 +0000 Subject: [PATCH] fix(tools): refuse write targets shaped as a read-selector list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The read-selector-misfire guard (#6123/#6387) short-circuited whenever `content` was non-empty, so a semicolon-joined list of read selectors (`a.txt:1-2;b/c.txt:3-4`) passed as a write path with content fell through to ordinary filesystem creation and silently built a nested directory tree in the workspace. `read` accepts no such list, so this shape is always a mis-dispatched multi-file read. Refuse any target that splits on `;` into 2+ segments each carrying its own read selector, regardless of `content` — the non-empty-content escape hatch covers a lone selector-shaped filename, never a `;`-list. Fixes #6809 --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/src/tools/write.ts | 31 +++++++++++++++++++ .../test/write-read-selector-misfire.test.ts | 23 ++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a59a4e7c1..41aed0372 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -11,6 +11,7 @@ - Fixed Python cell errors (`$` commands and the eval tool) leaking runner-internal traceback frames. Cell syntax errors now render as the bare caret display with a `` filename instead of a `_handle_request_async`/`ast.parse` stack dump, and runtime tracebacks start at user code, matching the Ruby runner's user-frame filtering. - Dropped unavailable forced tool choices through the queue rejection lifecycle and discarded their remaining sequence yields so a skipped force cannot disable tools on the next request ([#6543](https://github.com/can1357/oh-my-pi/pull/6543) by [@paralin](https://github.com/paralin)). +- Fixed the `write` tool treating a semicolon-joined list of read selectors (e.g. `a.txt:1-2;b/c.txt:3-4`) as a filesystem path, silently creating a nested directory tree when a read-only step mis-dispatched a multi-file `read` as `write`. Such targets are now refused regardless of `content`, since no real write targets a `;`-list whose every segment carries a read selector ([#6809](https://github.com/can1357/oh-my-pi/issues/6809)). ## [17.1.5] - 2026-07-27 diff --git a/packages/coding-agent/src/tools/write.ts b/packages/coding-agent/src/tools/write.ts index 639fc87cf..a2d8a3ce9 100644 --- a/packages/coding-agent/src/tools/write.ts +++ b/packages/coding-agent/src/tools/write.ts @@ -153,7 +153,38 @@ function throwReadSelectorMisfire(target: string, sel: string): never { ); } +/** + * Recognize a semicolon-joined list of read-tool selectors mis-dispatched as a + * single write target — the multi-file read expression the scout emitted in + * issue #6809 (`a.txt:1-2;b/c.txt:3-4`). Every `;`-segment must be non-empty and + * carry its own read selector ({@link splitPathAndSel} peels a `:N-M`, `:raw`, + * or `:conflicts` tail). No real call targets such a list: `read` accepts one + * path, `write` writes one file. Unlike {@link readSelectorForEmptyWrite} this + * fires regardless of `content` — the non-empty-content escape hatch exists for + * a lone selector-shaped *filename*, never a `;`-list, and honoring it here + * silently creates a nested directory tree (`a.txt:1-2;b/`) in the workspace. + */ +function readSelectorListMisfire(target: string): number | undefined { + if (!target.includes(";")) return undefined; + const segments = target.split(";"); + if (segments.length < 2) return undefined; + for (const segment of segments) { + const trimmed = segment.trim(); + if (trimmed.length === 0 || splitPathAndSel(trimmed).sel === undefined) return undefined; + } + return segments.length; +} + +function throwReadSelectorListMisfire(target: string, count: number): never { + throw new ToolError( + `write target '${target}' is a semicolon-joined list of ${count} read-tool selectors, not a filesystem path — refusing to create it. ` + + `write creates a single file; issue one read() per path to read these ranges (e.g. read({ path: ":" })).`, + ); +} + async function assertNotReadSelectorMisfire(target: string, content: string, cwd: string): Promise { + const listCount = readSelectorListMisfire(target); + if (listCount !== undefined) throwReadSelectorListMisfire(target, listCount); const sel = readSelectorForEmptyWrite(target, content); if (sel === undefined) return; if ((await probeLiteralPathExists(target, cwd)) !== "missing") return; diff --git a/packages/coding-agent/test/write-read-selector-misfire.test.ts b/packages/coding-agent/test/write-read-selector-misfire.test.ts index 917ba7482..ea5192844 100644 --- a/packages/coding-agent/test/write-read-selector-misfire.test.ts +++ b/packages/coding-agent/test/write-read-selector-misfire.test.ts @@ -103,4 +103,27 @@ describe("write refuses read-selector misfires", () => { expect(entries.get(member)).toEqual(new Uint8Array()); await fs.rm(dir, { recursive: true, force: true }); }); + + it("rejects a semicolon-joined selector list with non-empty content and creates nothing", async () => { + const dir = await makeWorkspace(); + const write = new WriteTool(session(dir)); + const target = "a.txt:1-2;b/c.txt:3-4"; + await expect(write.execute("c", { path: target, content: "{}" })).rejects.toThrow( + /semicolon-joined list of 2 read-tool selectors/, + ); + expect(await Bun.file(path.join(dir, target)).exists()).toBe(false); + expect(await Bun.file(path.join(dir, "a.txt:1-2;b/c.txt:3-4")).exists()).toBe(false); + expect(await fs.readdir(dir)).toEqual(["src"]); + await fs.rm(dir, { recursive: true, force: true }); + }); + + it("still writes a real path that merely contains a semicolon (no per-segment selectors)", async () => { + const dir = await makeWorkspace(); + const write = new WriteTool(session(dir)); + const target = "notes;draft.txt"; + const res = await write.execute("c", { path: target, content: "hi" }); + expect(res.isError).toBeUndefined(); + expect(await Bun.file(path.join(dir, target)).text()).toBe("hi"); + await fs.rm(dir, { recursive: true, force: true }); + }); });