bluebubbles: consolidate HTTP traffic through typed BlueBubblesClient#68234
bluebubbles: consolidate HTTP traffic through typed BlueBubblesClient#68234omarshahine merged 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ce51f7bd7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| baseUrl: resolved.baseUrl, | ||
| password: resolved.password, | ||
| }); | ||
| const cached = clientFingerprints.get(cacheKey); | ||
| if (cached && cached.fingerprint === fingerprint) { |
There was a problem hiding this comment.
Include policy inputs in BlueBubbles client cache key
The cache fingerprint only includes {baseUrl,password}, so createBlueBubblesClient can return a stale instance when config changes that alter client behavior but keep the same credentials (for example network.dangerouslyAllowPrivateNetwork, or other constructor-level options like auth strategy/default timeout). In this commit, the SSRF policy is resolved once in the constructor, so reusing a cached client after such a config update keeps the old policy active; because invalidateBlueBubblesClient is not called in production paths, these updates won’t take effect until restart.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Partially addressed. 6dfb043d80 brought authStrategy into the fingerprint via a stable id, so strategy swaps for the same credential rebuild. Leaving this thread open because the allowPrivateNetwork/timeoutMs dimensions you also flagged are not yet fingerprinted — production reload paths today go through gateway restart (which blows away the cache), so it's asymptomatic, but if a future hot-reload path lands it would resurface. Will track as a follow-up rather than expanding the fingerprint reactively.
Greptile SummaryThis PR consolidates all BlueBubbles outbound HTTP traffic through a single typed One correctness issue is worth addressing before the header-auth cutover (#66869): Confidence Score: 4/5Safe to merge for the query-string auth default; the header-auth readiness gap in requestMultipart and downloadAttachment should be fixed before #66869 lands. The SSRF consolidation logic is correct, tests are thorough, and the net effect is a meaningful bug fix for #34749 and #59722. The P1 finding (auth headers dropped for multipart/download in header-auth mode) does not affect the current QS-auth default but makes the 'one-line flip' claim inaccurate and will silently break attachment operations when #66869 is applied. The P2 cache fingerprint issue is unlikely to bite in practice but is a latent correctness gap in the public API. extensions/bluebubbles/src/client.ts — requestMultipart (lines 269-287) and downloadAttachment (lines 383-403) both need to forward the auth-decorated init to their respective lower-level fetch helpers before header auth is enabled. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/bluebubbles/src/client.ts
Line: 269-287
Comment:
**`requestMultipart` discards header-auth credentials — "one-line flip" claim is incomplete**
`buildAuthorizedRequest` applies auth to `prepared.init` (headers for `blueBubblesHeaderAuth`, URL for query-string). `requestMultipart` uses `prepared.url` but drops `prepared.init` entirely when calling `postMultipartFormData`. Since `postMultipartFormData` in `multipart.ts` builds its own hardcoded headers object (`{ "Content-Type": "multipart/form-data; boundary=..." }`) with no provision for additional headers, switching the default to `blueBubblesHeaderAuth` would silently omit auth for every attachment upload (`/api/v1/message/attachment`) and group-icon set (`/api/v1/chat/{guid}/icon`).
The same issue applies to `downloadAttachment` (lines 383–403): `prepared.init.headers` is built with the auth header but is never passed to `fetchRemoteMedia` or the `fetchImpl` callback.
Fixing this requires `postMultipartFormData` to accept an optional `extraHeaders` param, and `downloadAttachment` needs to merge `prepared.init` into the `fetchImpl` call. Neither change is here, so the PR description's "one-line default flip" for #66869 is inaccurate as-is.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/bluebubbles/src/client.ts
Line: 431-456
Comment:
**Cache fingerprint excludes `authStrategy` — wrong strategy can be silently returned**
`buildClientFingerprint` keys only on `baseUrl` and `password`. If `createBlueBubblesClient` is called first with the default strategy and later with `authStrategy: blueBubblesHeaderAuth` for the same account (same URL + credential), the second call returns the cached QS-auth instance rather than building a new header-auth client. This would be silent and hard to debug, since all current call-sites use the default strategy but the option is in the public API.
Consider including a stable auth-strategy identifier in the fingerprint so that different strategies for the same account don't collapse onto the same cached entry.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "bluebubbles: consolidate HTTP traffic th..." | Re-trigger Greptile |
6ce51f7 to
6dfb043
Compare
…ache fingerprint) Greptile findings from the first review cycle on #68234 — both real, both asymptomatic under today's query-string auth default but blocking for the header-auth flip (#66869). P1: Thread auth-decorated headers through multipart + media paths. - `postMultipartFormData` now accepts an optional `extraHeaders` param and merges caller headers with its own Content-Type (Content-Type always wins so the multipart boundary stays authoritative). - `client.requestMultipart` forwards `prepared.init.headers` — attachment uploads and group-icon sets keep working under header auth. - `client.downloadAttachment` fetchImpl merges `prepared.init.headers` with runtime-supplied headers (runtime wins on conflict, e.g. Range for partial reads) — attachment downloads via `fetchRemoteMedia` now carry auth too. P2: Include auth strategy in the client cache fingerprint. - `BlueBubblesAuthStrategy` gets a stable `id` field (`query-string` / `header:<name>`). - Built-in factories set it; the cache key is now `{baseUrl}|{password}|{authStrategyId}` so different strategies for the same account + credential no longer silently share a cached instance. Three new regression tests pin the above behavior (465 total, was 462). Full BB suite green, lint clean.
🔒 Aisle Security AnalysisWe found 4 potential security issue(s) in this PR:
1. 🟠 SSRF guard bypass via `allowedHostnames` disabling private-IP/DNS-rebinding checks for BlueBubbles server hostname
Description
In
As a result, a configured BlueBubbles hostname that is allowlisted via Vulnerable code: if (trustedHostname && (params.allowPrivateNetworkConfig !== false || !trustedHostnameIsPrivate)) {
return { ssrfPolicy: { allowedHostnames: [trustedHostname] }, ... };
}This weakens SSRF protections for the common case where RecommendationDo not use Instead:
Example fix: if (params.allowPrivateNetwork) {
return { ssrfPolicy: { allowPrivateNetwork: true }, trustedHostname, trustedHostnameIsPrivate };
}
if (trustedHostname) {
// Always constrain to the configured hostname
const hostnameAllowlist = [trustedHostname];
if (trustedHostnameIsPrivate && params.allowPrivateNetworkConfig !== false) {
// Explicitly allow private targets for the configured private hostname
return {
ssrfPolicy: { hostnameAllowlist, allowedHostnames: [trustedHostname] },
trustedHostname,
trustedHostnameIsPrivate,
};
}
// Public hostname: allow only this hostname, but keep DNS/private-IP blocking
return {
ssrfPolicy: { hostnameAllowlist },
trustedHostname,
trustedHostnameIsPrivate,
};
}
return { ssrfPolicy: {}, trustedHostname, trustedHostnameIsPrivate };Add tests ensuring that with a public 2. 🟡 BlueBubbles API password appended to request URLs by default (query-string auth)
DescriptionThe new consolidated This is a credential leakage risk because URLs are commonly:
In this change, the client centralization makes this behavior more consistent across code paths (including attachment downloads and multipart), increasing the likelihood that the password appears in logs. Vulnerable code: export function blueBubblesQueryStringAuth(password: string): BlueBubblesAuthStrategy {
return {
id: "query-string",
decorate({ url }) {
url.searchParams.set("password", password);
},
};
}
// default:
const authFactory = opts.authStrategy ?? blueBubblesQueryStringAuth;RecommendationPrefer header-based authentication so secrets are not placed in URLs.
Example safer default: export function createBlueBubblesClient(opts: BlueBubblesClientOptions = {}): BlueBubblesClient {
// ...resolve baseUrl/password...
const authFactory = opts.authStrategy ?? blueBubblesHeaderAuth;
const authStrategy = authFactory(resolved.password);
// ...
}If query-string auth must remain for compatibility, add a redaction helper before any logging/telemetry of request URLs (e.g., replace 3. 🟡 SSRF guard bypass via exported test hook `_setFetchGuardForTesting`
Description
Vulnerable code: // Overridable guard for testing; production code uses fetchWithSsrFGuard.
let _fetchGuard = fetchWithSsrFGuard;
/** @internal Replace the SSRF fetch guard in tests. */
export function _setFetchGuardForTesting(impl: typeof fetchWithSsrFGuard | null): void {
_fetchGuard = impl ?? fetchWithSsrFGuard;
}While intended for tests (and used by RecommendationRemove this hook from production builds or strictly gate it to test environments. Options:
let _fetchGuard = fetchWithSsrFGuard;
export function _setFetchGuardForTesting(impl: typeof fetchWithSsrFGuard | null): void {
if (process.env.NODE_ENV !== "test") {
throw new Error("_setFetchGuardForTesting is test-only");
}
_fetchGuard = impl ?? fetchWithSsrFGuard;
}
4. 🔵 Plaintext BlueBubbles password stored in global client-cache fingerprint string
Description
Vulnerable code: return `${params.baseUrl}|${params.password}|${params.authStrategyId}`;RecommendationAvoid storing plaintext credentials in cache keys/values. Use a one-way hash (or keyed HMAC) of the tuple instead of embedding the password directly: import { createHash } from "node:crypto";
function buildClientFingerprint(params: { baseUrl: string; password: string; authStrategyId: string }): string {
const raw = `${params.baseUrl}|${params.password}|${params.authStrategyId}`;
return createHash("sha256").update(raw, "utf8").digest("hex");
}Optionally, consider:
Analyzed PR: #68234 at commit Last updated on: 2026-04-19T15:46:15Z |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6dfb043d80
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…(aisle #68234 HIGH) Aisle security analysis flagged `resolveBlueBubblesClientSsrfPolicy` mode 3 as an SSRF bypass: it returned `ssrfPolicy: undefined`, which `blueBubblesFetchWithTimeout` treats as a signal to skip `fetchWithSsrFGuard` and call `fetch()` directly. Mode 3 fires in two cases: A. Private hostname + user explicitly opted out (`network.dangerouslyAllowPrivateNetwork: false`). The user asked us to block private networks and we instead sent the request via the unguarded fallback path — a real bypass. B. Unparseable baseUrl. Would route through the unguarded path (cosmetic in practice since the fetch would fail elsewhere, but still wrong). Fix: return `ssrfPolicy: {}` (default-deny guarded policy) instead of `undefined`. All three modes now go through `fetchWithSsrFGuard`: 1. `{ allowPrivateNetwork: true }` — user opted in 2. `{ allowedHostnames: [host] }` — narrow allowlist for trusted host 3. `{}` — default-deny, honors opt-out Tightened `ssrfPolicy` field/param types from `SsrFPolicy | undefined` to `SsrFPolicy` so the bypass cannot be reintroduced by a future caller handing back `undefined`. Added a mode-3 invariant regression test that walks every resolution case and asserts `ssrfPolicy` is always defined (caught any future `undefined` reintroduction). Updated `attachments.test.ts` case that was asserting the old (buggy) `undefined` behavior. Existing-user impact: only users with `dangerouslyAllowPrivateNetwork: false` set on a private-address BB server see a behavior change (they now get a guarded refusal instead of an accidental success). That's the correct posture per their config.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ef012f20c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ache fingerprint) Greptile findings from the first review cycle on #68234 — both real, both asymptomatic under today's query-string auth default but blocking for the header-auth flip (#66869). P1: Thread auth-decorated headers through multipart + media paths. - `postMultipartFormData` now accepts an optional `extraHeaders` param and merges caller headers with its own Content-Type (Content-Type always wins so the multipart boundary stays authoritative). - `client.requestMultipart` forwards `prepared.init.headers` — attachment uploads and group-icon sets keep working under header auth. - `client.downloadAttachment` fetchImpl merges `prepared.init.headers` with runtime-supplied headers (runtime wins on conflict, e.g. Range for partial reads) — attachment downloads via `fetchRemoteMedia` now carry auth too. P2: Include auth strategy in the client cache fingerprint. - `BlueBubblesAuthStrategy` gets a stable `id` field (`query-string` / `header:<name>`). - Built-in factories set it; the cache key is now `{baseUrl}|{password}|{authStrategyId}` so different strategies for the same account + credential no longer silently share a cached instance. Three new regression tests pin the above behavior (465 total, was 462). Full BB suite green, lint clean.
33b8827 to
81b0ae1
Compare
…(aisle #68234 HIGH) Aisle security analysis flagged `resolveBlueBubblesClientSsrfPolicy` mode 3 as an SSRF bypass: it returned `ssrfPolicy: undefined`, which `blueBubblesFetchWithTimeout` treats as a signal to skip `fetchWithSsrFGuard` and call `fetch()` directly. Mode 3 fires in two cases: A. Private hostname + user explicitly opted out (`network.dangerouslyAllowPrivateNetwork: false`). The user asked us to block private networks and we instead sent the request via the unguarded fallback path — a real bypass. B. Unparseable baseUrl. Would route through the unguarded path (cosmetic in practice since the fetch would fail elsewhere, but still wrong). Fix: return `ssrfPolicy: {}` (default-deny guarded policy) instead of `undefined`. All three modes now go through `fetchWithSsrFGuard`: 1. `{ allowPrivateNetwork: true }` — user opted in 2. `{ allowedHostnames: [host] }` — narrow allowlist for trusted host 3. `{}` — default-deny, honors opt-out Tightened `ssrfPolicy` field/param types from `SsrFPolicy | undefined` to `SsrFPolicy` so the bypass cannot be reintroduced by a future caller handing back `undefined`. Added a mode-3 invariant regression test that walks every resolution case and asserts `ssrfPolicy` is always defined (caught any future `undefined` reintroduction). Updated `attachments.test.ts` case that was asserting the old (buggy) `undefined` behavior. Existing-user impact: only users with `dangerouslyAllowPrivateNetwork: false` set on a private-address BB server see a behavior change (they now get a guarded refusal instead of an accidental success). That's the correct posture per their config.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81b0ae1121
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return await blueBubblesFetchWithTimeout( | ||
| resolveRequestUrl(input), | ||
| { ...init, method: init?.method ?? "GET", headers: mergedHeaders }, | ||
| effectiveTimeoutMs, | ||
| clientSsrfPolicy, |
There was a problem hiding this comment.
Preserve streaming in attachment download fetch callback
Passing clientSsrfPolicy here forces blueBubblesFetchWithTimeout onto its guarded branch, which buffers the entire response body before returning (response.arrayBuffer() in types.ts). In this call path, fetchRemoteMedia is supposed to enforce maxBytes while streaming, so pre-buffering defeats that protection for large/chunked attachments and can cause avoidable memory spikes or OOMs before the size limit is enforced.
Useful? React with 👍 / 👎.
|
. |
Closes #34749, closes #59722. All outbound BlueBubbles HTTP calls now route through a single typed BlueBubblesClient that resolves SSRF policy and auth material once at construction, instead of each of the 17 callsites computing its own SsrFPolicy and threading it through blueBubblesFetchWithTimeout. Three-mode policy resolved in one place (previously inlined in attachments.ts only): 1. { allowPrivateNetwork: true } — user opted in 2. { allowedHostnames: [trustedHost] } — narrow allowlist 3. undefined — non-SSRF fallback path Pluggable auth via BlueBubblesAuthStrategy so the ?password= → header-auth switch (#66869) becomes a one-line default flip. downloadAttachment threads this.ssrfPolicy to BOTH fetchRemoteMedia AND the fetchImpl callback, closing #34749 where the legacy helper silently omitted the policy on the callback path. reactions.ts now uses the same policy as every other call, closing #59722 where the asymmetric `{}` fallback blocked private-IP BB deployments. Monitor test files install the shared SSRF-guard passthrough to accommodate the guarded request path — required because the consolidation moves callers from the unguarded fallback onto the mode-2 allowlist by default for self-hosted BB servers. Net: +171 / -310 LOC across BB files; BB test suite stays green at 462/462. Lint clean. pnpm tsgo and pnpm build have pre-existing unrelated errors on origin/main (codex/discord/qa-lab and amazon-bedrock-mantle runtime-deps staging) — both identical on clean main before this branch. Future-proofed (not flipped here): - #66869 header auth cutover (one-line default change) - #67752 SSRF tightening (now single-point-of-change)
…ache fingerprint) Greptile findings from the first review cycle on #68234 — both real, both asymptomatic under today's query-string auth default but blocking for the header-auth flip (#66869). P1: Thread auth-decorated headers through multipart + media paths. - `postMultipartFormData` now accepts an optional `extraHeaders` param and merges caller headers with its own Content-Type (Content-Type always wins so the multipart boundary stays authoritative). - `client.requestMultipart` forwards `prepared.init.headers` — attachment uploads and group-icon sets keep working under header auth. - `client.downloadAttachment` fetchImpl merges `prepared.init.headers` with runtime-supplied headers (runtime wins on conflict, e.g. Range for partial reads) — attachment downloads via `fetchRemoteMedia` now carry auth too. P2: Include auth strategy in the client cache fingerprint. - `BlueBubblesAuthStrategy` gets a stable `id` field (`query-string` / `header:<name>`). - Built-in factories set it; the cache key is now `{baseUrl}|{password}|{authStrategyId}` so different strategies for the same account + credential no longer silently share a cached instance. Three new regression tests pin the above behavior (465 total, was 462). Full BB suite green, lint clean.
…(aisle #68234 HIGH) Aisle security analysis flagged `resolveBlueBubblesClientSsrfPolicy` mode 3 as an SSRF bypass: it returned `ssrfPolicy: undefined`, which `blueBubblesFetchWithTimeout` treats as a signal to skip `fetchWithSsrFGuard` and call `fetch()` directly. Mode 3 fires in two cases: A. Private hostname + user explicitly opted out (`network.dangerouslyAllowPrivateNetwork: false`). The user asked us to block private networks and we instead sent the request via the unguarded fallback path — a real bypass. B. Unparseable baseUrl. Would route through the unguarded path (cosmetic in practice since the fetch would fail elsewhere, but still wrong). Fix: return `ssrfPolicy: {}` (default-deny guarded policy) instead of `undefined`. All three modes now go through `fetchWithSsrFGuard`: 1. `{ allowPrivateNetwork: true }` — user opted in 2. `{ allowedHostnames: [host] }` — narrow allowlist for trusted host 3. `{}` — default-deny, honors opt-out Tightened `ssrfPolicy` field/param types from `SsrFPolicy | undefined` to `SsrFPolicy` so the bypass cannot be reintroduced by a future caller handing back `undefined`. Added a mode-3 invariant regression test that walks every resolution case and asserts `ssrfPolicy` is always defined (caught any future `undefined` reintroduction). Updated `attachments.test.ts` case that was asserting the old (buggy) `undefined` behavior. Existing-user impact: only users with `dangerouslyAllowPrivateNetwork: false` set on a private-address BB server see a behavior change (they now get a guarded refusal instead of an accidental success). That's the correct posture per their config.
81b0ae1 to
ee72657
Compare
|
Merged via squash.
Thanks @omarshahine! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee72657bc8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const client = createBlueBubblesClientFromParts({ | ||
| baseUrl, | ||
| password, | ||
| allowPrivateNetwork, | ||
| timeoutMs: opts.timeoutMs ?? 10000, |
There was a problem hiding this comment.
Preserve explicit private-network opt-out in history fetch
Fresh evidence beyond the existing send/probe thread: this migration switches history reads from an explicit {} SSRF policy to createBlueBubblesClientFromParts while only passing allowPrivateNetwork. In the network.dangerouslyAllowPrivateNetwork: false + private-host case, allowPrivateNetworkConfig becomes undefined, so resolveBlueBubblesClientSsrfPolicy takes the allowlist branch and still permits calls to the private BlueBubbles host; the pre-change code in this function used {} and blocked that path. This means an operator’s explicit opt-out is not enforced for history requests (and the same pattern appears in catchup).
Useful? React with 👍 / 👎.
…openclaw#68234) Merged via squash. Prepared head SHA: ee72657 Co-authored-by: omarshahine <10343873+omarshahine@users.noreply.github.com> Co-authored-by: omarshahine <10343873+omarshahine@users.noreply.github.com> Reviewed-by: @omarshahine
…openclaw#68234) Merged via squash. Prepared head SHA: ee72657 Co-authored-by: omarshahine <10343873+omarshahine@users.noreply.github.com> Co-authored-by: omarshahine <10343873+omarshahine@users.noreply.github.com> Reviewed-by: @omarshahine
…nt merge, dead code) - Route reply-context fetch through resolveBlueBubblesClientSsrfPolicy so the SSRF guard is always engaged. The three-mode resolver never returns undefined; previously we passed undefined when private-network opt-in was off, which silently skipped fetchWithSsrFGuard. Same bug class as openclaw#68234. - Drop .default(false) from per-account replyContextApiFallback so accounts that omit the flag inherit the channel-level value through mergeAccountConfig instead of clobbering it with false. - Remove redundant if (body || sender) guard left after the early-return on the previous line. - Add three-mode SSRF coverage in monitor-reply-fetch.test.ts (mode 1 opt-in, mode 2 hostname allowlist, mode 3 default-deny) plus an explicit regression test, and end-to-end inheritance tests in setup-surface.test.ts through resolveBlueBubblesAccount.
…openclaw#68234) Merged via squash. Prepared head SHA: ee72657 Co-authored-by: omarshahine <10343873+omarshahine@users.noreply.github.com> Co-authored-by: omarshahine <10343873+omarshahine@users.noreply.github.com> Reviewed-by: @omarshahine
…openclaw#68234) Merged via squash. Prepared head SHA: ee72657 Co-authored-by: omarshahine <10343873+omarshahine@users.noreply.github.com> Co-authored-by: omarshahine <10343873+omarshahine@users.noreply.github.com> Reviewed-by: @omarshahine
Ready for review. Smoke-tested live on Lobster on
3ef012f20c.Summary
All outbound BlueBubbles HTTP calls now route through a single typed
BlueBubblesClientthat resolves SSRF policy and auth material once at construction. Previously each of the 17 callsites computed its ownSsrFPolicyand threaded it throughblueBubblesFetchWithTimeout— the per-callsite divergence is what caused #34749 (image attachments blocked on localhost) and #59722 (reactions blocked on private IPs).What closes
downloadAttachmentnow threadsthis.ssrfPolicyto BOTHfetchRemoteMediaAND thefetchImplcallback, where the legacy helper silently omitted the policy on the callback path.reactions.tsnow uses the same policy as every other call; the asymmetric{}fallback that blocked private-IP BB deployments is gone.How
New
extensions/bluebubbles/src/client.ts:BlueBubblesClientclass with typed operations (request,requestJson,requestMultipart,ping,getServerInfo,react,getMessageAttachments,downloadAttachment).createBlueBubblesClient({cfg, accountId, serverUrl, password, ...})for full-config callers.createBlueBubblesClientFromParts({baseUrl, password, allowPrivateNetwork, ...})for low-level helpers that already have the resolved tuple.fetchWithSsrFGuard— mode 3 returns{}(default-deny guarded), neverundefined, so a config that opts out of private networks cannot slip onto the unguarded path:{ allowPrivateNetwork: true }— user opted in{ allowedHostnames: [trustedHost] }— narrow allowlist{}— default-deny guarded (opt-out or unparseable URL)BlueBubblesAuthStrategywithblueBubblesQueryStringAuth(default, preserves current?password=) andblueBubblesHeaderAuthso the BlueBubbles: move?password=URL auth to header-based auth across all BB API calls #66869 header-auth cutover becomes a one-line default flip. Both strategies carry a stableidthat participates in the client cache fingerprint so different strategies for the same credential don't collapse.Migrated:
attachments.ts,reactions.ts,probe.ts,chat.ts,send.ts,history.ts,catchup.ts,monitor-processing.tsall now route through the client. Public function signatures preserved for compatibility.Multipart + download auth plumbing:
postMultipartFormDataacceptsextraHeadersandclient.requestMultipartforwardsprepared.init.headers;downloadAttachmentmergesprepared.init.headersinto thefetchImplinit so a futureblueBubblesHeaderAuthflip covers attachment uploads and downloads without a follow-up refactor.Test fixtures:
monitor.test.tsandmonitor.webhook-auth.test.tsinstall the shared SSRF-guard passthrough (createBlueBubblesFetchGuardPassthroughInstaller) — required because the consolidation moves callers from the unguarded fallback onto the mode-2 allowlist for self-hosted BB servers.Size
465 tests green (was 462 at initial open). New coverage locks in the header-auth plumbing, the cache-fingerprint authStrategy dimension, and the mode-3 "ssrfPolicy is never undefined" invariant.
Verification
pnpm test extensions/bluebubbles— 22 files / 465 tests passpnpm lint extensions/bluebubbles/src/— 0 warnings / 0 errorspnpm build— clean on this branch (theamazon-bedrock-mantlestaging error referenced at initial open has been resolved upstream)pnpm tsgo— clean on bluebubbles; remaining errors (discord / qa-lab / src/commands) are pre-existing onorigin/mainand unrelated to this change3ef012f20c)[gateway] ready (18 plugins ... 22.1s); BlueBubbles provider + Private API upagent:lobster:bluebubbles:direct:<handle>)downloadAttachment→fetchRemoteMediasaved to~/.openclaw/media/inbound/<uuid>.jpg(BlueBubbles: image attachments blocked by SSRF guard (localhost URL) #34749 regression path green)sendMessageBlueBubbles+send.ts, both OKreactviaBlueBubblesClient.reactuses the client SSRF policy (no asymmetric{}fallback) — BlueBubbles: SSRF allowlist doesn't cover reactions (react action blocked on private IP) #59722 regression path greenreplayed=1 failed=0 fetched=2, zero errors in stderr/sessionReview feedback addressed
requestMultipart/downloadAttachmentdrop auth headers —6dfb043d80.authStrategy—6dfb043d80(stableidfield on the strategy interface).dangerouslyAllowPrivateNetwork: falsenot honored on probe / send paths —3ef012f20c(mode 3 returns guarded{}, type tightened toSsrFPolicy; invariant test assertsssrfPolicyis always defined).Deferred to follow-ups (intentionally NOT in this PR)
?password=URL auth to header-based auth across all BB API calls #66869 header auth cutover —AuthStrategyinterface is in place; flipping the default requires BB Server coordination.allowPrivateNetwork+timeoutMs— open thread on the cache-staleness P1 comment. Asymptomatic today (config reload currently goes through gateway restart, which evicts the cache), tracking as a follow-up.blueBubblesFetchWithTimeout/buildBlueBubblesApiUrlfromtypes.ts— they're now private-to-client infrastructure; safe to remove once this lands.Blocks #57181, #60715 (both become instance-level tests against
BlueBubblesClientonce this lands).🤖 Generated with Claude Code