fix(tools-manager): add download-stream byte cap and regression tests

LeonidasLux · LeonidasLux · commit 089b49e87f0a · 2026-07-02T16:27:26.000+08:00
Add a maxBytes parameter to downloadFile that checks Content-Length before reading the body and enforces a streaming byte cap during transfer, so oversized archives are rejected before hitting disk. Extract MAX_ARCHIVE_BYTES as a module-level constant shared between downloadFile and extractArchiveSafe, ensuring both gates use the same 100 MB limit. Add two regression tests: - rejects downloads with Content-Length exceeding the archive byte cap - accepts downloads with Content-Length under the archive byte cap Ref. openclaw#98988
diff --git a/src/agents/utils/tools-manager.test.ts b/src/agents/utils/tools-manager.test.ts
@@ -135,6 +135,70 @@ describe("ensureTool", () => {
       }),
     );
   });
+
+  it("rejects downloads with Content-Length exceeding archive byte cap", async () => {
+    vi.doMock("node:os", async (importOriginal) => ({
+      ...(await importOriginal<typeof import("node:os")>()),
+      arch: () => "x64",
+      platform: () => "linux",
+    }));
+
+    const { ensureTool } = await import("./tools-manager.js");
+    const releaseCheckRelease = vi.fn(async () => {});
+    const downloadRelease = vi.fn(async () => {});
+    fetchWithSsrFGuardMock
+      .mockResolvedValueOnce({
+        response: new Response(JSON.stringify({ tag_name: "14.1.1" }), { status: 200 }),
+        release: releaseCheckRelease,
+        finalUrl: "https://api.github.com/repos/BurntSushi/ripgrep/releases/latest",
+      })
+      .mockResolvedValueOnce({
+        response: new Response("oversized-body", {
+          status: 200,
+          headers: { "content-length": String(200 * 1024 * 1024) },
+        }),
+        release: downloadRelease,
+        finalUrl: "https://github.com/BurntSushi/ripgrep/releases/download/14.1.1/archive.tar.gz",
+      });
+
+    await expect(ensureTool("rg", true)).resolves.toBeUndefined();
+
+    expect(downloadRelease).toHaveBeenCalledOnce();
+  });
+
+  it("accepts downloads with Content-Length under the archive byte cap", async () => {
+    vi.doMock("node:os", async (importOriginal) => ({
+      ...(await importOriginal<typeof import("node:os")>()),
+      arch: () => "x64",
+      platform: () => "linux",
+    }));
+
+    const { ensureTool } = await import("./tools-manager.js");
+    const releaseCheckRelease = vi.fn(async () => {});
+    const downloadRelease = vi.fn(async () => {});
+    extractArchiveMock.mockRejectedValue(new Error("extraction error (expected)"));
+    fetchWithSsrFGuardMock
+      .mockResolvedValueOnce({
+        response: new Response(JSON.stringify({ tag_name: "14.1.1" }), { status: 200 }),
+        release: releaseCheckRelease,
+        finalUrl: "https://api.github.com/repos/BurntSushi/ripgrep/releases/latest",
+      })
+      .mockResolvedValueOnce({
+        response: new Response("small-body", {
+          status: 200,
+          headers: { "content-length": "10" },
+        }),
+        release: downloadRelease,
+        finalUrl: "https://github.com/BurntSushi/ripgrep/releases/download/14.1.1/archive.tar.gz",
+      });
+
+    await expect(ensureTool("rg", true)).resolves.toBeUndefined();
+
+    // Download proceeded past the Content-Length check, then extraction failed
+    // as expected (no real archive). The important thing is we didn't reject
+    // preemptively.
+    expect(downloadRelease).toHaveBeenCalledOnce();
+  });
 });
 
 describe("getToolPath exit-status handling", () => {
diff --git a/src/agents/utils/tools-manager.ts b/src/agents/utils/tools-manager.ts
@@ -16,7 +16,7 @@ import {
 } from "node:fs";
 import { arch, platform } from "node:os";
 import { join } from "node:path";
-import { Readable } from "node:stream";
+import { PassThrough, Readable } from "node:stream";
 import { pipeline } from "node:stream/promises";
 import type { ReadableStream as NodeReadableStream } from "node:stream/web";
 import chalk from "chalk";
@@ -27,6 +27,10 @@ import { APP_NAME, getBinDir } from "../config.js";
 const TOOLS_DIR = getBinDir();
 const NETWORK_TIMEOUT_MS = 10_000;
 const DOWNLOAD_TIMEOUT_MS = 120_000;
+// Maximum compressed archive size allowed for tool downloads (100 MB).
+// Matches the limit enforced during extraction so oversized archives are
+// rejected before hitting disk, not just when extractArchiveSafe runs.
+const MAX_ARCHIVE_BYTES = 100 * 1024 * 1024;
 
 async function cancelUnreadResponseBody(response: Response): Promise<void> {
   if (!response.bodyUsed) {
@@ -158,8 +162,12 @@ async function getLatestVersion(repo: string): Promise<string> {
   }
 }
 
-// Download a file from URL
-async function downloadFile(url: string, dest: string): Promise<void> {
+// Download a file from URL with an optional byte cap. When maxBytes is set the
+// Content-Length header is checked before the body is read and a streaming byte
+// counter aborts the pipeline mid-transfer if the cap is exceeded. Without this
+// check an oversized release asset would fully download to disk before the
+// compression-bomb guard in extractArchiveSafe has a chance to reject it.
+async function downloadFile(url: string, dest: string, maxBytes?: number): Promise<void> {
   const guarded = await fetchWithSsrFGuard({
     url,
     timeoutMs: DOWNLOAD_TIMEOUT_MS,
@@ -177,8 +185,46 @@ async function downloadFile(url: string, dest: string): Promise<void> {
       throw new Error("No response body");
     }
 
+    // Check Content-Length before reading the body so oversized responses are
+    // rejected without consuming any download bandwidth or disk space.
+    if (maxBytes !== undefined) {
+      const contentLength = response.headers.get("content-length");
+      if (contentLength !== null) {
+        const length = parseInt(contentLength, 10);
+        if (!isNaN(length) && length > maxBytes) {
+          await cancelUnreadResponseBody(response);
+          throw new Error(
+            `Download aborted: content-length ${length} exceeds ${maxBytes} byte limit`,
+          );
+        }
+      }
+    }
+
     const fileStream = createWriteStream(dest);
-    await pipeline(Readable.fromWeb(response.body as NodeReadableStream<Uint8Array>), fileStream);
+
+    if (maxBytes !== undefined) {
+      // Streaming byte counter — catches oversized responses even when the
+      // Content-Length header is missing or under-reported.
+      let downloaded = 0;
+      const byteCap = new PassThrough();
+      byteCap.on("data", (chunk: Buffer) => {
+        downloaded += chunk.length;
+        if (downloaded > maxBytes) {
+          byteCap.destroy(
+            new Error(
+              `Download aborted: exceeded ${maxBytes} byte limit after ${downloaded} bytes`,
+            ),
+          );
+        }
+      });
+      await pipeline(
+        Readable.fromWeb(response.body as NodeReadableStream<Uint8Array>),
+        byteCap,
+        fileStream,
+      );
+    } else {
+      await pipeline(Readable.fromWeb(response.body as NodeReadableStream<Uint8Array>), fileStream);
+    }
   } finally {
     await guarded.release();
   }
@@ -223,7 +269,7 @@ async function extractArchiveSafe(
       destDir: extractDir,
       timeoutMs: 60_000,
       limits: {
-        maxArchiveBytes: 100 * 1024 * 1024,
+        maxArchiveBytes: MAX_ARCHIVE_BYTES,
         maxExtractedBytes: 500 * 1024 * 1024,
         maxEntries: 1000,
       },
@@ -265,8 +311,9 @@ async function downloadTool(tool: "fd" | "rg"): Promise<string> {
   const binaryExt = plat === "win32" ? ".exe" : "";
   const binaryPath = join(TOOLS_DIR, config.binaryName + binaryExt);
 
-  // Download
-  await downloadFile(downloadUrl, archivePath);
+  // Download with byte cap so oversized archives are rejected before
+  // hitting disk, not just during extraction.
+  await downloadFile(downloadUrl, archivePath, MAX_ARCHIVE_BYTES);
 
   // Extract into a unique temp directory. fd and rg downloads can run concurrently
   // during startup, so sharing a fixed directory causes races.
