test: align fs-safe dependency expectations

2026-05-07 14:50:42 +00:00 · 2026-05-07 04:16:04 +01:00
parent a859638cc2
commit 58fa23b4a2
6 changed files with 188 additions and 132 deletions
--- a/src/infra/archive.test.ts
+++ b/src/infra/archive.test.ts
@@ -240,7 +240,7 @@ describe("archive utils", () => {
      } catch (error) {
        rejected = true;
        expect(error).toMatchObject({
-          code: "destination-symlink-traversal",
+          code: expect.stringMatching(/destination-symlink-traversal|not-file/),
        } satisfies Partial<ArchiveSecurityError>);
      }

--- a/src/infra/fs-safe.test.ts
+++ b/src/infra/fs-safe.test.ts
@@ -38,7 +38,9 @@ async function runWriteOpenRace(params: {
        await params.runWrite();
      } catch (err) {
        expect(err).toMatchObject({
-          code: expect.stringMatching(/outside-workspace|path-mismatch|path-alias|invalid-path/),
+          code: expect.stringMatching(
+            /outside-workspace|path-mismatch|path-alias|invalid-path|not-file/,
+          ),
        });
      }
    },
--- a/src/infra/json-file.test.ts
+++ b/src/infra/json-file.test.ts
@@ -175,10 +175,9 @@ describe("json-file helpers", () => {
    },
  );

-  it("falls back to copy when rename-based overwrite fails", async () => {
+  it("preserves payload when rename-based overwrite reports EPERM", async () => {
    await withJsonPath(({ root, pathname }) => {
      writeExistingJson(pathname);
-      const copySpy = vi.spyOn(fs, "copyFileSync");
      const renameSpy = vi.spyOn(fs, "renameSync").mockImplementationOnce(() => {
        const err = new Error("EPERM") as NodeJS.ErrnoException;
        err.code = "EPERM";
@@ -187,8 +186,7 @@ describe("json-file helpers", () => {

      saveJsonFile(pathname, SAVED_PAYLOAD);

-      expect(renameSpy).toHaveBeenCalledOnce();
-      expect(copySpy).toHaveBeenCalledOnce();
+      expect(renameSpy).toHaveBeenCalled();
      expect(loadJsonFile(pathname)).toEqual(SAVED_PAYLOAD);
      expect(fs.readdirSync(root)).toEqual(["config.json"]);
    });
--- a/src/infra/json-files.test.ts
+++ b/src/infra/json-files.test.ts
@@ -96,7 +96,7 @@ describe("json file helpers", () => {
    });
  });

-  it("falls back to copy-on-replace for Windows rename EPERM", async () => {
+  it("preserves text when Windows rename reports EPERM", async () => {
    await withTempDir({ prefix: "openclaw-json-files-" }, async (base) => {
      const filePath = path.join(base, "state.json");
      await fs.writeFile(filePath, "old", "utf8");
@@ -104,12 +104,10 @@ describe("json file helpers", () => {
      Object.defineProperty(process, "platform", { value: "win32", configurable: true });
      const renameError = Object.assign(new Error("EPERM"), { code: "EPERM" });
      const renameSpy = vi.spyOn(fs, "rename").mockRejectedValueOnce(renameError);
-      const copySpy = vi.spyOn(fs, "copyFile");

      await writeTextAtomic(filePath, "new");

      expect(renameSpy).toHaveBeenCalledOnce();
-      expect(copySpy).toHaveBeenCalledOnce();
      await expect(fs.readFile(filePath, "utf8")).resolves.toBe("new");
    });
  });
--- a/src/media/store.test.ts
+++ b/src/media/store.test.ts
@@ -52,30 +52,97 @@ describe("media store", () => {
    segment: string;
    run: (store: typeof import("./store.js"), home: string) => Promise<{ path: string }>;
  }) {
-    await withTempStore(async (store, home) => {
-      const originalWriteFile = fs.writeFile.bind(fs);
-      let injectedEnoent = false;
-      vi.spyOn(fs, "writeFile").mockImplementation(async (...args) => {
-        const [filePath] = args;
-        if (
-          !injectedEnoent &&
-          typeof filePath === "string" &&
-          filePath.includes(`${path.sep}${params.segment}${path.sep}`)
-        ) {
-          injectedEnoent = true;
-          await fs.rm(path.dirname(filePath), { recursive: true, force: true });
-          const err = new Error("missing dir") as NodeJS.ErrnoException;
-          err.code = "ENOENT";
-          throw err;
-        }
-        return await originalWriteFile(...args);
-      });
-
-      const saved = await params.run(store, home);
-      const savedStat = await fs.stat(saved.path);
-      expect(injectedEnoent).toBe(true);
-      expect(savedStat.isFile()).toBe(true);
+    const mockKey = `./store.js?scope=retry-pruned-write-${params.segment}-${Date.now()}-${Math.random().toString(36).slice(2)}`;
+    let injectedEnoent = false;
+    vi.doMock("../infra/file-store.js", async (importOriginal) => {
+      const actual = await importOriginal<typeof import("../infra/file-store.js")>();
+      return {
+        ...actual,
+        fileStore: (options: Parameters<typeof actual.fileStore>[0]) => {
+          const actualStore = actual.fileStore(options);
+          return {
+            ...actualStore,
+            write: async (...args: Parameters<typeof actualStore.write>) => {
+              const [relativePath] = args;
+              if (!injectedEnoent && relativePath.includes(`${params.segment}${path.sep}`)) {
+                injectedEnoent = true;
+                await fs.rm(path.dirname(actualStore.path(relativePath)), {
+                  recursive: true,
+                  force: true,
+                });
+                const err = new Error("missing dir") as NodeJS.ErrnoException;
+                err.code = "ENOENT";
+                throw err;
+              }
+              return await actualStore.write(...args);
+            },
+          };
+        },
+      };
    });
+
+    try {
+      const storeWithMock = await importFreshModule<typeof import("./store.js")>(
+        import.meta.url,
+        mockKey,
+      );
+      await withTempStore(async (_store, home) => {
+        const saved = await params.run(storeWithMock, home);
+        const savedStat = await fs.stat(saved.path);
+        expect(injectedEnoent).toBe(true);
+        expect(savedStat.isFile()).toBe(true);
+      });
+    } finally {
+      vi.doUnmock("../infra/file-store.js");
+    }
+  }
+
+  async function expectFailedBufferWriteCase() {
+    const mockKey = `./store.js?scope=failed-buffer-write-${Date.now()}-${Math.random().toString(36).slice(2)}`;
+    const attemptedRelPaths: string[] = [];
+    vi.doMock("../infra/file-store.js", async (importOriginal) => {
+      const actual = await importOriginal<typeof import("../infra/file-store.js")>();
+      return {
+        ...actual,
+        fileStore: (options: Parameters<typeof actual.fileStore>[0]) => {
+          const actualStore = actual.fileStore(options);
+          return {
+            ...actualStore,
+            write: async (...args: Parameters<typeof actualStore.write>) => {
+              const [relativePath] = args;
+              if (relativePath.includes(`failed-buffer${path.sep}`)) {
+                attemptedRelPaths.push(relativePath);
+                const err = new Error("no space left on device") as NodeJS.ErrnoException;
+                err.code = "ENOSPC";
+                throw err;
+              }
+              return await actualStore.write(...args);
+            },
+          };
+        },
+      };
+    });
+
+    try {
+      const storeWithMock = await importFreshModule<typeof import("./store.js")>(
+        import.meta.url,
+        mockKey,
+      );
+      await withTempStore(async (_store) => {
+        const mediaDir = await storeWithMock.ensureMediaDir();
+        await expect(
+          storeWithMock.saveMediaBuffer(Buffer.from("voice"), "audio/ogg", "failed-buffer"),
+        ).rejects.toMatchObject({ code: "ENOSPC" });
+
+        const failedDir = path.join(mediaDir, "failed-buffer");
+        const entries = await fs.readdir(failedDir).catch(() => []);
+        expect(attemptedRelPaths).toHaveLength(1);
+        expect(path.basename(attemptedRelPaths[0] ?? "")).toMatch(/^[^/\\]+\.ogg$/);
+        expect(entries).toEqual([]);
+      });
+    } finally {
+      vi.doUnmock("../infra/file-store.js");
+    }
  }

  async function expectSavedOriginalFilenameCase(params: {
@@ -310,35 +377,7 @@ describe("media store", () => {
    {
      name: "does not leave final media artifacts when buffer writes fail",
      run: async () => {
-        await withTempStore(async (store) => {
-          const mediaDir = await store.ensureMediaDir();
-          const originalWriteFile = fs.writeFile.bind(fs);
-          const attemptedPaths: string[] = [];
-          vi.spyOn(fs, "writeFile").mockImplementation(async (...args) => {
-            const [filePath] = args;
-            if (
-              typeof filePath === "string" &&
-              filePath.includes(`${path.sep}failed-buffer${path.sep}`)
-            ) {
-              attemptedPaths.push(filePath);
-              await originalWriteFile(filePath, Buffer.alloc(0), args[2]);
-              const err = new Error("no space left on device") as NodeJS.ErrnoException;
-              err.code = "ENOSPC";
-              throw err;
-            }
-            return await originalWriteFile(...args);
-          });
-
-          await expect(
-            store.saveMediaBuffer(Buffer.from("voice"), "audio/ogg", "failed-buffer"),
-          ).rejects.toMatchObject({ code: "ENOSPC" });
-
-          const failedDir = path.join(mediaDir, "failed-buffer");
-          const entries = await fs.readdir(failedDir).catch(() => []);
-          expect(attemptedPaths).toHaveLength(1);
-          expect(path.basename(attemptedPaths[0] ?? "")).toMatch(/^\..+\.tmp$/);
-          expect(entries).toEqual([]);
-        });
+        await expectFailedBufferWriteCase();
      },
    },
    {
--- a/src/plugin-sdk/browser-maintenance.test.ts
+++ b/src/plugin-sdk/browser-maintenance.test.ts
@@ -1,10 +1,16 @@
 import fs from "node:fs";
 import os from "node:os";
-import { beforeEach, describe, expect, it, vi } from "vitest";
+import path from "node:path";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";

 const closeTrackedBrowserTabsForSessionsImpl = vi.hoisted(() => vi.fn());
 const loadBundledPluginPublicSurfaceModuleSync = vi.hoisted(() => vi.fn());
 const runExec = vi.hoisted(() => vi.fn());
+const realMkdirSync = fs.mkdirSync.bind(fs);
+const realMkdtempSync = fs.mkdtempSync.bind(fs);
+const realRmSync = fs.rmSync.bind(fs);
+const realWriteFileSync = fs.writeFileSync.bind(fs);
+const realRealpathSyncNative = fs.realpathSync.native.bind(fs.realpathSync);

 vi.mock("./facade-loader.js", () => ({
  loadBundledPluginPublicSurfaceModuleSync,
@@ -19,29 +25,53 @@ function mockTrashContainer(...suffixes: string[]) {
  return vi.spyOn(fs, "mkdtempSync").mockImplementation((prefix) => {
    const suffix = suffixes[call] ?? "secure";
    call += 1;
-    return `${prefix}${suffix}`;
+    const container = `${prefix}${suffix}`;
+    realMkdirSync(container, { recursive: true });
+    return container;
  });
 }

 describe("browser maintenance", () => {
+  let testRoot = "";
+  let homeDir = "";
+  let tmpDir = "";
+
  beforeEach(() => {
    vi.restoreAllMocks();
+    testRoot = realRealpathSyncNative(
+      realMkdtempSync(path.join(os.tmpdir(), "openclaw-browser-maintenance-")),
+    );
+    homeDir = path.join(testRoot, "home", "test");
+    tmpDir = path.join(testRoot, "tmp");
+    realMkdirSync(path.join(homeDir, ".Trash"), { recursive: true, mode: 0o700 });
+    realMkdirSync(tmpDir, { recursive: true, mode: 0o700 });
    closeTrackedBrowserTabsForSessionsImpl.mockReset();
    loadBundledPluginPublicSurfaceModuleSync.mockReset();
    runExec.mockReset();
    vi.spyOn(Date, "now").mockReturnValue(123);
-    vi.spyOn(os, "homedir").mockReturnValue("/home/test");
-    vi.spyOn(os, "tmpdir").mockReturnValue("/tmp");
-    vi.spyOn(fs, "lstatSync").mockReturnValue({
-      isDirectory: () => true,
-      isSymbolicLink: () => false,
-    } as fs.Stats);
-    vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) => String(candidate));
+    vi.spyOn(os, "homedir").mockReturnValue(homeDir);
+    vi.spyOn(os, "tmpdir").mockReturnValue(tmpDir);
+    vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) =>
+      realRealpathSyncNative(candidate),
+    );
    loadBundledPluginPublicSurfaceModuleSync.mockReturnValue({
      closeTrackedBrowserTabsForSessions: closeTrackedBrowserTabsForSessionsImpl,
    });
  });

+  afterEach(() => {
+    vi.restoreAllMocks();
+    if (testRoot) {
+      realRmSync(testRoot, { recursive: true, force: true });
+    }
+  });
+
+  function writeTrashTarget(name = "demo"): string {
+    const target = path.join(tmpDir, name);
+    realWriteFileSync(target, "demo");
+    return target;
+  }
+
  it("skips browser cleanup when no session keys are provided", async () => {
    const { closeTrackedBrowserTabsForSessions } = await import("./browser-maintenance.js");

@@ -74,46 +104,47 @@ describe("browser maintenance", () => {
    const rmSync = vi.spyOn(fs, "rmSync");

    const { movePathToTrash } = await import("./browser-maintenance.js");
+    const target = writeTrashTarget();
+    const expected = path.join(homeDir, ".Trash", "demo-123-secure", "demo");

-    await expect(movePathToTrash("/tmp/demo")).resolves.toBe(
-      "/home/test/.Trash/demo-123-secure/demo",
-    );
+    await expect(movePathToTrash(target)).resolves.toBe(expected);
    expect(runExec).not.toHaveBeenCalled();
-    expect(mkdirSync).toHaveBeenCalledWith("/home/test/.Trash", {
+    expect(mkdirSync).toHaveBeenCalledWith(path.join(homeDir, ".Trash"), {
      recursive: true,
      mode: 0o700,
    });
-    expect(mkdtempSync).toHaveBeenCalledWith("/home/test/.Trash/demo-123-");
-    expect(renameSync).toHaveBeenCalledWith("/tmp/demo", "/home/test/.Trash/demo-123-secure/demo");
+    expect(mkdtempSync).toHaveBeenCalledWith(path.join(homeDir, ".Trash", "demo-123-"));
+    expect(renameSync).toHaveBeenCalledWith(target, expected);
    expect(cpSync).not.toHaveBeenCalled();
    expect(rmSync).not.toHaveBeenCalled();
  });

  it("uses the resolved trash directory for reserved destinations", async () => {
    vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined);
+    const resolvedHomeDir = path.join(testRoot, "real", "home", "test");
+    const resolvedTrashDir = path.join(resolvedHomeDir, ".Trash");
+    realMkdirSync(path.join(homeDir, ".Trash"), { recursive: true, mode: 0o700 });
+    realMkdirSync(resolvedTrashDir, { recursive: true, mode: 0o700 });
    vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) => {
      const value = String(candidate);
-      if (value === "/home/test") {
-        return "/real/home/test";
+      if (value === homeDir) {
+        return resolvedHomeDir;
      }
-      if (value === "/home/test/.Trash") {
-        return "/real/home/test/.Trash";
+      if (value === path.join(homeDir, ".Trash")) {
+        return resolvedTrashDir;
      }
-      return value;
+      return realRealpathSyncNative(candidate);
    });
    const mkdtempSync = mockTrashContainer("secure");
    const renameSync = vi.spyOn(fs, "renameSync").mockImplementation(() => undefined);

    const { movePathToTrash } = await import("./browser-maintenance.js");
+    const target = writeTrashTarget();
+    const expected = path.join(resolvedTrashDir, "demo-123-secure", "demo");

-    await expect(movePathToTrash("/tmp/demo")).resolves.toBe(
-      "/real/home/test/.Trash/demo-123-secure/demo",
-    );
-    expect(mkdtempSync).toHaveBeenCalledWith("/real/home/test/.Trash/demo-123-");
-    expect(renameSync).toHaveBeenCalledWith(
-      "/tmp/demo",
-      "/real/home/test/.Trash/demo-123-secure/demo",
-    );
+    await expect(movePathToTrash(target)).resolves.toBe(expected);
+    expect(mkdtempSync).toHaveBeenCalledWith(path.join(resolvedTrashDir, "demo-123-"));
+    expect(renameSync).toHaveBeenCalledWith(target, expected);
  });

  it("refuses to trash filesystem roots", async () => {
@@ -124,8 +155,12 @@ describe("browser maintenance", () => {

  it("refuses to trash paths outside allowed roots", async () => {
    const { movePathToTrash } = await import("./browser-maintenance.js");
+    const outsideDir = path.join(testRoot, "outside");
+    realMkdirSync(outsideDir, { recursive: true });
+    const outsidePath = path.join(outsideDir, "openclaw-demo");
+    realWriteFileSync(outsidePath, "outside");

-    await expect(movePathToTrash("/etc/openclaw-demo")).rejects.toThrow(
+    await expect(movePathToTrash(outsidePath)).rejects.toThrow(
      "Refusing to trash path outside allowed roots",
    );
  });
@@ -139,7 +174,7 @@ describe("browser maintenance", () => {

    const { movePathToTrash } = await import("./browser-maintenance.js");

-    await expect(movePathToTrash("/tmp/demo")).rejects.toThrow(
+    await expect(movePathToTrash(writeTrashTarget())).rejects.toThrow(
      "Refusing to use non-directory/symlink trash directory",
    );
  });
@@ -155,16 +190,16 @@ describe("browser maintenance", () => {
    const rmSync = vi.spyOn(fs, "rmSync").mockImplementation(() => undefined);

    const { movePathToTrash } = await import("./browser-maintenance.js");
+    const target = writeTrashTarget();
+    const expected = path.join(homeDir, ".Trash", "demo-123-secure", "demo");

-    await expect(movePathToTrash("/tmp/demo")).resolves.toBe(
-      "/home/test/.Trash/demo-123-secure/demo",
-    );
-    expect(cpSync).toHaveBeenCalledWith("/tmp/demo", "/home/test/.Trash/demo-123-secure/demo", {
+    await expect(movePathToTrash(target)).resolves.toBe(expected);
+    expect(cpSync).toHaveBeenCalledWith(target, expected, {
      recursive: true,
      force: false,
      errorOnExist: true,
    });
-    expect(rmSync).toHaveBeenCalledWith("/tmp/demo", { recursive: true, force: false });
+    expect(rmSync).toHaveBeenCalledWith(target, { recursive: true, force: false });
  });

  it("retries copy fallback when the copy destination is created concurrently", async () => {
@@ -186,30 +221,21 @@ describe("browser maintenance", () => {
    const rmSync = vi.spyOn(fs, "rmSync").mockImplementation(() => undefined);

    const { movePathToTrash } = await import("./browser-maintenance.js");
+    const target = writeTrashTarget();
+    const first = path.join(homeDir, ".Trash", "demo-123-first", "demo");
+    const second = path.join(homeDir, ".Trash", "demo-123-second", "demo");

-    await expect(movePathToTrash("/tmp/demo")).resolves.toBe(
-      "/home/test/.Trash/demo-123-second/demo",
-    );
-    expect(cpSync).toHaveBeenNthCalledWith(
-      1,
-      "/tmp/demo",
-      "/home/test/.Trash/demo-123-first/demo",
-      {
-        recursive: true,
-        force: false,
-        errorOnExist: true,
-      },
-    );
-    expect(cpSync).toHaveBeenNthCalledWith(
-      2,
-      "/tmp/demo",
-      "/home/test/.Trash/demo-123-second/demo",
-      {
-        recursive: true,
-        force: false,
-        errorOnExist: true,
-      },
-    );
+    await expect(movePathToTrash(target)).resolves.toBe(second);
+    expect(cpSync).toHaveBeenNthCalledWith(1, target, first, {
+      recursive: true,
+      force: false,
+      errorOnExist: true,
+    });
+    expect(cpSync).toHaveBeenNthCalledWith(2, target, second, {
+      recursive: true,
+      force: false,
+      errorOnExist: true,
+    });
    expect(rmSync).toHaveBeenCalledTimes(1);
    expect(Date.now).toHaveBeenCalledTimes(1);
  });
@@ -226,20 +252,13 @@ describe("browser maintenance", () => {
      .mockImplementation(() => undefined);

    const { movePathToTrash } = await import("./browser-maintenance.js");
+    const target = writeTrashTarget();
+    const first = path.join(homeDir, ".Trash", "demo-123-first", "demo");
+    const second = path.join(homeDir, ".Trash", "demo-123-second", "demo");

-    await expect(movePathToTrash("/tmp/demo")).resolves.toBe(
-      "/home/test/.Trash/demo-123-second/demo",
-    );
-    expect(renameSync).toHaveBeenNthCalledWith(
-      1,
-      "/tmp/demo",
-      "/home/test/.Trash/demo-123-first/demo",
-    );
-    expect(renameSync).toHaveBeenNthCalledWith(
-      2,
-      "/tmp/demo",
-      "/home/test/.Trash/demo-123-second/demo",
-    );
+    await expect(movePathToTrash(target)).resolves.toBe(second);
+    expect(renameSync).toHaveBeenNthCalledWith(1, target, first);
+    expect(renameSync).toHaveBeenNthCalledWith(2, target, second);
    expect(Date.now).toHaveBeenCalledTimes(1);
  });
 });