From f461815af4d2870bfcd3f49c3a641b41b2255bad Mon Sep 17 00:00:00 2001 From: Amolith Date: Sat, 7 Feb 2026 23:25:57 +0000 Subject: [PATCH] Guard against symlink escape in workspace sandboxing ensureWorkspacePath() previously used path.resolve() which normalizes .. segments but does not follow symlinks. A symlink inside the workspace pointing outside (e.g. /escape -> /etc) would pass the textual prefix check and let tools operate beyond the sandbox boundary. Fix: - Add safeRealpath() that calls fs.realpathSync() and walks up to the nearest existing ancestor on ENOENT (needed for write targets that don't exist yet) - ensureWorkspacePath() now resolves both the workspace and target via realpath before checking containment - The cheap textual check runs first as a fast path; the realpath check catches symlink-based escapes Tests: 9 new cases covering symlink dirs/files pointing outside, symlinks within workspace (allowed), nested escapes, non-existent write targets through escaped parents, and writeWorkspaceFile integration. Co-authored-by: Shelley --- src/agent/tools/path-utils.ts | 44 ++++++++++++-- test/workspace-containment.test.ts | 93 ++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 5 deletions(-) diff --git a/src/agent/tools/path-utils.ts b/src/agent/tools/path-utils.ts index e0d59e62963b271527b5c61819c0f3bc435f99da..cb6209a736a82186efd06ff9275135626cb9074a 100644 --- a/src/agent/tools/path-utils.ts +++ b/src/agent/tools/path-utils.ts @@ -1,5 +1,5 @@ -import { accessSync, constants } from "node:fs"; -import { isAbsolute, resolve as resolvePath, sep } from "node:path"; +import { accessSync, constants, realpathSync } from "node:fs"; +import { dirname, isAbsolute, resolve as resolvePath, sep } from "node:path"; import { ToolInputError } from "../../util/errors.js"; const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g; @@ -67,13 +67,47 @@ export function resolveReadPath(filePath: string, cwd: string): string { return resolved; } +/** + * Resolve the real path of `p`, following symlinks. If `p` does not exist, + * walk up to the nearest existing ancestor, resolve *that*, and re-append + * the remaining segments. This lets us validate write targets that don't + * exist yet while still catching symlink escapes in any ancestor directory. + */ +function safeRealpath(p: string): string { + try { + return realpathSync(p); + } catch (err: any) { + if (err?.code === "ENOENT") { + const parent = dirname(p); + if (parent === p) { + // filesystem root — nothing more to resolve + return p; + } + const realParent = safeRealpath(parent); + const tail = p.slice(parent.length); + return realParent + tail; + } + throw err; + } +} + export function ensureWorkspacePath(workspacePath: string, targetPath: string): string { const resolved = resolvePath(workspacePath, targetPath); + + // Quick textual check first (catches the common case cheaply) const root = workspacePath.endsWith(sep) ? workspacePath : `${workspacePath}${sep}`; + if (resolved !== workspacePath && !resolved.startsWith(root)) { + throw new ToolInputError(`Path escapes workspace: ${targetPath}`); + } - if (resolved === workspacePath || resolved.startsWith(root)) { - return resolved; + // Resolve symlinks to catch symlink-based escapes + const realWorkspace = safeRealpath(workspacePath); + const realTarget = safeRealpath(resolved); + const realRoot = realWorkspace.endsWith(sep) ? realWorkspace : `${realWorkspace}${sep}`; + + if (realTarget !== realWorkspace && !realTarget.startsWith(realRoot)) { + throw new ToolInputError(`Path escapes workspace via symlink: ${targetPath}`); } - throw new ToolInputError(`Path escapes workspace: ${targetPath}`); + return resolved; } diff --git a/test/workspace-containment.test.ts b/test/workspace-containment.test.ts index b6765b15b33375be2606c961b8eecd09116f3126..6f44c3ada7ad667b47f4dc2c4e2cd9d84538104d 100644 --- a/test/workspace-containment.test.ts +++ b/test/workspace-containment.test.ts @@ -1,5 +1,6 @@ import { describe, test, expect, beforeAll, afterAll } from "bun:test"; import { mkdtemp, rm, writeFile, mkdir } from "node:fs/promises"; +import { symlinkSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { ensureWorkspacePath } from "../src/agent/tools/index.js"; @@ -235,3 +236,95 @@ describe("writeWorkspaceFile - workspace containment", () => { expect(result.filePath).toBe(join(workspace, "~", "evil.txt")); }); }); + +// ─── Symlink containment ──────────────────────────────────────────── + +describe("symlink containment", () => { + let symlinkWorkspace: string; + let outsideDir: string; + + beforeAll(async () => { + symlinkWorkspace = await mkdtemp(join(tmpdir(), "rumilo-symlink-test-")); + outsideDir = await mkdtemp(join(tmpdir(), "rumilo-outside-")); + + // Create a regular file inside workspace + await writeFile(join(symlinkWorkspace, "legit.txt"), "safe content"); + + // Create a subdirectory inside workspace + await mkdir(join(symlinkWorkspace, "subdir"), { recursive: true }); + await writeFile(join(symlinkWorkspace, "subdir", "inner.txt"), "inner content"); + + // Create a file outside workspace + await writeFile(join(outsideDir, "secret.txt"), "secret content"); + + // Symlink inside workspace pointing outside + symlinkSync(outsideDir, join(symlinkWorkspace, "escape-link")); + + // Symlink inside workspace pointing to file outside + symlinkSync(join(outsideDir, "secret.txt"), join(symlinkWorkspace, "secret-link.txt")); + + // Symlink inside workspace pointing to a file inside workspace (benign) + symlinkSync(join(symlinkWorkspace, "legit.txt"), join(symlinkWorkspace, "good-link.txt")); + + // Nested symlink escape: subdir/nested-escape -> outsideDir + symlinkSync(outsideDir, join(symlinkWorkspace, "subdir", "nested-escape")); + }); + + afterAll(async () => { + await rm(symlinkWorkspace, { recursive: true, force: true }); + await rm(outsideDir, { recursive: true, force: true }); + }); + + test("rejects symlink directory pointing outside workspace", () => { + expect(() => + ensureWorkspacePath(symlinkWorkspace, "escape-link/secret.txt"), + ).toThrow(/escapes workspace via symlink/); + }); + + test("rejects symlink file pointing outside workspace", () => { + expect(() => + ensureWorkspacePath(symlinkWorkspace, "secret-link.txt"), + ).toThrow(/escapes workspace via symlink/); + }); + + test("allows symlink pointing within workspace", () => { + const result = ensureWorkspacePath(symlinkWorkspace, "good-link.txt"); + expect(result).toBe(join(symlinkWorkspace, "good-link.txt")); + }); + + test("rejects nested symlink escape (subdir/nested-escape)", () => { + expect(() => + ensureWorkspacePath(symlinkWorkspace, "subdir/nested-escape/secret.txt"), + ).toThrow(/escapes workspace via symlink/); + }); + + test("rejects symlink escape via directory symlink alone", () => { + expect(() => + ensureWorkspacePath(symlinkWorkspace, "escape-link"), + ).toThrow(/escapes workspace via symlink/); + }); + + test("handles non-existent file in real directory (write target)", () => { + // File doesn't exist but parent is a real dir inside workspace — should pass + const result = ensureWorkspacePath(symlinkWorkspace, "subdir/new-file.txt"); + expect(result).toBe(join(symlinkWorkspace, "subdir", "new-file.txt")); + }); + + test("rejects non-existent file under symlink-escaped parent", () => { + // Parent is a symlink pointing outside — even though target file doesn't exist + expect(() => + ensureWorkspacePath(symlinkWorkspace, "escape-link/new-file.txt"), + ).toThrow(/escapes workspace via symlink/); + }); + + test("writeWorkspaceFile rejects path through symlink escape", async () => { + await expect( + writeWorkspaceFile(symlinkWorkspace, "escape-link/evil.txt", "pwned"), + ).rejects.toThrow(/escapes workspace via symlink/); + }); + + test("writeWorkspaceFile allows normal nested write", async () => { + const result = await writeWorkspaceFile(symlinkWorkspace, "new-dir/file.txt", "ok"); + expect(result.filePath).toBe(join(symlinkWorkspace, "new-dir", "file.txt")); + }); +});