fs: pass symlink type in cp when filter is provided

shulaoda · shulaoda · commit 7c3cdf1560ce · 2026-04-10T00:19:17.000+08:00
diff --git a/lib/internal/fs/cp/cp-sync.js b/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 };
diff --git a/lib/internal/fs/cp/cp.js b/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 = {
diff --git a/test/parallel/test-fs-cp-async-verbatim-dir-symlinks-with-filter.mjs b/test/parallel/test-fs-cp-async-verbatim-dir-symlinks-with-filter.mjs
@@ -0,0 +1,47 @@
+// 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']);
diff --git a/test/parallel/test-fs-cp-sync-verbatim-dir-symlinks-with-filter.mjs b/test/parallel/test-fs-cp-sync-verbatim-dir-symlinks-with-filter.mjs
@@ -0,0 +1,47 @@
+// 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']);

Original file line number	Diff line number	Diff line change
`@@ -191,8 +191,10 @@ function onLink(destStat, src, dest, verbatimSymlinks) {`
`191`	`191`	`if (!verbatimSymlinks && !isAbsolute(resolvedSrc)) {`
`192`	`192`	`resolvedSrc = resolve(dirname(src), resolvedSrc);`
`193`	`193`	`}`
	`194`	`+ const srcIsDir = fsBinding.internalModuleStat(src) === 1;`
	`195`	`+ const symlinkType = srcIsDir ? 'dir' : 'file';`
`194`	`196`	`if (!destStat) {`
`195`		`- return symlinkSync(resolvedSrc, dest);`
	`197`	`+ return symlinkSync(resolvedSrc, dest, symlinkType);`
`196`	`198`	`}`
`197`	`199`	`let resolvedDest;`
`198`	`200`	`try {`
`@@ -202,14 +204,13 @@ function onLink(destStat, src, dest, verbatimSymlinks) {`
`202`	`204`	`// Windows may throw UNKNOWN error. If dest already exists,`
`203`	`205`	`// fs throws error anyway, so no need to guard against it here.`
`204`	`206`	`if (err.code === 'EINVAL' \|\| err.code === 'UNKNOWN') {`
`205`		`- return symlinkSync(resolvedSrc, dest);`
	`207`	`+ return symlinkSync(resolvedSrc, dest, symlinkType);`
`206`	`208`	`}`
`207`	`209`	`throw err;`
`208`	`210`	`}`
`209`	`211`	`if (!isAbsolute(resolvedDest)) {`
`210`	`212`	`resolvedDest = resolve(dirname(dest), resolvedDest);`
`211`	`213`	`}`
`212`		`- const srcIsDir = fsBinding.internalModuleStat(src) === 1;`
`213`	`214`
`214`	`215`	`if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) {`
`215`	`216`	`throw new ERR_FS_CP_EINVAL({`
`@@ -233,12 +234,12 @@ function onLink(destStat, src, dest, verbatimSymlinks) {`
`233`	`234`	`code: 'EINVAL',`
`234`	`235`	`});`
`235`	`236`	`}`
`236`		`- return copyLink(resolvedSrc, dest);`
	`237`	`+ return copyLink(resolvedSrc, dest, symlinkType);`
`237`	`238`	`}`
`238`	`239`
`239`		`-function copyLink(resolvedSrc, dest) {`
	`240`	`+function copyLink(resolvedSrc, dest, symlinkType) {`
`240`	`241`	`unlinkSync(dest);`
`241`		`- return symlinkSync(resolvedSrc, dest);`
	`242`	`+ return symlinkSync(resolvedSrc, dest, symlinkType);`
`242`	`243`	`}`
`243`	`244`
`244`	`245`	`module.exports = { cpSyncFn };`
Original file line number	Diff line number	Diff line change
`@@ -336,8 +336,10 @@ async function onLink(destStat, src, dest, opts) {`
`336`	`336`	`if (!opts.verbatimSymlinks && !isAbsolute(resolvedSrc)) {`
`337`	`337`	`resolvedSrc = resolve(dirname(src), resolvedSrc);`
`338`	`338`	`}`
	`339`	`+ const srcIsDir = fsBinding.internalModuleStat(src) === 1;`
	`340`	`+ const symlinkType = srcIsDir ? 'dir' : 'file';`
`339`	`341`	`if (!destStat) {`
`340`		`- return symlink(resolvedSrc, dest);`
	`342`	`+ return symlink(resolvedSrc, dest, symlinkType);`
`341`	`343`	`}`
`342`	`344`	`let resolvedDest;`
`343`	`345`	`try {`
`@@ -347,16 +349,14 @@ async function onLink(destStat, src, dest, opts) {`
`347`	`349`	`// Windows may throw UNKNOWN error. If dest already exists,`
`348`	`350`	`// fs throws error anyway, so no need to guard against it here.`
`349`	`351`	`if (err.code === 'EINVAL' \|\| err.code === 'UNKNOWN') {`
`350`		`- return symlink(resolvedSrc, dest);`
	`352`	`+ return symlink(resolvedSrc, dest, symlinkType);`
`351`	`353`	`}`
`352`	`354`	`throw err;`
`353`	`355`	`}`
`354`	`356`	`if (!isAbsolute(resolvedDest)) {`
`355`	`357`	`resolvedDest = resolve(dirname(dest), resolvedDest);`
`356`	`358`	`}`
`357`	`359`
`358`		`- const srcIsDir = fsBinding.internalModuleStat(src) === 1;`
`359`		`-`
`360`	`360`	`if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) {`
`361`	`361`	`throw new ERR_FS_CP_EINVAL({`
`362`	`362`	message: `cannot copy ${resolvedSrc} to a subdirectory of self ` +
`@@ -380,12 +380,12 @@ async function onLink(destStat, src, dest, opts) {`
`380`	`380`	`code: 'EINVAL',`
`381`	`381`	`});`
`382`	`382`	`}`
`383`		`- return copyLink(resolvedSrc, dest);`
	`383`	`+ return copyLink(resolvedSrc, dest, symlinkType);`
`384`	`384`	`}`
`385`	`385`
`386`		`-async function copyLink(resolvedSrc, dest) {`
	`386`	`+async function copyLink(resolvedSrc, dest, symlinkType) {`
`387`	`387`	`await unlink(dest);`
`388`		`- return symlink(resolvedSrc, dest);`
	`388`	`+ return symlink(resolvedSrc, dest, symlinkType);`
`389`	`389`	`}`
`390`	`390`
`391`	`391`	`module.exports = {`