fix(bluebubbles): address aisle review on routing-guard PR

Four findings on this PR, all addressed in this commit:

1. **Cross-chat guard bypass when ctx.chatGuid present but cached lacks chatGuid**
   (CWE-697). Earlier `isCrossChatMismatch` gated chatIdentifier and chatId
   fallback comparisons on `!ctxChatGuid`, which let any non-empty
   ctx.chatGuid suppress the fallback checks when the cached entry happened
   to lack chatGuid — letting a short id from chat A be reused while acting
   in chat B. Rewrite the function so chatIdentifier/chatId comparisons
   run independently based on availability on each side, not on whether
   ctx.chatGuid happens to be present.

2. **Sensitive chat identifiers exposed via thrown cross-chat error**
   (CWE-200). `describeChatForError` interpolated raw chatGuid /
   chatIdentifier / chatId into the error message — these can leak phone
   numbers / email addresses / chat GUIDs into agent transcripts, tool
   results, remote channel deliveries, or third-party log aggregators.
   Surface only the *shape* of the chat target with `=<redacted>` values.

3. **Group reaction drop-guard bypass via whitespace chatIdentifier**.
   Earlier guard treated "" as missing but accepted " " / "\t". Trim
   chatGuid/chatIdentifier before the missing-check so a webhook sender
   supplying whitespace cannot satisfy the guard and have peerId degrade
   to the literal "group".

4. **Log injection via webhook senderId/messageId in verbose log lines**
   (CWE-117). Untrusted webhook fields were interpolated directly into
   `logVerbose` calls without sanitization, allowing log forging if a
   sender carried CR/LF/control bytes. Wrap with the existing
   `sanitizeForLog()` helper at all such sites.

Test updates: monitor-reply-cache.test.ts cross-chat error assertions
now expect `chatGuid=<redacted>` instead of raw values.

extensions/bluebubbles/src/monitor-processing.ts

@@ -1359,7 +1359,7 @@ async function processMessageAfterDedupe(
       logVerbose(
         core,
         runtime,
-        `cannot resolve chatGuid for group inbound (chatGuid/chatId/chatIdentifier all missing); senderId=${message.senderId}`,
+        `cannot resolve chatGuid for group inbound (chatGuid/chatId/chatIdentifier all missing); senderId=${sanitizeForLog(message.senderId)}`,
       );
     }
   }
@@ -1915,16 +1915,21 @@ export async function processReaction(
   // unrelated to any real binding — worse, an isGroup=false misclassification
   // upstream would have routed this to the sender's DM session, surfacing
   // a group tapback inside an unrelated 1:1 transcript. Drop+log instead.
+  // Treat whitespace-only chatGuid/chatIdentifier as missing — a webhook
+  // sender that supplies " " or "\t" must not be able to satisfy the guard
+  // and have peerId degrade to the literal "group" anyway.
+  const trimmedReactionChatGuid = reaction.chatGuid?.trim();
+  const trimmedReactionChatIdentifier = reaction.chatIdentifier?.trim();
   if (
     reaction.isGroup &&
-    !reaction.chatGuid &&
+    !trimmedReactionChatGuid &&
     reaction.chatId == null &&
-    !reaction.chatIdentifier
+    !trimmedReactionChatIdentifier
   ) {
     logVerbose(
       core,
       runtime,
-      `dropping group reaction with no chat identifiers (senderId=${reaction.senderId} messageId=${reaction.messageId} action=${reaction.action})`,
+      `dropping group reaction with no chat identifiers (senderId=${sanitizeForLog(reaction.senderId)} messageId=${sanitizeForLog(reaction.messageId)} action=${sanitizeForLog(reaction.action)})`,
     );
     return;
   }

extensions/bluebubbles/src/monitor-reply-cache.test.ts

@@ -223,8 +223,10 @@ describe("resolveBlueBubblesMessageId chat-scoped short-id guard", () => {
       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");
+      // Chat identifiers redacted in error message (PII / log-stream hardening).
+      expect(message).toContain("chatGuid=<redacted>");
+      expect(message).not.toContain("iMessage;+;chat240698944142298252");
+      expect(message).not.toContain("iMessage;-;+8618621181874");
       expect(message).toContain("correct chat target");
       expect(message).not.toContain("Retry with the full message GUID");
     }
@@ -264,8 +266,10 @@ describe("resolveBlueBubblesMessageId chat-scoped short-id guard", () => {
       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");
+      // Chat identifiers redacted in error message (PII / log-stream hardening).
+      expect(message).toContain("chatGuid=<redacted>");
+      expect(message).not.toContain("iMessage;+;chat240698944142298252");
+      expect(message).not.toContain("iMessage;-;+8618621181874");
       expect(message).toContain("full message GUID");
     }
   });

extensions/bluebubbles/src/monitor-reply-cache.ts

@@ -111,31 +111,25 @@ function isCrossChatMismatch(
   cached: BlueBubblesReplyCacheEntry,
   ctx: BlueBubblesChatContext,
 ): boolean {
+  // Compare each identifier independently based on availability on both sides.
+  // Earlier versions gated chatIdentifier/chatId comparisons on `!ctxChatGuid`,
+  // which let any non-empty `ctx.chatGuid` suppress the fallback checks when
+  // the cached entry happened to lack chatGuid — letting a short id from
+  // chat A be reused while acting in chat B.
   const cachedChatGuid = normalizeOptionalString(cached.chatGuid);
   const ctxChatGuid = normalizeOptionalString(ctx.chatGuid);
-  if (cachedChatGuid && ctxChatGuid && cachedChatGuid !== ctxChatGuid) {
-    return true;
+  if (cachedChatGuid && ctxChatGuid) {
+    return cachedChatGuid !== ctxChatGuid;
   }
   const cachedChatIdentifier = normalizeOptionalString(cached.chatIdentifier);
   const ctxChatIdentifier = normalizeOptionalString(ctx.chatIdentifier);
-  if (
-    !ctxChatGuid &&
-    cachedChatIdentifier &&
-    ctxChatIdentifier &&
-    cachedChatIdentifier !== ctxChatIdentifier
-  ) {
-    return true;
+  if (cachedChatIdentifier && ctxChatIdentifier) {
+    return cachedChatIdentifier !== ctxChatIdentifier;
   }
   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;
+  if (cachedChatId !== undefined && ctxChatId !== undefined) {
+    return cachedChatId !== ctxChatId;
   }
   return false;
 }
@@ -145,17 +139,19 @@ function describeChatForError(values: {
   chatIdentifier?: string;
   chatId?: number;
 }): string {
+  // Surface only the *shape* of the chat target, never the raw identifier,
+  // to avoid leaking phone numbers / email addresses / chat GUIDs into
+  // error messages that may end up in agent transcripts, tool results,
+  // remote channel deliveries, or third-party log aggregators.
   const parts: string[] = [];
-  const guid = normalizeOptionalString(values.chatGuid);
-  if (guid) {
-    parts.push(`chatGuid=${guid}`);
+  if (normalizeOptionalString(values.chatGuid)) {
+    parts.push("chatGuid=<redacted>");
   }
-  const identifier = normalizeOptionalString(values.chatIdentifier);
-  if (identifier) {
-    parts.push(`chatIdentifier=${identifier}`);
+  if (normalizeOptionalString(values.chatIdentifier)) {
+    parts.push("chatIdentifier=<redacted>");
   }
   if (typeof values.chatId === "number") {
-    parts.push(`chatId=${values.chatId}`);
+    parts.push("chatId=<redacted>");
   }
   return parts.length === 0 ? "<unknown chat>" : parts.join(", ");
 }