LLM/moltbot
1
0
Fork 0
mirror of https://github.com/moltbot/moltbot.git synced 2026-03-07 22:44:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): default apply_patch workspace containment

Browse Source
This commit is contained in:
Peter Steinberger
2026-02-15 01:21:07 +01:00
parent 68c78c4b43
commit 4a44da7d91
9 changed files with 191 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
docs/gateway/security/index.md
View File

@@ -579,7 +579,7 @@ We may add a single `readOnlyMode` flag later to simplify this configuration.
Additional hardening options:
- `tools.exec.applyPatch.workspaceOnly: true` (recommended): ensures `apply_patch` cannot write/delete outside the workspace directory even when sandboxing is off.
- `tools.exec.applyPatch.workspaceOnly: true` (default): ensures `apply_patch` cannot write/delete outside the workspace directory even when sandboxing is off. Set to `false` only if you intentionally want `apply_patch` to touch files outside the workspace.
- `tools.fs.workspaceOnly: true` (optional): restricts `read`/`write`/`edit`/`apply_patch` paths to the workspace directory (useful if you allow absolute paths today and want a single guardrail).
### 5) Secure baseline (copy/paste)

2
docs/tools/apply-patch.md
View File

@@ -33,7 +33,7 @@ The tool accepts a single `input` string that wraps one or more file operations:
## Notes
- Patch paths support relative paths (from the workspace directory) and absolute paths.
- Optional: set `tools.exec.applyPatch.workspaceOnly: true` to restrict patch paths to the workspace directory (recommended when untrusted users can trigger tool execution).
- `tools.exec.applyPatch.workspaceOnly` defaults to `true` (workspace-contained). Set it to `false` only if you intentionally want `apply_patch` to write/delete outside the workspace directory.
- Use `*** Move to:` within an `*** Update File:` hunk to rename files.
- `*** End of File` marks an EOF-only insert when needed.
- Experimental and disabled by default. Enable with `tools.exec.applyPatch.enabled`.

2
docs/tools/exec.md
View File

@@ -178,4 +178,4 @@ Notes:
- Only available for OpenAI/OpenAI Codex models.
- Tool policy still applies; `allow: ["exec"]` implicitly allows `apply_patch`.
- Config lives under `tools.exec.applyPatch`.
- Optional: set `tools.exec.applyPatch.workspaceOnly: true` to restrict patch paths to the workspace directory (recommended when untrusted users can trigger tool execution).
- `tools.exec.applyPatch.workspaceOnly` defaults to `true` (workspace-contained). Set it to `false` only if you intentionally want `apply_patch` to write/delete outside the workspace directory.

2
docs/tools/index.md
View File

@@ -181,7 +181,7 @@ Optional plugin tools:
Apply structured patches across one or more files. Use for multi-hunk edits.
Experimental: enable via `tools.exec.applyPatch.enabled` (OpenAI models only).
Optional: restrict patch paths to the workspace directory with `tools.exec.applyPatch.workspaceOnly: true`.
`tools.exec.applyPatch.workspaceOnly` defaults to `true` (workspace-contained). Set it to `false` only if you intentionally want `apply_patch` to write/delete outside the workspace directory.
### `exec`

88
src/agents/apply-patch.e2e.test.ts
View File

@@ -71,9 +71,12 @@ describe("applyPatch", () => {
});
});
it("rejects path traversal outside cwd", async () => {
it("rejects path traversal outside cwd by default", async () => {
await withTempDir(async (dir) => {
const escapedPath = path.join(path.dirname(dir), "escaped.txt");
const escapedPath = path.join(
path.dirname(dir),
`escaped-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}.txt`,
);
const relativeEscape = path.relative(dir, escapedPath);
const patch = `*** Begin Patch
@@ -81,14 +84,16 @@ describe("applyPatch", () => {
+escaped
*** End Patch`;
await expect(applyPatch(patch, { cwd: dir, workspaceOnly: true })).rejects.toThrow(
/Path escapes sandbox root/,
);
await expect(fs.readFile(escapedPath, "utf8")).rejects.toBeDefined();
try {
await expect(applyPatch(patch, { cwd: dir })).rejects.toThrow(/Path escapes sandbox root/);
await expect(fs.readFile(escapedPath, "utf8")).rejects.toBeDefined();
} finally {
await fs.rm(escapedPath, { force: true });
}
});
});
it("rejects absolute paths outside cwd", async () => {
it("rejects absolute paths outside cwd by default", async () => {
await withTempDir(async (dir) => {
const escapedPath = path.join(os.tmpdir(), `openclaw-apply-patch-${Date.now()}.txt`);
@@ -98,9 +103,7 @@ describe("applyPatch", () => {
*** End Patch`;
try {
await expect(applyPatch(patch, { cwd: dir, workspaceOnly: true })).rejects.toThrow(
/Path escapes sandbox root/,
);
await expect(applyPatch(patch, { cwd: dir })).rejects.toThrow(/Path escapes sandbox root/);
await expect(fs.readFile(escapedPath, "utf8")).rejects.toBeDefined();
} finally {
await fs.rm(escapedPath, { force: true });
@@ -108,7 +111,7 @@ describe("applyPatch", () => {
});
});
it("allows absolute paths within cwd", async () => {
it("allows absolute paths within cwd by default", async () => {
await withTempDir(async (dir) => {
const target = path.join(dir, "nested", "inside.txt");
const patch = `*** Begin Patch
@@ -116,13 +119,13 @@ describe("applyPatch", () => {
+inside
*** End Patch`;
await applyPatch(patch, { cwd: dir, workspaceOnly: true });
await applyPatch(patch, { cwd: dir });
const contents = await fs.readFile(target, "utf8");
expect(contents).toBe("inside\n");
});
});
it("rejects symlink escape attempts", async () => {
it("rejects symlink escape attempts by default", async () => {
await withTempDir(async (dir) => {
const outside = path.join(path.dirname(dir), "outside-target.txt");
const linkPath = path.join(dir, "link.txt");
@@ -136,16 +139,14 @@ describe("applyPatch", () => {
+pwned
*** End Patch`;
await expect(applyPatch(patch, { cwd: dir, workspaceOnly: true })).rejects.toThrow(
/Symlink escapes sandbox root/,
);
await expect(applyPatch(patch, { cwd: dir })).rejects.toThrow(/Symlink escapes sandbox root/);
const outsideContents = await fs.readFile(outside, "utf8");
expect(outsideContents).toBe("initial\n");
await fs.rm(outside, { force: true });
});
});
it("allows symlinks that resolve within cwd", async () => {
it("allows symlinks that resolve within cwd by default", async () => {
await withTempDir(async (dir) => {
const target = path.join(dir, "target.txt");
const linkPath = path.join(dir, "link.txt");
@@ -159,9 +160,60 @@ describe("applyPatch", () => {
+updated
*** End Patch`;
await applyPatch(patch, { cwd: dir, workspaceOnly: true });
await applyPatch(patch, { cwd: dir });
const contents = await fs.readFile(target, "utf8");
expect(contents).toBe("updated\n");
});
});
it("rejects delete path traversal via symlink directories by default", async () => {
await withTempDir(async (dir) => {
const outsideDir = path.join(path.dirname(dir), `outside-dir-${process.pid}-${Date.now()}`);
const outsideFile = path.join(outsideDir, "victim.txt");
await fs.mkdir(outsideDir, { recursive: true });
await fs.writeFile(outsideFile, "victim\n", "utf8");
const linkDir = path.join(dir, "linkdir");
await fs.symlink(outsideDir, linkDir);
const patch = `*** Begin Patch
*** Delete File: linkdir/victim.txt
*** End Patch`;
try {
await expect(applyPatch(patch, { cwd: dir })).rejects.toThrow(
/Symlink escapes sandbox root/,
);
const stillThere = await fs.readFile(outsideFile, "utf8");
expect(stillThere).toBe("victim\n");
} finally {
await fs.rm(outsideFile, { force: true });
await fs.rm(outsideDir, { recursive: true, force: true });
}
});
});
it("allows path traversal when workspaceOnly is explicitly disabled", async () => {
await withTempDir(async (dir) => {
const escapedPath = path.join(
path.dirname(dir),
`escaped-allow-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}.txt`,
);
const relativeEscape = path.relative(dir, escapedPath);
const patch = `*** Begin Patch
*** Add File: ${relativeEscape}
+escaped
*** End Patch`;
try {
const result = await applyPatch(patch, { cwd: dir, workspaceOnly: false });
expect(result.summary.added.length).toBe(1);
const contents = await fs.readFile(escapedPath, "utf8");
expect(contents).toBe("escaped\n");
} finally {
await fs.rm(escapedPath, { force: true });
}
});
});
});

28
src/agents/apply-patch.ts
View File

@@ -5,7 +5,7 @@ import os from "node:os";
import path from "node:path";
import type { SandboxFsBridge } from "./sandbox/fs-bridge.js";
import { applyUpdateHunk } from "./apply-patch-update.js";
import { assertSandboxPath, resolveSandboxPath } from "./sandbox-paths.js";
import { assertSandboxPath } from "./sandbox-paths.js";
const BEGIN_PATCH_MARKER = "*** Begin Patch";
const END_PATCH_MARKER = "*** End Patch";
@@ -67,7 +67,7 @@ type SandboxApplyPatchConfig = {
type ApplyPatchOptions = {
cwd: string;
sandbox?: SandboxApplyPatchConfig;
/** When true, restrict patch paths to the workspace root (cwd). Default: false. */
/** Restrict patch paths to the workspace root (cwd). Default: true. Set false to opt out. */
workspaceOnly?: boolean;
signal?: AbortSignal;
};
@@ -83,7 +83,7 @@ export function createApplyPatchTool(
): AgentTool<typeof applyPatchSchema, ApplyPatchToolDetails> {
const cwd = options.cwd ?? process.cwd();
const sandbox = options.sandbox;
const workspaceOnly = options.workspaceOnly === true;
const workspaceOnly = options.workspaceOnly !== false;
return {
name: "apply_patch",
@@ -155,7 +155,7 @@ export async function applyPatch(
}
if (hunk.kind === "delete") {
const target = await resolvePatchPath(hunk.path, options, "unlink");
const target = await resolvePatchPath(hunk.path, options);
await fileOps.remove(target.resolved);
recordSummary(summary, seen, "deleted", target.display);
continue;
@@ -254,7 +254,6 @@ async function ensureDir(filePath: string, ops: PatchFileOps) {
async function resolvePatchPath(
filePath: string,
options: ApplyPatchOptions,
purpose: "readWrite" | "unlink" = "readWrite",
): Promise<{ resolved: string; display: string }> {
if (options.sandbox) {
const resolved = options.sandbox.bridge.resolvePath({
@@ -267,16 +266,15 @@ async function resolvePatchPath(
};
}
const resolved = options.workspaceOnly
? purpose === "unlink"
? resolveSandboxPath({ filePath, cwd: options.cwd, root: options.cwd }).resolved
: (
await assertSandboxPath({
filePath,
cwd: options.cwd,
root: options.cwd,
})
).resolved
const workspaceOnly = options.workspaceOnly !== false;
const resolved = workspaceOnly
? (
await assertSandboxPath({
filePath,
cwd: options.cwd,
root: options.cwd,
})
).resolved
: resolvePathFromCwd(filePath, options.cwd);
return {
resolved,

100
src/agents/pi-tools-agent-config.e2e.test.ts
View File

@@ -1,3 +1,6 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { describe, expect, it } from "vitest";
import "./test-helpers/fast-coding-tools.js";
import type { OpenClawConfig } from "../config/config.js";
@@ -5,6 +8,10 @@ import type { SandboxDockerConfig } from "./sandbox.js";
import type { SandboxFsBridge } from "./sandbox/fs-bridge.js";
import { createOpenClawCodingTools } from "./pi-tools.js";
type ToolWithExecute = {
execute: (toolCallId: string, args: unknown, signal?: AbortSignal) => Promise<unknown>;
};
describe("Agent-specific tool filtering", () => {
const sandboxFsBridgeStub: SandboxFsBridge = {
resolvePath: () => ({
@@ -110,6 +117,99 @@ describe("Agent-specific tool filtering", () => {
expect(toolNames).toContain("apply_patch");
});
it("defaults apply_patch to workspace-only (blocks traversal)", async () => {
const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-pi-tools-"));
const escapedPath = path.join(
path.dirname(workspaceDir),
`escaped-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}.txt`,
);
const relativeEscape = path.relative(workspaceDir, escapedPath);
try {
const cfg: OpenClawConfig = {
tools: {
allow: ["read", "exec"],
exec: {
applyPatch: { enabled: true },
},
},
};
const tools = createOpenClawCodingTools({
config: cfg,
sessionKey: "agent:main:main",
workspaceDir,
agentDir: "/tmp/agent",
modelProvider: "openai",
modelId: "gpt-5.2",
});
const applyPatchTool = tools.find((t) => t.name === "apply_patch");
if (!applyPatchTool) {
throw new Error("apply_patch tool missing");
}
const patch = `*** Begin Patch
*** Add File: ${relativeEscape}
+escaped
*** End Patch`;
await expect(
(applyPatchTool as unknown as ToolWithExecute).execute("tc1", { input: patch }),
).rejects.toThrow(/Path escapes sandbox root/);
await expect(fs.readFile(escapedPath, "utf8")).rejects.toBeDefined();
} finally {
await fs.rm(escapedPath, { force: true });
await fs.rm(workspaceDir, { recursive: true, force: true });
}
});
it("allows disabling apply_patch workspace-only via config (dangerous)", async () => {
const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-pi-tools-"));
const escapedPath = path.join(
path.dirname(workspaceDir),
`escaped-allow-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}.txt`,
);
const relativeEscape = path.relative(workspaceDir, escapedPath);
try {
const cfg: OpenClawConfig = {
tools: {
allow: ["read", "exec"],
exec: {
applyPatch: { enabled: true, workspaceOnly: false },
},
},
};
const tools = createOpenClawCodingTools({
config: cfg,
sessionKey: "agent:main:main",
workspaceDir,
agentDir: "/tmp/agent",
modelProvider: "openai",
modelId: "gpt-5.2",
});
const applyPatchTool = tools.find((t) => t.name === "apply_patch");
if (!applyPatchTool) {
throw new Error("apply_patch tool missing");
}
const patch = `*** Begin Patch
*** Add File: ${relativeEscape}
+escaped
*** End Patch`;
await (applyPatchTool as unknown as ToolWithExecute).execute("tc2", { input: patch });
const contents = await fs.readFile(escapedPath, "utf8");
expect(contents).toBe("escaped\n");
} finally {
await fs.rm(escapedPath, { force: true });
await fs.rm(workspaceDir, { recursive: true, force: true });
}
});
it("should apply agent-specific tool policy", () => {
const cfg: OpenClawConfig = {
tools: {

4
src/agents/pi-tools.ts
View File

@@ -256,7 +256,9 @@ export function createOpenClawCodingTools(options?: {
const workspaceRoot = options?.workspaceDir ?? process.cwd();
const workspaceOnly = fsConfig.workspaceOnly === true;
const applyPatchConfig = execConfig.applyPatch;
const applyPatchWorkspaceOnly = workspaceOnly || applyPatchConfig?.workspaceOnly === true;
// Secure by default: apply_patch is workspace-contained unless explicitly disabled.
// (tools.fs.workspaceOnly is a separate umbrella flag for read/write/edit/apply_patch.)
const applyPatchWorkspaceOnly = workspaceOnly || applyPatchConfig?.workspaceOnly !== false;
const applyPatchEnabled =
!!applyPatchConfig?.enabled &&
isOpenAIProvider(options?.modelProvider) &&

2
src/config/schema.help.ts
View File

@@ -59,7 +59,7 @@ export const FIELD_HELP: Record<string, string> = {
"tools.exec.applyPatch.enabled":
"Experimental. Enables apply_patch for OpenAI models when allowed by tool policy.",
"tools.exec.applyPatch.workspaceOnly":
"Restrict apply_patch paths to the workspace directory (default: false).",
"Restrict apply_patch paths to the workspace directory (default: true). Set false to allow writing outside the workspace (dangerous).",
"tools.exec.applyPatch.allowModels":
'Optional allowlist of model ids (e.g. "gpt-5.2" or "openai/gpt-5.2").',
"tools.exec.notifyOnExit":