From 73fab7e44596edde4ed20e3c916b0b104bf30e2a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 21:33:46 +0100 Subject: [PATCH] fix(agents): map container workdir paths in workspace guard Co-authored-by: Explorer1092 <32663226+Explorer1092@users.noreply.github.com> --- CHANGELOG.md | 1 + src/agents/pi-tools.read.ts | 60 ++++++++++++++- ...pi-tools.read.workspace-root-guard.test.ts | 76 +++++++++++++++++++ src/agents/pi-tools.ts | 19 ++++- 4 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 src/agents/pi-tools.read.workspace-root-guard.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f55588a24..4608399fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Plugins/Media sandbox: propagate trusted `mediaLocalRoots` through plugin action dispatch (including Discord/Telegram action adapters) so plugin send paths enforce the same agent-scoped local-media sandbox roots as core outbound sends. (#20258, #22718) +- Agents/Workspace guard: map sandbox container-workdir file-tool paths (for example `/workspace/...` and `file:///workspace/...`) to host workspace roots before workspace-only validation, preventing false `Path escapes sandbox root` rejections for sandbox file tools. (#9560) - Slack/Threading: sessions: keep parent-session forking and thread-history context active beyond first turn by removing first-turn-only gates in session init, thread-history fetch, and reply prompt context injection. (#23843, #23090) Thanks @vincentkoc and @Taskle. - Slack/Threading: respect `replyToMode` when Slack auto-populates top-level `thread_ts`, and ignore inline `replyToId` directive tags when `replyToMode` is `off` so thread forcing stays disabled unless explicitly configured. (#23839, #23320, #23513) Thanks @vincentkoc and @dorukardahan. - Slack/Extension: forward `message read` `threadId` to `readMessages` and use delivery-context `threadId` as outbound `thread_ts` fallback so extension replies/reads stay in the correct Slack thread. (#22216, #22485, #23836) Thanks @vincentkoc, @lan17 and @dorukardahan. diff --git a/src/agents/pi-tools.read.ts b/src/agents/pi-tools.read.ts index 5dc70817c..7816596cd 100644 --- a/src/agents/pi-tools.read.ts +++ b/src/agents/pi-tools.read.ts @@ -1,3 +1,5 @@ +import path from "node:path"; +import { fileURLToPath } from "node:url"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; import { createEditTool, createReadTool, createWriteTool } from "@mariozechner/pi-coding-agent"; import { detectMime } from "../media/mime.js"; @@ -548,6 +550,57 @@ export function wrapToolParamNormalization( } export function wrapToolWorkspaceRootGuard(tool: AnyAgentTool, root: string): AnyAgentTool { + return wrapToolWorkspaceRootGuardWithOptions(tool, root); +} + +function mapContainerPathToWorkspaceRoot(params: { + filePath: string; + root: string; + containerWorkdir?: string; +}): string { + const containerWorkdir = params.containerWorkdir?.trim(); + if (!containerWorkdir) { + return params.filePath; + } + const normalizedWorkdir = containerWorkdir.replace(/\\/g, "/").replace(/\/+$/, ""); + if (!normalizedWorkdir.startsWith("/")) { + return params.filePath; + } + if (!normalizedWorkdir) { + return params.filePath; + } + + let candidate = params.filePath; + if (/^file:\/\//i.test(candidate)) { + try { + candidate = fileURLToPath(candidate); + } catch { + return params.filePath; + } + } + + const normalizedCandidate = candidate.replace(/\\/g, "/"); + if (normalizedCandidate === normalizedWorkdir) { + return path.resolve(params.root); + } + const prefix = `${normalizedWorkdir}/`; + if (!normalizedCandidate.startsWith(prefix)) { + return candidate; + } + const relative = normalizedCandidate.slice(prefix.length); + if (!relative) { + return path.resolve(params.root); + } + return path.resolve(params.root, ...relative.split("/").filter(Boolean)); +} + +export function wrapToolWorkspaceRootGuardWithOptions( + tool: AnyAgentTool, + root: string, + options?: { + containerWorkdir?: string; + }, +): AnyAgentTool { return { ...tool, execute: async (toolCallId, args, signal, onUpdate) => { @@ -557,7 +610,12 @@ export function wrapToolWorkspaceRootGuard(tool: AnyAgentTool, root: string): An (args && typeof args === "object" ? (args as Record) : undefined); const filePath = record?.path; if (typeof filePath === "string" && filePath.trim()) { - await assertSandboxPath({ filePath, cwd: root, root }); + const sandboxPath = mapContainerPathToWorkspaceRoot({ + filePath, + root, + containerWorkdir: options?.containerWorkdir, + }); + await assertSandboxPath({ filePath: sandboxPath, cwd: root, root }); } return tool.execute(toolCallId, normalized ?? args, signal, onUpdate); }, diff --git a/src/agents/pi-tools.read.workspace-root-guard.test.ts b/src/agents/pi-tools.read.workspace-root-guard.test.ts new file mode 100644 index 000000000..b94e04b63 --- /dev/null +++ b/src/agents/pi-tools.read.workspace-root-guard.test.ts @@ -0,0 +1,76 @@ +import path from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { wrapToolWorkspaceRootGuardWithOptions } from "./pi-tools.read.js"; +import type { AnyAgentTool } from "./pi-tools.types.js"; + +const assertSandboxPath = vi.fn(async () => ({ resolved: "/tmp/root", relative: "" })); + +vi.mock("./sandbox-paths.js", () => ({ + assertSandboxPath: (...args: unknown[]) => assertSandboxPath(...args), +})); + +function createToolHarness() { + const execute = vi.fn(async () => ({ + content: [{ type: "text", text: "ok" }], + })); + const tool = { + name: "read", + description: "test tool", + inputSchema: { type: "object", properties: {} }, + execute, + } as unknown as AnyAgentTool; + return { execute, tool }; +} + +describe("wrapToolWorkspaceRootGuardWithOptions", () => { + const root = "/tmp/root"; + + beforeEach(() => { + assertSandboxPath.mockClear(); + }); + + it("maps container workspace paths to host workspace root", async () => { + const { tool } = createToolHarness(); + const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, { + containerWorkdir: "/workspace", + }); + + await wrapped.execute("tc1", { path: "/workspace/docs/readme.md" }); + + expect(assertSandboxPath).toHaveBeenCalledWith({ + filePath: path.resolve(root, "docs", "readme.md"), + cwd: root, + root, + }); + }); + + it("maps file:// container workspace paths to host workspace root", async () => { + const { tool } = createToolHarness(); + const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, { + containerWorkdir: "/workspace", + }); + + await wrapped.execute("tc2", { path: "file:///workspace/docs/readme.md" }); + + expect(assertSandboxPath).toHaveBeenCalledWith({ + filePath: path.resolve(root, "docs", "readme.md"), + cwd: root, + root, + }); + }); + + it("does not remap absolute paths outside the configured container workdir", async () => { + const { tool } = createToolHarness(); + const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, { + containerWorkdir: "/workspace", + }); + + await wrapped.execute("tc3", { path: "/workspace-two/secret.txt" }); + + expect(assertSandboxPath).toHaveBeenCalledWith({ + filePath: "/workspace-two/secret.txt", + cwd: root, + root, + }); + }); +}); diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 9c53c3b0d..a338236c7 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -42,6 +42,7 @@ import { normalizeToolParams, patchToolSchemaForClaudeCompatibility, wrapToolWorkspaceRootGuard, + wrapToolWorkspaceRootGuardWithOptions, wrapToolParamNormalization, } from "./pi-tools.read.js"; import { cleanToolSchemaForGemini, normalizeToolParameters } from "./pi-tools.schema.js"; @@ -317,7 +318,13 @@ export function createOpenClawCodingTools(options?: { modelContextWindowTokens: options?.modelContextWindowTokens, imageSanitization, }); - return [workspaceOnly ? wrapToolWorkspaceRootGuard(sandboxed, sandboxRoot) : sandboxed]; + return [ + workspaceOnly + ? wrapToolWorkspaceRootGuardWithOptions(sandboxed, sandboxRoot, { + containerWorkdir: sandbox.containerWorkdir, + }) + : sandboxed, + ]; } const freshReadTool = createReadTool(workspaceRoot); const wrapped = createOpenClawReadTool(freshReadTool, { @@ -410,15 +417,21 @@ export function createOpenClawCodingTools(options?: { ? allowWorkspaceWrites ? [ workspaceOnly - ? wrapToolWorkspaceRootGuard( + ? wrapToolWorkspaceRootGuardWithOptions( createSandboxedEditTool({ root: sandboxRoot, bridge: sandboxFsBridge! }), sandboxRoot, + { + containerWorkdir: sandbox.containerWorkdir, + }, ) : createSandboxedEditTool({ root: sandboxRoot, bridge: sandboxFsBridge! }), workspaceOnly - ? wrapToolWorkspaceRootGuard( + ? wrapToolWorkspaceRootGuardWithOptions( createSandboxedWriteTool({ root: sandboxRoot, bridge: sandboxFsBridge! }), sandboxRoot, + { + containerWorkdir: sandbox.containerWorkdir, + }, ) : createSandboxedWriteTool({ root: sandboxRoot, bridge: sandboxFsBridge! }), ]