feat(bluebubbles): fetch quoted message via SSRF-guarded API on reply cache miss + surface attachment download errors by zqchris · Pull Request #73241 · openclaw/openclaw

zqchris · 2026-04-28T04:00:01Z

Summary

Three small, related BlueBubbles enhancements bundled because the SSRF guard fix only makes sense in the context of the new feature it hardens:

1. `bluebubbles: fetch quoted message from API when reply cache misses`

When a user replies to a message in iMessage, the BlueBubbles webhook may include only replyToGuid without the quoted message body. The in-memory reply cache (6h TTL, 2000 entries) doesn't always have it — e.g. after a Gateway restart, or when the original message is older than the cache window. Today the agent receives a reply with no context for what was replied to.

Adds a fallback that fetches the message by GUID from /api/v1/message/<guid> so the agent can see the quoted content when the cache misses. Falls back gracefully (returns null, no throw) on auth/network/parse errors so the existing reply path keeps working when the API is unreachable.

2. `fix(bluebubbles): route quoted-message API fallback through SSRF-guarded client`

Direct follow-up to #1 from a self-review: fetchBlueBubblesMessageByGuid initially called blueBubblesFetchWithTimeout directly without threading ssrfPolicy, which is the only switch that puts the request on the SSRF-guarded fetch path. Same shape as fetchBlueBubblesHistory in this file, which correctly uses createBlueBubblesClientFromParts to get the SSRF-aware client.

Switch to the same client construction (resolveAccount returns allowPrivateNetwork; client.request applies the SSRF policy). Drop the now-unused buildBlueBubblesApiUrl + blueBubblesFetchWithTimeout import. Same threat model as #68234's typed-client consolidation — this codepath was missed there.

3. `fix(bluebubbles): surface attachment download failures at error level`

processMessage's attachment download catch block only called logVerbose, so download failures were invisible under normal log levels. I hit a real case where a pinned-dispatcher compat bug silently dropped every inbound image attachment, and the agent only saw the upstream-generated <media:image> (1 image) placeholder text — there was nothing in the log to indicate the actual download had failed.

Adds a runtime.error call alongside the existing logVerbose so future download failures are visible at default log level, while keeping verbose detail for debug sessions. Three lines.

Test plan

pnpm test extensions/bluebubbles — 511/511 passing
pnpm check:changed — clean
Manual: replying in iMessage to a message older than reply-cache TTL surfaces the quoted body in the agent transcript (pre-fix: empty/orphan)

Notes for reviewers

Commit 1 is the feature, commit 2 is a same-day SSRF-guard self-review fix. Happy to squash if preferred — kept separate so the threat model rationale stays visible in history.
Commit 3 is independent and could be its own PR; bundled because it's a 3-line observability fix that complements the same channel.

When a user replies to a message in iMessage, the webhook may only include the replyToGuid without the quoted message body. The in-memory cache (6h TTL, 2000 entries) doesn't always have the original message. Add a fallback that fetches the message by GUID from the BlueBubbles server API so the agent can see the quoted content.

…ded client fetchBlueBubblesMessageByGuid called blueBubblesFetchWithTimeout without threading ssrfPolicy, which is the only switch that puts the request on the SSRF-guarded fetch path (see types.ts request flag). Same shape as fetchBlueBubblesHistory in this file, which correctly uses createBlueBubblesClientFromParts to get the SSRF-aware client. Switch to the same client construction (resolveAccount returns allowPrivateNetwork; client.request applies the SSRF policy). Drop the now-unused buildBlueBubblesApiUrl + blueBubblesFetchWithTimeout import. Reported by codex review on patch/chris.

processMessage's attachment download catch block only called logVerbose, so download failures were invisible under normal log levels. The preceding pinned-dispatcher compat bug hid itself behind this: every inbound image silently dropped its attachment and the agent only saw the `<media:image> (1 image)` placeholder generated upstream. Add a runtime.error call alongside the existing logVerbose so future download failures are visible without enabling verbose mode, while keeping the verbose detail for debug sessions.

…t error logging

greptile-apps · 2026-04-28T04:01:52Z

Greptile Summary

This PR adds three related BlueBubbles improvements: an API fallback to fetch quoted-message bodies when the reply cache misses, routing that fallback through the SSRF-guarded client, and promoting attachment download failures from verbose to error-level logging. The SSRF hardening and logging fix are clean; the new fetchBlueBubblesMessageByGuid function has a minor inconsistency in how it extracts the sender compared to fetchBlueBubblesHistory.

Confidence Score: 4/5

Safe to merge; only a P2 sender-extraction inconsistency found — reply body text is unaffected.

All three changes are well-scoped. SSRF guard is correctly applied. The only finding is that fetchBlueBubblesMessageByGuid checks msg["sender"] as a string while the existing fetchBlueBubblesHistory treats it as an object — meaning the sender field may go unpopulated on a cache miss fallback. The reply text itself is still fetched correctly, so this is a P2 that doesn't block merging.

extensions/bluebubbles/src/history.ts — sender extraction logic in fetchBlueBubblesMessageByGuid

Prompt To Fix All With AI

This is a comment left during a code review.
Path: extensions/bluebubbles/src/history.ts
Line: 241-245

Comment:
**Sender extraction may miss object-shaped `sender` field**

`fetchBlueBubblesHistory` (same file, line 152) reads the sender as `msg.sender?.display_name || msg.sender?.address`, treating `sender` as an object — matching `BlueBubblesMessageData`. Here, `typeof msg["sender"] === "string"` will be `false` for an object-valued `sender`, so the address/display-name from that field is silently dropped, leaving only the `handle.address` or `is_from_me` fallback.

If the `/api/v1/message/<guid>` response returns the same shape as the message-list endpoint, the reply-context sender will be empty even when the API response contains it.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "docs(changelog): note BlueBubbles quoted..." | Re-trigger Greptile}

greptile-apps · 2026-04-28T04:01:55Z

+    const handle = msg["handle"] as Record<string, unknown> | undefined;
+    const sender =
+      (typeof handle?.["address"] === "string" ? handle["address"].trim() : undefined) ||
+      (typeof msg["sender"] === "string" ? msg["sender"].trim() : undefined) ||
+      (msg["is_from_me"] === true || msg["isFromMe"] === true ? "me" : undefined);


Sender extraction may miss object-shaped sender field

fetchBlueBubblesHistory (same file, line 152) reads the sender as msg.sender?.display_name || msg.sender?.address, treating sender as an object — matching BlueBubblesMessageData. Here, typeof msg["sender"] === "string" will be false for an object-valued sender, so the address/display-name from that field is silently dropped, leaving only the handle.address or is_from_me fallback.

If the /api/v1/message/<guid> response returns the same shape as the message-list endpoint, the reply-context sender will be empty even when the API response contains it.

Prompt To Fix With AI

This is a comment left during a code review. Path: extensions/bluebubbles/src/history.ts Line: 241-245 Comment: **Sender extraction may miss object-shaped `sender` field** `fetchBlueBubblesHistory` (same file, line 152) reads the sender as `msg.sender?.display_name || msg.sender?.address`, treating `sender` as an object — matching `BlueBubblesMessageData`. Here, `typeof msg["sender"] === "string"` will be `false` for an object-valued `sender`, so the address/display-name from that field is silently dropped, leaving only the `handle.address` or `is_from_me` fallback. If the `/api/v1/message/<guid>` response returns the same shape as the message-list endpoint, the reply-context sender will be empty even when the API response contains it. How can I resolve this? If you propose a fix, please make it concise.

Two findings on this PR addressed: 1. **Unsanitized error string in attachment download log** (CWE-117). `runtime.error` and `logVerbose` interpolated `String(err)` directly, allowing log forging if a remote HTTP error message carried CR/LF (or sensitive request details such as URLs with tokens). Wrap both `attachment.guid` and `err` with the existing `sanitizeForLog()` helper. 2. **Unbounded per-message API fallback request** (CWE-400). The reply-context API fallback ran on every inbound message with `replyToId`, with no shape validation on the input. Add a length-bound (≤128 chars) and charset check (alnum + `._:-`) before issuing the outbound API call so a webhook payload with a pathological replyToId cannot drive arbitrary outbound load. Also wrap log fields in the verbose log line at the same site with `sanitizeForLog()` so a webhook-supplied replyToId / sender label cannot inject newlines into the log file. Note: the third finding (Reply-context API fallback can bypass group allowlist/visibility gating) is not actually reachable in the inbound-message handler that calls this fallback, because that handler runs *after* allowlist gating has already accepted the inbound message — the fallback only enriches reply context for an already-admitted message. Closing that finding without a code change.

Three findings from the second pass: 1. **HIGH — Possible BlueBubbles password leakage via attachment download error logs (CWE-532).** BlueBubbles uses query-string auth by default (`?password=...`), so attachment download failures and similar errors can carry the API password in the captured request URL. The previous `sanitizeForLog()` only stripped control characters — it did not redact secrets. Extend the helper to redact common secret-bearing patterns (`?password=`, `?token=`, `?api_key=`, `?secret=`, `Authorization: Bearer / Basic …`) before the value reaches the log sink. All call sites of `sanitizeForLog()` benefit immediately. 2. **MEDIUM — Missing validation of derived GUID in `fetchBlueBubblesMessageByGuid` (CWE-20).** A trailing `/` on the input would leave `bareGuid` empty and turn the request into a list query against `/api/v1/message/`; a malformed GUID would let the encoded segment steer arbitrary BlueBubbles routes. Reject empty, oversize, or out-of-charset bareGuids (length ≤128, alnum + `._:-`) before issuing the request. The function is exported and may be called from paths that do not apply the existing replyToId validation in `monitor-processing.ts`. 3. **MEDIUM — Sensitive message content logged in verbose reply-context logs (CWE-532).** Both the cache-hit path and the API-fallback path logged a 120-char preview of quoted-message text. Verbose logs are often persisted / shipped to aggregators and would otherwise leak private chat content. Replace the body preview with a `bodyLen=` metadata field — operator can still see whether a body was resolved without the actual text.

coletebou · 2026-04-28T11:34:14Z

Hi @zqchris — your PR overlaps with #71820 (which I opened a few days earlier). Wanted to flag this directly rather than have it land as a maintainer surprise.

Just pushed an update to #71820 that folds in your architectural improvements:

Routed through the typed BlueBubblesClient (createBlueBubblesClientFromParts + client.request()) instead of resolving the SSRF policy by hand. This was your cleaner pattern, and it also matches what the codex meta-review on feat(bluebubbles): add reply-context API fallback for cache misses #71820 had suggested as a follow-up.
Adopted your reply-id shape validation (sanitizeReplyToId with the part-index strip + 128-char regex cap) as defense-in-depth on top of the typed client's URL construction.
Picked up your attachment download error promotion to runtime.error plus the sanitizeForLog secret-redaction (?password=…, ?token=…, Authorization: Bearer …). Both are real wins; full attribution preserved in the CHANGELOG entry and commit messages (9d081574a3).

#71820 also keeps two production behaviors yours doesn't currently have:

In-flight dedupe so concurrent webhooks for the same replyToId (e.g. group chats with N recipients replying near-simultaneously) coalesce into a single BB API call.
Reply-cache write-back on success so subsequent webhooks for the same message resolve from RAM.

If you'd prefer to consolidate into a single PR, two options:

Close feat(bluebubbles): fetch quoted message via SSRF-guarded API on reply cache miss + surface attachment download errors #73241 and let feat(bluebubbles): add reply-context API fallback for cache misses #71820 land — yours stays in the history via co-author/changelog attribution, and the SSRF + observability improvements you contributed are preserved verbatim.
Land feat(bluebubbles): fetch quoted message via SSRF-guarded API on reply cache miss + surface attachment download errors #73241 and close feat(bluebubbles): add reply-context API fallback for cache misses #71820 — happy to do this if maintainers prefer a fresh review surface; I'd ask that you pull in the in-flight dedupe + cache write-back before merge so the production benefits aren't lost.

Either way, thanks for the typed-client pointer and the secret-redaction fix — both are clear improvements to my original implementation.

clawsweeper · 2026-04-28T12:04:23Z

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

This PR is superseded by #71820. Current main has not implemented the BlueBubbles reply-context fallback or error-level attachment logging yet, but #71820 is the older open canonical PR and its current head folds in this PR's SSRF-guarded typed-client approach, reply-id validation, attachment error logging, secret redaction, and attribution, while adding dedupe/cache write-back and focused tests.

Best possible solution:

Close this PR as superseded and use #71820 as the single review surface for the BlueBubbles reply-context fallback and attachment logging work. Review or land #71820, which preserves this contribution and adds the missing production safeguards and tests.

What I checked:

Current main lacks reply-context API fallback: On current main, the BlueBubbles inbound path resolves reply context only from the in-memory cache, then proceeds to short-id lookup and context visibility. There is no API fallback between cache miss and transcript construction. (extensions/bluebubbles/src/monitor-processing.ts:1187, 2ccdbc7dd93b)
Current main still logs attachment download failures only at verbose level: The attachment download catch block on main only calls logVerbose with the raw error string; it does not call runtime.error or redact query/header secrets at this site. (extensions/bluebubbles/src/monitor-processing.ts:1168, 2ccdbc7dd93b)
Current main has no replyContextApiFallback config surface: The BlueBubbles config schema and account config type do not include replyContextApiFallback on current main, confirming the feature has not already landed independently. (extensions/bluebubbles/src/config-schema.ts:72, 2ccdbc7dd93b)
Canonical PR feat(bluebubbles): add reply-context API fallback for cache misses #71820 is live and current: The remote pull-request ref for feat(bluebubbles): add reply-context API fallback for cache misses #71820 resolves to 9d081574a35125ce8d74ada2b69ed3b8f027ebff. The provided GitHub context shows feat(bluebubbles): add reply-context API fallback for cache misses #71820 is open and was updated after this PR discussion to incorporate this PR's architectural and logging improvements. (9d081574a351)
feat(bluebubbles): add reply-context API fallback for cache misses #71820 includes the same core behavior plus stronger production handling: The feat(bluebubbles): add reply-context API fallback for cache misses #71820 patch adds replyContextApiFallback, fetchBlueBubblesReplyContext, in-flight dedupe, reply-cache write-back, typed BlueBubblesClient routing through createBlueBubblesClientFromParts, sanitizeReplyToId validation, and client.request against /api/v1/message/. It also adds focused tests for config inheritance, SSRF/client routing, body/sender parsing, dedupe, cache population, invalid IDs, and timeout behavior. (extensions/bluebubbles/src/monitor-reply-fetch.ts, 9d081574a351)
feat(bluebubbles): add reply-context API fallback for cache misses #71820 carries forward this PR's attachment logging and attribution: The feat(bluebubbles): add reply-context API fallback for cache misses #71820 patch promotes BlueBubbles attachment download failures to runtime.error, extends sanitizeForLog to redact password/token/API-key/Authorization secrets, and updates the changelog entry with Thanks @coletebou and @zqchris. (CHANGELOG.md, 9d081574a351)

So I’m closing this here and keeping the remaining discussion on the canonical linked item.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 2ccdbc7dd93b.

…eBubblesClient Replaces the manual SSRF-policy-resolve + raw-fetch path with createBlueBubblesClientFromParts, so the request goes through the same typed client surface every other BlueBubbles call uses. Picks up the folding-into-typed-client suggestion the codex meta-review left as a follow-up on the original PR review, and matches the architectural pattern in openclaw#73241. - Drop direct imports of buildBlueBubblesApiUrl, blueBubblesFetchWithTimeout, and resolveBlueBubblesClientSsrfPolicy. The typed client owns SSRF policy resolution internally and cannot produce an undefined policy. - Add sanitizeReplyToId: strip part-index prefix (`p:0/<guid>` → `<guid>`) and validate the bare GUID against the known charset + 128-char cap before it reaches the API path. Defense-in-depth alongside the typed client's URL construction (CWE-20). - Swap the test seam from fetchImpl (raw fetch) to clientFactory (typed client constructor). New tests assert factory-call params for SSRF modes 1/2/3 plus the regression invariant from the original review. - Keep in-flight dedupe and reply-cache write-back (production wins for high-volume group chats — concurrent webhooks for the same replyToId coalesce into one fetch, and successful responses populate the cache so subsequent webhooks resolve from RAM). Tests: 23 in monitor-reply-fetch.test.ts (+1 new pathological-id case, +1 part-index-strip case), 78 BB-targeted total green.

@zqchris

…r and redact secrets in logs Carries forward two BlueBubbles observability/security fixes from openclaw#73241 so this PR is a strict superset of the competing implementation. - Promote attachment download failures from logVerbose to runtime.error alongside a verbose copy. Previously, BB attachment download failures (pinned-dispatcher compat bugs, BB server 500s, transient network errors) were invisible at default log level — agents only saw the `<media:image> (1 image)` placeholder text with nothing in the log pointing at the actual failure. - Extend sanitizeForLog to redact `?password=…` / `?token=…` / `?api_key=…` / `?secret=…` query params and `Authorization: Bearer …` headers before they reach the log sink. BB uses query-string auth by default, so error chains carrying captured request URLs were vulnerable to leaking the API password into log aggregators (CWE-532). - Move the changelog entry to the bottom of `## Unreleased` → `### Changes`. Insertions at the top of that section are the most common cause of CHANGELOG conflicts on long-lived PRs; bottom position keeps the diff context stable. Both behavioral changes originated in @zqchris's PR openclaw#73241; attribution preserved in the CHANGELOG entry.

…penclaw#73241, openclaw#73400 Second hardening pass after the first round triggered new findings on Aisle's re-scan. PR openclaw#73406 (auto-reply voice silent) was meanwhile landed by the maintainer in upstream commit 28bf71d — that one will drop out at the next rebase via patch-id match, no source change here. PR openclaw#73241 (BB http hardening) — 1 HIGH + 2 MEDIUM: - monitor-processing.sanitizeForLog: redact common secret-bearing patterns (`?password=`, `?token=`, `?api_key=`, `?secret=`, `Authorization: Bearer / Basic …`) before the value reaches the log sink. BlueBubbles uses query-string auth by default, so attachment download failures and similar errors can carry the API password in the captured request URL (CWE-532). - history.fetchBlueBubblesMessageByGuid: validate the derived bareGuid (length ≤128, charset `[A-Za-z0-9._:-]+`, non-empty) before issuing the request. A trailing `/` would otherwise produce an empty bareGuid and turn the call into an unintended `/api/v1/message/` collection query (CWE-20). - monitor-processing reply-context verbose logs: log only `bodyLen=` metadata, not the quoted message text itself. Verbose logs are retained / shipped to aggregators and would otherwise leak private chat content (CWE-532). PR openclaw#73235 (BB routing guards) — 1 MEDIUM + 2 LOW: - monitor-reply-cache.resolveBlueBubblesMessageId: when `requireKnownShortId=true` and `chatContext` lacks any identifier, throw "requires a chat scope" instead of resolving the short id. Short ids are allocated from a single global counter across every account and chat, so an action call without chat scope could silently apply to the wrong conversation (CWE-285). Test updated to expect fail-closed (was previously fail-open with a comment that acknowledged the risk). - monitor-reply-cache.buildCrossChatError: replace raw inputId in the thrown error message with `<short:N-digit>` or `<uuid:prefix…>` shape descriptors. Combined with the earlier chatGuid redaction, cross-chat errors now leak no concrete identifier (CWE-117 / CWE-200). The Low PII finding on monitor-processing verbose logs is resolved automatically by the sanitizeForLog redaction extension added for PR openclaw#73241 openclaw#1. PR openclaw#73400 (silent-reply 🧵) — 1 MEDIUM: - silent-reply-policy.classifySilentReplyConversationType: classify a session key as `internal` only when `parseThreadSessionSuffix` returns a real `🧵<id>` suffix, not on a free-form `🧵` substring match. Caller-supplied session keys can no longer embed the marker mid-string to force the conversation type to `internal` and bypass silent-reply rewrites (CWE-840).

, openclaw#73241 Bundled hardening for the three open PRs: - session-files: use path.posix.join + strip CR/LF/TAB instead of rewriting backslashes to forward slashes. POSIX filenames may legally contain `\`, and the prior translation would synthesize fake path segments that bypass `excludeSourcePathRegex` (CWE-20). NUL is already rejected by Node's fs path layer. - monitor-reply-cache: redact chat identifiers in cross-chat error messages (CWE-200). Phone numbers / email addresses / chat GUIDs must not leak into agent transcripts, tool results, or remote channel deliveries. - monitor-reply-cache: rewrite isCrossChatMismatch so chatIdentifier and chatId comparisons run independently. Earlier version gated fallback comparisons on `!ctxChatGuid`, which let any non-empty ctx.chatGuid suppress the fallback checks when cached entry lacked chatGuid — letting a short id from chat A be reused while acting in chat B (CWE-697). - monitor-processing: drop group reactions where chatGuid / chatIdentifier is whitespace-only, not just empty. A webhook sender supplying " " or "\t" must not satisfy the guard and degrade peerId to the literal "group". - monitor-processing: sanitizeForLog around webhook senderId / messageId / action / attachment guid / err in verbose log lines and attachment download error logs (CWE-117). - monitor-processing: validate replyToId shape (length ≤128, charset alnum + `._:-`) before issuing the API fallback request, so a webhook with a pathological replyToId cannot drive arbitrary outbound load (cheap CWE-400 mitigation). Tests updated: monitor-reply-cache.test.ts cross-chat error assertions now expect `chatGuid=<redacted>` instead of raw values. Also fix unrelated typecheck regression: delivery-dispatch.mirror- bluebubbles.test.ts makeBaseParams missing the `runSessionKey` field introduced in DispatchCronDeliveryParams as part of v2026.4.26.

…penclaw#73241, openclaw#73400 Second hardening pass after the first round triggered new findings on Aisle's re-scan. PR openclaw#73406 (auto-reply voice silent) was meanwhile landed by the maintainer in upstream commit 28bf71d — that one will drop out at the next rebase via patch-id match, no source change here. PR openclaw#73241 (BB http hardening) — 1 HIGH + 2 MEDIUM: - monitor-processing.sanitizeForLog: redact common secret-bearing patterns (`?password=`, `?token=`, `?api_key=`, `?secret=`, `Authorization: Bearer / Basic …`) before the value reaches the log sink. BlueBubbles uses query-string auth by default, so attachment download failures and similar errors can carry the API password in the captured request URL (CWE-532). - history.fetchBlueBubblesMessageByGuid: validate the derived bareGuid (length ≤128, charset `[A-Za-z0-9._:-]+`, non-empty) before issuing the request. A trailing `/` would otherwise produce an empty bareGuid and turn the call into an unintended `/api/v1/message/` collection query (CWE-20). - monitor-processing reply-context verbose logs: log only `bodyLen=` metadata, not the quoted message text itself. Verbose logs are retained / shipped to aggregators and would otherwise leak private chat content (CWE-532). PR openclaw#73235 (BB routing guards) — 1 MEDIUM + 2 LOW: - monitor-reply-cache.resolveBlueBubblesMessageId: when `requireKnownShortId=true` and `chatContext` lacks any identifier, throw "requires a chat scope" instead of resolving the short id. Short ids are allocated from a single global counter across every account and chat, so an action call without chat scope could silently apply to the wrong conversation (CWE-285). Test updated to expect fail-closed (was previously fail-open with a comment that acknowledged the risk). - monitor-reply-cache.buildCrossChatError: replace raw inputId in the thrown error message with `<short:N-digit>` or `<uuid:prefix…>` shape descriptors. Combined with the earlier chatGuid redaction, cross-chat errors now leak no concrete identifier (CWE-117 / CWE-200). The Low PII finding on monitor-processing verbose logs is resolved automatically by the sanitizeForLog redaction extension added for PR openclaw#73241 openclaw#1. PR openclaw#73400 (silent-reply 🧵) — 1 MEDIUM: - silent-reply-policy.classifySilentReplyConversationType: classify a session key as `internal` only when `parseThreadSessionSuffix` returns a real `🧵<id>` suffix, not on a free-form `🧵` substring match. Caller-supplied session keys can no longer embed the marker mid-string to force the conversation type to `internal` and bypass silent-reply rewrites (CWE-840).

, openclaw#73241 Bundled hardening for the three open PRs: - session-files: use path.posix.join + strip CR/LF/TAB instead of rewriting backslashes to forward slashes. POSIX filenames may legally contain `\`, and the prior translation would synthesize fake path segments that bypass `excludeSourcePathRegex` (CWE-20). NUL is already rejected by Node's fs path layer. - monitor-reply-cache: redact chat identifiers in cross-chat error messages (CWE-200). Phone numbers / email addresses / chat GUIDs must not leak into agent transcripts, tool results, or remote channel deliveries. - monitor-reply-cache: rewrite isCrossChatMismatch so chatIdentifier and chatId comparisons run independently. Earlier version gated fallback comparisons on `!ctxChatGuid`, which let any non-empty ctx.chatGuid suppress the fallback checks when cached entry lacked chatGuid — letting a short id from chat A be reused while acting in chat B (CWE-697). - monitor-processing: drop group reactions where chatGuid / chatIdentifier is whitespace-only, not just empty. A webhook sender supplying " " or "\t" must not satisfy the guard and degrade peerId to the literal "group". - monitor-processing: sanitizeForLog around webhook senderId / messageId / action / attachment guid / err in verbose log lines and attachment download error logs (CWE-117). - monitor-processing: validate replyToId shape (length ≤128, charset alnum + `._:-`) before issuing the API fallback request, so a webhook with a pathological replyToId cannot drive arbitrary outbound load (cheap CWE-400 mitigation). Tests updated: monitor-reply-cache.test.ts cross-chat error assertions now expect `chatGuid=<redacted>` instead of raw values. Also fix unrelated typecheck regression: delivery-dispatch.mirror- bluebubbles.test.ts makeBaseParams missing the `runSessionKey` field introduced in DispatchCronDeliveryParams as part of v2026.4.26.

…penclaw#73241, openclaw#73400 Second hardening pass after the first round triggered new findings on Aisle's re-scan. PR openclaw#73406 (auto-reply voice silent) was meanwhile landed by the maintainer in upstream commit 28bf71d — that one will drop out at the next rebase via patch-id match, no source change here. PR openclaw#73241 (BB http hardening) — 1 HIGH + 2 MEDIUM: - monitor-processing.sanitizeForLog: redact common secret-bearing patterns (`?password=`, `?token=`, `?api_key=`, `?secret=`, `Authorization: Bearer / Basic …`) before the value reaches the log sink. BlueBubbles uses query-string auth by default, so attachment download failures and similar errors can carry the API password in the captured request URL (CWE-532). - history.fetchBlueBubblesMessageByGuid: validate the derived bareGuid (length ≤128, charset `[A-Za-z0-9._:-]+`, non-empty) before issuing the request. A trailing `/` would otherwise produce an empty bareGuid and turn the call into an unintended `/api/v1/message/` collection query (CWE-20). - monitor-processing reply-context verbose logs: log only `bodyLen=` metadata, not the quoted message text itself. Verbose logs are retained / shipped to aggregators and would otherwise leak private chat content (CWE-532). PR openclaw#73235 (BB routing guards) — 1 MEDIUM + 2 LOW: - monitor-reply-cache.resolveBlueBubblesMessageId: when `requireKnownShortId=true` and `chatContext` lacks any identifier, throw "requires a chat scope" instead of resolving the short id. Short ids are allocated from a single global counter across every account and chat, so an action call without chat scope could silently apply to the wrong conversation (CWE-285). Test updated to expect fail-closed (was previously fail-open with a comment that acknowledged the risk). - monitor-reply-cache.buildCrossChatError: replace raw inputId in the thrown error message with `<short:N-digit>` or `<uuid:prefix…>` shape descriptors. Combined with the earlier chatGuid redaction, cross-chat errors now leak no concrete identifier (CWE-117 / CWE-200). The Low PII finding on monitor-processing verbose logs is resolved automatically by the sanitizeForLog redaction extension added for PR openclaw#73241 openclaw#1. PR openclaw#73400 (silent-reply 🧵) — 1 MEDIUM: - silent-reply-policy.classifySilentReplyConversationType: classify a session key as `internal` only when `parseThreadSessionSuffix` returns a real `🧵<id>` suffix, not on a free-form `🧵` substring match. Caller-supplied session keys can no longer embed the marker mid-string to force the conversation type to `internal` and bypass silent-reply rewrites (CWE-840).

Chris Zhang added 4 commits April 28, 2026 11:57

docs(changelog): note BlueBubbles quoted-message fallback + attachmen…

81e5f69

…t error logging

openclaw-barnacle Bot added channel: bluebubbles Channel integration: bluebubbles size: S labels Apr 28, 2026

greptile-apps Bot reviewed Apr 28, 2026

View reviewed changes

clawsweeper Bot mentioned this pull request Apr 28, 2026

feat(bluebubbles): add reply-context API fallback for cache misses #71820

Merged

clawsweeper Bot closed this Apr 28, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(bluebubbles): fetch quoted message via SSRF-guarded API on reply cache miss + surface attachment download errors#73241

feat(bluebubbles): fetch quoted message via SSRF-guarded API on reply cache miss + surface attachment download errors#73241
zqchris wants to merge 6 commits intoopenclaw:mainfrom
zqchris:fix/bluebubbles-http-hardening

zqchris commented Apr 28, 2026

Uh oh!

greptile-apps Bot commented Apr 28, 2026

Uh oh!

greptile-apps Bot Apr 28, 2026

Uh oh!

coletebou commented Apr 28, 2026

Uh oh!

clawsweeper Bot commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

zqchris commented Apr 28, 2026

Summary

1. bluebubbles: fetch quoted message from API when reply cache misses

2. fix(bluebubbles): route quoted-message API fallback through SSRF-guarded client

3. fix(bluebubbles): surface attachment download failures at error level

Test plan

Notes for reviewers

Uh oh!

greptile-apps Bot commented Apr 28, 2026

Greptile Summary

Confidence Score: 4/5

Uh oh!

greptile-apps Bot Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

coletebou commented Apr 28, 2026

Uh oh!

clawsweeper Bot commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1. `bluebubbles: fetch quoted message from API when reply cache misses`

2. `fix(bluebubbles): route quoted-message API fallback through SSRF-guarded client`

3. `fix(bluebubbles): surface attachment download failures at error level`