mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-10 02:30:44 +00:00
fix(imessage): wire reply attachments through send-rich --file (with feature gate) (#79864)
Merged via squash.
Prepared head SHA: 5e5cdfed79
Co-authored-by: omarshahine <10343873+omarshahine@users.noreply.github.com>
Co-authored-by: omarshahine <10343873+omarshahine@users.noreply.github.com>
Reviewed-by: @omarshahine
This commit is contained in:
@@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Slack: wake the resolved thread session after interactive reply button/select clicks and carry Slack delivery context through the queued interaction event, so clicks continue the visible conversation. Fixes #79676 and #61502. (#79836) Thanks @velvet-shark, @tianxiaochannel-oss88, and @Saicheg.
|
||||
- WhatsApp/streaming: send only the new suffix when text-end block replies repeat prior preambles across tool-call cycles, preventing cumulative WhatsApp preamble messages. Fixes #78946. (#79120) Thanks @brokemac79 and @papawattu.
|
||||
- Tests/security audit: sandbox `audit-exec-surface.test.ts` under a per-case OpenClaw home tempdir, redirecting `OPENCLAW_HOME` (which wins over `HOME`/`USERPROFILE` in `resolveRawHomeDir`) alongside `HOME` and `USERPROFILE`, so its `saveExecApprovals(...)` calls never touch the live `~/.openclaw/exec-approvals.json` on the host running the suite. Sibling exec-approvals tests already used the tempdir pattern; this file did not, so running `pnpm test` against a contributor's local checkout was silently truncating their real approvals to `{ "version": 1, "agents": {} }`. (#79885) Thanks @omarshahine.
|
||||
- Channels/iMessage: wire `action: "reply"` attachments through `imsg send-rich --file` when the installed imsg build advertises that capability (probed once via `imsg send-rich --help` and cached on the private-API status). Reply now hydrates `media`/`mediaUrl`/`fileUrl`/`mediaUrls[0]`/`filePath`/`path`/base64 `buffer`+`filename` through the shared outbound resolver, stages buffers via the existing `withTempFile` helper, rejects `http(s)://` URL attachments with a targeted error pointing callers at `send`'s full attachment-resolver pipeline, and falls back to the explicit `imsg#114 not landed yet` error on older imsg builds. Depends on the upstream `openclaw/imsg#114` capability landing in an installable release; until then the new path stays gated and users see the same explicit fallback `#79822` introduced. (#79864) Thanks @omarshahine.
|
||||
|
||||
## 2026.5.9
|
||||
|
||||
|
||||
@@ -395,6 +395,15 @@ export const imessageActionsRuntime = {
|
||||
effectId?: string;
|
||||
replyToMessageId?: string;
|
||||
partIndex?: number;
|
||||
// Optional attachment as an in-memory buffer that we stage to a temp
|
||||
// file before invoking imsg. The buffer must already have been loaded
|
||||
// by the outbound media resolver (mediaLocalRoots/sandbox/size limits)
|
||||
// — this runtime intentionally does not accept a raw filesystem path,
|
||||
// because that would let an attacker-controlled path bypass the
|
||||
// resolver and let imsg send any host-readable file. Requires an imsg
|
||||
// build that accepts `send-rich --file` (openclaw/imsg#114); callers
|
||||
// must feature-detect via the cached private-api status first.
|
||||
attachment?: { kind: "buffer"; buffer: Uint8Array; filename: string };
|
||||
options: IMessageBridgeActionOptions;
|
||||
}): Promise<IMessageBridgeSendResult> {
|
||||
// Extract markdown bold/italic/underline/strikethrough into typed-run
|
||||
@@ -403,21 +412,31 @@ export const imessageActionsRuntime = {
|
||||
// any caller that hits the bridge via `imsg send-rich` benefits without
|
||||
// needing to pre-format the text themselves.
|
||||
const formatted = extractMarkdownFormatRuns(params.text);
|
||||
const result = await runIMessageCliJson(
|
||||
[
|
||||
"send-rich",
|
||||
"--chat",
|
||||
params.chatGuid,
|
||||
"--text",
|
||||
formatted.text,
|
||||
"--part",
|
||||
String(params.partIndex ?? 0),
|
||||
...(params.effectId ? ["--effect", params.effectId] : []),
|
||||
...(params.replyToMessageId ? ["--reply-to", params.replyToMessageId] : []),
|
||||
...(formatted.ranges.length > 0 ? ["--format", JSON.stringify(formatted.ranges)] : []),
|
||||
],
|
||||
params.options,
|
||||
);
|
||||
const buildArgs = (filePath?: string): string[] => [
|
||||
"send-rich",
|
||||
"--chat",
|
||||
params.chatGuid,
|
||||
"--text",
|
||||
formatted.text,
|
||||
"--part",
|
||||
String(params.partIndex ?? 0),
|
||||
...(params.effectId ? ["--effect", params.effectId] : []),
|
||||
...(params.replyToMessageId ? ["--reply-to", params.replyToMessageId] : []),
|
||||
...(formatted.ranges.length > 0 ? ["--format", JSON.stringify(formatted.ranges)] : []),
|
||||
...(filePath ? ["--file", filePath] : []),
|
||||
];
|
||||
|
||||
if (params.attachment) {
|
||||
return await withTempFile(
|
||||
{ buffer: params.attachment.buffer, filename: params.attachment.filename },
|
||||
async (filePath) => {
|
||||
const result = await runIMessageCliJson(buildArgs(filePath), params.options);
|
||||
return { messageId: resolveMessageId(result) };
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
const result = await runIMessageCliJson(buildArgs(), params.options);
|
||||
return { messageId: resolveMessageId(result) };
|
||||
},
|
||||
|
||||
|
||||
@@ -278,6 +278,144 @@ describe("imessage message actions", () => {
|
||||
);
|
||||
});
|
||||
|
||||
describe("reply with attachment (openclaw/imsg#114 plumbing)", () => {
|
||||
// The core message-action runner hydrates path/media/filePath/etc.
|
||||
// through the outbound media resolver (mediaLocalRoots/sandbox/size)
|
||||
// before reaching this handler, writing the result into `buffer` +
|
||||
// `filename`. These tests cover the post-hydration contract: the
|
||||
// handler trusts only the buffer and refuses any unhydrated path
|
||||
// param so an agent cannot bypass the resolver.
|
||||
const stringPath = "/tmp/cute-lobster.png";
|
||||
const base64Png = Buffer.from("PNGDATA").toString("base64");
|
||||
|
||||
function readLastAttachment():
|
||||
| {
|
||||
kind?: string;
|
||||
buffer?: Uint8Array;
|
||||
filename?: string;
|
||||
}
|
||||
| undefined {
|
||||
const call = runtimeMock.sendRichMessage.mock.calls.at(-1)?.[0] as
|
||||
| { attachment?: { kind: string; buffer?: Uint8Array; filename?: string } }
|
||||
| undefined;
|
||||
return call?.attachment;
|
||||
}
|
||||
|
||||
it("threads a hydrated buffer attachment through to sendRichMessage when imsg supports send-rich --file", async () => {
|
||||
probeMock.getCachedIMessagePrivateApiStatus.mockReturnValue({
|
||||
available: true,
|
||||
v2Ready: true,
|
||||
selectors: {},
|
||||
cliCapabilities: { sendRichSupportsAttachment: true },
|
||||
});
|
||||
runtimeMock.resolveChatGuidForTarget.mockResolvedValue("iMessage;+;resolved-ident");
|
||||
runtimeMock.sendRichMessage.mockResolvedValue({ messageId: "reply-guid" });
|
||||
|
||||
await imessageMessageActions.handleAction?.({
|
||||
action: "reply",
|
||||
cfg: cfg(),
|
||||
params: {
|
||||
chatIdentifier: "team-thread",
|
||||
messageId: "message-guid",
|
||||
text: "🦞 here it is",
|
||||
buffer: base64Png,
|
||||
filename: "card.png",
|
||||
},
|
||||
} as never);
|
||||
expect(runtimeMock.sendRichMessage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ replyToMessageId: "message-guid" }),
|
||||
);
|
||||
const attachment = readLastAttachment();
|
||||
expect(attachment?.kind).toBe("buffer");
|
||||
expect(attachment?.filename).toBe("card.png");
|
||||
expect(Buffer.from(attachment?.buffer ?? new Uint8Array()).toString()).toBe("PNGDATA");
|
||||
});
|
||||
|
||||
it("falls back to attachment.bin when filename is missing (post-hydration)", async () => {
|
||||
probeMock.getCachedIMessagePrivateApiStatus.mockReturnValue({
|
||||
available: true,
|
||||
v2Ready: true,
|
||||
selectors: {},
|
||||
cliCapabilities: { sendRichSupportsAttachment: true },
|
||||
});
|
||||
runtimeMock.resolveChatGuidForTarget.mockResolvedValue("iMessage;+;resolved-ident");
|
||||
runtimeMock.sendRichMessage.mockResolvedValue({ messageId: "reply-guid" });
|
||||
|
||||
await imessageMessageActions.handleAction?.({
|
||||
action: "reply",
|
||||
cfg: cfg(),
|
||||
params: {
|
||||
chatIdentifier: "team-thread",
|
||||
messageId: "message-guid",
|
||||
text: "🦞 here it is",
|
||||
buffer: base64Png,
|
||||
},
|
||||
} as never);
|
||||
expect(readLastAttachment()?.filename).toBe("attachment.bin");
|
||||
});
|
||||
|
||||
it("rejects unhydrated path-shaped params so agents cannot bypass the media resolver", async () => {
|
||||
// The runner's hydrateAttachmentParamsForAction loads any
|
||||
// path/media/filePath/mediaUrl/fileUrl through the media resolver
|
||||
// and writes the result into `buffer`. If we ever see a path-shaped
|
||||
// param without a `buffer`, hydration was skipped — refuse instead
|
||||
// of forwarding a raw host path to imsg.
|
||||
probeMock.getCachedIMessagePrivateApiStatus.mockReturnValue({
|
||||
available: true,
|
||||
v2Ready: true,
|
||||
selectors: {},
|
||||
cliCapabilities: { sendRichSupportsAttachment: true },
|
||||
});
|
||||
runtimeMock.resolveChatGuidForTarget.mockResolvedValue("iMessage;+;resolved-ident");
|
||||
|
||||
for (const field of ["filePath", "path", "media", "mediaUrl", "fileUrl"]) {
|
||||
runtimeMock.sendRichMessage.mockClear();
|
||||
await expect(
|
||||
imessageMessageActions.handleAction?.({
|
||||
action: "reply",
|
||||
cfg: cfg(),
|
||||
params: {
|
||||
chatIdentifier: "team-thread",
|
||||
messageId: "message-guid",
|
||||
text: "🦞 here it is",
|
||||
[field]: stringPath,
|
||||
},
|
||||
} as never),
|
||||
).rejects.toThrow(/did not pass through the outbound media resolver/);
|
||||
expect(runtimeMock.sendRichMessage).not.toHaveBeenCalled();
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects reply + attachment when imsg does not advertise send-rich --file", async () => {
|
||||
// Older imsg builds reject `--file` on send-rich, so refuse loudly
|
||||
// here rather than letting send-rich ship the text alone and silently
|
||||
// drop the attachment (the original openclaw/openclaw#79822 symptom).
|
||||
probeMock.getCachedIMessagePrivateApiStatus.mockReturnValue({
|
||||
available: true,
|
||||
v2Ready: true,
|
||||
selectors: {},
|
||||
cliCapabilities: { sendRichSupportsAttachment: false },
|
||||
});
|
||||
runtimeMock.resolveChatGuidForTarget.mockResolvedValue("iMessage;+;resolved-ident");
|
||||
|
||||
runtimeMock.sendRichMessage.mockClear();
|
||||
await expect(
|
||||
imessageMessageActions.handleAction?.({
|
||||
action: "reply",
|
||||
cfg: cfg(),
|
||||
params: {
|
||||
chatIdentifier: "team-thread",
|
||||
messageId: "message-guid",
|
||||
text: "🦞 here it is",
|
||||
buffer: base64Png,
|
||||
filename: "card.png",
|
||||
},
|
||||
} as never),
|
||||
).rejects.toThrow(/needs an imsg build that exposes `send-rich --file`/);
|
||||
expect(runtimeMock.sendRichMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("phone-number target end-to-end (regressions caught the hard way)", () => {
|
||||
it("synthesizes iMessage;-;<phone> chat_identifier from a handle target and sends through to sendReaction", async () => {
|
||||
// Scenario from prod: agent calls react with `target:"+12069106512"` and a
|
||||
|
||||
@@ -240,6 +240,51 @@ function decodeBase64Buffer(params: Record<string, unknown>, action: string): Ui
|
||||
return Uint8Array.from(Buffer.from(base64Buffer, "base64"));
|
||||
}
|
||||
|
||||
// Path-shaped attachment params the message-tool schema declares. We only
|
||||
// look at these to detect an unhydrated bypass attempt — the resolver in
|
||||
// hydrateAttachmentParamsForAction is responsible for loading them into
|
||||
// `buffer`/`filename` after enforcing localRoots, sandbox, and size limits.
|
||||
const REPLY_ATTACHMENT_PATH_PARAM_NAMES: readonly string[] = [
|
||||
"filePath",
|
||||
"path",
|
||||
"media",
|
||||
"mediaUrl",
|
||||
"fileUrl",
|
||||
] as const;
|
||||
|
||||
type ReplyAttachmentSpec = { kind: "buffer"; buffer: Uint8Array; filename: string };
|
||||
|
||||
// Reply attachments must arrive hydrated: the core message-action runner
|
||||
// loads `path`/`media`/`mediaUrl`/`filePath`/`fileUrl` through the outbound
|
||||
// media resolver (mediaLocalRoots / sandbox / size limits / SSRF) and writes
|
||||
// the result into `buffer` + `filename`. We deliberately do not consume raw
|
||||
// path params here — accepting them would let an agent send any host file
|
||||
// imsg can read, bypassing the resolver. If a path-shaped param is present
|
||||
// without a corresponding `buffer`, the caller skipped hydration (most
|
||||
// likely calling handleAction directly in a test); fail loudly instead.
|
||||
function extractReplyAttachment(
|
||||
params: Record<string, unknown>,
|
||||
): { spec: ReplyAttachmentSpec; sourceParam: string } | { spec: null; bypassParam: string } | null {
|
||||
const buffer = readStringParam(params, "buffer");
|
||||
if (buffer) {
|
||||
const filename = readStringParam(params, "filename") ?? "attachment.bin";
|
||||
return {
|
||||
spec: {
|
||||
kind: "buffer",
|
||||
buffer: Uint8Array.from(Buffer.from(buffer, "base64")),
|
||||
filename,
|
||||
},
|
||||
sourceParam: "buffer",
|
||||
};
|
||||
}
|
||||
for (const name of REPLY_ATTACHMENT_PATH_PARAM_NAMES) {
|
||||
if (readStringParam(params, name)) {
|
||||
return { spec: null, bypassParam: name };
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
// Whitelist of expressive-send effect IDs the bridge accepts. Restricting
|
||||
// to a fixed set lets us return a clear error for typos ("invisible_ink"
|
||||
// vs "invisibleink") instead of silently forwarding gibberish to the
|
||||
@@ -468,6 +513,28 @@ export const imessageMessageActions: ChannelMessageActionAdapter = {
|
||||
if (!text) {
|
||||
throw new Error("iMessage reply requires text or message.");
|
||||
}
|
||||
const attachment = extractReplyAttachment(params);
|
||||
if (attachment) {
|
||||
if (attachment.spec === null) {
|
||||
throw new Error(
|
||||
`iMessage reply rejected \`${attachment.bypassParam}\` because it did not pass through the outbound media resolver. ` +
|
||||
'Pass a base64 `buffer` + `filename` directly, or invoke message(action: "reply") through the runner so the resolver ' +
|
||||
"can validate the path against mediaLocalRoots/sandbox/size before sending.",
|
||||
);
|
||||
}
|
||||
// Reply-with-attachment requires the `imsg send-rich --file` flag
|
||||
// (openclaw/imsg#114). Older imsg builds reject the option, so
|
||||
// refuse loudly here rather than letting send-rich ship the text
|
||||
// alone and silently drop the attachment — the original symptom
|
||||
// of openclaw/openclaw#79822.
|
||||
if (privateApiStatus?.cliCapabilities?.sendRichSupportsAttachment !== true) {
|
||||
throw new Error(
|
||||
"iMessage reply with an attachment needs an imsg build that exposes `send-rich --file` " +
|
||||
"(openclaw/imsg#114). Upgrade imsg, or use action 'upload-file' (with filePath/filename) " +
|
||||
"or action 'send' (with media) to deliver the file plus a separate 'reply' for any text.",
|
||||
);
|
||||
}
|
||||
}
|
||||
const partIndex = readNumberParam(params, "partIndex", { integer: true });
|
||||
const resolvedChatGuid = await chatGuid();
|
||||
const result = await runtime.sendRichMessage({
|
||||
@@ -475,6 +542,7 @@ export const imessageMessageActions: ChannelMessageActionAdapter = {
|
||||
text,
|
||||
replyToMessageId: resolvedMessageId,
|
||||
partIndex: typeof partIndex === "number" ? partIndex : undefined,
|
||||
attachment: attachment?.spec ?? undefined,
|
||||
options: { ...opts, chatGuid: resolvedChatGuid },
|
||||
});
|
||||
return jsonResult({ ok: true, messageId: result.messageId, repliedTo: resolvedMessageId });
|
||||
|
||||
@@ -3,6 +3,12 @@ export type IMessagePrivateApiStatus = {
|
||||
v2Ready: boolean;
|
||||
selectors: Record<string, boolean>;
|
||||
rpcMethods: string[];
|
||||
// CLI-flag-level capabilities probed from `imsg <cmd> --help`. Only fields
|
||||
// we actively branch on are listed; missing entries mean "not yet probed"
|
||||
// and callers should treat them as unsupported.
|
||||
cliCapabilities?: {
|
||||
sendRichSupportsAttachment?: boolean;
|
||||
};
|
||||
error?: string;
|
||||
};
|
||||
|
||||
|
||||
@@ -150,6 +150,28 @@ function rpcMethodsFromPayload(payload: Record<string, unknown>): string[] {
|
||||
return raw.filter((entry): entry is string => typeof entry === "string");
|
||||
}
|
||||
|
||||
// Probe whether the installed imsg CLI accepts `--file` on the `send-rich`
|
||||
// subcommand (added by openclaw/imsg#114, which lets a single bridge call
|
||||
// combine `--reply-to` and an attachment). We grep the help output rather
|
||||
// than trying a real send so the probe is side-effect-free, and we resolve
|
||||
// to `false` on any failure (timeout, non-zero exit, missing binary) so
|
||||
// callers fall back to the legacy throw rather than silently dropping.
|
||||
async function probeSendRichSupportsAttachment(
|
||||
cliPath: string,
|
||||
timeoutMs: number,
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
const result = await runCommandWithTimeout([cliPath, "send-rich", "--help"], { timeoutMs });
|
||||
if (result.code !== 0) {
|
||||
return false;
|
||||
}
|
||||
const combined = `${result.stdout}\n${result.stderr}`;
|
||||
return /(?:^|\s)--file\b/m.test(combined);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function clearIMessagePrivateApiCache(cliPath?: string): void {
|
||||
if (cliPath) {
|
||||
const key = cliPath.trim() || "imsg";
|
||||
@@ -181,11 +203,19 @@ export async function probeIMessagePrivateApi(
|
||||
const rpcMethods = payload ? rpcMethodsFromPayload(payload) : [];
|
||||
const advancedFeatures = payload?.advanced_features === true;
|
||||
const v2Ready = payload?.v2_ready === true;
|
||||
// Probe `imsg send-rich --help` for the `--file` flag added by
|
||||
// openclaw/imsg#114. We do this even when the bridge is unavailable
|
||||
// because the help output ships with the CLI binary itself, and the
|
||||
// result is what gates whether reply-with-attachment can route through
|
||||
// the threaded send path. Treat any failure as "not supported" so
|
||||
// callers fall back to the legacy throw rather than silently dropping.
|
||||
const sendRichSupportsAttachment = await probeSendRichSupportsAttachment(key, timeoutMs);
|
||||
const status: NonNullable<IMessageProbe["privateApi"]> = {
|
||||
available: result.code === 0 && advancedFeatures && v2Ready,
|
||||
v2Ready,
|
||||
selectors,
|
||||
rpcMethods,
|
||||
cliCapabilities: { sendRichSupportsAttachment },
|
||||
...(result.code === 0
|
||||
? !payload && firstLineSnippet
|
||||
? {
|
||||
@@ -208,6 +238,7 @@ export async function probeIMessagePrivateApi(
|
||||
v2Ready: false,
|
||||
selectors: {},
|
||||
rpcMethods: [],
|
||||
cliCapabilities: { sendRichSupportsAttachment: false },
|
||||
error: String(err),
|
||||
};
|
||||
setCachedIMessagePrivateApiStatus(key, status, Date.now() + PRIVATE_API_NEGATIVE_TTL_MS);
|
||||
|
||||
@@ -407,6 +407,50 @@ describe("message action media helpers", () => {
|
||||
|
||||
expect(args.filename).toBe("attachment");
|
||||
});
|
||||
|
||||
it("hydrates reply attachments through the resolver so threaded sends don't bypass mediaLocalRoots", async () => {
|
||||
// Locks in coverage for the reply-with-attachment path: when an agent
|
||||
// calls message(action: "reply") with a `path`/`media`/etc., the
|
||||
// resolver — not the channel runtime — must run. Pre-PR this was
|
||||
// gated only on sendAttachment/setGroupIcon/upload-file, letting
|
||||
// imessage reply forward an arbitrary host path to imsg.
|
||||
const args: Record<string, unknown> = {
|
||||
mediaUrl: "https://example.com/cute.png",
|
||||
};
|
||||
|
||||
await hydrateAttachmentParamsForAction({
|
||||
cfg,
|
||||
channel: "imessage",
|
||||
args,
|
||||
action: "reply",
|
||||
dryRun: true,
|
||||
mediaPolicy: { mode: "host" },
|
||||
});
|
||||
|
||||
expect(args.filename).toBe("cute.png");
|
||||
});
|
||||
|
||||
it("does not fall back caption->message on reply (reply has its own text field)", async () => {
|
||||
// sendAttachment uses caption as the body text and falls back from
|
||||
// message -> caption when the agent only supplied `message`. Reply has
|
||||
// its own `text`/`message` field, so caption fallback would invent a
|
||||
// bogus caption param on the reply payload.
|
||||
const args: Record<string, unknown> = {
|
||||
mediaUrl: "https://example.com/cute.png",
|
||||
message: "🦞",
|
||||
};
|
||||
|
||||
await hydrateAttachmentParamsForAction({
|
||||
cfg,
|
||||
channel: "imessage",
|
||||
args,
|
||||
action: "reply",
|
||||
dryRun: true,
|
||||
mediaPolicy: { mode: "host" },
|
||||
});
|
||||
|
||||
expect(args.caption).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("message action sandbox media hydration", () => {
|
||||
|
||||
@@ -384,9 +384,14 @@ export async function hydrateAttachmentParamsForAction(params: {
|
||||
mediaPolicy: AttachmentMediaPolicy;
|
||||
}): Promise<void> {
|
||||
const shouldHydrateUploadFile = params.action === "upload-file";
|
||||
// Reply gets the same hydration as sendAttachment so threaded sends with
|
||||
// an attachment go through the resolver's localRoots/sandbox/size checks
|
||||
// instead of forwarding raw paths to the channel runtime. Reply has its
|
||||
// own `text`/`message` field, so don't fall back caption -> message.
|
||||
if (
|
||||
params.action !== "sendAttachment" &&
|
||||
params.action !== "setGroupIcon" &&
|
||||
params.action !== "reply" &&
|
||||
!shouldHydrateUploadFile
|
||||
) {
|
||||
return;
|
||||
|
||||
@@ -548,6 +548,160 @@ describe("runMessageAction media behavior", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("reply hydration", () => {
|
||||
// The reply action accepts attachments via the same media/path/filePath
|
||||
// params as send. Before openclaw#79864 the runner only hydrated
|
||||
// sendAttachment/setGroupIcon/upload-file, so a channel plugin's reply
|
||||
// handler saw the raw path and could forward it directly to its CLI —
|
||||
// bypassing localRoots, sandbox, and size checks. These tests pin the
|
||||
// wiring at the runner level: paths must arrive at the plugin handler
|
||||
// as a hydrated buffer, paths outside the resolver's policy must
|
||||
// reject before the handler runs, and reply must not inherit the
|
||||
// sendAttachment caption-fallback that would synthesize a bogus
|
||||
// caption from the agent's reply text.
|
||||
const cfg = {
|
||||
channels: {
|
||||
replychat: {
|
||||
enabled: true,
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
const handleActionMock = vi.fn();
|
||||
const replyPlugin: ChannelPlugin = {
|
||||
id: "replychat",
|
||||
meta: {
|
||||
id: "replychat",
|
||||
label: "ReplyChat",
|
||||
selectionLabel: "ReplyChat",
|
||||
docsPath: "/channels/replychat",
|
||||
blurb: "ReplyChat test plugin.",
|
||||
},
|
||||
capabilities: { chatTypes: ["direct", "group"], media: true },
|
||||
config: {
|
||||
listAccountIds: () => ["default"],
|
||||
resolveAccount: () => ({ enabled: true }),
|
||||
isConfigured: () => true,
|
||||
},
|
||||
actions: {
|
||||
describeMessageTool: () => ({ actions: ["reply"] }),
|
||||
supportsAction: ({ action }) => action === "reply",
|
||||
handleAction: async ({ params }) => {
|
||||
handleActionMock(params);
|
||||
return jsonResult({
|
||||
ok: true,
|
||||
buffer: params.buffer,
|
||||
filename: params.filename,
|
||||
caption: params.caption,
|
||||
contentType: params.contentType,
|
||||
text: params.text,
|
||||
message: params.message,
|
||||
});
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
handleActionMock.mockReset();
|
||||
setActivePluginRegistry(
|
||||
createTestRegistry([
|
||||
{
|
||||
pluginId: "replychat",
|
||||
source: "test",
|
||||
plugin: replyPlugin,
|
||||
},
|
||||
]),
|
||||
);
|
||||
vi.mocked(loadWebMedia).mockResolvedValue({
|
||||
buffer: Buffer.from("hello"),
|
||||
contentType: "image/png",
|
||||
kind: "image",
|
||||
fileName: "pic.png",
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
setActivePluginRegistry(createTestRegistry([]));
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("hydrates buffer and filename from a remote URL before the reply handler runs", async () => {
|
||||
const result = await runMessageAction({
|
||||
cfg,
|
||||
action: "reply",
|
||||
params: {
|
||||
channel: "replychat",
|
||||
target: "+15551234567",
|
||||
messageId: "parent-id",
|
||||
text: "look at this",
|
||||
media: "https://example.com/pic.png",
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.kind).toBe("action");
|
||||
expect(handleActionMock).toHaveBeenCalledTimes(1);
|
||||
const handlerParams = handleActionMock.mock.calls[0]?.[0] as Record<string, unknown>;
|
||||
expect(handlerParams.buffer).toBe(Buffer.from("hello").toString("base64"));
|
||||
expect(handlerParams.filename).toBe("pic.png");
|
||||
expect(handlerParams.contentType).toBe("image/png");
|
||||
});
|
||||
|
||||
it("rejects host paths outside mediaLocalRoots before invoking the reply handler", async () => {
|
||||
// Use the real loader so its localRoots/workspaceOnly enforcement runs.
|
||||
const actual = await vi.importActual<typeof import("../../media/web-media.js")>(
|
||||
"../../media/web-media.js",
|
||||
);
|
||||
vi.mocked(loadWebMedia).mockImplementation(actual.loadWebMedia);
|
||||
|
||||
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-reply-bypass-"));
|
||||
try {
|
||||
const outsidePath = path.join(tempDir, "secret.txt");
|
||||
await fs.writeFile(outsidePath, "secret", "utf8");
|
||||
|
||||
await expect(
|
||||
runMessageAction({
|
||||
cfg: {
|
||||
...cfg,
|
||||
tools: { fs: { workspaceOnly: true } },
|
||||
},
|
||||
action: "reply",
|
||||
params: {
|
||||
channel: "replychat",
|
||||
target: "+15551234567",
|
||||
messageId: "parent-id",
|
||||
text: "look at this",
|
||||
path: outsidePath,
|
||||
},
|
||||
}),
|
||||
).rejects.toThrow(/allowed directory|path-not-allowed|workspace/i);
|
||||
expect(handleActionMock).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
await fs.rm(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("does not synthesize a caption from message on reply", async () => {
|
||||
// sendAttachment falls back caption -> message when caption is missing.
|
||||
// Reply has its own text/message body, so caption fallback would
|
||||
// invent a bogus caption param the channel handler shouldn't see.
|
||||
await runMessageAction({
|
||||
cfg,
|
||||
action: "reply",
|
||||
params: {
|
||||
channel: "replychat",
|
||||
target: "+15551234567",
|
||||
messageId: "parent-id",
|
||||
message: "look at this",
|
||||
media: "https://example.com/pic.png",
|
||||
},
|
||||
});
|
||||
|
||||
expect(handleActionMock).toHaveBeenCalledTimes(1);
|
||||
const handlerParams = handleActionMock.mock.calls[0]?.[0] as Record<string, unknown>;
|
||||
expect(handlerParams.caption).toBeUndefined();
|
||||
expect(handlerParams.message).toBe("look at this");
|
||||
});
|
||||
});
|
||||
|
||||
describe("plugin-owned media-source discovery routing", () => {
|
||||
const profilePlugin: ChannelPlugin = {
|
||||
...createChannelTestPluginBase({
|
||||
|
||||
Reference in New Issue
Block a user