diff --git a/CHANGELOG.md b/CHANGELOG.md index a829728274f..36a89c49699 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- fix(agents): guard nodes tool outPath against workspace boundary [AI-assisted]. (#63551) Thanks @pgondhi987. - fix(qqbot): enforce media storage boundary for all outbound local file paths [AI]. (#63271) Thanks @pgondhi987. - iMessage/self-chat: distinguish normal DM outbound rows from true self-chat using `destination_caller_id` plus chat participants, while preserving multi-handle self-chat aliases so outbound DM replies stop looping back as inbound messages. (#61619) Thanks @neeravmakwana. - fix(browser): auto-generate browser control auth token for none/trusted-proxy modes [AI]. (#63280) Thanks @pgondhi987. diff --git a/src/agents/openclaw-tools.nodes-workspace-guard.test.ts b/src/agents/openclaw-tools.nodes-workspace-guard.test.ts new file mode 100644 index 00000000000..d22645ad5b9 --- /dev/null +++ b/src/agents/openclaw-tools.nodes-workspace-guard.test.ts @@ -0,0 +1,175 @@ +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import type { AnyAgentTool } from "./tools/common.js"; + +const mocks = vi.hoisted(() => ({ + assertSandboxPath: vi.fn(async (params: { filePath: string; cwd: string; root: string }) => { + const root = `/${params.root.replace(/\\/g, "/").replace(/^\/+|\/+$/g, "")}`; + const candidate = params.filePath.replace(/\\/g, "/"); + const input = candidate.startsWith("/") ? candidate : `${root}/${candidate}`; + const segments = input.split("/"); + const stack: string[] = []; + for (const segment of segments) { + if (!segment || segment === ".") { + continue; + } + if (segment === "..") { + stack.pop(); + continue; + } + stack.push(segment); + } + const resolved = `/${stack.join("/")}`; + const inside = resolved === root || resolved.startsWith(`${root}/`); + if (!inside) { + throw new Error(`Path escapes sandbox root (${root}): ${params.filePath}`); + } + const relative = resolved === root ? "" : resolved.slice(root.length + 1); + return { resolved, relative }; + }), + nodesExecute: vi.fn(async () => ({ + content: [{ type: "text", text: "ok" }], + details: {}, + })), +})); + +vi.mock("./sandbox-paths.js", () => ({ + assertSandboxPath: mocks.assertSandboxPath, +})); + +vi.mock("./tools/nodes-tool.js", () => ({ + createNodesTool: () => + ({ + name: "nodes", + label: "Nodes", + description: "nodes test tool", + parameters: { + type: "object", + properties: {}, + }, + execute: mocks.nodesExecute, + }) as unknown as AnyAgentTool, +})); + +let createOpenClawTools: typeof import("./openclaw-tools.js").createOpenClawTools; + +const WORKSPACE_ROOT = "/tmp/openclaw-workspace-nodes-guard"; + +describe("createOpenClawTools nodes workspace guard", () => { + beforeAll(async () => { + vi.resetModules(); + ({ createOpenClawTools } = await import("./openclaw-tools.js")); + }); + + beforeEach(() => { + mocks.assertSandboxPath.mockClear(); + mocks.nodesExecute.mockClear(); + }); + + function getNodesTool( + workspaceOnly: boolean, + options?: { sandboxRoot?: string; sandboxContainerWorkdir?: string }, + ): AnyAgentTool { + const tools = createOpenClawTools({ + workspaceDir: WORKSPACE_ROOT, + fsPolicy: { workspaceOnly }, + sandboxRoot: options?.sandboxRoot, + sandboxContainerWorkdir: options?.sandboxContainerWorkdir, + disablePluginTools: true, + disableMessageTool: true, + }); + const nodesTool = tools.find((tool) => tool.name === "nodes"); + expect(nodesTool).toBeDefined(); + if (!nodesTool) { + throw new Error("missing nodes tool"); + } + return nodesTool; + } + + it("guards outPath when workspaceOnly is enabled", async () => { + const nodesTool = getNodesTool(true); + await nodesTool.execute("call-1", { + action: "screen_record", + outPath: `${WORKSPACE_ROOT}/videos/capture.mp4`, + }); + + expect(mocks.assertSandboxPath).toHaveBeenCalledWith({ + filePath: `${WORKSPACE_ROOT}/videos/capture.mp4`, + cwd: WORKSPACE_ROOT, + root: WORKSPACE_ROOT, + }); + expect(mocks.nodesExecute).toHaveBeenCalledTimes(1); + }); + + it("normalizes relative outPath to an absolute workspace path before execute", async () => { + const nodesTool = getNodesTool(true); + await nodesTool.execute("call-rel", { + action: "screen_record", + outPath: "videos/capture.mp4", + }); + + expect(mocks.assertSandboxPath).toHaveBeenCalledWith({ + filePath: "videos/capture.mp4", + cwd: WORKSPACE_ROOT, + root: WORKSPACE_ROOT, + }); + expect(mocks.nodesExecute).toHaveBeenCalledWith( + "call-rel", + { + action: "screen_record", + outPath: `${WORKSPACE_ROOT}/videos/capture.mp4`, + }, + undefined, + undefined, + ); + }); + + it("maps sandbox container outPath to host root when containerWorkdir is provided", async () => { + const nodesTool = getNodesTool(true, { + sandboxRoot: WORKSPACE_ROOT, + sandboxContainerWorkdir: "/workspace", + }); + await nodesTool.execute("call-sandbox", { + action: "screen_record", + outPath: "/workspace/videos/capture.mp4", + }); + + expect(mocks.assertSandboxPath).toHaveBeenCalledWith({ + filePath: `${WORKSPACE_ROOT}/videos/capture.mp4`, + cwd: WORKSPACE_ROOT, + root: WORKSPACE_ROOT, + }); + expect(mocks.nodesExecute).toHaveBeenCalledWith( + "call-sandbox", + { + action: "screen_record", + outPath: `${WORKSPACE_ROOT}/videos/capture.mp4`, + }, + undefined, + undefined, + ); + }); + + it("rejects outPath outside workspace when workspaceOnly is enabled", async () => { + const nodesTool = getNodesTool(true); + await expect( + nodesTool.execute("call-2", { + action: "screen_record", + outPath: "/etc/passwd", + }), + ).rejects.toThrow(/Path escapes sandbox root/); + + expect(mocks.assertSandboxPath).toHaveBeenCalledTimes(1); + expect(mocks.nodesExecute).not.toHaveBeenCalled(); + }); + + it("does not guard outPath when workspaceOnly is disabled", async () => { + const nodesTool = getNodesTool(false); + await nodesTool.execute("call-3", { + action: "screen_record", + outPath: "/etc/passwd", + }); + + expect(mocks.assertSandboxPath).not.toHaveBeenCalled(); + expect(mocks.nodesExecute).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/agents/openclaw-tools.ts b/src/agents/openclaw-tools.ts index 2268ddbaf85..7955f766d20 100644 --- a/src/agents/openclaw-tools.ts +++ b/src/agents/openclaw-tools.ts @@ -9,6 +9,7 @@ import { collectPresentOpenClawTools, isUpdatePlanToolEnabledForOpenClawTools, } from "./openclaw-tools.registration.js"; +import { wrapToolWorkspaceRootGuardWithOptions } from "./pi-tools.read.js"; import type { SandboxFsBridge } from "./sandbox/fs-bridge.js"; import type { SpawnedToolContext } from "./spawned-context.js"; import type { ToolFsPolicy } from "./tool-fs-policy.js"; @@ -60,6 +61,7 @@ export function createOpenClawTools( agentThreadId?: string | number; agentDir?: string; sandboxRoot?: string; + sandboxContainerWorkdir?: string; sandboxFsBridge?: SandboxFsBridge; fsPolicy?: ToolFsPolicy; sandboxed?: boolean; @@ -205,18 +207,27 @@ export function createOpenClawTools( requireExplicitTarget: options?.requireExplicitMessageTarget, requesterSenderId: options?.requesterSenderId ?? undefined, }); + const nodesToolBase = createNodesTool({ + agentSessionKey: options?.agentSessionKey, + agentChannel: options?.agentChannel, + agentAccountId: options?.agentAccountId, + currentChannelId: options?.currentChannelId, + currentThreadTs: options?.currentThreadTs, + config: options?.config, + modelHasVision: options?.modelHasVision, + allowMediaInvokeCommands: options?.allowMediaInvokeCommands, + }); + const nodesTool = + options?.fsPolicy?.workspaceOnly === true + ? wrapToolWorkspaceRootGuardWithOptions(nodesToolBase, options?.sandboxRoot ?? workspaceDir, { + containerWorkdir: options?.sandboxContainerWorkdir, + pathParamKeys: ["outPath"], + normalizeGuardedPathParams: true, + }) + : nodesToolBase; const tools: AnyAgentTool[] = [ createCanvasTool({ config: options?.config }), - createNodesTool({ - agentSessionKey: options?.agentSessionKey, - agentChannel: options?.agentChannel, - agentAccountId: options?.agentAccountId, - currentChannelId: options?.currentChannelId, - currentThreadTs: options?.currentThreadTs, - config: options?.config, - modelHasVision: options?.modelHasVision, - allowMediaInvokeCommands: options?.allowMediaInvokeCommands, - }), + nodesTool, createCronTool({ agentSessionKey: options?.agentSessionKey, }), diff --git a/src/agents/pi-tools.read.ts b/src/agents/pi-tools.read.ts index 91050b20699..fa6f7e3b095 100644 --- a/src/agents/pi-tools.read.ts +++ b/src/agents/pi-tools.read.ts @@ -551,22 +551,34 @@ export function wrapToolWorkspaceRootGuardWithOptions( root: string, options?: { containerWorkdir?: string; + pathParamKeys?: readonly string[]; + normalizeGuardedPathParams?: boolean; }, ): AnyAgentTool { + const pathParamKeys = + options?.pathParamKeys && options.pathParamKeys.length > 0 ? options.pathParamKeys : ["path"]; return { ...tool, execute: async (toolCallId, args, signal, onUpdate) => { const record = getToolParamsRecord(args); - const filePath = record?.path; - if (typeof filePath === "string" && filePath.trim()) { + let normalizedRecord: Record | undefined; + for (const key of pathParamKeys) { + const filePath = record?.[key]; + if (typeof filePath !== "string" || !filePath.trim()) { + continue; + } const sandboxPath = mapContainerPathToWorkspaceRoot({ filePath, root, containerWorkdir: options?.containerWorkdir, }); - await assertSandboxPath({ filePath: sandboxPath, cwd: root, root }); + const sandboxResult = await assertSandboxPath({ filePath: sandboxPath, cwd: root, root }); + if (options?.normalizeGuardedPathParams && record) { + normalizedRecord ??= { ...record }; + normalizedRecord[key] = sandboxResult.resolved; + } } - return tool.execute(toolCallId, args, signal, onUpdate); + return tool.execute(toolCallId, normalizedRecord ?? 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 index 7cd4ad29e55..2ecfcf5aa2b 100644 --- a/src/agents/pi-tools.read.workspace-root-guard.test.ts +++ b/src/agents/pi-tools.read.workspace-root-guard.test.ts @@ -127,4 +127,38 @@ describe("wrapToolWorkspaceRootGuardWithOptions", () => { root, }); }); + + it("does not guard outPath by default", async () => { + const { tool } = createToolHarness(); + const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, { + containerWorkdir: "/workspace", + }); + + await wrapped.execute("tc-outpath-default", { outPath: "/workspace/videos/capture.mp4" }); + + expect(mocks.assertSandboxPath).not.toHaveBeenCalled(); + }); + + it("guards custom outPath params when configured", async () => { + const { execute, tool } = createToolHarness(); + const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, { + containerWorkdir: "/workspace", + pathParamKeys: ["outPath"], + normalizeGuardedPathParams: true, + }); + + await wrapped.execute("tc-outpath-custom", { outPath: "videos/capture.mp4" }); + + expect(mocks.assertSandboxPath).toHaveBeenCalledWith({ + filePath: "videos/capture.mp4", + cwd: root, + root, + }); + expect(execute).toHaveBeenCalledWith( + "tc-outpath-custom", + { outPath: path.resolve(root, "videos", "capture.mp4") }, + undefined, + undefined, + ); + }); }); diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 734157e1280..192e06e1b84 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -532,6 +532,7 @@ export function createOpenClawCodingTools(options?: { agentGroupSpace: options?.groupSpace ?? null, agentDir: options?.agentDir, sandboxRoot, + sandboxContainerWorkdir: sandbox?.containerWorkdir, sandboxFsBridge, fsPolicy, workspaceDir: workspaceRoot,