From a7c0aa94d98654123a8bc3ee6de3de59fca1e59d Mon Sep 17 00:00:00 2001 From: Mariano <132747814+mbelinky@users.noreply.github.com> Date: Thu, 19 Feb 2026 09:59:21 +0000 Subject: [PATCH] refactor(security): share safe temp media path builder (#20810) Merged via /review-pr -> /prepare-pr -> /merge-pr. Prepared head SHA: 7a088e6801d4ec45858ba47d20a8c8615ba35389 Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky --- CHANGELOG.md | 1 + extensions/feishu/src/media.ts | 8 +++---- src/line/download.ts | 10 ++------ src/plugin-sdk/index.ts | 1 + src/plugin-sdk/temp-path.test.ts | 32 ++++++++++++++++++++++++++ src/plugin-sdk/temp-path.ts | 39 ++++++++++++++++++++++++++++++++ 6 files changed, 78 insertions(+), 13 deletions(-) create mode 100644 src/plugin-sdk/temp-path.test.ts create mode 100644 src/plugin-sdk/temp-path.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e34a9584e..da2ae55c91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Docs: https://docs.openclaw.ai - Security/Skills: for the next npm release, reject symlinks during skill packaging to prevent external file inclusion in distributed `.skill` archives. Thanks @aether-ai-agent for reporting. - Security/Feishu: prevent path traversal in Feishu inbound media temp-file writes by replacing key-derived temp filenames with UUID-based names. Thanks @allsmog for reporting. - LINE/Security: harden inbound media temp-file naming by using UUID-based temp paths for downloaded media instead of external message IDs. (#20792) Thanks @mbelinky. +- Security/Refactor: centralize hardened temp-file path generation for Feishu and LINE media downloads via shared `buildRandomTempFilePath` helper to reduce drift risk. (#20810) Thanks @mbelinky. - Security/Media: harden local media ingestion against TOCTOU/symlink swap attacks by pinning reads to a single file descriptor with symlink rejection and inode/device verification in `saveMediaSource`. Thanks @dorjoos for reporting. - Security/Lobster (Windows): for the next npm release, remove shell-based fallback when launching Lobster wrappers (`.cmd`/`.bat`) and switch to explicit argv execution with wrapper entrypoint resolution, preventing command injection while preserving Windows wrapper compatibility. Thanks @tdjackey for reporting. - Agents/Streaming: keep assistant partial streaming active during reasoning streams, handle native `thinking_*` stream events consistently, dedupe mixed reasoning-end signals, and clear stale mutating tool errors after same-target retry success. (#20635) Thanks @obviyus. diff --git a/extensions/feishu/src/media.ts b/extensions/feishu/src/media.ts index cfd37e816b..8f25282981 100644 --- a/extensions/feishu/src/media.ts +++ b/extensions/feishu/src/media.ts @@ -1,9 +1,7 @@ import fs from "fs"; -import crypto from "node:crypto"; -import os from "os"; import path from "path"; import { Readable } from "stream"; -import type { ClawdbotConfig } from "openclaw/plugin-sdk"; +import { buildRandomTempFilePath, type ClawdbotConfig } from "openclaw/plugin-sdk"; import { resolveFeishuAccount } from "./accounts.js"; import { createFeishuClient } from "./client.js"; import { getFeishuRuntime } from "./runtime.js"; @@ -100,7 +98,7 @@ export async function downloadImageFeishu(params: { path: { image_key: imageKey }, }); - const tmpPath = path.join(os.tmpdir(), `feishu_img_${Date.now()}_${crypto.randomUUID()}`); + const tmpPath = buildRandomTempFilePath({ prefix: "feishu_img" }); const buffer = await readFeishuResponseBuffer({ response, tmpPath, @@ -133,7 +131,7 @@ export async function downloadMessageResourceFeishu(params: { params: { type }, }); - const tmpPath = path.join(os.tmpdir(), `feishu_${Date.now()}_${crypto.randomUUID()}`); + const tmpPath = buildRandomTempFilePath({ prefix: "feishu" }); const buffer = await readFeishuResponseBuffer({ response, tmpPath, diff --git a/src/line/download.ts b/src/line/download.ts index f61e4f9367..ceb6916a52 100644 --- a/src/line/download.ts +++ b/src/line/download.ts @@ -1,9 +1,7 @@ -import crypto from "node:crypto"; import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; import { messagingApi } from "@line/bot-sdk"; import { logVerbose } from "../globals.js"; +import { buildRandomTempFilePath } from "../plugin-sdk/temp-path.js"; interface DownloadResult { path: string; @@ -11,10 +9,6 @@ interface DownloadResult { size: number; } -function buildLineTempMediaPath(extension: string): string { - return path.join(os.tmpdir(), `line-media-${Date.now()}-${crypto.randomUUID()}${extension}`); -} - export async function downloadLineMedia( messageId: string, channelAccessToken: string, @@ -45,7 +39,7 @@ export async function downloadLineMedia( const ext = getExtensionForContentType(contentType); // Use random temp names; never derive paths from external message identifiers. - const filePath = buildLineTempMediaPath(ext); + const filePath = buildRandomTempFilePath({ prefix: "line-media", extension: ext }); await fs.promises.writeFile(filePath, buffer); diff --git a/src/plugin-sdk/index.ts b/src/plugin-sdk/index.ts index b674d985bb..b1c2a92812 100644 --- a/src/plugin-sdk/index.ts +++ b/src/plugin-sdk/index.ts @@ -154,6 +154,7 @@ export { extractToolSend } from "./tool-send.js"; export { resolveChannelAccountConfigBasePath } from "./config-paths.js"; export { chunkTextForOutbound } from "./text-chunking.js"; export { readJsonFileWithFallback, writeJsonFileAtomically } from "./json-store.js"; +export { buildRandomTempFilePath } from "./temp-path.js"; export type { ChatType } from "../channels/chat-type.js"; /** @deprecated Use ChatType instead */ export type { RoutePeerKind } from "../routing/resolve-route.js"; diff --git a/src/plugin-sdk/temp-path.test.ts b/src/plugin-sdk/temp-path.test.ts new file mode 100644 index 0000000000..63f307f98b --- /dev/null +++ b/src/plugin-sdk/temp-path.test.ts @@ -0,0 +1,32 @@ +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { buildRandomTempFilePath } from "./temp-path.js"; + +describe("buildRandomTempFilePath", () => { + it("builds deterministic paths when now/uuid are provided", () => { + const result = buildRandomTempFilePath({ + prefix: "line-media", + extension: ".jpg", + tmpDir: "/tmp", + now: 123, + uuid: "abc", + }); + expect(result).toBe(path.join("/tmp", "line-media-123-abc.jpg")); + }); + + it("sanitizes prefix and extension to avoid path traversal segments", () => { + const result = buildRandomTempFilePath({ + prefix: "../../line/../media", + extension: "/../.jpg", + now: 123, + uuid: "abc", + }); + const tmpRoot = path.resolve(os.tmpdir()); + const resolved = path.resolve(result); + const rel = path.relative(tmpRoot, resolved); + expect(rel === ".." || rel.startsWith(`..${path.sep}`)).toBe(false); + expect(path.basename(result)).toBe("line-media-123-abc.jpg"); + expect(result).not.toContain(".."); + }); +}); diff --git a/src/plugin-sdk/temp-path.ts b/src/plugin-sdk/temp-path.ts new file mode 100644 index 0000000000..f140012884 --- /dev/null +++ b/src/plugin-sdk/temp-path.ts @@ -0,0 +1,39 @@ +import crypto from "node:crypto"; +import os from "node:os"; +import path from "node:path"; + +function sanitizePrefix(prefix: string): string { + const normalized = prefix.replace(/[^a-zA-Z0-9_-]+/g, "-").replace(/^-+|-+$/g, ""); + return normalized || "tmp"; +} + +function sanitizeExtension(extension?: string): string { + if (!extension) { + return ""; + } + const normalized = extension.startsWith(".") ? extension : `.${extension}`; + const suffix = normalized.match(/[a-zA-Z0-9._-]+$/)?.[0] ?? ""; + const token = suffix.replace(/^[._-]+/, ""); + if (!token) { + return ""; + } + return `.${token}`; +} + +export function buildRandomTempFilePath(params: { + prefix: string; + extension?: string; + tmpDir?: string; + now?: number; + uuid?: string; +}): string { + const prefix = sanitizePrefix(params.prefix); + const extension = sanitizeExtension(params.extension); + const nowCandidate = params.now; + const now = + typeof nowCandidate === "number" && Number.isFinite(nowCandidate) + ? Math.trunc(nowCandidate) + : Date.now(); + const uuid = params.uuid?.trim() || crypto.randomUUID(); + return path.join(params.tmpDir ?? os.tmpdir(), `${prefix}-${now}-${uuid}${extension}`); +}