From 9b6fffd00a4e7b0dfbb90c829e7183da6cbb0de4 Mon Sep 17 00:00:00 2001 From: Leszek Szpunar <13106764+leszekszpunar@users.noreply.github.com> Date: Sun, 1 Feb 2026 23:19:09 +0100 Subject: [PATCH] security(message-tool): validate filePath/path against sandbox root (#6398) * security(message-tool): validate filePath/path against sandbox root * style: translate Polish comments to English for consistency --- src/agents/openclaw-tools.ts | 1 + src/agents/tools/message-tool.test.ts | 106 ++++++++++++++++++++++++++ src/agents/tools/message-tool.ts | 13 ++++ 3 files changed, 120 insertions(+) diff --git a/src/agents/openclaw-tools.ts b/src/agents/openclaw-tools.ts index 4604ae097..9bad8943a 100644 --- a/src/agents/openclaw-tools.ts +++ b/src/agents/openclaw-tools.ts @@ -92,6 +92,7 @@ export function createOpenClawTools(options?: { currentThreadTs: options?.currentThreadTs, replyToMode: options?.replyToMode, hasRepliedRef: options?.hasRepliedRef, + sandboxRoot: options?.sandboxRoot, }), createTtsTool({ agentChannel: options?.agentChannel, diff --git a/src/agents/tools/message-tool.test.ts b/src/agents/tools/message-tool.test.ts index accff1d94..15d416fd8 100644 --- a/src/agents/tools/message-tool.test.ts +++ b/src/agents/tools/message-tool.test.ts @@ -1,3 +1,6 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; import { describe, expect, it, vi } from "vitest"; import type { ChannelPlugin } from "../../channels/plugins/types.js"; import type { MessageActionRunResult } from "../../infra/outbound/message-action-runner.js"; @@ -161,3 +164,106 @@ describe("message tool description", () => { setActivePluginRegistry(createTestRegistry([])); }); }); + +describe("message tool sandbox path validation", () => { + it("rejects filePath that escapes sandbox root", async () => { + const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-")); + try { + const tool = createMessageTool({ + config: {} as never, + sandboxRoot: sandboxDir, + }); + + await expect( + tool.execute("1", { + action: "send", + target: "telegram:123", + filePath: "/etc/passwd", + message: "", + }), + ).rejects.toThrow(/sandbox/i); + } finally { + await fs.rm(sandboxDir, { recursive: true, force: true }); + } + }); + + it("rejects path param with traversal sequence", async () => { + const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-")); + try { + const tool = createMessageTool({ + config: {} as never, + sandboxRoot: sandboxDir, + }); + + await expect( + tool.execute("1", { + action: "send", + target: "telegram:123", + path: "../../../etc/shadow", + message: "", + }), + ).rejects.toThrow(/sandbox/i); + } finally { + await fs.rm(sandboxDir, { recursive: true, force: true }); + } + }); + + it("allows filePath inside sandbox root", async () => { + mocks.runMessageAction.mockClear(); + mocks.runMessageAction.mockResolvedValue({ + kind: "send", + action: "send", + channel: "telegram", + to: "telegram:123", + handledBy: "plugin", + payload: {}, + dryRun: true, + } satisfies MessageActionRunResult); + + const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-")); + try { + const tool = createMessageTool({ + config: {} as never, + sandboxRoot: sandboxDir, + }); + + await tool.execute("1", { + action: "send", + target: "telegram:123", + filePath: "./data/file.txt", + message: "", + }); + + expect(mocks.runMessageAction).toHaveBeenCalledTimes(1); + } finally { + await fs.rm(sandboxDir, { recursive: true, force: true }); + } + }); + + it("skips validation when no sandboxRoot is set", async () => { + mocks.runMessageAction.mockClear(); + mocks.runMessageAction.mockResolvedValue({ + kind: "send", + action: "send", + channel: "telegram", + to: "telegram:123", + handledBy: "plugin", + payload: {}, + dryRun: true, + } satisfies MessageActionRunResult); + + const tool = createMessageTool({ + config: {} as never, + }); + + await tool.execute("1", { + action: "send", + target: "telegram:123", + filePath: "/etc/passwd", + message: "", + }); + + // Without sandboxRoot the validation is skipped — unsandboxed sessions work normally. + expect(mocks.runMessageAction).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/agents/tools/message-tool.ts b/src/agents/tools/message-tool.ts index ebb70c162..359b075d2 100644 --- a/src/agents/tools/message-tool.ts +++ b/src/agents/tools/message-tool.ts @@ -19,6 +19,7 @@ import { normalizeAccountId } from "../../routing/session-key.js"; import { normalizeMessageChannel } from "../../utils/message-channel.js"; import { resolveSessionAgentId } from "../agent-scope.js"; import { listChannelSupportedActions } from "../channel-tools.js"; +import { assertSandboxPath } from "../sandbox-paths.js"; import { channelTargetSchema, channelTargetsSchema, stringEnum } from "../schema/typebox.js"; import { jsonResult, readNumberParam, readStringParam } from "./common.js"; @@ -252,6 +253,7 @@ type MessageToolOptions = { currentThreadTs?: string; replyToMode?: "off" | "first" | "all"; hasRepliedRef?: { value: boolean }; + sandboxRoot?: string; }; function buildMessageToolSchema(cfg: OpenClawConfig) { @@ -362,6 +364,17 @@ export function createMessageTool(options?: MessageToolOptions): AnyAgentTool { required: true, }) as ChannelMessageActionName; + // Validate file paths against sandbox root to prevent host file access. + const sandboxRoot = options?.sandboxRoot; + if (sandboxRoot) { + for (const key of ["filePath", "path"] as const) { + const raw = readStringParam(params, key, { trim: false }); + if (raw) { + await assertSandboxPath({ filePath: raw, cwd: sandboxRoot, root: sandboxRoot }); + } + } + } + const accountId = readStringParam(params, "accountId") ?? agentAccountId; if (accountId) { params.accountId = accountId;