fix(skills): guard canonical stores from mirror aliases

packages/cli/src/commands/skills.test.ts

@@ -119,7 +119,7 @@ vi.mock("../utils/skillsManifest.js", async (importOriginal) => {
 // the real $HOME. Stub it so these arg-shape tests never create symlinks in the
 // dev machine's agent dirs — the mirror has its own isolated-HOME unit tests.
 vi.mock("../utils/skillsMirror.js", () => ({
-  mirrorGlobalSkills: vi.fn(() => ({ source: null, mirrored: [] })),
+  mirrorGlobalSkills: vi.fn(() => ({ source: null, mirrored: [], skipped: [] })),
 }));
 
 // The reconcile commands drop the background nudge's cached verdict on

packages/cli/src/commands/skills.ts

@@ -176,7 +176,7 @@ function mirrorToInstalledAgents(): void {
   try {
     const names = hyperframesSkillNames({ scope: "global" });
     if (names.length === 0) return;
-    const { mirrored } = mirrorGlobalSkills({ skills: names });
+    const { mirrored, skipped } = mirrorGlobalSkills({ skills: names });
     const n = mirrored.length;
     if (n > 0) {
       // stderr (via diag): reachable from `skills update --json` (via installSkills)
@@ -185,6 +185,14 @@ function mirrorToInstalledAgents(): void {
         c.dim(`Linked skills into ${n} other agent ${n === 1 ? "directory" : "directories"}.`),
       );
     }
+    if (skipped.length > 0) {
+      const agents = [...new Set(skipped.map((entry) => entry.agent))].join(", ");
+      diag.warn(
+        c.warn(
+          `Skipped unsafe skill mirror target${skipped.length === 1 ? "" : "s"} for ${agents}; canonical skill stores were left unchanged.`,
+        ),
+      );
+    }
   } catch {
     // best-effort
   }

packages/cli/src/utils/skillsMirror.test.ts

@@ -8,6 +8,7 @@ import {
   readFileSync,
   realpathSync,
   rmSync,
+  symlinkSync,
   writeFileSync,
 } from "node:fs";
 import { tmpdir } from "node:os";
@@ -46,6 +47,62 @@ afterEach(() => {
 });
 
 describe("mirrorGlobalSkills", () => {
+  it.each([
+    [
+      "absolute",
+      (home: string, source: string) => symlinkSync(source, join(home, ".cursor", "skills")),
+    ],
+    [
+      "relative",
+      (home: string) => symlinkSync("../.claude/skills", join(home, ".cursor", "skills")),
+    ],
+    ["intermediate", (home: string) => symlinkSync(".claude", join(home, ".cursor"))],
+  ])(
+    "fails closed when the target reaches the canonical store through a %s alias",
+    (_kind, alias) => {
+      const home = makeHome();
+      seedStore(home, ["hyperframes"]);
+      const source = join(home, ".claude", "skills");
+      if (_kind !== "intermediate") installMarker(home, ".cursor");
+      alias(home, source);
+
+      const result = mirrorGlobalSkills({
+        skills: ["hyperframes"],
+        home,
+        platform: "linux",
+        env: ENV,
+      });
+
+      expect(lstatSync(join(source, "hyperframes")).isDirectory()).toBe(true);
+      expect(readFileSync(join(source, "hyperframes", "SKILL.md"), "utf8")).toBe("# hyperframes\n");
+      expect(result.mirrored.map((entry) => entry.agent)).not.toContain("cursor");
+      expect(result.skipped).toContainEqual(
+        expect.objectContaining({ agent: "cursor", reason: "aliases_install_owned_store" }),
+      );
+    },
+  );
+
+  it("fails closed and reports an unresolvable self-loop before destructive mirroring", () => {
+    const home = makeHome();
+    seedStore(home, ["hyperframes"]);
+    installMarker(home, ".cursor");
+    symlinkSync("skills", join(home, ".cursor", "skills"));
+
+    const result = mirrorGlobalSkills({
+      skills: ["hyperframes"],
+      home,
+      platform: "linux",
+      env: ENV,
+    });
+
+    expect(readFileSync(join(home, ".claude", "skills", "hyperframes", "SKILL.md"), "utf8")).toBe(
+      "# hyperframes\n",
+    );
+    expect(result.skipped).toContainEqual(
+      expect.objectContaining({ agent: "cursor", reason: "unresolvable_target" }),
+    );
+  });
+
   it("no-ops when there is no global Claude store", () => {
     const home = makeHome();
     const result = mirrorGlobalSkills({

packages/cli/src/utils/skillsMirror.ts

@@ -22,9 +22,19 @@
 // uses (XDG_CONFIG_HOME, CODEX_HOME, CLAUDE_CONFIG_DIR, …), so a machine with
 // those set mirrors into the exact dir the agent reads.
 
-import { cpSync, existsSync, mkdirSync, readdirSync, rmSync, symlinkSync } from "node:fs";
+import {
+  cpSync,
+  existsSync,
+  lstatSync,
+  mkdirSync,
+  readdirSync,
+  realpathSync,
+  rmSync,
+  statSync,
+  symlinkSync,
+} from "node:fs";
 import { homedir } from "node:os";
-import { dirname, isAbsolute, join, relative } from "node:path";
+import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "node:path";
 import { AGENT_GLOBAL_DIRS, type AgentDirBase } from "./agentDirs.generated.js";
 
 /**
@@ -48,6 +58,101 @@ export interface MirrorResult {
   source: string | null;
   /** Agents whose global dir was (re)populated. */
   mirrored: { agent: string; dir: string }[];
+  /** Agent targets skipped because their filesystem identity was unsafe. */
+  skipped: {
+    agent: string;
+    dir: string;
+    reason: "aliases_install_owned_store" | "unresolvable_target";
+  }[];
+}
+
+type MirrorSkipReason = MirrorResult["skipped"][number]["reason"];
+
+/** Resolve a path through existing ancestors without creating its missing tail. */
+function canonicalCandidate(input: string): string | null {
+  let current = resolve(input);
+  const missing: string[] = [];
+  while (true) {
+    try {
+      lstatSync(current);
+      break;
+    } catch (error) {
+      const code = (error as NodeJS.ErrnoException).code;
+      if (code !== "ENOENT" && code !== "ENOTDIR") return null;
+      const parent = dirname(current);
+      if (parent === current) return null;
+      missing.unshift(basename(current));
+      current = parent;
+    }
+  }
+  try {
+    return resolve(realpathSync(current), ...missing);
+  } catch {
+    return null;
+  }
+}
+
+function pathsOverlap(left: string, right: string): boolean {
+  const fromLeft = relative(left, right);
+  const leftContainsRight =
+    fromLeft === "" ||
+    (fromLeft !== ".." && !fromLeft.startsWith(`..${sep}`) && !isAbsolute(fromLeft));
+  if (leftContainsRight) return true;
+  const fromRight = relative(right, left);
+  return fromRight !== ".." && !fromRight.startsWith(`..${sep}`) && !isAbsolute(fromRight);
+}
+
+function sameExistingNode(left: string, right: string): boolean {
+  try {
+    const leftStat = statSync(left);
+    const rightStat = statSync(right);
+    return leftStat.dev === rightStat.dev && leftStat.ino === rightStat.ino;
+  } catch {
+    return false;
+  }
+}
+
+function targetSafety(
+  target: string,
+  protectedPaths: ReadonlyArray<{ lexical: string; canonical: string }>,
+): MirrorSkipReason | null {
+  const canonical = canonicalCandidate(target);
+  if (!canonical) return "unresolvable_target";
+  for (const protectedPath of protectedPaths) {
+    if (
+      pathsOverlap(canonical, protectedPath.canonical) ||
+      sameExistingNode(target, protectedPath.lexical)
+    ) {
+      return "aliases_install_owned_store";
+    }
+  }
+  return null;
+}
+
+function skillTargetSafety(
+  sourceSkill: string,
+  targetSkill: string,
+  targetDirSafety: () => MirrorSkipReason | null,
+): MirrorSkipReason | null {
+  const directoryReason = targetDirSafety();
+  if (directoryReason) return directoryReason;
+  try {
+    // A final symlink is the normal Unix mirror shape. rmSync unlinks it
+    // without traversing its target, so canonical equality is safe here.
+    if (lstatSync(targetSkill).isSymbolicLink()) return null;
+  } catch (error) {
+    if ((error as NodeJS.ErrnoException).code !== "ENOENT") return "unresolvable_target";
+  }
+  const sourceCanonical = canonicalCandidate(sourceSkill);
+  const targetCanonical = canonicalCandidate(targetSkill);
+  if (!sourceCanonical || !targetCanonical) return "unresolvable_target";
+  if (
+    pathsOverlap(sourceCanonical, targetCanonical) ||
+    sameExistingNode(sourceSkill, targetSkill)
+  ) {
+    return "aliases_install_owned_store";
+  }
+  return null;
 }
 
 /** Resolve each env-overridable base dir exactly as upstream agents.ts does. */
@@ -78,39 +183,57 @@ function listSkillDirs(store: string): string[] {
  * copy, or a previous install) is removed first so the mirror always reflects
  * the canonical store — that's the whole point of "update".
  */
-function linkOrCopy(sourceSkill: string, targetSkill: string, platform: NodeJS.Platform): void {
+function linkOrCopy(
+  sourceSkill: string,
+  targetSkill: string,
+  platform: NodeJS.Platform,
+  safety: () => MirrorSkipReason | null,
+): MirrorSkipReason | null {
+  const unsafe = safety();
+  if (unsafe) return unsafe;
   rmSync(targetSkill, { recursive: true, force: true });
   if (platform === "win32") {
     cpSync(sourceSkill, targetSkill, { recursive: true });
   } else {
     symlinkSync(relative(dirname(targetSkill), sourceSkill), targetSkill);
   }
+  return null;
 }
 
 /**
  * Populate one agent's global dir from the store. Best-effort and idempotent;
- * per-skill failures don't abort the others. Returns false if the dir couldn't
- * be created at all.
+ * per-skill failures don't abort the others. Unsafe target identity stops the
+ * agent before the next destructive operation and returns a reportable reason.
  */
 function mirrorInto(
   targetDir: string,
   source: string,
   skills: string[],
   platform: NodeJS.Platform,
-): boolean {
+  safety: () => MirrorSkipReason | null,
+): { mirrored: boolean; skipReason?: MirrorSkipReason } {
+  const beforeCreate = safety();
+  if (beforeCreate) return { mirrored: false, skipReason: beforeCreate };
   try {
     mkdirSync(targetDir, { recursive: true });
   } catch {
-    return false;
+    return { mirrored: false };
   }
+  const afterCreate = safety();
+  if (afterCreate) return { mirrored: false, skipReason: afterCreate };
   for (const skill of skills) {
     try {
-      linkOrCopy(join(source, skill), join(targetDir, skill), platform);
+      const sourceSkill = join(source, skill);
+      const targetSkill = join(targetDir, skill);
+      const skipReason = linkOrCopy(sourceSkill, targetSkill, platform, () =>
+        skillTargetSafety(sourceSkill, targetSkill, safety),
+      );
+      if (skipReason) return { mirrored: false, skipReason };
     } catch {
       // best-effort per skill
     }
   }
-  return true;
+  return { mirrored: true };
 }
 
 /**
@@ -132,7 +255,7 @@ export function mirrorGlobalSkills(opts: {
   // reads from the Claude store and must never link/copy onto either of them.
   const source = join(bases.claudeHome, "skills");
   const universalStore = join(home, ".agents", "skills");
-  if (!existsSync(source)) return { source: null, mirrored: [] };
+  if (!existsSync(source)) return { source: null, mirrored: [], skipped: [] };
 
   // Mirror ONLY HyperFrames' own skills (by name), NEVER everything in the
   // store: ~/.claude/skills is shared, so a user's gstack / personal / company
@@ -141,15 +264,32 @@ export function mirrorGlobalSkills(opts: {
   // hyperframesSkillNames).
   const allowed = new Set(opts.skills);
   const skills = listSkillDirs(source).filter((name) => allowed.has(name));
-  if (skills.length === 0) return { source, mirrored: [] };
+  if (skills.length === 0) return { source, mirrored: [], skipped: [] };
+
+  const protectedPaths = [source, universalStore].map((lexical) => ({
+    lexical,
+    canonical: canonicalCandidate(lexical),
+  }));
+  const resolvedProtectedPaths = protectedPaths.filter(
+    (entry): entry is { lexical: string; canonical: string } => entry.canonical !== null,
+  );
+  if (resolvedProtectedPaths.length !== protectedPaths.length) {
+    return { source, mirrored: [], skipped: [] };
+  }
 
   const mirrored: { agent: string; dir: string }[] = [];
+  const skipped: MirrorResult["skipped"] = [];
   for (const { agent, base, sub } of AGENT_GLOBAL_DIRS) {
     const targetDir = join(bases[base], ...sub.split("/").filter(Boolean));
     if (targetDir === source || targetDir === universalStore) continue; // install-owned
     if (UNIVERSAL_STORE_READERS.has(agent)) continue; // already reads the universal store (#3294)
     if (!existsSync(dirname(targetDir))) continue; // agent not installed (no marker)
-    if (mirrorInto(targetDir, source, skills, platform)) mirrored.push({ agent, dir: targetDir });
+    const attempt = mirrorInto(targetDir, source, skills, platform, () =>
+      targetSafety(targetDir, resolvedProtectedPaths),
+    );
+    if (attempt.mirrored) mirrored.push({ agent, dir: targetDir });
+    else if (attempt.skipReason)
+      skipped.push({ agent, dir: targetDir, reason: attempt.skipReason });
   }
-  return { source, mirrored };
+  return { source, mirrored, skipped };
 }