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 | |
| 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.
| -rw-r--r-- | packages/tool-edit-file/src/edit-file.test.ts | 68 | ||||
| -rw-r--r-- | packages/tool-edit-file/src/edit-file.ts | 47 | ||||
| -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 | ||||
| -rw-r--r-- | packages/tool-write-file/src/index.ts | 1 | ||||
| -rw-r--r-- | packages/tool-write-file/src/write-file.test.ts | 60 | ||||
| -rw-r--r-- | packages/tool-write-file/src/write-file.ts | 68 |
7 files changed, 9 insertions, 380 deletions
diff --git a/packages/tool-edit-file/src/edit-file.test.ts b/packages/tool-edit-file/src/edit-file.test.ts index dba3d9e..5ef8376 100644 --- a/packages/tool-edit-file/src/edit-file.test.ts +++ b/packages/tool-edit-file/src/edit-file.test.ts @@ -3,12 +3,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { createLogger, type ToolExecuteContext } from "@dispatch/kernel"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { - computeReplacement, - createEditFileTool, - isPathWithinWorkdir, - validateArgs, -} from "./edit-file.js"; +import { computeReplacement, createEditFileTool, validateArgs } from "./edit-file.js"; function stubCtx(overrides?: Partial<ToolExecuteContext>): ToolExecuteContext { return { @@ -145,24 +140,6 @@ describe("computeReplacement", () => { }); }); -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("createEditFileTool", () => { it("replaces a single occurrence", async () => { const filePath = join(workdir, "test.txt"); @@ -251,49 +228,6 @@ describe("createEditFileTool", () => { expect(result.content).toContain("not found"); }); - it("rejects a path outside the working directory", async () => { - const tool = createEditFileTool(workdir); - const result = await tool.execute( - { path: "../escape.txt", oldString: "a", newString: "b" }, - stubCtx(), - ); - - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); - }); - - it("rejects an absolute path outside workdir", async () => { - const tool = createEditFileTool(workdir); - const result = await tool.execute( - { path: "/etc/passwd", oldString: "a", newString: "b" }, - stubCtx(), - ); - - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); - }); - - it("handles symlink escape attempt", async () => { - 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 = createEditFileTool(workdir); - const result = await tool.execute( - { path: "link.txt", oldString: "secret", newString: "leaked" }, - stubCtx(), - ); - - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); - - await rm(outsideDir, { recursive: true, force: true }); - }); - it("reads file under ctx.cwd when set", async () => { const ctxDir = await mkdtemp(join(tmpdir(), "ctx-cwd-test-")); try { diff --git a/packages/tool-edit-file/src/edit-file.ts b/packages/tool-edit-file/src/edit-file.ts index af630aa..36f99e0 100644 --- a/packages/tool-edit-file/src/edit-file.ts +++ b/packages/tool-edit-file/src/edit-file.ts @@ -1,5 +1,5 @@ -import { readFile, realpath, writeFile } from "node:fs/promises"; -import { resolve, sep } from "node:path"; +import { readFile, writeFile } from "node:fs/promises"; +import { resolve } from "node:path"; import type { ToolContract, ToolResult } from "@dispatch/kernel"; // --- Pure types --- @@ -103,12 +103,6 @@ export function computeReplacement( }; } -/** 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); -} - // --- Shell / edge --- /** @@ -156,46 +150,9 @@ export function createEditFileTool(workingDirectory: string): ToolContract { const { path: relPath, oldString, newString, replaceAll } = 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. - 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 accessing 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, - }; - } - // Read the file. let content: string; try { 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 { diff --git a/packages/tool-write-file/src/index.ts b/packages/tool-write-file/src/index.ts index 521a429..8a2cb36 100644 --- a/packages/tool-write-file/src/index.ts +++ b/packages/tool-write-file/src/index.ts @@ -2,6 +2,5 @@ export { extension } from "./extension.js"; export { createWriteFileTool, decideOverwrite, - isPathWithinWorkdir, validateArgs, } from "./write-file.js"; diff --git a/packages/tool-write-file/src/write-file.test.ts b/packages/tool-write-file/src/write-file.test.ts index cf4fa64..6b316bc 100644 --- a/packages/tool-write-file/src/write-file.test.ts +++ b/packages/tool-write-file/src/write-file.test.ts @@ -3,12 +3,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { createLogger, type ToolExecuteContext } from "@dispatch/kernel"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { - createWriteFileTool, - decideOverwrite, - isPathWithinWorkdir, - validateArgs, -} from "./write-file.js"; +import { createWriteFileTool, decideOverwrite, validateArgs } from "./write-file.js"; function stubCtx(overrides?: Partial<ToolExecuteContext>): ToolExecuteContext { return { @@ -75,24 +70,6 @@ describe("decideOverwrite", () => { }); }); -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("validateArgs", () => { it("returns validated args for valid input", () => { const result = validateArgs({ path: "foo.txt", content: "hello" }); @@ -192,23 +169,7 @@ describe("createWriteFileTool", () => { const result = await tool.execute({ path: "no/such/dir/file.txt", content: "data" }, stubCtx()); expect(result.isError).toBe(true); - expect(result.content).toContain("Parent directory"); - }); - - it("rejects a path outside the working directory", async () => { - const tool = createWriteFileTool(workdir); - const result = await tool.execute({ path: "../escape.txt", content: "data" }, stubCtx()); - - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); - }); - - it("rejects an absolute path outside workdir", async () => { - const tool = createWriteFileTool(workdir); - const result = await tool.execute({ path: "/tmp/escape.txt", content: "data" }, stubCtx()); - - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); + expect(result.content).toContain("Error"); }); it("concurrencySafe is false", () => { @@ -253,23 +214,6 @@ describe("createWriteFileTool", () => { } }); - it("handles symlink escape attempt", async () => { - const outsideDir = await mkdtemp(join(tmpdir(), "outside-")); - try { - const symlinkPath = join(workdir, "link.txt"); - const { symlink } = await import("node:fs/promises"); - await symlink(join(outsideDir, "target.txt"), symlinkPath); - - const tool = createWriteFileTool(workdir); - const result = await tool.execute({ path: "link.txt", content: "escape" }, stubCtx()); - - expect(result.isError).toBe(true); - expect(result.content).toContain("outside the working directory"); - } finally { - await rm(outsideDir, { recursive: true, force: true }); - } - }); - it("writes empty content", async () => { const tool = createWriteFileTool(workdir); const result = await tool.execute({ path: "empty.txt", content: "" }, stubCtx()); diff --git a/packages/tool-write-file/src/write-file.ts b/packages/tool-write-file/src/write-file.ts index 16cfc83..1317ce8 100644 --- a/packages/tool-write-file/src/write-file.ts +++ b/packages/tool-write-file/src/write-file.ts @@ -1,5 +1,5 @@ -import { access, lstat, readlink, realpath, stat, writeFile } from "node:fs/promises"; -import { dirname, resolve, sep } from "node:path"; +import { access, stat, writeFile } from "node:fs/promises"; +import { resolve } from "node:path"; import type { ToolContract, ToolResult } from "@dispatch/kernel"; interface ValidatedArgs { @@ -20,12 +20,6 @@ export function decideOverwrite(fileExists: boolean, overwrite: boolean): Overwr return { error: "Error: overwrite: true but the file does not exist." }; } -/** 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: validate and coerce args from the model. */ export function validateArgs(args: unknown): ValidatedArgs | { readonly error: string } { if (args === null || args === undefined || typeof args !== "object") { @@ -103,64 +97,6 @@ export function createWriteFileTool(workingDirectory: string): ToolContract { const effectiveBase = ctx.cwd ? resolve(ctx.cwd) : workdir; const resolvedPath = resolve(effectiveBase, relPath); - if (!isPathWithinWorkdir(resolvedPath, effectiveBase)) { - return { - content: `Error: Path "${relPath}" is outside the working directory.`, - isError: true, - }; - } - - // Symlink hardening: realpath the parent directory and the base, then re-check. - let realParent: string; - let realBase: string; - try { - const parentDir = dirname(resolvedPath); - [realParent, realBase] = await Promise.all([realpath(parentDir), realpath(effectiveBase)]); - } catch (err: unknown) { - const code = (err as NodeJS.ErrnoException).code; - if (code === "ENOENT") { - return { - content: `Error: Parent directory for "${relPath}" does not exist.`, - isError: true, - }; - } - return { - content: `Error resolving path: ${err instanceof Error ? err.message : String(err)}`, - isError: true, - }; - } - - const realResolvedPath = realParent + sep + resolvedPath.split(sep).at(-1); - if (!isPathWithinWorkdir(realResolvedPath, realBase)) { - return { - content: `Error: Path "${relPath}" is outside the working directory.`, - isError: true, - }; - } - - // If the resolved path itself is a symlink, verify the target is contained. - try { - const linkStat = await lstat(resolvedPath); - if (linkStat.isSymbolicLink()) { - const linkTarget = await readlink(resolvedPath); - const resolvedTarget = resolve(dirname(resolvedPath), linkTarget); - if (!isPathWithinWorkdir(resolvedTarget, realBase)) { - return { - content: `Error: Path "${relPath}" is outside the working directory.`, - isError: true, - }; - } - } - } catch (err: unknown) { - const code = (err as NodeJS.ErrnoException).code; - if (code !== "ENOENT") { - return { - content: `Error checking path: ${err instanceof Error ? err.message : String(err)}`, - isError: true, - }; - } - } - // Check existence. let fileExists = false; try { |
