fix: comprehensive BlueBubbles and channel cleanup (#11093)
* feat(bluebubbles): auto-strip markdown from outbound messages (#7402) * fix(security): add timeout to webhook body reading (#6762) Adds 30-second timeout to readBody() in voice-call, bluebubbles, and nostr webhook handlers. Prevents Slow-Loris DoS (CWE-400, CVSS 7.5). Merged with existing maxBytes protection in voice-call. * fix(security): unify Error objects and lint fixes in webhook timeouts (#6762) * fix: prevent plugins from auto-enabling without user consent (#3961) Changes default plugin enabled state from true to false in enablePluginEntry(). Preserves existing enabled:true values. Fixes #3932. * fix: apply hierarchical mediaMaxMb config to all channels (#8749) Generalizes resolveAttachmentMaxBytes() to use account → channel → global config resolution for all channels, not just BlueBubbles. Fixes #7847. * fix(bluebubbles): sanitize attachment filenames against header injection (#10333) Strip ", \r, \n, and \\ from filenames after path.basename() to prevent multipart Content-Disposition header injection (CWE-93, CVSS 5.4). Also adds sanitization to setGroupIconBlueBubbles which had zero filename sanitization. * fix(lint): exclude extensions/ from Oxlint preflight check (#9313) Extensions use PluginRuntime|null patterns that trigger no-redundant-type-constituents because PluginRuntime resolves to any. Excluding extensions/ from Oxlint unblocks user upgrades. Re-applies the approach from closed PR #10087. * fix(bluebubbles): add tempGuid to createNewChatWithMessage payload (#7745) Non-Private-API mode (AppleScript) requires tempGuid in send payloads. The main sendMessageBlueBubbles already had it, but createNewChatWithMessage was missing it, causing 400 errors for new chat creation without Private API. * fix: send stop-typing signal when run ends with NO_REPLY (#8785) Adds onCleanup callback to the typing controller that fires when the controller is cleaned up while typing was active (e.g., after NO_REPLY). Channels using createTypingCallbacks automatically get stop-typing on cleanup. This prevents the typing indicator from lingering in group chats when the agent decides not to reply. * fix(telegram): deduplicate skill commands in multi-agent setup (#5717) Two fixes: 1. Skip duplicate workspace dirs when listing skill commands across agents. Multiple agents sharing the same workspace would produce duplicate commands with _2, _3 suffixes. 2. Clear stale commands via deleteMyCommands before registering new ones. Commands from deleted skills now get cleaned up on restart. * fix: add size limits to unbounded in-memory caches (#4948) Adds max-size caps with oldest-entry eviction to prevent OOM in long-running deployments: - BlueBubbles serverInfoCache: 64 entries (already has TTL) - Google Chat authCache: 32 entries - Matrix directRoomCache: 1024 entries - Discord presenceCache: 5000 entries per account * fix: address review concerns (#11093) - Chain deleteMyCommands → setMyCommands to prevent race condition (#5717) - Rename enablePluginEntry to registerPluginEntry (now sets enabled: false) - Add Slow-Loris timeout test for readJsonBody (#6023)
This commit is contained in:
@@ -26,7 +26,9 @@ const AUDIO_MIME_CAF = new Set(["audio/x-caf", "audio/caf"]);
|
||||
function sanitizeFilename(input: string | undefined, fallback: string): string {
|
||||
const trimmed = input?.trim() ?? "";
|
||||
const base = trimmed ? path.basename(trimmed) : "";
|
||||
return base || fallback;
|
||||
const name = base || fallback;
|
||||
// Strip characters that could enable multipart header injection (CWE-93)
|
||||
return name.replace(/[\r\n"\\]/g, "_");
|
||||
}
|
||||
|
||||
function ensureExtension(filename: string, extension: string, fallbackBase: string): string {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk";
|
||||
import crypto from "node:crypto";
|
||||
import path from "node:path";
|
||||
import { resolveBlueBubblesAccount } from "./accounts.js";
|
||||
import { blueBubblesFetchWithTimeout, buildBlueBubblesApiUrl } from "./types.js";
|
||||
|
||||
@@ -336,10 +337,13 @@ export async function setGroupIconBlueBubbles(
|
||||
const parts: Uint8Array[] = [];
|
||||
const encoder = new TextEncoder();
|
||||
|
||||
// Sanitize filename to prevent multipart header injection (CWE-93)
|
||||
const safeFilename = path.basename(filename).replace(/[\r\n"\\]/g, "_") || "icon.png";
|
||||
|
||||
// Add file field named "icon" as per API spec
|
||||
parts.push(encoder.encode(`--${boundary}\r\n`));
|
||||
parts.push(
|
||||
encoder.encode(`Content-Disposition: form-data; name="icon"; filename="${filename}"\r\n`),
|
||||
encoder.encode(`Content-Disposition: form-data; name="icon"; filename="${safeFilename}"\r\n`),
|
||||
);
|
||||
parts.push(
|
||||
encoder.encode(`Content-Type: ${opts.contentType ?? "application/octet-stream"}\r\n\r\n`),
|
||||
|
||||
@@ -393,6 +393,48 @@ describe("BlueBubbles webhook monitor", () => {
|
||||
expect(res.statusCode).toBe(400);
|
||||
});
|
||||
|
||||
it("returns 400 when request body times out (Slow-Loris protection)", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const account = createMockAccount();
|
||||
const config: OpenClawConfig = {};
|
||||
const core = createMockRuntime();
|
||||
setBlueBubblesRuntime(core);
|
||||
|
||||
unregister = registerBlueBubblesWebhookTarget({
|
||||
account,
|
||||
config,
|
||||
runtime: { log: vi.fn(), error: vi.fn() },
|
||||
core,
|
||||
path: "/bluebubbles-webhook",
|
||||
});
|
||||
|
||||
// Create a request that never sends data or ends (simulates slow-loris)
|
||||
const req = new EventEmitter() as IncomingMessage;
|
||||
req.method = "POST";
|
||||
req.url = "/bluebubbles-webhook";
|
||||
req.headers = {};
|
||||
(req as unknown as { socket: { remoteAddress: string } }).socket = {
|
||||
remoteAddress: "127.0.0.1",
|
||||
};
|
||||
req.destroy = vi.fn();
|
||||
|
||||
const res = createMockResponse();
|
||||
|
||||
const handledPromise = handleBlueBubblesWebhookRequest(req, res);
|
||||
|
||||
// Advance past the 30s timeout
|
||||
await vi.advanceTimersByTimeAsync(31_000);
|
||||
|
||||
const handled = await handledPromise;
|
||||
expect(handled).toBe(true);
|
||||
expect(res.statusCode).toBe(400);
|
||||
expect(req.destroy).toHaveBeenCalled();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("authenticates via password query parameter", async () => {
|
||||
const account = createMockAccount({ password: "secret-token" });
|
||||
const config: OpenClawConfig = {};
|
||||
|
||||
@@ -508,14 +508,29 @@ export function registerBlueBubblesWebhookTarget(target: WebhookTarget): () => v
|
||||
};
|
||||
}
|
||||
|
||||
async function readJsonBody(req: IncomingMessage, maxBytes: number) {
|
||||
async function readJsonBody(req: IncomingMessage, maxBytes: number, timeoutMs = 30_000) {
|
||||
const chunks: Buffer[] = [];
|
||||
let total = 0;
|
||||
return await new Promise<{ ok: boolean; value?: unknown; error?: string }>((resolve) => {
|
||||
let done = false;
|
||||
const finish = (result: { ok: boolean; value?: unknown; error?: string }) => {
|
||||
if (done) {
|
||||
return;
|
||||
}
|
||||
done = true;
|
||||
clearTimeout(timer);
|
||||
resolve(result);
|
||||
};
|
||||
|
||||
const timer = setTimeout(() => {
|
||||
finish({ ok: false, error: "request body timeout" });
|
||||
req.destroy();
|
||||
}, timeoutMs);
|
||||
|
||||
req.on("data", (chunk: Buffer) => {
|
||||
total += chunk.length;
|
||||
if (total > maxBytes) {
|
||||
resolve({ ok: false, error: "payload too large" });
|
||||
finish({ ok: false, error: "payload too large" });
|
||||
req.destroy();
|
||||
return;
|
||||
}
|
||||
@@ -525,27 +540,30 @@ async function readJsonBody(req: IncomingMessage, maxBytes: number) {
|
||||
try {
|
||||
const raw = Buffer.concat(chunks).toString("utf8");
|
||||
if (!raw.trim()) {
|
||||
resolve({ ok: false, error: "empty payload" });
|
||||
finish({ ok: false, error: "empty payload" });
|
||||
return;
|
||||
}
|
||||
try {
|
||||
resolve({ ok: true, value: JSON.parse(raw) as unknown });
|
||||
finish({ ok: true, value: JSON.parse(raw) as unknown });
|
||||
return;
|
||||
} catch {
|
||||
const params = new URLSearchParams(raw);
|
||||
const payload = params.get("payload") ?? params.get("data") ?? params.get("message");
|
||||
if (payload) {
|
||||
resolve({ ok: true, value: JSON.parse(payload) as unknown });
|
||||
finish({ ok: true, value: JSON.parse(payload) as unknown });
|
||||
return;
|
||||
}
|
||||
throw new Error("invalid json");
|
||||
}
|
||||
} catch (err) {
|
||||
resolve({ ok: false, error: err instanceof Error ? err.message : String(err) });
|
||||
finish({ ok: false, error: err instanceof Error ? err.message : String(err) });
|
||||
}
|
||||
});
|
||||
req.on("error", (err) => {
|
||||
resolve({ ok: false, error: err instanceof Error ? err.message : String(err) });
|
||||
finish({ ok: false, error: err instanceof Error ? err.message : String(err) });
|
||||
});
|
||||
req.on("close", () => {
|
||||
finish({ ok: false, error: "connection closed" });
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
@@ -16,7 +16,9 @@ export type BlueBubblesServerInfo = {
|
||||
computer_id?: string;
|
||||
};
|
||||
|
||||
/** Cache server info by account ID to avoid repeated API calls */
|
||||
/** Cache server info by account ID to avoid repeated API calls.
|
||||
* Size-capped to prevent unbounded growth (#4948). */
|
||||
const MAX_SERVER_INFO_CACHE_SIZE = 64;
|
||||
const serverInfoCache = new Map<string, { info: BlueBubblesServerInfo; expires: number }>();
|
||||
const CACHE_TTL_MS = 10 * 60 * 1000; // 10 minutes
|
||||
|
||||
@@ -56,6 +58,13 @@ export async function fetchBlueBubblesServerInfo(params: {
|
||||
const data = payload?.data as BlueBubblesServerInfo | undefined;
|
||||
if (data) {
|
||||
serverInfoCache.set(cacheKey, { info: data, expires: Date.now() + CACHE_TTL_MS });
|
||||
// Evict oldest entries if cache exceeds max size
|
||||
if (serverInfoCache.size > MAX_SERVER_INFO_CACHE_SIZE) {
|
||||
const oldest = serverInfoCache.keys().next().value;
|
||||
if (oldest !== undefined) {
|
||||
serverInfoCache.delete(oldest);
|
||||
}
|
||||
}
|
||||
}
|
||||
return data ?? null;
|
||||
} catch {
|
||||
|
||||
@@ -370,6 +370,16 @@ describe("send", () => {
|
||||
).rejects.toThrow("requires text");
|
||||
});
|
||||
|
||||
it("throws when text becomes empty after markdown stripping", async () => {
|
||||
// Edge case: input like "***" or "---" passes initial check but becomes empty after stripMarkdown
|
||||
await expect(
|
||||
sendMessageBlueBubbles("+15551234567", "***", {
|
||||
serverUrl: "http://localhost:1234",
|
||||
password: "test",
|
||||
}),
|
||||
).rejects.toThrow("empty after markdown removal");
|
||||
});
|
||||
|
||||
it("throws when serverUrl is missing", async () => {
|
||||
await expect(sendMessageBlueBubbles("+15551234567", "Hello", {})).rejects.toThrow(
|
||||
"serverUrl is required",
|
||||
@@ -438,6 +448,77 @@ describe("send", () => {
|
||||
expect(body.method).toBeUndefined();
|
||||
});
|
||||
|
||||
it("strips markdown formatting from outbound messages", async () => {
|
||||
mockFetch
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () =>
|
||||
Promise.resolve({
|
||||
data: [
|
||||
{
|
||||
guid: "iMessage;-;+15551234567",
|
||||
participants: [{ address: "+15551234567" }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
text: () =>
|
||||
Promise.resolve(
|
||||
JSON.stringify({
|
||||
data: { guid: "msg-uuid-stripped" },
|
||||
}),
|
||||
),
|
||||
});
|
||||
|
||||
const result = await sendMessageBlueBubbles(
|
||||
"+15551234567",
|
||||
"**Bold** and *italic* with `code`\n## Header",
|
||||
{
|
||||
serverUrl: "http://localhost:1234",
|
||||
password: "test",
|
||||
},
|
||||
);
|
||||
|
||||
expect(result.messageId).toBe("msg-uuid-stripped");
|
||||
|
||||
const sendCall = mockFetch.mock.calls[1];
|
||||
const body = JSON.parse(sendCall[1].body);
|
||||
// Markdown should be stripped: no asterisks, backticks, or hashes
|
||||
expect(body.message).toBe("Bold and italic with code\nHeader");
|
||||
});
|
||||
|
||||
it("strips markdown when creating a new chat", async () => {
|
||||
mockFetch
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({ data: [] }),
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
text: () =>
|
||||
Promise.resolve(
|
||||
JSON.stringify({
|
||||
data: { guid: "new-msg-stripped" },
|
||||
}),
|
||||
),
|
||||
});
|
||||
|
||||
const result = await sendMessageBlueBubbles("+15550009999", "**Welcome** to the _chat_!", {
|
||||
serverUrl: "http://localhost:1234",
|
||||
password: "test",
|
||||
});
|
||||
|
||||
expect(result.messageId).toBe("new-msg-stripped");
|
||||
|
||||
const createCall = mockFetch.mock.calls[1];
|
||||
expect(createCall[0]).toContain("/api/v1/chat/new");
|
||||
const body = JSON.parse(createCall[1].body);
|
||||
// Markdown should be stripped
|
||||
expect(body.message).toBe("Welcome to the chat!");
|
||||
});
|
||||
|
||||
it("creates a new chat when handle target is missing", async () => {
|
||||
mockFetch
|
||||
.mockResolvedValueOnce({
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk";
|
||||
import crypto from "node:crypto";
|
||||
import { stripMarkdown } from "openclaw/plugin-sdk";
|
||||
import { resolveBlueBubblesAccount } from "./accounts.js";
|
||||
import {
|
||||
extractHandleFromChatGuid,
|
||||
@@ -332,6 +333,7 @@ async function createNewChatWithMessage(params: {
|
||||
const payload = {
|
||||
addresses: [params.address],
|
||||
message: params.message,
|
||||
tempGuid: `temp-${crypto.randomUUID()}`,
|
||||
};
|
||||
const res = await blueBubblesFetchWithTimeout(
|
||||
url,
|
||||
@@ -377,6 +379,11 @@ export async function sendMessageBlueBubbles(
|
||||
if (!trimmedText.trim()) {
|
||||
throw new Error("BlueBubbles send requires text");
|
||||
}
|
||||
// Strip markdown early and validate - ensures messages like "***" or "---" don't become empty
|
||||
const strippedText = stripMarkdown(trimmedText);
|
||||
if (!strippedText.trim()) {
|
||||
throw new Error("BlueBubbles send requires text (message was empty after markdown removal)");
|
||||
}
|
||||
|
||||
const account = resolveBlueBubblesAccount({
|
||||
cfg: opts.cfg ?? {},
|
||||
@@ -406,7 +413,7 @@ export async function sendMessageBlueBubbles(
|
||||
baseUrl,
|
||||
password,
|
||||
address: target.address,
|
||||
message: trimmedText,
|
||||
message: strippedText,
|
||||
timeoutMs: opts.timeoutMs,
|
||||
});
|
||||
}
|
||||
@@ -419,7 +426,7 @@ export async function sendMessageBlueBubbles(
|
||||
const payload: Record<string, unknown> = {
|
||||
chatGuid,
|
||||
tempGuid: crypto.randomUUID(),
|
||||
message: trimmedText,
|
||||
message: strippedText,
|
||||
};
|
||||
if (needsPrivateApi) {
|
||||
payload.method = "private-api";
|
||||
|
||||
Reference in New Issue
Block a user