fix(file-transfer): recheck dir fetch archive policy after fetch

clawsweeper-repair · clawsweeper-repair · commit b712aa4d5af4 · 2026-04-30T03:25:06.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -43,7 +43,7 @@ Docs: https://docs.openclaw.ai
 - Plugins/runtime-deps: verify staged package entry files before reusing mirrored runtime roots, so browser-control repairs incomplete `ajv`/MCP SDK installs after update instead of failing after restart on a missing `ajv/dist/ajv.js`. Refs #74630. Thanks @spickeringlr.
 - Heartbeat: resolve `responsePrefix` template variables with the selected provider, model, and thinking context before delivering alerts or suppressing prefixed `HEARTBEAT_OK` replies. Fixes #43064; repairs #43065; supersedes #46858. Thanks @yweiii and @JunJD.
 - Memory/LanceDB: show full memory UUIDs in the `memory_forget` candidate list so agents can pass the displayed ID back to targeted deletion without hitting the full-UUID validator. (#66913) Thanks @amittell.
-- File-transfer plugin: require canonical read-path preflight authorization for `file.fetch` and fail closed when `dir.fetch` preflight entries are missing, absolute, or traversing before archive bytes are requested from the node. Carries forward #74134. Thanks @omarshahine.
+- File-transfer plugin: require canonical read-path preflight authorization for `file.fetch`, fail closed when `dir.fetch` preflight entries are missing, absolute, or traversing, and recheck returned archive entries before handing archive bytes to callers. Carries forward #74134. Thanks @omarshahine.
 - Channels/Feishu: retry file-typed iOS video resource downloads as `media` after a Feishu/Lark HTTP 502 and preserve the original 502 when the fallback also fails. Fixes #49855; carries forward #50164 and #73986. Thanks @alex-xuweilong.
 - Providers/Amazon Bedrock: expose the full Claude Opus 4.7 thinking profile (`xhigh`, `adaptive`, and `max`) for Bedrock model refs, while keeping Opus/Sonnet 4.6 on adaptive-by-default, so `/think` menus and validation match the Anthropic transport behavior. Fixes #74701. Thanks @prasad-yashdeep, @sparkleHazard, @Sanjays2402, and @hclsys.
 - Plugins/tokenjuice: compile the bundled plugin against tokenjuice 0.7.0's published OpenClaw host types instead of a local compatibility shim, so package contract drift fails in OpenClaw validation before release. Thanks @vincentkoc.
diff --git a/extensions/file-transfer/src/node-host/dir-fetch.test.ts b/extensions/file-transfer/src/node-host/dir-fetch.test.ts
@@ -110,6 +110,8 @@ describe("handleDirFetch — happy path", () => {
     // file count covers the regular files we created (3); BSD tar may also
     // list directory entries, so be generous.
     expect(r.fileCount).toBeGreaterThanOrEqual(3);
+    expect(r.entries).toEqual(expect.arrayContaining(["a.txt", "b.txt", "sub", "sub/c.txt"]));
+    expect(r.fileCount).toBe(r.entries?.length);
   });
 });
 
diff --git a/extensions/file-transfer/src/node-host/dir-fetch.ts b/extensions/file-transfer/src/node-host/dir-fetch.ts
@@ -88,10 +88,10 @@ async function preflightDu(dirPath: string, maxBytes: number): Promise<boolean>
   });
 }
 
-async function countTarEntries(tarBuffer: Buffer): Promise<number> {
+async function listTarEntries(tarBuffer: Buffer): Promise<string[]> {
   // Async spawn so a slow `tar -tzf` doesn't park the node-host event
   // loop for up to 10s. Other in-flight requests continue to be served.
-  return new Promise((resolve) => {
+  return new Promise<string[]>((resolve) => {
     const child = spawn("tar", ["-tzf", "-"], { stdio: ["pipe", "pipe", "ignore"] });
     let stdoutBuf = "";
     let aborted = false;
@@ -102,7 +102,7 @@ async function countTarEntries(tarBuffer: Buffer): Promise<number> {
       } catch {
         /* gone */
       }
-      resolve(0);
+      resolve([]);
     }, 10_000);
     child.stdout.on("data", (chunk: Buffer) => {
       stdoutBuf += chunk.toString();
@@ -115,7 +115,7 @@ async function countTarEntries(tarBuffer: Buffer): Promise<number> {
           /* gone */
         }
         clearTimeout(watchdog);
-        resolve(0);
+        resolve([]);
       }
     });
     child.on("close", (code) => {
@@ -124,16 +124,19 @@ async function countTarEntries(tarBuffer: Buffer): Promise<number> {
         return;
       }
       if (code !== 0) {
-        resolve(0);
+        resolve([]);
         return;
       }
-      const lines = stdoutBuf.split("\n").filter((l) => l.trim().length > 0 && l !== "./");
-      resolve(lines.length);
+      const lines = stdoutBuf
+        .split("\n")
+        .map((line) => line.replace(/\\/gu, "/").replace(/^\.\//u, "").replace(/\/$/u, ""))
+        .filter((line) => line.length > 0);
+      resolve(lines);
     });
     child.on("error", () => {
       clearTimeout(watchdog);
       if (!aborted) {
-        resolve(0);
+        resolve([]);
       }
     });
     child.stdin.end(tarBuffer);
@@ -364,14 +367,15 @@ export async function handleDirFetch(params: DirFetchParams): Promise<DirFetchRe
   const sha256 = crypto.createHash("sha256").update(tarBuffer).digest("hex");
   const tarBase64 = tarBuffer.toString("base64");
   const tarBytes = tarBuffer.byteLength;
-  const fileCount = await countTarEntries(tarBuffer);
+  const entries = await listTarEntries(tarBuffer);
 
   return {
     ok: true,
     path: canonical,
     tarBase64,
     tarBytes,
     sha256,
-    fileCount,
+    fileCount: entries.length,
+    entries,
   };
 }
diff --git a/extensions/file-transfer/src/shared/node-invoke-policy.test.ts b/extensions/file-transfer/src/shared/node-invoke-policy.test.ts
@@ -1,5 +1,9 @@
+import { spawn } from "node:child_process";
+import fs from "node:fs/promises";
+import os from "node:os";
+import path from "node:path";
 import type { OpenClawPluginNodeInvokePolicyContext } from "openclaw/plugin-sdk/plugin-entry";
-import { describe, expect, it, vi } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 import { createFileTransferNodeInvokePolicy } from "./node-invoke-policy.js";
 
 vi.mock("./audit.js", () => ({
@@ -14,6 +18,44 @@ vi.mock("./policy.js", async (importOriginal) => {
   };
 });
 
+const tmpRoots: string[] = [];
+const testUnlessWindows = process.platform === "win32" ? it.skip : it;
+
+afterEach(async () => {
+  await Promise.all(tmpRoots.map((tmpRoot) => fs.rm(tmpRoot, { recursive: true, force: true })));
+  tmpRoots.length = 0;
+});
+
+async function tarEntries(entries: Record<string, string>): Promise<string> {
+  const tmpRoot = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "node-policy-tar-")));
+  tmpRoots.push(tmpRoot);
+  for (const [relPath, contents] of Object.entries(entries)) {
+    const absPath = path.join(tmpRoot, relPath);
+    await fs.mkdir(path.dirname(absPath), { recursive: true });
+    await fs.writeFile(absPath, contents);
+  }
+  return await new Promise<string>((resolve, reject) => {
+    const tarBin = process.platform !== "win32" ? "/usr/bin/tar" : "tar";
+    const child = spawn(tarBin, ["-czf", "-", "-C", tmpRoot, "."], {
+      stdio: ["ignore", "pipe", "pipe"],
+    });
+    const chunks: Buffer[] = [];
+    let stderr = "";
+    child.stdout.on("data", (chunk: Buffer) => chunks.push(chunk));
+    child.stderr.on("data", (chunk: Buffer) => {
+      stderr += chunk.toString();
+    });
+    child.on("close", (code) => {
+      if (code !== 0) {
+        reject(new Error(`tar exited ${code}: ${stderr}`));
+        return;
+      }
+      resolve(Buffer.concat(chunks).toString("base64"));
+    });
+    child.on("error", reject);
+  });
+}
+
 function createCtx(overrides: {
   command?: string;
   params?: Record<string, unknown>;
@@ -403,20 +445,123 @@ describe("file-transfer node invoke policy", () => {
     expect(invokeNode).toHaveBeenCalledTimes(1);
   });
 
-  it("continues dir.fetch after preflight without forwarding caller preflightOnly", async () => {
+  testUnlessWindows(
+    "continues dir.fetch after preflight without forwarding caller preflightOnly",
+    async () => {
+      const policy = createFileTransferNodeInvokePolicy();
+      const tarBase64 = await tarEntries({
+        "a.txt": "a",
+        "sub/b.txt": "b",
+      });
+      const { ctx, invokeNode } = createCtx({
+        command: "dir.fetch",
+        params: { path: "/tmp/project", preflightOnly: true },
+      });
+      invokeNode
+        .mockResolvedValueOnce({
+          ok: true,
+          payload: {
+            ok: true,
+            path: "/tmp/project",
+            entries: ["a.txt", "sub/b.txt"],
+            fileCount: 2,
+            preflightOnly: true,
+          },
+        })
+        .mockResolvedValueOnce({
+          ok: true,
+          payload: {
+            ok: true,
+            path: "/tmp/project",
+            tarBase64,
+            tarBytes: 7,
+            sha256: "c".repeat(64),
+            fileCount: 2,
+            entries: ["a.txt", "sub/b.txt"],
+          },
+        });
+
+      const result = await policy.handle(ctx);
+
+      expect(result).toMatchObject({ ok: true });
+      expect(invokeNode).toHaveBeenCalledTimes(2);
+      expect(invokeNode).toHaveBeenNthCalledWith(1, {
+        params: expect.objectContaining({ path: "/tmp/project", preflightOnly: true }),
+      });
+      expect(invokeNode).toHaveBeenNthCalledWith(2, {
+        params: expect.not.objectContaining({ preflightOnly: true }),
+      });
+    },
+  );
+
+  testUnlessWindows(
+    "checks final dir.fetch archive entries before returning the archive",
+    async () => {
+      const policy = createFileTransferNodeInvokePolicy();
+      const tarBase64 = await tarEntries({
+        "ok.txt": "ok",
+        ".ssh/id_rsa": "secret",
+      });
+      const { ctx, invokeNode } = createCtx({
+        command: "dir.fetch",
+        params: { path: "/home/me" },
+        pluginConfig: {
+          nodes: {
+            "node-1": {
+              allowReadPaths: ["/home/me", "/home/me/**"],
+              denyPaths: ["**/.ssh/**"],
+            },
+          },
+        },
+      });
+      invokeNode
+        .mockResolvedValueOnce({
+          ok: true,
+          payload: {
+            ok: true,
+            path: "/home/me",
+            entries: ["ok.txt"],
+            fileCount: 1,
+            preflightOnly: true,
+          },
+        })
+        .mockResolvedValueOnce({
+          ok: true,
+          payload: {
+            ok: true,
+            path: "/home/me",
+            tarBase64,
+            tarBytes: 7,
+            sha256: "c".repeat(64),
+            fileCount: 2,
+          },
+        });
+
+      const result = await policy.handle(ctx);
+
+      expect(result).toMatchObject({
+        ok: false,
+        code: "PATH_POLICY_DENIED",
+        details: { path: "/home/me/.ssh/id_rsa" },
+      });
+      expect(invokeNode).toHaveBeenCalledTimes(2);
+    },
+  );
+
+  it("rejects final dir.fetch archive responses without readable archive entries", async () => {
     const policy = createFileTransferNodeInvokePolicy();
     const { ctx, invokeNode } = createCtx({
       command: "dir.fetch",
-      params: { path: "/tmp/project", preflightOnly: true },
+      params: { path: "/tmp/project" },
     });
     invokeNode
       .mockResolvedValueOnce({
         ok: true,
         payload: {
           ok: true,
           path: "/tmp/project",
-          entries: ["a.txt", "sub/b.txt"],
-          fileCount: 2,
+          entries: ["a.txt"],
+          fileCount: 1,
           preflightOnly: true,
         },
       })
@@ -425,22 +570,15 @@ describe("file-transfer node invoke policy", () => {
         payload: {
           ok: true,
           path: "/tmp/project",
-          tarBase64: Buffer.from("archive").toString("base64"),
           tarBytes: 7,
           sha256: "c".repeat(64),
-          fileCount: 2,
+          fileCount: 1,
         },
       });
 
     const result = await policy.handle(ctx);
 
-    expect(result).toMatchObject({ ok: true });
+    expect(result).toMatchObject({ ok: false, code: "ARCHIVE_ENTRIES_MISSING" });
     expect(invokeNode).toHaveBeenCalledTimes(2);
-    expect(invokeNode).toHaveBeenNthCalledWith(1, {
-      params: expect.objectContaining({ path: "/tmp/project", preflightOnly: true }),
-    });
-    expect(invokeNode).toHaveBeenNthCalledWith(2, {
-      params: expect.not.objectContaining({ preflightOnly: true }),
-    });
   });
 });
diff --git a/extensions/file-transfer/src/shared/node-invoke-policy.ts b/extensions/file-transfer/src/shared/node-invoke-policy.ts
