…und chatGuid

When a BlueBubbles inbound webhook arrives without `chatGuid`, processMessage
falls back to `resolveChatGuidForTarget` to look it up. The previous fallback
target was:

    isGroup && (chatId || chatIdentifier)
      ? <chat_id or chat_identifier>
      : { kind: "handle", address: message.senderId }

That `else` branch quietly covered two very different cases:

1. DM with no chatGuid — resolving via sender handle is correct, the chat
   IS the conversation with that handle.
2. **Group with no chatGuid AND no chatId AND no chatIdentifier** — resolving
   via sender handle yields *that sender's DM chatGuid*, then the rest of
   processMessage uses it for ack reactions, mark-read, outbound reply cache,
   typing indicators, and outboundTarget.

Case 2 is reachable: `monitor.webhook.test-helpers.ts` ships a default
`createMessageReactionPayloadForTest` payload with no chatGuid/chatId/
chatIdentifier and `isGroup` defaulted to `false`, mirroring real BlueBubbles
reaction/tapback webhooks. When a group reaction or tapback arrives in that
shape and isGroup is later corrected to true (or the message takes the same
poisoned path), `chatGuidForActions` becomes the sender's DM chatGuid. The
poisoned chatGuid then writes the outbound reply cache (line ~1395) with the
wrong chat, defeating the cross-chat short-id guard added in
9912472289 — a later short id resolved against that cache cannot detect the
mismatch and the agent's reaction/reply silently lands in the DM.

Symptom Chris observed (recurring after 9912472289 baked): group messages
getting reacted to from the agent's side show up in a DM transcript with
that sender, attached to a message GUID the user can no longer locate in
the DM.

Extract the fallback target construction into
`buildBlueBubblesInboundChatResolveTarget` so the rule is testable in
isolation and the wrong fallback can never be reached again:

- Group inbound + chatId present → `chat_id`
- Group inbound + chatIdentifier present → `chat_identifier`
- **Group inbound + neither → return null (caller skips chatGuid-dependent actions)**
- DM inbound → `handle` (unchanged: the conversation IS that sender)

processMessage now logs at verbose when the group case returns null instead
of silently degrading to the sender's DM.

Tests: extensions/bluebubbles/src/monitor-processing-chat-resolve.test.ts
covers the eight branches (group with id, group with identifier, group
preferring id, group with neither, blank/non-finite/null variants, DM, DM
with chat_id present, DM with empty sender).

Local patch for upstream consideration — pairs with the short-id chat guard
landed in the previous commit.

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.

…dentifier

processReaction's peerId calculation:

    const peerId = reaction.isGroup
      ? (chatGuid ?? chatIdentifier ?? (chatId ? String(chatId) : "group"))
      : reaction.senderId;

reads as "if it's a group with at least one chat hint, use that hint;
otherwise fall through to either the literal string 'group' (group case)
or the sender id (DM case)". Two failure modes hide here:

1. BlueBubbles fires a `message-reaction` event with `isGroup: true` but
   omits chatGuid AND chatId AND chatIdentifier — peerId becomes the
   literal "group" and resolveBlueBubblesConversationRoute synthesizes
   a session key unrelated to any real binding. The reaction surfaces in
   whatever session the binding fallback picks, never the right one.

2. The same payload arrives with isGroup misclassified as false (BB's
   group-flag inference relies on chatGuid, explicit isGroup, or
   participants > 2 — none of which are guaranteed for reaction events;
   monitor.webhook.test-helpers.ts even ships a default reaction fixture
   with no chatGuid and isGroup defaulted to false). peerId then becomes
   reaction.senderId and the event is enqueued into the sender's DM
   session — the group tapback shows up inside an unrelated 1:1
   transcript Chris was looking at.

Neither outcome is recoverable without a chat hint — without chatGuid,
chatId, or chatIdentifier we cannot identify which group the reaction
belongs to. Drop the event with a verbose-log and let the agent miss
that reaction rather than route it incorrectly. DM reactions (which
legitimately may arrive with no chat hint and only a sender) keep
working because the guard is gated on `reaction.isGroup === true`.

A latent risk remains: if BB ever sends an isGroup-misclassified-as-false
payload, this guard does not catch it. That would require teaching
normalize to surface group-flag confidence, which is a larger change
left for follow-up.

Tests (extensions/bluebubbles/src/monitor.test.ts):
- Group reaction with no chat identifiers → not enqueued
- Group reaction with at least one chat identifier → still enqueued
  (regression sentinel for the new guard)

Local patch for upstream consideration.

…on route

resolveBlueBubblesOutboundSessionRoute classified all `chat_guid:`
prefixed targets as groups:

    const isGroup =
      parsed.kind === "chat_id" ||
      parsed.kind === "chat_guid" ||
      parsed.kind === "chat_identifier";

But BlueBubbles also encodes DM chatGuids in the same `chat_guid:`
form — they look like `iMessage;-;+15551234567` (the `;-;` separator
is the DM marker; groups use `;+;`). Treating those as groups gave
the same DM two different sessionKeys depending on how the caller
addressed it:

- handle form (`bluebubbles:imessage:+15551234567`)
  → peer.kind = "direct", from = `bluebubbles:+15551234567`
- chat_guid form (`bluebubbles:chat_guid:iMessage;-;+15551234567`)
  → peer.kind = "group", from = `group:iMessage;-;+15551234567`

When a bound DM session was looked up against the second form, no
binding matched and the outbound landed in a freshly-synthesized
"group" sessionKey — a degenerate session that the next inbound
message also failed to find, surfacing the conversation in the
wrong place.

Use resolveGroupFlagFromChatGuid (already used by monitor-normalize
to read the same marker for inbound webhooks) so both directions
agree on what counts as a group. Unknown chatGuid shapes still
fall back to "group" to preserve prior behavior — we never
silently downgrade a real group to direct.

Tests: extensions/bluebubbles/src/session-route.test.ts (new)
- chat_guid `;-;` → direct
- chat_guid `;+;` → group
- chat_guid with no recognizable marker → group (back-compat)
- handle target → direct
- chat_id / chat_identifier → group (unchanged)
- DM addressed two ways converges on the same peer kind

Local patch for upstream consideration. Latent bug introduced by
0f7cd59 (BlueBubbles: move outbound session routing behind plugin
boundary), not commonly hit because most outbound DM call sites use
the handle form, but a real foot-gun for callers that pass the
chat_guid form.

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.

Three findings from the second pass:

1. **MEDIUM — Cross-chat short message ID guard bypassed on empty chat
   context (CWE-285).** When `requireKnownShortId=true` and `chatContext`
   was missing or `{}`, `resolveBlueBubblesMessageId` would still resolve
   the short id. Short ids are allocated from a single global counter
   across every account and chat, so an action call without a chat
   scope could silently apply to the wrong conversation. Throw "requires
   a chat scope" instead. The previous behavior was an explicit
   "fail-open" choice with a comment acknowledging the risk; the
   underlying assumption (downstream call carries chatGuid) does not
   hold for every action handler. Test rewritten to expect fail-closed.

2. **LOW — Unsanitized messageId reflected in cross-chat guard error
   (CWE-117 / CWE-200).** The thrown error embedded the raw inputId
   (and the raw chatGuid / chatIdentifier from the cached entry until
   the previous pass). Replace the inputId with a shape descriptor
   (`<short:N-digit>` or `<uuid:prefix…>`) so cross-chat errors no
   longer leak any concrete identifier. Combined with the chat
   identifier redaction in describeChatForError (already in place),
   the error is fully redacted.

3. **LOW — PII exposure via verbose logs (CWE-532).** Untrusted webhook
   identifiers (senderId / messageId / action) were already passed
   through `sanitizeForLog`, but the helper only stripped control
   characters — it did not redact secrets such as `?password=` query
   strings or `Authorization: Bearer …` headers that occasionally
   bleed into error chains. Extend `sanitizeForLog` to redact those
   patterns. All call sites benefit immediately.

…hanks @zqchris)

)

… (thanks @zqchris)

…hanks @zqchris)