diff --git a/extensions/bluebubbles/src/actions.test.ts b/extensions/bluebubbles/src/actions.test.ts index b575834217a..eead12620aa 100644 --- a/extensions/bluebubbles/src/actions.test.ts +++ b/extensions/bluebubbles/src/actions.test.ts @@ -530,7 +530,15 @@ describe("bluebubblesMessageActions", () => { accountId: null, }); - expect(resolveBlueBubblesMessageId).toHaveBeenCalledWith("1", { requireKnownShortId: true }); + expect(resolveBlueBubblesMessageId).toHaveBeenCalledWith( + "1", + expect.objectContaining({ + requireKnownShortId: true, + chatContext: expect.objectContaining({ + chatGuid: "iMessage;-;+15551234567", + }), + }), + ); expect(sendBlueBubblesReaction).toHaveBeenCalledWith( expect.objectContaining({ messageGuid: "resolved-uuid", diff --git a/extensions/bluebubbles/src/actions.ts b/extensions/bluebubbles/src/actions.ts index e3d7634b075..2b32e6edf39 100644 --- a/extensions/bluebubbles/src/actions.ts +++ b/extensions/bluebubbles/src/actions.ts @@ -17,9 +17,11 @@ import { type ChannelMessageActionAdapter, type ChannelMessageActionName, } from "./actions-api.js"; +import type { BlueBubblesChatContext } from "./monitor-reply-cache.js"; import { getCachedBlueBubblesPrivateApiStatus, isMacOS26OrHigher } from "./probe.js"; import { normalizeSecretInputString } from "./secret-input.js"; import { + buildBlueBubblesChatContextFromTarget, normalizeBlueBubblesHandle, normalizeBlueBubblesMessagingTarget, parseBlueBubblesTarget, @@ -51,6 +53,32 @@ function mapTarget(raw: string): BlueBubblesSendTarget { }; } +/** + * Collect any chat-identifying hints the action caller supplied, so short + * message id resolution can reject cross-chat collisions. The order of + * precedence mirrors resolveChatGuid: explicit chat* params first, then the + * `to`/`target` param, then the current session channel as a last resort. + */ +function buildChatContextFromActionParams(params: { + actionParams: Record; + currentChannelId?: string; +}): BlueBubblesChatContext { + const explicitChatGuid = readStringParam(params.actionParams, "chatGuid")?.trim(); + const explicitChatIdentifier = readStringParam(params.actionParams, "chatIdentifier")?.trim(); + const explicitChatId = readNumberParam(params.actionParams, "chatId", { integer: true }); + const rawTarget = + readStringParam(params.actionParams, "to") ?? + readStringParam(params.actionParams, "target") ?? + params.currentChannelId ?? + undefined; + const targetContext = buildBlueBubblesChatContextFromTarget(rawTarget); + return { + chatGuid: explicitChatGuid || targetContext.chatGuid, + chatIdentifier: explicitChatIdentifier || targetContext.chatIdentifier, + chatId: typeof explicitChatId === "number" ? explicitChatId : targetContext.chatId, + }; +} + function readMessageText(params: Record): string | undefined { return readStringParam(params, "text") ?? readStringParam(params, "message"); } @@ -201,9 +229,15 @@ export const bluebubblesMessageActions: ChannelMessageActionAdapter = { "Use action=react with messageId=, emoji=, and to/chatGuid to identify the chat.", ); } - // Resolve short ID (e.g., "1", "2") to full UUID + // Resolve short ID (e.g., "1", "2") to full UUID, scoped to the chat the + // caller is acting on so a short ID from a different chat cannot be + // silently accepted (see cross-chat guard in resolveBlueBubblesMessageId). const messageId = runtime.resolveBlueBubblesMessageId(rawMessageId, { requireKnownShortId: true, + chatContext: buildChatContextFromActionParams({ + actionParams: params, + currentChannelId: toolContext?.currentChannelId, + }), }); const partIndex = readNumberParam(params, "partIndex", { integer: true }); const resolvedChatGuid = await resolveChatGuid(); @@ -248,9 +282,14 @@ export const bluebubblesMessageActions: ChannelMessageActionAdapter = { `Use action=edit with messageId=, text=.`, ); } - // Resolve short ID (e.g., "1", "2") to full UUID + // Resolve short ID (e.g., "1", "2") to full UUID, scoped to the chat + // the caller is acting on (cross-chat guard). const messageId = runtime.resolveBlueBubblesMessageId(rawMessageId, { requireKnownShortId: true, + chatContext: buildChatContextFromActionParams({ + actionParams: params, + currentChannelId: toolContext?.currentChannelId, + }), }); const partIndex = readNumberParam(params, "partIndex", { integer: true }); const backwardsCompatMessage = readStringParam(params, "backwardsCompatMessage"); @@ -274,9 +313,14 @@ export const bluebubblesMessageActions: ChannelMessageActionAdapter = { "Use action=unsend with messageId=.", ); } - // Resolve short ID (e.g., "1", "2") to full UUID + // Resolve short ID (e.g., "1", "2") to full UUID, scoped to the chat + // the caller is acting on (cross-chat guard). const messageId = runtime.resolveBlueBubblesMessageId(rawMessageId, { requireKnownShortId: true, + chatContext: buildChatContextFromActionParams({ + actionParams: params, + currentChannelId: toolContext?.currentChannelId, + }), }); const partIndex = readNumberParam(params, "partIndex", { integer: true }); @@ -310,9 +354,14 @@ export const bluebubblesMessageActions: ChannelMessageActionAdapter = { `Use action=reply with messageId=, message=, target=.`, ); } - // Resolve short ID (e.g., "1", "2") to full UUID + // Resolve short ID (e.g., "1", "2") to full UUID, scoped to the chat + // the caller is acting on (cross-chat guard). const messageId = runtime.resolveBlueBubblesMessageId(rawMessageId, { requireKnownShortId: true, + chatContext: buildChatContextFromActionParams({ + actionParams: params, + currentChannelId: toolContext?.currentChannelId, + }), }); const partIndex = readNumberParam(params, "partIndex", { integer: true }); diff --git a/extensions/bluebubbles/src/channel.ts b/extensions/bluebubbles/src/channel.ts index 3f01fa8d39b..44cac6d1235 100644 --- a/extensions/bluebubbles/src/channel.ts +++ b/extensions/bluebubbles/src/channel.ts @@ -46,6 +46,7 @@ import { blueBubblesSetupAdapter } from "./setup-core.js"; import { blueBubblesSetupWizard } from "./setup-surface.js"; import { collectBlueBubblesStatusIssues } from "./status-issues.js"; import { + buildBlueBubblesChatContextFromTarget, extractHandleFromChatGuid, inferBlueBubblesTargetChatType, looksLikeBlueBubblesExplicitTargetId, @@ -320,7 +321,10 @@ export const bluebubblesPlugin: ChannelPlugin 0) { diff --git a/extensions/bluebubbles/src/monitor-reply-cache.test.ts b/extensions/bluebubbles/src/monitor-reply-cache.test.ts new file mode 100644 index 00000000000..0ed76784fb2 --- /dev/null +++ b/extensions/bluebubbles/src/monitor-reply-cache.test.ts @@ -0,0 +1,235 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { + _resetBlueBubblesShortIdState, + rememberBlueBubblesReplyCache, + resolveBlueBubblesMessageId, +} from "./monitor-reply-cache.js"; +import { buildBlueBubblesChatContextFromTarget } from "./targets.js"; + +describe("resolveBlueBubblesMessageId chat-scoped short-id guard", () => { + beforeEach(() => { + _resetBlueBubblesShortIdState(); + }); + + afterEach(() => { + _resetBlueBubblesShortIdState(); + }); + + function seedMessage(args: { + accountId: string; + messageId: string; + chatGuid?: string; + chatIdentifier?: string; + chatId?: number; + }) { + return rememberBlueBubblesReplyCache({ + accountId: args.accountId, + messageId: args.messageId, + chatGuid: args.chatGuid, + chatIdentifier: args.chatIdentifier, + chatId: args.chatId, + timestamp: Date.now(), + }); + } + + it("returns the cached uuid when the short id resolves within the same chatGuid", () => { + const entry = seedMessage({ + accountId: "default", + messageId: "uuid-in-group", + chatGuid: "iMessage;+;chat240698944142298252", + }); + + const resolved = resolveBlueBubblesMessageId(entry.shortId, { + requireKnownShortId: true, + chatContext: { chatGuid: "iMessage;+;chat240698944142298252" }, + }); + + expect(resolved).toBe("uuid-in-group"); + }); + + it("throws when a short id points at a message in a different chatGuid", () => { + const groupEntry = seedMessage({ + accountId: "default", + messageId: "uuid-in-group", + chatGuid: "iMessage;+;chat240698944142298252", + }); + + // Agent tries to react in a DM but passes a short id that was allocated + // for a group message. Should throw instead of silently letting BB + // server route the tapback to the group (or worse, to an old DM that + // happens to share the short id slot). + expect(() => + resolveBlueBubblesMessageId(groupEntry.shortId, { + requireKnownShortId: true, + chatContext: { chatGuid: "iMessage;-;+8618621181874" }, + }), + ).toThrow(/different chat/); + }); + + it("fails open when caller cannot supply any chat identifier", () => { + const entry = seedMessage({ + accountId: "default", + messageId: "uuid-no-ctx", + chatGuid: "iMessage;+;chat240698944142298252", + }); + + // Empty context means "caller could not derive any chat hint" (e.g. + // tool invocation with only messageId). Permit resolution; downstream + // API will still carry whatever chatGuid the call site provides. + const resolved = resolveBlueBubblesMessageId(entry.shortId, { + requireKnownShortId: true, + chatContext: {}, + }); + expect(resolved).toBe("uuid-no-ctx"); + }); + + it("falls back to chatIdentifier comparison when the caller has no chatGuid", () => { + const dmEntry = seedMessage({ + accountId: "default", + messageId: "uuid-dm-1", + chatIdentifier: "+8618621181874", + }); + + expect( + resolveBlueBubblesMessageId(dmEntry.shortId, { + requireKnownShortId: true, + chatContext: { chatIdentifier: "+8618621181874" }, + }), + ).toBe("uuid-dm-1"); + + expect(() => + resolveBlueBubblesMessageId(dmEntry.shortId, { + requireKnownShortId: true, + chatContext: { chatIdentifier: "+8618621185125" }, + }), + ).toThrow(/different chat/); + }); + + it("catches a handle-only caller against a cached entry that carries chatGuid", () => { + // Real-world failure mode: inbound webhooks populate cached entries with + // chatGuid (group or DM). A caller that only resolved a handle supplies + // ctx.chatIdentifier without ctx.chatGuid. The guard must still catch + // the mismatch so a group short-id cannot slip through when the call is + // for a DM, which is exactly how group reactions were leaking into DMs. + const groupEntry = seedMessage({ + accountId: "default", + messageId: "uuid-in-group", + chatGuid: "iMessage;+;chat240698944142298252", + chatIdentifier: "chat240698944142298252", + }); + + expect(() => + resolveBlueBubblesMessageId(groupEntry.shortId, { + requireKnownShortId: true, + chatContext: { chatIdentifier: "+8618621181874" }, + }), + ).toThrow(/different chat/); + }); + + it("falls back to chatId comparison when neither chatGuid nor chatIdentifier is available", () => { + const entry = seedMessage({ + accountId: "default", + messageId: "uuid-with-id", + chatId: 42, + }); + + expect( + resolveBlueBubblesMessageId(entry.shortId, { + requireKnownShortId: true, + chatContext: { chatId: 42 }, + }), + ).toBe("uuid-with-id"); + + expect(() => + resolveBlueBubblesMessageId(entry.shortId, { + requireKnownShortId: true, + chatContext: { chatId: 99 }, + }), + ).toThrow(/different chat/); + }); + + it("accepts a full uuid input unchanged regardless of chat context", () => { + // Non-numeric input is treated as a full GUID already; the guard does + // not apply. Callers supplying the full GUID have presumably resolved + // the chat themselves. + const resolved = resolveBlueBubblesMessageId("1E7E6B6A-0000-4C6C-BCA7-000000000001", { + requireKnownShortId: true, + chatContext: { chatGuid: "iMessage;+;anything" }, + }); + expect(resolved).toBe("1E7E6B6A-0000-4C6C-BCA7-000000000001"); + }); + + it("reports the conflicting chats in the error message for debugability", () => { + const entry = seedMessage({ + accountId: "default", + messageId: "uuid-in-group", + chatGuid: "iMessage;+;chat240698944142298252", + }); + + try { + resolveBlueBubblesMessageId(entry.shortId, { + requireKnownShortId: true, + chatContext: { chatGuid: "iMessage;-;+8618621181874" }, + }); + expect.fail("expected cross-chat guard to throw"); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + expect(message).toContain("iMessage;+;chat240698944142298252"); + expect(message).toContain("iMessage;-;+8618621181874"); + expect(message).toContain("full message GUID"); + } + }); + + it("still throws requireKnownShortId for unknown numeric inputs", () => { + expect(() => + resolveBlueBubblesMessageId("999", { + requireKnownShortId: true, + chatContext: { chatGuid: "iMessage;+;anything" }, + }), + ).toThrow(/no longer available/); + }); + + it("accepts same-chat short ids when the caller's target uses a non-canonical handle format", () => { + // Real-world: a cached entry carries the BlueBubbles-normalized handle + // (`+15551234567`) as its chatIdentifier. A tool call like + // `react to: "imessage:(555) 123-4567"` has to project into the same + // chatIdentifier before the guard compares — otherwise the raw handle + // `(555) 123-4567` would fail the mismatch check against the cached + // `+15551234567` and legitimate same-chat reactions/replies would be + // blocked. + const dmEntry = seedMessage({ + accountId: "default", + messageId: "uuid-dm-handle", + chatIdentifier: "+15551234567", + }); + const cachedChatIdentifier = dmEntry.chatIdentifier; + + for (const target of ["imessage:+15551234567", "sms:+15551234567", "+15551234567"]) { + const ctx = buildBlueBubblesChatContextFromTarget(target); + expect(ctx.chatIdentifier, `ctx.chatIdentifier for ${target}`).toBe(cachedChatIdentifier); + expect( + resolveBlueBubblesMessageId(dmEntry.shortId, { + requireKnownShortId: true, + chatContext: ctx, + }), + `resolve for ${target}`, + ).toBe("uuid-dm-handle"); + } + + // Mixed-case email handle: cached as lowercase; caller supplies mixed + // case. Still resolves. + const emailEntry = seedMessage({ + accountId: "default", + messageId: "uuid-email", + chatIdentifier: "user@example.com", + }); + const emailCtx = buildBlueBubblesChatContextFromTarget("imessage:User@Example.COM"); + expect(emailCtx.chatIdentifier).toBe("user@example.com"); + expect( + resolveBlueBubblesMessageId(emailEntry.shortId, { + requireKnownShortId: true, + chatContext: emailCtx, + }), + ).toBe("uuid-email"); + }); +}); diff --git a/extensions/bluebubbles/src/monitor-reply-cache.ts b/extensions/bluebubbles/src/monitor-reply-cache.ts index 3ff362aa660..329031818dd 100644 --- a/extensions/bluebubbles/src/monitor-reply-cache.ts +++ b/extensions/bluebubbles/src/monitor-reply-cache.ts @@ -81,13 +81,99 @@ export function rememberBlueBubblesReplyCache( return fullEntry; } +export type BlueBubblesChatContext = { + chatGuid?: string; + chatIdentifier?: string; + chatId?: number; +}; + +/** + * Cross-chat guard: compare a cached entry's chat fields with a caller-provided + * context. Returns true when the two clearly reference different chats. + * + * Comparison rules mirror resolveReplyContextFromCache so outbound short-ID + * resolution and inbound reply-context lookup agree on scope: + * + * - If both sides carry a chatGuid and they differ, that is the strongest + * signal of a cross-chat reuse. + * - Otherwise, if the caller has no chatGuid but both sides carry a + * chatIdentifier and they differ, that is also a mismatch. This covers + * handle-only callers (tapback into a DM where the caller only resolved + * a handle) against cached entries that still carry chatGuid from the + * inbound webhook. + * - Otherwise, if the caller has neither chatGuid nor chatIdentifier but + * both sides carry a chatId and they differ, that is also a mismatch. + * + * Absent identifiers on either side are treated as "no information" rather + * than a mismatch, so ambiguous calls fall through as-is. + */ +function isCrossChatMismatch( + cached: BlueBubblesReplyCacheEntry, + ctx: BlueBubblesChatContext, +): boolean { + const cachedChatGuid = normalizeOptionalString(cached.chatGuid); + const ctxChatGuid = normalizeOptionalString(ctx.chatGuid); + if (cachedChatGuid && ctxChatGuid && cachedChatGuid !== ctxChatGuid) { + return true; + } + const cachedChatIdentifier = normalizeOptionalString(cached.chatIdentifier); + const ctxChatIdentifier = normalizeOptionalString(ctx.chatIdentifier); + if ( + !ctxChatGuid && + cachedChatIdentifier && + ctxChatIdentifier && + cachedChatIdentifier !== ctxChatIdentifier + ) { + return true; + } + const cachedChatId = typeof cached.chatId === "number" ? cached.chatId : undefined; + const ctxChatId = typeof ctx.chatId === "number" ? ctx.chatId : undefined; + if ( + !ctxChatGuid && + !ctxChatIdentifier && + cachedChatId !== undefined && + ctxChatId !== undefined && + cachedChatId !== ctxChatId + ) { + return true; + } + return false; +} + +function describeChatForError(values: { + chatGuid?: string; + chatIdentifier?: string; + chatId?: number; +}): string { + const parts: string[] = []; + const guid = normalizeOptionalString(values.chatGuid); + if (guid) { + parts.push(`chatGuid=${guid}`); + } + const identifier = normalizeOptionalString(values.chatIdentifier); + if (identifier) { + parts.push(`chatIdentifier=${identifier}`); + } + if (typeof values.chatId === "number") { + parts.push(`chatId=${values.chatId}`); + } + return parts.length === 0 ? "" : parts.join(", "); +} + /** * Resolves a short message ID (e.g., "1", "2") to a full BlueBubbles GUID. * Returns the input unchanged if it's already a GUID or not found in the mapping. + * + * When `chatContext` is provided, the resolved UUID's cached chat must match + * the caller's chat or the call throws. This prevents a short ID that points + * at a message in chat A from being silently reused in chat B — the common + * symptom being tapbacks and quoted replies landing in the wrong conversation + * (e.g. a group reaction showing up in a DM) because short IDs are allocated + * from a single global counter across every account and chat. */ export function resolveBlueBubblesMessageId( shortOrUuid: string, - opts?: { requireKnownShortId?: boolean }, + opts?: { requireKnownShortId?: boolean; chatContext?: BlueBubblesChatContext }, ): string { const trimmed = shortOrUuid.trim(); if (!trimmed) { @@ -98,6 +184,17 @@ export function resolveBlueBubblesMessageId( if (/^\d+$/.test(trimmed)) { const uuid = blueBubblesShortIdToUuid.get(trimmed); if (uuid) { + if (opts?.chatContext) { + const cached = blueBubblesReplyCacheByMessageId.get(uuid); + if (cached && isCrossChatMismatch(cached, opts.chatContext)) { + throw new Error( + `BlueBubbles short message id "${trimmed}" belongs to a different chat ` + + `(${describeChatForError(cached)}) than the current call target ` + + `(${describeChatForError(opts.chatContext)}). Retry with the full message GUID ` + + `to avoid cross-chat reactions/replies landing in the wrong conversation.`, + ); + } + } return uuid; } if (opts?.requireKnownShortId) { diff --git a/extensions/bluebubbles/src/targets.ts b/extensions/bluebubbles/src/targets.ts index 83ae14eab50..9a7b1424386 100644 --- a/extensions/bluebubbles/src/targets.ts +++ b/extensions/bluebubbles/src/targets.ts @@ -426,3 +426,52 @@ export function formatBlueBubblesChatTarget(params: { } return ""; } + +/** + * Derive a chat context ({chatGuid, chatIdentifier, chatId}) from a raw + * BlueBubbles target string such as `chat_guid:iMessage;+;chat123`, + * `chat_id:42`, `imessage:+15551234567`, or a bare handle. Returns an empty + * object for unparseable input. + * + * Used by short-ID message resolution to constrain short IDs to the chat the + * caller is acting on, preventing a short ID allocated for a message in one + * chat from silently pointing at a different chat on a later tool call. + */ +export function buildBlueBubblesChatContextFromTarget(raw: string | undefined | null): { + chatGuid?: string; + chatIdentifier?: string; + chatId?: number; +} { + const trimmed = normalizeOptionalString(raw); + if (!trimmed) { + return {}; + } + try { + const parsed = parseBlueBubblesTarget(trimmed); + if (parsed.kind === "chat_guid") { + return { chatGuid: parsed.chatGuid }; + } + if (parsed.kind === "chat_identifier") { + return { chatIdentifier: parsed.chatIdentifier }; + } + if (parsed.kind === "chat_id") { + return { chatId: parsed.chatId }; + } + if (parsed.kind === "handle") { + // BlueBubbles chat records store DM handles in the third component of + // their chatGuid (service;-;address), and `chatIdentifier` on a chat + // record is typically the same address. Treat a handle target as a + // chatIdentifier hint; it disambiguates DM↔DM and DM↔group mixes. + // Normalize the handle (strip service prefix / whitespace / lowercase + // emails) so the comparison matches what the send path resolves to + // and what inbound webhooks write into the reply cache; otherwise + // formats like `imessage:(555) 123-4567` or mixed-case email handles + // would compare unequal against their normalized cached form and + // legitimate same-chat short IDs would be rejected as cross-chat. + return { chatIdentifier: normalizeBlueBubblesHandle(parsed.to) }; + } + return {}; + } catch { + return {}; + } +}