From bedcba2b9bde56541b8b6027e969bd26640e1355 Mon Sep 17 00:00:00 2001 From: Miorbnli Date: Sat, 11 Jul 2026 00:20:03 +0800 Subject: [PATCH] fix(signal): clean up signal-cli download temp dir on every exit path (#103339) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(signal): clean up signal-cli download temp dir on every exit path installSignalCliFromRelease created a temp dir with fs.mkdtemp for the download archive but never removed it. Every return path — including the success return and the extraction-failure / binary-not-found errors — leaked the directory plus the downloaded archive (signal-cli Linux-native archives are ~50-130 MB). Repeated installs or retries accumulated multi-hundred-MB temp dirs under the preferred tmp dir. Wrap the body after mkdtemp in try/finally that recursively removes the temp dir. The extracted binary is installed under CONFIG_DIR and is unaffected. Co-Authored-By: Claude * style(signal): drop unnecessary String() conversion in test spy dir is already a string (mkdtemp return); String(dir) triggers the no-unnecessary-type-conversion lint rule. Co-Authored-By: Claude * test(signal): cover success-path temp dir cleanup with real archive Add a success-path test that mocks the network fetch to return a real tar.gz archive (containing a signal-cli binary) so installSignalCliFromRelease runs the full download -> extract -> find-binary -> chmod path against real bytes, then asserts the temp dir and downloaded archive are removed by the finally clause. This complements the existing error-path test so both exit paths are covered. Co-Authored-By: Claude * fix(signal): use owned temp download workspace * fix(signal): allow bounded native extraction --------- Co-authored-by: Claude Co-authored-by: Peter Steinberger --- .../signal/src/install-signal-cli.test.ts | 185 ++++++++++++++++-- extensions/signal/src/install-signal-cli.ts | 82 +++++--- 2 files changed, 218 insertions(+), 49 deletions(-) diff --git a/extensions/signal/src/install-signal-cli.test.ts b/extensions/signal/src/install-signal-cli.test.ts index 679d641d567a..9650310624d8 100644 --- a/extensions/signal/src/install-signal-cli.test.ts +++ b/extensions/signal/src/install-signal-cli.test.ts @@ -2,18 +2,24 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { gzipSync } from "node:zlib"; import JSZip from "jszip"; import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime-env"; import * as tar from "tar"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { ReleaseAsset } from "./install-signal-cli.js"; -const { fetchWithSsrFGuardMock, resolveBrewExecutableMock, runPluginCommandWithTimeoutMock } = - vi.hoisted(() => ({ - fetchWithSsrFGuardMock: vi.fn(), - resolveBrewExecutableMock: vi.fn(), - runPluginCommandWithTimeoutMock: vi.fn(), - })); +const { + fetchWithSsrFGuardMock, + resolveBrewExecutableMock, + runPluginCommandWithTimeoutMock, + tempDownloadPaths, +} = vi.hoisted(() => ({ + fetchWithSsrFGuardMock: vi.fn(), + resolveBrewExecutableMock: vi.fn(), + runPluginCommandWithTimeoutMock: vi.fn(), + tempDownloadPaths: [] as string[], +})); vi.mock("openclaw/plugin-sdk/ssrf-runtime", () => ({ fetchWithSsrFGuard: fetchWithSsrFGuardMock, @@ -31,12 +37,28 @@ vi.mock("openclaw/plugin-sdk/run-command", () => ({ runPluginCommandWithTimeout: runPluginCommandWithTimeoutMock, })); +vi.mock("openclaw/plugin-sdk/temp-path", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + withTempDownloadPath: async ( + params: { prefix: string; fileName?: string; tmpDir?: string }, + run: (tmpPath: string) => Promise, + ) => + await actual.withTempDownloadPath(params, async (tmpPath) => { + tempDownloadPaths.push(tmpPath); + return await run(tmpPath); + }), + }; +}); + const { downloadToFile, extractSignalCliArchive, installSignalCli, installSignalCliFromRelease, looksLikeArchive, + MAX_SIGNAL_CLI_EXTRACTED_BYTES, pickAsset, } = await import("./install-signal-cli.js"); @@ -88,10 +110,24 @@ async function withTempFile(run: (filePath: string) => Promise) { } } +const originalPlatform = process.platform; +const originalArch = process.arch; + +function setProcessPlatform(platform: NodeJS.Platform, arch: string) { + Object.defineProperty(process, "platform", { configurable: true, value: platform }); + Object.defineProperty(process, "arch", { configurable: true, value: arch }); +} + beforeEach(() => { fetchWithSsrFGuardMock.mockReset(); resolveBrewExecutableMock.mockReset(); runPluginCommandWithTimeoutMock.mockReset(); + tempDownloadPaths.length = 0; +}); + +afterEach(() => { + Object.defineProperty(process, "platform", { configurable: true, value: originalPlatform }); + Object.defineProperty(process, "arch", { configurable: true, value: originalArch }); }); function requireAsset(asset: ReleaseAsset | undefined, label: string): ReleaseAsset { @@ -110,6 +146,15 @@ async function expectPathMissing(targetPath: string): Promise { } } +async function expectTempDownloadDirMissing(): Promise { + expect(tempDownloadPaths).toHaveLength(1); + const [tmpPath] = tempDownloadPaths; + if (!tmpPath) { + throw new Error("expected one captured temp download path"); + } + await expectPathMissing(path.dirname(tmpPath)); +} + describe("looksLikeArchive", () => { it("recognises .tar.gz", () => { expect(looksLikeArchive("foo.tar.gz")).toBe(true); @@ -372,22 +417,102 @@ describe("installSignalCliFromRelease", () => { }); expect(fetchResult.release).toHaveBeenCalledTimes(1); }); + + it("removes the download temp dir even when extraction fails", async () => { + setProcessPlatform("linux", "x64"); + fetchWithSsrFGuardMock.mockResolvedValueOnce( + okDownloadResponse( + JSON.stringify({ + tag_name: "v0.0.0-leak-test", + assets: [ + { + name: "signal-cli-0.0.0-Linux-native.tar.gz", + browser_download_url: "https://example.com/linux-native.tar.gz", + }, + ], + }), + { headers: { "content-type": "application/json" } }, + ), + ); + fetchWithSsrFGuardMock.mockResolvedValueOnce(okDownloadResponse("not-a-real-archive")); + + const result = await installSignalCliFromRelease({ log: vi.fn() } as unknown as RuntimeEnv); + + expect(result.ok).toBe(false); + await expectTempDownloadDirMissing(); + }); + + it("removes the download temp dir on the success path too", async () => { + setProcessPlatform("linux", "x64"); + const staging = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-signal-staging-")); + try { + const inner = path.join(staging, "signal-cli-0.0.0-success-test"); + await fs.mkdir(inner, { recursive: true }); + await fs.writeFile(path.join(inner, "signal-cli"), "#!/bin/sh\necho ok\n", "utf-8"); + const archivePath = path.join(staging, "signal-cli-0.0.0-Linux-native.tar.gz"); + await tar.c({ cwd: staging, file: archivePath, gzip: true }, [ + "signal-cli-0.0.0-success-test", + ]); + const archiveBytes = await fs.readFile(archivePath); + + fetchWithSsrFGuardMock.mockResolvedValueOnce( + okDownloadResponse( + JSON.stringify({ + tag_name: "v0.0.0-success-test", + assets: [ + { + name: "signal-cli-0.0.0-Linux-native.tar.gz", + browser_download_url: "https://example.com/linux-native.tar.gz", + }, + ], + }), + { headers: { "content-type": "application/json" } }, + ), + ); + fetchWithSsrFGuardMock.mockResolvedValueOnce(okDownloadResponse(archiveBytes)); + + const result = await installSignalCliFromRelease({ log: vi.fn() } as unknown as RuntimeEnv); + + expect(result.ok).toBe(true); + if (!result.cliPath) { + throw new Error("expected the installed signal-cli path"); + } + const installedStat = await fs.stat(result.cliPath); + expect(installedStat.isFile()).toBe(true); + expect(installedStat.mode & 0o111).not.toBe(0); + await expectTempDownloadDirMissing(); + } finally { + await fs.rm(staging, { recursive: true, force: true }).catch(() => undefined); + } + }); + + it("removes the download temp dir when the download throws", async () => { + setProcessPlatform("linux", "x64"); + fetchWithSsrFGuardMock.mockResolvedValueOnce( + okDownloadResponse( + JSON.stringify({ + tag_name: "v0.0.0-download-failure-test", + assets: [ + { + name: "signal-cli-0.0.0-Linux-native.tar.gz", + browser_download_url: "https://example.com/linux-native.tar.gz", + }, + ], + }), + { headers: { "content-type": "application/json" } }, + ), + ); + fetchWithSsrFGuardMock.mockRejectedValueOnce(new Error("download failed")); + + await expect( + installSignalCliFromRelease({ log: vi.fn() } as unknown as RuntimeEnv), + ).rejects.toThrow("download failed"); + + await expectTempDownloadDirMissing(); + }); }); describe("installSignalCli", () => { - const originalPlatform = process.platform; - const originalArch = process.arch; - - function setProcessPlatform(platform: NodeJS.Platform, arch: string) { - Object.defineProperty(process, "platform", { configurable: true, value: platform }); - Object.defineProperty(process, "arch", { configurable: true, value: arch }); - } - - afterEach(() => { - Object.defineProperty(process, "platform", { configurable: true, value: originalPlatform }); - Object.defineProperty(process, "arch", { configurable: true, value: originalArch }); - }); - it("uses Homebrew on macOS instead of downloading the first GitHub release archive", async () => { setProcessPlatform("darwin", "arm64"); const brewPrefix = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-signal-brew-")); @@ -474,4 +599,26 @@ describe("extractSignalCliArchive", () => { await expectExtractedSignalCli(archivePath, extractDir); }); }); + + it("rejects native entries beyond the Signal-specific extraction limit", async () => { + await withArchiveWorkspace(async (workDir) => { + const archivePath = path.join(workDir, "oversized.tgz"); + const extractDir = path.join(workDir, "extract"); + const headerBlock = Buffer.alloc(512); + const header = new tar.Header({ + path: "signal-cli", + type: "File", + mode: 0o755, + size: MAX_SIGNAL_CLI_EXTRACTED_BYTES + 1, + }); + header.encode(headerBlock); + await fs.writeFile(archivePath, gzipSync(Buffer.concat([headerBlock, Buffer.alloc(1024)]))); + await fs.mkdir(extractDir, { recursive: true }); + + await expect(extractSignalCliArchive(archivePath, extractDir, 5_000)).rejects.toThrow( + "archive entry extracted size exceeds limit", + ); + await expectPathMissing(path.join(extractDir, "signal-cli")); + }); + }); }); diff --git a/extensions/signal/src/install-signal-cli.ts b/extensions/signal/src/install-signal-cli.ts index 1cb2c55f8bf5..7722cf2521eb 100644 --- a/extensions/signal/src/install-signal-cli.ts +++ b/extensions/signal/src/install-signal-cli.ts @@ -11,7 +11,7 @@ import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime-env"; import { CONFIG_DIR, extractArchive, resolveBrewExecutable } from "openclaw/plugin-sdk/setup-tools"; import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime"; import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; -import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path"; +import { withTempDownloadPath } from "openclaw/plugin-sdk/temp-path"; import { truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime"; export type ReleaseAsset = { @@ -30,6 +30,8 @@ type ReleaseResponse = { }; const MAX_SIGNAL_CLI_ARCHIVE_BYTES = 256 * 1024 * 1024; +/** @internal Exported for testing. */ +export const MAX_SIGNAL_CLI_EXTRACTED_BYTES = 384 * 1024 * 1024; const SIGNAL_CLI_DOWNLOAD_TIMEOUT_MS = 5 * 60_000; const SIGNAL_CLI_RELEASE_INFO_TIMEOUT_MS = 30_000; const CONTENT_LENGTH_RE = /^\d+$/; @@ -47,7 +49,19 @@ export async function extractSignalCliArchive( installRoot: string, timeoutMs: number, ): Promise { - await extractArchive({ archivePath, destDir: installRoot, timeoutMs }); + // v0.14.5 is a 105,553,779-byte archive containing one 354,813,880-byte + // native binary. Keep 13% extraction headroom without relaxing global limits. + await extractArchive({ + archivePath, + destDir: installRoot, + timeoutMs, + limits: { + maxArchiveBytes: MAX_SIGNAL_CLI_ARCHIVE_BYTES, + maxEntries: 32, + maxEntryBytes: MAX_SIGNAL_CLI_EXTRACTED_BYTES, + maxExtractedBytes: MAX_SIGNAL_CLI_EXTRACTED_BYTES, + }, + }); } /** @internal Exported for testing. */ @@ -326,39 +340,47 @@ export async function installSignalCliFromRelease( }; } - const tmpDir = await fs.mkdtemp(path.join(resolvePreferredOpenClawTmpDir(), "openclaw-signal-")); - const archivePath = path.join(tmpDir, asset.name); + // Keep the large release archive in an owned workspace so every callback exit + // cleans it without touching the installed tree under CONFIG_DIR. + return await withTempDownloadPath( + { prefix: "openclaw-signal", fileName: asset.name }, + async (archivePath) => { + runtime.log(`Downloading signal-cli ${version} (${asset.name})…`); + await downloadToFile(asset.browser_download_url, archivePath); - runtime.log(`Downloading signal-cli ${version} (${asset.name})…`); - await downloadToFile(asset.browser_download_url, archivePath); + const installRoot = path.join(CONFIG_DIR, "tools", "signal-cli", version); + await fs.mkdir(installRoot, { recursive: true }); - const installRoot = path.join(CONFIG_DIR, "tools", "signal-cli", version); - await fs.mkdir(installRoot, { recursive: true }); + if (!looksLikeArchive(normalizeLowercaseStringOrEmpty(asset.name))) { + return { ok: false, error: `Unsupported archive type: ${asset.name}` }; + } + try { + await extractSignalCliArchive(archivePath, installRoot, 60_000); + } catch (err) { + const message = formatErrorMessage(err); + return { + ok: false, + error: `Failed to extract ${asset.name}: ${message}`, + }; + } - if (!looksLikeArchive(normalizeLowercaseStringOrEmpty(asset.name))) { - return { ok: false, error: `Unsupported archive type: ${asset.name}` }; - } - try { - await extractSignalCliArchive(archivePath, installRoot, 60_000); - } catch (err) { - const message = formatErrorMessage(err); - return { - ok: false, - error: `Failed to extract ${asset.name}: ${message}`, - }; - } + const cliPath = await findSignalCliBinary(installRoot); + if (!cliPath) { + return { + ok: false, + error: `signal-cli binary not found after extracting ${asset.name}`, + }; + } - const cliPath = await findSignalCliBinary(installRoot); - if (!cliPath) { - return { - ok: false, - error: `signal-cli binary not found after extracting ${asset.name}`, - }; - } + await fs.chmod(cliPath, 0o755).catch(() => {}); - await fs.chmod(cliPath, 0o755).catch(() => {}); - - return { ok: true, cliPath, version }; + return { + ok: true, + cliPath, + version, + }; + }, + ); } // ---------------------------------------------------------------------------