diff --git a/src/permission/policy/matchPermissionRule.ts b/src/permission/policy/matchPermissionRule.ts index 47b7173ec..52be77b37 100644 --- a/src/permission/policy/matchPermissionRule.ts +++ b/src/permission/policy/matchPermissionRule.ts @@ -1,4 +1,6 @@ import path from "node:path"; +import { realpathSync } from "node:fs"; +import { resolveRealWritePath } from "../../tool/builtin/filesystem/pathSafety.js"; import type { PermissionContext, PermissionRule } from "../protocol/types.js"; const FILE_WRITE_TOOLS = new Set(["write_file", "edit_file"]); @@ -61,9 +63,19 @@ function matchFilePathPattern(pattern: string, input: unknown, context: Permissi function isFileInputInsideWorkspace(input: unknown, context: PermissionContext | undefined): boolean { const filePath = resolveInputFilePath(input, context); if (!filePath || !context) return false; - return [context.cwd, ...context.additionalWorkingDirectories] - .map((root) => path.resolve(root)) - .some((root) => isPathWithinRoot(filePath, root)); + const roots = [context.cwd, ...context.additionalWorkingDirectories].map((root) => path.resolve(root)); + if (!roots.some((root) => isPathWithinRoot(filePath, root))) return false; + // A symlink inside the workspace can still point the write elsewhere. + const realFilePath = resolveRealWritePath(filePath); + return roots.map(safeRealpath).some((root) => isPathWithinRoot(realFilePath, root)); +} + +function safeRealpath(value: string): string { + try { + return realpathSync(value); + } catch { + return value; + } } function resolveInputFilePath(input: unknown, context: PermissionContext | undefined): string | undefined { diff --git a/src/tool/builtin/filesystem/pathSafety.ts b/src/tool/builtin/filesystem/pathSafety.ts index f97177589..1840240aa 100644 --- a/src/tool/builtin/filesystem/pathSafety.ts +++ b/src/tool/builtin/filesystem/pathSafety.ts @@ -1,5 +1,5 @@ import path from "node:path"; -import { realpathSync, statSync } from "node:fs"; +import { readlinkSync, realpathSync, statSync } from "node:fs"; import { homedir } from "node:os"; import type { PilotDeckToolRuntimeContext } from "../../protocol/types.js"; import type { PilotDeckToolError } from "../../protocol/errors.js"; @@ -10,6 +10,7 @@ export type PilotDeckPathSafetyResult = | { ok: false; error: PilotDeckToolError }; const DEFAULT_WRITE_DENY_DIRECTORIES = new Set([".git", "node_modules", "dist"]); +const MAX_SYMLINK_HOPS = 40; export function resolvePilotDeckWorkspacePath( inputPath: string, @@ -24,10 +25,13 @@ export function resolvePilotDeckWorkspacePath( } const absolutePath = path.resolve(path.isAbsolute(inputPath) ? inputPath : path.join(context.cwd, inputPath)); + // Writes follow symlinks, so authorization must also hold for the path the + // OS will actually write to, not just the literal path the caller supplied. + const realWritePath = options?.forWrite ? resolveRealWritePath(absolutePath) : undefined; if (context.permissionMode === "bypassPermissions") { const relativePath = path.relative(context.cwd, absolutePath) || "."; - if (options?.forWrite && isWriteDenied(relativePath)) { + if (options?.forWrite && (isWriteDenied(relativePath) || isRealWriteDenied(realWritePath, [context.cwd]))) { return { ok: false, error: toolError("path_not_allowed", `Writing to ${relativePath} is not allowed by default.`), @@ -62,7 +66,7 @@ export function resolvePilotDeckWorkspacePath( if (options?.allowOutsideWorkspace) { const relativePath = path.relative(context.cwd, absolutePath) || "."; - if (options?.forWrite && isWriteDenied(relativePath)) { + if (options?.forWrite && (isWriteDenied(relativePath) || isRealWriteDenied(realWritePath, roots))) { return { ok: false, error: toolError("path_not_allowed", `Writing to ${relativePath} is not allowed by default.`), @@ -78,13 +82,20 @@ export function resolvePilotDeckWorkspacePath( } const relativePath = path.relative(root, absolutePath) || "."; - if (options?.forWrite && isWriteDenied(relativePath)) { + if (options?.forWrite && (isWriteDenied(relativePath) || isRealWriteDenied(realWritePath, roots))) { return { ok: false, error: toolError("path_not_allowed", `Writing to ${relativePath} is not allowed by default.`), }; } + if (realWritePath && !findRealRoot(realWritePath, roots) && !options?.allowOutsideWorkspace) { + return { + ok: false, + error: toolError("path_not_allowed", `Path ${inputPath} resolves outside the PilotDeck workspace.`), + }; + } + if (options?.mustExist) { const real = safeRealpath(absolutePath); if (!real) { @@ -115,11 +126,55 @@ export function isPathWithinRoot(candidate: string, root: string): boolean { return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); } +/** + * Resolves the path a write to `absolutePath` would land on after the OS + * follows symlinks. Handles targets that do not exist yet (including dangling + * symlinks) by resolving the deepest existing ancestor and re-appending the + * remaining segments. + */ +export function resolveRealWritePath(absolutePath: string): string { + const pending: string[] = []; + let current = absolutePath; + for (let linkHops = 0; linkHops <= MAX_SYMLINK_HOPS;) { + const real = safeRealpath(current); + if (real) { + return path.join(real, ...pending); + } + const linkTarget = safeReadlink(current); + if (linkTarget !== undefined) { + current = path.resolve(path.dirname(current), linkTarget); + linkHops += 1; + continue; + } + const parent = path.dirname(current); + if (parent === current) { + break; + } + pending.unshift(path.basename(current)); + current = parent; + } + return path.join(current, ...pending); +} + function isWriteDenied(relativePath: string): boolean { const firstPart = relativePath.split(path.sep)[0]; return firstPart !== undefined && DEFAULT_WRITE_DENY_DIRECTORIES.has(firstPart); } +function isRealWriteDenied(realWritePath: string | undefined, roots: string[]): boolean { + if (!realWritePath) { + return false; + } + const realRoot = findRealRoot(realWritePath, roots); + return realRoot !== undefined && isWriteDenied(path.relative(realRoot, realWritePath)); +} + +function findRealRoot(realPath: string, roots: string[]): string | undefined { + return roots + .map((root) => safeRealpath(root) ?? path.resolve(root)) + .find((realRoot) => isPathWithinRoot(realPath, realRoot)); +} + function safeRealpath(value: string): string | undefined { try { return realpathSync(value); @@ -128,6 +183,14 @@ function safeRealpath(value: string): string | undefined { } } +function safeReadlink(value: string): string | undefined { + try { + return readlinkSync(value); + } catch { + return undefined; + } +} + function isManagedImAttachmentFile(realPath: string, context: PilotDeckToolRuntimeContext): boolean { const pilotHome = path.resolve(context.env?.PILOT_HOME ?? path.join(homedir(), ".pilotdeck")); const root = safeRealpath(path.join(pilotHome, "im-attachments")) ?? path.join(pilotHome, "im-attachments"); diff --git a/tests/tool/write-symlink-escape.spec.ts b/tests/tool/write-symlink-escape.spec.ts new file mode 100644 index 000000000..bbc7bb0e3 --- /dev/null +++ b/tests/tool/write-symlink-escape.spec.ts @@ -0,0 +1,156 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; + +import { createEditFileTool } from "../../src/tool/builtin/editFile.js"; +import { createWriteFileTool } from "../../src/tool/builtin/writeFile.js"; +import { checkFilesystemWritePermission } from "../../src/tool/builtin/filesystem/writePermissions.js"; +import type { PermissionMode } from "../../src/permission/index.js"; +import { matchPermissionRule } from "../../src/permission/policy/matchPermissionRule.js"; + +function context(cwd: string, permissionMode: PermissionMode = "default") { + return { + sessionId: "s1", + turnId: "t1", + cwd, + permissionMode, + permissionContext: { + mode: permissionMode, + cwd, + additionalWorkingDirectories: [], + canPrompt: true, + bypassAvailable: true, + rules: { allow: [], deny: [], ask: [] }, + }, + now: () => new Date("2026-09-28T00:00:00.000Z"), + }; +} + +async function withTempDirs(run: (workspace: string, outside: string) => Promise): Promise { + const workspace = await mkdtemp(join(tmpdir(), "pilotdeck-symlink-ws-")); + const outside = await mkdtemp(join(tmpdir(), "pilotdeck-symlink-out-")); + try { + await run(workspace, outside); + } finally { + await rm(workspace, { recursive: true, force: true }); + await rm(outside, { recursive: true, force: true }); + } +} + +for (const permissionMode of ["default", "bypassPermissions"] as const) { + test(`write_file rejects writes into .git through a directory symlink (${permissionMode})`, async () => { + await withTempDirs(async (workspace) => { + await mkdir(join(workspace, ".git")); + await symlink(".git", join(workspace, "repo-internals")); + const ctx = context(workspace, permissionMode); + + const permission = checkFilesystemWritePermission("write_file", "repo-internals/HEAD", ctx); + assert.equal(permission.type, "deny"); + + const validation = await createWriteFileTool().validateInput!({ + file_path: "repo-internals/HEAD", + content: "clobbered\n", + }, ctx); + assert.equal(validation.ok, false); + + await assert.rejects( + createWriteFileTool().execute({ file_path: "repo-internals/HEAD", content: "clobbered\n" }, ctx), + /not allowed/, + ); + await assert.rejects(readFile(join(workspace, ".git", "HEAD"), "utf8"), { code: "ENOENT" }); + }); + }); +} + +test("write_file rejects writes into .git through a file symlink", async () => { + await withTempDirs(async (workspace) => { + await mkdir(join(workspace, ".git")); + await writeFile(join(workspace, ".git", "HEAD"), "ref: refs/heads/main\n"); + await symlink(join(".git", "HEAD"), join(workspace, "head-link")); + const ctx = context(workspace); + + assert.equal(checkFilesystemWritePermission("write_file", "head-link", ctx).type, "deny"); + await assert.rejects( + createWriteFileTool().execute({ file_path: "head-link", content: "clobbered\n" }, ctx), + /not allowed/, + ); + assert.equal(await readFile(join(workspace, ".git", "HEAD"), "utf8"), "ref: refs/heads/main\n"); + }); +}); + +test("write_file rejects writes into .git through a dangling file symlink", async () => { + await withTempDirs(async (workspace) => { + await mkdir(join(workspace, ".git")); + await symlink(join(".git", "config"), join(workspace, "config-link")); + const ctx = context(workspace); + + assert.equal(checkFilesystemWritePermission("write_file", "config-link", ctx).type, "deny"); + await assert.rejects( + createWriteFileTool().execute({ file_path: "config-link", content: "clobbered\n" }, ctx), + /not allowed/, + ); + await assert.rejects(readFile(join(workspace, ".git", "config"), "utf8"), { code: "ENOENT" }); + }); +}); + +test("edit_file rejects edits into .git through a directory symlink", async () => { + await withTempDirs(async (workspace) => { + await mkdir(join(workspace, ".git")); + await symlink(".git", join(workspace, "repo-internals")); + const ctx = context(workspace); + + assert.equal(checkFilesystemWritePermission("edit_file", "repo-internals/HEAD", ctx).type, "deny"); + await assert.rejects( + createEditFileTool().execute({ + file_path: "repo-internals/HEAD", + old_string: "", + new_string: "clobbered\n", + }, ctx), + /not allowed/, + ); + await assert.rejects(readFile(join(workspace, ".git", "HEAD"), "utf8"), { code: "ENOENT" }); + }); +}); + +test("write_file asks before writing outside the workspace through a directory symlink", async () => { + await withTempDirs(async (workspace, outside) => { + await symlink(outside, join(workspace, "escape")); + const ctx = context(workspace); + + const permission = checkFilesystemWritePermission("write_file", "escape/created.txt", ctx); + assert.equal(permission.type, "ask"); + + await assert.rejects( + createWriteFileTool().execute({ file_path: "escape/created.txt", content: "outside\n" }, ctx), + /outside the PilotDeck workspace/, + ); + await assert.rejects(readFile(join(outside, "created.txt"), "utf8"), { code: "ENOENT" }); + }); +}); + +test("write_file still writes through symlinks that stay inside the workspace", async () => { + await withTempDirs(async (workspace) => { + await mkdir(join(workspace, "real-dir")); + await symlink("real-dir", join(workspace, "alias-dir")); + const ctx = context(workspace); + + assert.equal(checkFilesystemWritePermission("write_file", "alias-dir/new.txt", ctx).type, "passthrough"); + await createWriteFileTool().execute({ file_path: "alias-dir/new.txt", content: "inside\n" }, ctx); + assert.equal(await readFile(join(workspace, "real-dir", "new.txt"), "utf8"), "inside\n"); + }); +}); + +test("a workspace-scoped write_file allow rule does not cover symlinks that escape the workspace", async () => { + await withTempDirs(async (workspace, outside) => { + await mkdir(join(workspace, "real-dir")); + await symlink("real-dir", join(workspace, "alias-dir")); + await symlink(outside, join(workspace, "escape")); + const { permissionContext } = context(workspace); + const rule = { source: "session" as const, behavior: "allow" as const, toolName: "write_file" }; + + assert.equal(matchPermissionRule(rule, "write_file", { file_path: "alias-dir/new.txt" }, permissionContext), true); + assert.equal(matchPermissionRule(rule, "write_file", { file_path: "escape/new.txt" }, permissionContext), false); + }); +});