mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-10 14:44:57 +00:00
fix(tools): honor tools.fs.workspaceOnly=false for host write/edit (#28822)
Merged via squash.
Prepared head SHA: 83d432961d
Co-authored-by: lailoo <20536249+lailoo@users.noreply.github.com>
Co-authored-by: velvet-shark <126378+velvet-shark@users.noreply.github.com>
Reviewed-by: @velvet-shark
This commit is contained in:
@@ -50,6 +50,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
|
|
||||||
### Fixes
|
### Fixes
|
||||||
|
|
||||||
|
- FS tools/workspaceOnly: honor `tools.fs.workspaceOnly=false` for host write and edit operations so FS tools can access paths outside the workspace when sandbox is off. (#28822) thanks @lailoo. Fixes #28763. Thanks @cjscld for reporting.
|
||||||
- Telegram/DM allowlist runtime inheritance: enforce `dmPolicy: "allowlist"` `allowFrom` requirements using effective account-plus-parent config across account-capable channels (Telegram, Discord, Slack, Signal, iMessage, IRC, BlueBubbles, WhatsApp), and align `openclaw doctor` checks to the same inheritance logic so DM traffic is not silently dropped after upgrades. (#27936) Thanks @widingmarcus-cyber.
|
- Telegram/DM allowlist runtime inheritance: enforce `dmPolicy: "allowlist"` `allowFrom` requirements using effective account-plus-parent config across account-capable channels (Telegram, Discord, Slack, Signal, iMessage, IRC, BlueBubbles, WhatsApp), and align `openclaw doctor` checks to the same inheritance logic so DM traffic is not silently dropped after upgrades. (#27936) Thanks @widingmarcus-cyber.
|
||||||
- Delivery queue/recovery backoff: prevent retry starvation by persisting `lastAttemptAt` on failed sends and deferring recovery retries until each entry's `lastAttemptAt + backoff` window is eligible, while continuing to recover ready entries behind deferred ones. Landed from contributor PR #27710 by @Jimmy-xuzimo. Thanks @Jimmy-xuzimo.
|
- Delivery queue/recovery backoff: prevent retry starvation by persisting `lastAttemptAt` on failed sends and deferring recovery retries until each entry's `lastAttemptAt + backoff` window is eligible, while continuing to recover ready entries behind deferred ones. Landed from contributor PR #27710 by @Jimmy-xuzimo. Thanks @Jimmy-xuzimo.
|
||||||
- Gemini OAuth/Auth flow: align OAuth project discovery metadata and endpoint fallback handling for Gemini CLI auth, including fallback coverage for environment-provided project IDs. (#16684) Thanks @vincentkoc.
|
- Gemini OAuth/Auth flow: align OAuth project discovery metadata and endpoint fallback handling for Gemini CLI auth, including fallback coverage for environment-provided project IDs. (#16684) Thanks @vincentkoc.
|
||||||
|
|||||||
@@ -667,16 +667,16 @@ export function createSandboxedEditTool(params: SandboxToolParams) {
|
|||||||
return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit);
|
return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit);
|
||||||
}
|
}
|
||||||
|
|
||||||
export function createHostWorkspaceWriteTool(root: string) {
|
export function createHostWorkspaceWriteTool(root: string, options?: { workspaceOnly?: boolean }) {
|
||||||
const base = createWriteTool(root, {
|
const base = createWriteTool(root, {
|
||||||
operations: createHostWriteOperations(root),
|
operations: createHostWriteOperations(root, options),
|
||||||
}) as unknown as AnyAgentTool;
|
}) as unknown as AnyAgentTool;
|
||||||
return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.write);
|
return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.write);
|
||||||
}
|
}
|
||||||
|
|
||||||
export function createHostWorkspaceEditTool(root: string) {
|
export function createHostWorkspaceEditTool(root: string, options?: { workspaceOnly?: boolean }) {
|
||||||
const base = createEditTool(root, {
|
const base = createEditTool(root, {
|
||||||
operations: createHostEditOperations(root),
|
operations: createHostEditOperations(root, options),
|
||||||
}) as unknown as AnyAgentTool;
|
}) as unknown as AnyAgentTool;
|
||||||
return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit);
|
return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit);
|
||||||
}
|
}
|
||||||
@@ -757,7 +757,26 @@ function createSandboxEditOperations(params: SandboxToolParams) {
|
|||||||
} as const;
|
} as const;
|
||||||
}
|
}
|
||||||
|
|
||||||
function createHostWriteOperations(root: string) {
|
function createHostWriteOperations(root: string, options?: { workspaceOnly?: boolean }) {
|
||||||
|
const workspaceOnly = options?.workspaceOnly !== false;
|
||||||
|
|
||||||
|
if (!workspaceOnly) {
|
||||||
|
// When workspaceOnly is false, allow writes anywhere on the host
|
||||||
|
return {
|
||||||
|
mkdir: async (dir: string) => {
|
||||||
|
const resolved = path.resolve(dir);
|
||||||
|
await fs.mkdir(resolved, { recursive: true });
|
||||||
|
},
|
||||||
|
writeFile: async (absolutePath: string, content: string) => {
|
||||||
|
const resolved = path.resolve(absolutePath);
|
||||||
|
const dir = path.dirname(resolved);
|
||||||
|
await fs.mkdir(dir, { recursive: true });
|
||||||
|
await fs.writeFile(resolved, content, "utf-8");
|
||||||
|
},
|
||||||
|
} as const;
|
||||||
|
}
|
||||||
|
|
||||||
|
// When workspaceOnly is true (default), enforce workspace boundary
|
||||||
return {
|
return {
|
||||||
mkdir: async (dir: string) => {
|
mkdir: async (dir: string) => {
|
||||||
const relative = toRelativePathInRoot(root, dir, { allowRoot: true });
|
const relative = toRelativePathInRoot(root, dir, { allowRoot: true });
|
||||||
@@ -777,7 +796,30 @@ function createHostWriteOperations(root: string) {
|
|||||||
} as const;
|
} as const;
|
||||||
}
|
}
|
||||||
|
|
||||||
function createHostEditOperations(root: string) {
|
function createHostEditOperations(root: string, options?: { workspaceOnly?: boolean }) {
|
||||||
|
const workspaceOnly = options?.workspaceOnly !== false;
|
||||||
|
|
||||||
|
if (!workspaceOnly) {
|
||||||
|
// When workspaceOnly is false, allow edits anywhere on the host
|
||||||
|
return {
|
||||||
|
readFile: async (absolutePath: string) => {
|
||||||
|
const resolved = path.resolve(absolutePath);
|
||||||
|
return await fs.readFile(resolved);
|
||||||
|
},
|
||||||
|
writeFile: async (absolutePath: string, content: string) => {
|
||||||
|
const resolved = path.resolve(absolutePath);
|
||||||
|
const dir = path.dirname(resolved);
|
||||||
|
await fs.mkdir(dir, { recursive: true });
|
||||||
|
await fs.writeFile(resolved, content, "utf-8");
|
||||||
|
},
|
||||||
|
access: async (absolutePath: string) => {
|
||||||
|
const resolved = path.resolve(absolutePath);
|
||||||
|
await fs.access(resolved);
|
||||||
|
},
|
||||||
|
} as const;
|
||||||
|
}
|
||||||
|
|
||||||
|
// When workspaceOnly is true (default), enforce workspace boundary
|
||||||
return {
|
return {
|
||||||
readFile: async (absolutePath: string) => {
|
readFile: async (absolutePath: string) => {
|
||||||
const relative = toRelativePathInRoot(root, absolutePath);
|
const relative = toRelativePathInRoot(root, absolutePath);
|
||||||
|
|||||||
@@ -359,14 +359,14 @@ export function createOpenClawCodingTools(options?: {
|
|||||||
if (sandboxRoot) {
|
if (sandboxRoot) {
|
||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
const wrapped = createHostWorkspaceWriteTool(workspaceRoot);
|
const wrapped = createHostWorkspaceWriteTool(workspaceRoot, { workspaceOnly });
|
||||||
return [workspaceOnly ? wrapToolWorkspaceRootGuard(wrapped, workspaceRoot) : wrapped];
|
return [workspaceOnly ? wrapToolWorkspaceRootGuard(wrapped, workspaceRoot) : wrapped];
|
||||||
}
|
}
|
||||||
if (tool.name === "edit") {
|
if (tool.name === "edit") {
|
||||||
if (sandboxRoot) {
|
if (sandboxRoot) {
|
||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
const wrapped = createHostWorkspaceEditTool(workspaceRoot);
|
const wrapped = createHostWorkspaceEditTool(workspaceRoot, { workspaceOnly });
|
||||||
return [workspaceOnly ? wrapToolWorkspaceRootGuard(wrapped, workspaceRoot) : wrapped];
|
return [workspaceOnly ? wrapToolWorkspaceRootGuard(wrapped, workspaceRoot) : wrapped];
|
||||||
}
|
}
|
||||||
return [tool];
|
return [tool];
|
||||||
|
|||||||
199
src/agents/pi-tools.workspace-only-false.test.ts
Normal file
199
src/agents/pi-tools.workspace-only-false.test.ts
Normal file
@@ -0,0 +1,199 @@
|
|||||||
|
import fs from "node:fs/promises";
|
||||||
|
import os from "node:os";
|
||||||
|
import path from "node:path";
|
||||||
|
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||||
|
import { createOpenClawCodingTools } from "./pi-tools.js";
|
||||||
|
|
||||||
|
describe("FS tools with workspaceOnly=false", () => {
|
||||||
|
let tmpDir: string;
|
||||||
|
let workspaceDir: string;
|
||||||
|
let outsideFile: string;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-test-"));
|
||||||
|
workspaceDir = path.join(tmpDir, "workspace");
|
||||||
|
await fs.mkdir(workspaceDir);
|
||||||
|
outsideFile = path.join(tmpDir, "outside.txt");
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
await fs.rm(tmpDir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should allow write outside workspace when workspaceOnly=false", async () => {
|
||||||
|
const tools = createOpenClawCodingTools({
|
||||||
|
workspaceDir,
|
||||||
|
config: {
|
||||||
|
tools: {
|
||||||
|
fs: {
|
||||||
|
workspaceOnly: false,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const writeTool = tools.find((t) => t.name === "write");
|
||||||
|
expect(writeTool).toBeDefined();
|
||||||
|
|
||||||
|
const result = await writeTool!.execute("test-call-1", {
|
||||||
|
path: outsideFile,
|
||||||
|
content: "test content",
|
||||||
|
});
|
||||||
|
|
||||||
|
// Check if the operation succeeded (no error in content)
|
||||||
|
const hasError = result.content.some(
|
||||||
|
(c) => c.type === "text" && c.text.toLowerCase().includes("error"),
|
||||||
|
);
|
||||||
|
expect(hasError).toBe(false);
|
||||||
|
const content = await fs.readFile(outsideFile, "utf-8");
|
||||||
|
expect(content).toBe("test content");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should allow write outside workspace via ../ path when workspaceOnly=false", async () => {
|
||||||
|
const relativeOutsidePath = path.join("..", "outside-relative-write.txt");
|
||||||
|
const outsideRelativeFile = path.join(tmpDir, "outside-relative-write.txt");
|
||||||
|
|
||||||
|
const tools = createOpenClawCodingTools({
|
||||||
|
workspaceDir,
|
||||||
|
config: {
|
||||||
|
tools: {
|
||||||
|
fs: {
|
||||||
|
workspaceOnly: false,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const writeTool = tools.find((t) => t.name === "write");
|
||||||
|
expect(writeTool).toBeDefined();
|
||||||
|
|
||||||
|
const result = await writeTool!.execute("test-call-1b", {
|
||||||
|
path: relativeOutsidePath,
|
||||||
|
content: "relative test content",
|
||||||
|
});
|
||||||
|
|
||||||
|
const hasError = result.content.some(
|
||||||
|
(c) => c.type === "text" && c.text.toLowerCase().includes("error"),
|
||||||
|
);
|
||||||
|
expect(hasError).toBe(false);
|
||||||
|
const content = await fs.readFile(outsideRelativeFile, "utf-8");
|
||||||
|
expect(content).toBe("relative test content");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should allow edit outside workspace when workspaceOnly=false", async () => {
|
||||||
|
await fs.writeFile(outsideFile, "old content");
|
||||||
|
|
||||||
|
const tools = createOpenClawCodingTools({
|
||||||
|
workspaceDir,
|
||||||
|
config: {
|
||||||
|
tools: {
|
||||||
|
fs: {
|
||||||
|
workspaceOnly: false,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const editTool = tools.find((t) => t.name === "edit");
|
||||||
|
expect(editTool).toBeDefined();
|
||||||
|
|
||||||
|
const result = await editTool!.execute("test-call-2", {
|
||||||
|
path: outsideFile,
|
||||||
|
oldText: "old content",
|
||||||
|
newText: "new content",
|
||||||
|
});
|
||||||
|
|
||||||
|
// Check if the operation succeeded (no error in content)
|
||||||
|
const hasError = result.content.some(
|
||||||
|
(c) => c.type === "text" && c.text.toLowerCase().includes("error"),
|
||||||
|
);
|
||||||
|
expect(hasError).toBe(false);
|
||||||
|
const content = await fs.readFile(outsideFile, "utf-8");
|
||||||
|
expect(content).toBe("new content");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should allow edit outside workspace via ../ path when workspaceOnly=false", async () => {
|
||||||
|
const relativeOutsidePath = path.join("..", "outside-relative-edit.txt");
|
||||||
|
const outsideRelativeFile = path.join(tmpDir, "outside-relative-edit.txt");
|
||||||
|
await fs.writeFile(outsideRelativeFile, "old relative content");
|
||||||
|
|
||||||
|
const tools = createOpenClawCodingTools({
|
||||||
|
workspaceDir,
|
||||||
|
config: {
|
||||||
|
tools: {
|
||||||
|
fs: {
|
||||||
|
workspaceOnly: false,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const editTool = tools.find((t) => t.name === "edit");
|
||||||
|
expect(editTool).toBeDefined();
|
||||||
|
|
||||||
|
const result = await editTool!.execute("test-call-2b", {
|
||||||
|
path: relativeOutsidePath,
|
||||||
|
oldText: "old relative content",
|
||||||
|
newText: "new relative content",
|
||||||
|
});
|
||||||
|
|
||||||
|
const hasError = result.content.some(
|
||||||
|
(c) => c.type === "text" && c.text.toLowerCase().includes("error"),
|
||||||
|
);
|
||||||
|
expect(hasError).toBe(false);
|
||||||
|
const content = await fs.readFile(outsideRelativeFile, "utf-8");
|
||||||
|
expect(content).toBe("new relative content");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should allow read outside workspace when workspaceOnly=false", async () => {
|
||||||
|
await fs.writeFile(outsideFile, "test read content");
|
||||||
|
|
||||||
|
const tools = createOpenClawCodingTools({
|
||||||
|
workspaceDir,
|
||||||
|
config: {
|
||||||
|
tools: {
|
||||||
|
fs: {
|
||||||
|
workspaceOnly: false,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const readTool = tools.find((t) => t.name === "read");
|
||||||
|
expect(readTool).toBeDefined();
|
||||||
|
|
||||||
|
const result = await readTool!.execute("test-call-3", {
|
||||||
|
path: outsideFile,
|
||||||
|
});
|
||||||
|
|
||||||
|
// Check if the operation succeeded (no error in content)
|
||||||
|
const hasError = result.content.some(
|
||||||
|
(c) => c.type === "text" && c.text.toLowerCase().includes("error"),
|
||||||
|
);
|
||||||
|
expect(hasError).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should block write outside workspace when workspaceOnly=true", async () => {
|
||||||
|
const tools = createOpenClawCodingTools({
|
||||||
|
workspaceDir,
|
||||||
|
config: {
|
||||||
|
tools: {
|
||||||
|
fs: {
|
||||||
|
workspaceOnly: true,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const writeTool = tools.find((t) => t.name === "write");
|
||||||
|
expect(writeTool).toBeDefined();
|
||||||
|
|
||||||
|
// When workspaceOnly=true, the guard throws an error
|
||||||
|
await expect(
|
||||||
|
writeTool!.execute("test-call-4", {
|
||||||
|
path: outsideFile,
|
||||||
|
content: "test content",
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/Path escapes (workspace|sandbox) root/);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user