mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 19:28:46 +01:00
Block untracked symlink traversal in workspace diff
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
99d472f3eb
commit
e1c6cf855b
3 files changed
+128
-6
No files matched your search
@@ -26,6 +26,7 @@ export const workspaceDiffWarningCodeSchema = z.enum([
|
||||
"non_git_workspace",
|
||||
"patch_truncated",
|
||||
"path_filter_invalid",
|
||||
"symlink_target_outside_workspace",
|
||||
"workspace_path_invalid",
|
||||
]);
|
||||
|
||||
|
||||
@@ -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<ReturnType<typeof fs.open>>;
|
||||
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<WorkspaceDiffFilePatch> {
|
||||
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<ReturnType<typeof fs.open>>;
|
||||
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 {
|
||||
|
||||
@@ -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()))
|
||||
|
||||
Reference in new issue
Block a user