diff --git a/CHANGELOG.md b/CHANGELOG.md index d36f1a94850..efc5fd03cdb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Docs: https://docs.openclaw.ai - Gateway/Control UI: return 404 for missing static-asset paths instead of serving SPA fallback HTML, while preserving client-route fallback behavior for extensionless and non-asset dotted paths. (#12060) thanks @mcaxtr. - Gateway/Pairing: prevent device-token rotate scope escalation by enforcing an approved-scope baseline, preserving approved scopes across metadata updates, and rejecting rotate requests that exceed approved role scope implications. (#20703) thanks @coygeek. - Gateway/Security: require secure context and paired-device checks for Control UI auth even when `gateway.controlUi.allowInsecureAuth` is set, and align audit messaging with the hardened behavior. (#20684) thanks @coygeek. +- Security/Agents: restrict local MEDIA tool attachments to core tools and the OpenClaw temp root to prevent untrusted MCP tool file exfiltration. Thanks @NucleiAv and @thewilloftheshadow. - macOS/Build: default release packaging to `BUNDLE_ID=ai.openclaw.mac` in `scripts/package-mac-dist.sh`, so Sparkle feed URL is retained and auto-update no longer fails with an empty appcast feed. (#19750) thanks @loganprit. - Gateway/Pairing: clear persisted paired-device state when the gateway client closes with `device token mismatch` (`1008`) so reconnect flows can cleanly re-enter pairing. (#22071) Thanks @mbelinky. diff --git a/src/agents/pi-embedded-subscribe.handlers.tools.media.test.ts b/src/agents/pi-embedded-subscribe.handlers.tools.media.test.ts index d5e0a796a22..7d5db0bbde0 100644 --- a/src/agents/pi-embedded-subscribe.handlers.tools.media.test.ts +++ b/src/agents/pi-embedded-subscribe.handlers.tools.media.test.ts @@ -103,6 +103,42 @@ describe("handleToolExecutionEnd media emission", () => { }); }); + it("does NOT emit local media for untrusted tools", async () => { + const onToolResult = vi.fn(); + const ctx = createMockContext({ shouldEmitToolOutput: false, onToolResult }); + + await handleToolExecutionEnd(ctx, { + type: "tool_execution_end", + toolName: "plugin_tool", + toolCallId: "tc-1", + isError: false, + result: { + content: [{ type: "text", text: "MEDIA:/tmp/secret.png" }], + }, + }); + + expect(onToolResult).not.toHaveBeenCalled(); + }); + + it("emits remote media for untrusted tools", async () => { + const onToolResult = vi.fn(); + const ctx = createMockContext({ shouldEmitToolOutput: false, onToolResult }); + + await handleToolExecutionEnd(ctx, { + type: "tool_execution_end", + toolName: "plugin_tool", + toolCallId: "tc-1", + isError: false, + result: { + content: [{ type: "text", text: "MEDIA:https://example.com/file.png" }], + }, + }); + + expect(onToolResult).toHaveBeenCalledWith({ + mediaUrls: ["https://example.com/file.png"], + }); + }); + it("does NOT emit media when verbose is full (emitToolOutput handles it)", async () => { const onToolResult = vi.fn(); const ctx = createMockContext({ shouldEmitToolOutput: true, onToolResult }); diff --git a/src/agents/pi-embedded-subscribe.handlers.tools.ts b/src/agents/pi-embedded-subscribe.handlers.tools.ts index e5569ae5d5c..17d6eabf000 100644 --- a/src/agents/pi-embedded-subscribe.handlers.tools.ts +++ b/src/agents/pi-embedded-subscribe.handlers.tools.ts @@ -9,10 +9,11 @@ import type { ToolHandlerContext, } from "./pi-embedded-subscribe.handlers.types.js"; import { + extractMessagingToolSend, extractToolErrorMessage, extractToolResultMediaPaths, extractToolResultText, - extractMessagingToolSend, + filterToolResultMediaUrls, isToolResultError, sanitizeToolResult, } from "./pi-embedded-subscribe.tools.js"; @@ -381,7 +382,7 @@ export async function handleToolExecutionEnd( // When shouldEmitToolOutput() is true, emitToolOutput already delivers media // via parseReplyDirectives (MEDIA: text extraction), so skip to avoid duplicates. if (ctx.params.onToolResult && !isToolError && !ctx.shouldEmitToolOutput()) { - const mediaPaths = extractToolResultMediaPaths(result); + const mediaPaths = filterToolResultMediaUrls(toolName, extractToolResultMediaPaths(result)); if (mediaPaths.length > 0) { try { void ctx.params.onToolResult({ mediaUrls: mediaPaths }); diff --git a/src/agents/pi-embedded-subscribe.tools.ts b/src/agents/pi-embedded-subscribe.tools.ts index 55362aa6ea1..996e0c10c6c 100644 --- a/src/agents/pi-embedded-subscribe.tools.ts +++ b/src/agents/pi-embedded-subscribe.tools.ts @@ -4,6 +4,7 @@ import { MEDIA_TOKEN_RE } from "../media/parse.js"; import { truncateUtf16Safe } from "../utils.js"; import { collectTextContentBlocks } from "./content-blocks.js"; import { type MessagingToolSend } from "./pi-embedded-messaging.js"; +import { normalizeToolName } from "./tool-policy.js"; const TOOL_RESULT_MAX_CHARS = 8000; const TOOL_ERROR_MAX_CHARS = 400; @@ -129,6 +130,58 @@ export function extractToolResultText(result: unknown): string | undefined { return texts.join("\n"); } +// Core tool names that are allowed to emit local MEDIA: paths. +// Plugin/MCP tools are intentionally excluded to prevent untrusted file reads. +const TRUSTED_TOOL_RESULT_MEDIA = new Set([ + "agents_list", + "apply_patch", + "browser", + "canvas", + "cron", + "edit", + "exec", + "gateway", + "image", + "memory_get", + "memory_search", + "message", + "nodes", + "process", + "read", + "session_status", + "sessions_history", + "sessions_list", + "sessions_send", + "sessions_spawn", + "subagents", + "tts", + "web_fetch", + "web_search", + "write", +]); +const HTTP_URL_RE = /^https?:\/\//i; + +export function isToolResultMediaTrusted(toolName?: string): boolean { + if (!toolName) { + return false; + } + const normalized = normalizeToolName(toolName); + return TRUSTED_TOOL_RESULT_MEDIA.has(normalized); +} + +export function filterToolResultMediaUrls( + toolName: string | undefined, + mediaUrls: string[], +): string[] { + if (mediaUrls.length === 0) { + return mediaUrls; + } + if (isToolResultMediaTrusted(toolName)) { + return mediaUrls; + } + return mediaUrls.filter((url) => HTTP_URL_RE.test(url.trim())); +} + /** * Extract media file paths from a tool result. * diff --git a/src/agents/pi-embedded-subscribe.ts b/src/agents/pi-embedded-subscribe.ts index 8e7a7fec295..b3326c39e3a 100644 --- a/src/agents/pi-embedded-subscribe.ts +++ b/src/agents/pi-embedded-subscribe.ts @@ -16,6 +16,7 @@ import type { EmbeddedPiSubscribeContext, EmbeddedPiSubscribeState, } from "./pi-embedded-subscribe.handlers.types.js"; +import { filterToolResultMediaUrls } from "./pi-embedded-subscribe.tools.js"; import type { SubscribeEmbeddedPiSessionParams } from "./pi-embedded-subscribe.types.js"; import { formatReasoningMessage, stripDowngradedToolCallText } from "./pi-embedded-utils.js"; import { hasNonzeroUsage, normalizeUsage, type UsageLike } from "./usage.js"; @@ -324,13 +325,14 @@ export function subscribeEmbeddedPiSession(params: SubscribeEmbeddedPiSessionPar markdown: useMarkdown, }); const { text: cleanedText, mediaUrls } = parseReplyDirectives(agg); - if (!cleanedText && (!mediaUrls || mediaUrls.length === 0)) { + const filteredMediaUrls = filterToolResultMediaUrls(toolName, mediaUrls ?? []); + if (!cleanedText && filteredMediaUrls.length === 0) { return; } try { void params.onToolResult({ text: cleanedText, - mediaUrls: mediaUrls?.length ? mediaUrls : undefined, + mediaUrls: filteredMediaUrls.length ? filteredMediaUrls : undefined, }); } catch { // ignore tool result delivery failures @@ -345,13 +347,14 @@ export function subscribeEmbeddedPiSession(params: SubscribeEmbeddedPiSessionPar }); const message = `${agg}\n${formatToolOutputBlock(output)}`; const { text: cleanedText, mediaUrls } = parseReplyDirectives(message); - if (!cleanedText && (!mediaUrls || mediaUrls.length === 0)) { + const filteredMediaUrls = filterToolResultMediaUrls(toolName, mediaUrls ?? []); + if (!cleanedText && filteredMediaUrls.length === 0) { return; } try { void params.onToolResult({ text: cleanedText, - mediaUrls: mediaUrls?.length ? mediaUrls : undefined, + mediaUrls: filteredMediaUrls.length ? filteredMediaUrls : undefined, }); } catch { // ignore tool result delivery failures diff --git a/src/cli/nodes-media-utils.ts b/src/cli/nodes-media-utils.ts index eb8f853c6f3..2f07f0ea0ea 100644 --- a/src/cli/nodes-media-utils.ts +++ b/src/cli/nodes-media-utils.ts @@ -1,5 +1,6 @@ import { randomUUID } from "node:crypto"; -import * as os from "node:os"; +import fs from "node:fs"; +import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; export function asRecord(value: unknown): Record { return typeof value === "object" && value !== null ? (value as Record) : {}; @@ -22,8 +23,12 @@ export function resolveTempPathParts(opts: { ext: string; tmpDir?: string; id?: tmpDir: string; id: string; } { + const tmpDir = opts.tmpDir ?? resolvePreferredOpenClawTmpDir(); + if (!opts.tmpDir) { + fs.mkdirSync(tmpDir, { recursive: true, mode: 0o700 }); + } return { - tmpDir: opts.tmpDir ?? os.tmpdir(), + tmpDir, id: opts.id ?? randomUUID(), ext: opts.ext.startsWith(".") ? opts.ext : `.${opts.ext}`, }; diff --git a/src/media/local-roots.ts b/src/media/local-roots.ts index f926aba2f2e..8f203d15f7b 100644 --- a/src/media/local-roots.ts +++ b/src/media/local-roots.ts @@ -1,13 +1,14 @@ -import os from "node:os"; import path from "node:path"; import { resolveAgentWorkspaceDir } from "../agents/agent-scope.js"; import type { OpenClawConfig } from "../config/config.js"; import { resolveStateDir } from "../config/paths.js"; +import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; function buildMediaLocalRoots(stateDir: string): string[] { const resolvedStateDir = path.resolve(stateDir); + const preferredTmpDir = resolvePreferredOpenClawTmpDir(); return [ - os.tmpdir(), + preferredTmpDir, path.join(resolvedStateDir, "media"), path.join(resolvedStateDir, "agents"), path.join(resolvedStateDir, "workspace"), diff --git a/src/tts/tts.ts b/src/tts/tts.ts index 85f3454c34b..fb27eddd2d6 100644 --- a/src/tts/tts.ts +++ b/src/tts/tts.ts @@ -9,7 +9,6 @@ import { renameSync, unlinkSync, } from "node:fs"; -import { tmpdir } from "node:os"; import path from "node:path"; import type { ReplyPayload } from "../auto-reply/types.js"; import { normalizeChannelId } from "../channels/plugins/index.js"; @@ -23,6 +22,7 @@ import type { TtsModelOverrideConfig, } from "../config/types.tts.js"; import { logVerbose } from "../globals.js"; +import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; import { stripMarkdown } from "../line/markdown-to-line.js"; import { isVoiceCompatibleAudio } from "../media/audio.js"; import { CONFIG_DIR, resolveUserPath } from "../utils.js"; @@ -563,7 +563,9 @@ export async function textToSpeech(params: { continue; } - const tempDir = mkdtempSync(path.join(tmpdir(), "tts-")); + const tempRoot = resolvePreferredOpenClawTmpDir(); + mkdirSync(tempRoot, { recursive: true, mode: 0o700 }); + const tempDir = mkdtempSync(path.join(tempRoot, "tts-")); let edgeOutputFormat = resolveEdgeOutputFormat(config); const fallbackEdgeOutputFormat = edgeOutputFormat !== DEFAULT_EDGE_OUTPUT_FORMAT ? DEFAULT_EDGE_OUTPUT_FORMAT : undefined; @@ -670,7 +672,9 @@ export async function textToSpeech(params: { const latencyMs = Date.now() - providerStart; - const tempDir = mkdtempSync(path.join(tmpdir(), "tts-")); + const tempRoot = resolvePreferredOpenClawTmpDir(); + mkdirSync(tempRoot, { recursive: true, mode: 0o700 }); + const tempDir = mkdtempSync(path.join(tempRoot, "tts-")); const audioPath = path.join(tempDir, `voice-${Date.now()}${output.extension}`); writeFileSync(audioPath, audioBuffer); scheduleCleanup(tempDir); diff --git a/src/web/media.test.ts b/src/web/media.test.ts index 8dc7a987483..ea50ecd1c73 100644 --- a/src/web/media.test.ts +++ b/src/web/media.test.ts @@ -108,7 +108,7 @@ afterEach(() => { describe("web media loading", () => { beforeAll(() => { // Ensure state dir is stable and not influenced by other tests that stub OPENCLAW_STATE_DIR. - // Also keep it outside os.tmpdir() so tmpdir localRoots doesn't accidentally make all state readable. + // Also keep it outside the OpenClaw temp root so default localRoots doesn't accidentally make all state readable. stateDirSnapshot = captureEnv(["OPENCLAW_STATE_DIR"]); process.env.OPENCLAW_STATE_DIR = path.join( path.parse(os.tmpdir()).root, diff --git a/src/web/media.ts b/src/web/media.ts index 04f6307da3f..d07e5269050 100644 --- a/src/web/media.ts +++ b/src/web/media.ts @@ -73,9 +73,9 @@ async function assertLocalMediaAllowed( resolved = path.resolve(mediaPath); } - // Hardening: the default allowlist includes `os.tmpdir()`, and tests/CI may + // Hardening: the default allowlist includes the OpenClaw temp dir, and tests/CI may // override the state dir into tmp. Avoid accidentally allowing per-agent - // `workspace-*` state roots via the tmpdir prefix match; require explicit + // `workspace-*` state roots via the temp-root prefix match; require explicit // localRoots for those. if (localRoots === undefined) { const workspaceRoot = roots.find((root) => path.basename(root) === "workspace");