From 450c3a8ed24e54aa0615be84d7bae03bf49e929b Mon Sep 17 00:00:00 2001 From: slepybear <108438815+slepybear@users.noreply.github.com> Date: Wed, 15 Apr 2026 00:22:29 +0800 Subject: [PATCH] fix(security): include Matrix avatar params in sandbox media normalization + preserve mxc:// URLs + log gmail watcher stop failures [AI-assisted] (#64701) Merged via squash. Prepared head SHA: 54de3f019b8977826d1a40acb827568835bce780 Co-authored-by: slepybear <108438815+slepybear@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras --- CHANGELOG.md | 1 + .../.generated/plugin-sdk-api-baseline.sha256 | 4 +- docs/plugins/architecture.md | 9 ++ docs/plugins/sdk-channel-plugins.md | 10 ++ extensions/matrix/src/actions.test.ts | 3 + extensions/matrix/src/actions.ts | 57 ++++---- src/agents/sandbox-paths.test.ts | 10 ++ src/agents/sandbox-paths.ts | 4 +- src/auto-reply/reply/reply-media-paths.ts | 4 +- .../plugins/message-action-discovery.ts | 98 +++++++++++-- src/channels/plugins/message-actions.test.ts | 77 ++++++++++ src/channels/plugins/types.core.ts | 14 +- src/gateway/server-reload-handlers.ts | 4 +- .../outbound/message-action-params.test.ts | 132 ++++++++++++++++++ src/infra/outbound/message-action-params.ts | 56 +++++++- .../message-action-runner.media.test.ts | 131 +++++++++++++++++ src/infra/outbound/message-action-runner.ts | 29 ++-- src/media/local-roots.test.ts | 6 + src/media/local-roots.ts | 4 +- src/media/media-source-url.ts | 7 + 20 files changed, 593 insertions(+), 67 deletions(-) create mode 100644 src/media/media-source-url.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f77ca9f3154..33dc3964efd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai - Memory-core/QMD `memory_get`: reject reads of arbitrary workspace markdown paths and only allow canonical memory files (`MEMORY.md`, `memory.md`, `DREAMS.md`, `dreams.md`, `memory/**`) plus exact paths of active indexed QMD workspace documents, so the QMD memory backend can no longer be used as a generic workspace-file read shim that bypasses `read` tool-policy denials. (#66026) Thanks @eleqtrizit. - Cron/agents: forward embedded-run tool policy and internal event params into the attempt layer so `--tools` allowlists, cron-owned message-tool suppression, explicit message targeting, and command-path internal events all take effect at runtime again. (#62675) Thanks @hexsprite. - Setup/providers: guard preferred-provider lookup during setup so malformed plugin metadata with a missing provider id no longer crashes the wizard with `Cannot read properties of undefined (reading 'trim')`. (#66649) Thanks @Tianworld. +- Matrix/security: normalize sandboxed profile avatar params, preserve `mxc://` avatar URLs, and surface gmail watcher stop failures during reload. (#64701) Thanks @slepybear. ## 2026.4.14 diff --git a/docs/.generated/plugin-sdk-api-baseline.sha256 b/docs/.generated/plugin-sdk-api-baseline.sha256 index 371c47785d6..113eff3aedb 100644 --- a/docs/.generated/plugin-sdk-api-baseline.sha256 +++ b/docs/.generated/plugin-sdk-api-baseline.sha256 @@ -1,2 +1,2 @@ -a1e765cf426077085975f1f00847026b71f301cad35cb9168713e2b6249c4a47 plugin-sdk-api-baseline.json -9f1cdbe8d9bfbd582edb671729c4c09e578fb1940e787cfd6aa82dee0bdf5de7 plugin-sdk-api-baseline.jsonl +cd06d41c9302b068d2d998e478a4cca5e0bdd0b165e381cc68740698a5921d21 plugin-sdk-api-baseline.json +8131372bd1fb433d24de85c94e3fe58368579abed10ec80f39370c6f6fee6373 plugin-sdk-api-baseline.jsonl diff --git a/docs/plugins/architecture.md b/docs/plugins/architecture.md index 88ce9a749a5..c336f92787c 100644 --- a/docs/plugins/architecture.md +++ b/docs/plugins/architecture.md @@ -173,6 +173,15 @@ For channel plugins, the SDK surface is call lets a plugin return its visible actions, capabilities, and schema contributions together so those pieces do not drift apart. +When a channel-specific message-tool param carries a media source such as a +local path or remote media URL, the plugin should also return +`mediaSourceParams` from `describeMessageTool(...)`. Core uses that explicit +list to apply sandbox path normalization and outbound media-access hints +without hardcoding plugin-owned param names. +Prefer action-scoped maps there, not one channel-wide flat list, so a +profile-only media param does not get normalized on unrelated actions like +`send`. + Core passes runtime scope into that discovery step. Important fields include: - `accountId` diff --git a/docs/plugins/sdk-channel-plugins.md b/docs/plugins/sdk-channel-plugins.md index 04042a65306..5e51b8b6e4a 100644 --- a/docs/plugins/sdk-channel-plugins.md +++ b/docs/plugins/sdk-channel-plugins.md @@ -35,6 +35,16 @@ shared `message` tool in core. Your plugin owns: Core owns the shared message tool, prompt wiring, the outer session-key shape, generic `:thread:` bookkeeping, and dispatch. +If your channel adds message-tool params that carry media sources, expose those +param names through `describeMessageTool(...).mediaSourceParams`. Core uses +that explicit list for sandbox path normalization and outbound media-access +policy, so plugins do not need shared-core special cases for provider-specific +avatar, attachment, or cover-image params. +Prefer returning an action-keyed map such as +`{ "set-profile": ["avatarUrl", "avatarPath"] }` so unrelated actions do not +inherit another action's media args. A flat array still works for params that +are intentionally shared across every exposed action. + If your platform stores extra scope inside conversation ids, keep that parsing in the plugin with `messaging.resolveSessionConversation(...)`. That is the canonical hook for mapping `rawId` to the base conversation id, optional thread diff --git a/extensions/matrix/src/actions.test.ts b/extensions/matrix/src/actions.test.ts index cf360467526..70cff604395 100644 --- a/extensions/matrix/src/actions.test.ts +++ b/extensions/matrix/src/actions.test.ts @@ -92,6 +92,9 @@ describe("matrixMessageActions", () => { expect(actions).toContain(profileAction); expect(supportsAction({ action: profileAction } as never)).toBe(true); + expect(discovery.mediaSourceParams).toEqual({ + "set-profile": ["avatarUrl", "avatar_url", "avatarPath", "avatar_path"], + }); expect(properties.displayName).toBeDefined(); expect(properties.avatarUrl).toBeDefined(); expect(properties.avatarPath).toBeDefined(); diff --git a/extensions/matrix/src/actions.ts b/extensions/matrix/src/actions.ts index 607be5dcfd0..6797786b23b 100644 --- a/extensions/matrix/src/actions.ts +++ b/extensions/matrix/src/actions.ts @@ -31,6 +31,35 @@ const MATRIX_PLUGIN_HANDLED_ACTIONS = new Set([ "channel-info", "permissions", ]); +const MATRIX_PROFILE_MEDIA_PROPERTIES = { + avatarUrl: Type.Optional( + Type.String({ + description: + "Profile avatar URL for Matrix self-profile update actions. Matrix accepts mxc:// and http(s) URLs.", + }), + ), + avatar_url: Type.Optional( + Type.String({ + description: + "snake_case alias of avatarUrl for Matrix self-profile update actions. Matrix accepts mxc:// and http(s) URLs.", + }), + ), + avatarPath: Type.Optional( + Type.String({ + description: + "Local avatar file path for Matrix self-profile update actions. Matrix uploads this file and sets the resulting MXC URI.", + }), + ), + avatar_path: Type.Optional( + Type.String({ + description: + "snake_case alias of avatarPath for Matrix self-profile update actions. Matrix uploads this file and sets the resulting MXC URI.", + }), + ), +} as const; +const MATRIX_PROFILE_MEDIA_SOURCE_PARAMS = Object.freeze( + Object.keys(MATRIX_PROFILE_MEDIA_PROPERTIES), +); function createMatrixExposedActions(params: { gate: ReturnType; @@ -81,30 +110,7 @@ function buildMatrixProfileToolSchema(): NonNullable MATRIX_PLUGIN_HANDLED_ACTIONS.has(action), diff --git a/src/agents/sandbox-paths.test.ts b/src/agents/sandbox-paths.test.ts index 61d3c70ecb3..49609eda75b 100644 --- a/src/agents/sandbox-paths.test.ts +++ b/src/agents/sandbox-paths.test.ts @@ -130,6 +130,16 @@ describe("resolveSandboxedMediaSource", () => { }); }); + it("preserves remote mxc:// media sources", async () => { + await withSandboxRoot(async (sandboxDir) => { + const result = await resolveSandboxedMediaSource({ + media: "mxc://matrix.org/abc123def456", + sandboxRoot: sandboxDir, + }); + expect(result).toBe("mxc://matrix.org/abc123def456"); + }); + }); + // Group 3: Rejections (security) it.each([ { diff --git a/src/agents/sandbox-paths.ts b/src/agents/sandbox-paths.ts index 11eaa930752..77e16c16be6 100644 --- a/src/agents/sandbox-paths.ts +++ b/src/agents/sandbox-paths.ts @@ -10,9 +10,9 @@ import { import { assertNoPathAliasEscape, type PathAliasPolicy } from "../infra/path-alias-guards.js"; import { isPathInside } from "../infra/path-guards.js"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; +import { isPassThroughRemoteMediaSource } from "../media/media-source-url.js"; const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g; -const HTTP_URL_RE = /^https?:\/\//i; const DATA_URL_RE = /^data:/i; const SANDBOX_CONTAINER_WORKDIR = "/workspace"; @@ -108,7 +108,7 @@ export async function resolveSandboxedMediaSource(params: { if (!raw) { return raw; } - if (HTTP_URL_RE.test(raw)) { + if (isPassThroughRemoteMediaSource(raw)) { return raw; } let candidate = raw; diff --git a/src/auto-reply/reply/reply-media-paths.ts b/src/auto-reply/reply/reply-media-paths.ts index 473dd17b91a..b4fa59849e8 100644 --- a/src/auto-reply/reply/reply-media-paths.ts +++ b/src/auto-reply/reply/reply-media-paths.ts @@ -9,11 +9,11 @@ import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { logVerbose } from "../../globals.js"; import { resolvePreferredOpenClawTmpDir } from "../../infra/tmp-openclaw-dir.js"; import { resolveConfiguredMediaMaxBytes } from "../../media/configured-max-bytes.js"; +import { isPassThroughRemoteMediaSource } from "../../media/media-source-url.js"; import { saveMediaSource } from "../../media/store.js"; import { resolveConfigDir } from "../../utils.js"; import type { ReplyPayload } from "../types.js"; -const HTTP_URL_RE = /^https?:\/\//i; const FILE_URL_RE = /^file:\/\//i; const WINDOWS_DRIVE_RE = /^[a-zA-Z]:[\\/]/; const SCHEME_RE = /^[a-zA-Z][a-zA-Z0-9+.-]*:/; @@ -156,7 +156,7 @@ export function createReplyMediaPathNormalizer(params: { return media; } assertMediaNotDataUrl(media); - if (HTTP_URL_RE.test(media)) { + if (isPassThroughRemoteMediaSource(media)) { return media; } const sandboxRoot = await resolveSandboxRoot(); diff --git a/src/channels/plugins/message-action-discovery.ts b/src/channels/plugins/message-action-discovery.ts index 964844bd6c9..c07fe442085 100644 --- a/src/channels/plugins/message-action-discovery.ts +++ b/src/channels/plugins/message-action-discovery.ts @@ -108,12 +108,54 @@ type ResolvedChannelMessageActionDiscovery = { actions: ChannelMessageActionName[]; capabilities: readonly ChannelMessageCapability[]; schemaContributions: ChannelMessageToolSchemaContribution[]; + mediaSourceParams: readonly string[]; }; +type MessageToolMediaSourceParamMap = Partial>; + +function normalizeMessageToolMediaSourceParams( + mediaSourceParams: ChannelMessageToolDiscovery["mediaSourceParams"], + action?: ChannelMessageActionName, +): readonly string[] { + if (Array.isArray(mediaSourceParams)) { + return mediaSourceParams; + } + if (!mediaSourceParams || typeof mediaSourceParams !== "object") { + return []; + } + const scopedMediaSourceParams = mediaSourceParams as MessageToolMediaSourceParamMap; + if (action) { + const scoped = scopedMediaSourceParams[action]; + return Array.isArray(scoped) ? scoped : []; + } + return Object.values(scopedMediaSourceParams).flatMap((scoped) => + Array.isArray(scoped) ? scoped : [], + ); +} + +function resolveCurrentChannelPluginActions(channel?: string): { + pluginId: string; + actions: ChannelActions; +} | null { + const channelId = resolveMessageActionDiscoveryChannelId(channel); + if (!channelId) { + return null; + } + const plugin = getChannelPlugin(channelId as Parameters[0]); + if (!plugin?.actions) { + return null; + } + return { + pluginId: plugin.id, + actions: plugin.actions, + }; +} + export function resolveMessageActionDiscoveryForPlugin(params: { pluginId: string; actions?: ChannelActions; context: ChannelMessageActionDiscoveryContext; + action?: ChannelMessageActionName; includeActions?: boolean; includeCapabilities?: boolean; includeSchema?: boolean; @@ -124,6 +166,7 @@ export function resolveMessageActionDiscoveryForPlugin(params: { actions: [], capabilities: [], schemaContributions: [], + mediaSourceParams: [], }; } @@ -142,6 +185,10 @@ export function resolveMessageActionDiscoveryForPlugin(params: { schemaContributions: params.includeSchema ? normalizeToolSchemaContributions(described?.schema) : [], + mediaSourceParams: normalizeMessageToolMediaSourceParams( + described?.mediaSourceParams, + params.action, + ), }; } @@ -188,21 +235,18 @@ export function listChannelMessageCapabilitiesForChannel(params: { requesterSenderId?: string | null; senderIsOwner?: boolean; }): ChannelMessageCapability[] { - const channelId = resolveMessageActionDiscoveryChannelId(params.channel); - if (!channelId) { + const pluginActions = resolveCurrentChannelPluginActions(params.channel); + if (!pluginActions) { return []; } - const plugin = getChannelPlugin(channelId as Parameters[0]); - return plugin?.actions - ? Array.from( - resolveMessageActionDiscoveryForPlugin({ - pluginId: plugin.id, - actions: plugin.actions, - context: createMessageActionDiscoveryContext(params), - includeCapabilities: true, - }).capabilities, - ) - : []; + return Array.from( + resolveMessageActionDiscoveryForPlugin({ + pluginId: pluginActions.pluginId, + actions: pluginActions.actions, + context: createMessageActionDiscoveryContext(params), + includeCapabilities: true, + }).capabilities, + ); } function mergeToolSchemaProperties( @@ -260,6 +304,34 @@ export function resolveChannelMessageToolSchemaProperties(params: { return properties; } +export function resolveChannelMessageToolMediaSourceParamKeys(params: { + cfg: OpenClawConfig; + action?: ChannelMessageActionName; + channel?: string; + currentChannelId?: string | null; + currentThreadTs?: string | null; + currentMessageId?: string | number | null; + accountId?: string | null; + sessionKey?: string | null; + sessionId?: string | null; + agentId?: string | null; + requesterSenderId?: string | null; + senderIsOwner?: boolean; +}): string[] { + const pluginActions = resolveCurrentChannelPluginActions(params.channel); + if (!pluginActions) { + return []; + } + const described = resolveMessageActionDiscoveryForPlugin({ + pluginId: pluginActions.pluginId, + actions: pluginActions.actions, + context: createMessageActionDiscoveryContext(params), + action: params.action, + includeSchema: false, + }); + return Array.from(new Set(described.mediaSourceParams)); +} + export function channelSupportsMessageCapability( cfg: OpenClawConfig, capability: ChannelMessageCapability, diff --git a/src/channels/plugins/message-actions.test.ts b/src/channels/plugins/message-actions.test.ts index 75eac104e95..58858be9a44 100644 --- a/src/channels/plugins/message-actions.test.ts +++ b/src/channels/plugins/message-actions.test.ts @@ -14,6 +14,7 @@ import { listChannelMessageActions, listChannelMessageCapabilities, listChannelMessageCapabilitiesForChannel, + resolveChannelMessageToolMediaSourceParamKeys, resolveChannelMessageToolSchemaProperties, } from "./message-action-discovery.js"; import type { ChannelMessageCapability } from "./message-capabilities.js"; @@ -199,6 +200,82 @@ describe("message action capability checks", () => { ).toHaveProperty("components"); }); + it("derives plugin-owned media-source params for the current action", () => { + const mediaPlugin: ChannelPlugin = { + ...createChannelTestPluginBase({ + id: "demo-media", + label: "Demo Media", + capabilities: { chatTypes: ["direct", "group"] }, + config: { + listAccountIds: () => ["default"], + }, + }), + actions: { + describeMessageTool: () => ({ + actions: ["send", "set-profile"], + mediaSourceParams: { + "set-profile": ["avatarUrl", "avatarPath"], + }, + schema: { + properties: { + avatarUrl: Type.Optional(Type.String({ description: "Remote avatar URL" })), + avatarPath: Type.Optional(Type.String({ description: "Local avatar path" })), + displayName: Type.Optional(Type.String()), + }, + }, + }), + }, + }; + setActivePluginRegistry( + createTestRegistry([{ pluginId: "demo-media", source: "test", plugin: mediaPlugin }]), + ); + + expect( + resolveChannelMessageToolMediaSourceParamKeys({ + cfg: {} as OpenClawConfig, + action: "set-profile", + channel: "demo-media", + }), + ).toEqual(["avatarUrl", "avatarPath"]); + expect( + resolveChannelMessageToolMediaSourceParamKeys({ + cfg: {} as OpenClawConfig, + action: "send", + channel: "demo-media", + }), + ).toEqual([]); + }); + + it("keeps flat media-source param discovery for backward compatibility", () => { + const mediaPlugin: ChannelPlugin = { + ...createChannelTestPluginBase({ + id: "demo-media-flat", + label: "Demo Media Flat", + capabilities: { chatTypes: ["direct", "group"] }, + config: { + listAccountIds: () => ["default"], + }, + }), + actions: { + describeMessageTool: () => ({ + actions: ["set-profile"], + mediaSourceParams: ["avatarUrl", "avatarPath"], + }), + }, + }; + setActivePluginRegistry( + createTestRegistry([{ pluginId: "demo-media-flat", source: "test", plugin: mediaPlugin }]), + ); + + expect( + resolveChannelMessageToolMediaSourceParamKeys({ + cfg: {} as OpenClawConfig, + action: "set-profile", + channel: "demo-media-flat", + }), + ).toEqual(["avatarUrl", "avatarPath"]); + }); + it("skips crashing action/capability discovery paths and logs once", () => { const crashingPlugin: ChannelPlugin = { ...createChannelTestPluginBase({ diff --git a/src/channels/plugins/types.core.ts b/src/channels/plugins/types.core.ts index 065db04e7eb..00c4c044094 100644 --- a/src/channels/plugins/types.core.ts +++ b/src/channels/plugins/types.core.ts @@ -62,10 +62,21 @@ export type ChannelMessageToolSchemaContribution = { visibility?: "current-channel" | "all-configured"; }; +type ChannelMessageToolMediaSourceParams = + | readonly string[] + | Partial>; + export type ChannelMessageToolDiscovery = { actions?: readonly ChannelMessageActionName[] | null; capabilities?: readonly ChannelMessageCapability[] | null; schema?: ChannelMessageToolSchemaContribution | ChannelMessageToolSchemaContribution[] | null; + /** + * Plugin-owned message-tool params that carry media sources. + * Core uses this to derive sandbox path normalization and host media-access + * hints without hardcoding plugin-specific param names. Prefer scoping keys + * by action so unrelated actions do not inherit another action's media args. + */ + mediaSourceParams?: ChannelMessageToolMediaSourceParams | null; }; /** Shared setup input bag used by CLI, onboarding, and setup adapters. */ @@ -630,7 +641,8 @@ export type ChannelMessageActionAdapter = { /** * Unified discovery surface for the shared `message` tool. * This returns the scoped actions, - * capabilities, and schema fragments together so they cannot drift. + * capabilities, schema fragments, and any plugin-owned media-source params + * together so they cannot drift. */ describeMessageTool: ( params: ChannelMessageActionDiscoveryContext, diff --git a/src/gateway/server-reload-handlers.ts b/src/gateway/server-reload-handlers.ts index 42b4774be7c..36d4aff8525 100644 --- a/src/gateway/server-reload-handlers.ts +++ b/src/gateway/server-reload-handlers.ts @@ -106,7 +106,9 @@ export function createGatewayReloadHandlers(params: { } if (plan.restartGmailWatcher) { - await stopGmailWatcher().catch(() => {}); + await stopGmailWatcher().catch((err) => { + params.logHooks.warn(`gmail watcher stop failed during reload: ${String(err)}`); + }); await startGmailWatcherWithLogs({ cfg: nextConfig, log: params.logHooks, diff --git a/src/infra/outbound/message-action-params.test.ts b/src/infra/outbound/message-action-params.test.ts index e7156eb24db..f9a2c910b63 100644 --- a/src/infra/outbound/message-action-params.test.ts +++ b/src/infra/outbound/message-action-params.test.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../../config/config.js"; import { + collectActionMediaSourceHints, hydrateAttachmentParamsForAction, normalizeSandboxMediaList, normalizeSandboxMediaParams, @@ -12,6 +13,12 @@ import { const cfg = {} as OpenClawConfig; const maybeIt = process.platform === "win32" ? it.skip : it; +const matrixMediaSourceParamKeys = [ + "avatarPath", + "avatar_path", + "avatarUrl", + "avatar_url", +] as const; describe("message action media helpers", () => { it("prefers sandbox media policy when sandbox roots are non-blank", () => { @@ -104,6 +111,131 @@ describe("message action media helpers", () => { } }); + maybeIt("normalizes Matrix avatarPath and avatarUrl sandbox media params", async () => { + const sandboxRoot = await fs.mkdtemp(path.join(os.tmpdir(), "msg-params-avatar-")); + try { + const args: Record = { + avatarPath: "/workspace/avatars/profile.png", + avatarUrl: "file:///workspace/avatars/remote-avatar.jpg", + }; + + await normalizeSandboxMediaParams({ + args, + mediaPolicy: { + mode: "sandbox", + sandboxRoot, + }, + extraParamKeys: matrixMediaSourceParamKeys, + }); + + expect(args).toMatchObject({ + avatarPath: path.join(sandboxRoot, "avatars", "profile.png"), + avatarUrl: path.join(sandboxRoot, "avatars", "remote-avatar.jpg"), + }); + } finally { + await fs.rm(sandboxRoot, { recursive: true, force: true }); + } + }); + + it("collects host media source hints from the shared media-source key set", () => { + expect( + collectActionMediaSourceHints( + { + media: " /workspace/uploads/photo.png ", + filePath: "", + image: "file:///workspace/assets/event-cover.png", + avatarPath: "/workspace/avatars/profile.png", + avatar_url: "mxc://matrix.org/abc123def456", + ignored: "/workspace/not-included.png", + }, + matrixMediaSourceParamKeys, + ), + ).toEqual([ + " /workspace/uploads/photo.png ", + "file:///workspace/assets/event-cover.png", + "/workspace/avatars/profile.png", + "mxc://matrix.org/abc123def456", + ]); + }); + + maybeIt("normalizes Matrix snake_case avatar_path and avatar_url aliases", async () => { + const sandboxRoot = await fs.mkdtemp(path.join(os.tmpdir(), "msg-params-avatar-snake-")); + try { + const args: Record = { + avatar_path: "/workspace/avatars/profile.png", + avatar_url: "file:///workspace/avatars/remote-avatar.jpg", + }; + + await normalizeSandboxMediaParams({ + args, + mediaPolicy: { + mode: "sandbox", + sandboxRoot, + }, + extraParamKeys: matrixMediaSourceParamKeys, + }); + + expect(args).toMatchObject({ + avatar_path: path.join(sandboxRoot, "avatars", "profile.png"), + avatar_url: path.join(sandboxRoot, "avatars", "remote-avatar.jpg"), + }); + } finally { + await fs.rm(sandboxRoot, { recursive: true, force: true }); + } + }); + + maybeIt("keeps remote HTTP avatarUrl unchanged under sandbox normalization", async () => { + const sandboxRoot = await fs.mkdtemp(path.join(os.tmpdir(), "msg-params-avatar-remote-")); + try { + const args: Record = { + avatarUrl: "https://example.com/avatars/profile.png", + avatarPath: "/workspace/avatars/local.png", + }; + + await normalizeSandboxMediaParams({ + args, + mediaPolicy: { + mode: "sandbox", + sandboxRoot, + }, + extraParamKeys: matrixMediaSourceParamKeys, + }); + + expect(args).toMatchObject({ + avatarUrl: "https://example.com/avatars/profile.png", + avatarPath: path.join(sandboxRoot, "avatars", "local.png"), + }); + } finally { + await fs.rm(sandboxRoot, { recursive: true, force: true }); + } + }); + + maybeIt("keeps mxc:// avatarUrl unchanged under sandbox normalization", async () => { + const sandboxRoot = await fs.mkdtemp(path.join(os.tmpdir(), "msg-params-avatar-mxc-")); + try { + const args: Record = { + avatarUrl: "mxc://matrix.org/abc123def456", + avatarPath: "/workspace/avatars/local.png", + }; + + await normalizeSandboxMediaParams({ + args, + mediaPolicy: { + mode: "sandbox", + sandboxRoot, + }, + extraParamKeys: matrixMediaSourceParamKeys, + }); + + expect(args).toMatchObject({ + avatarUrl: "mxc://matrix.org/abc123def456", + avatarPath: path.join(sandboxRoot, "avatars", "local.png"), + }); + } finally { + await fs.rm(sandboxRoot, { recursive: true, force: true }); + } + }); + maybeIt( "keeps remote HTTP mediaUrl and fileUrl aliases unchanged under sandbox normalization", async () => { diff --git a/src/infra/outbound/message-action-params.ts b/src/infra/outbound/message-action-params.ts index 5d802bc358f..ed9559113fa 100644 --- a/src/infra/outbound/message-action-params.ts +++ b/src/infra/outbound/message-action-params.ts @@ -1,5 +1,6 @@ import { assertMediaNotDataUrl, resolveSandboxedMediaSource } from "../../agents/sandbox-paths.js"; import { readStringParam } from "../../agents/tools/common.js"; +import { resolveChannelMessageToolMediaSourceParamKeys } from "../../channels/plugins/message-action-discovery.js"; import type { ChannelId, ChannelMessageActionName } from "../../channels/plugins/types.public.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { createRootScopedReadFile } from "../../infra/fs-safe.js"; @@ -13,10 +14,11 @@ import { import { extensionForMime } from "../../media/mime.js"; import { loadWebMedia } from "../../media/web-media.js"; import { readBooleanParam as readBooleanParamShared } from "../../plugin-sdk/boolean-param.js"; +import { normalizeOptionalString } from "../../shared/string-coerce.js"; export const readBooleanParam = readBooleanParamShared; -const SANDBOX_MEDIA_PARAM_KEYS = [ +export const BASE_ACTION_MEDIA_SOURCE_PARAM_KEYS = [ "media", "path", "filePath", @@ -25,13 +27,54 @@ const SANDBOX_MEDIA_PARAM_KEYS = [ "image", ] as const; -function readMediaParam( - args: Record, - key: (typeof SANDBOX_MEDIA_PARAM_KEYS)[number], -): string | undefined { +function readMediaParam(args: Record, key: string): string | undefined { return readStringParam(args, key, { trim: false }); } +function buildActionMediaSourceParamKeys(extraParamKeys?: readonly string[]): string[] { + const keys = new Set(BASE_ACTION_MEDIA_SOURCE_PARAM_KEYS); + extraParamKeys?.forEach((key) => keys.add(key)); + return Array.from(keys); +} + +export function resolveExtraActionMediaSourceParamKeys(params: { + cfg: OpenClawConfig; + action?: ChannelMessageActionName; + channel?: string; + accountId?: string | null; + sessionKey?: string | null; + sessionId?: string | null; + agentId?: string | null; + requesterSenderId?: string | null; + senderIsOwner?: boolean; +}): string[] { + return resolveChannelMessageToolMediaSourceParamKeys({ + cfg: params.cfg, + action: params.action, + channel: params.channel, + accountId: params.accountId, + sessionKey: params.sessionKey, + sessionId: params.sessionId, + agentId: params.agentId, + requesterSenderId: params.requesterSenderId, + senderIsOwner: params.senderIsOwner, + }); +} + +export function collectActionMediaSourceHints( + args: Record, + extraParamKeys?: readonly string[], +): string[] { + const sources: string[] = []; + for (const key of buildActionMediaSourceParamKeys(extraParamKeys)) { + const source = typeof args[key] === "string" ? args[key] : undefined; + if (source && normalizeOptionalString(source)) { + sources.push(source); + } + } + return sources; +} + function readAttachmentMediaHint(args: Record): string | undefined { return readMediaParam(args, "media") ?? readMediaParam(args, "mediaUrl"); } @@ -229,10 +272,11 @@ async function hydrateAttachmentPayload(params: { export async function normalizeSandboxMediaParams(params: { args: Record; mediaPolicy: AttachmentMediaPolicy; + extraParamKeys?: readonly string[]; }): Promise { const sandboxRoot = params.mediaPolicy.mode === "sandbox" ? params.mediaPolicy.sandboxRoot.trim() : undefined; - for (const key of SANDBOX_MEDIA_PARAM_KEYS) { + for (const key of buildActionMediaSourceParamKeys(params.extraParamKeys)) { const raw = readMediaParam(params.args, key); if (!raw) { continue; diff --git a/src/infra/outbound/message-action-runner.media.test.ts b/src/infra/outbound/message-action-runner.media.test.ts index 48786637d79..82fd485d520 100644 --- a/src/infra/outbound/message-action-runner.media.test.ts +++ b/src/infra/outbound/message-action-runner.media.test.ts @@ -1,6 +1,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { Type } from "@sinclair/typebox"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { jsonResult } from "../../agents/tools/common.js"; import type { ChannelPlugin } from "../../channels/plugins/types.js"; @@ -524,6 +525,136 @@ describe("runMessageAction media behavior", () => { }); }); + describe("plugin-owned media-source discovery routing", () => { + const profilePlugin: ChannelPlugin = { + ...createChannelTestPluginBase({ + id: "profile-demo", + label: "Profile Demo", + capabilities: { chatTypes: ["direct"] }, + config: { + listAccountIds: () => ["default"], + isConfigured: () => true, + }, + }), + outbound: { + deliveryMode: "direct", + resolveTarget: ({ to }) => ({ ok: true, to: to?.trim() ?? "profile-demo-target" }), + sendText: async () => ({ channel: "profile-demo", messageId: "msg-test" }), + sendMedia: async () => ({ channel: "profile-demo", messageId: "msg-test" }), + }, + actions: { + describeMessageTool: () => ({ + actions: ["send", "set-profile"], + mediaSourceParams: { + "set-profile": ["avatarPath", "avatarUrl"], + }, + schema: { + properties: { + avatarPath: Type.Optional(Type.String({ description: "Local avatar path" })), + avatarUrl: Type.Optional(Type.String({ description: "Remote avatar URL" })), + displayName: Type.Optional(Type.String()), + }, + }, + }), + supportsAction: ({ action }) => action === "set-profile" || action === "send", + handleAction: async ({ params, mediaLocalRoots }) => + jsonResult({ + ok: true, + avatarPath: params.avatarPath, + avatarUrl: params.avatarUrl, + mediaLocalRoots, + }), + }, + }; + + beforeEach(() => { + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "profile-demo", + source: "test", + plugin: profilePlugin, + }, + ]), + ); + }); + + afterEach(() => { + setActivePluginRegistry(createTestRegistry([])); + }); + + it("rewrites plugin-owned sandbox media params and preserves mxc URLs", async () => { + await withSandbox(async (sandboxDir) => { + const result = await runMessageAction({ + cfg: {} as OpenClawConfig, + action: "set-profile", + params: { + channel: "profile-demo", + avatarPath: "/workspace/avatars/profile.png", + avatarUrl: "mxc://matrix.org/abc123def456", + }, + sandboxRoot: sandboxDir, + }); + + expect(result.kind).toBe("action"); + expect(result.payload).toMatchObject({ + ok: true, + avatarPath: path.join(sandboxDir, "avatars", "profile.png"), + avatarUrl: "mxc://matrix.org/abc123def456", + }); + }); + }); + + it("routes plugin-owned host media hints into local-root expansion", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-profile-media-")); + try { + const avatarPath = path.join(tempDir, "profile.png"); + await fs.writeFile(avatarPath, onePixelPng); + + const result = await runMessageAction({ + cfg: { + tools: { fs: { workspaceOnly: false } }, + } as OpenClawConfig, + action: "set-profile", + params: { + channel: "profile-demo", + avatarPath, + }, + }); + + expect(result.kind).toBe("action"); + expect((result.payload as { mediaLocalRoots?: string[] }).mediaLocalRoots).toEqual( + expect.arrayContaining([tempDir]), + ); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); + + it("does not apply set-profile media params to send actions", async () => { + await withSandbox(async (sandboxDir) => { + const avatarUrl = "data:text/plain;base64,SGVsbG8="; + const result = await runMessageAction({ + cfg: {} as OpenClawConfig, + action: "send", + dryRun: true, + params: { + channel: "profile-demo", + target: "@profile-demo", + message: "hi", + avatarUrl, + }, + sandboxRoot: sandboxDir, + }); + + expect(result.kind).toBe("send"); + expect(result.sendResult).toMatchObject({ + channel: "profile-demo", + }); + }); + }); + }); + describe("sandboxed media validation", () => { beforeEach(() => { setActivePluginRegistry( diff --git a/src/infra/outbound/message-action-runner.ts b/src/infra/outbound/message-action-runner.ts index fb7a1542f0b..5f7737a0b5b 100644 --- a/src/infra/outbound/message-action-runner.ts +++ b/src/infra/outbound/message-action-runner.ts @@ -42,6 +42,7 @@ import { import type { OutboundSendDeps } from "./deliver.js"; import { normalizeMessageActionInput } from "./message-action-normalization.js"; import { + collectActionMediaSourceHints, hydrateAttachmentParamsForAction, normalizeSandboxMediaList, normalizeSandboxMediaParams, @@ -51,6 +52,7 @@ import { parseInteractiveParam, readBooleanParam, resolveAttachmentMediaPolicy, + resolveExtraActionMediaSourceParamKeys, } from "./message-action-params.js"; import { prepareOutboundMirrorRoute, @@ -203,19 +205,6 @@ async function resolveGatewayActionIdempotencyKey(idempotencyKey?: string): Prom const { randomIdempotencyKey } = await loadMessageActionGatewayRuntime(); return randomIdempotencyKey(); } - -function collectActionMediaSourceHints(params: Record): string[] { - const sources: string[] = []; - for (const key of ["media", "mediaUrl", "path", "filePath", "fileUrl"] as const) { - const source = typeof params[key] === "string" ? params[key] : undefined; - const normalized = normalizeOptionalString(source); - if (normalized && source) { - sources.push(source); - } - } - return sources; -} - function applyCrossContextMessageDecoration({ params, message, @@ -861,16 +850,28 @@ export async function runMessageAction( sandboxRoot: input.sandboxRoot, mediaLocalRoots: getAgentScopedMediaLocalRoots(cfg, resolvedAgentId), }); + const extraActionMediaSourceParamKeys = resolveExtraActionMediaSourceParamKeys({ + cfg, + action, + channel, + accountId, + sessionKey: input.sessionKey, + sessionId: input.sessionId, + agentId: resolvedAgentId, + requesterSenderId: input.requesterSenderId, + senderIsOwner: input.senderIsOwner, + }); await normalizeSandboxMediaParams({ args: params, mediaPolicy: normalizationPolicy, + extraParamKeys: extraActionMediaSourceParamKeys, }); const mediaAccess = resolveAgentScopedOutboundMediaAccess({ cfg, agentId: resolvedAgentId, - mediaSources: collectActionMediaSourceHints(params), + mediaSources: collectActionMediaSourceHints(params, extraActionMediaSourceParamKeys), sessionKey: input.sessionKey, messageProvider: input.sessionKey ? undefined : channel, accountId: input.sessionKey ? (input.requesterAccountId ?? accountId) : accountId, diff --git a/src/media/local-roots.test.ts b/src/media/local-roots.test.ts index 8017acfafbf..25f617d909d 100644 --- a/src/media/local-roots.test.ts +++ b/src/media/local-roots.test.ts @@ -128,6 +128,12 @@ describe("local media roots", () => { expect(roots.map(normalizeHostPath)).not.toContain(normalizeHostPath("/")); }); + it("does not widen local roots for pass-through remote media schemes", () => { + const roots = appendLocalMediaParentRoots(["/tmp/base"], ["mxc://matrix.org/abc123def456"]); + + expect(roots.map(normalizeHostPath)).toEqual([normalizeHostPath("/tmp/base")]); + }); + it.each([ { name: "widens agent media roots for concrete local sources when workspaceOnly is disabled", diff --git a/src/media/local-roots.ts b/src/media/local-roots.ts index 0e750efea97..a91052a4315 100644 --- a/src/media/local-roots.ts +++ b/src/media/local-roots.ts @@ -10,13 +10,13 @@ import { safeFileURLToPath } from "../infra/local-file-access.js"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; import { normalizeOptionalString } from "../shared/string-coerce.js"; import { resolveConfigDir, resolveUserPath } from "../utils.js"; +import { isPassThroughRemoteMediaSource } from "./media-source-url.js"; type BuildMediaLocalRootsOptions = { preferredTmpDir?: string; }; let cachedPreferredTmpDir: string | undefined; -const HTTP_URL_RE = /^https?:\/\//i; const DATA_URL_RE = /^data:/i; const WINDOWS_DRIVE_RE = /^[A-Za-z]:[\\/]/; @@ -73,7 +73,7 @@ export function getAgentScopedMediaLocalRoots( function resolveLocalMediaPath(source: string): string | undefined { const trimmed = source.trim(); - if (!trimmed || HTTP_URL_RE.test(trimmed) || DATA_URL_RE.test(trimmed)) { + if (!trimmed || isPassThroughRemoteMediaSource(trimmed) || DATA_URL_RE.test(trimmed)) { return undefined; } if (trimmed.startsWith("file://")) { diff --git a/src/media/media-source-url.ts b/src/media/media-source-url.ts new file mode 100644 index 00000000000..3b2e3bbbf11 --- /dev/null +++ b/src/media/media-source-url.ts @@ -0,0 +1,7 @@ +const HTTP_URL_RE = /^https?:\/\//i; +const MXC_URL_RE = /^mxc:\/\//i; + +export function isPassThroughRemoteMediaSource(value: string | null | undefined): boolean { + const normalized = value?.trim() ?? ""; + return Boolean(normalized) && (HTTP_URL_RE.test(normalized) || MXC_URL_RE.test(normalized)); +}