mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 09:40:43 +00:00
fix(bluebubbles): resolve review findings — SSRF policy, reuse extractAttachments, add tests
- F1 (BLOCKER): pass undefined instead of {} for SSRF policy when
allowPrivateNetwork is false, so localhost BB servers are not blocked.
- F2 (IMPORTANT): reuse exported extractAttachments() from monitor-normalize
instead of duplicating field extraction logic.
- F3 (IMPORTANT): simplify asRecord(asRecord(payload)?.data) to
asRecord(payload.data) since payload is already Record<string, unknown>.
- F4 (NIT): bind retryMessageId before the guard to eliminate non-null assertion.
- F5 (IMPORTANT): add 4 tests for fetchBlueBubblesMessageAttachments covering
success, non-ok HTTP, empty data, and guid-less entries.
- Add CHANGELOG entry for the user-facing fix.
This commit is contained in:
@@ -42,6 +42,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Extensions/lmstudio: add exponential backoff to the inference-preload wrapper so an LM Studio model-load failure (for example the built-in memory guardrail rejecting a load because the swap is saturated) no longer produces a WARN line every ~2s for every chat request. The wrapper now records consecutive preload failures per `(baseUrl, modelKey, contextLength)` tuple with a 5s → 10s → 20s → … → 5min cooldown and skips the preload step entirely while a cooldown is active, letting chat requests proceed directly to the stream (the model is often already loaded via the LM Studio UI). The combined `preload failed` log line now reports consecutive-failure count and remaining cooldown so operators can act on the real issue instead of drowning in repeated warnings. (#67401) Thanks @xantorres.
|
||||
- Agents/replay: re-run tool/result pairing after strict replay tool-call ID sanitization on outbound requests so Anthropic-compatible providers like MiniMax no longer receive malformed orphan tool-result IDs such as `...toolresult1` during compaction and retry flows. (#67620) Thanks @stainlu.
|
||||
- Gateway/startup: fix spurious SIGUSR1 restart loop on Linux/systemd when plugin auto-enable is the only startup config write; the config hash guard was not captured for that write path, causing chokidar to treat each boot write as an external change and trigger a reload → restart cycle that corrupts manifest.db after repeated cycles. Fixes #67436. (#67557) thanks @openperf
|
||||
- BlueBubbles/inbound: restore inbound image attachment downloads on Node 22+ by stripping incompatible bundled-undici dispatchers from the non-SSRF fetch path, accept `updated-message` webhooks carrying attachments, use event-type-aware dedup keys so attachment follow-ups are not rejected as duplicates, and retry attachment fetch from the BB API when the initial webhook arrives with an empty array. (#64105, #61861, #65430, #67510) Thanks @omarshahine.
|
||||
|
||||
## 2026.4.15-beta.1
|
||||
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import "./test-mocks.js";
|
||||
import { downloadBlueBubblesAttachment, sendBlueBubblesAttachment } from "./attachments.js";
|
||||
import {
|
||||
downloadBlueBubblesAttachment,
|
||||
fetchBlueBubblesMessageAttachments,
|
||||
sendBlueBubblesAttachment,
|
||||
} from "./attachments.js";
|
||||
import { fetchBlueBubblesServerInfo, getCachedBlueBubblesPrivateApiStatus } from "./probe.js";
|
||||
import type { PluginRuntime } from "./runtime-api.js";
|
||||
import { setBlueBubblesRuntime } from "./runtime.js";
|
||||
@@ -769,3 +773,86 @@ describe("sendBlueBubblesAttachment", () => {
|
||||
).rejects.toThrow("chatGuid not found");
|
||||
});
|
||||
});
|
||||
|
||||
describe("fetchBlueBubblesMessageAttachments", () => {
|
||||
beforeEach(() => {
|
||||
mockFetch.mockReset();
|
||||
});
|
||||
|
||||
it("returns attachments from the BB API response", async () => {
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () =>
|
||||
Promise.resolve({
|
||||
data: {
|
||||
attachments: [
|
||||
{
|
||||
guid: "att-1",
|
||||
mimeType: "image/jpeg",
|
||||
transferName: "photo.jpg",
|
||||
totalBytes: 1024,
|
||||
},
|
||||
{
|
||||
guid: "att-2",
|
||||
mime_type: "image/png",
|
||||
transfer_name: "screenshot.png",
|
||||
total_bytes: 2048,
|
||||
},
|
||||
],
|
||||
},
|
||||
}),
|
||||
});
|
||||
const result = await fetchBlueBubblesMessageAttachments("msg-guid", {
|
||||
baseUrl: "http://localhost:1234",
|
||||
password: "test",
|
||||
});
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0].guid).toBe("att-1");
|
||||
expect(result[0].mimeType).toBe("image/jpeg");
|
||||
expect(result[1].guid).toBe("att-2");
|
||||
expect(result[1].mimeType).toBe("image/png");
|
||||
});
|
||||
|
||||
it("returns empty array on non-ok HTTP response", async () => {
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 404,
|
||||
});
|
||||
const result = await fetchBlueBubblesMessageAttachments("msg-guid", {
|
||||
baseUrl: "http://localhost:1234",
|
||||
password: "test",
|
||||
});
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it("returns empty array when data has no attachments", async () => {
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({ data: {} }),
|
||||
});
|
||||
const result = await fetchBlueBubblesMessageAttachments("msg-guid", {
|
||||
baseUrl: "http://localhost:1234",
|
||||
password: "test",
|
||||
});
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it("includes entries without a guid (downstream download handles filtering)", async () => {
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () =>
|
||||
Promise.resolve({
|
||||
data: {
|
||||
attachments: [{ mimeType: "image/jpeg" }, { guid: "att-valid", mimeType: "image/png" }],
|
||||
},
|
||||
}),
|
||||
});
|
||||
const result = await fetchBlueBubblesMessageAttachments("msg-guid", {
|
||||
baseUrl: "http://localhost:1234",
|
||||
password: "test",
|
||||
});
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0].guid).toBeUndefined();
|
||||
expect(result[1].guid).toBe("att-valid");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
normalizeOptionalString,
|
||||
} from "openclaw/plugin-sdk/text-runtime";
|
||||
import { resolveBlueBubblesServerAccount } from "./account-resolve.js";
|
||||
import { extractAttachments } from "./monitor-normalize.js";
|
||||
import { assertMultipartActionOk, postMultipartFormData } from "./multipart.js";
|
||||
import {
|
||||
fetchBlueBubblesServerInfo,
|
||||
@@ -117,7 +118,12 @@ export async function fetchBlueBubblesMessageAttachments(
|
||||
path: `/api/v1/message/${encodeURIComponent(messageGuid)}`,
|
||||
password: opts.password,
|
||||
});
|
||||
const policy = opts.allowPrivateNetwork ? { allowPrivateNetwork: true } : {};
|
||||
// Pass undefined (not {}) when private network is not opted-in so the
|
||||
// non-SSRF fallback path is used — an empty {} triggers the SSRF-guarded
|
||||
// path which blocks localhost BB servers by default. (#64105)
|
||||
const policy: SsrFPolicy | undefined = opts.allowPrivateNetwork
|
||||
? { allowPrivateNetwork: true }
|
||||
: undefined;
|
||||
const response = await blueBubblesFetchWithTimeout(
|
||||
url,
|
||||
{ method: "GET" },
|
||||
@@ -129,43 +135,10 @@ export async function fetchBlueBubblesMessageAttachments(
|
||||
}
|
||||
const json = (await response.json()) as Record<string, unknown>;
|
||||
const data = json.data as Record<string, unknown> | undefined;
|
||||
const rawAttachments = data?.attachments;
|
||||
if (!Array.isArray(rawAttachments)) {
|
||||
if (!data) {
|
||||
return [];
|
||||
}
|
||||
const out: BlueBubblesAttachment[] = [];
|
||||
for (const entry of rawAttachments) {
|
||||
if (!entry || typeof entry !== "object") {
|
||||
continue;
|
||||
}
|
||||
const record = entry as Record<string, unknown>;
|
||||
const guid = typeof record.guid === "string" ? record.guid.trim() : undefined;
|
||||
if (!guid) {
|
||||
continue;
|
||||
}
|
||||
out.push({
|
||||
guid,
|
||||
mimeType:
|
||||
typeof record.mimeType === "string"
|
||||
? record.mimeType
|
||||
: typeof record.mime_type === "string"
|
||||
? record.mime_type
|
||||
: undefined,
|
||||
transferName:
|
||||
typeof record.transferName === "string"
|
||||
? record.transferName
|
||||
: typeof record.transfer_name === "string"
|
||||
? record.transfer_name
|
||||
: undefined,
|
||||
totalBytes:
|
||||
typeof record.totalBytes === "number"
|
||||
? record.totalBytes
|
||||
: typeof record.total_bytes === "number"
|
||||
? record.total_bytes
|
||||
: undefined,
|
||||
});
|
||||
}
|
||||
return out;
|
||||
return extractAttachments(data);
|
||||
}
|
||||
|
||||
export async function downloadBlueBubblesAttachment(
|
||||
|
||||
@@ -34,7 +34,7 @@ function readNumberLike(record: Record<string, unknown> | null, key: string): nu
|
||||
return parseFiniteNumber(record[key]);
|
||||
}
|
||||
|
||||
function extractAttachments(message: Record<string, unknown>): BlueBubblesAttachment[] {
|
||||
export function extractAttachments(message: Record<string, unknown>): BlueBubblesAttachment[] {
|
||||
const raw = message["attachments"];
|
||||
if (!Array.isArray(raw)) {
|
||||
return [];
|
||||
|
||||
@@ -1074,16 +1074,17 @@ async function processMessageAfterDedupe(
|
||||
// `updated-message` event, wait briefly and re-fetch from the BB API as a
|
||||
// fallback for cases where BB doesn't send a follow-up webhook. (#65430, #67437)
|
||||
let resolvedAttachments = attachments;
|
||||
const retryMessageId = message.messageId?.trim();
|
||||
const shouldRetryAttachments =
|
||||
resolvedAttachments.length === 0 &&
|
||||
message.messageId &&
|
||||
retryMessageId &&
|
||||
baseUrl &&
|
||||
password &&
|
||||
(text.length === 0 || message.eventType === "updated-message");
|
||||
if (shouldRetryAttachments) {
|
||||
try {
|
||||
await new Promise<void>((resolve) => setTimeout(resolve, 2_000));
|
||||
const fetched = await fetchBlueBubblesMessageAttachments(message.messageId!, {
|
||||
const fetched = await fetchBlueBubblesMessageAttachments(retryMessageId, {
|
||||
baseUrl,
|
||||
password,
|
||||
timeoutMs: 10_000,
|
||||
|
||||
@@ -252,7 +252,7 @@ export async function handleBlueBubblesWebhookRequest(
|
||||
// BlueBubbles fires `updated-message` when attachments are indexed after the
|
||||
// initial `new-message` (which may arrive with attachments: []). Let those
|
||||
// through so the agent can ingest the image. (#65430)
|
||||
const dataRecord = asRecord(asRecord(payload)?.data);
|
||||
const dataRecord = asRecord(payload.data);
|
||||
const dataAttachments = dataRecord?.attachments;
|
||||
const isAttachmentUpdate =
|
||||
eventType === "updated-message" &&
|
||||
|
||||
Reference in New Issue
Block a user