diff options
| author | Adam Malczewski <[email protected]> | 2026-06-21 23:32:23 +0900 |
|---|---|---|
| committer | Adam Malczewski <[email protected]> | 2026-06-21 23:32:23 +0900 |
| commit | 7f6fd218ceeb3a1cf9420f5f6cfa4d70da6987bb (patch) | |
| tree | e706f49d2df591c468fb66febe2a3f29d23cb0bb /packages/tool-read-file/src | |
| parent | 62ea07f56ff066bbf05041aadf8006cbc65e5c53 (diff) | |
| download | dispatch-7f6fd218ceeb3a1cf9420f5f6cfa4d70da6987bb.tar.gz dispatch-7f6fd218ceeb3a1cf9420f5f6cfa4d70da6987bb.zip | |
feat: remove CWD path containment from file tools
read_file, write_file, and edit_file no longer restrict access to
paths outside the working directory. The isPathWithinWorkdir prefix
check and symlink hardening have been removed from all three tools.
This allows agents to read and write files anywhere on the filesystem,
not just within the per-turn cwd. The shell tool already had no such
restriction.
Diffstat (limited to 'packages/tool-read-file/src')
| -rw-r--r-- | packages/tool-read-file/src/read-file.test.ts | 98 | ||||
| -rw-r--r-- | packages/tool-read-file/src/read-file.ts | 47 |
2 files changed, 2 insertions, 143 deletions
diff --git a/packages/tool-read-file/src/read-file.test.ts b/packages/tool-read-file/src/read-file.test.ts index 25b29ff..619ba34 100644 --- a/packages/tool-read-file/src/read-file.test.ts +++ b/packages/tool-read-file/src/read-file.test.ts @@ -6,7 +6,6 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { createReadFileTool, formatDirectoryEntries, - isPathWithinWorkdir, renderLines, sliceLines, validateArgs, @@ -108,24 +107,6 @@ describe("sliceLines", () => { }); }); -describe("isPathWithinWorkdir", () => { - it("accepts a path within workdir", () => { - expect(isPathWithinWorkdir("/tmp/workdir/file.txt", "/tmp/workdir")).toBe(true); - }); - - it("accepts the workdir itself", () => { - expect(isPathWithinWorkdir("/tmp/workdir", "/tmp/workdir")).toBe(true); - }); - - it("rejects a path outside workdir", () => { - expect(isPathWithinWorkdir("/tmp/other/file.txt", "/tmp/workdir")).toBe(false); - }); - - it("rejects a prefix attack (workdir prefix but different dir)", () => { - expect(isPathWithinWorkdir("/tmp/workdir-evil/file.txt", "/tmp/workdir")).toBe(false); - }); -}); - describe("renderLines", () => { it("renders lines with 1-indexed line numbers", () => { const result = renderLines(["a", "b", "c"], 1); @@ -197,22 +178,6 @@ describe("createReadFileTool", () => { expect(result.content).toContain("not found"); }); - it("returns error for path escape via ..", async () => { - const tool = createReadFileTool(workdir); - const result = await tool.execute({ path: "../escape.txt" }, stubCtx()); - - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); - }); - - it("returns error for absolute path outside workdir", async () => { - const tool = createReadFileTool(workdir); - const result = await tool.execute({ path: "/etc/passwd" }, stubCtx()); - - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); - }); - it("returns empty-file content for empty file", async () => { const filePath = join(workdir, "empty.txt"); await writeFile(filePath, "", "utf8"); @@ -247,26 +212,6 @@ describe("createReadFileTool", () => { } }); - it("handles symlink escape attempt", async () => { - // Create a symlink inside workdir pointing outside - const outsideDir = await mkdtemp(join(tmpdir(), "outside-")); - const outsideFile = join(outsideDir, "secret.txt"); - await writeFile(outsideFile, "secret data", "utf8"); - - const symlinkPath = join(workdir, "link.txt"); - const { symlink } = await import("node:fs/promises"); - await symlink(outsideFile, symlinkPath); - - const tool = createReadFileTool(workdir); - const result = await tool.execute({ path: "link.txt" }, stubCtx()); - - // The symlink resolves to outside workdir, so should be rejected - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); - - await rm(outsideDir, { recursive: true, force: true }); - }); - it("concurrencySafe is true", () => { const tool = createReadFileTool(workdir); expect(tool.concurrencySafe).toBe(true); @@ -296,41 +241,6 @@ describe("createReadFileTool", () => { } }); - it("rejects path escaping ctx.cwd via ..", async () => { - const ctxDir = await mkdtemp(join(tmpdir(), "ctx-escape-test-")); - try { - const tool = createReadFileTool(workdir); - const result = await tool.execute({ path: "../escape.txt" }, stubCtx({ cwd: ctxDir })); - - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); - } finally { - await rm(ctxDir, { recursive: true, force: true }); - } - }); - - it("rejects symlink escaping ctx.cwd", async () => { - const ctxDir = await mkdtemp(join(tmpdir(), "ctx-symlink-test-")); - const outsideDir = await mkdtemp(join(tmpdir(), "ctx-outside-")); - try { - const outsideFile = join(outsideDir, "secret.txt"); - await writeFile(outsideFile, "secret data", "utf8"); - - const symlinkPath = join(ctxDir, "link.txt"); - const { symlink } = await import("node:fs/promises"); - await symlink(outsideFile, symlinkPath); - - const tool = createReadFileTool(workdir); - const result = await tool.execute({ path: "link.txt" }, stubCtx({ cwd: ctxDir })); - - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); - } finally { - await rm(ctxDir, { recursive: true, force: true }); - await rm(outsideDir, { recursive: true, force: true }); - } - }); - it("falls back to baked workdir when ctx.cwd is omitted", async () => { const filePath = join(workdir, "baked-file.txt"); await writeFile(filePath, "from baked workdir", "utf8"); @@ -377,14 +287,6 @@ describe("createReadFileTool", () => { expect(result.content).toBe("2: b\n3: c\n4: d"); }); - it("rejects a directory path outside the working directory (containment still enforced)", async () => { - const tool = createReadFileTool(workdir); - const result = await tool.execute({ path: "../outside-dir" }, stubCtx()); - - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); - }); - it("returns not-found for a nonexistent path", async () => { const tool = createReadFileTool(workdir); const result = await tool.execute({ path: "nonexistent-path" }, stubCtx()); diff --git a/packages/tool-read-file/src/read-file.ts b/packages/tool-read-file/src/read-file.ts index 99b396e..216f165 100644 --- a/packages/tool-read-file/src/read-file.ts +++ b/packages/tool-read-file/src/read-file.ts @@ -1,5 +1,5 @@ -import { readdir, readFile, realpath, stat } from "node:fs/promises"; -import { resolve, sep } from "node:path"; +import { readdir, readFile, stat } from "node:fs/promises"; +import { resolve } from "node:path"; import type { ToolContract, ToolResult } from "@dispatch/kernel"; const DEFAULT_LIMIT = 500; @@ -59,12 +59,6 @@ export function sliceLines( return { lines: sliced, totalLines }; } -/** Pure: check that a resolved absolute path is within the workdir (prefix check). */ -export function isPathWithinWorkdir(resolvedPath: string, workdir: string): boolean { - const normalizedWorkdir = workdir.endsWith(sep) ? workdir : workdir + sep; - return resolvedPath === workdir || resolvedPath.startsWith(normalizedWorkdir); -} - /** Pure: render lines into a string with line numbers. */ export function renderLines(lines: readonly string[], offset: number): string { return lines.map((line, i) => `${offset + i}: ${line}`).join("\n"); @@ -131,46 +125,9 @@ export function createReadFileTool(workingDirectory: string): ToolContract { const { path: relPath, offset, limit } = validated; - // Effective base: per-turn ctx.cwd overrides the baked workdir. const effectiveBase = ctx.cwd ? resolve(ctx.cwd) : workdir; - - // Resolve the requested path against the effective base. const resolvedPath = resolve(effectiveBase, relPath); - // Basic prefix check (catches ".." and absolute paths outside effectiveBase). - if (!isPathWithinWorkdir(resolvedPath, effectiveBase)) { - return { - content: `Error: Path "${relPath}" is outside the working directory.`, - isError: true, - }; - } - - // Symlink hardening: realpath both and re-check containment. - let realResolved: string; - let realBase: string; - try { - [realResolved, realBase] = await Promise.all([ - realpath(resolvedPath), - realpath(effectiveBase), - ]); - } catch (err: unknown) { - const code = (err as NodeJS.ErrnoException).code; - if (code === "ENOENT") { - return { content: `Error: File "${relPath}" not found.`, isError: true }; - } - return { - content: `Error reading file: ${err instanceof Error ? err.message : String(err)}`, - isError: true, - }; - } - - if (!isPathWithinWorkdir(realResolved, realBase)) { - return { - content: `Error: Path "${relPath}" is outside the working directory.`, - isError: true, - }; - } - // Stat to determine if this is a file or directory. let pathStat: import("node:fs").Stats; try { |
