diff --git a/packages/plugins/plugin-workspace-diff/src/contracts.ts b/packages/plugins/plugin-workspace-diff/src/contracts.ts index 14cf2e1f2fb..6267b9b867f 100644 --- a/packages/plugins/plugin-workspace-diff/src/contracts.ts +++ b/packages/plugins/plugin-workspace-diff/src/contracts.ts @@ -26,6 +26,7 @@ export const workspaceDiffWarningCodeSchema = z.enum([ "non_git_workspace", "patch_truncated", "path_filter_invalid", + "symlink_target_outside_workspace", "workspace_path_invalid", ]); diff --git a/packages/plugins/plugin-workspace-diff/src/workspace-diff.ts b/packages/plugins/plugin-workspace-diff/src/workspace-diff.ts index 20c86f8dccc..3bc08b1533a 100644 --- a/packages/plugins/plugin-workspace-diff/src/workspace-diff.ts +++ b/packages/plugins/plugin-workspace-diff/src/workspace-diff.ts @@ -1,4 +1,5 @@ import { execFile } from "node:child_process"; +import { constants as fsConstants } from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; import { promisify } from "node:util"; @@ -26,6 +27,7 @@ export const WORKSPACE_DIFF_CAPS: WorkspaceDiffCaps = { const GIT_TIMEOUT_MS = 10_000; const GIT_LIST_MAX_BUFFER = 2 * 1024 * 1024; +const OPEN_NOFOLLOW = fsConstants.O_NOFOLLOW ?? 0; interface GitStatusEntry { status: WorkspaceDiffFileStatus; @@ -301,13 +303,37 @@ async function readNumstat(cwd: string, scopeArgs: string[], filePath: string) { } async function statWorkspaceFile(repoRoot: string, filePath: string) { - const target = path.resolve(repoRoot, filePath); - if (!isWithinDirectory(target, repoRoot)) return null; + const resolved = await resolveWorkspaceFilePath(repoRoot, filePath); + if (resolved.status !== "ok") return null; + let handle: Awaited>; try { - const stat = await fs.stat(target); + handle = await fs.open(resolved.realPath, fsConstants.O_RDONLY | OPEN_NOFOLLOW); + } catch { + return null; + } + try { + const stat = await handle.stat(); return stat.isFile() ? stat.size : null; } catch { return null; + } finally { + await handle.close(); + } +} + +async function resolveWorkspaceFilePath(repoRoot: string, filePath: string): Promise< + | { status: "ok"; realPath: string } + | { status: "missing" } + | { status: "outside_workspace" } +> { + const target = path.resolve(repoRoot, filePath); + if (!isWithinDirectory(target, repoRoot)) return { status: "outside_workspace" }; + try { + const realPath = await fs.realpath(target); + if (!isWithinDirectory(realPath, repoRoot)) return { status: "outside_workspace" }; + return { status: "ok", realPath }; + } catch { + return { status: "missing" }; } } @@ -461,8 +487,13 @@ async function buildUntrackedFilePatch(input: { budget: PatchBudget; }): Promise { const warnings: WorkspaceDiffWarning[] = []; - const sizeBytes = await statWorkspaceFile(input.repoRoot, input.filePath); - if (sizeBytes === null) { + const resolved = await resolveWorkspaceFilePath(input.repoRoot, input.filePath); + if (resolved.status === "outside_workspace") { + warnings.push(warning( + "symlink_target_outside_workspace", + "Untracked file resolves outside the workspace and is summarized without reading target bytes.", + input.filePath, + )); return { kind: "untracked", patch: null, @@ -474,6 +505,59 @@ async function buildUntrackedFilePatch(input: { warnings, }; } + if (resolved.status === "missing") { + return { + kind: "untracked", + patch: null, + additions: 0, + deletions: 0, + binary: false, + oversized: false, + truncated: false, + warnings, + }; + } + + let handle: Awaited>; + try { + handle = await fs.open(resolved.realPath, fsConstants.O_RDONLY | OPEN_NOFOLLOW); + } catch { + return { + kind: "untracked", + patch: null, + additions: 0, + deletions: 0, + binary: false, + oversized: false, + truncated: false, + warnings, + }; + } + + let sizeBytes: number; + let buffer: Buffer | null = null; + try { + const stat = await handle.stat(); + if (!stat.isFile()) { + return { + kind: "untracked", + patch: null, + additions: 0, + deletions: 0, + binary: false, + oversized: false, + truncated: false, + warnings, + }; + } + sizeBytes = stat.size; + if (sizeBytes <= WORKSPACE_DIFF_CAPS.maxFileBytes) { + buffer = await handle.readFile(); + } + } finally { + await handle.close(); + } + if (sizeBytes > WORKSPACE_DIFF_CAPS.maxFileBytes) { warnings.push(warning("file_oversized", "Untracked file is too large to include a text patch.", input.filePath)); return { @@ -488,7 +572,18 @@ async function buildUntrackedFilePatch(input: { }; } - const buffer = await fs.readFile(path.join(input.repoRoot, input.filePath)); + if (!buffer) { + return { + kind: "untracked", + patch: null, + additions: 0, + deletions: 0, + binary: false, + oversized: false, + truncated: false, + warnings, + }; + } if (isProbablyBinary(buffer)) { warnings.push(warning("binary_file", "Binary files are summarized without a text patch.", input.filePath)); return { diff --git a/packages/plugins/plugin-workspace-diff/tests/workspace-diff.spec.ts b/packages/plugins/plugin-workspace-diff/tests/workspace-diff.spec.ts index 3976fd4328e..65bb619f4f9 100644 --- a/packages/plugins/plugin-workspace-diff/tests/workspace-diff.spec.ts +++ b/packages/plugins/plugin-workspace-diff/tests/workspace-diff.spec.ts @@ -155,6 +155,32 @@ describe("plugin workspace diff service", () => { expect(diff.warnings).toContainEqual(expect.objectContaining({ code: "file_count_truncated" })); }, 20_000); + it("does not follow untracked symlinks outside the repo", async () => { + const repoRoot = await createTempRepo(); + const outsideDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-plugin-workspace-diff-secret-")); + tempDirs.add(outsideDir); + const secretContent = "external secret should not appear\n"; + const secretPath = path.join(outsideDir, "secret.txt"); + await fs.writeFile(secretPath, secretContent, "utf8"); + await fs.symlink(secretPath, path.join(repoRoot, "leak.txt")); + + const diff = await workspaceDiffService().getDiff(createWorkspace(repoRoot), workingTreeQuery()); + const leak = diff.files.find((file) => file.path === "leak.txt"); + const serialized = JSON.stringify(diff); + + expect(leak).toMatchObject({ untracked: true, status: "untracked", additions: 0, sizeBytes: null }); + expect(leak?.patches[0]).toMatchObject({ + kind: "untracked", + patch: null, + warnings: [expect.objectContaining({ code: "symlink_target_outside_workspace" })], + }); + expect(diff.warnings).toContainEqual(expect.objectContaining({ + code: "symlink_target_outside_workspace", + path: "leak.txt", + })); + expect(serialized).not.toContain(secretContent.trim()); + }, 20_000); + it("surfaces missing cwd, non-git, invalid base refs, and unsafe path filters as plugin errors", async () => { const svc = workspaceDiffService(); await expect(svc.getDiff(createWorkspace(null), workingTreeQuery()))