From 85df769312d76492e08f4ee10aea550f8a7cf861 Mon Sep 17 00:00:00 2001 From: MatricalDefunkt Date: Thu, 8 Jan 2026 22:34:04 +0530 Subject: [PATCH 1/2] fs: add option for recursive symlink traversal --- doc/api/fs.md | 2 ++ lib/internal/fs/glob.js | 52 ++++++++++++++++++++++++---------- test/parallel/test-fs-glob.mjs | 19 +++++++++++++ 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 6e86072ca6031e..d8be5dbf5c94af 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1115,6 +1115,8 @@ changes: If a string array is provided, each string should be a glob pattern that specifies paths to exclude. Note: Negation patterns (e.g., '!foo.js') are not supported. + * `followSymlinks` {boolean} `true` to traverse matching symbolic links to directories, + `false` otherwise. **Default:** `false`. * `withFileTypes` {boolean} `true` if the glob should return paths as Dirents, `false` otherwise. **Default:** `false`. * Returns: {AsyncIterator} An AsyncIterator that yields the paths of files diff --git a/lib/internal/fs/glob.js b/lib/internal/fs/glob.js index 1bfa39150e5196..80ba6d1e5d6094 100644 --- a/lib/internal/fs/glob.js +++ b/lib/internal/fs/glob.js @@ -16,8 +16,8 @@ const { StringPrototypeEndsWith, } = primordials; -const { lstatSync, readdirSync } = require('fs'); -const { lstat, readdir } = require('fs/promises'); +const { lstatSync, readdirSync, statSync: fsStatSync } = require('fs'); +const { lstat, readdir, stat: fsStat } = require('fs/promises'); const { join, resolve, basename, isAbsolute, dirname } = require('path'); const { @@ -264,12 +264,14 @@ class Glob { #subpatterns = new SafeMap(); #patterns; #withFileTypes; + #followSymlinks; #isExcluded = () => false; constructor(pattern, options = kEmptyObject) { validateObject(options, 'options'); - const { exclude, cwd, withFileTypes } = options; + const { exclude, cwd, withFileTypes, followSymlinks } = options; this.#root = toPathIfFileURL(cwd) ?? '.'; this.#withFileTypes = !!withFileTypes; + this.#followSymlinks = !!followSymlinks; if (exclude != null) { validateStringArrayOrFunction(exclude, 'options.exclude'); if (ArrayIsArray(exclude)) { @@ -429,6 +431,16 @@ class Glob { const entryPath = join(path, entry.name); this.#cache.addToStatCache(join(fullpath, entry.name), entry); + let isDirectory = entry.isDirectory(); + if (entry.isSymbolicLink() && this.#followSymlinks) { + try { + const stat = fsStatSync(join(fullpath, entry.name)); + isDirectory = stat.isDirectory(); + } catch { + // ignore + } + } + const subPatterns = new SafeSet(); const nSymlinks = new SafeSet(); for (const index of pattern.indexes) { @@ -456,7 +468,7 @@ class Glob { (this.#exclude && this.#exclude(this.#withFileTypes ? entry : entry.name))) { continue; } - if (!fromSymlink && entry.isDirectory()) { + if (!fromSymlink && isDirectory) { // If directory, add ** to its potential patterns subPatterns.add(index); } else if (!fromSymlink && index === last) { @@ -469,24 +481,24 @@ class Glob { if (nextMatches && nextIndex === last && !isLast) { // If next pattern is the last one, add to results this.#results.add(entryPath); - } else if (nextMatches && entry.isDirectory()) { + } else if (nextMatches && isDirectory) { // Pattern matched, meaning two patterns forward // are also potential patterns // e.g **/b/c when entry is a/b - add c to potential patterns subPatterns.add(index + 2); } if ((nextMatches || pattern.at(0) === '.') && - (entry.isDirectory() || entry.isSymbolicLink()) && !fromSymlink) { + (isDirectory || entry.isSymbolicLink()) && !fromSymlink) { // If pattern after ** matches, or pattern starts with "." // and entry is a directory or symlink, add to potential patterns subPatterns.add(nextIndex); } - if (entry.isSymbolicLink()) { + if (entry.isSymbolicLink() && !this.#followSymlinks) { nSymlinks.add(index); } - if (next === '..' && entry.isDirectory()) { + if (next === '..' && isDirectory) { // In case pattern is "**/..", // both parent and current directory should be added to the queue // if this is the last pattern, add to results instead @@ -529,7 +541,7 @@ class Glob { // add next pattern to potential patterns, or to results if it's the last pattern if (index === last) { this.#results.add(entryPath); - } else if (entry.isDirectory()) { + } else if (isDirectory) { subPatterns.add(nextIndex); } } @@ -639,6 +651,16 @@ class Glob { const entryPath = join(path, entry.name); this.#cache.addToStatCache(join(fullpath, entry.name), entry); + let isDirectory = entry.isDirectory(); + if (entry.isSymbolicLink() && this.#followSymlinks) { + try { + const s = await fsStat(join(fullpath, entry.name)); + isDirectory = s.isDirectory(); + } catch { + // ignore + } + } + const subPatterns = new SafeSet(); const nSymlinks = new SafeSet(); for (const index of pattern.indexes) { @@ -666,7 +688,7 @@ class Glob { (this.#exclude && this.#exclude(this.#withFileTypes ? entry : entry.name))) { continue; } - if (!fromSymlink && entry.isDirectory()) { + if (!fromSymlink && isDirectory) { // If directory, add ** to its potential patterns subPatterns.add(index); } else if (!fromSymlink && index === last) { @@ -683,24 +705,24 @@ class Glob { if (!this.#results.has(entryPath) && this.#results.add(entryPath)) { yield this.#withFileTypes ? entry : entryPath; } - } else if (nextMatches && entry.isDirectory()) { + } else if (nextMatches && isDirectory) { // Pattern matched, meaning two patterns forward // are also potential patterns // e.g **/b/c when entry is a/b - add c to potential patterns subPatterns.add(index + 2); } if ((nextMatches || pattern.at(0) === '.') && - (entry.isDirectory() || entry.isSymbolicLink()) && !fromSymlink) { + (isDirectory || entry.isSymbolicLink()) && !fromSymlink) { // If pattern after ** matches, or pattern starts with "." // and entry is a directory or symlink, add to potential patterns subPatterns.add(nextIndex); } - if (entry.isSymbolicLink()) { + if (entry.isSymbolicLink() && !this.#followSymlinks) { nSymlinks.add(index); } - if (next === '..' && entry.isDirectory()) { + if (next === '..' && isDirectory) { // In case pattern is "**/..", // both parent and current directory should be added to the queue // if this is the last pattern, add to results instead @@ -759,7 +781,7 @@ class Glob { yield this.#withFileTypes ? entry : entryPath; } } - } else if (entry.isDirectory()) { + } else if (isDirectory) { subPatterns.add(nextIndex); } } diff --git a/test/parallel/test-fs-glob.mjs b/test/parallel/test-fs-glob.mjs index 54523219cc2d97..cdb1dd2856aa51 100644 --- a/test/parallel/test-fs-glob.mjs +++ b/test/parallel/test-fs-glob.mjs @@ -543,3 +543,22 @@ describe('glob - with restricted directory', function() { } }); }); + +describe('glob follow', () => { + test('should return matched files in symlinked directory when follow is true', async () => { + if (common.isWindows) return; + const relFilesPromise = asyncGlob('**', { cwd: fixtureDir, followSymlinks: true }); + let count = 0; + // eslint-disable-next-line no-unused-vars + for await (const file of relFilesPromise) { + count++; + } + assert.ok(count > 0); + }); + + test('should return matched files in symlinked directory when follow is true (sync)', () => { + if (common.isWindows) return; + const relFiles = globSync('**', { cwd: fixtureDir, followSymlinks: true }); + assert.ok(relFiles.length > 0); + }); +}); From 7b0c22d4bd16b96948b1b49011f4429dc23b2c4a Mon Sep 17 00:00:00 2001 From: MatricalDefunkt Date: Fri, 9 Jan 2026 20:00:28 +0530 Subject: [PATCH 2/2] fs: bubble errors, improving UX --- lib/internal/fs/glob.js | 65 ++++++++++++++++++++++++-------- test-stat-identity.mjs | 34 +++++++++++++++++ test/parallel/test-fs-glob.mjs | 69 ++++++++++++++++++++++++++++------ 3 files changed, 141 insertions(+), 27 deletions(-) create mode 100644 test-stat-identity.mjs diff --git a/lib/internal/fs/glob.js b/lib/internal/fs/glob.js index 80ba6d1e5d6094..28d3356e27fa63 100644 --- a/lib/internal/fs/glob.js +++ b/lib/internal/fs/glob.js @@ -182,12 +182,14 @@ class Pattern { indexes; symlinks; last; + visited; - constructor(pattern, globStrings, indexes, symlinks) { + constructor(pattern, globStrings, indexes, symlinks, visited = null) { this.#pattern = pattern; this.#globStrings = globStrings; this.indexes = indexes; this.symlinks = symlinks; + this.visited = visited; this.last = pattern.length - 1; } @@ -205,8 +207,8 @@ class Pattern { at(index) { return ArrayPrototypeAt(this.#pattern, index); } - child(indexes, symlinks = new SafeSet()) { - return new Pattern(this.#pattern, this.#globStrings, indexes, symlinks); + child(indexes, symlinks = new SafeSet(), visited = null) { + return new Pattern(this.#pattern, this.#globStrings, indexes, symlinks, visited ?? this.visited); } test(index, path) { if (index > this.#pattern.length) { @@ -432,12 +434,24 @@ class Glob { this.#cache.addToStatCache(join(fullpath, entry.name), entry); let isDirectory = entry.isDirectory(); + let symlinkStat = null; if (entry.isSymbolicLink() && this.#followSymlinks) { - try { - const stat = fsStatSync(join(fullpath, entry.name)); - isDirectory = stat.isDirectory(); - } catch { - // ignore + const stat = fsStatSync(join(fullpath, entry.name)); + isDirectory = stat.isDirectory(); + if (isDirectory) { + // Check for cycles by looking at visited inodes + let node = pattern.visited; + while (node) { + if (node.ino === stat.ino && node.dev === stat.dev) { + // Cycle detected, skip this symlink + isDirectory = false; + break; + } + node = node.parent; + } + if (isDirectory) { + symlinkStat = stat; + } } } @@ -548,7 +562,12 @@ class Glob { } if (subPatterns.size > 0) { // If there are potential patterns, add to queue - this.#addSubpattern(entryPath, pattern.child(subPatterns, nSymlinks)); + let newVisited = pattern.visited; + if (symlinkStat) { + // Add this symlink target to the visited chain + newVisited = { __proto__: null, ino: symlinkStat.ino, dev: symlinkStat.dev, parent: pattern.visited }; + } + this.#addSubpattern(entryPath, pattern.child(subPatterns, nSymlinks, newVisited)); } } } @@ -652,12 +671,24 @@ class Glob { this.#cache.addToStatCache(join(fullpath, entry.name), entry); let isDirectory = entry.isDirectory(); + let symlinkStat = null; if (entry.isSymbolicLink() && this.#followSymlinks) { - try { - const s = await fsStat(join(fullpath, entry.name)); - isDirectory = s.isDirectory(); - } catch { - // ignore + const s = await fsStat(join(fullpath, entry.name)); + isDirectory = s.isDirectory(); + if (isDirectory) { + // Check for cycles by looking at visited inodes + let node = pattern.visited; + while (node) { + if (node.ino === s.ino && node.dev === s.dev) { + // Cycle detected, skip this symlink + isDirectory = false; + break; + } + node = node.parent; + } + if (isDirectory) { + symlinkStat = s; + } } } @@ -788,7 +819,11 @@ class Glob { } if (subPatterns.size > 0) { // If there are potential patterns, add to queue - this.#addSubpattern(entryPath, pattern.child(subPatterns, nSymlinks)); + let newVisited = pattern.visited; + if (symlinkStat) { + newVisited = { __proto__: null, ino: symlinkStat.ino, dev: symlinkStat.dev, parent: pattern.visited }; + } + this.#addSubpattern(entryPath, pattern.child(subPatterns, nSymlinks, newVisited)); } } } diff --git a/test-stat-identity.mjs b/test-stat-identity.mjs new file mode 100644 index 00000000000000..db13348363e8ac --- /dev/null +++ b/test-stat-identity.mjs @@ -0,0 +1,34 @@ +// test-stat-identity.mjs +import { statSync, rmSync } from 'fs'; +import { mkdirSync, symlinkSync } from 'fs'; +import { join, resolve } from 'path'; + +const tmp = resolve('tmp_stat_test'); +try { + rmSync(tmp, { recursive: true, force: true }); +} catch {} +try { + mkdirSync(tmp); +} catch {} + +const dir = join(tmp, 'real_dir'); +try { + mkdirSync(dir); +} catch {} + +const link1 = join(tmp, 'link1'); +// Link points to 'real_dir' which is in the same folder as 'link1' +try { + symlinkSync('real_dir', link1); +} catch {} + +try { + const s1 = statSync(dir); + const s2 = statSync(link1); + + console.log(`Real: dev=${s1.dev}, ino=${s1.ino}`); + console.log(`Link: dev=${s2.dev}, ino=${s2.ino}`); + console.log(`Match? ${s1.dev === s2.dev && s1.ino === s2.ino}`); +} catch (e) { + console.error(e); +} diff --git a/test/parallel/test-fs-glob.mjs b/test/parallel/test-fs-glob.mjs index cdb1dd2856aa51..15fe6c9c0de341 100644 --- a/test/parallel/test-fs-glob.mjs +++ b/test/parallel/test-fs-glob.mjs @@ -2,7 +2,7 @@ import * as common from '../common/index.mjs'; import tmpdir from '../common/tmpdir.js'; import { resolve, dirname, sep, relative, join, isAbsolute } from 'node:path'; import { mkdir, writeFile, symlink, glob as asyncGlob } from 'node:fs/promises'; -import { glob, globSync, Dirent, chmodSync } from 'node:fs'; +import { glob, globSync, Dirent, chmodSync, mkdirSync, writeFileSync, symlinkSync } from 'node:fs'; import { test, describe } from 'node:test'; import { pathToFileURL } from 'node:url'; import { promisify } from 'node:util'; @@ -544,21 +544,66 @@ describe('glob - with restricted directory', function() { }); }); -describe('glob follow', () => { - test('should return matched files in symlinked directory when follow is true', async () => { +describe('glob followSymlinks', () => { + test('should not throw ELOOP with followSymlinks on symlink loop (async)', async () => { if (common.isWindows) return; - const relFilesPromise = asyncGlob('**', { cwd: fixtureDir, followSymlinks: true }); - let count = 0; - // eslint-disable-next-line no-unused-vars - for await (const file of relFilesPromise) { - count++; + const files = []; + for await (const file of asyncGlob('**', { cwd: fixtureDir, followSymlinks: true })) { + files.push(file); } - assert.ok(count > 0); + // Should have results but no infinite loop + assert.ok(files.length > 0); + assert.ok(files.some((f) => f === 'a/symlink/a/b/c')); + assert.ok(files.some((f) => f.startsWith('a/symlink/a/b/c/a/'))); + const depth = Math.max(...files.filter((f) => f.startsWith('a/symlink/')).map((f) => f.split('/').length)); + assert.ok(depth < 20); }); - test('should return matched files in symlinked directory when follow is true (sync)', () => { + test('should not throw ELOOP with followSymlinks on symlink loop (sync)', () => { if (common.isWindows) return; - const relFiles = globSync('**', { cwd: fixtureDir, followSymlinks: true }); - assert.ok(relFiles.length > 0); + const files = globSync('**', { cwd: fixtureDir, followSymlinks: true }); + assert.ok(files.length > 0); + assert.ok(files.some((f) => f === 'a/symlink/a/b/c')); + assert.ok(files.some((f) => f.startsWith('a/symlink/a/b/c/a/'))); + const depth = Math.max(...files.filter((f) => f.startsWith('a/symlink/')).map((f) => f.split('/').length)); + assert.ok(depth < 20); + }); + + test('should handle symlinks without cycles (async)', async () => { + if (common.isWindows) return; + const deepLinkDir = tmpdir.resolve('deep-links'); + await mkdir(deepLinkDir, { recursive: true }); + + await mkdir(join(deepLinkDir, 'level1'), { recursive: true }); + await mkdir(join(deepLinkDir, 'level1', 'level2'), { recursive: true }); + await writeFile(join(deepLinkDir, 'level1', 'level2', 'file.txt'), 'deep'); + await symlink('level1/level2', join(deepLinkDir, 'link-to-level2')); + + const files = []; + for await (const file of asyncGlob('**/*.txt', { cwd: deepLinkDir, followSymlinks: true })) { + files.push(file); + } + + assert.ok(files.some((f) => f.includes('level1/level2/file.txt'))); + assert.ok(files.some((f) => f.includes('link-to-level2/file.txt'))); + }); + + test('should handle symlinks without cycles (sync)', () => { + if (common.isWindows) return; + const deepLinkDir = tmpdir.resolve('deep-links-sync'); + try { + mkdirSync(deepLinkDir, { recursive: true }); + mkdirSync(join(deepLinkDir, 'level1'), { recursive: true }); + mkdirSync(join(deepLinkDir, 'level1', 'level2'), { recursive: true }); + writeFileSync(join(deepLinkDir, 'level1', 'level2', 'file.txt'), 'deep'); + symlinkSync('level1/level2', join(deepLinkDir, 'link-to-level2')); + + const files = globSync('**/*.txt', { cwd: deepLinkDir, followSymlinks: true }); + + assert.ok(files.some((f) => f.includes('level1/level2/file.txt'))); + assert.ok(files.some((f) => f.includes('link-to-level2/file.txt'))); + } catch { + // Cleanup errors are ok + } }); });