diff options
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 { |