Fix fake sandbox cleanup of sealed runtime directories

packages/plugins/paperclip-plugin-fake-sandbox/src/lease-cleanup.test.ts

@@ -0,0 +1,127 @@
+import { randomUUID } from "node:crypto";
+import { access, chmod, mkdir, mkdtemp, readFile, rename, rm, stat, symlink, writeFile } from "node:fs/promises";
+import os from "node:os";
+import path from "node:path";
+import { describe, expect, it, vi } from "vitest";
+import { createEnvironmentTestHarness } from "@paperclipai/plugin-sdk/testing";
+import manifest from "./manifest.js";
+import plugin from "./plugin.js";
+
+vi.mock("node:fs/promises", async (importOriginal) => {
+  const original = await importOriginal<typeof import("node:fs/promises")>();
+  return { ...original, rm: vi.fn(original.rm) };
+});
+
+async function createLease() {
+  const definition = plugin.definition;
+  const harness = createEnvironmentTestHarness({
+    manifest,
+    environmentDriver: {
+      driverKey: "fake-plugin",
+      onAcquireLease: definition.onEnvironmentAcquireLease,
+      onReleaseLease: definition.onEnvironmentReleaseLease,
+      onInspectLease: definition.onEnvironmentInspectLease,
+      onDestroyLease: definition.onEnvironmentDestroyLease,
+    },
+  });
+  const base = {
+    driverKey: "fake-plugin",
+    companyId: "company-cleanup",
+    environmentId: randomUUID(),
+    config: { image: "fake:cleanup", reuseLease: false },
+  };
+  const lease = await harness.acquireLease({ ...base, runId: randomUUID() });
+  const root = lease.metadata?.fakeRootDir;
+  const providerLeaseId = lease.providerLeaseId;
+  if (typeof root !== "string") throw new Error("Fake lease did not identify its owned root");
+  if (typeof providerLeaseId !== "string") throw new Error("Fake lease did not identify its provider lease");
+  return { harness, root, params: { ...base, providerLeaseId } };
+}
+
+describe("fake sandbox lease cleanup", () => {
+  it.each(["destroy", "release"] as const)("%s removes sealed native runtime context", async (operation) => {
+    const { harness, root, params } = await createLease();
+    const instructions = path.join(root, "workspace/.paperclip-runtime/paperclip-runner/sessions/session/filesystem/context/instructions");
+    const nested = path.join(instructions, "nested");
+    await mkdir(nested, { recursive: true });
+    await writeFile(path.join(nested, "AGENTS.md"), "Assigned immutable instructions", { mode: 0o444 });
+    await chmod(nested, 0o555);
+    await chmod(instructions, 0o555);
+    try {
+      const result = operation === "destroy"
+        ? await harness.destroyLease(params)
+        : await harness.releaseLease(params);
+      expect(result).toBeUndefined();
+      await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" });
+      expect(await harness.inspectLease(params)).toMatchObject({ state: "destroyed" });
+    } finally {
+      await chmod(instructions, 0o700).catch(() => undefined);
+      await chmod(nested, 0o700).catch(() => undefined);
+      await rm(root, { recursive: true, force: true });
+    }
+  });
+
+  it("does not change a symlink target outside the lease tree", async () => {
+    const { harness, root, params } = await createLease();
+    const outside = await mkdtemp(path.join(os.tmpdir(), "paperclip-fake-cleanup-outside-"));
+    const target = path.join(outside, "sealed");
+    const file = path.join(target, "instructions.md");
+    await mkdir(target);
+    await writeFile(file, "Unrelated instructions", { mode: 0o444 });
+    await chmod(target, 0o500);
+    await symlink(target, path.join(root, "workspace/outside-context"), "dir");
+    const mode = (await stat(target)).mode;
+    try {
+      await harness.destroyLease(params);
+      await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" });
+      expect((await stat(target)).mode).toBe(mode);
+      expect(await readFile(file, "utf8")).toBe("Unrelated instructions");
+      expect((await stat(file)).mode & 0o777).toBe(0o444);
+    } finally {
+      await chmod(target, 0o700);
+      await rm(outside, { recursive: true, force: true });
+      await rm(root, { recursive: true, force: true });
+    }
+  });
+
+  it("retains a failed cleanup for inspection and retry", async () => {
+    const { harness, root, params } = await createLease();
+    const failure = Object.assign(new Error("Injected removal failure"), { code: "EACCES" });
+    vi.mocked(rm).mockRejectedValueOnce(failure);
+    try {
+      await expect(harness.destroyLease(params)).rejects.toBe(failure);
+      expect(await harness.inspectLease(params)).toMatchObject({ state: "active" });
+      expect((await stat(root)).isDirectory()).toBe(true);
+
+      await expect(harness.destroyLease(params)).resolves.toBeUndefined();
+      await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" });
+      expect(await harness.inspectLease(params)).toMatchObject({ state: "destroyed" });
+    } finally {
+      await rm(root, { recursive: true, force: true });
+    }
+  });
+
+  it("refuses a replaced root without changing it or forgetting the lease", async () => {
+    const { harness, root, params } = await createLease();
+    const retained = `${root}-retained`;
+    await rename(root, retained);
+    await mkdir(root);
+    const file = path.join(root, "unrelated.txt");
+    await writeFile(file, "Unrelated replacement");
+    await chmod(root, 0o500);
+    const mode = (await stat(root)).mode;
+    try {
+      await expect(harness.destroyLease(params)).rejects.toThrow("replaced fake sandbox lease root");
+      expect(await harness.inspectLease(params)).toMatchObject({ state: "active" });
+      expect((await stat(root)).mode).toBe(mode);
+      expect(await readFile(file, "utf8")).toBe("Unrelated replacement");
+    } finally {
+      await chmod(root, 0o700);
+      await rm(root, { recursive: true, force: true });
+      await rename(retained, root);
+      await harness.destroyLease(params);
+    }
+    await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" });
+    expect(await harness.inspectLease(params)).toMatchObject({ state: "destroyed" });
+  });
+});

packages/plugins/paperclip-plugin-fake-sandbox/src/lease-cleanup.ts

@@ -0,0 +1,40 @@
+import { constants } from "node:fs";
+import { open, readdir, rm } from "node:fs/promises";
+import path from "node:path";
+
+export interface LeaseRootIdentity {
+  dev: number;
+  ino: number;
+}
+
+async function makeDirectoriesWritable(directory: string, expected?: LeaseRootIdentity): Promise<boolean> {
+  const handle = await open(
+    directory,
+    constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW,
+  ).catch((error: NodeJS.ErrnoException) => {
+    if (error.code === "ENOENT") return null;
+    throw error;
+  });
+  if (!handle) return false;
+  try {
+    const identity = await handle.stat();
+    if (!identity.isDirectory() || (expected && (identity.dev !== expected.dev || identity.ino !== expected.ino))) {
+      throw new Error("Refusing to remove a replaced fake sandbox lease root");
+    }
+    // Native context directories are sealed. Restore deletion permissions on
+    // directories only; opening without following links protects outside targets.
+    await handle.chmod(0o700);
+    for (const entry of await readdir(directory, { withFileTypes: true })) {
+      if (entry.isDirectory()) await makeDirectoriesWritable(path.join(directory, entry.name));
+    }
+    return true;
+  } finally {
+    await handle.close();
+  }
+}
+
+export async function removeOwnedLeaseDirectory(rootDir: string, identity: LeaseRootIdentity): Promise<void> {
+  if (await makeDirectoriesWritable(rootDir, identity)) {
+    await rm(rootDir, { recursive: true, force: true });
+  }
+}

packages/plugins/paperclip-plugin-fake-sandbox/src/plugin.ts

@@ -1,9 +1,10 @@
 import { randomUUID } from "node:crypto";
-import { mkdir, mkdtemp, rm } from "node:fs/promises";
+import { lstat, mkdir, mkdtemp } from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
 import { spawn } from "node:child_process";
 import { definePlugin } from "@paperclipai/plugin-sdk";
+import { removeOwnedLeaseDirectory, type LeaseRootIdentity } from "./lease-cleanup.js";
 import type {
   PluginEnvironmentAcquireLeaseParams,
   PluginEnvironmentCancelInteractiveSetupParams,
@@ -40,6 +41,7 @@ interface FakeDriverConfig {
 interface FakeLeaseState {
   providerLeaseId: string;
   rootDir: string;
+  rootIdentity: LeaseRootIdentity;
   remoteCwd: string;
   image: string;
   reuseLease: boolean;
@@ -87,11 +89,13 @@ async function createLeaseState(input: {
   reuseLease: boolean;
 }): Promise<FakeLeaseState> {
   const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-fake-sandbox-"));
+  const { dev, ino } = await lstat(rootDir);
   const remoteCwd = path.join(rootDir, "workspace");
   await mkdir(remoteCwd, { recursive: true });
   const state = {
     providerLeaseId: input.providerLeaseId,
     rootDir,
+    rootIdentity: { dev, ino },
     remoteCwd,
     image: input.image,
     reuseLease: input.reuseLease,
@@ -183,9 +187,9 @@ function cancelStatusFromReason(
 async function removeLease(providerLeaseId: string | null | undefined): Promise<void> {
   if (!providerLeaseId) return;
   const state = leases.get(providerLeaseId);
-  leases.delete(providerLeaseId);
   if (state) {
-    await rm(state.rootDir, { recursive: true, force: true });
+    await removeOwnedLeaseDirectory(state.rootDir, state.rootIdentity);
+    leases.delete(providerLeaseId);
   }
 }