Clean immutable cache directories from isolated test homes safely

Co-Authored-By: Paperclip <noreply@paperclip.ing>

server/src/__tests__/helpers/isolated-test-home.ts

@@ -0,0 +1,36 @@
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+/** Own one test-created home, including immutable runtime cache directories. */
+export function createIsolatedTestHome() {
+  // Capture before suites install filesystem spies: worker-exit cleanup runs
+  // after their teardown and must not retain a mocked filesystem operation.
+  const { mkdtempSync, lstatSync, openSync, fchmodSync, closeSync, readdirSync, rmSync, constants } = fs;
+  const home = mkdtempSync(path.join(os.tmpdir(), "paperclip-vitest-home-"));
+  const identity = lstatSync(home);
+  const cleanup = () => {
+    let current;
+    try { current = lstatSync(home); }
+    catch (error) {
+      if ((error as NodeJS.ErrnoException).code === "ENOENT") return;
+      throw error;
+    }
+    if (!current.isDirectory() || current.dev !== identity.dev || current.ino !== identity.ino) {
+      throw new Error("Refusing to clean a replaced isolated test home");
+    }
+    const makeDirectoriesWritable = (directory: string) => {
+      // Open the directory itself without following a leaf symlink. Only its
+      // permissions change; read-only files and external symlink targets do not.
+      const fd = openSync(directory, constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW);
+      try { fchmodSync(fd, 0o700); }
+      finally { closeSync(fd); }
+      for (const entry of readdirSync(directory, { withFileTypes: true })) {
+        if (entry.isDirectory()) makeDirectoriesWritable(path.join(directory, entry.name));
+      }
+    };
+    makeDirectoriesWritable(home);
+    rmSync(home, { recursive: true, force: true, maxRetries: 8, retryDelay: 50 });
+  };
+  return { home, cleanup };
+}

server/src/__tests__/isolated-test-home.test.ts

@@ -0,0 +1,62 @@
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { execFileSync } from "node:child_process";
+import { fileURLToPath } from "node:url";
+import { createRequire } from "node:module";
+import { afterEach, expect, it, vi } from "vitest";
+import { createIsolatedTestHome } from "./helpers/isolated-test-home.js";
+
+afterEach(() => vi.restoreAllMocks());
+
+it("removes immutable cache directories at process exit even after filesystem spies change", () => {
+  const helper = fileURLToPath(new URL("./helpers/isolated-test-home.ts", import.meta.url));
+  const result = execFileSync(process.execPath, ["--import", createRequire(import.meta.url).resolve("tsx"), "--input-type=module", "-e", `
+    import fs from 'node:fs';
+    import path from 'node:path';
+    const { createIsolatedTestHome } = await import(process.argv[1]);
+    const owned = createIsolatedTestHome();
+    const cache = path.join(owned.home, 'runtime-skills', 'immutable');
+    fs.mkdirSync(cache, { recursive: true });
+    fs.writeFileSync(path.join(cache, 'SKILL.md'), 'read-only fixture');
+    fs.chmodSync(path.join(cache, 'SKILL.md'), 0o444);
+    fs.chmodSync(cache, 0o555);
+    process.once('exit', owned.cleanup);
+    fs.rmSync = () => { throw Object.assign(new Error('simulated read-only filesystem'), { code: 'EACCES' }); };
+    console.log(owned.home);
+  `, helper], { encoding: "utf8" });
+  const home = result.trim();
+  expect(path.basename(home)).toMatch(/^paperclip-vitest-home-/);
+  expect(fs.existsSync(home)).toBe(false);
+});
+
+it("never follows links from an owned home into another directory", () => {
+  const owned = createIsolatedTestHome();
+  const outside = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-cleanup-outside-"));
+  try {
+    fs.writeFileSync(path.join(outside, "private"), "retained");
+    fs.chmodSync(outside, 0o555);
+    fs.symlinkSync(outside, path.join(owned.home, "external"));
+    owned.cleanup();
+    expect(fs.existsSync(owned.home)).toBe(false);
+    expect(fs.readFileSync(path.join(outside, "private"), "utf8")).toBe("retained");
+    expect(fs.statSync(outside).mode & 0o777).toBe(0o555);
+  } finally {
+    fs.chmodSync(outside, 0o700);
+    fs.rmSync(outside, { recursive: true, force: true });
+  }
+});
+
+it("refuses a replaced home instead of deleting its replacement", () => {
+  const owned = createIsolatedTestHome();
+  const retained = `${owned.home}-retained`;
+  fs.renameSync(owned.home, retained);
+  fs.symlinkSync(retained, owned.home);
+  try {
+    expect(owned.cleanup).toThrow("Refusing to clean a replaced isolated test home");
+    expect(fs.statSync(retained).isDirectory()).toBe(true);
+  } finally {
+    fs.unlinkSync(owned.home);
+    fs.rmSync(retained, { recursive: true, force: true });
+  }
+});

server/src/__tests__/setup-supertest.ts

@@ -1,4 +1,5 @@
 import fs from "node:fs";
+import { createIsolatedTestHome } from "./helpers/isolated-test-home.js";
 import { createRequire } from "node:module";
 import type { AddressInfo, Server as NetServer } from "node:net";
 import os from "node:os";
@@ -25,11 +26,11 @@ type SupertestTestConstructor = {
 process.env.PAPERCLIP_TEST_HOST_HOME ??= process.env.PAPERCLIP_HOME?.trim() || path.join(os.homedir(), ".paperclip");
 
 // Receipt-spool recovery must never scan a developer's instance during tests.
-const paperclipTestHome = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-vitest-home-"));
+const { home: paperclipTestHome, cleanup: cleanupPaperclipTestHome } = createIsolatedTestHome();
 process.env.PAPERCLIP_HOME = paperclipTestHome;
 // Setup-file afterAll hooks run before suites drain asynchronous heartbeats.
 // Cleanup at worker exit, after all suite teardown, avoids deleting live state.
-process.once("exit", () => fs.rmSync(paperclipTestHome, { recursive: true, force: true, maxRetries: 8, retryDelay: 50 }));
+process.once("exit", cleanupPaperclipTestHome);
 
 const require = createRequire(import.meta.url);
 const SupertestTest = require("supertest/lib/test.js") as SupertestTestConstructor;