refactor: remove workspace path sandbox and shell gate
- Replace resolvePathInWorkspace with simple resolvePath (no boundary check) - Remove UWF_BUILTIN_ALLOW_SHELL env gate from run_command - Update tests accordingly Per review: sandbox was false security with shell=true, and path restrictions are unnecessary for a trusted agent environment.
This commit is contained in:
@@ -1,17 +1,21 @@
|
|||||||
import { describe, expect, test } from "bun:test";
|
import { describe, expect, test } from "bun:test";
|
||||||
import { join } from "node:path";
|
import { resolvePath } from "../src/tools/path.js";
|
||||||
|
import { resolve } from "node:path";
|
||||||
|
|
||||||
import { resolvePathInWorkspace } from "../src/tools/path.js";
|
describe("resolvePath", () => {
|
||||||
|
test("resolves relative paths against cwd", () => {
|
||||||
describe("resolvePathInWorkspace", () => {
|
const root = "/workspace/project";
|
||||||
const root = join("/tmp", "uwf-workspace");
|
const resolved = resolvePath(root, "src/foo.ts");
|
||||||
|
expect(resolved).toBe(resolve(root, "src/foo.ts"));
|
||||||
test("resolves relative paths inside root", () => {
|
|
||||||
const resolved = resolvePathInWorkspace(root, "src/foo.ts");
|
|
||||||
expect(resolved).toBe(join(root, "src/foo.ts"));
|
|
||||||
});
|
});
|
||||||
|
|
||||||
test("rejects parent traversal", () => {
|
test("resolves absolute paths as-is", () => {
|
||||||
expect(resolvePathInWorkspace(root, "../etc/passwd")).toBeNull();
|
const resolved = resolvePath("/workspace", "/etc/hosts");
|
||||||
|
expect(resolved).toBe("/etc/hosts");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("resolves parent traversal normally", () => {
|
||||||
|
const resolved = resolvePath("/workspace/project", "../other/file.ts");
|
||||||
|
expect(resolved).toBe(resolve("/workspace/project", "../other/file.ts"));
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ import { runCommandTool } from "./run-command.js";
|
|||||||
import type { BuiltinTool, ToolContext } from "./types.js";
|
import type { BuiltinTool, ToolContext } from "./types.js";
|
||||||
import { writeFileTool } from "./write-file.js";
|
import { writeFileTool } from "./write-file.js";
|
||||||
|
|
||||||
export { resolvePathInWorkspace } from "./path.js";
|
export { resolvePath } from "./path.js";
|
||||||
export type { BuiltinTool, ToolContext } from "./types.js";
|
export type { BuiltinTool, ToolContext } from "./types.js";
|
||||||
|
|
||||||
const BUILTIN_TOOLS: BuiltinTool[] = [readFileTool, writeFileTool, runCommandTool];
|
const BUILTIN_TOOLS: BuiltinTool[] = [readFileTool, writeFileTool, runCommandTool];
|
||||||
|
|||||||
@@ -1,12 +1,6 @@
|
|||||||
import { isAbsolute, relative, resolve } from "node:path";
|
import { resolve } from "node:path";
|
||||||
|
|
||||||
/** Reject paths that escape the workspace root via `..` segments. */
|
/** Resolve a path relative to the working directory. */
|
||||||
export function resolvePathInWorkspace(cwd: string, inputPath: string): string | null {
|
export function resolvePath(cwd: string, inputPath: string): string {
|
||||||
const root = resolve(cwd);
|
return resolve(cwd, inputPath);
|
||||||
const target = resolve(root, inputPath);
|
|
||||||
const rel = relative(root, target);
|
|
||||||
if (rel.startsWith("..") || isAbsolute(rel)) {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
return target;
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { readFile, stat } from "node:fs/promises";
|
import { readFile, stat } from "node:fs/promises";
|
||||||
import { resolvePathInWorkspace } from "./path.js";
|
import { resolvePath } from "./path.js";
|
||||||
import type { BuiltinTool } from "./types.js";
|
import type { BuiltinTool } from "./types.js";
|
||||||
|
|
||||||
const MAX_READ_BYTES = 512 * 1024;
|
const MAX_READ_BYTES = 512 * 1024;
|
||||||
@@ -23,10 +23,7 @@ export const readFileTool: BuiltinTool = {
|
|||||||
if (!isRecord(args) || typeof args.path !== "string") {
|
if (!isRecord(args) || typeof args.path !== "string") {
|
||||||
return "Error: path must be a string";
|
return "Error: path must be a string";
|
||||||
}
|
}
|
||||||
const resolved = resolvePathInWorkspace(ctx.cwd, args.path);
|
const resolved = resolvePath(ctx.cwd, args.path);
|
||||||
if (resolved === null) {
|
|
||||||
return "Error: path escapes workspace root";
|
|
||||||
}
|
|
||||||
try {
|
try {
|
||||||
const info = await stat(resolved);
|
const info = await stat(resolved);
|
||||||
if (!info.isFile()) {
|
if (!info.isFile()) {
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { spawn } from "node:child_process";
|
import { spawn } from "node:child_process";
|
||||||
import { resolvePathInWorkspace } from "./path.js";
|
import { resolvePath } from "./path.js";
|
||||||
import type { BuiltinTool } from "./types.js";
|
import type { BuiltinTool } from "./types.js";
|
||||||
|
|
||||||
const COMMAND_TIMEOUT_MS = 60_000;
|
const COMMAND_TIMEOUT_MS = 60_000;
|
||||||
@@ -57,7 +57,7 @@ function runShell(
|
|||||||
export const runCommandTool: BuiltinTool = {
|
export const runCommandTool: BuiltinTool = {
|
||||||
name: "run_command",
|
name: "run_command",
|
||||||
description:
|
description:
|
||||||
"Run a shell command in the workspace. Requires UWF_BUILTIN_ALLOW_SHELL=1. Output is truncated.",
|
"Run a shell command. Output is truncated to 32KB.",
|
||||||
parameters: {
|
parameters: {
|
||||||
type: "object",
|
type: "object",
|
||||||
required: ["command"],
|
required: ["command"],
|
||||||
@@ -71,9 +71,6 @@ export const runCommandTool: BuiltinTool = {
|
|||||||
additionalProperties: false,
|
additionalProperties: false,
|
||||||
},
|
},
|
||||||
execute: async (args, ctx) => {
|
execute: async (args, ctx) => {
|
||||||
if (process.env.UWF_BUILTIN_ALLOW_SHELL !== "1") {
|
|
||||||
return "Error: run_command disabled. Set UWF_BUILTIN_ALLOW_SHELL=1 to enable.";
|
|
||||||
}
|
|
||||||
if (!isRecord(args) || typeof args.command !== "string") {
|
if (!isRecord(args) || typeof args.command !== "string") {
|
||||||
return "Error: command must be a string";
|
return "Error: command must be a string";
|
||||||
}
|
}
|
||||||
@@ -82,11 +79,7 @@ export const runCommandTool: BuiltinTool = {
|
|||||||
if (typeof args.cwd !== "string") {
|
if (typeof args.cwd !== "string") {
|
||||||
return "Error: cwd must be a string";
|
return "Error: cwd must be a string";
|
||||||
}
|
}
|
||||||
const resolved = resolvePathInWorkspace(ctx.cwd, args.cwd);
|
workDir = resolvePath(ctx.cwd, args.cwd);
|
||||||
if (resolved === null) {
|
|
||||||
return "Error: cwd escapes workspace root";
|
|
||||||
}
|
|
||||||
workDir = resolved;
|
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
const { stdout, stderr, code } = await runShell(args.command, workDir);
|
const { stdout, stderr, code } = await runShell(args.command, workDir);
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import { mkdir, writeFile } from "node:fs/promises";
|
import { mkdir, writeFile } from "node:fs/promises";
|
||||||
import { dirname } from "node:path";
|
import { dirname } from "node:path";
|
||||||
import { resolvePathInWorkspace } from "./path.js";
|
import { resolvePath } from "./path.js";
|
||||||
import type { BuiltinTool } from "./types.js";
|
import type { BuiltinTool } from "./types.js";
|
||||||
|
|
||||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||||
@@ -23,10 +23,7 @@ export const writeFileTool: BuiltinTool = {
|
|||||||
if (!isRecord(args) || typeof args.path !== "string" || typeof args.content !== "string") {
|
if (!isRecord(args) || typeof args.path !== "string" || typeof args.content !== "string") {
|
||||||
return "Error: path and content must be strings";
|
return "Error: path and content must be strings";
|
||||||
}
|
}
|
||||||
const resolved = resolvePathInWorkspace(ctx.cwd, args.path);
|
const resolved = resolvePath(ctx.cwd, args.path);
|
||||||
if (resolved === null) {
|
|
||||||
return "Error: path escapes workspace root";
|
|
||||||
}
|
|
||||||
try {
|
try {
|
||||||
await mkdir(dirname(resolved), { recursive: true });
|
await mkdir(dirname(resolved), { recursive: true });
|
||||||
await writeFile(resolved, args.content, "utf8");
|
await writeFile(resolved, args.content, "utf8");
|
||||||
|
|||||||
Reference in New Issue
Block a user