fix: skip symlink cycles during pack instead of failing with ELOOP

src/node/files.ts

@@ -1,4 +1,10 @@
-import { existsSync, readdirSync, readFileSync, statSync } from "fs";
+import {
+  existsSync,
+  readdirSync,
+  readFileSync,
+  realpathSync,
+  statSync,
+} from "fs";
 import ignore from "ignore";
 import { join, relative, sep } from "path";
 
@@ -76,11 +82,30 @@ export function shouldExclude(
   return buildIgnoreChecker(additionalPatterns).ignores(filePath);
 }
 
+/**
+ * Returns the real path of a directory if it is safe to recurse into, or
+ * undefined if doing so would revisit a directory already on the current
+ * traversal path (i.e. a symlink cycle).
+ */
+function checkForSymlinkCycle(
+  dirPath: string,
+  relativePath: string,
+  visitedRealPaths: Set<string>,
+): string | undefined {
+  const realPath = realpathSync(dirPath);
+  if (visitedRealPaths.has(realPath)) {
+    console.warn(`Warning: Symlink cycle detected, skipping: ${relativePath}`);
+    return undefined;
+  }
+  return realPath;
+}
+
 export function getAllFiles(
   dirPath: string,
   baseDir: string = dirPath,
   fileList: Record<string, Uint8Array> = {},
   additionalPatterns: string[] = [],
+  visitedRealPaths: Set<string> = new Set([realpathSync(dirPath)]),
 ): Record<string, Uint8Array> {
   const files = readdirSync(dirPath);
 
@@ -97,7 +122,23 @@ export function getAllFiles(
     const stat = statSync(filePath);
 
     if (stat.isDirectory()) {
-      getAllFiles(filePath, baseDir, fileList, additionalPatterns);
+      const realPath = checkForSymlinkCycle(
+        filePath,
+        relativePath,
+        visitedRealPaths,
+      );
+      if (realPath === undefined) {
+        continue;
+      }
+      visitedRealPaths.add(realPath);
+      getAllFiles(
+        filePath,
+        baseDir,
+        fileList,
+        additionalPatterns,
+        visitedRealPaths,
+      );
+      visitedRealPaths.delete(realPath);
     } else {
       // Use forward slashes in zip file paths
       const zipPath = relativePath.split(sep).join("/");
@@ -123,6 +164,7 @@ export function getAllFilesWithCount(
   fileList: Record<string, FileWithPermissions> = {},
   additionalPatterns: string[] = [],
   ignoredCount = 0,
+  visitedRealPaths: Set<string> = new Set([realpathSync(dirPath)]),
 ): GetAllFilesResult {
   const files = readdirSync(dirPath);
 
@@ -140,14 +182,25 @@ export function getAllFilesWithCount(
     const stat = statSync(filePath);
 
     if (stat.isDirectory()) {
+      const realPath = checkForSymlinkCycle(
+        filePath,
+        relativePath,
+        visitedRealPaths,
+      );
+      if (realPath === undefined) {
+        continue;
+      }
+      visitedRealPaths.add(realPath);
       const result = getAllFilesWithCount(
         filePath,
         baseDir,
         fileList,
         additionalPatterns,
         ignoredCount,
+        visitedRealPaths,
       );
       ignoredCount = result.ignoredCount;
+      visitedRealPaths.delete(realPath);
     } else {
       // Use forward slashes in zip file paths
       const zipPath = relativePath.split(sep).join("/");

test/symlink-cycle.test.ts

@@ -0,0 +1,99 @@
+import * as fs from "fs";
+import * as os from "os";
+import * as path from "path";
+
+import { getAllFiles, getAllFilesWithCount } from "../src/node/files.js";
+
+// Creating symlinks on Windows requires elevated privileges or Developer
+// Mode; junctions do not, and Node ignores the type argument elsewhere.
+function trySymlinkDir(target: string, linkPath: string): boolean {
+  try {
+    fs.symlinkSync(target, linkPath, "junction");
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+describe("Symlink cycle handling", () => {
+  let tempDir: string;
+  let extensionDir: string;
+  let warnSpy: jest.SpyInstance;
+
+  beforeEach(() => {
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "mcpb-symlink-test-"));
+    extensionDir = path.join(tempDir, "extension");
+    fs.mkdirSync(extensionDir);
+    fs.writeFileSync(path.join(extensionDir, "manifest.json"), "{}");
+    fs.writeFileSync(path.join(extensionDir, "index.js"), "console.log('hi')");
+    warnSpy = jest.spyOn(console, "warn").mockImplementation(() => undefined);
+  });
+
+  afterEach(() => {
+    warnSpy.mockRestore();
+    fs.rmSync(tempDir, { recursive: true, force: true });
+  });
+
+  it("should skip a self-referential symlink instead of failing with ELOOP", () => {
+    if (!trySymlinkDir(extensionDir, path.join(extensionDir, "selfloop"))) {
+      return; // symlinks not supported in this environment
+    }
+
+    const { files } = getAllFilesWithCount(extensionDir);
+
+    expect(Object.keys(files).sort()).toEqual(["index.js", "manifest.json"]);
+    expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("selfloop"));
+  });
+
+  it("should skip mutually recursive symlinks instead of failing with ELOOP", () => {
+    const dirA = path.join(extensionDir, "a");
+    const dirB = path.join(extensionDir, "b");
+    fs.mkdirSync(dirA);
+    fs.mkdirSync(dirB);
+    fs.writeFileSync(path.join(dirA, "a.txt"), "a");
+    fs.writeFileSync(path.join(dirB, "b.txt"), "b");
+    if (
+      !trySymlinkDir(dirB, path.join(dirA, "to-b")) ||
+      !trySymlinkDir(dirA, path.join(dirB, "to-a"))
+    ) {
+      return; // symlinks not supported in this environment
+    }
+
+    const { files } = getAllFilesWithCount(extensionDir);
+    const packedPaths = Object.keys(files).sort();
+
+    // Real files are packed; the walk terminates instead of raising ELOOP.
+    expect(packedPaths).toContain("a/a.txt");
+    expect(packedPaths).toContain("b/b.txt");
+    expect(packedPaths).toContain("index.js");
+    expect(packedPaths).toContain("manifest.json");
+  });
+
+  it("should still follow symlinks that do not form a cycle", () => {
+    const sharedDir = path.join(tempDir, "shared");
+    fs.mkdirSync(sharedDir);
+    fs.writeFileSync(path.join(sharedDir, "shared.txt"), "shared");
+    if (!trySymlinkDir(sharedDir, path.join(extensionDir, "linked"))) {
+      return; // symlinks not supported in this environment
+    }
+
+    const { files } = getAllFilesWithCount(extensionDir);
+
+    expect(Object.keys(files).sort()).toEqual([
+      "index.js",
+      "linked/shared.txt",
+      "manifest.json",
+    ]);
+    expect(warnSpy).not.toHaveBeenCalled();
+  });
+
+  it("getAllFiles should also skip symlink cycles", () => {
+    if (!trySymlinkDir(extensionDir, path.join(extensionDir, "selfloop"))) {
+      return; // symlinks not supported in this environment
+    }
+
+    const files = getAllFiles(extensionDir);
+
+    expect(Object.keys(files).sort()).toEqual(["index.js", "manifest.json"]);
+  });
+});