mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:30:42 +00:00
fix(bluebubbles): apply cross-chat guard to full message GUIDs as well
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.
This commit is contained in:
committed by
Peter Steinberger
parent
4bd3d258cd
commit
6ade320421
@@ -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",
|
||||
|
||||
@@ -160,16 +160,37 @@ function describeChatForError(values: {
|
||||
return parts.length === 0 ? "<unknown chat>" : 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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user