mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-08 06:54:24 +00:00
security: block apply_patch path traversal outside workspace (#16405)
Merged via /review-pr -> /prepare-pr -> /merge-pr.
Prepared head SHA: 0fcd3f8c3a
Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com>
Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com>
Reviewed-by: @mbelinky
This commit is contained in:
@@ -45,6 +45,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Security/BlueBubbles: reject ambiguous shared-path webhook routing when multiple webhook targets match the same guid/password.
|
||||
- Security/BlueBubbles: harden BlueBubbles webhook auth behind reverse proxies by only accepting passwordless webhooks for direct localhost loopback requests (forwarded/proxied requests now require a password). Thanks @simecek.
|
||||
- Security/BlueBubbles: require explicit `mediaLocalRoots` allowlists for local outbound media path reads to prevent local file disclosure. (#16322) Thanks @mbelinky.
|
||||
- Security/Agents: enforce workspace-root path bounds for `apply_patch` in non-sandbox mode to block traversal and symlink escape writes. Thanks @p80n-sec.
|
||||
- Cron/Slack: preserve agent identity (name and icon) when cron jobs deliver outbound messages. (#16242) Thanks @robbyczgw-cla.
|
||||
- Cron: deliver text-only output directly when `delivery.to` is set so cron recipients get full output instead of summaries. (#16360) Thanks @rubyrunsstuff.
|
||||
- Cron: repair missing/corrupt `nextRunAtMs` for the updated job without globally recomputing unrelated due jobs during `cron update`. (#15750)
|
||||
|
||||
@@ -70,4 +70,72 @@ describe("applyPatch", () => {
|
||||
expect(contents).toBe("line1\nline2\n");
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects path traversal outside cwd", async () => {
|
||||
await withTempDir(async (dir) => {
|
||||
const escapedPath = path.join(path.dirname(dir), "escaped.txt");
|
||||
const relativeEscape = path.relative(dir, escapedPath);
|
||||
|
||||
const patch = `*** Begin Patch
|
||||
*** Add File: ${relativeEscape}
|
||||
+escaped
|
||||
*** End Patch`;
|
||||
|
||||
await expect(applyPatch(patch, { cwd: dir })).rejects.toThrow(/Path escapes sandbox root/);
|
||||
await expect(fs.readFile(escapedPath, "utf8")).rejects.toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects absolute paths outside cwd", async () => {
|
||||
await withTempDir(async (dir) => {
|
||||
const escapedPath = path.join(os.tmpdir(), `openclaw-apply-patch-${Date.now()}.txt`);
|
||||
|
||||
const patch = `*** Begin Patch
|
||||
*** Add File: ${escapedPath}
|
||||
+escaped
|
||||
*** End Patch`;
|
||||
|
||||
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("allows absolute paths within cwd", async () => {
|
||||
await withTempDir(async (dir) => {
|
||||
const target = path.join(dir, "nested", "inside.txt");
|
||||
const patch = `*** Begin Patch
|
||||
*** Add File: ${target}
|
||||
+inside
|
||||
*** End Patch`;
|
||||
|
||||
await applyPatch(patch, { cwd: dir });
|
||||
const contents = await fs.readFile(target, "utf8");
|
||||
expect(contents).toBe("inside\n");
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects symlink escape attempts", async () => {
|
||||
await withTempDir(async (dir) => {
|
||||
const outside = path.join(path.dirname(dir), "outside-target.txt");
|
||||
const linkPath = path.join(dir, "link.txt");
|
||||
await fs.writeFile(outside, "initial\n", "utf8");
|
||||
await fs.symlink(outside, linkPath);
|
||||
|
||||
const patch = `*** Begin Patch
|
||||
*** Update File: link.txt
|
||||
@@
|
||||
-initial
|
||||
+pwned
|
||||
*** End Patch`;
|
||||
|
||||
await expect(applyPatch(patch, { cwd: dir })).rejects.toThrow(/Symlink not allowed/);
|
||||
const outsideContents = await fs.readFile(outside, "utf8");
|
||||
expect(outsideContents).toBe("initial\n");
|
||||
await fs.rm(outside, { force: true });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
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 type { SandboxFsBridge } from "./sandbox/fs-bridge.js";
|
||||
import { applyUpdateHunk } from "./apply-patch-update.js";
|
||||
import { assertSandboxPath } from "./sandbox-paths.js";
|
||||
|
||||
const BEGIN_PATCH_MARKER = "*** Begin Patch";
|
||||
const END_PATCH_MARKER = "*** End Patch";
|
||||
@@ -15,7 +15,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";
|
||||
@@ -262,36 +261,17 @@ async function resolvePatchPath(
|
||||
};
|
||||
}
|
||||
|
||||
const resolved = resolvePathFromCwd(filePath, options.cwd);
|
||||
const resolved = await assertSandboxPath({
|
||||
filePath,
|
||||
cwd: options.cwd,
|
||||
root: options.cwd,
|
||||
});
|
||||
return {
|
||||
resolved,
|
||||
display: toDisplayPath(resolved, options.cwd),
|
||||
resolved: resolved.resolved,
|
||||
display: toDisplayPath(resolved.resolved, options.cwd),
|
||||
};
|
||||
}
|
||||
|
||||
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 === "") {
|
||||
|
||||
Reference in New Issue
Block a user