From f334ca2b509f9c9a653582176bba4123a315bdbc Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Fri, 17 Apr 2026 13:58:08 -0400 Subject: [PATCH] Auto-reply: fast-path sandbox media root resolution --- extensions/imessage/contract-api.ts | 8 +- extensions/imessage/media-contract-api.ts | 7 + ...tage-sandbox-media.scp-remote-path.test.ts | 8 + ...bound-media-into-sandbox-workspace.test.ts | 175 ++++++++++-------- src/auto-reply/reply/stage-sandbox-media.ts | 4 +- .../stage-sandbox-media.test-harness.ts | 2 +- .../channel-inbound-roots.fast-path.test.ts | 142 ++++++++++++++ src/media/channel-inbound-roots.ts | 83 +++++++++ 8 files changed, 343 insertions(+), 86 deletions(-) create mode 100644 extensions/imessage/media-contract-api.ts create mode 100644 src/media/channel-inbound-roots.fast-path.test.ts diff --git a/extensions/imessage/contract-api.ts b/extensions/imessage/contract-api.ts index 25a36a96735..8347289b0c2 100644 --- a/extensions/imessage/contract-api.ts +++ b/extensions/imessage/contract-api.ts @@ -1,12 +1,10 @@ -export { - resolveIMessageAttachmentRoots as resolveInboundAttachmentRoots, - resolveIMessageRemoteAttachmentRoots as resolveRemoteInboundAttachmentRoots, -} from "./src/media-contract.js"; export { DEFAULT_IMESSAGE_ATTACHMENT_ROOTS, + resolveIMessageAttachmentRoots as resolveInboundAttachmentRoots, resolveIMessageAttachmentRoots, + resolveIMessageRemoteAttachmentRoots as resolveRemoteInboundAttachmentRoots, resolveIMessageRemoteAttachmentRoots, -} from "./src/media-contract.js"; +} from "./media-contract-api.js"; export { __testing as imessageConversationBindingTesting, createIMessageConversationBindingManager, diff --git a/extensions/imessage/media-contract-api.ts b/extensions/imessage/media-contract-api.ts new file mode 100644 index 00000000000..f6ae1fd7279 --- /dev/null +++ b/extensions/imessage/media-contract-api.ts @@ -0,0 +1,7 @@ +export { + DEFAULT_IMESSAGE_ATTACHMENT_ROOTS, + resolveIMessageAttachmentRoots, + resolveIMessageAttachmentRoots as resolveInboundAttachmentRoots, + resolveIMessageRemoteAttachmentRoots, + resolveIMessageRemoteAttachmentRoots as resolveRemoteInboundAttachmentRoots, +} from "./src/media-contract.js"; diff --git a/src/auto-reply/reply.stage-sandbox-media.scp-remote-path.test.ts b/src/auto-reply/reply.stage-sandbox-media.scp-remote-path.test.ts index e905c984d88..cd793d07420 100644 --- a/src/auto-reply/reply.stage-sandbox-media.scp-remote-path.test.ts +++ b/src/auto-reply/reply.stage-sandbox-media.scp-remote-path.test.ts @@ -13,8 +13,12 @@ const sandboxMocks = vi.hoisted(() => ({ const childProcessMocks = vi.hoisted(() => ({ spawn: vi.fn(), })); +const mediaRootMocks = vi.hoisted(() => ({ + resolveChannelRemoteInboundAttachmentRoots: vi.fn(), +})); vi.mock("../agents/sandbox.js", () => sandboxMocks); +vi.mock("../media/channel-inbound-roots.js", () => mediaRootMocks); vi.mock("node:child_process", async () => { const actual = await vi.importActual("node:child_process"); return { @@ -28,6 +32,7 @@ import { stageSandboxMedia } from "./reply/stage-sandbox-media.js"; afterEach(() => { vi.restoreAllMocks(); childProcessMocks.spawn.mockClear(); + mediaRootMocks.resolveChannelRemoteInboundAttachmentRoots.mockReset(); }); function createRemoteStageParams(home: string): { @@ -38,6 +43,9 @@ function createRemoteStageParams(home: string): { } { const sessionKey = "agent:main:main"; vi.mocked(sandboxMocks.ensureSandboxWorkspaceForSession).mockResolvedValue(null); + mediaRootMocks.resolveChannelRemoteInboundAttachmentRoots.mockReturnValue([ + "/Users/demo/Library/Messages/Attachments", + ]); return { cfg: createSandboxMediaStageConfig(home), workspaceDir: join(home, "openclaw"), diff --git a/src/auto-reply/reply.triggers.trigger-handling.stages-inbound-media-into-sandbox-workspace.test.ts b/src/auto-reply/reply.triggers.trigger-handling.stages-inbound-media-into-sandbox-workspace.test.ts index b71ae36c656..d4cbbd12630 100644 --- a/src/auto-reply/reply.triggers.trigger-handling.stages-inbound-media-into-sandbox-workspace.test.ts +++ b/src/auto-reply/reply.triggers.trigger-handling.stages-inbound-media-into-sandbox-workspace.test.ts @@ -1,7 +1,8 @@ import fs from "node:fs/promises"; import path, { basename, dirname, join } from "node:path"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { MEDIA_MAX_BYTES } from "../media/store.js"; +import { stageSandboxMedia } from "./reply/stage-sandbox-media.js"; import { createSandboxMediaContexts, createSandboxMediaStageConfig, @@ -10,93 +11,111 @@ import { const sandboxMocks = vi.hoisted(() => ({ ensureSandboxWorkspaceForSession: vi.fn(), + assertSandboxPath: vi.fn(), })); const childProcessMocks = vi.hoisted(() => ({ spawn: vi.fn(), })); -const sandboxModuleId = new URL("../agents/sandbox.js", import.meta.url).pathname; -const fsSafeModuleId = new URL("../infra/fs-safe.js", import.meta.url).pathname; +const fsSafeMocks = vi.hoisted(() => { + class MockSafeOpenError extends Error { + readonly code: string; -let stageSandboxMedia: typeof import("./reply/stage-sandbox-media.js").stageSandboxMedia; + constructor(code: string, message: string) { + super(message); + this.name = "SafeOpenError"; + this.code = code; + } + } -async function loadFreshStageSandboxMediaModuleForTest() { - vi.resetModules(); - vi.doMock(sandboxModuleId, () => sandboxMocks); - vi.doMock("node:child_process", async () => { - const actual = await vi.importActual("node:child_process"); - return { - ...actual, - spawn: childProcessMocks.spawn, - }; - }); - vi.doMock(fsSafeModuleId, async () => { - const actual = await vi.importActual(fsSafeModuleId); - return { - ...actual, - copyFileWithinRoot: vi.fn(async ({ sourcePath, rootDir, relativePath, maxBytes }) => { - const sourceStat = await fs.stat(sourcePath); - if (typeof maxBytes === "number" && sourceStat.size > maxBytes) { - throw new actual.SafeOpenError( - "too-large", - `file exceeds limit of ${maxBytes} bytes (got ${sourceStat.size})`, - ); - } - - await fs.mkdir(rootDir, { recursive: true }); - const rootReal = await fs.realpath(rootDir); - const destPath = path.resolve(rootReal, relativePath); - const rootPrefix = `${rootReal}${path.sep}`; - if (destPath !== rootReal && !destPath.startsWith(rootPrefix)) { - throw new actual.SafeOpenError("outside-workspace", "file is outside workspace root"); - } - - const parentDir = dirname(destPath); - const relativeParent = path.relative(rootReal, parentDir); - if (relativeParent && !relativeParent.startsWith("..")) { - let cursor = rootReal; - for (const segment of relativeParent.split(path.sep)) { - cursor = path.join(cursor, segment); - try { - const stat = await fs.lstat(cursor); - if (stat.isSymbolicLink()) { - throw new actual.SafeOpenError("symlink", "symlink not allowed"); - } - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") { - await fs.mkdir(cursor, { recursive: true }); - continue; - } - throw error; - } - } - } - - try { - const destStat = await fs.lstat(destPath); - if (destStat.isSymbolicLink()) { - throw new actual.SafeOpenError("symlink", "symlink not allowed"); - } - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "ENOENT") { - throw error; - } - } - - await fs.copyFile(sourcePath, destPath); - }), - }; - }); - const replyModule = await import("./reply/stage-sandbox-media.js"); return { - stageSandboxMedia: replyModule.stageSandboxMedia, + SafeOpenError: MockSafeOpenError, + copyFileWithinRoot: vi.fn(), + readLocalFileSafely: vi.fn(), }; +}); +const mediaRootMocks = vi.hoisted(() => ({ + resolveChannelRemoteInboundAttachmentRoots: vi.fn(), +})); + +vi.mock("../agents/sandbox.js", () => sandboxMocks); +vi.mock("../agents/sandbox-paths.js", () => ({ + assertSandboxPath: sandboxMocks.assertSandboxPath, +})); +vi.mock("node:child_process", () => childProcessMocks); +vi.mock("../infra/fs-safe.js", () => fsSafeMocks); +vi.mock("../media/channel-inbound-roots.js", () => mediaRootMocks); + +async function copyFileWithinRootForTest({ + sourcePath, + rootDir, + relativePath, + maxBytes, +}: { + sourcePath: string; + rootDir: string; + relativePath: string; + maxBytes?: number; +}) { + const sourceStat = await fs.stat(sourcePath); + if (typeof maxBytes === "number" && sourceStat.size > maxBytes) { + throw new fsSafeMocks.SafeOpenError( + "too-large", + `file exceeds limit of ${maxBytes} bytes (got ${sourceStat.size})`, + ); + } + + await fs.mkdir(rootDir, { recursive: true }); + const rootReal = await fs.realpath(rootDir); + const destPath = path.resolve(rootReal, relativePath); + const rootPrefix = `${rootReal}${path.sep}`; + if (destPath !== rootReal && !destPath.startsWith(rootPrefix)) { + throw new fsSafeMocks.SafeOpenError("outside-workspace", "file is outside workspace root"); + } + + const parentDir = dirname(destPath); + const relativeParent = path.relative(rootReal, parentDir); + if (relativeParent && !relativeParent.startsWith("..")) { + let cursor = rootReal; + for (const segment of relativeParent.split(path.sep)) { + cursor = path.join(cursor, segment); + try { + const stat = await fs.lstat(cursor); + if (stat.isSymbolicLink()) { + throw new fsSafeMocks.SafeOpenError("symlink", "symlink not allowed"); + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + await fs.mkdir(cursor, { recursive: true }); + continue; + } + throw error; + } + } + } + + try { + const destStat = await fs.lstat(destPath); + if (destStat.isSymbolicLink()) { + throw new fsSafeMocks.SafeOpenError("symlink", "symlink not allowed"); + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; + } + } + + await fs.copyFile(sourcePath, destPath); } -async function loadStageSandboxMediaInTempHome() { +beforeEach(() => { sandboxMocks.ensureSandboxWorkspaceForSession.mockReset(); + sandboxMocks.assertSandboxPath.mockReset().mockResolvedValue({ resolved: "", relative: "" }); childProcessMocks.spawn.mockClear(); - ({ stageSandboxMedia } = await loadFreshStageSandboxMediaModuleForTest()); -} + fsSafeMocks.copyFileWithinRoot.mockReset().mockImplementation(copyFileWithinRootForTest); + mediaRootMocks.resolveChannelRemoteInboundAttachmentRoots + .mockReset() + .mockReturnValue(["/Users/demo/Library/Messages/Attachments"]); +}); afterEach(() => { vi.restoreAllMocks(); @@ -134,7 +153,6 @@ async function writeInboundMedia( describe("stageSandboxMedia", () => { it("stages allowed media and blocks unsafe paths", async () => { await withSandboxMediaTempHome("openclaw-triggers-", async (home) => { - await loadStageSandboxMediaInTempHome(); const { cfg, workspaceDir, sandboxDir } = await setupSandboxWorkspace(home); { @@ -179,6 +197,7 @@ describe("stageSandboxMedia", () => { } { + expect(mediaRootMocks.resolveChannelRemoteInboundAttachmentRoots).not.toHaveBeenCalled(); childProcessMocks.spawn.mockClear(); const { ctx, sessionCtx } = createSandboxMediaContexts("/etc/passwd"); ctx.Provider = "imessage"; @@ -202,7 +221,6 @@ describe("stageSandboxMedia", () => { it("blocks destination symlink escapes when staging into sandbox workspace", async () => { await withSandboxMediaTempHome("openclaw-triggers-", async (home) => { - await loadStageSandboxMediaInTempHome(); const { cfg, workspaceDir, sandboxDir } = await setupSandboxWorkspace(home); const mediaPath = await writeInboundMedia(home, "payload.txt", "PAYLOAD"); @@ -234,7 +252,6 @@ describe("stageSandboxMedia", () => { it("skips oversized media staging and keeps original media paths", async () => { await withSandboxMediaTempHome("openclaw-triggers-", async (home) => { - await loadStageSandboxMediaInTempHome(); const { cfg, workspaceDir, sandboxDir } = await setupSandboxWorkspace(home); const mediaPath = await writeInboundMedia( diff --git a/src/auto-reply/reply/stage-sandbox-media.ts b/src/auto-reply/reply/stage-sandbox-media.ts index 46e4980427f..1aab3f7371b 100644 --- a/src/auto-reply/reply/stage-sandbox-media.ts +++ b/src/auto-reply/reply/stage-sandbox-media.ts @@ -48,7 +48,9 @@ export async function stageSandboxMedia(params: { } await fs.mkdir(effectiveWorkspaceDir, { recursive: true }); - const remoteAttachmentRoots = resolveChannelRemoteInboundAttachmentRoots({ cfg, ctx }) ?? []; + const remoteAttachmentRoots = ctx.MediaRemoteHost + ? (resolveChannelRemoteInboundAttachmentRoots({ cfg, ctx }) ?? []) + : []; const usedNames = new Set(); const staged = new Map(); // absolute source -> relative sandbox path diff --git a/src/auto-reply/stage-sandbox-media.test-harness.ts b/src/auto-reply/stage-sandbox-media.test-harness.ts index f9c36364cb2..f72613f3f6f 100644 --- a/src/auto-reply/stage-sandbox-media.test-harness.ts +++ b/src/auto-reply/stage-sandbox-media.test-harness.ts @@ -7,7 +7,7 @@ export async function withSandboxMediaTempHome( prefix: string, fn: (home: string) => Promise, ): Promise { - return withTempHomeBase(async (home) => await fn(home), { prefix }); + return withTempHomeBase(async (home) => await fn(home), { prefix, skipSessionCleanup: true }); } export function createSandboxMediaContexts(mediaPath: string): { diff --git a/src/media/channel-inbound-roots.fast-path.test.ts b/src/media/channel-inbound-roots.fast-path.test.ts new file mode 100644 index 00000000000..803229ecb1f --- /dev/null +++ b/src/media/channel-inbound-roots.fast-path.test.ts @@ -0,0 +1,142 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { MsgContext } from "../auto-reply/templating.js"; +import type { OpenClawConfig } from "../config/types.js"; + +const publicSurfaceLoaderMocks = vi.hoisted(() => ({ + loadBundledPluginPublicArtifactModuleSync: vi.fn(), +})); +const bootstrapRegistryMocks = vi.hoisted(() => ({ + getBootstrapChannelPlugin: vi.fn(), +})); + +vi.mock("../plugins/public-surface-loader.js", () => publicSurfaceLoaderMocks); +vi.mock("../channels/plugins/bootstrap-registry.js", () => bootstrapRegistryMocks); + +import { + resolveChannelInboundAttachmentRoots, + resolveChannelRemoteInboundAttachmentRoots, +} from "./channel-inbound-roots.js"; + +const cfg = { + channels: {}, +} as OpenClawConfig; + +function unableToResolve(dirName: string, artifactBasename: string): Error { + return new Error( + `Unable to resolve bundled plugin public surface ${dirName}/${artifactBasename}`, + ); +} + +function createContext(provider: string, accountId = "work"): MsgContext { + return { + Body: "hi", + From: "imessage:work:demo", + To: "+2000", + ChatType: "direct", + Provider: provider, + AccountId: accountId, + }; +} + +beforeEach(() => { + publicSurfaceLoaderMocks.loadBundledPluginPublicArtifactModuleSync.mockReset(); + bootstrapRegistryMocks.getBootstrapChannelPlugin.mockReset(); +}); + +describe("channel inbound roots fast path", () => { + it("prefers media contract artifacts over full channel bootstrap", () => { + publicSurfaceLoaderMocks.loadBundledPluginPublicArtifactModuleSync.mockImplementation( + ({ artifactBasename, dirName }: { artifactBasename: string; dirName: string }) => { + if (dirName === "imessage" && artifactBasename === "media-contract-api.js") { + return { + resolveInboundAttachmentRoots: ({ accountId }: { accountId?: string }) => [ + `/local/${accountId}`, + ], + resolveRemoteInboundAttachmentRoots: ({ accountId }: { accountId?: string }) => [ + `/remote/${accountId}`, + ], + }; + } + throw unableToResolve(dirName, artifactBasename); + }, + ); + + expect( + resolveChannelInboundAttachmentRoots({ + cfg, + ctx: createContext("imessage"), + }), + ).toEqual(["/local/work"]); + expect( + resolveChannelRemoteInboundAttachmentRoots({ + cfg, + ctx: createContext("imessage"), + }), + ).toEqual(["/remote/work"]); + expect(bootstrapRegistryMocks.getBootstrapChannelPlugin).not.toHaveBeenCalled(); + expect(publicSurfaceLoaderMocks.loadBundledPluginPublicArtifactModuleSync).toHaveBeenCalledWith( + { + dirName: "imessage", + artifactBasename: "media-contract-api.js", + }, + ); + }); + + it("falls back to generic contract artifacts before full channel bootstrap", () => { + publicSurfaceLoaderMocks.loadBundledPluginPublicArtifactModuleSync.mockImplementation( + ({ artifactBasename, dirName }: { artifactBasename: string; dirName: string }) => { + if (dirName === "legacy-channel" && artifactBasename === "contract-api.js") { + return { + resolveRemoteInboundAttachmentRoots: () => ["/legacy-remote"], + }; + } + throw unableToResolve(dirName, artifactBasename); + }, + ); + + expect( + resolveChannelRemoteInboundAttachmentRoots({ + cfg, + ctx: createContext("legacy-channel"), + }), + ).toEqual(["/legacy-remote"]); + expect(bootstrapRegistryMocks.getBootstrapChannelPlugin).not.toHaveBeenCalled(); + expect(publicSurfaceLoaderMocks.loadBundledPluginPublicArtifactModuleSync).toHaveBeenCalledWith( + { + dirName: "legacy-channel", + artifactBasename: "media-contract-api.js", + }, + ); + expect(publicSurfaceLoaderMocks.loadBundledPluginPublicArtifactModuleSync).toHaveBeenCalledWith( + { + dirName: "legacy-channel", + artifactBasename: "contract-api.js", + }, + ); + }); + + it("uses channel bootstrap when no public root contract exists", () => { + publicSurfaceLoaderMocks.loadBundledPluginPublicArtifactModuleSync.mockImplementation( + ({ artifactBasename, dirName }: { artifactBasename: string; dirName: string }) => { + throw unableToResolve(dirName, artifactBasename); + }, + ); + bootstrapRegistryMocks.getBootstrapChannelPlugin.mockReturnValue({ + messaging: { + resolveRemoteInboundAttachmentRoots: ({ accountId }: { accountId?: string }) => [ + `/bootstrap/${accountId}`, + ], + }, + }); + + expect( + resolveChannelRemoteInboundAttachmentRoots({ + cfg, + ctx: createContext("bootstrap-channel"), + }), + ).toEqual(["/bootstrap/work"]); + expect(bootstrapRegistryMocks.getBootstrapChannelPlugin).toHaveBeenCalledWith( + "bootstrap-channel", + ); + }); +}); diff --git a/src/media/channel-inbound-roots.ts b/src/media/channel-inbound-roots.ts index 41259b6bfdf..c03e759748d 100644 --- a/src/media/channel-inbound-roots.ts +++ b/src/media/channel-inbound-roots.ts @@ -1,8 +1,71 @@ import type { MsgContext } from "../auto-reply/templating.js"; import { getBootstrapChannelPlugin } from "../channels/plugins/bootstrap-registry.js"; import type { OpenClawConfig } from "../config/types.js"; +import { loadBundledPluginPublicArtifactModuleSync } from "../plugins/public-surface-loader.js"; import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js"; +type ChannelMediaContractApi = { + resolveInboundAttachmentRoots?: (params: { + cfg: OpenClawConfig; + accountId?: string; + }) => readonly string[] | undefined; + resolveRemoteInboundAttachmentRoots?: (params: { + cfg: OpenClawConfig; + accountId?: string; + }) => readonly string[] | undefined; +}; +type ChannelMediaRootResolver = keyof ChannelMediaContractApi; + +const mediaContractApiByResolver = new Map(); + +function mediaContractCacheKey(channelId: string, resolver: ChannelMediaRootResolver): string { + return `${channelId}:${resolver}`; +} + +function loadChannelMediaContractApi( + channelId: string, + resolver: ChannelMediaRootResolver, +): ChannelMediaContractApi | undefined { + const cacheKey = mediaContractCacheKey(channelId, resolver); + if (mediaContractApiByResolver.has(cacheKey)) { + return mediaContractApiByResolver.get(cacheKey) ?? undefined; + } + + for (const artifactBasename of ["media-contract-api.js", "contract-api.js"]) { + try { + const loaded = loadBundledPluginPublicArtifactModuleSync({ + dirName: channelId, + artifactBasename, + }); + if (typeof loaded[resolver] === "function") { + mediaContractApiByResolver.set(cacheKey, loaded); + return loaded; + } + } catch (error) { + if ( + error instanceof Error && + error.message.startsWith("Unable to resolve bundled plugin public surface ") + ) { + continue; + } + } + } + + mediaContractApiByResolver.set(cacheKey, null); + return undefined; +} + +function findChannelMediaContractApi( + channelId: string | null | undefined, + resolver: ChannelMediaRootResolver, +) { + const normalized = normalizeOptionalLowercaseString(channelId); + if (!normalized) { + return undefined; + } + return loadChannelMediaContractApi(normalized, resolver); +} + function findChannelMessagingAdapter(channelId?: string | null) { const normalized = normalizeOptionalLowercaseString(channelId); if (!normalized) { @@ -15,6 +78,16 @@ export function resolveChannelInboundAttachmentRoots(params: { cfg: OpenClawConfig; ctx: MsgContext; }): readonly string[] | undefined { + const contractApi = findChannelMediaContractApi( + params.ctx.Surface ?? params.ctx.Provider, + "resolveInboundAttachmentRoots", + ); + if (contractApi?.resolveInboundAttachmentRoots) { + return contractApi.resolveInboundAttachmentRoots({ + cfg: params.cfg, + accountId: params.ctx.AccountId, + }); + } const messaging = findChannelMessagingAdapter(params.ctx.Surface ?? params.ctx.Provider); return messaging?.resolveInboundAttachmentRoots?.({ cfg: params.cfg, @@ -26,6 +99,16 @@ export function resolveChannelRemoteInboundAttachmentRoots(params: { cfg: OpenClawConfig; ctx: MsgContext; }): readonly string[] | undefined { + const contractApi = findChannelMediaContractApi( + params.ctx.Surface ?? params.ctx.Provider, + "resolveRemoteInboundAttachmentRoots", + ); + if (contractApi?.resolveRemoteInboundAttachmentRoots) { + return contractApi.resolveRemoteInboundAttachmentRoots({ + cfg: params.cfg, + accountId: params.ctx.AccountId, + }); + } const messaging = findChannelMessagingAdapter(params.ctx.Surface ?? params.ctx.Provider); return messaging?.resolveRemoteInboundAttachmentRoots?.({ cfg: params.cfg,