[fs.cp] fails with EPERM despite filter

[c-vetter]

Version

v18.9.0

Platform

Microsoft Windows NT 10.0.19044.0 x64

Subsystem

No response

What steps will reproduce the bug?

$ node
Welcome to Node.js v18.9.0.
Type ".help" for more information.
> require(`fs`).cpSync(`B:`, `A:\\test`, { recursive: true, filter:(fp)=>(!fp.includes("System")) })
Uncaught Error: EPERM: operation not permitted, lstat '\\?\B:\System Volume Information'
    at lstatSync (node:fs:1574:3)
    at statFunc (node:internal/fs/cp/cp-sync:114:15)
    at getStatsSync (node:internal/fs/cp/cp-sync:115:19)
    at checkPathsSync (node:internal/fs/cp/cp-sync:64:33)
    at copyDir (node:internal/fs/cp/cp-sync:287:28)
    at onDir (node:internal/fs/cp/cp-sync:268:10)
    at getStats (node:internal/fs/cp/cp-sync:171:12)
    at handleFilterAndCopy (node:internal/fs/cp/cp-sync:158:10)
    at cpSyncFn (node:internal/fs/cp/cp-sync:60:10)
    at Object.cpSync (node:fs:2904:3) {
  errno: -4048,
  syscall: 'lstat',
  code: 'EPERM',
  path: '\\\\?\\B:\\System Volume Information'
}

How often does it reproduce? Is there a required condition?

Happens every time. Both drives are external, connected via USB.
Source can also be a subdirectory, analogous error.

I generally use Powershell Core, not the built-in Powershell.
But the same happens there.

What is the expected behavior?

The copy operation proceeds without trying to access the filtered-out directory.

What do you see instead?

Uncaught Error: EPERM: operation not permitted, lstat '\\?\B:\System Volume Information'

Additional information

This was triggered by jprichardson/node-fs-extra#965 (comment)

[tniessen]

I assume it is because copyDir calls checkPathsSync(), which invokes lstat(), before it calls startCopy(), which applies the filter. Given that the filter function only receives the paths and not the result of lstat(), it should be relatively simple to change that.

[thoqbk]

@tniessen am I right if we should add this check before calling checkPathsSync()?

if (opts.filter && !opts.filter(srcItem, destItem)) continue;

And since the startCopy has 2 lines and one of them is the filter call, should we move its main logic to copyDir and remove this function? Overall, the copyDir will look like this:

function copyDir(src, dest, opts) {
  const dir = opendirSync(src);

  try {
    let dirent;

    while ((dirent = dir.readSync()) !== null) {
      const { name } = dirent;
      const srcItem = join(src, name);
      const destItem = join(dest, name);
      if (opts.filter && !opts.filter(srcItem, destItem)) continue;
      const { destStat } = checkPathsSync(srcItem, destItem, opts);
      getStats(destStat, srcItem, destItem, opts);
    }
  } finally {
    dir.closeSync();
  }
}