fix(file-transfer): keep fetched media attachable in sandboxed replies (#116400)

Fixes #116338

Co-authored-by: wangmiao0668000666 <wang.miao86@xydigit.com>

extensions/file-transfer/src/tools/descriptors.ts

@@ -8,9 +8,9 @@ type FileTransferToolDescriptor = Pick<
   "label" | "name" | "description" | "parameters"
 >;
 
-// Stash fetched files in a non-TTL subdir so follow-up tool calls within
-// the same turn can still reference them.
-export const FILE_TRANSFER_SUBDIR = "file-transfer";
+// Keep fetched files in the managed tool-media namespace so sandboxed replies
+// can attach them and follow-up file_write calls can reuse the media id.
+export const FILE_TRANSFER_SUBDIR = "tool-file-transfer";
 
 export const FILE_FETCH_DEFAULT_MAX_BYTES = 8 * 1024 * 1024;
 export const FILE_FETCH_HARD_MAX_BYTES = 16 * 1024 * 1024;

extensions/file-transfer/src/tools/dir-fetch-tool.test.ts

@@ -5,6 +5,7 @@ import os from "node:os";
 import path from "node:path";
 import * as tar from "tar";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { DIR_FETCH_HARD_MAX_BYTES, FILE_TRANSFER_SUBDIR } from "./descriptors.js";
 
 let tmpRoot: string;
 
@@ -37,12 +38,13 @@ async function createTarBuffer(params: {
 async function importTool(tarBuffer: Buffer) {
   const archivePath = path.join(tmpRoot, `archive-${randomUUID()}.tar.gz`);
   const appendFileTransferAudit = vi.fn(async () => undefined);
+  const saveMediaBuffer = vi.fn(async () => {
+    await fs.writeFile(archivePath, tarBuffer);
+    return { path: archivePath };
+  });
   vi.resetModules();
   vi.doMock("openclaw/plugin-sdk/media-store", () => ({
-    saveMediaBuffer: vi.fn(async () => {
-      await fs.writeFile(archivePath, tarBuffer);
-      return { path: archivePath };
-    }),
+    saveMediaBuffer,
   }));
   vi.doMock("../shared/audit.js", () => ({ appendFileTransferAudit }));
   vi.doMock("./node-tool-invoke.js", () => ({
@@ -67,6 +69,7 @@ async function importTool(tarBuffer: Buffer) {
   return {
     archivePath,
     appendFileTransferAudit,
+    saveMediaBuffer,
     module: await import("./dir-fetch-tool.js"),
   };
 }
@@ -86,7 +89,7 @@ describe("dir.fetch archive extraction", () => {
         await fs.writeFile(path.join(sourceDir, "ok.txt"), "ok");
       },
     });
-    const { appendFileTransferAudit, module } = await importTool(tarBuffer);
+    const { appendFileTransferAudit, module, saveMediaBuffer } = await importTool(tarBuffer);
 
     const result = await executeDirFetch(module);
 
@@ -106,6 +109,12 @@ describe("dir.fetch archive extraction", () => {
     const localPath = (result.details as { files: Array<{ localPath: string }> }).files[0]
       ?.localPath;
     await expect(fs.readFile(localPath!, "utf8")).resolves.toBe("ok");
+    expect(saveMediaBuffer).toHaveBeenCalledWith(
+      tarBuffer,
+      "application/gzip",
+      FILE_TRANSFER_SUBDIR,
+      DIR_FETCH_HARD_MAX_BYTES,
+    );
     expect(appendFileTransferAudit).toHaveBeenLastCalledWith(
       expect.objectContaining({ decision: "allowed" }),
     );

extensions/file-transfer/src/tools/dir-fetch-tool.ts

@@ -169,7 +169,7 @@ export function createDirFetchTool(): AnyAgentTool {
         throw new Error("dir.fetch sha256 mismatch (integrity failure)");
       }
 
-      // Save tarball under the file-transfer subdir (no 2-min TTL).
+      // Keep the tarball and extracted paths under the same managed tool namespace.
       const savedTar = await saveMediaBuffer(
         tarBuffer,
         "application/gzip",

extensions/file-transfer/src/tools/file-fetch-tool.test.ts

@@ -7,6 +7,7 @@ import {
 } from "openclaw/plugin-sdk/agent-harness-runtime";
 import { saveMediaBuffer } from "openclaw/plugin-sdk/media-store";
 import { afterEach, describe, expect, it, vi } from "vitest";
+import { FILE_TRANSFER_SUBDIR } from "./descriptors.js";
 import { createFileFetchTool } from "./file-fetch-tool.js";
 
 vi.mock("openclaw/plugin-sdk/agent-harness-runtime", () => ({
@@ -57,7 +58,7 @@ describe("file_fetch tool", () => {
     });
     vi.mocked(saveMediaBuffer).mockResolvedValue({
       id: "media-1",
-      path: "/gateway/media/file-transfer/report.md",
+      path: "/gateway/media/tool-file-transfer/report.md",
       size: Buffer.byteLength(fileText),
       contentType: "text/markdown",
     });
@@ -95,7 +96,7 @@ describe("file_fetch tool", () => {
     });
     vi.mocked(saveMediaBuffer).mockResolvedValue({
       id: "media-1",
-      path: "/gateway/media/file-transfer/bom.md",
+      path: "/gateway/media/tool-file-transfer/bom.md",
       size: originalBuffer.byteLength,
       contentType: "text/markdown",
     });
@@ -111,7 +112,7 @@ describe("file_fetch tool", () => {
     expect(saveMediaBuffer).toHaveBeenCalledWith(
       originalBuffer,
       "text/markdown",
-      expect.any(String),
+      FILE_TRANSFER_SUBDIR,
       expect.any(Number),
     );
     const details = result.details as { sha256: string; size: number };
@@ -134,7 +135,7 @@ describe("file_fetch tool", () => {
     });
     vi.mocked(saveMediaBuffer).mockResolvedValue({
       id: "media-1",
-      path: "/gateway/media/file-transfer/empty.png",
+      path: "/gateway/media/tool-file-transfer/empty.png",
       size: 0,
       contentType: "image/png",
     });
@@ -148,7 +149,7 @@ describe("file_fetch tool", () => {
     expect(result.content[0]?.type).toBe("text");
     const text = result.content[0]?.type === "text" ? result.content[0].text : "";
     expect(text).toContain("Fetched /tmp/empty.png");
-    expect(text).toContain("saved at /gateway/media/file-transfer/empty.png");
+    expect(text).toContain("saved at /gateway/media/tool-file-transfer/empty.png");
   });
 
   it("still inlines a non-empty image payload", async () => {
@@ -167,7 +168,7 @@ describe("file_fetch tool", () => {
     });
     vi.mocked(saveMediaBuffer).mockResolvedValue({
       id: "media-1",
-      path: "/gateway/media/file-transfer/photo.png",
+      path: "/gateway/media/tool-file-transfer/photo.png",
       size: buffer.byteLength,
       contentType: "image/png",
     });

extensions/file-transfer/src/tools/file-write-tool.test.ts

@@ -1,12 +1,14 @@
 // File Transfer tests cover file write tool plugin behavior.
+import crypto from "node:crypto";
 import {
   callGatewayTool,
   listNodes,
   resolveNodeIdFromList,
 } from "openclaw/plugin-sdk/agent-harness-runtime";
+import { readMediaBuffer } from "openclaw/plugin-sdk/media-store";
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import { humanSize } from "../shared/params.js";
-import { FILE_WRITE_HARD_MAX_BYTES } from "./descriptors.js";
+import { FILE_TRANSFER_SUBDIR, FILE_WRITE_HARD_MAX_BYTES } from "./descriptors.js";
 import { createFileWriteTool } from "./file-write-tool.js";
 
 vi.mock("openclaw/plugin-sdk/agent-harness-runtime", () => ({
@@ -101,4 +103,38 @@ describe("file_write tool", () => {
 
     expect(callGatewayTool).toHaveBeenCalledOnce();
   });
+
+  it("reads file_fetch media from the shared managed tool namespace", async () => {
+    const buffer = Buffer.from("copied");
+    vi.mocked(readMediaBuffer).mockResolvedValue({
+      id: "media-1",
+      buffer,
+      path: "/gateway/media/tool-file-transfer/media-1.bin",
+      size: buffer.byteLength,
+    });
+    vi.mocked(listNodes).mockResolvedValue([{ nodeId: "node-1", displayName: "Node 1" }]);
+    vi.mocked(resolveNodeIdFromList).mockReturnValue("node-1");
+    vi.mocked(callGatewayTool).mockResolvedValue({
+      payload: {
+        ok: true,
+        path: "/tmp/out.bin",
+        size: buffer.byteLength,
+        sha256: crypto.createHash("sha256").update(buffer).digest("hex"),
+        overwritten: false,
+      },
+    });
+
+    const result = await createFileWriteTool().execute("tool-call-1", {
+      node: "node-1",
+      path: "/tmp/out.bin",
+      sourceMediaId: "media-1",
+    });
+
+    expect(readMediaBuffer).toHaveBeenCalledWith(
+      "media-1",
+      FILE_TRANSFER_SUBDIR,
+      FILE_WRITE_HARD_MAX_BYTES,
+    );
+    expect(result.details).toMatchObject({ source: "media", size: buffer.byteLength });
+  });
 });

src/agents/sandbox-paths.test.ts

@@ -41,6 +41,7 @@ async function withManagedMediaRoot<T>(run: (ctx: { stateDir: string }) => Promi
   try {
     return await withEnvAsync({ OPENCLAW_STATE_DIR: stateDir }, async () => {
       await fs.mkdir(path.join(stateDir, "media", "outbound"), { recursive: true });
+      await fs.mkdir(path.join(stateDir, "media", "tool-file-transfer"), { recursive: true });
       await fs.mkdir(path.join(stateDir, "media", "tool-image-generation"), { recursive: true });
       return await run({ stateDir });
     });
@@ -242,6 +243,10 @@ describe("resolveSandboxedMediaSource", () => {
       name: "managed outbound media",
       relative: path.join("media", "outbound", "reply.png"),
     },
+    {
+      name: "managed file-transfer tool media",
+      relative: path.join("media", "tool-file-transfer", "fetched.png"),
+    },
     {
       name: "managed tool media",
       relative: path.join("media", "tool-image-generation", "generated.png"),
@@ -475,47 +480,90 @@ describe("resolveSandboxedMediaSource", () => {
     );
   });
 
-  it("rejects symlinked managed media paths escaping the managed media root", async () => {
-    if (process.platform === "win32") {
-      return;
-    }
-    await withManagedMediaRoot(async ({ stateDir }) => {
-      await withSandboxRoot(async (sandboxDir) => {
+  it.each(["outbound", "tool-file-transfer"])(
+    "rejects symlinked managed media paths escaping the %s root",
+    async (subdir) => {
+      if (process.platform === "win32") {
+        return;
+      }
+      await withManagedMediaRoot(async ({ stateDir }) => {
+        await withSandboxRoot(async (sandboxDir) => {
+          const outsideDir = await fs.mkdtemp(path.join(os.tmpdir(), "managed-media-outside-"));
+          const outsideFile = path.join(outsideDir, "secret.png");
+          const symlinkPath = path.join(stateDir, "media", subdir, "linked-secret.png");
+          try {
+            await fs.writeFile(outsideFile, "secret", "utf8");
+            await fs.symlink(outsideFile, symlinkPath);
+
+            await expectSandboxRejection(symlinkPath, sandboxDir, /managed media root|symlink/i);
+          } finally {
+            await fs.rm(symlinkPath, { force: true });
+            await fs.rm(outsideDir, { recursive: true, force: true });
+          }
+        });
+      });
+    },
+  );
+
+  it.each(["outbound", "tool-file-transfer"])(
+    "rejects checked managed media symlinks escaping the %s root",
+    async (subdir) => {
+      if (process.platform === "win32") {
+        return;
+      }
+      await withManagedMediaRoot(async ({ stateDir }) => {
         const outsideDir = await fs.mkdtemp(path.join(os.tmpdir(), "managed-media-outside-"));
         const outsideFile = path.join(outsideDir, "secret.png");
-        const symlinkPath = path.join(stateDir, "media", "outbound", "linked-secret.png");
+        const symlinkPath = path.join(stateDir, "media", subdir, "linked-secret.png");
         try {
           await fs.writeFile(outsideFile, "secret", "utf8");
           await fs.symlink(outsideFile, symlinkPath);
 
-          await expectSandboxRejection(symlinkPath, sandboxDir, /managed media root|symlink/i);
+          await expect(resolveAllowedManagedMediaPath(symlinkPath)).rejects.toThrow(
+            /managed media root|symlink/i,
+          );
         } finally {
           await fs.rm(symlinkPath, { force: true });
           await fs.rm(outsideDir, { recursive: true, force: true });
         }
       });
-    });
-  });
+    },
+  );
 
-  it("rejects checked managed media symlinks escaping the managed media root", async () => {
+  it("rejects hardlinked file-transfer media that aliases a file outside managed media", async () => {
     if (process.platform === "win32") {
       return;
     }
     await withManagedMediaRoot(async ({ stateDir }) => {
-      const outsideDir = await fs.mkdtemp(path.join(os.tmpdir(), "managed-media-outside-"));
-      const outsideFile = path.join(outsideDir, "secret.png");
-      const symlinkPath = path.join(stateDir, "media", "outbound", "linked-secret.png");
-      try {
-        await fs.writeFile(outsideFile, "secret", "utf8");
-        await fs.symlink(outsideFile, symlinkPath);
-
-        await expect(resolveAllowedManagedMediaPath(symlinkPath)).rejects.toThrow(
-          /managed media root|symlink/i,
+      await withSandboxRoot(async (sandboxDir) => {
+        const outsideDir = await fs.mkdtemp(
+          path.join(path.dirname(stateDir), "managed-media-hardlink-outside-"),
         );
-      } finally {
-        await fs.rm(symlinkPath, { force: true });
-        await fs.rm(outsideDir, { recursive: true, force: true });
-      }
+        const outsideFile = path.join(outsideDir, "secret.png");
+        const hardlinkPath = path.join(
+          stateDir,
+          "media",
+          "tool-file-transfer",
+          "linked-secret.png",
+        );
+        try {
+          await fs.writeFile(outsideFile, "secret", "utf8");
+          try {
+            await fs.link(outsideFile, hardlinkPath);
+          } catch (err) {
+            if ((err as NodeJS.ErrnoException).code === "EXDEV") {
+              return;
+            }
+            throw err;
+          }
+
+          await expect(resolveAllowedManagedMediaPath(hardlinkPath)).rejects.toThrow(/hard.?link/i);
+          await expectSandboxRejection(hardlinkPath, sandboxDir, /hard.?link|managed media root/i);
+        } finally {
+          await fs.rm(hardlinkPath, { force: true });
+          await fs.rm(outsideDir, { recursive: true, force: true });
+        }
+      });
     });
   });