fix(signal): clean up signal-cli download temp dir on every exit path (#103339)

* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * fix(signal): use owned temp download workspace * fix(signal): allow bounded native extraction --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
2026-07-17 19:21:35 +00:00 · 2026-07-11 00:20:03 +08:00
parent cbfa7d4aa7
commit bedcba2b9b
2 changed files with 218 additions and 49 deletions
--- 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<typeof import("openclaw/plugin-sdk/temp-path")>();
+  return {
+    ...actual,
+    withTempDownloadPath: async (
+      params: { prefix: string; fileName?: string; tmpDir?: string },
+      run: (tmpPath: string) => Promise<unknown>,
+    ) =>
+      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<void>) {
  }
 }

+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<void> {
  }
 }

+async function expectTempDownloadDirMissing(): Promise<void> {
+  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"));
+    });
+  });
 });
--- 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<void> {
-  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,
+      };
+    },
+  );
 }

 // ---------------------------------------------------------------------------