Skip to content

Fix Path Traversal Vulnerability in Sandbox Path Resolution #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kolega-dev-github-app wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix Path Traversal Vulnerability in Sandbox Path Resolution #5

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 13 additions & 38 deletions src/agents/apply-patch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { AgentTool } from "@mariozechner/pi-agent-core";
import { Type } from "@sinclair/typebox";
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { applyUpdateHunk } from "./apply-patch-update.js";
import { assertSandboxPath } from "./sandbox-paths.js";
Expand All @@ -15,7 +14,6 @@ const MOVE_TO_MARKER = "*** Move to: ";
const EOF_MARKER = "*** End of File";
const CHANGE_CONTEXT_MARKER = "@@ ";
const EMPTY_CHANGE_CONTEXT_MARKER = "@@";
const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g;

type AddFileHunk = {
kind: "add";
Expand Down Expand Up @@ -228,46 +226,23 @@ async function resolvePatchPath(
};
}

const resolved = resolvePathFromCwd(filePath, options.cwd);
// When no sandbox is provided, use the current working directory as a safe default sandbox
// This prevents path traversal attacks by constraining all operations to the project directory
const effectiveSandbox = options.cwd;
const resolved = await assertSandboxPath({
filePath,
cwd: options.cwd,
root: effectiveSandbox,
});
return {
resolved,
display: toDisplayPath(resolved, options.cwd),
resolved: resolved.resolved,
display: resolved.relative || resolved.resolved,
};
}

function normalizeUnicodeSpaces(value: string): string {
return value.replace(UNICODE_SPACES, " ");
}

function expandPath(filePath: string): string {
const normalized = normalizeUnicodeSpaces(filePath);
if (normalized === "~") {
return os.homedir();
}
if (normalized.startsWith("~/")) {
return os.homedir() + normalized.slice(1);
}
return normalized;
}

function resolvePathFromCwd(filePath: string, cwd: string): string {
const expanded = expandPath(filePath);
if (path.isAbsolute(expanded)) {
return path.normalize(expanded);
}
return path.resolve(cwd, expanded);
}

function toDisplayPath(resolved: string, cwd: string): string {
const relative = path.relative(cwd, resolved);
if (!relative || relative === "") {
return path.basename(resolved);
}
if (relative.startsWith("..") || path.isAbsolute(relative)) {
return resolved;
}
return relative;
}
// Note: normalizeUnicodeSpaces, expandPath, resolvePathFromCwd, and toDisplayPath
// functions have been removed as they are now handled by the secure assertSandboxPath function
// in sandbox-paths.ts, which provides the same functionality with proper security validation.

function parsePatchText(input: string): { hunks: Hunk[]; patch: string } {
const trimmed = input.trim();
Expand Down