fs: enhance glob methods to handle symlink cycles with followSymlinks option

Author: MatricalDefunkt

lib/internal/fs/glob.js

@@ -16,8 +16,8 @@ const {
   StringPrototypeEndsWith,
 } = primordials;
 
-const { lstatSync, readdirSync, statSync } = require('fs');
-const { lstat, readdir, stat } = require('fs/promises');
+const { lstatSync, readdirSync, statSync, realpathSync } = require('fs');
+const { lstat, readdir, stat, realpath } = require('fs/promises');
 const { join, resolve, basename, isAbsolute, dirname } = require('path');
 
 const {
@@ -134,6 +134,7 @@ class Cache {
   #statsCache = new SafeMap();
   #readdirCache = new SafeMap();
   #followSymlinks = false;
+  #visitedRealpaths = new SafeSet();
 
   setFollowSymlinks(followSymlinks) {
     this.#followSymlinks = followSymlinks;
@@ -143,6 +144,14 @@ class Cache {
     return this.#followSymlinks;
   }
 
+  hasVisitedRealPath(path) {
+    return this.#visitedRealpaths.has(path);
+  }
+
+  addVisitedRealPath(path) {
+    this.#visitedRealpaths.add(path);
+  }
+
   stat(path) {
     const cached = this.#statsCache.get(path);
     if (cached) {
@@ -457,18 +466,33 @@ class Glob {
     for (let i = 0; i < children.length; i++) {
       const entry = children[i];
       const entryPath = join(path, entry.name);
+      const entryFullPath = join(fullpath, entry.name);
 
       // If followSymlinks is enabled and entry is a symlink, resolve it
       let resolvedEntry = entry;
+      let resolvedRealpath;
       if (this.#cache.isFollowSymlinks() && entry.isSymbolicLink()) {
-        const resolved = this.#cache.statSync(join(fullpath, entry.name));
+        const resolved = this.#cache.statSync(entryFullPath);
         if (resolved && !resolved.isSymbolicLink()) {
           resolvedEntry = resolved;
           resolvedEntry.name = entry.name;
+          try {
+            resolvedRealpath = realpathSync(entryFullPath);
+          } catch {
+            // broken symlink or permission issue – fall back to lstat view
+          }
+        }
+      }
+
+      // Guard against cycles when following symlinks into directories
+      if (resolvedRealpath && resolvedEntry.isDirectory()) {
+        if (this.#cache.hasVisitedRealPath(resolvedRealpath)) {
+          continue;
         }
+        this.#cache.addVisitedRealPath(resolvedRealpath);
       }
 
-      this.#cache.addToStatCache(join(fullpath, entry.name), resolvedEntry);
+      this.#cache.addToStatCache(entryFullPath, resolvedEntry);
 
       const subPatterns = new SafeSet();
       const nSymlinks = new SafeSet();
@@ -678,18 +702,33 @@ class Glob {
     for (let i = 0; i < children.length; i++) {
       const entry = children[i];
       const entryPath = join(path, entry.name);
+      const entryFullPath = join(fullpath, entry.name);
 
       // If followSymlinks is enabled and entry is a symlink, resolve it
       let resolvedEntry = entry;
+      let resolvedRealpath;
       if (this.#cache.isFollowSymlinks() && entry.isSymbolicLink()) {
-        const resolved = await this.#cache.stat(join(fullpath, entry.name));
+        const resolved = await this.#cache.stat(entryFullPath);
         if (resolved && !resolved.isSymbolicLink()) {
           resolvedEntry = resolved;
           resolvedEntry.name = entry.name;
+          try {
+            resolvedRealpath = await realpath(entryFullPath);
+          } catch {
+            // broken symlink or permission issue – fall back to lstat view
+          }
+        }
+      }
+
+      // Guard against cycles when following symlinks into directories
+      if (resolvedRealpath && resolvedEntry.isDirectory()) {
+        if (this.#cache.hasVisitedRealPath(resolvedRealpath)) {
+          continue;
         }
+        this.#cache.addVisitedRealPath(resolvedRealpath);
       }
 
-      this.#cache.addToStatCache(join(fullpath, entry.name), resolvedEntry);
+      this.#cache.addToStatCache(entryFullPath, resolvedEntry);
 
       const subPatterns = new SafeSet();
       const nSymlinks = new SafeSet();

test/parallel/test-fs-glob-followsymlinks.mjs

@@ -2,7 +2,7 @@ import * as common from '../common/index.mjs';
 import tmpdir from '../common/tmpdir.js';
 import { resolve } from 'node:path';
 import { mkdir, writeFile, symlink, glob as asyncGlob } from 'node:fs/promises';
-import { globSync } from 'node:fs';
+import { globSync, mkdirSync, symlinkSync } from 'node:fs';
 import { test } from 'node:test';
 import assert from 'node:assert';
 
@@ -204,3 +204,43 @@ test('glob with withFileTypes and followSymlinks', async () => {
     assert.ok(entry.isFile() || entry.isDirectory(), `Entry ${entry.name} should be a file or directory`);
   });
 });
+
+test('glob with followSymlinks avoids cycles', async () => {
+  if (common.isWindows) {
+    return;
+  }
+
+  // Create a symlink that points back up the tree to test cycle protection
+  const loopDir = resolve(fixtureDir, 'loop');
+  await mkdir(loopDir, { recursive: true });
+  const loopTarget = resolve(loopDir, 'back');
+  await symlink(fixtureDir, loopTarget, 'dir');
+
+  const results = [];
+  for await (const file of asyncGlob('**/*.txt', { cwd: fixtureDir, followSymlinks: true })) {
+    results.push(file);
+    // Hard break guard: if we ever blow up beyond a sane bound, bail to avoid hanging tests
+    assert.ok(results.length < 200, 'Traversal should not loop infinitely when following symlinks');
+  }
+
+  // Ensure we still find original files
+  assert.ok(results.includes('real-dir/file.txt'));
+  assert.ok(results.includes('regular-dir/regular.txt'));
+});
+
+test('globSync with followSymlinks avoids cycles', () => {
+  if (common.isWindows) {
+    return;
+  }
+
+  const loopDir = resolve(fixtureDir, 'loop-sync');
+  mkdirSync(loopDir, { recursive: true });
+  const loopTarget = resolve(loopDir, 'back');
+  symlinkSync(fixtureDir, loopTarget, 'dir');
+
+  const results = globSync('**/*.txt', { cwd: fixtureDir, followSymlinks: true });
+
+  assert.ok(results.includes('real-dir/file.txt'));
+  assert.ok(results.includes('regular-dir/regular.txt'));
+  assert.ok(results.length < 200, 'Traversal should not loop infinitely when following symlinks (sync)');
+});