fs: pass symlink type in cp when filter is provided

When `fs.cp`/`fs.cpSync` is called with both `verbatimSymlinks: true`
and a `filter` function, directory symlinks were incorrectly created
as file symlinks on Windows.

Without a `filter`, cp takes the C++ fast path (`cpSyncCopyDir`) which
uses `std::filesystem::copy_symlink()` and preserves the symlink type
automatically. With a `filter`, the JS fallback calls `symlinkSync`/
`symlink` without a `type` argument. On Windows, that causes the type
to be auto-detected by stat-ing the resolved target at the destination,
but during a recursive copy the target directory may not exist yet at
the destination (e.g. `linked/` is copied before `packages/` in
alphabetical order). The stat fails and `type` falls back to `'file'`,
producing a file symlink in place of a directory symlink.

Detect the symlink type from the source (which always exists) via
`internalModuleStat(src)` and pass it explicitly to the `symlinkSync`/
`symlink` call sites. `onLink` already computed `srcIsDir` for
subdirectory validation; hoist that computation above the early-return
paths and thread the derived `symlinkType` through `copyLink` as well.
Both the sync (`cp-sync.js`) and async (`cp.js`) implementations are
fixed.

Add two regression tests that copy a tree containing a relative
directory symlink with `verbatimSymlinks: true` and a `filter`
function, then assert the destination link still resolves as a
directory.

Fixes: #62653
Signed-off-by: shulaoda <165626830+shulaoda@users.noreply.github.com>

Author: shulaoda

lib/internal/fs/cp/cp-sync.js

@@ -191,8 +191,10 @@ function onLink(destStat, src, dest, verbatimSymlinks) {
   if (!verbatimSymlinks && !isAbsolute(resolvedSrc)) {
     resolvedSrc = resolve(dirname(src), resolvedSrc);
   }
+  const srcIsDir = fsBinding.internalModuleStat(src) === 1;
+  const symlinkType = srcIsDir ? 'dir' : 'file';
   if (!destStat) {
-    return symlinkSync(resolvedSrc, dest);
+    return symlinkSync(resolvedSrc, dest, symlinkType);
   }
   let resolvedDest;
   try {
@@ -202,14 +204,13 @@ function onLink(destStat, src, dest, verbatimSymlinks) {
     // Windows may throw UNKNOWN error. If dest already exists,
     // fs throws error anyway, so no need to guard against it here.
     if (err.code === 'EINVAL' || err.code === 'UNKNOWN') {
-      return symlinkSync(resolvedSrc, dest);
+      return symlinkSync(resolvedSrc, dest, symlinkType);
     }
     throw err;
   }
   if (!isAbsolute(resolvedDest)) {
     resolvedDest = resolve(dirname(dest), resolvedDest);
   }
-  const srcIsDir = fsBinding.internalModuleStat(src) === 1;
 
   if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) {
     throw new ERR_FS_CP_EINVAL({
@@ -233,12 +234,12 @@ function onLink(destStat, src, dest, verbatimSymlinks) {
       code: 'EINVAL',
     });
   }
-  return copyLink(resolvedSrc, dest);
+  return copyLink(resolvedSrc, dest, symlinkType);
 }
 
-function copyLink(resolvedSrc, dest) {
+function copyLink(resolvedSrc, dest, symlinkType) {
   unlinkSync(dest);
-  return symlinkSync(resolvedSrc, dest);
+  return symlinkSync(resolvedSrc, dest, symlinkType);
 }
 
 module.exports = { cpSyncFn };

lib/internal/fs/cp/cp.js

@@ -336,8 +336,10 @@ async function onLink(destStat, src, dest, opts) {
   if (!opts.verbatimSymlinks && !isAbsolute(resolvedSrc)) {
     resolvedSrc = resolve(dirname(src), resolvedSrc);
   }
+  const srcIsDir = fsBinding.internalModuleStat(src) === 1;
+  const symlinkType = srcIsDir ? 'dir' : 'file';
   if (!destStat) {
-    return symlink(resolvedSrc, dest);
+    return symlink(resolvedSrc, dest, symlinkType);
   }
   let resolvedDest;
   try {
@@ -347,16 +349,14 @@ async function onLink(destStat, src, dest, opts) {
     // Windows may throw UNKNOWN error. If dest already exists,
     // fs throws error anyway, so no need to guard against it here.
     if (err.code === 'EINVAL' || err.code === 'UNKNOWN') {
-      return symlink(resolvedSrc, dest);
+      return symlink(resolvedSrc, dest, symlinkType);
     }
     throw err;
   }
   if (!isAbsolute(resolvedDest)) {
     resolvedDest = resolve(dirname(dest), resolvedDest);
   }
 
-  const srcIsDir = fsBinding.internalModuleStat(src) === 1;
-
   if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) {
     throw new ERR_FS_CP_EINVAL({
       message: `cannot copy ${resolvedSrc} to a subdirectory of self ` +
@@ -380,12 +380,12 @@ async function onLink(destStat, src, dest, opts) {
       code: 'EINVAL',
     });
   }
-  return copyLink(resolvedSrc, dest);
+  return copyLink(resolvedSrc, dest, symlinkType);
 }
 
-async function copyLink(resolvedSrc, dest) {
+async function copyLink(resolvedSrc, dest, symlinkType) {
   await unlink(dest);
-  return symlink(resolvedSrc, dest);
+  return symlink(resolvedSrc, dest, symlinkType);
 }
 
 module.exports = {

test/parallel/test-fs-cp-async-verbatim-dir-symlinks-with-filter.mjs

@@ -0,0 +1,46 @@
+// This tests that cp with verbatimSymlinks and filter preserves
+// the directory symlink type on Windows (does not create a file symlink).
+import { mustNotMutateObjectDeep, isWindows } from '../common/index.mjs';
+import { nextdir } from '../common/fs.js';
+import assert from 'node:assert';
+import {
+  mkdirSync,
+  writeFileSync,
+  symlinkSync,
+  readlinkSync,
+  readdirSync,
+  statSync,
+} from 'node:fs';
+import { cp } from 'node:fs/promises';
+import { join } from 'node:path';
+
+import tmpdir from '../common/tmpdir.js';
+tmpdir.refresh();
+
+// Setup source with a relative directory symlink
+const src = nextdir();
+mkdirSync(join(src, 'packages', 'my-lib'), mustNotMutateObjectDeep({ recursive: true }));
+writeFileSync(join(src, 'packages', 'my-lib', 'index.js'), 'module.exports = "hello"');
+mkdirSync(join(src, 'linked'), mustNotMutateObjectDeep({ recursive: true }));
+symlinkSync(join('..', 'packages', 'my-lib'), join(src, 'linked', 'my-lib'), 'dir');
+
+// Copy with verbatimSymlinks: true AND a filter function
+const dest = nextdir();
+await cp(src, dest, mustNotMutateObjectDeep({
+  recursive: true,
+  verbatimSymlinks: true,
+  filter: () => true,
+}));
+
+// Verify the symlink target is preserved verbatim
+const link = readlinkSync(join(dest, 'linked', 'my-lib'));
+if (isWindows) {
+  assert.strictEqual(link.toLowerCase(), join('..', 'packages', 'my-lib').toLowerCase());
+} else {
+  assert.strictEqual(link, join('..', 'packages', 'my-lib'));
+}
+
+// Verify the symlink works as a directory (not a file symlink)
+const destSymlink = join(dest, 'linked', 'my-lib');
+assert.ok(statSync(destSymlink).isDirectory(), 'symlink target should be accessible as a directory');
+assert.deepStrictEqual(readdirSync(destSymlink), ['index.js']);

test/parallel/test-fs-cp-sync-verbatim-dir-symlinks-with-filter.mjs

@@ -0,0 +1,46 @@
+// This tests that cpSync with verbatimSymlinks and filter preserves
+// the directory symlink type on Windows (does not create a file symlink).
+import { mustNotMutateObjectDeep, isWindows } from '../common/index.mjs';
+import { nextdir } from '../common/fs.js';
+import assert from 'node:assert';
+import {
+  cpSync,
+  mkdirSync,
+  writeFileSync,
+  symlinkSync,
+  readlinkSync,
+  readdirSync,
+  statSync,
+} from 'node:fs';
+import { join } from 'node:path';
+
+import tmpdir from '../common/tmpdir.js';
+tmpdir.refresh();
+
+// Setup source with a relative directory symlink
+const src = nextdir();
+mkdirSync(join(src, 'packages', 'my-lib'), mustNotMutateObjectDeep({ recursive: true }));
+writeFileSync(join(src, 'packages', 'my-lib', 'index.js'), 'module.exports = "hello"');
+mkdirSync(join(src, 'linked'), mustNotMutateObjectDeep({ recursive: true }));
+symlinkSync(join('..', 'packages', 'my-lib'), join(src, 'linked', 'my-lib'), 'dir');
+
+// Copy with verbatimSymlinks: true AND a filter function
+const dest = nextdir();
+cpSync(src, dest, mustNotMutateObjectDeep({
+  recursive: true,
+  verbatimSymlinks: true,
+  filter: () => true,
+}));
+
+// Verify the symlink target is preserved verbatim
+const link = readlinkSync(join(dest, 'linked', 'my-lib'));
+if (isWindows) {
+  assert.strictEqual(link.toLowerCase(), join('..', 'packages', 'my-lib').toLowerCase());
+} else {
+  assert.strictEqual(link, join('..', 'packages', 'my-lib'));
+}
+
+// Verify the symlink works as a directory (not a file symlink)
+const destSymlink = join(dest, 'linked', 'my-lib');
+assert.ok(statSync(destSymlink).isDirectory(), 'symlink target should be accessible as a directory');
+assert.deepStrictEqual(readdirSync(destSymlink), ['index.js']);