diff --git a/extensions/bluebubbles/src/monitor-processing-chat-resolve.test.ts b/extensions/bluebubbles/src/monitor-processing-chat-resolve.test.ts index ecd5e57283c..7b8c0b4be8e 100644 --- a/extensions/bluebubbles/src/monitor-processing-chat-resolve.test.ts +++ b/extensions/bluebubbles/src/monitor-processing-chat-resolve.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from "vitest"; -import { buildBlueBubblesInboundChatResolveTarget } from "./monitor-processing.js"; +import { + _sanitizeBlueBubblesLogValueForTest, + buildBlueBubblesInboundChatResolveTarget, +} from "./monitor-processing.js"; describe("buildBlueBubblesInboundChatResolveTarget", () => { it("uses chat_id for group inbound when chatId is present", () => { @@ -111,3 +114,24 @@ describe("buildBlueBubblesInboundChatResolveTarget", () => { expect(target).toBeNull(); }); }); + +describe("BlueBubbles monitor log sanitization", () => { + it("redacts BlueBubbles query auth and Authorization headers", () => { + const input = + "GET /api/v1/attachment?password=secret&guid=socket-secret&token=api-token Authorization: Bearer abc123"; + + const sanitized = _sanitizeBlueBubblesLogValueForTest(input); + + expect(sanitized).toContain("password="); + expect(sanitized).toContain("guid="); + expect(sanitized).toContain("token="); + expect(sanitized).toContain("Authorization: Bearer "); + expect(sanitized).not.toContain("secret"); + expect(sanitized).not.toContain("api-token"); + expect(sanitized).not.toContain("abc123"); + }); + + it("strips control characters before logging", () => { + expect(_sanitizeBlueBubblesLogValueForTest("one\ntwo\tt\u0000hree")).toBe("one two t hree"); + }); +}); diff --git a/extensions/bluebubbles/src/monitor-processing.ts b/extensions/bluebubbles/src/monitor-processing.ts index 27f2cab92ed..9e8371d24f2 100644 --- a/extensions/bluebubbles/src/monitor-processing.ts +++ b/extensions/bluebubbles/src/monitor-processing.ts @@ -631,18 +631,21 @@ function buildInboundHistorySnapshot(params: { function sanitizeForLog(value: unknown, maxLen = 200): string { let cleaned = String(value).replace(/[\r\n\t\p{C}]/gu, " "); // Redact common secret-bearing patterns before logging. BlueBubbles uses - // query-string auth (`?password=...`) by default, so attachment download - // failures and similar errors can carry the API password in the captured - // request URL; other libraries occasionally surface `Authorization: Bearer …` - // headers in error chains. Strip both before they reach the log sink (CWE-532). + // query-string auth (`?password=...`, `?guid=...`, or `?token=...`) by + // default, so attachment download failures and similar errors can carry the + // API password in the captured request URL; other libraries occasionally + // surface `Authorization: Bearer ...` headers in error chains. Strip both + // before they reach the log sink (CWE-532). cleaned = cleaned.replace( - /([?&](?:password|token|api[_-]?key|secret)=)[^&\s"]+/gi, + /([?&](?:password|guid|token|api[_-]?key|secret)=)[^&\s"]+/gi, "$1", ); cleaned = cleaned.replace(/(authorization\s*:\s*(?:bearer|basic)\s+)[^\s"]+/gi, "$1"); return cleaned.length > maxLen ? cleaned.slice(0, maxLen) + "..." : cleaned; } +export const _sanitizeBlueBubblesLogValueForTest = sanitizeForLog; + /** * Signal object threaded through `processMessageAfterDedupe` so the outer * wrapper can distinguish "reply delivery failed silently" from "returned @@ -810,7 +813,7 @@ async function processMessageAfterDedupe( logVerbose( core, runtime, - `attachment retry failed for msgId=${message.messageId}: ${String(err)}`, + `attachment retry failed for msgId=${sanitizeForLog(message.messageId)}: ${sanitizeForLog(err)}`, ); } } @@ -904,18 +907,22 @@ async function processMessageAfterDedupe( } if (isSelfChatMessage && hasBlueBubblesSelfChatCopy(selfChatLookup)) { - logVerbose(core, runtime, `drop: reflected self-chat duplicate sender=${message.senderId}`); + logVerbose( + core, + runtime, + `drop: reflected self-chat duplicate sender=${sanitizeForLog(message.senderId)}`, + ); return; } if (!rawBody) { - logVerbose(core, runtime, `drop: empty text sender=${message.senderId}`); + logVerbose(core, runtime, `drop: empty text sender=${sanitizeForLog(message.senderId)}`); return; } logVerbose( core, runtime, - `msg sender=${message.senderId} group=${isGroup} textLen=${text.length} attachments=${attachments.length} chatGuid=${message.chatGuid ?? ""} chatId=${message.chatId ?? ""}`, + `msg sender=${sanitizeForLog(message.senderId)} group=${isGroup} textLen=${text.length} attachments=${attachments.length} chatGuid=${sanitizeForLog(message.chatGuid ?? "")} chatId=${sanitizeForLog(message.chatId ?? "")}`, ); const dmPolicy = account.config.dmPolicy ?? "pairing"; @@ -1011,8 +1018,14 @@ async function processMessageAfterDedupe( senderIdLine: `Your BlueBubbles sender id: ${message.senderId}`, meta: { name: message.senderName }, onCreated: () => { - runtime.log?.(`[bluebubbles] pairing request sender=${message.senderId} created=true`); - logVerbose(core, runtime, `bluebubbles pairing request sender=${message.senderId}`); + runtime.log?.( + `[bluebubbles] pairing request sender=${sanitizeForLog(message.senderId)} created=true`, + ); + logVerbose( + core, + runtime, + `bluebubbles pairing request sender=${sanitizeForLog(message.senderId)}`, + ); }, sendPairingReply: async (text) => { await sendMessageBlueBubbles(message.senderId, text, { @@ -1025,10 +1038,10 @@ async function processMessageAfterDedupe( logVerbose( core, runtime, - `bluebubbles pairing reply failed for ${message.senderId}: ${String(err)}`, + `bluebubbles pairing reply failed for ${sanitizeForLog(message.senderId)}: ${sanitizeForLog(err)}`, ); runtime.error?.( - `[bluebubbles] pairing reply failed sender=${message.senderId}: ${String(err)}`, + `[bluebubbles] pairing reply failed sender=${sanitizeForLog(message.senderId)}: ${sanitizeForLog(err)}`, ); }, }); @@ -1159,7 +1172,7 @@ async function processMessageAfterDedupe( logVerbose( core, runtime, - `bluebubbles: participant fallback lookup failed chat=${peerId}: ${String(err)}`, + `bluebubbles: participant fallback lookup failed chat=${sanitizeForLog(peerId)}: ${sanitizeForLog(err)}`, ); } } @@ -1225,7 +1238,7 @@ async function processMessageAfterDedupe( logVerbose( core, runtime, - `attachment download failed guid=${attachment.guid} err=${String(err)}`, + `attachment download failed guid=${sanitizeForLog(attachment.guid)} err=${sanitizeForLog(err)}`, ); } } @@ -1410,7 +1423,7 @@ async function processMessageAfterDedupe( logVerbose( core, runtime, - `ack reaction failed chatGuid=${chatGuidForActions} msg=${ackMessageId}: ${String(err)}`, + `ack reaction failed chatGuid=${sanitizeForLog(chatGuidForActions)} msg=${sanitizeForLog(ackMessageId)}: ${sanitizeForLog(err)}`, ); return false; }, @@ -1425,9 +1438,9 @@ async function processMessageAfterDedupe( cfg: config, accountId: account.accountId, }); - logVerbose(core, runtime, `marked read chatGuid=${chatGuidForActions}`); + logVerbose(core, runtime, `marked read chatGuid=${sanitizeForLog(chatGuidForActions)}`); } catch (err) { - runtime.error?.(`[bluebubbles] mark read failed: ${String(err)}`); + runtime.error?.(`[bluebubbles] mark read failed: ${sanitizeForLog(err)}`); } } else if (!sendReadReceipts) { logVerbose(core, runtime, "mark read skipped (sendReadReceipts=false)"); @@ -1569,7 +1582,7 @@ async function processMessageAfterDedupe( logVerbose( core, runtime, - `history backfill failed for ${historyIdentifier}: ${String(err)} (retries left=${Math.max(remainingAttempts, 0)} next_in_ms=${nextBackoffMs})`, + `history backfill failed for ${sanitizeForLog(historyIdentifier)}: ${sanitizeForLog(err)} (retries left=${Math.max(remainingAttempts, 0)} next_in_ms=${nextBackoffMs})`, ); } } @@ -1660,7 +1673,7 @@ async function processMessageAfterDedupe( cfg: config, accountId: account.accountId, }).catch((err) => { - runtime.error?.(`[bluebubbles] typing restart failed: ${String(err)}`); + runtime.error?.(`[bluebubbles] typing restart failed: ${sanitizeForLog(err)}`); }); }, typingRestartDelayMs); }; @@ -1686,7 +1699,7 @@ async function processMessageAfterDedupe( accountId: account.accountId, }); } catch (err) { - runtime.error?.(`[bluebubbles] typing start failed: ${String(err)}`); + runtime.error?.(`[bluebubbles] typing start failed: ${sanitizeForLog(err)}`); } }, onIdle: () => { @@ -1848,7 +1861,7 @@ async function processMessageAfterDedupe( if (info.kind === "final") { dedupeSignal.deliveryFailed = true; } - runtime.error?.(`BlueBubbles ${info.kind} reply failed: ${String(err)}`); + runtime.error?.(`BlueBubbles ${info.kind} reply failed: ${sanitizeForLog(err)}`); }, }, replyOptions: { diff --git a/extensions/bluebubbles/src/session-route.test.ts b/extensions/bluebubbles/src/session-route.test.ts index 8b29c8f5026..59cdfd1f248 100644 --- a/extensions/bluebubbles/src/session-route.test.ts +++ b/extensions/bluebubbles/src/session-route.test.ts @@ -3,10 +3,13 @@ import type { OpenClawConfig } from "./runtime-api.js"; import { resolveBlueBubblesOutboundSessionRoute } from "./session-route.js"; const EMPTY_CFG = {} as OpenClawConfig; +const PER_PEER_CFG = { + session: { dmScope: "per-peer" }, +} as OpenClawConfig; -function call(target: string) { +function call(target: string, cfg = EMPTY_CFG) { return resolveBlueBubblesOutboundSessionRoute({ - cfg: EMPTY_CFG, + cfg, agentId: "agent-1", accountId: "default", target, @@ -25,8 +28,10 @@ describe("resolveBlueBubblesOutboundSessionRoute DM/group disambiguation", () => const route = call("bluebubbles:chat_guid:iMessage;-;+15551234567"); expect(route).not.toBeNull(); expect(route?.peer.kind).toBe("direct"); + expect(route?.peer.id).toBe("+15551234567"); expect(route?.chatType).toBe("direct"); - expect(route?.from).toMatch(/^bluebubbles:/); + expect(route?.from).toBe("bluebubbles:+15551234567"); + expect(route?.to).toBe("bluebubbles:chat_guid:iMessage;-;+15551234567"); expect(route?.from).not.toMatch(/^group:/); }); @@ -71,11 +76,14 @@ describe("resolveBlueBubblesOutboundSessionRoute DM/group disambiguation", () => it("DM via chat_guid and DM via handle land on the same session key", () => { // The point of disambiguation: a DM addressed two different ways must // converge on the same sessionKey so existing bindings keep matching. - const handleRoute = call("bluebubbles:imessage:+15551234567"); - const chatGuidRoute = call("bluebubbles:chat_guid:iMessage;-;+15551234567"); + const handleRoute = call("bluebubbles:imessage:+15551234567", PER_PEER_CFG); + const chatGuidRoute = call("bluebubbles:chat_guid:iMessage;-;+15551234567", PER_PEER_CFG); expect(handleRoute?.sessionKey).toBeDefined(); expect(chatGuidRoute?.sessionKey).toBeDefined(); - // Both are direct now; sessionKey base derives from peer.id. expect(handleRoute?.peer.kind).toBe(chatGuidRoute?.peer.kind); + expect(handleRoute?.peer.id).toBe(chatGuidRoute?.peer.id); + expect(handleRoute?.from).toBe(chatGuidRoute?.from); + expect(handleRoute?.sessionKey).toBe(chatGuidRoute?.sessionKey); + expect(chatGuidRoute?.to).toBe("bluebubbles:chat_guid:iMessage;-;+15551234567"); }); }); diff --git a/extensions/bluebubbles/src/session-route.ts b/extensions/bluebubbles/src/session-route.ts index 5414987d722..42adfd3c3a2 100644 --- a/extensions/bluebubbles/src/session-route.ts +++ b/extensions/bluebubbles/src/session-route.ts @@ -4,7 +4,7 @@ import { type ChannelOutboundSessionRouteParams, } from "openclaw/plugin-sdk/channel-core"; import { resolveGroupFlagFromChatGuid } from "./monitor-normalize.js"; -import { parseBlueBubblesTarget } from "./targets.js"; +import { extractHandleFromChatGuid, parseBlueBubblesTarget } from "./targets.js"; export function resolveBlueBubblesOutboundSessionRoute(params: ChannelOutboundSessionRouteParams) { const stripped = stripChannelTargetPrefix(params.target, "bluebubbles"); @@ -27,11 +27,15 @@ export function resolveBlueBubblesOutboundSessionRoute(params: ChannelOutboundSe : parsed.kind === "chat_guid" ? (groupFromChatGuid ?? true) : false; + const dmHandleFromChatGuid = + parsed.kind === "chat_guid" && groupFromChatGuid === false + ? extractHandleFromChatGuid(parsed.chatGuid) + : null; const peerId = parsed.kind === "chat_id" ? String(parsed.chatId) : parsed.kind === "chat_guid" - ? parsed.chatGuid + ? (dmHandleFromChatGuid ?? parsed.chatGuid) : parsed.kind === "chat_identifier" ? parsed.chatIdentifier : parsed.to;