diff --git a/src/node/files.ts b/src/node/files.ts index 36e64f8..28efb6b 100644 --- a/src/node/files.ts +++ b/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 | 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 = {}, additionalPatterns: string[] = [], + visitedRealPaths: Set = new Set([realpathSync(dirPath)]), ): Record { 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 = {}, additionalPatterns: string[] = [], ignoredCount = 0, + visitedRealPaths: Set = 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("/"); diff --git a/test/symlink-cycle.test.ts b/test/symlink-cycle.test.ts new file mode 100644 index 0000000..7476389 --- /dev/null +++ b/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"]); + }); +});