fix: enforce apply_patch workspaceOnly in sandbox mounts

This commit is contained in:
Peter Steinberger
2026-02-24 02:23:30 +00:00
parent c070be1bc4
commit 6634030be3
3 changed files with 83 additions and 0 deletions

View File

@@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai
- Security/Commands: enforce sender-only matching for `commands.allowFrom` by blocking conversation-shaped `From` identities (`channel:`, `group:`, `thread:`, `@g.us`) while preserving direct-message fallback when sender fields are missing. Ships in the next npm release. Thanks @jiseoung. - Security/Commands: enforce sender-only matching for `commands.allowFrom` by blocking conversation-shaped `From` identities (`channel:`, `group:`, `thread:`, `@g.us`) while preserving direct-message fallback when sender fields are missing. Ships in the next npm release. Thanks @jiseoung.
- Config/Kilo Gateway: Kilo provider flow now surfaces an updated list of models. (#24921) thanks @gumadeiras. - Config/Kilo Gateway: Kilo provider flow now surfaces an updated list of models. (#24921) thanks @gumadeiras.
- Security/Sandbox: enforce `tools.exec.applyPatch.workspaceOnly` and `tools.fs.workspaceOnly` for `apply_patch` in sandbox-mounted paths so writes/deletes cannot escape the workspace boundary via mounts like `/agent` unless explicitly opted out (`tools.exec.applyPatch.workspaceOnly=false`). This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Config writes: block reserved prototype keys in account-id normalization and route account config resolution through own-key lookups, hardening `/allowlist` and account-scoped config paths against prototype-chain pollution. - Security/Config writes: block reserved prototype keys in account-id normalization and route account config resolution through own-key lookups, hardening `/allowlist` and account-scoped config paths against prototype-chain pollution.
- Security/Exec: harden `safeBins` long-option validation by rejecting unknown/ambiguous GNU long-option abbreviations and denying sort filesystem-dependent flags (`--random-source`, `--temporary-directory`, `-T`), closing safe-bin denylist bypasses. Thanks @jiseoung. - Security/Exec: harden `safeBins` long-option validation by rejecting unknown/ambiguous GNU long-option abbreviations and denying sort filesystem-dependent flags (`--random-source`, `--temporary-directory`, `-T`), closing safe-bin denylist bypasses. Thanks @jiseoung.
- Security/Channels: unify dangerous name-matching policy checks (`dangerouslyAllowNameMatching`) across core and extension channels, share mutable-allowlist detectors between `openclaw doctor` and `openclaw security audit`, and scan all configured accounts (not only the default account) in channel security audit findings. - Security/Channels: unify dangerous name-matching policy checks (`dangerouslyAllowNameMatching`) across core and extension channels, share mutable-allowlist detectors between `openclaw doctor` and `openclaw security audit`, and scan all configured accounts (not only the default account) in channel security audit findings.

View File

@@ -260,6 +260,14 @@ async function resolvePatchPath(
filePath, filePath,
cwd: options.cwd, cwd: options.cwd,
}); });
if (options.workspaceOnly !== false) {
await assertSandboxPath({
filePath: resolved.hostPath,
cwd: options.cwd,
root: options.cwd,
allowFinalSymlink: purpose === "unlink",
});
}
return { return {
resolved: resolved.hostPath, resolved: resolved.hostPath,
display: resolved.relativePath || resolved.hostPath, display: resolved.relativePath || resolved.hostPath,

View File

@@ -73,6 +73,10 @@ function createSandbox(params: {
}); });
} }
type ToolWithExecute = {
execute: (toolCallId: string, args: unknown, signal?: AbortSignal) => Promise<unknown>;
};
async function withUnsafeMountedSandboxHarness( async function withUnsafeMountedSandboxHarness(
run: (ctx: { sandboxRoot: string; agentRoot: string; sandbox: SandboxContext }) => Promise<void>, run: (ctx: { sandboxRoot: string; agentRoot: string; sandbox: SandboxContext }) => Promise<void>,
) { ) {
@@ -131,4 +135,74 @@ describe("tools.fs.workspaceOnly", () => {
expect(await fs.readFile(path.join(agentRoot, "secret.txt"), "utf8")).toBe("shh"); expect(await fs.readFile(path.join(agentRoot, "secret.txt"), "utf8")).toBe("shh");
}); });
}); });
it("enforces apply_patch workspace-only in sandbox mounts by default", async () => {
await withUnsafeMountedSandboxHarness(async ({ sandboxRoot, agentRoot, sandbox }) => {
const cfg: OpenClawConfig = {
tools: {
allow: ["read", "exec"],
exec: { applyPatch: { enabled: true } },
},
};
const tools = createOpenClawCodingTools({
sandbox,
workspaceDir: sandboxRoot,
config: cfg,
modelProvider: "openai",
modelId: "gpt-5.2",
});
const applyPatchTool = tools.find((t) => t.name === "apply_patch") as
| ToolWithExecute
| undefined;
if (!applyPatchTool) {
throw new Error("apply_patch tool missing");
}
const patch = `*** Begin Patch
*** Add File: /agent/pwned.txt
+owned-by-apply-patch
*** End Patch`;
await expect(applyPatchTool.execute("t1", { input: patch })).rejects.toThrow(
/Path escapes sandbox root/i,
);
await expect(fs.stat(path.join(agentRoot, "pwned.txt"))).rejects.toMatchObject({
code: "ENOENT",
});
});
});
it("allows apply_patch outside workspace root when explicitly disabled", async () => {
await withUnsafeMountedSandboxHarness(async ({ sandboxRoot, agentRoot, sandbox }) => {
const cfg: OpenClawConfig = {
tools: {
allow: ["read", "exec"],
exec: { applyPatch: { enabled: true, workspaceOnly: false } },
},
};
const tools = createOpenClawCodingTools({
sandbox,
workspaceDir: sandboxRoot,
config: cfg,
modelProvider: "openai",
modelId: "gpt-5.2",
});
const applyPatchTool = tools.find((t) => t.name === "apply_patch") as
| ToolWithExecute
| undefined;
if (!applyPatchTool) {
throw new Error("apply_patch tool missing");
}
const patch = `*** Begin Patch
*** Add File: /agent/pwned.txt
+owned-by-apply-patch
*** End Patch`;
await applyPatchTool.execute("t2", { input: patch });
expect(await fs.readFile(path.join(agentRoot, "pwned.txt"), "utf8")).toBe(
"owned-by-apply-patch\n",
);
});
});
}); });