From 11541b86d6f84545d97c55abf155b142c11e9144 Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Mon, 31 Aug 2026 06:40:25 +0530 Subject: [PATCH 1/3] fix(fs): resolve allowed paths canonically --- src/roots.test.ts | 55 ++++++++++++++++++++++++++++++++++++++++-- src/roots.ts | 61 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 113 insertions(+), 3 deletions(-) diff --git a/src/roots.test.ts b/src/roots.test.ts index 90bdaa243..f821b57a7 100644 --- a/src/roots.test.ts +++ b/src/roots.test.ts @@ -1,7 +1,13 @@ import assert from "node:assert/strict"; -import { homedir } from "node:os"; +import { mkdtemp, mkdir, rm, symlink, writeFile } from "node:fs/promises"; +import { homedir, tmpdir } from "node:os"; import { join, resolve } from "node:path"; -import { assertAllowedPath, expandHomePath, resolveAllowedPath } from "./roots.js"; +import { + assertAllowedPath, + expandHomePath, + resolveAllowedPath, + resolveCanonicalAllowedPath, +} from "./roots.js"; const home = homedir(); @@ -31,3 +37,48 @@ if (process.platform === "win32") { /Path is outside allowed roots/, ); } + +const fixtureRoot = await mkdtemp(join(tmpdir(), "devspace-roots-test-")); +try { + const workspace = join(fixtureRoot, "workspace"); + const outside = join(fixtureRoot, "outside"); + await mkdir(join(workspace, "inside"), { recursive: true }); + await mkdir(outside, { recursive: true }); + await writeFile(join(outside, "secret.txt"), "secret\n"); + + const outsideLink = join(workspace, "outside-link"); + await symlink(outside, outsideLink, process.platform === "win32" ? "junction" : "dir"); + await assert.rejects( + resolveCanonicalAllowedPath(join(outsideLink, "secret.txt"), workspace, [workspace]), + /outside allowed roots/, + ); + await assert.rejects( + resolveCanonicalAllowedPath(join(outsideLink, "new.txt"), workspace, [workspace]), + /outside allowed roots/, + ); + + const insideLink = join(workspace, "inside-link"); + await symlink(join(workspace, "inside"), insideLink, process.platform === "win32" ? "junction" : "dir"); + assert.equal( + await resolveCanonicalAllowedPath(join(insideLink, "new.txt"), workspace, [workspace]), + join(workspace, "inside", "new.txt"), + ); + + const outsideAlias = join(outside, "workspace-link"); + await symlink(workspace, outsideAlias, process.platform === "win32" ? "junction" : "dir"); + await assert.rejects( + resolveCanonicalAllowedPath(join(outsideAlias, "inside"), workspace, [workspace]), + /outside allowed roots/, + ); + + if (process.platform !== "win32") { + const danglingLink = join(workspace, "dangling-link"); + await symlink(join(outside, "missing.txt"), danglingLink); + await assert.rejects( + resolveCanonicalAllowedPath(danglingLink, workspace, [workspace]), + /Cannot resolve symbolic link/, + ); + } +} finally { + await rm(fixtureRoot, { recursive: true, force: true }); +} diff --git a/src/roots.ts b/src/roots.ts index 214ffb2b0..bb86b8db6 100644 --- a/src/roots.ts +++ b/src/roots.ts @@ -1,5 +1,6 @@ import { homedir } from "node:os"; -import { isAbsolute, relative, resolve, sep } from "node:path"; +import { lstat, realpath } from "node:fs/promises"; +import { dirname, isAbsolute, relative, resolve, sep } from "node:path"; export class AccessDeniedError extends Error { constructor(message: string) { @@ -44,3 +45,61 @@ export function resolveAllowedPath(inputPath: string, cwd: string, allowedRoots: const absolutePath = resolve(cwd, inputPath); return assertAllowedPath(absolutePath, allowedRoots); } + +export async function resolveCanonicalAllowedPath( + inputPath: string, + cwd: string, + allowedRoots: string[], +): Promise { + const absolutePath = resolveAllowedPath(inputPath, cwd, allowedRoots); + const canonicalPath = await canonicalizePath(absolutePath); + + for (const root of allowedRoots) { + const canonicalRoot = await canonicalizePath(root); + if (isPathInsideRoot(canonicalPath, canonicalRoot)) return canonicalPath; + } + + throw new AccessDeniedError(`Path is outside allowed roots: ${inputPath}`); +} + +export async function assertCanonicalAllowedPath(path: string, allowedRoots: string[]): Promise { + return resolveCanonicalAllowedPath(path, process.cwd(), allowedRoots); +} + +async function canonicalizePath(path: string): Promise { + const absolutePath = resolve(expandHomePath(path)); + let candidate = absolutePath; + + for (;;) { + try { + const boundaryPath = await realpath(candidate); + const suffix = relative(candidate, absolutePath); + return suffix ? resolve(boundaryPath, suffix) : boundaryPath; + } catch (error) { + if (!isMissingPathError(error)) throw error; + + try { + const stats = await lstat(candidate); + if (stats.isSymbolicLink()) { + throw new AccessDeniedError(`Cannot resolve symbolic link: ${path}`); + } + } catch (lstatError) { + if (!isMissingPathError(lstatError)) throw lstatError; + } + + const parent = dirname(candidate); + if (parent === candidate) throw error; + candidate = parent; + } + } +} + +function isMissingPathError(error: unknown): boolean { + return Boolean( + typeof error === "object" && + error && + "code" in error && + ((error as NodeJS.ErrnoException).code === "ENOENT" || + (error as NodeJS.ErrnoException).code === "ENOTDIR"), + ); +} From 8bcec18eb31cb6e598d1cf60be456f006fa67cb1 Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Mon, 31 Aug 2026 06:40:25 +0530 Subject: [PATCH 2/3] fix(tools): block symlink escapes in file tools --- package.json | 2 +- src/pi-tools.test.ts | 64 ++++++++++++++++++++++++++++++++++++++++++++ src/pi-tools.ts | 12 ++++++--- 3 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 src/pi-tools.test.ts diff --git a/package.json b/package.json index 4c64031c5..584189555 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "postinstall": "node scripts/fix-node-pty-permissions.mjs", "schema:config": "tsx scripts/generate-config-schema.ts", "start": "node dist/cli.js serve", - "test": "tsx src/user-config.test.ts && tsx src/config.test.ts && tsx src/onboarding.test.ts && tsx src/cli-workspace.test.ts && tsx src/request-meta.test.ts && tsx src/incoming-artifacts.test.ts && tsx src/artifact-download.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/tool-result.test.ts && tsx src/ui/patch-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/mcp-sessions.test.ts && tsx src/server-shutdown.test.ts && tsx src/local-agent-config.test.ts && tsx src/local-agent-catalog.test.ts && tsx src/local-agent-presentation.test.ts && tsx src/local-agent-runtime.test.ts && tsx src/local-agent-daemon-lifecycle.test.ts && tsx src/local-agent-daemon-protocol.test.ts && tsx src/local-agent-daemon.test.ts && tsx src/local-agent-codex.test.ts && tsx src/local-agent-opencode.test.ts && tsx src/local-agent-acp.test.ts && tsx src/local-agent-grok.test.ts && tsx src/local-agent-pi-sandbox.test.ts && tsx src/local-agent-pi.test.ts && tsx src/local-agent-claude.test.ts && tsx src/local-agent-adapters.test.ts && tsx src/local-agent-availability.test.ts && tsx src/local-agent-profiles.test.ts && tsx src/local-agent-targets.test.ts && tsx src/local-agent-store.test.ts && tsx src/local-agent-manager.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/workspace-conversation.test.ts && tsx src/review-checkpoints.test.ts && tsx src/server.test.ts && tsx src/oauth-store.test.ts && tsx src/cli-show-changes.test.ts && tsx src/cli.test.ts", + "test": "tsx src/user-config.test.ts && tsx src/config.test.ts && tsx src/onboarding.test.ts && tsx src/cli-workspace.test.ts && tsx src/request-meta.test.ts && tsx src/incoming-artifacts.test.ts && tsx src/artifact-download.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/tool-result.test.ts && tsx src/ui/patch-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/mcp-sessions.test.ts && tsx src/server-shutdown.test.ts && tsx src/local-agent-config.test.ts && tsx src/local-agent-catalog.test.ts && tsx src/local-agent-presentation.test.ts && tsx src/local-agent-runtime.test.ts && tsx src/local-agent-daemon-lifecycle.test.ts && tsx src/local-agent-daemon-protocol.test.ts && tsx src/local-agent-daemon.test.ts && tsx src/local-agent-codex.test.ts && tsx src/local-agent-opencode.test.ts && tsx src/local-agent-acp.test.ts && tsx src/local-agent-grok.test.ts && tsx src/local-agent-pi-sandbox.test.ts && tsx src/local-agent-pi.test.ts && tsx src/local-agent-claude.test.ts && tsx src/local-agent-adapters.test.ts && tsx src/local-agent-availability.test.ts && tsx src/local-agent-profiles.test.ts && tsx src/local-agent-targets.test.ts && tsx src/local-agent-store.test.ts && tsx src/local-agent-manager.test.ts && tsx src/roots.test.ts && tsx src/pi-tools.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/workspace-conversation.test.ts && tsx src/review-checkpoints.test.ts && tsx src/server.test.ts && tsx src/oauth-store.test.ts && tsx src/cli-show-changes.test.ts && tsx src/cli.test.ts", "typecheck": "tsx src/config-schema.test.ts && tsc -p tsconfig.json --noEmit" }, "keywords": [], diff --git a/src/pi-tools.test.ts b/src/pi-tools.test.ts new file mode 100644 index 000000000..d949d6088 --- /dev/null +++ b/src/pi-tools.test.ts @@ -0,0 +1,64 @@ +import assert from "node:assert/strict"; +import { mkdtemp, mkdir, readFile, rm, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { editFileTool, readFileTool, writeFileTool } from "./pi-tools.js"; + +const fixtureRoot = await mkdtemp(join(tmpdir(), "devspace-pi-tools-test-")); +try { + const workspace = join(fixtureRoot, "workspace"); + const outside = join(fixtureRoot, "outside"); + const inside = join(workspace, "inside"); + await mkdir(inside, { recursive: true }); + await mkdir(outside, { recursive: true }); + await writeFile(join(outside, "secret.txt"), "outside secret\n"); + await writeFile(join(outside, "editable.txt"), "before\n"); + + const outsideLink = join(workspace, "outside-link"); + await symlink(outside, outsideLink, process.platform === "win32" ? "junction" : "dir"); + const context = { cwd: workspace, root: workspace }; + + await assert.rejects( + readFileTool({ path: "outside-link/secret.txt" }, context), + /outside allowed roots/, + ); + await assert.rejects( + writeFileTool({ path: "outside-link/new.txt", content: "escaped\n" }, context), + /outside allowed roots/, + ); + await assert.rejects(readFile(join(outside, "new.txt"), "utf8"), /ENOENT/); + + await assert.rejects( + editFileTool( + { + path: "outside-link/editable.txt", + edits: [{ oldText: "before", newText: "after" }], + }, + context, + ), + /outside allowed roots/, + ); + assert.equal(await readFile(join(outside, "editable.txt"), "utf8"), "before\n"); + + const insideLink = join(workspace, "inside-link"); + await symlink(inside, insideLink, process.platform === "win32" ? "junction" : "dir"); + const safeWrite = await writeFileTool( + { path: "inside-link/new.txt", content: "inside\n" }, + context, + ); + assert.equal(safeWrite.isError, undefined); + assert.equal(await readFile(join(inside, "new.txt"), "utf8"), "inside\n"); + + if (process.platform !== "win32") { + const danglingLink = join(workspace, "dangling-link"); + const danglingTarget = join(outside, "dangling-created.txt"); + await symlink(danglingTarget, danglingLink); + await assert.rejects( + writeFileTool({ path: "dangling-link", content: "escaped\n" }, context), + /Cannot resolve symbolic link/, + ); + await assert.rejects(readFile(danglingTarget, "utf8"), /ENOENT/); + } +} finally { + await rm(fixtureRoot, { recursive: true, force: true }); +} diff --git a/src/pi-tools.ts b/src/pi-tools.ts index 06f821976..b936df5a8 100644 --- a/src/pi-tools.ts +++ b/src/pi-tools.ts @@ -10,7 +10,7 @@ import { type WriteToolInput, type AgentToolResult, } from "@earendil-works/pi-coding-agent"; -import { resolveAllowedPath } from "./roots.js"; +import { resolveCanonicalAllowedPath } from "./roots.js"; type McpContent = { type: "text"; text: string } | { type: "image"; data: string; mimeType: string }; export type ToolResponse = { @@ -61,7 +61,11 @@ async function runTool( } export async function readFileTool(input: ReadToolInput, context: ToolContext): Promise { - const path = resolveAllowedPath(input.path, context.cwd, context.readRoots ?? [context.root]); + const path = await resolveCanonicalAllowedPath( + input.path, + context.cwd, + context.readRoots ?? [context.root], + ); const tool = createReadTool(context.cwd); return runTool((params) => tool.execute("read_file", params), { @@ -72,7 +76,7 @@ export async function readFileTool(input: ReadToolInput, context: ToolContext): } export async function writeFileTool(input: WriteToolInput, context: ToolContext): Promise { - const path = resolveAllowedPath(input.path, context.cwd, [context.root]); + const path = await resolveCanonicalAllowedPath(input.path, context.cwd, [context.root]); const tool = createWriteTool(context.cwd); return runTool((params) => tool.execute("write_file", params), { @@ -82,7 +86,7 @@ export async function writeFileTool(input: WriteToolInput, context: ToolContext) } export async function editFileTool(input: EditToolInput, context: ToolContext): Promise> { - const path = resolveAllowedPath(input.path, context.cwd, [context.root]); + const path = await resolveCanonicalAllowedPath(input.path, context.cwd, [context.root]); const tool = createEditTool(context.cwd); return runTool((params) => tool.execute("edit_file", params), { From 5b419d46d6c8dcf8995c5b8e742eb18ac51ea7d2 Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Mon, 31 Aug 2026 06:40:25 +0530 Subject: [PATCH 3/3] fix(workspaces): reject symlink root escapes --- src/git-worktrees.ts | 4 +++- src/workspaces.test.ts | 19 +++++++++++++++++++ src/workspaces.ts | 4 ++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/git-worktrees.ts b/src/git-worktrees.ts index 04986c9a5..8a6f97af7 100644 --- a/src/git-worktrees.ts +++ b/src/git-worktrees.ts @@ -4,7 +4,7 @@ import { promisify } from "node:util"; import { mkdir, realpath, rm, stat } from "node:fs/promises"; import { basename, join, relative, resolve } from "node:path"; import type { ServerConfig } from "./config.js"; -import { assertAllowedPath, isPathInsideRoot } from "./roots.js"; +import { assertAllowedPath, assertCanonicalAllowedPath, isPathInsideRoot } from "./roots.js"; const execFileAsync = promisify(execFile); @@ -39,6 +39,7 @@ export async function createManagedWorktree(input: { config: ServerConfig; }): Promise { const sourcePath = assertAllowedPath(input.sourcePath, input.config.allowedRoots); + await assertCanonicalAllowedPath(sourcePath, input.config.allowedRoots); try { const sourceStats = await stat(sourcePath); @@ -67,6 +68,7 @@ export async function createManagedWorktree(input: { await mkdir(input.config.worktreeRoot, { recursive: true }); assertAllowedPath(worktreePath, [input.config.worktreeRoot]); + await assertCanonicalAllowedPath(worktreePath, [input.config.worktreeRoot]); try { await git(["worktree", "add", "--detach", worktreePath, baseSha], sourceRoot); diff --git a/src/workspaces.test.ts b/src/workspaces.test.ts index 3dab10807..82b85c1ad 100644 --- a/src/workspaces.test.ts +++ b/src/workspaces.test.ts @@ -142,6 +142,25 @@ test("workspace paths outside the allowed roots are rejected", async (t) => { ); }); +test("workspace paths cannot escape allowed roots through symlinks", async (t) => { + const context = await fixture(t); + const outsideLink = join(context.root, "outside-link"); + await symlink( + context.outsideRoot, + outsideLink, + platform() === "win32" ? "junction" : "dir", + ); + + await assert.rejects( + () => context.registry.openWorkspace(outsideLink), + /outside allowed roots/, + ); + await assert.rejects( + () => context.registry.openWorkspace({ path: outsideLink, mode: "worktree" }), + /outside allowed roots/, + ); +}); + test("a symlinked allowed root preserves checkout and worktree path behavior", { skip: platform() === "win32" }, async (t) => { const context = await fixture(t); const aliasRoot = join(context.root, "alias-root"); diff --git a/src/workspaces.ts b/src/workspaces.ts index 307626489..5cfb769b3 100644 --- a/src/workspaces.ts +++ b/src/workspaces.ts @@ -13,6 +13,7 @@ import { createManagedWorktree } from "./git-worktrees.js"; import { AccessDeniedError, assertAllowedPath, + assertCanonicalAllowedPath, isPathInsideRoot, resolveAllowedPath, } from "./roots.js"; @@ -201,6 +202,7 @@ export class WorkspaceRegistry { let root: string; try { root = this.assertWorkspaceRootAllowed(session.root, session.mode, session.sourceRoot); + await assertCanonicalAllowedPath(root, this.config.allowedRoots); const rootStats = await stat(root); if (!rootStats.isDirectory()) return undefined; } catch (error) { @@ -221,6 +223,7 @@ export class WorkspaceRegistry { private async conversationProjectKey(input: OpenWorkspaceInput): Promise { const path = assertAllowedPath(input.path, this.config.allowedRoots); + await assertCanonicalAllowedPath(path, this.config.allowedRoots); return canonicalPath(path); } @@ -327,6 +330,7 @@ export class WorkspaceRegistry { private async openCheckoutWorkspace(path: string): Promise { const root = assertAllowedPath(path, this.config.allowedRoots); + await assertCanonicalAllowedPath(root, this.config.allowedRoots); const rootStats = await ensureCheckoutWorkspaceRoot(root); if (!rootStats.isDirectory()) { throw new Error(`Workspace root must be a directory: ${path}`);