From 6ade32042182082ac3c92eb4cffca6dbb9ef61fe Mon Sep 17 00:00:00 2001 From: Chris Zhang Date: Sat, 25 Apr 2026 12:16:44 +0800 Subject: [PATCH] fix(bluebubbles): apply cross-chat guard to full message GUIDs as well MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cross-chat guard added in the prior commit (resolveBlueBubblesMessageId with chatContext) only ran on numeric short ids — `if (/^\d+$/.test(trimmed))`. Full GUID input fell through to `return trimmed` with no chat check. Once the short-id guard started rejecting cross-chat reuses, agents would retry the same call with the full GUID copied from history or a previous tool result. That second attempt bypassed the guard entirely and the group reaction landed in the DM anyway — exactly the symptom the prior commit was meant to close. Apply the same `isCrossChatMismatch` check to full GUID input. Cache miss still falls through (callers may legitimately supply a fresh-from-the-wire GUID the cache hasn't observed yet), but cache hits with a chat mismatch throw with a remediation hint pointed at the chat target rather than at the id format — telling an agent to "retry with the full GUID" makes no sense when it already supplied one. Tests (extensions/bluebubbles/src/monitor-reply-cache.test.ts): - UUID + same chat → resolves - UUID + different chat → throws (this is the regression) - UUID + cache miss → passes through (preserves behavior for fresh GUIDs) - UUID + empty chatContext → passes through (preserves prior behavior) - UUID error message hints at the chat target, not the id format - chatIdentifier fallback applies to UUID input too Local patch for upstream consideration — completes the cross-chat guard started in the prior commit so both id forms are protected symmetrically. --- .../src/monitor-reply-cache.test.ts | 98 ++++++++++++++++++- .../bluebubbles/src/monitor-reply-cache.ts | 41 ++++++-- 2 files changed, 127 insertions(+), 12 deletions(-) diff --git a/extensions/bluebubbles/src/monitor-reply-cache.test.ts b/extensions/bluebubbles/src/monitor-reply-cache.test.ts index 0ed76784fb2..64c769b0aeb 100644 --- a/extensions/bluebubbles/src/monitor-reply-cache.test.ts +++ b/extensions/bluebubbles/src/monitor-reply-cache.test.ts @@ -148,10 +148,10 @@ describe("resolveBlueBubblesMessageId chat-scoped short-id guard", () => { ).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. + it("passes a full uuid through unchanged when not in the reply cache", () => { + // Cache miss falls through. Callers supplying a GUID that the cache + // hasn't observed get the input back so fresh-from-the-wire GUIDs + // (e.g. from a `find` API call) still work. const resolved = resolveBlueBubblesMessageId("1E7E6B6A-0000-4C6C-BCA7-000000000001", { requireKnownShortId: true, chatContext: { chatGuid: "iMessage;+;anything" }, @@ -159,6 +159,96 @@ describe("resolveBlueBubblesMessageId chat-scoped short-id guard", () => { expect(resolved).toBe("1E7E6B6A-0000-4C6C-BCA7-000000000001"); }); + it("passes a full uuid through unchanged when caller supplies no chat context", () => { + // Belt-and-braces: even when the cache knows the GUID, callers that + // can't supply any chat hint at all (legacy tool invocations) fall + // through to preserve prior behavior. + seedMessage({ + accountId: "default", + messageId: "uuid-known", + chatGuid: "iMessage;+;chat240698944142298252", + }); + expect(resolveBlueBubblesMessageId("uuid-known")).toBe("uuid-known"); + expect(resolveBlueBubblesMessageId("uuid-known", { chatContext: {} })).toBe("uuid-known"); + }); + + it("accepts a full uuid that points at a same-chat cached entry", () => { + seedMessage({ + accountId: "default", + messageId: "uuid-in-group", + chatGuid: "iMessage;+;chat240698944142298252", + }); + + const resolved = resolveBlueBubblesMessageId("uuid-in-group", { + chatContext: { chatGuid: "iMessage;+;chat240698944142298252" }, + }); + expect(resolved).toBe("uuid-in-group"); + }); + + it("REJECTS a full uuid that points at a different chat in the cache", () => { + // Candidate-1 regression: the previous implementation only ran the + // cross-chat guard on numeric short ids. After the short-id guard + // landed, agents that retried with a full GUID (because the short id + // got rejected) silently bypassed the check. Group GUIDs reused in + // DM tool calls again leaked group reactions into DMs. + seedMessage({ + accountId: "default", + messageId: "uuid-in-group", + chatGuid: "iMessage;+;chat240698944142298252", + }); + + expect(() => + resolveBlueBubblesMessageId("uuid-in-group", { + chatContext: { chatGuid: "iMessage;-;+8618621181874" }, + }), + ).toThrow(/different chat/); + }); + + it("uuid-path error message hints at fixing the chat target, not the id format", () => { + // The short-id error tells the agent to retry with the full GUID. + // For UUID input that's already failed, advising "use the full GUID" + // would be wrong — the agent already supplied one. Make the + // remediation hint differ so a retrying agent is steered toward + // fixing the chat target. + seedMessage({ + accountId: "default", + messageId: "uuid-in-group", + chatGuid: "iMessage;+;chat240698944142298252", + }); + + try { + resolveBlueBubblesMessageId("uuid-in-group", { + 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("correct chat target"); + expect(message).not.toContain("Retry with the full message GUID"); + } + }); + + it("applies the chatIdentifier fallback to full uuid input as well", () => { + // Same handle-only-caller scenario as the short-id case: a tool + // invocation might only resolve the chatIdentifier (the bare handle). + // The guard must catch GUID reuse across mismatched chatIdentifiers + // even when the caller has no chatGuid hint. + seedMessage({ + accountId: "default", + messageId: "uuid-in-group", + chatGuid: "iMessage;+;chat240698944142298252", + chatIdentifier: "chat240698944142298252", + }); + + expect(() => + resolveBlueBubblesMessageId("uuid-in-group", { + chatContext: { chatIdentifier: "+8618621181874" }, + }), + ).toThrow(/different chat/); + }); + it("reports the conflicting chats in the error message for debugability", () => { const entry = seedMessage({ accountId: "default", diff --git a/extensions/bluebubbles/src/monitor-reply-cache.ts b/extensions/bluebubbles/src/monitor-reply-cache.ts index 329031818dd..334b5985003 100644 --- a/extensions/bluebubbles/src/monitor-reply-cache.ts +++ b/extensions/bluebubbles/src/monitor-reply-cache.ts @@ -160,16 +160,37 @@ function describeChatForError(values: { return parts.length === 0 ? "" : parts.join(", "); } +function buildCrossChatError( + inputId: string, + inputKind: "short" | "uuid", + cached: BlueBubblesReplyCacheEntry, + ctx: BlueBubblesChatContext, +): Error { + const remediation = + inputKind === "short" + ? `Retry with the full message GUID to avoid cross-chat reactions/replies landing in the wrong conversation.` + : `Retry with the correct chat target — even the full GUID cannot be reused across chats.`; + return new Error( + `BlueBubbles message id "${inputId}" belongs to a different chat ` + + `(${describeChatForError(cached)}) than the current call target ` + + `(${describeChatForError(ctx)}). ${remediation}`, + ); +} + /** * 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 + * the caller's chat or the call throws. This prevents a message 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. + * + * The guard runs on both numeric short ids AND full GUIDs: an agent can paste + * a GUID it harvested from history, a previous tool result, or another chat's + * transcript, and that path used to bypass the cross-chat check entirely. */ export function resolveBlueBubblesMessageId( shortOrUuid: string, @@ -187,12 +208,7 @@ export function resolveBlueBubblesMessageId( 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.`, - ); + throw buildCrossChatError(trimmed, "short", cached, opts.chatContext); } } return uuid; @@ -202,9 +218,18 @@ export function resolveBlueBubblesMessageId( `BlueBubbles short message id "${trimmed}" is no longer available. Use MessageSidFull.`, ); } + return trimmed; } - // Return as-is (either already a UUID or not found) + // Full GUID input — guard still applies. Cache miss falls through to + // returning the input unchanged so callers that supply a fresh-from-the-wire + // GUID (not yet seen by reply cache) keep working. + if (opts?.chatContext) { + const cached = blueBubblesReplyCacheByMessageId.get(trimmed); + if (cached && isCrossChatMismatch(cached, opts.chatContext)) { + throw buildCrossChatError(trimmed, "uuid", cached, opts.chatContext); + } + } return trimmed; }