summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--packages/tool-edit-file/src/edit-file.test.ts68
-rw-r--r--packages/tool-edit-file/src/edit-file.ts47
-rw-r--r--packages/tool-read-file/src/read-file.test.ts98
-rw-r--r--packages/tool-read-file/src/read-file.ts47
-rw-r--r--packages/tool-write-file/src/index.ts1
-rw-r--r--packages/tool-write-file/src/write-file.test.ts60
-rw-r--r--packages/tool-write-file/src/write-file.ts68
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 {