fs: do not descend into symlinks for ** unless following symlinks

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/65435
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>

lib/internal/fs/glob.js

@@ -949,9 +949,9 @@ class Glob {
             subPatterns.add(index + 2);
           }
           if ((nextMatches || pattern.at(0) === '.') &&
-           (entryIsDirectory || entry.isSymbolicLink()) && !fromSymlink) {
+           entryIsDirectory && !fromSymlink) {
             // If pattern after ** matches, or pattern starts with "."
-            // and entry is a directory or symlink, add to potential patterns
+            // and entry is a directory, add to potential patterns
             subPatterns.add(nextIndex);
           }
 
@@ -1184,9 +1184,9 @@ class Glob {
             subPatterns.add(index + 2);
           }
           if ((nextMatches || pattern.at(0) === '.') &&
-           (entryIsDirectory || entry.isSymbolicLink()) && !fromSymlink) {
+           entryIsDirectory && !fromSymlink) {
             // If pattern after ** matches, or pattern starts with "."
-            // and entry is a directory or symlink, add to potential patterns
+            // and entry is a directory, add to potential patterns
             subPatterns.add(nextIndex);
           }
 

test/parallel/test-fs-glob-no-follow-symlink-globstar.mjs

@@ -0,0 +1,81 @@
+import * as common from '../common/index.mjs';
+import tmpdir from '../common/tmpdir.js';
+import assert from 'node:assert';
+import path from 'node:path';
+import { spawnSync } from 'node:child_process';
+import { glob as globPromise, mkdir, symlink, writeFile } from 'node:fs/promises';
+import { globSync, glob as asyncGlob } from 'node:fs';
+
+if (common.isWindows) {
+  // Directory symlinks require elevated privileges on Windows.
+  common.skip('symlinks are unreliable on Windows');
+}
+
+tmpdir.refresh();
+
+// Layout:
+//   root/visible.txt
+//   root/link  ->  outside   (outside/ holds files that must NOT be listed)
+const base = tmpdir.resolve('globstar-symlink');
+const root = path.resolve(base, 'workspace');
+const outside = path.resolve(base, 'outside');
+const outsideNested = path.resolve(outside, 'nested');
+
+await mkdir(root, { recursive: true });
+await mkdir(outsideNested, { recursive: true });
+await writeFile(path.resolve(root, 'visible.txt'), 'visible');
+await writeFile(path.resolve(outside, 'secret-one.txt'), 'secret one');
+await writeFile(path.resolve(outsideNested, 'secret-two.txt'), 'secret two');
+await symlink(outside, path.resolve(root, 'link'), 'dir');
+
+const expected = ['link', 'visible.txt'].map((e) => e.replaceAll('/', path.sep)).sort();
+
+// `**/*` must not descend into the symlinked directory, neither by default nor
+// with followSymlinks explicitly disabled.
+assert.deepStrictEqual(globSync('**/*', { cwd: root }).sort(), expected);
+assert.deepStrictEqual(
+  globSync('**/*', { cwd: root, followSymlinks: false }).sort(),
+  expected,
+);
+
+const promiseMatches = [];
+for await (const entry of globPromise('**/*', { cwd: root, followSymlinks: false })) {
+  promiseMatches.push(entry);
+}
+assert.deepStrictEqual(promiseMatches.sort(), expected);
+
+const callbackMatches = await new Promise((resolve, reject) => {
+  asyncGlob('**/*', { cwd: root, followSymlinks: false }, (err, matches) => {
+    if (err) reject(err);
+    else resolve(matches);
+  });
+});
+assert.deepStrictEqual(callbackMatches.sort(), expected);
+
+// Under the permission model, `**/*` must not expose entries from a symlink
+// target that is outside the granted read set.
+if (common.hasCrypto) {
+  const child = `
+    const assert = require('node:assert');
+    const fs = require('node:fs');
+    const path = require('node:path');
+    const root = ${JSON.stringify(root)};
+    const outside = ${JSON.stringify(outside)};
+    const expected = ['link', 'visible.txt'].map((e) => e.replaceAll('/', path.sep)).sort();
+    assert.strictEqual(process.permission.has('fs.read', root), true);
+    assert.strictEqual(process.permission.has('fs.read', outside), false);
+    assert.throws(() => fs.readdirSync(outside), { code: 'ERR_ACCESS_DENIED' });
+    assert.deepStrictEqual(fs.globSync('**/*', { cwd: root }).sort(), expected);
+    assert.deepStrictEqual(
+      fs.globSync('**/*', { cwd: root, followSymlinks: false }).sort(),
+      expected,
+    );
+  `;
+  const { status, stdout, stderr } = spawnSync(process.execPath, [
+    '--permission',
+    `--allow-fs-read=${root}`,
+    '-e',
+    child,
+  ], { encoding: 'utf8' });
+  assert.strictEqual(status, 0, stderr || stdout);
+}

test/parallel/test-fs-glob.mjs

@@ -146,7 +146,7 @@ const patterns = {
     'a/x',
     'a/z',
   ],
-  './**/a': common.isWindows ? ['a'] : ['a', 'a/symlink/a', 'a/symlink/a/b/c/a'],
+  './**/a': common.isWindows ? ['a'] : ['a', 'a/symlink/a'],
   './**/a/**/': [
     'a',
     'a/abcdef',
@@ -166,10 +166,6 @@ const patterns = {
       'a/symlink',
       'a/symlink/a',
       'a/symlink/a/b',
-      'a/symlink/a/b/c',
-      'a/symlink/a/b/c/a',
-      'a/symlink/a/b/c/a/b',
-      'a/symlink/a/b/c/a/b/c',
     ]),
     'a/x',
     'a/z',
@@ -200,9 +196,6 @@ const patterns = {
       'a/symlink/a',
       'a/symlink/a/b',
       'a/symlink/a/b/c',
-      'a/symlink/a/b/c/a',
-      'a/symlink/a/b/c/a/b',
-      'a/symlink/a/b/c/a/b/c',
     ]),
     'a/x',
     'a/z',
@@ -210,13 +203,6 @@ const patterns = {
   './**/a/**/a/**/': common.isWindows ? [] : [
     'a/symlink/a',
     'a/symlink/a/b',
-    'a/symlink/a/b/c',
-    'a/symlink/a/b/c/a',
-    'a/symlink/a/b/c/a/b',
-    'a/symlink/a/b/c/a/b/c',
-    'a/symlink/a/b/c/a/b/c/a',
-    'a/symlink/a/b/c/a/b/c/a/b',
-    'a/symlink/a/b/c/a/b/c/a/b/c',
   ],
   '+(a|b|c)/a{/,bc*}/**': [
     'a/abcdef',
@@ -295,7 +281,6 @@ const patterns = {
   'a/symlink/a/**/*': common.isWindows ? [] : [
     'a/symlink/a/b',
     'a/symlink/a/b/c',
-    'a/symlink/a/b/c/a',
   ],
   'a/!(symlink)/**/..': [
     'a',