mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-19 07:37:27 +00:00
security: harden apply_patch workspace path enforcement
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: 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: 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/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/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: 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)
|
- 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");
|
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 { AgentTool } from "@mariozechner/pi-agent-core";
|
||||||
import { Type } from "@sinclair/typebox";
|
import { Type } from "@sinclair/typebox";
|
||||||
import fs from "node:fs/promises";
|
import fs from "node:fs/promises";
|
||||||
import os from "node:os";
|
|
||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
import type { SandboxFsBridge } from "./sandbox/fs-bridge.js";
|
import type { SandboxFsBridge } from "./sandbox/fs-bridge.js";
|
||||||
import { applyUpdateHunk } from "./apply-patch-update.js";
|
import { applyUpdateHunk } from "./apply-patch-update.js";
|
||||||
|
import { assertSandboxPath } from "./sandbox-paths.js";
|
||||||
|
|
||||||
const BEGIN_PATCH_MARKER = "*** Begin Patch";
|
const BEGIN_PATCH_MARKER = "*** Begin Patch";
|
||||||
const END_PATCH_MARKER = "*** End Patch";
|
const END_PATCH_MARKER = "*** End Patch";
|
||||||
@@ -15,7 +15,6 @@ const MOVE_TO_MARKER = "*** Move to: ";
|
|||||||
const EOF_MARKER = "*** End of File";
|
const EOF_MARKER = "*** End of File";
|
||||||
const CHANGE_CONTEXT_MARKER = "@@ ";
|
const CHANGE_CONTEXT_MARKER = "@@ ";
|
||||||
const EMPTY_CHANGE_CONTEXT_MARKER = "@@";
|
const EMPTY_CHANGE_CONTEXT_MARKER = "@@";
|
||||||
const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g;
|
|
||||||
|
|
||||||
type AddFileHunk = {
|
type AddFileHunk = {
|
||||||
kind: "add";
|
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 {
|
return {
|
||||||
resolved,
|
resolved: resolved.resolved,
|
||||||
display: toDisplayPath(resolved, options.cwd),
|
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 {
|
function toDisplayPath(resolved: string, cwd: string): string {
|
||||||
const relative = path.relative(cwd, resolved);
|
const relative = path.relative(cwd, resolved);
|
||||||
if (!relative || relative === "") {
|
if (!relative || relative === "") {
|
||||||
|
|||||||
Reference in New Issue
Block a user