fix(openshell): pin local mirror fs mutations

2026-06-29 11:33:33 +00:00 · 2026-06-24 17:03:15 -05:00
parent f47fb91d29
commit c6f5725906
2 changed files with 446 additions and 9 deletions
--- a/extensions/openshell/src/fs-bridge.ts
+++ b/extensions/openshell/src/fs-bridge.ts
@@ -8,9 +8,8 @@ import type {
  SandboxResolvedPath,
 } from "openclaw/plugin-sdk/sandbox";
 import { createWritableRenameTargetResolver } from "openclaw/plugin-sdk/sandbox";
-import { isPathInside } from "openclaw/plugin-sdk/security-runtime";
+import { FsSafeError, isPathInside } from "openclaw/plugin-sdk/security-runtime";
 import type { OpenShellFsBridgeContext, OpenShellSandboxBackend } from "./backend.types.js";
-import { movePathWithCopyFallback } from "./mirror.js";

 type ResolvedMountPath = SandboxResolvedPath & {
  mountHostRoot: string;
@@ -18,6 +17,9 @@ type ResolvedMountPath = SandboxResolvedPath & {
  source: "workspace" | "agent" | "protectedSkill";
 };

+type FsSafeRoot = Awaited<ReturnType<typeof fsRoot>>;
+type FsSafeStat = Awaited<ReturnType<FsSafeRoot["stat"]>>;
+
 const MATERIALIZED_SKILLS_CONTAINER_PARTS = [".openclaw", "sandbox-skills", "skills"] as const;

 export function createOpenShellFsBridge(params: {
@@ -117,7 +119,7 @@ class OpenShellFsBridge implements SandboxFsBridge {
      allowFinalSymlinkForUnlink: false,
    });
    await this.backend.mkdirpRemotePath(target.containerPath, params.signal);
-    await fsPromises.mkdir(hostPath, { recursive: true });
+    await mkdirLocalRootPath({ hostPath, target });
  }

  async remove(params: {
@@ -141,9 +143,11 @@ class OpenShellFsBridge implements SandboxFsBridge {
      signal: params.signal,
      ignoreMissing: params.force !== false,
    });
-    await fsPromises.rm(hostPath, {
-      recursive: params.recursive ?? false,
-      force: params.force !== false,
+    await removeLocalRootPath({
+      force: params.force,
+      hostPath,
+      recursive: params.recursive,
+      target,
    });
  }

@@ -168,9 +172,17 @@ class OpenShellFsBridge implements SandboxFsBridge {
      allowMissingLeaf: true,
      allowFinalSymlinkForUnlink: false,
    });
+    await assertRenameSourceSupported(fromHostPath);
+    if (from.mountHostRoot !== to.mountHostRoot) {
+      throw new Error("OpenShell cross-root mirror renames require pinned fs-safe support");
+    }
+    await assertSameDeviceRenameSupported({
+      fromHostPath,
+      root: from.mountHostRoot,
+      toHostPath,
+    });
    await this.backend.renameRemotePath(from.containerPath, to.containerPath, params.signal);
-    await fsPromises.mkdir(path.dirname(toHostPath), { recursive: true });
-    await movePathWithCopyFallback({ from: fromHostPath, to: toHostPath });
+    await moveLocalRootPath({ from, fromHostPath, to, toHostPath });
  }

  async stat(params: {
@@ -343,6 +355,162 @@ class OpenShellFsBridge implements SandboxFsBridge {
  }
 }

+async function mkdirLocalRootPath(params: {
+  target: ResolvedMountPath;
+  hostPath: string;
+}): Promise<void> {
+  const relativePath = relativeToRoot(params.target, params.hostPath);
+  if (!relativePath) {
+    return;
+  }
+  const root = await fsRoot(params.target.mountHostRoot);
+  await root.mkdir(relativePath);
+}
+
+async function removeLocalRootPath(params: {
+  target: ResolvedMountPath;
+  hostPath: string;
+  recursive?: boolean;
+  force?: boolean;
+}): Promise<void> {
+  const root = await fsRoot(params.target.mountHostRoot);
+  const relativePath = relativeToRoot(params.target, params.hostPath);
+  try {
+    if (params.force === false) {
+      await fsPromises.lstat(params.hostPath);
+    }
+    if (params.recursive) {
+      const stats = await fsPromises.lstat(params.hostPath).catch((err: unknown) => {
+        if (isNotFoundError(err)) {
+          return null;
+        }
+        throw err;
+      });
+      if (stats?.isSymbolicLink()) {
+        await root.remove(relativePath);
+        return;
+      }
+      await removeRootTree(root, relativePath);
+      return;
+    }
+    await root.remove(relativePath);
+  } catch (err) {
+    if (params.force !== false && isNotFoundError(err)) {
+      return;
+    }
+    throw err;
+  }
+}
+
+async function removeRootTree(
+  root: FsSafeRoot,
+  relativePath: string,
+  knownStats?: FsSafeStat,
+): Promise<void> {
+  const stats = knownStats ?? (await root.stat(relativePath));
+  if (stats.isDirectory && !stats.isSymbolicLink) {
+    const entries = await root.list(relativePath, { withFileTypes: true });
+    for (const entry of entries) {
+      await removeRootTree(root, path.join(relativePath, entry.name), entry);
+    }
+    if (!relativePath) {
+      return;
+    }
+  }
+  await root.remove(relativePath);
+}
+
+async function moveLocalRootPath(params: {
+  from: ResolvedMountPath;
+  fromHostPath: string;
+  to: ResolvedMountPath;
+  toHostPath: string;
+}): Promise<void> {
+  const root = await fsRoot(params.from.mountHostRoot);
+  const fromRelativePath = relativeToRoot(params.from, params.fromHostPath);
+  const toRelativePath = relativeToRoot(params.to, params.toHostPath);
+  await mkdirParentPath(root, toRelativePath);
+  await root.move(fromRelativePath, toRelativePath, { overwrite: true });
+}
+
+async function mkdirParentPath(root: FsSafeRoot, relativePath: string): Promise<void> {
+  const parentPath = path.dirname(relativePath);
+  if (parentPath === "." || parentPath === "") {
+    return;
+  }
+  await root.mkdir(parentPath);
+}
+
+function relativeToRoot(target: ResolvedMountPath, hostPath: string): string {
+  const relativePath = path.relative(target.mountHostRoot, hostPath);
+  return relativePath === "." ? "" : relativePath;
+}
+
+async function assertRenameSourceSupported(fromHostPath: string): Promise<void> {
+  const stats = await fsPromises.lstat(fromHostPath);
+  if (stats.isSymbolicLink()) {
+    throw new Error("Sandbox symlink rename sources are not supported by the local mirror bridge");
+  }
+  if (stats.isFile() && stats.nlink > 1) {
+    throw new Error(
+      "Sandbox hardlinked rename sources are not supported by the local mirror bridge",
+    );
+  }
+}
+
+async function assertSameDeviceRenameSupported(params: {
+  fromHostPath: string;
+  root: string;
+  toHostPath: string;
+}): Promise<void> {
+  const sourceStats = await fsPromises.lstat(params.fromHostPath);
+  const destinationParentStats = await nearestExistingDirectoryStats({
+    root: params.root,
+    targetPath: path.dirname(params.toHostPath),
+  });
+  if (sourceStats.dev !== destinationParentStats.dev) {
+    throw new Error("OpenShell cross-device mirror renames require pinned fs-safe support");
+  }
+}
+
+async function nearestExistingDirectoryStats(params: {
+  root: string;
+  targetPath: string;
+}): Promise<Awaited<ReturnType<typeof fsPromises.lstat>>> {
+  const rootPath = path.resolve(params.root);
+  let cursor = path.resolve(params.targetPath);
+  while (isPathInside(rootPath, cursor)) {
+    const stats = await fsPromises.lstat(cursor).catch((err: unknown) => {
+      if (isNotFoundError(err)) {
+        return null;
+      }
+      throw err;
+    });
+    if (stats) {
+      if (!stats.isDirectory()) {
+        throw new Error(`Sandbox rename destination parent is not a directory: ${cursor}`);
+      }
+      return stats;
+    }
+    const next = path.dirname(cursor);
+    if (next === cursor) {
+      break;
+    }
+    cursor = next;
+  }
+  return await fsPromises.lstat(rootPath);
+}
+
+function isNotFoundError(err: unknown): boolean {
+  return (
+    (err instanceof FsSafeError && err.code === "not-found") ||
+    (typeof err === "object" &&
+      err !== null &&
+      "code" in err &&
+      (err as { code?: unknown }).code === "ENOENT")
+  );
+}
+
 function resolveProtectedSkillTarget(params: {
  input: string;
  skillsRoot: string;
@@ -421,7 +589,11 @@ async function assertLocalPathSafety(params: {
  const canonicalRoot = await fsPromises
    .realpath(params.root)
    .catch(() => path.resolve(params.root));
-  const candidate = await resolveCanonicalCandidate(params.target.hostPath);
+  const targetStats = await fsPromises.lstat(params.target.hostPath).catch(() => null);
+  const candidate =
+    params.allowFinalSymlinkForUnlink && targetStats?.isSymbolicLink()
+      ? path.resolve(canonicalRoot, path.relative(params.root, params.target.hostPath))
+      : await resolveCanonicalCandidate(params.target.hostPath);
  if (!isPathInside(canonicalRoot, candidate)) {
    throw new Error(
      `Sandbox path escapes allowed mounts; cannot access: ${params.target.containerPath}`,
--- a/extensions/openshell/src/openshell-core.test.ts
+++ b/extensions/openshell/src/openshell-core.test.ts
@@ -733,6 +733,90 @@ describe("openshell fs bridges", () => {
    expect(backend["runRemoteShellScript"]).not.toHaveBeenCalled();
  });

+  it("rejects cross-root mirror renames before the remote backend commit", async () => {
+    const workspaceDir = await makeTempDir("openclaw-openshell-fs-");
+    const agentWorkspaceDir = await makeTempDir("openclaw-openshell-agent-fs-");
+    const sourcePath = path.join(workspaceDir, "source.txt");
+    await fs.writeFile(sourcePath, "payload", "utf8");
+    const backend = createMirrorBackendMock();
+    const sandbox = createSandboxTestContext({
+      overrides: {
+        backendId: "openshell",
+        workspaceDir,
+        agentWorkspaceDir,
+        containerWorkdir: "/sandbox",
+      },
+    });
+
+    const { createOpenShellFsBridge } = await import("./fs-bridge.js");
+    const bridge = createOpenShellFsBridge({ sandbox, backend });
+
+    await expect(bridge.rename({ from: "source.txt", to: "/agent/source.txt" })).rejects.toThrow(
+      "OpenShell cross-root mirror renames require pinned fs-safe support",
+    );
+    expect(backend["renameRemotePath"]).not.toHaveBeenCalled();
+    await expect(fs.readFile(sourcePath, "utf8")).resolves.toBe("payload");
+    await expectPathMissing(path.join(agentWorkspaceDir, "source.txt"));
+    await expect(fs.readdir(agentWorkspaceDir)).resolves.toStrictEqual([]);
+  });
+
+  it.runIf(process.platform !== "win32")(
+    "rejects local mirror symlink rename sources before the remote backend commit",
+    async () => {
+      const workspaceDir = await makeTempDir("openclaw-openshell-fs-");
+      await fs.writeFile(path.join(workspaceDir, "target.txt"), "payload", "utf8");
+      await fs.symlink("target.txt", path.join(workspaceDir, "link.txt"));
+      const backend = createMirrorBackendMock();
+      const sandbox = createSandboxTestContext({
+        overrides: {
+          backendId: "openshell",
+          workspaceDir,
+          agentWorkspaceDir: workspaceDir,
+          containerWorkdir: "/sandbox",
+        },
+      });
+
+      const { createOpenShellFsBridge } = await import("./fs-bridge.js");
+      const bridge = createOpenShellFsBridge({ sandbox, backend });
+
+      await expect(bridge.rename({ from: "link.txt", to: "moved-link.txt" })).rejects.toThrow(
+        "Sandbox symlink rename sources are not supported",
+      );
+      expect(backend["renameRemotePath"]).not.toHaveBeenCalled();
+      await expect(fs.readlink(path.join(workspaceDir, "link.txt"))).resolves.toBe("target.txt");
+      await expectPathMissing(path.join(workspaceDir, "moved-link.txt"));
+    },
+  );
+
+  it.runIf(process.platform !== "win32")(
+    "rejects local mirror hardlinked rename sources before the remote backend commit",
+    async () => {
+      const workspaceDir = await makeTempDir("openclaw-openshell-fs-");
+      const sourcePath = path.join(workspaceDir, "source.txt");
+      await fs.writeFile(sourcePath, "payload", "utf8");
+      await fs.link(sourcePath, path.join(workspaceDir, "other-link.txt"));
+      const backend = createMirrorBackendMock();
+      const sandbox = createSandboxTestContext({
+        overrides: {
+          backendId: "openshell",
+          workspaceDir,
+          agentWorkspaceDir: workspaceDir,
+          containerWorkdir: "/sandbox",
+        },
+      });
+
+      const { createOpenShellFsBridge } = await import("./fs-bridge.js");
+      const bridge = createOpenShellFsBridge({ sandbox, backend });
+
+      await expect(bridge.rename({ from: "source.txt", to: "moved.txt" })).rejects.toThrow(
+        "Sandbox hardlinked rename sources are not supported",
+      );
+      expect(backend["renameRemotePath"]).not.toHaveBeenCalled();
+      await expect(fs.readFile(sourcePath, "utf8")).resolves.toBe("payload");
+      await expectPathMissing(path.join(workspaceDir, "moved.txt"));
+    },
+  );
+
  it("removes remote mirror paths through the pinned backend operation", async () => {
    const workspaceDir = await makeTempDir("openclaw-openshell-fs-");
    await fs.writeFile(path.join(workspaceDir, "target.txt"), "payload", "utf8");
@@ -759,6 +843,187 @@ describe("openshell fs bridges", () => {
    expect(backend["runRemoteShellScript"]).not.toHaveBeenCalled();
  });

+  it("removes recursive local mirror directories without raw path deletion", async () => {
+    const workspaceDir = await makeTempDir("openclaw-openshell-fs-");
+    await fs.mkdir(path.join(workspaceDir, "nested", "child"), { recursive: true });
+    await fs.writeFile(path.join(workspaceDir, "nested", "child", "target.txt"), "payload", "utf8");
+    const backend = createMirrorBackendMock();
+    const sandbox = createSandboxTestContext({
+      overrides: {
+        backendId: "openshell",
+        workspaceDir,
+        agentWorkspaceDir: workspaceDir,
+        containerWorkdir: "/sandbox",
+      },
+    });
+
+    const { createOpenShellFsBridge } = await import("./fs-bridge.js");
+    const bridge = createOpenShellFsBridge({ sandbox, backend });
+    await bridge.remove({ filePath: "nested", recursive: true, force: true });
+
+    await expectPathMissing(path.join(workspaceDir, "nested"));
+    expect(backend["removeRemotePath"]).toHaveBeenCalledWith("/sandbox/nested", {
+      recursive: true,
+      signal: undefined,
+      ignoreMissing: true,
+    });
+  });
+
+  it.runIf(process.platform !== "win32")(
+    "removes recursive local mirror directories containing symlink leaves without following them",
+    async () => {
+      const workspaceDir = await makeTempDir("openclaw-openshell-fs-");
+      const outsideDir = await makeTempDir("openclaw-openshell-outside-");
+      const outsideTarget = path.join(outsideDir, "target.txt");
+      await fs.mkdir(path.join(workspaceDir, "nested"), { recursive: true });
+      await fs.writeFile(outsideTarget, "outside", "utf8");
+      await fs.symlink(outsideTarget, path.join(workspaceDir, "nested", "link.txt"));
+      const backend = createMirrorBackendMock();
+      const sandbox = createSandboxTestContext({
+        overrides: {
+          backendId: "openshell",
+          workspaceDir,
+          agentWorkspaceDir: workspaceDir,
+          containerWorkdir: "/sandbox",
+        },
+      });
+
+      const { createOpenShellFsBridge } = await import("./fs-bridge.js");
+      const bridge = createOpenShellFsBridge({ sandbox, backend });
+      await bridge.remove({ filePath: "nested", recursive: true, force: true });
+
+      await expectPathMissing(path.join(workspaceDir, "nested"));
+      await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("outside");
+    },
+  );
+
+  it.runIf(process.platform !== "win32")(
+    "removes local mirror symlink leaves when force is false",
+    async () => {
+      const workspaceDir = await makeTempDir("openclaw-openshell-fs-");
+      const outsideDir = await makeTempDir("openclaw-openshell-outside-");
+      const outsideTarget = path.join(outsideDir, "target.txt");
+      await fs.writeFile(outsideTarget, "outside", "utf8");
+      await fs.symlink(outsideTarget, path.join(workspaceDir, "link.txt"));
+      const backend = createMirrorBackendMock();
+      const sandbox = createSandboxTestContext({
+        overrides: {
+          backendId: "openshell",
+          workspaceDir,
+          agentWorkspaceDir: workspaceDir,
+          containerWorkdir: "/sandbox",
+        },
+      });
+
+      const { createOpenShellFsBridge } = await import("./fs-bridge.js");
+      const bridge = createOpenShellFsBridge({ sandbox, backend });
+      await bridge.remove({ filePath: "link.txt", force: false });
+
+      await expectPathMissing(path.join(workspaceDir, "link.txt"));
+      await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("outside");
+      expect(backend["removeRemotePath"]).toHaveBeenCalledWith("/sandbox/link.txt", {
+        recursive: false,
+        signal: undefined,
+        ignoreMissing: false,
+      });
+    },
+  );
+
+  it.runIf(process.platform !== "win32")(
+    "rejects local mirror mkdir when a validated parent is swapped to an outside symlink",
+    async () => {
+      const workspaceDir = await makeTempDir("openclaw-openshell-fs-");
+      const outsideDir = await makeTempDir("openclaw-openshell-outside-");
+      const slotPath = path.join(workspaceDir, "slot");
+      await fs.mkdir(slotPath, { recursive: true });
+      const backend = createMirrorBackendMock();
+      backend["mkdirpRemotePath"] = vi.fn().mockImplementation(async () => {
+        await fs.rm(slotPath, { recursive: true, force: true });
+        await fs.symlink(outsideDir, slotPath);
+      });
+      const sandbox = createSandboxTestContext({
+        overrides: {
+          backendId: "openshell",
+          workspaceDir,
+          agentWorkspaceDir: workspaceDir,
+          containerWorkdir: "/sandbox",
+        },
+      });
+
+      const { createOpenShellFsBridge } = await import("./fs-bridge.js");
+      const bridge = createOpenShellFsBridge({ sandbox, backend });
+
+      await expect(bridge.mkdirp({ filePath: "slot/escaped" })).rejects.toThrow();
+      await expectPathMissing(path.join(outsideDir, "escaped"));
+    },
+  );
+
+  it.runIf(process.platform !== "win32")(
+    "rejects local mirror remove when a validated parent is swapped to an outside symlink",
+    async () => {
+      const workspaceDir = await makeTempDir("openclaw-openshell-fs-");
+      const outsideDir = await makeTempDir("openclaw-openshell-outside-");
+      const slotPath = path.join(workspaceDir, "slot");
+      const outsideTarget = path.join(outsideDir, "target.txt");
+      await fs.mkdir(slotPath, { recursive: true });
+      await fs.writeFile(path.join(slotPath, "target.txt"), "inside", "utf8");
+      await fs.writeFile(outsideTarget, "outside", "utf8");
+      const backend = createMirrorBackendMock();
+      backend["removeRemotePath"] = vi.fn().mockImplementation(async () => {
+        await fs.rm(slotPath, { recursive: true, force: true });
+        await fs.symlink(outsideDir, slotPath);
+      });
+      const sandbox = createSandboxTestContext({
+        overrides: {
+          backendId: "openshell",
+          workspaceDir,
+          agentWorkspaceDir: workspaceDir,
+          containerWorkdir: "/sandbox",
+        },
+      });
+
+      const { createOpenShellFsBridge } = await import("./fs-bridge.js");
+      const bridge = createOpenShellFsBridge({ sandbox, backend });
+
+      await expect(bridge.remove({ filePath: "slot/target.txt", force: true })).rejects.toThrow();
+      await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("outside");
+    },
+  );
+
+  it.runIf(process.platform !== "win32")(
+    "rejects local mirror rename when a validated destination parent is swapped to an outside symlink",
+    async () => {
+      const workspaceDir = await makeTempDir("openclaw-openshell-fs-");
+      const outsideDir = await makeTempDir("openclaw-openshell-outside-");
+      const slotPath = path.join(workspaceDir, "slot");
+      const sourcePath = path.join(workspaceDir, "source.txt");
+      await fs.mkdir(slotPath, { recursive: true });
+      await fs.writeFile(sourcePath, "payload", "utf8");
+      const backend = createMirrorBackendMock();
+      backend["renameRemotePath"] = vi.fn().mockImplementation(async () => {
+        await fs.rm(slotPath, { recursive: true, force: true });
+        await fs.symlink(outsideDir, slotPath);
+      });
+      const sandbox = createSandboxTestContext({
+        overrides: {
+          backendId: "openshell",
+          workspaceDir,
+          agentWorkspaceDir: workspaceDir,
+          containerWorkdir: "/sandbox",
+        },
+      });
+
+      const { createOpenShellFsBridge } = await import("./fs-bridge.js");
+      const bridge = createOpenShellFsBridge({ sandbox, backend });
+
+      await expect(
+        bridge.rename({ from: "source.txt", to: "slot/parent/moved.txt" }),
+      ).rejects.toThrow();
+      await expect(fs.readFile(sourcePath, "utf8")).resolves.toBe("payload");
+      await expectPathMissing(path.join(outsideDir, "parent", "moved.txt"));
+    },
+  );
+
  it("keeps local mirror state unchanged when remote pinned mkdir is rejected", async () => {
    const workspaceDir = await makeTempDir("openclaw-openshell-fs-");
    const backend = createMirrorBackendMock();