fix(file-transfer): require canonical node policy authorization

clawsweeper-repair · omarshahine · clawsweeper-repair · commit fb6ca6747812 · 2026-04-30T03:25:06.000Z
Co-authored-by: Omar Shahine &lt;10343873+omarshahine@users.noreply.github.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -43,6 +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.
 - 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/file-fetch.test.ts b/extensions/file-transfer/src/node-host/file-fetch.test.ts
@@ -2,7 +2,7 @@ import crypto from "node:crypto";
 import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
-import { afterEach, beforeEach, describe, expect, it } from "vitest";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import {
   FILE_FETCH_DEFAULT_MAX_BYTES,
   FILE_FETCH_HARD_MAX_BYTES,
@@ -21,6 +21,7 @@ beforeEach(async () => {
 });
 
 afterEach(async () => {
+  vi.restoreAllMocks();
   await fs.rm(tmpRoot, { recursive: true, force: true });
 });
 
@@ -105,6 +106,24 @@ describe("handleFileFetch — happy path", () => {
     expect(path.basename(r.path)).toBe("hello.txt");
   });
 
+  it("preflights canonical path and size without reading bytes", async () => {
+    const target = path.join(tmpRoot, "hello.txt");
+    await fs.writeFile(target, "hello world\n");
+    const readFileSpy = vi.spyOn(fs, "readFile");
+
+    const r = await handleFileFetch({ path: target, preflightOnly: true });
+
+    expect(r).toMatchObject({
+      ok: true,
+      path: target,
+      size: 12,
+      base64: "",
+      sha256: "",
+      preflightOnly: true,
+    });
+    expect(readFileSpy).not.toHaveBeenCalled();
+  });
+
   it("returns a sensible mime type for known extensions", async () => {
     const target = path.join(tmpRoot, "readme.md");
     await fs.writeFile(target, "# heading\n");
diff --git a/extensions/file-transfer/src/node-host/file-fetch.ts b/extensions/file-transfer/src/node-host/file-fetch.ts
@@ -11,6 +11,7 @@ export type FileFetchParams = {
   path?: unknown;
   maxBytes?: unknown;
   followSymlinks?: unknown;
+  preflightOnly?: unknown;
 };
 
 export type FileFetchOk = {
@@ -20,6 +21,7 @@ export type FileFetchOk = {
   mimeType: string;
   base64: string;
   sha256: string;
+  preflightOnly?: boolean;
 };
 
 export type FileFetchErrCode =
@@ -95,6 +97,7 @@ export async function handleFileFetch(params: FileFetchParams): Promise<FileFetc
 
   const maxBytes = clampMaxBytes(params.maxBytes);
   const followSymlinks = params.followSymlinks === true;
+  const preflightOnly = params.preflightOnly === true;
 
   let canonical: string;
   try {
@@ -156,6 +159,18 @@ export async function handleFileFetch(params: FileFetchParams): Promise<FileFetc
     };
   }
 
+  if (preflightOnly) {
+    return {
+      ok: true,
+      path: canonical,
+      size: stats.size,
+      mimeType: "",
+      base64: "",
+      sha256: "",
+      preflightOnly: true,
+    };
+  }
+
   let buffer: Buffer;
   try {
     buffer = await fs.readFile(canonical);
diff --git a/extensions/file-transfer/src/shared/node-invoke-policy.test.ts b/extensions/file-transfer/src/shared/node-invoke-policy.test.ts
@@ -70,7 +70,15 @@ describe("file-transfer node invoke policy", () => {
     const result = await policy.handle(ctx);
 
     expect(result.ok).toBe(true);
-    expect(invokeNode).toHaveBeenCalledWith({
+    expect(invokeNode).toHaveBeenNthCalledWith(1, {
+      params: {
+        path: "/tmp/file.txt",
+        maxBytes: 512,
+        followSymlinks: false,
+        preflightOnly: true,
+      },
+    });
+    expect(invokeNode).toHaveBeenNthCalledWith(2, {
       params: {
         path: "/tmp/file.txt",
         maxBytes: 512,
@@ -118,7 +126,15 @@ describe("file-transfer node invoke policy", () => {
         toolName: "file.fetch",
       }),
     );
-    expect(invokeNode).toHaveBeenCalledWith({
+    expect(invokeNode).toHaveBeenNthCalledWith(1, {
+      params: {
+        path: "/tmp/new.txt",
+        followSymlinks: false,
+        maxBytes: 256,
+        preflightOnly: true,
+      },
+    });
+    expect(invokeNode).toHaveBeenNthCalledWith(2, {
       params: {
         path: "/tmp/new.txt",
         followSymlinks: false,
@@ -149,7 +165,7 @@ describe("file-transfer node invoke policy", () => {
     });
   });
 
-  it("rejects a postflight canonical path outside policy", async () => {
+  it("checks file.fetch canonical policy before requesting bytes", async () => {
     const policy = createFileTransferNodeInvokePolicy();
     const { ctx, invokeNode } = createCtx({
       params: { path: "/tmp/link.txt" },
@@ -167,6 +183,32 @@ describe("file-transfer node invoke policy", () => {
     const result = await policy.handle(ctx);
 
     expect(result).toMatchObject({ ok: false, code: "SYMLINK_TARGET_DENIED" });
+    expect(invokeNode).toHaveBeenCalledTimes(1);
+    expect(invokeNode).toHaveBeenCalledWith({
+      params: expect.objectContaining({
+        path: "/tmp/link.txt",
+        followSymlinks: false,
+        preflightOnly: true,
+      }),
+    });
+  });
+
+  it("continues file.fetch after preflight without forwarding caller preflightOnly", async () => {
+    const policy = createFileTransferNodeInvokePolicy();
+    const { ctx, invokeNode } = createCtx({
+      params: { path: "/tmp/file.txt", preflightOnly: true },
+    });
+
+    const result = await policy.handle(ctx);
+
+    expect(result).toMatchObject({ ok: true });
+    expect(invokeNode).toHaveBeenCalledTimes(2);
+    expect(invokeNode).toHaveBeenNthCalledWith(1, {
+      params: expect.objectContaining({ path: "/tmp/file.txt", preflightOnly: true }),
+    });
+    expect(invokeNode).toHaveBeenNthCalledWith(2, {
+      params: expect.not.objectContaining({ preflightOnly: true }),
+    });
   });
 
   it("checks file.write canonical policy before the mutating node call", async () => {
@@ -302,6 +344,65 @@ describe("file-transfer node invoke policy", () => {
     });
   });
 
+  it("rejects dir.fetch preflight responses without an entry list", async () => {
+    const policy = createFileTransferNodeInvokePolicy();
+    const { ctx, invokeNode } = createCtx({
+      command: "dir.fetch",
+      params: { path: "/home/me" },
+      pluginConfig: {
+        nodes: {
+          "node-1": {
+            allowReadPaths: ["/home/me", "/home/me/**"],
+          },
+        },
+      },
+    });
+    invokeNode.mockResolvedValueOnce({
+      ok: true,
+      payload: {
+        ok: true,
+        path: "/home/me",
+        fileCount: 2,
+        preflightOnly: true,
+      },
+    });
+
+    const result = await policy.handle(ctx);
+
+    expect(result).toMatchObject({ ok: false, code: "PREFLIGHT_ENTRIES_MISSING" });
+    expect(invokeNode).toHaveBeenCalledTimes(1);
+  });
+
+  it("rejects invalid dir.fetch preflight entries before requesting the archive", async () => {
+    const policy = createFileTransferNodeInvokePolicy();
+    const { ctx, invokeNode } = createCtx({
+      command: "dir.fetch",
+      params: { path: "/home/me" },
+      pluginConfig: {
+        nodes: {
+          "node-1": {
+            allowReadPaths: ["/home/me", "/home/me/**"],
+          },
+        },
+      },
+    });
+    invokeNode.mockResolvedValueOnce({
+      ok: true,
+      payload: {
+        ok: true,
+        path: "/home/me",
+        entries: ["ok.txt", "/etc/passwd"],
+        fileCount: 2,
+        preflightOnly: true,
+      },
+    });
+
+    const result = await policy.handle(ctx);
+
+    expect(result).toMatchObject({ ok: false, code: "PREFLIGHT_ENTRY_INVALID" });
+    expect(invokeNode).toHaveBeenCalledTimes(1);
+  });
+
   it("continues dir.fetch after preflight without forwarding caller preflightOnly", async () => {
     const policy = createFileTransferNodeInvokePolicy();
     const { ctx, invokeNode } = createCtx({
diff --git a/extensions/file-transfer/src/shared/node-invoke-policy.ts b/extensions/file-transfer/src/shared/node-invoke-policy.ts
