ShockVPN/shockbot
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

relax filesystem permissions: reads allow temp dir, writes conditional on bash (#308)

Browse Source 
reads (file_read, list_directory) now allow paths within the repo OR
PULLFROG_TEMP_DIR. this fixes the bug where agents with full permissions
couldn't read PR diffs, CI logs, review threads, or background bash
output because those files live under /tmp/pullfrog-xxx/.

writes (file_write, file_edit, file_delete) now only enforce repo-scoping
when bash !== "enabled". when bash=enabled the agent can write anywhere
via native bash, so restricting file_write was security theater. .git/
stays blocked in all modes as defense-in-depth.

replaced the conflated resolveAndValidatePath/validateWritePath helpers
with separate resolveReadPath and resolveWritePath functions that cleanly
separate path resolution from permission enforcement.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Colin McDonnell
2026-02-14 04:02:45 +00:00
committed by pullfrog[bot]
parent b80c78bdbe
commit 9a1f3bdb0a
2 changed files with 158 additions and 135 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mcp/file.ts
+103 -88
View File
@@ -9,6 +9,7 @@ import {
} from "node:fs";
import { dirname, resolve } from "node:path";
import { type } from "arktype";
import type { BashPermission } from "../external.ts";
import type { ToolContext } from "./server.ts";
import { execute, tool } from "./shared.ts";
@@ -38,59 +39,6 @@ export const ListDirectoryParams = type({
path: "string",
});
// resolve path and validate it is within the repository.
// uses realpathSync to follow symlinks and prevent symlink-based escapes.
//
// threat model: the primary scenario where symlink protection matters is a
// malicious PR that plants symlinks in the repo (e.g. `secrets -> /etc/shadow`).
// git materializes symlinks on linux, so after checkout the working tree contains
// live symlinks. without realpathSync, file_read("secrets") would leak host files.
//
// when bash is enabled the agent can already `cat /etc/shadow` directly, so the
// symlink check is defense-in-depth for that case. the check is most meaningful
// when bash is disabled — the agent's only filesystem access is through these MCP
// tools, and pre-planted symlinks become the sole escape vector.
//
// the path traversal check (resolve + startsWith) is always useful regardless of
// bash — it blocks `../../etc/passwd` style escapes on every configuration.
function resolveAndValidatePath(filePath: string): string {
const cwd = realpathSync(process.cwd());
const resolved = resolve(cwd, filePath);
// if the target exists, resolve symlinks to get the real location
if (existsSync(resolved)) {
const real = realpathSync(resolved);
if (real !== cwd && !real.startsWith(cwd + "/")) {
throw new Error(`path must be within the repository (symlink escape blocked): ${filePath}`);
}
return real;
}
// target doesn't exist yet (common for writes) — walk up to find
// the first existing ancestor and verify it resolves within the repo.
// this prevents creating files through symlinked parent directories.
let ancestor = dirname(resolved);
while (!existsSync(ancestor)) {
const parent = dirname(ancestor);
if (parent === ancestor) break; // reached filesystem root
ancestor = parent;
}
if (existsSync(ancestor)) {
const realAncestor = realpathSync(ancestor);
if (realAncestor !== cwd && !realAncestor.startsWith(cwd + "/")) {
throw new Error(`path must be within the repository (symlink escape blocked): ${filePath}`);
}
}
// also verify the resolved path (without symlink resolution) is within cwd
if (resolved !== cwd && !resolved.startsWith(cwd + "/")) {
throw new Error(`path must be within the repository: ${filePath}`);
}
return resolved;
}
// SECURITY: files that git interprets and can trigger code execution.
// .gitattributes can define filter drivers (clean/smudge) that execute arbitrary commands.
// .gitmodules can reference malicious submodule URLs that execute code on update.
@@ -98,27 +46,92 @@ function resolveAndValidatePath(filePath: string): string {
// and could write these files via shell, so blocking via MCP is redundant.
const GIT_INTERPRETED_FILES = [".gitattributes", ".gitmodules"];
type BashPermission = "disabled" | "restricted" | "enabled";
type ValidateWritePathParams = {
filePath: string;
bashPermission: BashPermission;
};
function validateWritePath(params: ValidateWritePathParams): string {
const resolved = resolveAndValidatePath(params.filePath);
// resolve and validate a read path. allows:
// 1. paths within the repo (with symlink protection to prevent malicious PR symlinks)
// 2. paths within PULLFROG_TEMP_DIR (tool result files: diffs, CI logs, review threads, etc.)
function resolveReadPath(filePath: string): string {
const cwd = realpathSync(process.cwd());
const relative = resolved.slice(cwd.length + 1);
if (relative === ".git" || relative.startsWith(".git/")) {
throw new Error(`writing to .git is not allowed: ${params.filePath}`);
const resolved = resolve(cwd, filePath);
// allow reads from PULLFROG_TEMP_DIR (tool result files)
const tempDir = process.env.PULLFROG_TEMP_DIR;
if (tempDir && (resolved === tempDir || resolved.startsWith(tempDir + "/"))) {
return resolved;
}
// block git-interpreted files only when bash is disabled (no shell = no other way to create them)
if (params.bashPermission === "disabled") {
const basename = relative.split("/").pop() || "";
// allow reads from the repo with symlink protection.
// threat model: a malicious PR plants symlinks (e.g. `secrets -> /etc/shadow`).
// git materializes symlinks on linux, so after checkout the working tree contains
// live symlinks. realpathSync catches these and blocks the read.
if (existsSync(resolved)) {
const real = realpathSync(resolved);
if (real === cwd || real.startsWith(cwd + "/")) {
return real;
}
throw new Error(`path must be within the repository (symlink escape blocked): ${filePath}`);
}
// path doesn't exist — check if it's within the repo
if (resolved === cwd || resolved.startsWith(cwd + "/")) {
return resolved;
}
throw new Error(`path must be within the repository or temp directory: ${filePath}`);
}
// resolve and validate a write path. enforces:
// - repo-scoping with symlink protection (when bash !== "enabled")
// - .git/ always blocked (defense-in-depth)
// - .gitattributes/.gitmodules blocked when bash === "disabled"
//
// when bash=enabled, repo-scoping is dropped — the agent can write anywhere via native
// bash, so restricting file_write to the repo would be security theater.
function resolveWritePath(filePath: string, bashPermission: BashPermission): string {
const cwd = realpathSync(process.cwd());
const resolved = resolve(cwd, filePath);
// repo-scoping: enforced when agent doesn't have full bash
if (bashPermission !== "enabled") {
if (existsSync(resolved)) {
const real = realpathSync(resolved);
if (real !== cwd && !real.startsWith(cwd + "/")) {
throw new Error(`path must be within the repository (symlink escape blocked): ${filePath}`);
}
} else {
// target doesn't exist yet — walk up to find the first existing ancestor
// and verify it resolves within the repo. prevents creating files through
// symlinked parent directories.
let ancestor = dirname(resolved);
while (!existsSync(ancestor)) {
const parent = dirname(ancestor);
if (parent === ancestor) break;
ancestor = parent;
}
if (existsSync(ancestor)) {
const realAncestor = realpathSync(ancestor);
if (realAncestor !== cwd && !realAncestor.startsWith(cwd + "/")) {
throw new Error(
`path must be within the repository (symlink escape blocked): ${filePath}`
);
}
}
if (resolved !== cwd && !resolved.startsWith(cwd + "/")) {
throw new Error(`path must be within the repository: ${filePath}`);
}
}
}
// .git always blocked anywhere in the path (defense-in-depth even with bash=enabled)
if (resolved.includes("/.git/") || resolved.endsWith("/.git")) {
throw new Error(`writing to .git is not allowed: ${filePath}`);
}
// git-interpreted files blocked anywhere in the path when bash is disabled
if (bashPermission === "disabled") {
const basename = resolved.split("/").pop() || "";
if (GIT_INTERPRETED_FILES.includes(basename)) {
throw new Error(
`writing to ${basename} is not allowed when bash is ${params.bashPermission} (can trigger code execution via git filter drivers): ${params.filePath}`
`writing to ${basename} is not allowed when bash is ${bashPermission} (can trigger code execution via git filter drivers): ${filePath}`
);
}
}
@@ -129,10 +142,12 @@ function validateWritePath(params: ValidateWritePathParams): string {
export function FileReadTool(_ctx: ToolContext) {
return tool({
name: "file_read",
description: `Read a file from the repository. Path is relative to the repository root. Only paths within the current repository are allowed.`,
description:
"Read a file. Path is relative to the repository root, or an absolute path " +
"to read tool result files (diffs, CI logs, etc.) from the temp directory.",
parameters: FileReadParams,
execute: execute(async (params) => {
const resolved = resolveAndValidatePath(params.path);
const resolved = resolveReadPath(params.path);
const raw = readFileSync(resolved, "utf-8");
const lines = raw.split("\n");
@@ -156,13 +171,12 @@ export function FileReadTool(_ctx: ToolContext) {
export function FileWriteTool(ctx: ToolContext) {
return tool({
name: "file_write",
description: `Write content to a file in the repository. Path is relative to the repository root. Only paths within the current repository are allowed. Writes to .git/ are blocked. Creates parent directories if needed.`,
description:
"Write content to a file. Path is relative to the repository root. " +
"Writes to .git/ are blocked. Creates parent directories if needed.",
parameters: FileWriteParams,
execute: execute(async (params) => {
const resolved = validateWritePath({
filePath: params.path,
bashPermission: ctx.payload.bash,
});
const resolved = resolveWritePath(params.path, ctx.payload.bash);
const dir = dirname(resolved);
mkdirSync(dir, { recursive: true });
writeFileSync(resolved, params.content, "utf-8");
@@ -174,7 +188,10 @@ export function FileWriteTool(ctx: ToolContext) {
export function FileEditTool(ctx: ToolContext) {
return tool({
name: "file_edit",
description: `Replace text in a file. old_string must match exactly (including whitespace and indentation). By default replaces a single unique occurrence — set replace_all to replace every occurrence. Path is relative to the repository root. Writes to .git/ are blocked.`,
description:
"Replace text in a file. old_string must match exactly (including whitespace and indentation). " +
"By default replaces a single unique occurrence — set replace_all to replace every occurrence. " +
"Path is relative to the repository root. Writes to .git/ are blocked.",
parameters: FileEditParams,
execute: execute(async (params) => {
if (params.old_string.length === 0) {
@@ -184,10 +201,7 @@ export function FileEditTool(ctx: ToolContext) {
throw new Error("old_string and new_string are identical");
}
const resolved = validateWritePath({
filePath: params.path,
bashPermission: ctx.payload.bash,
});
const resolved = resolveWritePath(params.path, ctx.payload.bash);
const content = readFileSync(resolved, "utf-8");
const count = content.split(params.old_string).length - 1;
@@ -213,13 +227,12 @@ export function FileEditTool(ctx: ToolContext) {
export function FileDeleteTool(ctx: ToolContext) {
return tool({
name: "file_delete",
description: `Delete a file from the repository. Path is relative to the repository root. Only paths within the current repository are allowed. Deletes to .git/ are blocked. Cannot delete directories.`,
description:
"Delete a file. Path is relative to the repository root. " +
"Deletes to .git/ are blocked. Cannot delete directories.",
parameters: FileDeleteParams,
execute: execute(async (params) => {
const resolved = validateWritePath({
filePath: params.path,
bashPermission: ctx.payload.bash,
});
const resolved = resolveWritePath(params.path, ctx.payload.bash);
unlinkSync(resolved);
return { path: params.path, deleted: true };
}),
@@ -229,10 +242,12 @@ export function FileDeleteTool(ctx: ToolContext) {
export function ListDirectoryTool(_ctx: ToolContext) {
return tool({
name: "list_directory",
description: `List files and directories. Path is relative to the repository root. Only paths within the current repository are allowed. Returns entries sorted with directories first, then alphabetically.`,
description:
"List files and directories. Path is relative to the repository root, or an absolute path " +
"to list tool result files from the temp directory. Returns entries sorted with directories first, then alphabetically.",
parameters: ListDirectoryParams,
execute: execute(async (params) => {
const resolved = resolveAndValidatePath(params.path);
const resolved = resolveReadPath(params.path);
const entries = readdirSync(resolved, { withFileTypes: true });
const sorted = entries.sort((a, b) => {
if (a.isDirectory() && !b.isDirectory()) return -1;