fix: stop the delete guard breaking the documented layout, and follow symlinks when checking containment

src/build.ts

@@ -26,7 +26,7 @@ export async function build(options: BuildOptions = {}): Promise<Artifact[]> {
   const targets = options.target
     ? [options.target]
     : targetNames.filter((target) => project.config.targets[target]);
-  const guard = buildDeleteGuard(
+  const guard = await buildDeleteGuard(
     project.rootDir,
     project.config,
     project.configPath,

src/cleanup.ts

@@ -26,7 +26,7 @@ export async function prune(
   } = {},
 ): Promise<CleanupResult[]> {
   const project = await loadProjectConfig(options.cwd, options.configPath);
-  const guard = buildDeleteGuard(
+  const guard = await buildDeleteGuard(
     project.rootDir,
     project.config,
     project.configPath,
@@ -58,7 +58,7 @@ export async function clean(
   } = {},
 ): Promise<CleanupResult[]> {
   const project = await loadProjectConfig(options.cwd, options.configPath);
-  const guard = buildDeleteGuard(
+  const guard = await buildDeleteGuard(
     project.rootDir,
     project.config,
     project.configPath,

src/config.ts

@@ -161,6 +161,36 @@ async function discoverSourcePlugins(
   return plugins;
 }
 
+/**
+ * Directories under `sourceRoot` that are actually source plugins.
+ *
+ * The delete guard needs this to protect source without protecting generated
+ * output: the recommended layout writes every target's output under
+ * `plugins/<target>/`, which is also the default source-plugin root, so
+ * protecting that root wholesale refuses to prune the tool's own output. A
+ * shallow filesystem scan is deliberate — `clean` must keep working when the
+ * config or source tree no longer loads.
+ */
+export async function listSourcePluginDirs(
+  sourceRoot: string,
+): Promise<string[]> {
+  if (!(await exists(sourceRoot))) {
+    return [];
+  }
+  const dirs: string[] = [];
+  const entries = await fs.readdir(sourceRoot, { withFileTypes: true });
+  for (const entry of entries) {
+    if (!entry.isDirectory() || entry.name.startsWith(".")) {
+      continue;
+    }
+    const dir = path.join(sourceRoot, entry.name);
+    if (await isSourcePluginDir(dir)) {
+      dirs.push(dir);
+    }
+  }
+  return dirs;
+}
+
 /**
  * A source plugin dir declares a manifest or has at least one component dir.
  * This keeps generated target output (e.g. plugins/cursor/ in a single-repo

src/fs.ts

@@ -19,6 +19,61 @@ export async function walkFiles(dir: string): Promise<string[]> {
   return entries.sort();
 }
 
+/** Whether `candidate` is `root` itself or sits beneath it, comparing already-resolved paths. */
+export function isInside(root: string, candidate: string): boolean {
+  return candidate === root || candidate.startsWith(`${root}${path.sep}`);
+}
+
+/**
+ * Resolves `target` with symlinks followed, or returns `null` if the result
+ * escapes `root`.
+ *
+ * `path.resolve` is purely lexical, so a prefix test against it is satisfied by
+ * a path whose *intermediate* directory is a symlink pointing elsewhere —
+ * `<outDir>/link/file` passes the string check while landing wherever `link`
+ * points. Checking the final entry with `lstat` does not help either, since the
+ * final entry is an ordinary file.
+ *
+ * Both sides go through the same resolution, which matters more than it looks:
+ * resolving only the target and comparing against a lexical root rejects every
+ * ordinary write whenever any ancestor of the output directory is itself a
+ * symlink — `/tmp` is one on macOS — and does so precisely when the output
+ * directory does not exist yet, i.e. on a first build.
+ */
+export async function resolveInside(
+  root: string,
+  target: string,
+): Promise<string | null> {
+  const realRoot = await realpathDeep(root);
+  const realTarget = await realpathDeep(target);
+  return isInside(realRoot, realTarget) ? realTarget : null;
+}
+
+/**
+ * `fs.realpath` for a path that may not exist yet: resolves symlinks on the
+ * deepest ancestor that does exist, then re-attaches the missing tail.
+ */
+async function realpathDeep(target: string): Promise<string> {
+  const missing: string[] = [];
+  let current = path.resolve(target);
+  for (;;) {
+    try {
+      const real = await fs.realpath(current);
+      return path.join(real, ...missing.reverse());
+    } catch (error) {
+      if (!isNotFoundError(error)) {
+        throw error;
+      }
+      const parent = path.dirname(current);
+      if (parent === current) {
+        return path.resolve(target);
+      }
+      missing.push(path.basename(current));
+      current = parent;
+    }
+  }
+}
+
 /** Writes every file in `files` under `outDir`, refusing to write outside it. */
 export async function writeArtifact(
   outDir: string,
@@ -27,14 +82,18 @@ export async function writeArtifact(
   const resolvedOut = path.resolve(outDir);
   for (const [relativePath, value] of files) {
     const destination = path.resolve(outDir, relativePath);
-    if (
-      destination !== resolvedOut &&
-      !destination.startsWith(resolvedOut + path.sep)
-    ) {
+    if (!isInside(resolvedOut, destination)) {
       throw new Error(
         `Refusing to write outside the output directory: ${relativePath}`,
       );
     }
+    // The lexical check above cannot see through a symlinked intermediate
+    // directory, so confirm the real destination too, before creating anything.
+    if ((await resolveInside(resolvedOut, destination)) === null) {
+      throw new Error(
+        `Refusing to write through a symlink that leaves the output directory: ${relativePath}`,
+      );
+    }
     await fs.mkdir(path.dirname(destination), { recursive: true });
     await fs.writeFile(destination, value);
   }

src/managed.ts

@@ -1,6 +1,7 @@
 import { promises as fs } from "node:fs";
 import path from "node:path";
-import { json, toPosix } from "./fs.js";
+import { isInside, json, resolveInside, toPosix } from "./fs.js";
+import { listSourcePluginDirs } from "./config.js";
 import type {
   Artifact,
   CleanupEntry,
@@ -134,23 +135,37 @@ export async function cleanManagedFiles(
   return { target, outDir, entries };
 }
 
-/** Builds the guard that stops prune/clean from deleting paths inside the config's source tree. */
-export function buildDeleteGuard(
+/**
+ * Builds the guard that stops prune/clean from deleting the config or a source tree.
+ *
+ * `source.plugins` gets different treatment depending on whether it was set:
+ * when the user names a directory as their source-plugin root, that whole
+ * directory is protected. When they don't, the default root is `plugins/` —
+ * which is also where the recommended layout writes every target's *output*, so
+ * protecting it wholesale made the documented layout refuse to prune its own
+ * generated files after any source file was removed. Under the default, protect
+ * only the directories that really are source plugins.
+ */
+export async function buildDeleteGuard(
   rootDir: string,
   config: PluginpackConfig,
   configPath: string,
   force?: boolean,
-): DeleteGuard {
+): Promise<DeleteGuard> {
   const protectedRoots: string[] = [];
   if (config.source?.skills) {
     protectedRoots.push(path.resolve(rootDir, config.source.skills));
   }
-  // Mirrors loadConfig's default in src/config.ts so the source-plugin
-  // discovery root is always protected, whether or not it's written out
-  // explicitly in config.
-  protectedRoots.push(
-    path.resolve(rootDir, config.source?.plugins ?? "plugins"),
-  );
+  if (config.source?.partials) {
+    protectedRoots.push(path.resolve(rootDir, config.source.partials));
+  }
+  if (config.source?.plugins) {
+    protectedRoots.push(path.resolve(rootDir, config.source.plugins));
+  } else {
+    protectedRoots.push(
+      ...(await listSourcePluginDirs(path.resolve(rootDir, "plugins"))),
+    );
+  }
   return { protectedRoots, configPath: path.resolve(configPath), force };
 }
 
@@ -163,31 +178,65 @@ function assertNoProtectedDeletions(
   if (!guard || guard.force) {
     return;
   }
-  const blocked = paths.filter((file) =>
-    isProtectedDeletion(outDir, file, guard),
-  );
+  const blocked = paths
+    .map((file) => ({ file, root: protectingRoot(outDir, file, guard) }))
+    .filter(
+      (entry): entry is { file: string; root: string } => entry.root !== null,
+    );
   if (blocked.length === 0) {
     return;
   }
   throw new Error(
     `Refusing to ${command} ${blocked.length} path(s) that resolve inside your source tree or config:\n` +
-      `${blocked.map((file) => `  ${file}`).join("\n")}\n` +
+      `${blocked.map(({ file, root }) => `  ${file} -> resolves inside ${root}`).join("\n")}\n` +
       `This usually means a target outDir overlaps source.skills/source.plugins. ` +
       `Fix the config, or re-run with --force to delete anyway.`,
   );
 }
 
-function isProtectedDeletion(
+/**
+ * The protected root `relativePath` resolves inside, or `null` if it is safe to
+ * delete. Returns the matching root rather than a boolean so the refusal can
+ * name what it collided with — "resolves inside <path>" is actionable in a way
+ * that a bare list of refused paths is not.
+ *
+ * Comparison is case- and normalization-folded, not exact. On a case-insensitive
+ * filesystem (APFS, NTFS) `fs.rm` resolves `Skills/x` and `skills/x` to the same
+ * file, so an exact-match guard can be walked straight past by one letter of
+ * case — reachable from a single typo in `source.skills` that the OS forgives,
+ * or a case-only rename in git history. Folding over-protects on a
+ * case-sensitive host, which is the correct direction for a guard whose job is
+ * refusing to delete.
+ */
+function protectingRoot(
   outDir: string,
   relativePath: string,
   guard: DeleteGuard,
-): boolean {
+): string | null {
   const absolute = path.resolve(outDir, normalizeManagedPath(relativePath));
-  if (guard.configPath && absolute === guard.configPath) {
-    return true;
+  if (guard.configPath && pathsEqual(absolute, guard.configPath)) {
+    return guard.configPath;
   }
-  return guard.protectedRoots.some(
-    (root) => absolute === root || absolute.startsWith(`${root}${path.sep}`),
+  return (
+    guard.protectedRoots.find(
+      (root) => pathsEqual(absolute, root) || isUnder(absolute, root),
+    ) ?? null
+  );
+}
+
+/** Case- and normalization-folded form, for comparing paths the filesystem treats as equal. */
+function fold(value: string): string {
+  return value.normalize("NFC").toLowerCase();
+}
+
+function pathsEqual(a: string, b: string): boolean {
+  return a === b || fold(a) === fold(b);
+}
+
+function isUnder(candidate: string, root: string): boolean {
+  return (
+    candidate.startsWith(`${root}${path.sep}`) ||
+    fold(candidate).startsWith(`${fold(root)}${path.sep}`)
   );
 }
 
@@ -218,13 +267,9 @@ async function removeManagedPath(
   const root = path.resolve(outDir);
   const normalized = normalizeManagedPath(relativePath);
   const destination = path.resolve(root, normalized);
-  if (destination !== root && !destination.startsWith(`${root}${path.sep}`)) {
+  if (!isInside(root, destination)) {
     throw new Error(`Managed path escapes output directory: ${relativePath}`);
   }
-  // Defense in depth: fs.rm on a symlink unlinks the symlink itself rather
-  // than following it, so this isn't currently exploitable — but refuse
-  // outright if the entry is a symlink pointing outside `root`, rather than
-  // relying on that fs.rm behavior remaining true forever.
   let stats;
   try {
     stats = await fs.lstat(destination);
@@ -239,13 +284,30 @@ async function removeManagedPath(
       path.dirname(destination),
       await fs.readlink(destination),
     );
-    if (target !== root && !target.startsWith(`${root}${path.sep}`)) {
+    if (!isInside(root, target)) {
       throw new Error(
         `Refusing to remove a symlink pointing outside the output directory: ${relativePath}`,
       );
     }
   }
-  await fs.rm(destination, { force: true });
+  // The checks above are lexical, or inspect only the final entry. Neither sees
+  // a path whose *intermediate* directory is a symlink pointing elsewhere: the
+  // string test passes and the final entry is an ordinary file. Resolve the
+  // real destination as the catch-all before deleting.
+  if ((await resolveInside(root, destination)) === null) {
+    throw new Error(
+      `Refusing to remove a path that resolves outside the output directory: ${relativePath}`,
+    );
+  }
+  try {
+    await fs.rm(destination, { force: true });
+  } catch (error) {
+    throw new Error(
+      `Failed to remove managed path "${relativePath}" under ${outDir}: ${(error as Error).message}. ` +
+        `The managed manifest still lists every path, so re-running is safe once the cause is fixed.`,
+      { cause: error },
+    );
+  }
   await removeEmptyParents(path.dirname(destination), root);
 }