From 71f357d9498cebb0efe016b0496d5fbe807539fc Mon Sep 17 00:00:00 2001 From: Mariano <132747814+mbelinky@users.noreply.github.com> Date: Sat, 14 Feb 2026 17:43:44 +0000 Subject: [PATCH] bluebubbles: harden local media path handling against LFI (#16322) * bluebubbles: harden local media path handling * bluebubbles: remove racy post-open symlink lstat * fix: bluebubbles mediaLocalRoots docs + typing fix (#16322) (thanks @mbelinky) --- CHANGELOG.md | 1 + docs/channels/bluebubbles.md | 1 + extensions/bluebubbles/src/config-schema.ts | 1 + extensions/bluebubbles/src/media-send.test.ts | 256 ++++++++++++++++++ extensions/bluebubbles/src/media-send.ts | 157 ++++++++++- extensions/bluebubbles/src/types.ts | 8 +- src/config/zod-schema.providers-core.ts | 1 + 7 files changed, 417 insertions(+), 8 deletions(-) create mode 100644 extensions/bluebubbles/src/media-send.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 61f3a66ae0..bd5a0d856b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Docs: https://docs.openclaw.ai - Security/Gateway: stop returning raw resolved config values in `skills.status` requirement checks (prevents operator.read clients from reading secrets). Thanks @simecek. - Security/Zalo: reject ambiguous shared-path webhook routing when multiple webhook targets match the same secret. - Security/BlueBubbles: reject ambiguous shared-path webhook routing when multiple webhook targets match the same guid/password. +- Security/BlueBubbles: require explicit `mediaLocalRoots` allowlists for local outbound media path reads to prevent local file disclosure. (#16322) Thanks @mbelinky. - Cron/Slack: preserve agent identity (name and icon) when cron jobs deliver outbound messages. (#16242) Thanks @robbyczgw-cla. - Discord: prefer gateway guild id when logging inbound messages so cached-miss guilds do not appear as `guild=dm`. Thanks @thewilloftheshadow. diff --git a/docs/channels/bluebubbles.md b/docs/channels/bluebubbles.md index ab852e9821..a63d2f1d35 100644 --- a/docs/channels/bluebubbles.md +++ b/docs/channels/bluebubbles.md @@ -300,6 +300,7 @@ Provider options: - `channels.bluebubbles.textChunkLimit`: Outbound chunk size in chars (default: 4000). - `channels.bluebubbles.chunkMode`: `length` (default) splits only when exceeding `textChunkLimit`; `newline` splits on blank lines (paragraph boundaries) before length chunking. - `channels.bluebubbles.mediaMaxMb`: Inbound media cap in MB (default: 8). +- `channels.bluebubbles.mediaLocalRoots`: Explicit allowlist of absolute local directories permitted for outbound local media paths. Local path sends are denied by default unless this is configured. Per-account override: `channels.bluebubbles.accounts..mediaLocalRoots`. - `channels.bluebubbles.historyLimit`: Max group messages for context (0 disables). - `channels.bluebubbles.dmHistoryLimit`: DM history limit. - `channels.bluebubbles.actions`: Enable/disable specific actions. diff --git a/extensions/bluebubbles/src/config-schema.ts b/extensions/bluebubbles/src/config-schema.ts index 3a5e1b393b..097071757c 100644 --- a/extensions/bluebubbles/src/config-schema.ts +++ b/extensions/bluebubbles/src/config-schema.ts @@ -40,6 +40,7 @@ const bluebubblesAccountSchema = z.object({ textChunkLimit: z.number().int().positive().optional(), chunkMode: z.enum(["length", "newline"]).optional(), mediaMaxMb: z.number().int().positive().optional(), + mediaLocalRoots: z.array(z.string()).optional(), sendReadReceipts: z.boolean().optional(), blockStreaming: z.boolean().optional(), groups: z.object({}).catchall(bluebubblesGroupConfigSchema).optional(), diff --git a/extensions/bluebubbles/src/media-send.test.ts b/extensions/bluebubbles/src/media-send.test.ts new file mode 100644 index 0000000000..c5c64d8a27 --- /dev/null +++ b/extensions/bluebubbles/src/media-send.test.ts @@ -0,0 +1,256 @@ +import type { OpenClawConfig, PluginRuntime } from "openclaw/plugin-sdk"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { sendBlueBubblesMedia } from "./media-send.js"; +import { setBlueBubblesRuntime } from "./runtime.js"; + +const sendBlueBubblesAttachmentMock = vi.hoisted(() => vi.fn()); +const sendMessageBlueBubblesMock = vi.hoisted(() => vi.fn()); +const resolveBlueBubblesMessageIdMock = vi.hoisted(() => vi.fn((id: string) => id)); + +vi.mock("./attachments.js", () => ({ + sendBlueBubblesAttachment: sendBlueBubblesAttachmentMock, +})); + +vi.mock("./send.js", () => ({ + sendMessageBlueBubbles: sendMessageBlueBubblesMock, +})); + +vi.mock("./monitor.js", () => ({ + resolveBlueBubblesMessageId: resolveBlueBubblesMessageIdMock, +})); + +type RuntimeMocks = { + detectMime: ReturnType; + fetchRemoteMedia: ReturnType; +}; + +let runtimeMocks: RuntimeMocks; +const tempDirs: string[] = []; + +function createMockRuntime(): { runtime: PluginRuntime; mocks: RuntimeMocks } { + const detectMime = vi.fn().mockResolvedValue("text/plain"); + const fetchRemoteMedia = vi.fn().mockResolvedValue({ + buffer: new Uint8Array([1, 2, 3]), + contentType: "image/png", + fileName: "remote.png", + }); + return { + runtime: { + version: "1.0.0", + media: { + detectMime, + }, + channel: { + media: { + fetchRemoteMedia, + }, + }, + } as unknown as PluginRuntime, + mocks: { detectMime, fetchRemoteMedia }, + }; +} + +function createConfig(overrides?: Record): OpenClawConfig { + return { + channels: { + bluebubbles: { + ...overrides, + }, + }, + } as unknown as OpenClawConfig; +} + +async function makeTempDir(): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-bb-media-")); + tempDirs.push(dir); + return dir; +} + +beforeEach(() => { + const runtime = createMockRuntime(); + runtimeMocks = runtime.mocks; + setBlueBubblesRuntime(runtime.runtime); + sendBlueBubblesAttachmentMock.mockReset(); + sendBlueBubblesAttachmentMock.mockResolvedValue({ messageId: "msg-1" }); + sendMessageBlueBubblesMock.mockReset(); + sendMessageBlueBubblesMock.mockResolvedValue({ messageId: "msg-caption" }); + resolveBlueBubblesMessageIdMock.mockClear(); +}); + +afterEach(async () => { + while (tempDirs.length > 0) { + const dir = tempDirs.pop(); + if (!dir) { + continue; + } + await fs.rm(dir, { recursive: true, force: true }); + } +}); + +describe("sendBlueBubblesMedia local-path hardening", () => { + it("rejects local paths when mediaLocalRoots is not configured", async () => { + await expect( + sendBlueBubblesMedia({ + cfg: createConfig(), + to: "chat:123", + mediaPath: "/etc/passwd", + }), + ).rejects.toThrow(/mediaLocalRoots/i); + + expect(sendBlueBubblesAttachmentMock).not.toHaveBeenCalled(); + }); + + it("rejects local paths outside configured mediaLocalRoots", async () => { + const allowedRoot = await makeTempDir(); + const outsideDir = await makeTempDir(); + const outsideFile = path.join(outsideDir, "outside.txt"); + await fs.writeFile(outsideFile, "not allowed", "utf8"); + + await expect( + sendBlueBubblesMedia({ + cfg: createConfig({ mediaLocalRoots: [allowedRoot] }), + to: "chat:123", + mediaPath: outsideFile, + }), + ).rejects.toThrow(/not under any configured mediaLocalRoots/i); + + expect(sendBlueBubblesAttachmentMock).not.toHaveBeenCalled(); + }); + + it("allows local paths that are explicitly configured", async () => { + const allowedRoot = await makeTempDir(); + const allowedFile = path.join(allowedRoot, "allowed.txt"); + await fs.writeFile(allowedFile, "allowed", "utf8"); + + const result = await sendBlueBubblesMedia({ + cfg: createConfig({ mediaLocalRoots: [allowedRoot] }), + to: "chat:123", + mediaPath: allowedFile, + }); + + expect(result).toEqual({ messageId: "msg-1" }); + expect(sendBlueBubblesAttachmentMock).toHaveBeenCalledTimes(1); + expect(sendBlueBubblesAttachmentMock.mock.calls[0]?.[0]).toEqual( + expect.objectContaining({ + filename: "allowed.txt", + contentType: "text/plain", + }), + ); + expect(runtimeMocks.detectMime).toHaveBeenCalled(); + }); + + it("allows file:// media paths and file:// local roots", async () => { + const allowedRoot = await makeTempDir(); + const allowedFile = path.join(allowedRoot, "allowed.txt"); + await fs.writeFile(allowedFile, "allowed", "utf8"); + + const result = await sendBlueBubblesMedia({ + cfg: createConfig({ mediaLocalRoots: [pathToFileURL(allowedRoot).toString()] }), + to: "chat:123", + mediaPath: pathToFileURL(allowedFile).toString(), + }); + + expect(result).toEqual({ messageId: "msg-1" }); + expect(sendBlueBubblesAttachmentMock).toHaveBeenCalledTimes(1); + expect(sendBlueBubblesAttachmentMock.mock.calls[0]?.[0]).toEqual( + expect.objectContaining({ + filename: "allowed.txt", + }), + ); + }); + + it("uses account-specific mediaLocalRoots over top-level roots", async () => { + const baseRoot = await makeTempDir(); + const accountRoot = await makeTempDir(); + const baseFile = path.join(baseRoot, "base.txt"); + const accountFile = path.join(accountRoot, "account.txt"); + await fs.writeFile(baseFile, "base", "utf8"); + await fs.writeFile(accountFile, "account", "utf8"); + + const cfg = createConfig({ + mediaLocalRoots: [baseRoot], + accounts: { + work: { + mediaLocalRoots: [accountRoot], + }, + }, + }); + + await expect( + sendBlueBubblesMedia({ + cfg, + to: "chat:123", + accountId: "work", + mediaPath: baseFile, + }), + ).rejects.toThrow(/not under any configured mediaLocalRoots/i); + + const result = await sendBlueBubblesMedia({ + cfg, + to: "chat:123", + accountId: "work", + mediaPath: accountFile, + }); + + expect(result).toEqual({ messageId: "msg-1" }); + }); + + it("rejects symlink escapes under an allowed root", async () => { + const allowedRoot = await makeTempDir(); + const outsideDir = await makeTempDir(); + const outsideFile = path.join(outsideDir, "secret.txt"); + const linkPath = path.join(allowedRoot, "link.txt"); + await fs.writeFile(outsideFile, "secret", "utf8"); + + try { + await fs.symlink(outsideFile, linkPath); + } catch { + // Some environments disallow symlink creation; skip without failing the suite. + return; + } + + await expect( + sendBlueBubblesMedia({ + cfg: createConfig({ mediaLocalRoots: [allowedRoot] }), + to: "chat:123", + mediaPath: linkPath, + }), + ).rejects.toThrow(/not under any configured mediaLocalRoots/i); + + expect(sendBlueBubblesAttachmentMock).not.toHaveBeenCalled(); + }); + + it("rejects relative mediaLocalRoots entries", async () => { + const allowedRoot = await makeTempDir(); + const allowedFile = path.join(allowedRoot, "allowed.txt"); + const relativeRoot = path.relative(process.cwd(), allowedRoot); + await fs.writeFile(allowedFile, "allowed", "utf8"); + + await expect( + sendBlueBubblesMedia({ + cfg: createConfig({ mediaLocalRoots: [relativeRoot] }), + to: "chat:123", + mediaPath: allowedFile, + }), + ).rejects.toThrow(/must be absolute paths/i); + + expect(sendBlueBubblesAttachmentMock).not.toHaveBeenCalled(); + }); + + it("keeps remote URL flow unchanged", async () => { + await sendBlueBubblesMedia({ + cfg: createConfig(), + to: "chat:123", + mediaUrl: "https://example.com/file.png", + }); + + expect(runtimeMocks.fetchRemoteMedia).toHaveBeenCalledWith( + expect.objectContaining({ url: "https://example.com/file.png" }), + ); + expect(sendBlueBubblesAttachmentMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/extensions/bluebubbles/src/media-send.ts b/extensions/bluebubbles/src/media-send.ts index ab75721056..797b2b92fa 100644 --- a/extensions/bluebubbles/src/media-send.ts +++ b/extensions/bluebubbles/src/media-send.ts @@ -1,6 +1,10 @@ +import { constants as fsConstants } from "node:fs"; +import fs from "node:fs/promises"; +import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { resolveChannelMediaMaxBytes, type OpenClawConfig } from "openclaw/plugin-sdk"; +import { resolveBlueBubblesAccount } from "./accounts.js"; import { sendBlueBubblesAttachment } from "./attachments.js"; import { resolveBlueBubblesMessageId } from "./monitor.js"; import { getBlueBubblesRuntime } from "./runtime.js"; @@ -32,6 +36,141 @@ function resolveLocalMediaPath(source: string): string { } } +function expandHomePath(input: string): string { + if (input === "~") { + return os.homedir(); + } + if (input.startsWith("~/") || input.startsWith(`~${path.sep}`)) { + return path.join(os.homedir(), input.slice(2)); + } + return input; +} + +function resolveConfiguredPath(input: string): string { + const trimmed = input.trim(); + if (!trimmed) { + throw new Error("Empty mediaLocalRoots entry is not allowed"); + } + if (trimmed.startsWith("file://")) { + let parsed: string; + try { + parsed = fileURLToPath(trimmed); + } catch { + throw new Error(`Invalid file:// URL in mediaLocalRoots: ${input}`); + } + if (!path.isAbsolute(parsed)) { + throw new Error(`mediaLocalRoots entries must be absolute paths: ${input}`); + } + return parsed; + } + const resolved = expandHomePath(trimmed); + if (!path.isAbsolute(resolved)) { + throw new Error(`mediaLocalRoots entries must be absolute paths: ${input}`); + } + return resolved; +} + +function isPathInsideRoot(candidate: string, root: string): boolean { + const normalizedCandidate = path.normalize(candidate); + const normalizedRoot = path.normalize(root); + const rootWithSep = normalizedRoot.endsWith(path.sep) + ? normalizedRoot + : normalizedRoot + path.sep; + if (process.platform === "win32") { + const candidateLower = normalizedCandidate.toLowerCase(); + const rootLower = normalizedRoot.toLowerCase(); + const rootWithSepLower = rootWithSep.toLowerCase(); + return candidateLower === rootLower || candidateLower.startsWith(rootWithSepLower); + } + return normalizedCandidate === normalizedRoot || normalizedCandidate.startsWith(rootWithSep); +} + +function resolveMediaLocalRoots(params: { cfg: OpenClawConfig; accountId?: string }): string[] { + const account = resolveBlueBubblesAccount({ + cfg: params.cfg, + accountId: params.accountId, + }); + return (account.config.mediaLocalRoots ?? []) + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0); +} + +async function assertLocalMediaPathAllowed(params: { + localPath: string; + localRoots: string[]; + accountId?: string; +}): Promise<{ data: Buffer; realPath: string; sizeBytes: number }> { + if (params.localRoots.length === 0) { + throw new Error( + `Local BlueBubbles media paths are disabled by default. Set channels.bluebubbles.mediaLocalRoots${ + params.accountId + ? ` or channels.bluebubbles.accounts.${params.accountId}.mediaLocalRoots` + : "" + } to explicitly allow local file directories.`, + ); + } + + const resolvedLocalPath = path.resolve(params.localPath); + const supportsNoFollow = process.platform !== "win32" && "O_NOFOLLOW" in fsConstants; + const openFlags = fsConstants.O_RDONLY | (supportsNoFollow ? fsConstants.O_NOFOLLOW : 0); + + for (const rootEntry of params.localRoots) { + const resolvedRootInput = resolveConfiguredPath(rootEntry); + const relativeToRoot = path.relative(resolvedRootInput, resolvedLocalPath); + if ( + relativeToRoot.startsWith("..") || + path.isAbsolute(relativeToRoot) || + relativeToRoot === "" + ) { + continue; + } + + let rootReal: string; + try { + rootReal = await fs.realpath(resolvedRootInput); + } catch { + rootReal = path.resolve(resolvedRootInput); + } + const candidatePath = path.resolve(rootReal, relativeToRoot); + + if (!isPathInsideRoot(candidatePath, rootReal)) { + continue; + } + + let handle: Awaited> | null = null; + try { + handle = await fs.open(candidatePath, openFlags); + const realPath = await fs.realpath(candidatePath); + if (!isPathInsideRoot(realPath, rootReal)) { + continue; + } + + const stat = await handle.stat(); + if (!stat.isFile()) { + continue; + } + const realStat = await fs.stat(realPath); + if (stat.ino !== realStat.ino || stat.dev !== realStat.dev) { + continue; + } + + const data = await handle.readFile(); + return { data, realPath, sizeBytes: stat.size }; + } catch { + // Try next configured root. + continue; + } finally { + if (handle) { + await handle.close().catch(() => {}); + } + } + } + + throw new Error( + `Local media path is not under any configured mediaLocalRoots entry: ${params.localPath}`, + ); +} + function resolveFilenameFromSource(source?: string): string | undefined { if (!source) { return undefined; @@ -88,6 +227,7 @@ export async function sendBlueBubblesMedia(params: { cfg.channels?.bluebubbles?.mediaMaxMb, accountId, }); + const mediaLocalRoots = resolveMediaLocalRoots({ cfg, accountId }); let buffer: Uint8Array; let resolvedContentType = contentType ?? undefined; @@ -121,24 +261,27 @@ export async function sendBlueBubblesMedia(params: { resolvedContentType = resolvedContentType ?? fetched.contentType ?? undefined; resolvedFilename = resolvedFilename ?? fetched.fileName; } else { - const localPath = resolveLocalMediaPath(source); - const fs = await import("node:fs/promises"); + const localPath = expandHomePath(resolveLocalMediaPath(source)); + const localFile = await assertLocalMediaPathAllowed({ + localPath, + localRoots: mediaLocalRoots, + accountId, + }); if (typeof maxBytes === "number" && maxBytes > 0) { - const stats = await fs.stat(localPath); - assertMediaWithinLimit(stats.size, maxBytes); + assertMediaWithinLimit(localFile.sizeBytes, maxBytes); } - const data = await fs.readFile(localPath); + const data = localFile.data; assertMediaWithinLimit(data.byteLength, maxBytes); buffer = new Uint8Array(data); if (!resolvedContentType) { const detected = await core.media.detectMime({ buffer: data, - filePath: localPath, + filePath: localFile.realPath, }); resolvedContentType = detected ?? undefined; } if (!resolvedFilename) { - resolvedFilename = resolveFilenameFromSource(localPath); + resolvedFilename = resolveFilenameFromSource(localFile.realPath); } } } diff --git a/extensions/bluebubbles/src/types.ts b/extensions/bluebubbles/src/types.ts index 24c82109cd..7346c4ff42 100644 --- a/extensions/bluebubbles/src/types.ts +++ b/extensions/bluebubbles/src/types.ts @@ -1,5 +1,6 @@ import type { DmPolicy, GroupPolicy } from "openclaw/plugin-sdk"; -export type { DmPolicy, GroupPolicy }; + +export type { DmPolicy, GroupPolicy } from "openclaw/plugin-sdk"; export type BlueBubblesGroupConfig = { /** If true, only respond in this group when mentioned. */ @@ -45,6 +46,11 @@ export type BlueBubblesAccountConfig = { blockStreamingCoalesce?: Record; /** Max outbound media size in MB. */ mediaMaxMb?: number; + /** + * Explicit allowlist of local directory roots permitted for outbound media paths. + * Local paths are rejected unless they resolve under one of these roots. + */ + mediaLocalRoots?: string[]; /** Send read receipts for incoming messages (default: true). */ sendReadReceipts?: boolean; /** Per-group configuration keyed by chat GUID or identifier. */ diff --git a/src/config/zod-schema.providers-core.ts b/src/config/zod-schema.providers-core.ts index ab6d198af9..048c8bf666 100644 --- a/src/config/zod-schema.providers-core.ts +++ b/src/config/zod-schema.providers-core.ts @@ -874,6 +874,7 @@ export const BlueBubblesAccountSchemaBase = z textChunkLimit: z.number().int().positive().optional(), chunkMode: z.enum(["length", "newline"]).optional(), mediaMaxMb: z.number().int().positive().optional(), + mediaLocalRoots: z.array(z.string()).optional(), sendReadReceipts: z.boolean().optional(), blockStreaming: z.boolean().optional(), blockStreamingCoalesce: BlockStreamingCoalesceSchema.optional(),