diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c274f7577..9ef3d0074e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,7 @@ Docs: https://docs.openclaw.ai - Web UI: resolve header logo path when `gateway.controlUi.basePath` is set. (#7178) Thanks @Yeom-JinHo. - Web UI: apply button styling to the new-messages indicator. - Security: keep untrusted channel metadata out of system prompts (Slack/Discord). Thanks @KonstantinMirin. -- Security: require explicit credentials for gateway URL overrides to prevent credential leakage. (#8113) Thanks @victormier. +- Security: enforce sandboxed media paths for message tool attachments. (#9182) Thanks @victormier. - Voice call: harden webhook verification with host allowlists/proxy trust and keep ngrok loopback bypass. - Cron: accept epoch timestamps and 0ms durations in CLI `--at` parsing. - Cron: reload store data when the store file is recreated or mtime changes. diff --git a/src/agents/sandbox-paths.ts b/src/agents/sandbox-paths.ts index 054a16a689..22c72947a5 100644 --- a/src/agents/sandbox-paths.ts +++ b/src/agents/sandbox-paths.ts @@ -1,8 +1,11 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { fileURLToPath } from "node:url"; const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g; +const HTTP_URL_RE = /^https?:\/\//i; +const DATA_URL_RE = /^data:/i; function normalizeUnicodeSpaces(str: string): string { return str.replace(UNICODE_SPACES, " "); @@ -49,6 +52,40 @@ export async function assertSandboxPath(params: { filePath: string; cwd: string; return resolved; } +export function assertMediaNotDataUrl(media: string): void { + const raw = media.trim(); + if (DATA_URL_RE.test(raw)) { + throw new Error("data: URLs are not supported for media. Use buffer instead."); + } +} + +export async function resolveSandboxedMediaSource(params: { + media: string; + sandboxRoot: string; +}): Promise { + const raw = params.media.trim(); + if (!raw) { + return raw; + } + if (HTTP_URL_RE.test(raw)) { + return raw; + } + let candidate = raw; + if (/^file:\/\//i.test(candidate)) { + try { + candidate = fileURLToPath(candidate); + } catch { + throw new Error(`Invalid file:// URL for sandboxed media: ${raw}`); + } + } + const resolved = await assertSandboxPath({ + filePath: candidate, + cwd: params.sandboxRoot, + root: params.sandboxRoot, + }); + return resolved.resolved; +} + async function assertNoSymlink(relative: string, root: string) { if (!relative) { return; diff --git a/src/agents/tools/message-tool.test.ts b/src/agents/tools/message-tool.test.ts index 15d416fd89..a289908ac1 100644 --- a/src/agents/tools/message-tool.test.ts +++ b/src/agents/tools/message-tool.test.ts @@ -1,6 +1,3 @@ -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"; @@ -165,50 +162,8 @@ describe("message tool description", () => { }); }); -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 () => { +describe("message tool sandbox passthrough", () => { + it("forwards sandboxRoot to runMessageAction", async () => { mocks.runMessageAction.mockClear(); mocks.runMessageAction.mockResolvedValue({ kind: "send", @@ -220,27 +175,22 @@ describe("message tool sandbox path validation", () => { dryRun: true, } satisfies MessageActionRunResult); - const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-")); - try { - const tool = createMessageTool({ - config: {} as never, - sandboxRoot: sandboxDir, - }); + const tool = createMessageTool({ + config: {} as never, + sandboxRoot: "/tmp/sandbox", + }); - await tool.execute("1", { - action: "send", - target: "telegram:123", - filePath: "./data/file.txt", - message: "", - }); + await tool.execute("1", { + action: "send", + target: "telegram:123", + message: "", + }); - expect(mocks.runMessageAction).toHaveBeenCalledTimes(1); - } finally { - await fs.rm(sandboxDir, { recursive: true, force: true }); - } + const call = mocks.runMessageAction.mock.calls[0]?.[0]; + expect(call?.sandboxRoot).toBe("/tmp/sandbox"); }); - it("skips validation when no sandboxRoot is set", async () => { + it("omits sandboxRoot when not configured", async () => { mocks.runMessageAction.mockClear(); mocks.runMessageAction.mockResolvedValue({ kind: "send", @@ -259,11 +209,10 @@ describe("message tool sandbox path validation", () => { 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); + const call = mocks.runMessageAction.mock.calls[0]?.[0]; + expect(call?.sandboxRoot).toBeUndefined(); }); }); diff --git a/src/agents/tools/message-tool.ts b/src/agents/tools/message-tool.ts index 9a8d3ab63a..aec6d9d050 100644 --- a/src/agents/tools/message-tool.ts +++ b/src/agents/tools/message-tool.ts @@ -19,7 +19,6 @@ 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"; @@ -422,17 +421,6 @@ export function createMessageTool(options?: MessageToolOptions): AnyAgentTool { } } - // 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; @@ -475,6 +463,7 @@ export function createMessageTool(options?: MessageToolOptions): AnyAgentTool { agentId: options?.agentSessionKey ? resolveSessionAgentId({ sessionKey: options.agentSessionKey, config: cfg }) : undefined, + sandboxRoot: options?.sandboxRoot, abortSignal: signal, }); diff --git a/src/infra/outbound/message-action-runner.test.ts b/src/infra/outbound/message-action-runner.test.ts index ed432b0704..5ca3a56436 100644 --- a/src/infra/outbound/message-action-runner.test.ts +++ b/src/infra/outbound/message-action-runner.test.ts @@ -1,3 +1,6 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { ChannelPlugin } from "../../channels/plugins/types.js"; import type { OpenClawConfig } from "../../config/config.js"; @@ -446,6 +449,164 @@ describe("runMessageAction sendAttachment hydration", () => { Buffer.from("hello").toString("base64"), ); }); + + it("rewrites sandboxed media paths for sendAttachment", async () => { + const cfg = { + channels: { + bluebubbles: { + enabled: true, + serverUrl: "http://localhost:1234", + password: "test-password", + }, + }, + } as OpenClawConfig; + const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-")); + try { + await runMessageAction({ + cfg, + action: "sendAttachment", + params: { + channel: "bluebubbles", + target: "+15551234567", + media: "./data/pic.png", + message: "caption", + }, + sandboxRoot: sandboxDir, + }); + + const call = vi.mocked(loadWebMedia).mock.calls[0]; + expect(call?.[0]).toBe(path.join(sandboxDir, "data", "pic.png")); + } finally { + await fs.rm(sandboxDir, { recursive: true, force: true }); + } + }); +}); + +describe("runMessageAction sandboxed media validation", () => { + beforeEach(async () => { + const { createPluginRuntime } = await import("../../plugins/runtime/index.js"); + const { setSlackRuntime } = await import("../../../extensions/slack/src/runtime.js"); + const runtime = createPluginRuntime(); + setSlackRuntime(runtime); + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "slack", + source: "test", + plugin: slackPlugin, + }, + ]), + ); + }); + + afterEach(() => { + setActivePluginRegistry(createTestRegistry([])); + }); + + it("rejects media outside the sandbox root", async () => { + const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-")); + try { + await expect( + runMessageAction({ + cfg: slackConfig, + action: "send", + params: { + channel: "slack", + target: "#C12345678", + media: "/etc/passwd", + message: "", + }, + sandboxRoot: sandboxDir, + dryRun: true, + }), + ).rejects.toThrow(/sandbox/i); + } finally { + await fs.rm(sandboxDir, { recursive: true, force: true }); + } + }); + + it("rejects file:// media outside the sandbox root", async () => { + const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-")); + try { + await expect( + runMessageAction({ + cfg: slackConfig, + action: "send", + params: { + channel: "slack", + target: "#C12345678", + media: "file:///etc/passwd", + message: "", + }, + sandboxRoot: sandboxDir, + dryRun: true, + }), + ).rejects.toThrow(/sandbox/i); + } finally { + await fs.rm(sandboxDir, { recursive: true, force: true }); + } + }); + + it("rewrites sandbox-relative media paths", async () => { + const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-")); + try { + const result = await runMessageAction({ + cfg: slackConfig, + action: "send", + params: { + channel: "slack", + target: "#C12345678", + media: "./data/file.txt", + message: "", + }, + sandboxRoot: sandboxDir, + dryRun: true, + }); + + expect(result.kind).toBe("send"); + expect(result.sendResult?.mediaUrl).toBe(path.join(sandboxDir, "data", "file.txt")); + } finally { + await fs.rm(sandboxDir, { recursive: true, force: true }); + } + }); + + it("rewrites MEDIA directives under sandbox", async () => { + const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-")); + try { + const result = await runMessageAction({ + cfg: slackConfig, + action: "send", + params: { + channel: "slack", + target: "#C12345678", + message: "Hello\\nMEDIA: ./data/note.ogg", + }, + sandboxRoot: sandboxDir, + dryRun: true, + }); + + expect(result.kind).toBe("send"); + expect(result.sendResult?.mediaUrl).toBe(path.join(sandboxDir, "data", "note.ogg")); + } finally { + await fs.rm(sandboxDir, { recursive: true, force: true }); + } + }); + + it("rejects data URLs in media params", async () => { + await expect( + runMessageAction({ + cfg: slackConfig, + action: "send", + params: { + channel: "slack", + target: "#C12345678", + media: "data:image/png;base64,abcd", + message: "", + }, + dryRun: true, + }), + ).rejects.toThrow(/data:/i); + }); }); describe("runMessageAction accountId defaults", () => { diff --git a/src/infra/outbound/message-action-runner.ts b/src/infra/outbound/message-action-runner.ts index e19ae85288..bc4ae734ff 100644 --- a/src/infra/outbound/message-action-runner.ts +++ b/src/infra/outbound/message-action-runner.ts @@ -10,6 +10,7 @@ import type { OpenClawConfig } from "../../config/config.js"; import type { OutboundSendDeps } from "./deliver.js"; import type { MessagePollResult, MessageSendResult } from "./message.js"; import { resolveSessionAgentId } from "../../agents/agent-scope.js"; +import { assertMediaNotDataUrl, resolveSandboxedMediaSource } from "../../agents/sandbox-paths.js"; import { readNumberParam, readStringArrayParam, @@ -62,6 +63,7 @@ export type RunMessageActionParams = { deps?: OutboundSendDeps; sessionKey?: string; agentId?: string; + sandboxRoot?: string; dryRun?: boolean; abortSignal?: AbortSignal; }; @@ -326,6 +328,53 @@ function normalizeBase64Payload(params: { base64?: string; contentType?: string }; } +async function normalizeSandboxMediaParams(params: { + args: Record; + sandboxRoot?: string; +}): Promise { + const sandboxRoot = params.sandboxRoot?.trim(); + const mediaKeys: Array<"media" | "path" | "filePath"> = ["media", "path", "filePath"]; + for (const key of mediaKeys) { + const raw = readStringParam(params.args, key, { trim: false }); + if (!raw) { + continue; + } + assertMediaNotDataUrl(raw); + if (!sandboxRoot) { + continue; + } + const normalized = await resolveSandboxedMediaSource({ media: raw, sandboxRoot }); + if (normalized !== raw) { + params.args[key] = normalized; + } + } +} + +async function normalizeSandboxMediaList(params: { + values: string[]; + sandboxRoot?: string; +}): Promise { + const sandboxRoot = params.sandboxRoot?.trim(); + const normalized: string[] = []; + const seen = new Set(); + for (const value of params.values) { + const raw = value?.trim(); + if (!raw) { + continue; + } + assertMediaNotDataUrl(raw); + const resolved = sandboxRoot + ? await resolveSandboxedMediaSource({ media: raw, sandboxRoot }) + : raw; + if (seen.has(resolved)) { + continue; + } + seen.add(resolved); + normalized.push(resolved); + } + return normalized; +} + async function hydrateSetGroupIconParams(params: { cfg: OpenClawConfig; channel: ChannelId; @@ -699,6 +748,14 @@ async function handleSendAction(ctx: ResolvedActionContext): Promise