feat: tool-output truncation+spill, read_file pagination, read_file_slice, symlink-safe path resolution

3 files changed, 224 insertions, 4 deletions

diff --git a/packages/core/tests/tools/list-files.test.ts b/packages/core/tests/tools/list-files.test.ts
index ead1df3..f371717 100644
--- a/packages/core/tests/tools/list-files.test.ts
+++ b/packages/core/tests/tools/list-files.test.ts
@@ -1,4 +1,4 @@
-import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
+import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
@@ -38,4 +38,57 @@ describe("list_files tool", () => {
 		const result = await tool.execute({ path: "../" });
 		expect(result).toMatch(/outside the working directory/i);
 	});
+
+	// Regression for `resolve(join(workingDirectory, relPath))` — when relPath
+	// is absolute, `join` does NOT short-circuit. The old code silently
+	// rewrote `/some/path` to `<workdir>/some/path` and either returned an
+	// ENOENT-style error or, worse, listed an unrelated path. After the fix,
+	// absolute paths resolve to themselves and the workdir gate behaves correctly.
+	describe("absolute path handling", () => {
+		it("lists an absolute path that lives under the workdir", async () => {
+			const tool = createListFilesTool(workDir);
+			await writeFile(join(workDir, "alpha.txt"), "a");
+			await writeFile(join(workDir, "beta.txt"), "b");
+			const result = await tool.execute({ path: workDir });
+			expect(result).toContain("alpha.txt");
+			expect(result).toContain("beta.txt");
+			// "Error listing files" would indicate the path was mangled into a
+			// non-existent location.
+			expect(result).not.toMatch(/error listing/i);
+		});
+
+		it("rejects absolute paths outside the workdir with the workdir error (not a generic ENOENT)", async () => {
+			const tool = createListFilesTool(workDir);
+			// Use a tmpdir path that's definitely not under workDir. Under the
+			// bug, this got rewritten to `<workdir>/tmp/...` and produced an
+			// `Error listing files` ENOENT message instead of the workdir error.
+			const evilPath = join(tmpdir(), `dispatch-evil-${Date.now()}`);
+			const result = await tool.execute({ path: evilPath });
+			expect(result).toMatch(/outside the working directory/i);
+		});
+	});
+
+	// A directory symlink inside the workdir pointing to an external
+	// directory is the classic escape vector for a `ls` style tool.
+	// `canonicalize` must resolve the symlink so the listing is denied.
+	describe("symlink handling", () => {
+		let externalDir: string;
+
+		beforeEach(async () => {
+			externalDir = await mkdtemp(join(tmpdir(), "dispatch-external-"));
+			await writeFile(join(externalDir, "secret.txt"), "secret");
+		});
+
+		afterEach(async () => {
+			await rm(externalDir, { recursive: true, force: true });
+		});
+
+		it("blocks listing through a symlinked directory that escapes the workdir", async () => {
+			const tool = createListFilesTool(workDir);
+			await symlink(externalDir, join(workDir, "peek"));
+			const result = await tool.execute({ path: "peek" });
+			expect(result).toMatch(/outside the working directory/i);
+			expect(result).not.toContain("secret.txt");
+		});
+	});
 });
diff --git a/packages/core/tests/tools/read-file.test.ts b/packages/core/tests/tools/read-file.test.ts
index ce65b37..90165d8 100644
--- a/packages/core/tests/tools/read-file.test.ts
+++ b/packages/core/tests/tools/read-file.test.ts
@@ -1,8 +1,10 @@
-import { mkdtemp, rm, writeFile } from "node:fs/promises";
+import { randomBytes } from "node:crypto";
+import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
 import { createReadFileTool } from "../../src/tools/read-file.js";
+import { SPILL_ROOT } from "../../src/tools/truncate.js";
 
 describe("read_file tool", () => {
 	let workDir: string;
@@ -19,7 +21,8 @@ describe("read_file tool", () => {
 		const tool = createReadFileTool(workDir);
 		await writeFile(join(workDir, "hello.txt"), "Hello, world!");
 		const result = await tool.execute({ path: "hello.txt" });
-		expect(result).toBe("Hello, world!");
+		expect(result).toContain("Hello, world!");
+		expect(result).toContain("[file: hello.txt — lines 1-1 of 1]");
 	});
 
 	it("returns error for non-existent file", async () => {
@@ -33,4 +36,107 @@ describe("read_file tool", () => {
 		const result = await tool.execute({ path: "../etc/passwd" });
 		expect(result).toMatch(/outside the working directory/i);
 	});
+
+	it("respects offset and limit", async () => {
+		const tool = createReadFileTool(workDir);
+		await writeFile(join(workDir, "multi.txt"), "line1\nline2\nline3\nline4\nline5");
+		const result = await tool.execute({ path: "multi.txt", offset: 2, limit: 2 });
+		expect(result).toContain("line2");
+		expect(result).toContain("line3");
+		expect(result).not.toContain("line1");
+		expect(result).not.toContain("line4");
+		expect(result).toContain("[file: multi.txt — lines 2-3 of 5]");
+	});
+
+	it("truncates long lines and points to read_file_slice", async () => {
+		const tool = createReadFileTool(workDir);
+		const longLine = "x".repeat(3000);
+		await writeFile(join(workDir, "wide.txt"), longLine);
+		const result = await tool.execute({ path: "wide.txt" });
+		expect(result).toContain("[line 1 truncated, total 3,000 chars");
+		expect(result).toContain("use read_file_slice");
+	});
+
+	// The universal truncator writes oversized tool output to
+	// `${SPILL_ROOT}/<tabId>/<callId>.txt` and the truncation notice tells
+	// the AI to read that absolute path back. A previous implementation
+	// used `resolve(join(workingDirectory, filePath))` which silently
+	// concatenated the absolute spill path *under* the workdir, producing
+	// a non-existent path and ENOENT — breaking the entire spill-and-resume
+	// flow. These tests guard that contract.
+	describe("absolute path handling (spill-file regression)", () => {
+		let spillSubdir: string;
+
+		beforeEach(async () => {
+			spillSubdir = join(SPILL_ROOT, `test-${Date.now()}-${randomBytes(4).toString("hex")}`);
+			await mkdir(spillSubdir, { recursive: true });
+		});
+
+		afterEach(async () => {
+			await rm(spillSubdir, { recursive: true, force: true });
+		});
+
+		it("reads a spill file via its absolute path", async () => {
+			const tool = createReadFileTool(workDir);
+			const spillFile = join(spillSubdir, "call-abc.txt");
+			const payload = "spilled output line 1\nspilled output line 2";
+			await writeFile(spillFile, payload);
+
+			const result = await tool.execute({ path: spillFile });
+
+			expect(result).toContain("spilled output line 1");
+			expect(result).toContain("spilled output line 2");
+			expect(result).not.toMatch(/not found/i);
+			expect(result).not.toMatch(/outside the working directory/i);
+		});
+
+		it("still rejects absolute paths that are neither in the workdir nor the spill root", async () => {
+			const tool = createReadFileTool(workDir);
+			// Path check happens before file read, so /etc/hostname existing
+			// (or not) is irrelevant — we just need an absolute path outside
+			// both the workdir and SPILL_ROOT.
+			const result = await tool.execute({ path: "/etc/hostname" });
+			expect(result).toMatch(/outside the working directory/i);
+		});
+	});
+
+	// Symlinks must resolve consistently across the agent permission gate
+	// and the tool itself. The containment check operates on the canonical
+	// path — so a symlink-in-workdir that points outside is treated as
+	// "outside" and gated like any other external path. Lexical-only
+	// checks would let these slip through silently.
+	describe("symlink handling", () => {
+		let externalDir: string;
+
+		beforeEach(async () => {
+			externalDir = await mkdtemp(join(tmpdir(), "dispatch-external-"));
+		});
+
+		afterEach(async () => {
+			await rm(externalDir, { recursive: true, force: true });
+		});
+
+		it("follows symlinks that stay inside the workdir", async () => {
+			const tool = createReadFileTool(workDir);
+			await writeFile(join(workDir, "real.txt"), "real content");
+			await symlink(join(workDir, "real.txt"), join(workDir, "link.txt"));
+			const result = await tool.execute({ path: "link.txt" });
+			expect(result).toContain("real content");
+			expect(result).not.toMatch(/outside the working directory/i);
+		});
+
+		it("blocks symlinks that escape the workdir", async () => {
+			const tool = createReadFileTool(workDir);
+			const secret = join(externalDir, "secret.txt");
+			await writeFile(secret, "leaked secret");
+			// Create a symlink *inside* workDir pointing to a file *outside*
+			// workDir. Lexical-only path validation would see "workdir/trap.txt"
+			// (under workdir) and allow it. Canonical resolution sees the
+			// symlink's target and correctly rejects.
+			await symlink(secret, join(workDir, "trap.txt"));
+			const result = await tool.execute({ path: "trap.txt" });
+			expect(result).toMatch(/outside the working directory/i);
+			expect(result).not.toContain("leaked secret");
+		});
+	});
 });
diff --git a/packages/core/tests/tools/write-file.test.ts b/packages/core/tests/tools/write-file.test.ts
index 01d7253..6853a8e 100644
--- a/packages/core/tests/tools/write-file.test.ts
+++ b/packages/core/tests/tools/write-file.test.ts
@@ -1,4 +1,4 @@
-import { mkdtemp, readFile, rm } from "node:fs/promises";
+import { access, mkdtemp, readFile, readdir, rm, symlink } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
@@ -42,4 +42,65 @@ describe("write_file tool", () => {
 		const result = await tool.execute({ path: "../evil.txt", content: "bad" });
 		expect(result).toMatch(/outside the working directory/i);
 	});
+
+	// Regression for `resolve(join(workingDirectory, filePath))` — when filePath
+	// is absolute, `join` does NOT short-circuit, it concatenates. The old code
+	// silently rewrote `/etc/foo` to `<workdir>/etc/foo` and "succeeded" by
+	// writing to the wrong location. After the fix, absolute paths resolve
+	// to themselves and the workdir gate behaves correctly.
+	describe("absolute path handling", () => {
+		it("writes an absolute path that lives under the workdir to the expected location", async () => {
+			const tool = createWriteFileTool(workDir);
+			const absoluteTarget = join(workDir, "abs.txt");
+			const result = await tool.execute({ path: absoluteTarget, content: "abs content" });
+			expect(result).toMatch(/successfully wrote/i);
+			// File must exist at exactly `absoluteTarget`, NOT at
+			// `<workdir>/<workdir>/abs.txt` (the old mangled location).
+			const written = await readFile(absoluteTarget, "utf8");
+			expect(written).toBe("abs content");
+		});
+
+		it("rejects absolute paths outside the workdir instead of silently mangling them", async () => {
+			const tool = createWriteFileTool(workDir);
+			// Pick a path under tmpdir that's definitely not under workDir.
+			// Under the bug, this got rewritten to `<workdir>/tmp/...` and the
+			// write "succeeded" at the wrong location.
+			const evilPath = join(tmpdir(), `dispatch-evil-${Date.now()}.txt`);
+			const result = await tool.execute({ path: evilPath, content: "should not land" });
+			expect(result).toMatch(/outside the working directory/i);
+		});
+	});
+
+	// Symlink containment: even when the *leaf* doesn't exist yet (the
+	// common case for write_file creating a new file), `canonicalize`
+	// must walk up to the nearest existing ancestor and resolve symlinks
+	// there. Otherwise, a directory symlink inside workdir pointing
+	// outside lets a write escape the workspace.
+	describe("symlink handling", () => {
+		let externalDir: string;
+
+		beforeEach(async () => {
+			externalDir = await mkdtemp(join(tmpdir(), "dispatch-external-"));
+		});
+
+		afterEach(async () => {
+			await rm(externalDir, { recursive: true, force: true });
+		});
+
+		it("blocks writes that escape through a parent symlink (leaf does not exist yet)", async () => {
+			const tool = createWriteFileTool(workDir);
+			// `escape` is a symlink *inside* workdir to a directory *outside*.
+			await symlink(externalDir, join(workDir, "escape"));
+			const result = await tool.execute({
+				path: "escape/payload.txt",
+				content: "malicious payload",
+			});
+			expect(result).toMatch(/outside the working directory/i);
+			// And the file must NOT exist in externalDir.
+			await expect(access(join(externalDir, "payload.txt"))).rejects.toThrow();
+			// And externalDir should be empty (nothing leaked through).
+			const entries = await readdir(externalDir);
+			expect(entries).toEqual([]);
+		});
+	});
 });