From ef09754f4fb234e70923e04d182d54039513d147 Mon Sep 17 00:00:00 2001 From: crawfordxx Date: Mon, 23 Mar 2026 12:14:50 +0800 Subject: [PATCH] src: fix cpSyncCopyDir to dereference symlinks when dereference option is set When fs.cpSync() is called with {dereference: true}, symlinks in the source directory should be followed and their targets copied to the destination instead of creating new symlinks. This behavior was correctly implemented in the JavaScript version of cpSync, but was lost when the inner copyDir loop was migrated to C++ in CpSyncCopyDir(). The dereference parameter was captured but only used for error-condition checks, not to decide whether to create a symlink or copy the actual content. Fix by checking dereference before creating symlinks: when true, copy the actual file (using copy_file) or recurse into the actual directory (using copy_dir_contents) that the symlink points to. Fixes: #59168 --- src/node_file.cc | 37 ++++++++++-- ...fs-cp-sync-dereference-symlinks-in-dir.mjs | 56 +++++++++++++++++++ 2 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-fs-cp-sync-dereference-symlinks-in-dir.mjs diff --git a/src/node_file.cc b/src/node_file.cc index 0fe01e8b08127c..5430bb5f0dd7a3 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3595,16 +3595,43 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { } auto symlink_target_absolute = std::filesystem::weakly_canonical( std::filesystem::absolute(src / symlink_target)); - if (dir_entry.is_directory()) { + if (dereference) { + // When dereference is true, copy the actual content the symlink + // points to rather than creating a new symlink at the destination. + error.clear(); + if (std::filesystem::is_directory(symlink_target_absolute, error)) { + std::filesystem::create_directory(dest_file_path, error); + if (error) { + env->ThrowStdErrException(error, "cp", dest_str.c_str()); + return false; + } + auto success = + copy_dir_contents(symlink_target_absolute, dest_file_path); + if (!success) return false; + } else { + error.clear(); + std::filesystem::copy_file( + symlink_target_absolute, dest_file_path, file_copy_opts, + error); + if (error) { + env->ThrowStdErrException(error, "cp", dest_str.c_str()); + return false; + } + } + } else if (dir_entry.is_directory()) { std::filesystem::create_directory_symlink( symlink_target_absolute, dest_file_path, error); + if (error) { + env->ThrowStdErrException(error, "cp", dest_str.c_str()); + return false; + } } else { std::filesystem::create_symlink( symlink_target_absolute, dest_file_path, error); - } - if (error) { - env->ThrowStdErrException(error, "cp", dest_str.c_str()); - return false; + if (error) { + env->ThrowStdErrException(error, "cp", dest_str.c_str()); + return false; + } } } } else if (dir_entry.is_directory()) { diff --git a/test/parallel/test-fs-cp-sync-dereference-symlinks-in-dir.mjs b/test/parallel/test-fs-cp-sync-dereference-symlinks-in-dir.mjs new file mode 100644 index 00000000000000..546b09aba54bca --- /dev/null +++ b/test/parallel/test-fs-cp-sync-dereference-symlinks-in-dir.mjs @@ -0,0 +1,56 @@ +// Refs: https://github.com/nodejs/node/issues/59168 +// +// When cpSync() is called with {dereference: true}, symlinks *inside* the +// source directory should be resolved and their targets copied as real +// files/directories, not as new symlinks. This was broken in CpSyncCopyDir() +// which always created symlinks regardless of the dereference option. + +import { mustNotMutateObjectDeep } from '../common/index.mjs'; +import { nextdir } from '../common/fs.js'; +import assert from 'node:assert'; +import { + cpSync, + lstatSync, + mkdirSync, + readFileSync, + symlinkSync, + writeFileSync, +} from 'node:fs'; +import { join } from 'node:path'; +import tmpdir from '../common/tmpdir.js'; + +tmpdir.refresh(); + +// Build source tree: +// src/ +// src/real-file.txt (regular file) +// src/link-to-file.txt --> src/real-file.txt (symlink to file) +// src/real-subdir/ +// src/real-subdir/inner.txt (regular file inside subdirectory) +// src/link-to-dir/ --> src/real-subdir/ (symlink to directory) + +const src = nextdir(); +const dest = nextdir(); + +mkdirSync(src, mustNotMutateObjectDeep({ recursive: true })); +writeFileSync(join(src, 'real-file.txt'), 'hello', 'utf8'); +symlinkSync(join(src, 'real-file.txt'), join(src, 'link-to-file.txt')); + +const realSubdir = join(src, 'real-subdir'); +mkdirSync(realSubdir); +writeFileSync(join(realSubdir, 'inner.txt'), 'inner', 'utf8'); +symlinkSync(realSubdir, join(src, 'link-to-dir')); + +cpSync(src, dest, mustNotMutateObjectDeep({ dereference: true, recursive: true })); + +// Symlinked file should have been dereferenced: copied as a real file. +const linkToFileStat = lstatSync(join(dest, 'link-to-file.txt')); +assert(!linkToFileStat.isSymbolicLink(), 'link-to-file.txt should not be a symlink in dest'); +assert(linkToFileStat.isFile(), 'link-to-file.txt should be a regular file in dest'); +assert.strictEqual(readFileSync(join(dest, 'link-to-file.txt'), 'utf8'), 'hello'); + +// Symlinked directory should have been dereferenced: copied as a real directory. +const linkToDirStat = lstatSync(join(dest, 'link-to-dir')); +assert(!linkToDirStat.isSymbolicLink(), 'link-to-dir should not be a symlink in dest'); +assert(linkToDirStat.isDirectory(), 'link-to-dir should be a regular directory in dest'); +assert.strictEqual(readFileSync(join(dest, 'link-to-dir', 'inner.txt'), 'utf8'), 'inner');