fix(tools): enforce workspace path containment

Amolith created

Address review issues #1 and #4:

Issue #1 - Path traversal in tools:
- Add ensureWorkspacePath() call after path resolution in read, grep,
  ls,
and find tools. Previously, paths were resolved but never checked
against workspace boundaries, allowing traversal via ../ or absolute
paths.
- Remove tilde (~) expansion from expandPath(). In a workspace-sandboxed
context, expanding ~ to the user's home directory bypasses containment.
Tildes are now treated as literal path characters.
- Move ensureWorkspacePath() from index.ts to path-utils.ts to avoid
circular imports, re-export from index.ts for backward compatibility.

Issue #4 - writeWorkspaceFile lacks traversal protection:
- Add ensureContained() validation in writeWorkspaceFile() that checks
  the
resolved file path stays within the workspace boundary before writing.

Also:
- Fix tsconfig.json rootDir from 'src' to '.' so test/ files are
  included
in type checking (the include array already listed 'test').
- Add comprehensive test suite (28 tests) covering workspace containment
for all affected modules.

Change summary

src/agent/tools/find.ts            |   3 
src/agent/tools/grep.ts            |   3 
src/agent/tools/index.ts           |  14 -
src/agent/tools/ls.ts              |   3 
src/agent/tools/path-utils.ts      |  25 ++-
src/agent/tools/read.ts            |   3 
src/workspace/content.ts           |  13 +
test/workspace-containment.test.ts | 237 ++++++++++++++++++++++++++++++++
tsconfig.json                      |   2 
9 files changed, 276 insertions(+), 27 deletions(-)

Detailed changes

src/agent/tools/find.ts πŸ”—

@@ -2,7 +2,7 @@ import { spawnSync } from "node:child_process";
 import { relative } from "node:path";
 import { Type } from "@sinclair/typebox";
 import type { AgentTool } from "@mariozechner/pi-ai";
-import { resolveToCwd } from "./path-utils.js";
+import { resolveToCwd, ensureWorkspacePath } from "./path-utils.js";
 import { DEFAULT_MAX_BYTES, formatSize, truncateHead } from "../../util/truncate.js";
 import { ToolInputError } from "../../util/errors.js";
 
@@ -28,6 +28,7 @@ export const createFindTool = (workspacePath: string): AgentTool => {
       const searchDir: string = params.path || ".";
       const effectiveLimit = params.limit ?? DEFAULT_LIMIT;
       const searchPath = resolveToCwd(searchDir, workspacePath);
+      ensureWorkspacePath(workspacePath, searchPath);
 
     const args = [
       "--glob",

src/agent/tools/grep.ts πŸ”—

@@ -4,7 +4,7 @@ import { readFileSync, statSync } from "node:fs";
 import { relative, basename } from "node:path";
 import { Type } from "@sinclair/typebox";
 import type { AgentTool } from "@mariozechner/pi-ai";
-import { resolveToCwd } from "./path-utils.js";
+import { resolveToCwd, ensureWorkspacePath } from "./path-utils.js";
 import {
   DEFAULT_MAX_BYTES,
   formatSize,
@@ -43,6 +43,7 @@ export const createGrepTool = (workspacePath: string): AgentTool => {
     execute: async (_toolCallId: string, params: any) => {
       const searchDir: string | undefined = params.path;
       const searchPath = resolveToCwd(searchDir || ".", workspacePath);
+      ensureWorkspacePath(workspacePath, searchPath);
       let isDirectory = false;
       try {
         isDirectory = statSync(searchPath).isDirectory();

src/agent/tools/index.ts πŸ”—

@@ -1,6 +1,4 @@
-import { resolve, sep } from "node:path";
 import type { AgentTool } from "@mariozechner/pi-ai";
-import { ToolInputError } from "../../util/errors.js";
 
 export type ToolFactory = (workspacePath: string) => AgentTool<any>;
 
@@ -9,17 +7,7 @@ export interface ToolBundle {
   tools: AgentTool<any>[];
 }
 
-export function ensureWorkspacePath(workspacePath: string, targetPath: string): string {
-  const resolved = resolve(workspacePath, targetPath);
-  const root = workspacePath.endsWith(sep) ? workspacePath : `${workspacePath}${sep}`;
-
-  if (resolved === workspacePath || resolved.startsWith(root)) {
-    return resolved;
-  }
-
-  throw new ToolInputError(`Path escapes workspace: ${targetPath}`);
-}
-
+export { ensureWorkspacePath } from "./path-utils.js";
 export { createReadTool } from "./read.js";
 export { createGrepTool } from "./grep.js";
 export { createLsTool } from "./ls.js";

src/agent/tools/ls.ts πŸ”—

@@ -2,7 +2,7 @@ import { existsSync, readdirSync, statSync } from "node:fs";
 import { join } from "node:path";
 import { Type } from "@sinclair/typebox";
 import type { AgentTool } from "@mariozechner/pi-ai";
-import { resolveToCwd } from "./path-utils.js";
+import { resolveToCwd, ensureWorkspacePath } from "./path-utils.js";
 import { DEFAULT_MAX_BYTES, formatSize, truncateHead } from "../../util/truncate.js";
 
 const DEFAULT_LIMIT = 500;
@@ -19,6 +19,7 @@ export const createLsTool = (workspacePath: string): AgentTool => ({
   parameters: LsSchema as any,
   execute: async (_toolCallId: string, params: any) => {
     const resolved = resolveToCwd(params.path || ".", workspacePath);
+    ensureWorkspacePath(workspacePath, resolved);
 
     if (!existsSync(resolved)) {
       throw new Error(`Path does not exist: ${params.path || "."}`);

src/agent/tools/path-utils.ts πŸ”—

@@ -1,6 +1,6 @@
 import { accessSync, constants } from "node:fs";
-import * as os from "node:os";
-import { isAbsolute, resolve as resolvePath } from "node:path";
+import { isAbsolute, resolve as resolvePath, sep } from "node:path";
+import { ToolInputError } from "../../util/errors.js";
 
 const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g;
 
@@ -33,12 +33,10 @@ export function expandPath(filePath: string): string {
 	let result = filePath.replace(UNICODE_SPACES, " ");
 	result = normalizeAtPrefix(result);
 
-	if (result === "~") {
-		return os.homedir();
-	}
-	if (result.startsWith("~/")) {
-		return resolvePath(os.homedir(), result.slice(2));
-	}
+	// NOTE: tilde expansion is intentionally omitted.
+	// In a workspace-sandboxed context, expanding ~ to the user's home
+	// directory would bypass workspace containment. Tildes are treated
+	// as literal path characters.
 
 	return result;
 }
@@ -68,3 +66,14 @@ export function resolveReadPath(filePath: string, cwd: string): string {
 
 	return resolved;
 }
+
+export function ensureWorkspacePath(workspacePath: string, targetPath: string): string {
+	const resolved = resolvePath(workspacePath, targetPath);
+	const root = workspacePath.endsWith(sep) ? workspacePath : `${workspacePath}${sep}`;
+
+	if (resolved === workspacePath || resolved.startsWith(root)) {
+		return resolved;
+	}
+
+	throw new ToolInputError(`Path escapes workspace: ${targetPath}`);
+}

src/agent/tools/read.ts πŸ”—

@@ -1,7 +1,7 @@
 import { readFile, stat } from "node:fs/promises";
 import { Type } from "@sinclair/typebox";
 import type { AgentTool } from "@mariozechner/pi-ai";
-import { resolveReadPath } from "./path-utils.js";
+import { resolveReadPath, ensureWorkspacePath } from "./path-utils.js";
 import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES, formatSize, truncateHead } from "../../util/truncate.js";
 import { ToolInputError } from "../../util/errors.js";
 
@@ -20,6 +20,7 @@ export const createReadTool = (workspacePath: string): AgentTool => ({
 	parameters: ReadSchema as any,
 	execute: async (_toolCallId: string, params: any) => {
 		const absolutePath = resolveReadPath(params.path, workspacePath);
+		ensureWorkspacePath(workspacePath, absolutePath);
 		const fileStats = await stat(absolutePath);
 
 		if (fileStats.size > MAX_READ_BYTES) {

src/workspace/content.ts πŸ”—

@@ -1,5 +1,6 @@
 import { mkdir, writeFile } from "node:fs/promises";
-import { dirname, join } from "node:path";
+import { dirname, join, resolve, sep } from "node:path";
+import { ToolInputError } from "../util/errors.js";
 
 export interface WorkspaceContent {
   filePath: string;
@@ -7,12 +8,22 @@ export interface WorkspaceContent {
   bytes: number;
 }
 
+function ensureContained(workspacePath: string, targetPath: string): void {
+  const resolved = resolve(workspacePath, targetPath);
+  const root = workspacePath.endsWith(sep) ? workspacePath : `${workspacePath}${sep}`;
+
+  if (resolved !== workspacePath && !resolved.startsWith(root)) {
+    throw new ToolInputError(`Path escapes workspace: ${targetPath}`);
+  }
+}
+
 export async function writeWorkspaceFile(
   workspacePath: string,
   relativePath: string,
   content: string,
 ): Promise<WorkspaceContent> {
   const filePath = join(workspacePath, relativePath);
+  ensureContained(workspacePath, filePath);
   await mkdir(dirname(filePath), { recursive: true });
   await writeFile(filePath, content, "utf8");
 

test/workspace-containment.test.ts πŸ”—

@@ -0,0 +1,237 @@
+import { describe, test, expect, beforeAll, afterAll } from "bun:test";
+import { mkdtemp, rm, writeFile, mkdir } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join, resolve } from "node:path";
+import { ensureWorkspacePath } from "../src/agent/tools/index.js";
+import { expandPath, resolveToCwd, resolveReadPath } from "../src/agent/tools/path-utils.js";
+import { writeWorkspaceFile } from "../src/workspace/content.js";
+
+let workspace: string;
+
+beforeAll(async () => {
+	workspace = await mkdtemp(join(tmpdir(), "rumilo-test-"));
+	await mkdir(join(workspace, "subdir"), { recursive: true });
+	await writeFile(join(workspace, "hello.txt"), "hello");
+	await writeFile(join(workspace, "subdir", "nested.txt"), "nested");
+});
+
+afterAll(async () => {
+	await rm(workspace, { recursive: true, force: true });
+});
+
+// ─── ensureWorkspacePath ────────────────────────────────────────────
+
+describe("ensureWorkspacePath", () => {
+	test("allows workspace root itself", () => {
+		const result = ensureWorkspacePath(workspace, ".");
+		expect(result).toBe(workspace);
+	});
+
+	test("allows a relative child path", () => {
+		const result = ensureWorkspacePath(workspace, "hello.txt");
+		expect(result).toBe(join(workspace, "hello.txt"));
+	});
+
+	test("allows nested relative path", () => {
+		const result = ensureWorkspacePath(workspace, "subdir/nested.txt");
+		expect(result).toBe(join(workspace, "subdir", "nested.txt"));
+	});
+
+	test("rejects .. traversal escaping workspace", () => {
+		expect(() => ensureWorkspacePath(workspace, "../../../etc/passwd")).toThrow("Path escapes workspace");
+	});
+
+	test("rejects absolute path outside workspace", () => {
+		expect(() => ensureWorkspacePath(workspace, "/etc/passwd")).toThrow("Path escapes workspace");
+	});
+
+	test("allows absolute path inside workspace", () => {
+		const absInside = join(workspace, "hello.txt");
+		const result = ensureWorkspacePath(workspace, absInside);
+		expect(result).toBe(absInside);
+	});
+});
+
+// ─── expandPath: tilde must NOT escape workspace ────────────────────
+
+describe("expandPath - tilde handling for workspace sandboxing", () => {
+	test("tilde alone must not expand to homedir", () => {
+		const result = expandPath("~");
+		// After fix, ~ should remain literal (not expand to homedir)
+		expect(result).toBe("~");
+	});
+
+	test("tilde-prefixed path must not expand to homedir", () => {
+		const result = expandPath("~/secret");
+		expect(result).not.toContain("/home");
+		expect(result).not.toContain("/Users");
+		// Should stay as literal path
+		expect(result).toBe("~/secret");
+	});
+});
+
+// ─── resolveToCwd: must stay within workspace ───────────────────────
+
+describe("resolveToCwd - workspace containment", () => {
+	test("resolves relative path within workspace", () => {
+		const result = resolveToCwd("hello.txt", workspace);
+		expect(result).toBe(join(workspace, "hello.txt"));
+	});
+
+	test("resolves '.' to workspace root", () => {
+		const result = resolveToCwd(".", workspace);
+		expect(result).toBe(workspace);
+	});
+});
+
+// ─── Tool-level containment (read tool) ─────────────────────────────
+
+describe("read tool - workspace containment", () => {
+	let readTool: any;
+
+	beforeAll(async () => {
+		const { createReadTool } = await import("../src/agent/tools/read.js");
+		readTool = createReadTool(workspace);
+	});
+
+	test("reads file inside workspace", async () => {
+		const result = await readTool.execute("id", { path: "hello.txt" });
+		expect(result.content[0].text).toBe("hello");
+	});
+
+	test("rejects traversal via ..", async () => {
+		await expect(readTool.execute("id", { path: "../../etc/passwd" })).rejects.toThrow(
+			/escapes workspace/i,
+		);
+	});
+
+	test("rejects absolute path outside workspace", async () => {
+		await expect(readTool.execute("id", { path: "/etc/passwd" })).rejects.toThrow(
+			/escapes workspace/i,
+		);
+	});
+
+	test("tilde path stays within workspace (no homedir expansion)", async () => {
+		// With tilde expansion removed, ~/foo resolves to <workspace>/~/foo
+		// which is safely inside the workspace. It will fail with ENOENT,
+		// NOT succeed in reading the user's homedir file.
+		await expect(readTool.execute("id", { path: "~/.bashrc" })).rejects.toThrow(/ENOENT/);
+	});
+});
+
+// ─── Tool-level containment (ls tool) ───────────────────────────────
+
+describe("ls tool - workspace containment", () => {
+	let lsTool: any;
+
+	beforeAll(async () => {
+		const { createLsTool } = await import("../src/agent/tools/ls.js");
+		lsTool = createLsTool(workspace);
+	});
+
+	test("lists workspace root", async () => {
+		const result = await lsTool.execute("id", {});
+		expect(result.content[0].text).toContain("hello.txt");
+	});
+
+	test("rejects traversal via ..", async () => {
+		await expect(lsTool.execute("id", { path: "../../" })).rejects.toThrow(
+			/escapes workspace/i,
+		);
+	});
+
+	test("rejects absolute path outside workspace", async () => {
+		await expect(lsTool.execute("id", { path: "/tmp" })).rejects.toThrow(
+			/escapes workspace/i,
+		);
+	});
+});
+
+// ─── Tool-level containment (grep tool) ─────────────────────────────
+
+describe("grep tool - workspace containment", () => {
+	let grepTool: any;
+
+	beforeAll(async () => {
+		const { createGrepTool } = await import("../src/agent/tools/grep.js");
+		grepTool = createGrepTool(workspace);
+	});
+
+	test("searches within workspace", async () => {
+		const result = await grepTool.execute("id", { pattern: "hello", literal: true });
+		expect(result.content[0].text).toContain("hello");
+	});
+
+	test("rejects traversal via ..", async () => {
+		await expect(
+			grepTool.execute("id", { pattern: "root", path: "../../etc" }),
+		).rejects.toThrow(/escapes workspace/i);
+	});
+
+	test("rejects absolute path outside workspace", async () => {
+		await expect(
+			grepTool.execute("id", { pattern: "root", path: "/etc" }),
+		).rejects.toThrow(/escapes workspace/i);
+	});
+});
+
+// ─── Tool-level containment (find tool) ─────────────────────────────
+
+describe("find tool - workspace containment", () => {
+	let findTool: any;
+
+	beforeAll(async () => {
+		const { createFindTool } = await import("../src/agent/tools/find.js");
+		findTool = createFindTool(workspace);
+	});
+
+	test("finds files in workspace", async () => {
+		const result = await findTool.execute("id", { pattern: "*.txt" });
+		expect(result.content[0].text).toContain("hello.txt");
+	});
+
+	test("rejects traversal via ..", async () => {
+		await expect(
+			findTool.execute("id", { pattern: "*", path: "../../" }),
+		).rejects.toThrow(/escapes workspace/i);
+	});
+
+	test("rejects absolute path outside workspace", async () => {
+		await expect(
+			findTool.execute("id", { pattern: "*", path: "/tmp" }),
+		).rejects.toThrow(/escapes workspace/i);
+	});
+});
+
+// ─── writeWorkspaceFile containment (Issue #4) ──────────────────────
+
+describe("writeWorkspaceFile - workspace containment", () => {
+	test("writes file inside workspace", async () => {
+		const result = await writeWorkspaceFile(workspace, "output.txt", "data");
+		expect(result.filePath).toBe(join(workspace, "output.txt"));
+	});
+
+	test("writes nested file inside workspace", async () => {
+		const result = await writeWorkspaceFile(workspace, "a/b/c.txt", "deep");
+		expect(result.filePath).toBe(join(workspace, "a", "b", "c.txt"));
+	});
+
+	test("rejects traversal via ..", async () => {
+		await expect(
+			writeWorkspaceFile(workspace, "../../../tmp/evil.txt", "pwned"),
+		).rejects.toThrow(/escapes workspace/i);
+	});
+
+	test("absolute path via join stays inside workspace", async () => {
+		// path.join(workspace, "/tmp/evil.txt") => "<workspace>/tmp/evil.txt"
+		// This is actually inside the workspace β€” join concatenates, doesn't replace.
+		const result = await writeWorkspaceFile(workspace, "/tmp/evil.txt", "safe");
+		expect(result.filePath).toBe(join(workspace, "tmp", "evil.txt"));
+	});
+
+	test("tilde path via join stays inside workspace", async () => {
+		// With no tilde expansion, ~/evil.txt joins as <workspace>/~/evil.txt
+		const result = await writeWorkspaceFile(workspace, "~/evil.txt", "safe");
+		expect(result.filePath).toBe(join(workspace, "~", "evil.txt"));
+	});
+});

tsconfig.json πŸ”—

@@ -11,7 +11,7 @@
     "allowJs": false,
     "types": ["bun-types"],
     "outDir": "dist",
-    "rootDir": "src",
+    "rootDir": ".",
     "skipLibCheck": true
   },
   "include": ["src", "test"],