Skip to content

Commit 1725839

Browse files
大猫子lailoovelvet-shark
authored
fix(tools): honor tools.fs.workspaceOnly=false for host write/edit (#28822)
Merged via squash. Prepared head SHA: 83d4329 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
1 parent ad804b0 commit 1725839

4 files changed

Lines changed: 250 additions & 8 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ Docs: https://docs.openclaw.ai
5050

5151
### Fixes
5252

53+
- 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.
5354
- 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.
5455
- 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.
5556
- 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.

‎src/agents/pi-tools.read.ts‎

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -667,16 +667,16 @@ export function createSandboxedEditTool(params: SandboxToolParams) {
667667
return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit);
668668
}
669669

670-
export function createHostWorkspaceWriteTool(root: string) {
670+
export function createHostWorkspaceWriteTool(root: string, options?: { workspaceOnly?: boolean }) {
671671
const base = createWriteTool(root, {
672-
operations: createHostWriteOperations(root),
672+
operations: createHostWriteOperations(root, options),
673673
}) as unknown as AnyAgentTool;
674674
return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.write);
675675
}
676676

677-
export function createHostWorkspaceEditTool(root: string) {
677+
export function createHostWorkspaceEditTool(root: string, options?: { workspaceOnly?: boolean }) {
678678
const base = createEditTool(root, {
679-
operations: createHostEditOperations(root),
679+
operations: createHostEditOperations(root, options),
680680
}) as unknown as AnyAgentTool;
681681
return wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit);
682682
}
@@ -757,7 +757,26 @@ function createSandboxEditOperations(params: SandboxToolParams) {
757757
} as const;
758758
}
759759

760-
function createHostWriteOperations(root: string) {
760+
function createHostWriteOperations(root: string, options?: { workspaceOnly?: boolean }) {
761+
const workspaceOnly = options?.workspaceOnly !== false;
762+
763+
if (!workspaceOnly) {
764+
// When workspaceOnly is false, allow writes anywhere on the host
765+
return {
766+
mkdir: async (dir: string) => {
767+
const resolved = path.resolve(dir);
768+
await fs.mkdir(resolved, { recursive: true });
769+
},
770+
writeFile: async (absolutePath: string, content: string) => {
771+
const resolved = path.resolve(absolutePath);
772+
const dir = path.dirname(resolved);
773+
await fs.mkdir(dir, { recursive: true });
774+
await fs.writeFile(resolved, content, "utf-8");
775+
},
776+
} as const;
777+
}
778+
779+
// When workspaceOnly is true (default), enforce workspace boundary
761780
return {
762781
mkdir: async (dir: string) => {
763782
const relative = toRelativePathInRoot(root, dir, { allowRoot: true });
@@ -777,7 +796,30 @@ function createHostWriteOperations(root: string) {
777796
} as const;
778797
}
779798

780-
function createHostEditOperations(root: string) {
799+
function createHostEditOperations(root: string, options?: { workspaceOnly?: boolean }) {
800+
const workspaceOnly = options?.workspaceOnly !== false;
801+
802+
if (!workspaceOnly) {
803+
// When workspaceOnly is false, allow edits anywhere on the host
804+
return {
805+
readFile: async (absolutePath: string) => {
806+
const resolved = path.resolve(absolutePath);
807+
return await fs.readFile(resolved);
808+
},
809+
writeFile: async (absolutePath: string, content: string) => {
810+
const resolved = path.resolve(absolutePath);
811+
const dir = path.dirname(resolved);
812+
await fs.mkdir(dir, { recursive: true });
813+
await fs.writeFile(resolved, content, "utf-8");
814+
},
815+
access: async (absolutePath: string) => {
816+
const resolved = path.resolve(absolutePath);
817+
await fs.access(resolved);
818+
},
819+
} as const;
820+
}
821+
822+
// When workspaceOnly is true (default), enforce workspace boundary
781823
return {
782824
readFile: async (absolutePath: string) => {
783825
const relative = toRelativePathInRoot(root, absolutePath);

‎src/agents/pi-tools.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,14 +359,14 @@ export function createOpenClawCodingTools(options?: {
359359
if (sandboxRoot) {
360360
return [];
361361
}
362-
const wrapped = createHostWorkspaceWriteTool(workspaceRoot);
362+
const wrapped = createHostWorkspaceWriteTool(workspaceRoot, { workspaceOnly });
363363
return [workspaceOnly ? wrapToolWorkspaceRootGuard(wrapped, workspaceRoot) : wrapped];
364364
}
365365
if (tool.name === "edit") {
366366
if (sandboxRoot) {
367367
return [];
368368
}
369-
const wrapped = createHostWorkspaceEditTool(workspaceRoot);
369+
const wrapped = createHostWorkspaceEditTool(workspaceRoot, { workspaceOnly });
370370
return [workspaceOnly ? wrapToolWorkspaceRootGuard(wrapped, workspaceRoot) : wrapped];
371371
}
372372
return [tool];

‎src/agents/pi-tools.workspace-only-false.test.ts‎

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
import fs from "node:fs/promises";
2+
import os from "node:os";
3+
import path from "node:path";
4+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
5+
import { createOpenClawCodingTools } from "./pi-tools.js";
6+
7+
describe("FS tools with workspaceOnly=false", () => {
8+
let tmpDir: string;
9+
let workspaceDir: string;
10+
let outsideFile: string;
11+
12+
beforeEach(async () => {
13+
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-test-"));
14+
workspaceDir = path.join(tmpDir, "workspace");
15+
await fs.mkdir(workspaceDir);
16+
outsideFile = path.join(tmpDir, "outside.txt");
17+
});
18+
19+
afterEach(async () => {
20+
await fs.rm(tmpDir, { recursive: true, force: true });
21+
});
22+
23+
it("should allow write outside workspace when workspaceOnly=false", async () => {
24+
const tools = createOpenClawCodingTools({
25+
workspaceDir,
26+
config: {
27+
tools: {
28+
fs: {
29+
workspaceOnly: false,
30+
},
31+
},
32+
},
33+
});
34+
35+
const writeTool = tools.find((t) => t.name === "write");
36+
expect(writeTool).toBeDefined();
37+
38+
const result = await writeTool!.execute("test-call-1", {
39+
path: outsideFile,
40+
content: "test content",
41+
});
42+
43+
// Check if the operation succeeded (no error in content)
44+
const hasError = result.content.some(
45+
(c) => c.type === "text" && c.text.toLowerCase().includes("error"),
46+
);
47+
expect(hasError).toBe(false);
48+
const content = await fs.readFile(outsideFile, "utf-8");
49+
expect(content).toBe("test content");
50+
});
51+
52+
it("should allow write outside workspace via ../ path when workspaceOnly=false", async () => {
53+
const relativeOutsidePath = path.join("..", "outside-relative-write.txt");
54+
const outsideRelativeFile = path.join(tmpDir, "outside-relative-write.txt");
55+
56+
const tools = createOpenClawCodingTools({
57+
workspaceDir,
58+
config: {
59+
tools: {
60+
fs: {
61+
workspaceOnly: false,
62+
},
63+
},
64+
},
65+
});
66+
67+
const writeTool = tools.find((t) => t.name === "write");
68+
expect(writeTool).toBeDefined();
69+
70+
const result = await writeTool!.execute("test-call-1b", {
71+
path: relativeOutsidePath,
72+
content: "relative test content",
73+
});
74+
75+
const hasError = result.content.some(
76+
(c) => c.type === "text" && c.text.toLowerCase().includes("error"),
77+
);
78+
expect(hasError).toBe(false);
79+
const content = await fs.readFile(outsideRelativeFile, "utf-8");
80+
expect(content).toBe("relative test content");
81+
});
82+
83+
it("should allow edit outside workspace when workspaceOnly=false", async () => {
84+
await fs.writeFile(outsideFile, "old content");
85+
86+
const tools = createOpenClawCodingTools({
87+
workspaceDir,
88+
config: {
89+
tools: {
90+
fs: {
91+
workspaceOnly: false,
92+
},
93+
},
94+
},
95+
});
96+
97+
const editTool = tools.find((t) => t.name === "edit");
98+
expect(editTool).toBeDefined();
99+
100+
const result = await editTool!.execute("test-call-2", {
101+
path: outsideFile,
102+
oldText: "old content",
103+
newText: "new content",
104+
});
105+
106+
// Check if the operation succeeded (no error in content)
107+
const hasError = result.content.some(
108+
(c) => c.type === "text" && c.text.toLowerCase().includes("error"),
109+
);
110+
expect(hasError).toBe(false);
111+
const content = await fs.readFile(outsideFile, "utf-8");
112+
expect(content).toBe("new content");
113+
});
114+
115+
it("should allow edit outside workspace via ../ path when workspaceOnly=false", async () => {
116+
const relativeOutsidePath = path.join("..", "outside-relative-edit.txt");
117+
const outsideRelativeFile = path.join(tmpDir, "outside-relative-edit.txt");
118+
await fs.writeFile(outsideRelativeFile, "old relative content");
119+
120+
const tools = createOpenClawCodingTools({
121+
workspaceDir,
122+
config: {
123+
tools: {
124+
fs: {
125+
workspaceOnly: false,
126+
},
127+
},
128+
},
129+
});
130+
131+
const editTool = tools.find((t) => t.name === "edit");
132+
expect(editTool).toBeDefined();
133+
134+
const result = await editTool!.execute("test-call-2b", {
135+
path: relativeOutsidePath,
136+
oldText: "old relative content",
137+
newText: "new relative content",
138+
});
139+
140+
const hasError = result.content.some(
141+
(c) => c.type === "text" && c.text.toLowerCase().includes("error"),
142+
);
143+
expect(hasError).toBe(false);
144+
const content = await fs.readFile(outsideRelativeFile, "utf-8");
145+
expect(content).toBe("new relative content");
146+
});
147+
148+
it("should allow read outside workspace when workspaceOnly=false", async () => {
149+
await fs.writeFile(outsideFile, "test read content");
150+
151+
const tools = createOpenClawCodingTools({
152+
workspaceDir,
153+
config: {
154+
tools: {
155+
fs: {
156+
workspaceOnly: false,
157+
},
158+
},
159+
},
160+
});
161+
162+
const readTool = tools.find((t) => t.name === "read");
163+
expect(readTool).toBeDefined();
164+
165+
const result = await readTool!.execute("test-call-3", {
166+
path: outsideFile,
167+
});
168+
169+
// Check if the operation succeeded (no error in content)
170+
const hasError = result.content.some(
171+
(c) => c.type === "text" && c.text.toLowerCase().includes("error"),
172+
);
173+
expect(hasError).toBe(false);
174+
});
175+
176+
it("should block write outside workspace when workspaceOnly=true", async () => {
177+
const tools = createOpenClawCodingTools({
178+
workspaceDir,
179+
config: {
180+
tools: {
181+
fs: {
182+
workspaceOnly: true,
183+
},
184+
},
185+
},
186+
});
187+
188+
const writeTool = tools.find((t) => t.name === "write");
189+
expect(writeTool).toBeDefined();
190+
191+
// When workspaceOnly=true, the guard throws an error
192+
await expect(
193+
writeTool!.execute("test-call-4", {
194+
path: outsideFile,
195+
content: "test content",
196+
}),
197+
).rejects.toThrow(/Path escapes (workspace|sandbox) root/);
198+
});
199+
});

0 commit comments

Comments
 (0)