From 872079d42fe105ece2900a1dd6ab321b92da2d59 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 14 Feb 2026 20:08:38 +0100 Subject: [PATCH] fix(imessage): keep DM pairing-store identities out of group allowlist auth --- ...essages-without-mention-by-default.test.ts | 144 ++++++++++++++++++ src/imessage/monitor/monitor-provider.ts | 90 ++++++++++- 2 files changed, 231 insertions(+), 3 deletions(-) diff --git a/src/imessage/monitor.skips-group-messages-without-mention-by-default.test.ts b/src/imessage/monitor.skips-group-messages-without-mention-by-default.test.ts index 3b8032f28..c5d94d600 100644 --- a/src/imessage/monitor.skips-group-messages-without-mention-by-default.test.ts +++ b/src/imessage/monitor.skips-group-messages-without-mention-by-default.test.ts @@ -31,6 +31,29 @@ function getConfig(): TestConfig { } describe("monitorIMessageProvider", () => { + it("ignores malformed rpc message payloads", async () => { + const run = monitorIMessageProvider(); + await waitForSubscribe(); + + notificationHandler?.({ + method: "message", + params: { + message: { + id: 1, + sender: { nested: "not-a-string" }, + text: "hello", + }, + }, + }); + + await flush(); + closeResolve?.(); + await run; + + expect(replyMock).not.toHaveBeenCalled(); + expect(sendMock).not.toHaveBeenCalled(); + }); + it("skips group messages without a mention by default", async () => { const run = monitorIMessageProvider(); await waitForSubscribe(); @@ -364,6 +387,127 @@ describe("monitorIMessageProvider", () => { await run; expect(replyMock).not.toHaveBeenCalled(); + expect(sendMock).not.toHaveBeenCalled(); + }); + + it("does not allow group sender from pairing store when groupPolicy is allowlist", async () => { + config = { + ...config, + channels: { + ...config.channels, + imessage: { + ...config.channels?.imessage, + dmPolicy: "pairing", + allowFrom: [], + groupPolicy: "allowlist", + groupAllowFrom: [], + }, + }, + }; + readAllowFromStoreMock.mockResolvedValue(["+15550003333"]); + const run = monitorIMessageProvider(); + await waitForSubscribe(); + + notificationHandler?.({ + method: "message", + params: { + message: { + id: 30, + chat_id: 909, + sender: "+15550003333", + is_from_me: false, + text: "@openclaw hi from paired sender", + is_group: true, + }, + }, + }); + + await flush(); + closeResolve?.(); + await run; + + expect(replyMock).not.toHaveBeenCalled(); + expect(sendMock).not.toHaveBeenCalled(); + }); + + it("does not allow sender from pairing store when groupAllowFrom is restricted to a different chat_id", async () => { + config = { + ...config, + channels: { + ...config.channels, + imessage: { + ...config.channels?.imessage, + dmPolicy: "pairing", + allowFrom: [], + groupPolicy: "allowlist", + groupAllowFrom: ["chat_id:101"], + }, + }, + }; + readAllowFromStoreMock.mockResolvedValue(["+15550003333"]); + const run = monitorIMessageProvider(); + await waitForSubscribe(); + + notificationHandler?.({ + method: "message", + params: { + message: { + id: 31, + chat_id: 202, + sender: "+15550003333", + is_from_me: false, + text: "@openclaw hi from paired sender", + is_group: true, + }, + }, + }); + + await flush(); + closeResolve?.(); + await run; + + expect(replyMock).not.toHaveBeenCalled(); + expect(sendMock).not.toHaveBeenCalled(); + }); + + it("does not authorize control command via pairing-store sender in non-allowlisted chat", async () => { + config = { + ...config, + channels: { + ...config.channels, + imessage: { + ...config.channels?.imessage, + dmPolicy: "pairing", + allowFrom: [], + groupPolicy: "allowlist", + groupAllowFrom: ["chat_id:101"], + }, + }, + }; + readAllowFromStoreMock.mockResolvedValue(["+15550003333"]); + const run = monitorIMessageProvider(); + await waitForSubscribe(); + + notificationHandler?.({ + method: "message", + params: { + message: { + id: 32, + chat_id: 202, + sender: "+15550003333", + is_from_me: false, + text: "/status", + is_group: true, + }, + }, + }); + + await flush(); + closeResolve?.(); + await run; + + expect(replyMock).not.toHaveBeenCalled(); + expect(sendMock).not.toHaveBeenCalled(); }); it("blocks group messages when groupPolicy is disabled", async () => { diff --git a/src/imessage/monitor/monitor-provider.ts b/src/imessage/monitor/monitor-provider.ts index 445fe73ae..894a52961 100644 --- a/src/imessage/monitor/monitor-provider.ts +++ b/src/imessage/monitor/monitor-provider.ts @@ -111,6 +111,88 @@ function describeReplyContext(message: IMessagePayload): IMessageReplyContext | return { body, id, sender }; } +function isRecord(value: unknown): value is Record { + return Boolean(value) && typeof value === "object" && !Array.isArray(value); +} + +function isOptionalString(value: unknown): value is string | null | undefined { + return value === undefined || value === null || typeof value === "string"; +} + +function isOptionalStringOrNumber(value: unknown): value is string | number | null | undefined { + return ( + value === undefined || value === null || typeof value === "string" || typeof value === "number" + ); +} + +function isOptionalNumber(value: unknown): value is number | null | undefined { + return value === undefined || value === null || typeof value === "number"; +} + +function isOptionalBoolean(value: unknown): value is boolean | null | undefined { + return value === undefined || value === null || typeof value === "boolean"; +} + +function isOptionalStringArray(value: unknown): value is string[] | null | undefined { + return ( + value === undefined || + value === null || + (Array.isArray(value) && value.every((entry) => typeof entry === "string")) + ); +} + +function isOptionalAttachments(value: unknown): value is IMessagePayload["attachments"] { + if (value === undefined || value === null) { + return true; + } + if (!Array.isArray(value)) { + return false; + } + return value.every((attachment) => { + if (!isRecord(attachment)) { + return false; + } + return ( + isOptionalString(attachment.original_path) && + isOptionalString(attachment.mime_type) && + isOptionalBoolean(attachment.missing) + ); + }); +} + +function parseIMessageNotification(raw: unknown): IMessagePayload | null { + if (!isRecord(raw)) { + return null; + } + const maybeMessage = raw.message; + if (!isRecord(maybeMessage)) { + return null; + } + + const message: IMessagePayload = maybeMessage; + if ( + !isOptionalNumber(message.id) || + !isOptionalNumber(message.chat_id) || + !isOptionalString(message.sender) || + !isOptionalBoolean(message.is_from_me) || + !isOptionalString(message.text) || + !isOptionalStringOrNumber(message.reply_to_id) || + !isOptionalString(message.reply_to_text) || + !isOptionalString(message.reply_to_sender) || + !isOptionalString(message.created_at) || + !isOptionalAttachments(message.attachments) || + !isOptionalString(message.chat_identifier) || + !isOptionalString(message.chat_guid) || + !isOptionalString(message.chat_name) || + !isOptionalStringArray(message.participants) || + !isOptionalBoolean(message.is_group) + ) { + return null; + } + + return message; +} + /** * Cache for recently sent messages, used for echo detection. * Keys are scoped by conversation (accountId:target) so the same text in different chats is not conflated. @@ -294,7 +376,9 @@ export async function monitorIMessageProvider(opts: MonitorIMessageOpts = {}): P const effectiveDmAllowFrom = Array.from(new Set([...allowFrom, ...storeAllowFrom])) .map((v) => String(v).trim()) .filter(Boolean); - const effectiveGroupAllowFrom = Array.from(new Set([...groupAllowFrom, ...storeAllowFrom])) + // Keep DM pairing-store authorization scoped to DMs; group access must come + // from explicit group allowlist config. + const effectiveGroupAllowFrom = Array.from(new Set(groupAllowFrom)) .map((v) => String(v).trim()) .filter(Boolean); @@ -676,9 +760,9 @@ export async function monitorIMessageProvider(opts: MonitorIMessageOpts = {}): P } const handleMessage = async (raw: unknown) => { - const params = raw as { message?: IMessagePayload | null }; - const message = params?.message ?? null; + const message = parseIMessageNotification(raw); if (!message) { + logVerbose("imessage: dropping malformed RPC message payload"); return; } await inboundDebouncer.enqueue({ message });