From 346ed1cc655457a3d0d404d4e0ee7601de11c2af Mon Sep 17 00:00:00 2001 From: Christian Aurich Date: Tue, 1 Sep 2026 22:13:52 -0300 Subject: [PATCH] fs: honor dereference for symlinks nested in cpSync trees When no filter is given, cpSync copies directory contents in C++. That loop recreated every symlink it found, consulting dereference only for the subdirectory-of-self guards, so a symlink nested in the tree was copied as a link even with dereference set. Only a symlink passed as src was dereferenced, because that one is resolved by stat() in JavaScript before the C++ copy starts. The directory and regular file branches already follow symlinks, so links that resolve to those types can fall through to them. Dangling links have no target to copy and are reported instead. Signed-off-by: Christian Aurich --- src/node_file.cc | 78 +++++++- ...-fs-cp-sync-dereference-nested-symlink.mjs | 167 ++++++++++++++++++ 2 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-fs-cp-sync-dereference-nested-symlink.mjs diff --git a/src/node_file.cc b/src/node_file.cc index 8a09b656e5a1..49035148f305 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -4114,7 +4114,27 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { auto dest_file_path = dest / dir_entry.path().filename(); auto dest_str = ConvertPathToUTF8(dest); - if (dir_entry.is_symlink()) { + // With dereference, links that resolve to a directory or a regular file + // fall through to the branches below, which follow symlinks. A dangling + // link has no target to copy and is reported here. + const bool is_symlink = dir_entry.is_symlink(); + const bool copy_as_symlink = is_symlink && !dereference; + + if (is_symlink && dereference) { + std::error_code target_error; + if (!std::filesystem::exists(dir_entry.path(), target_error)) { + auto entry_str = ConvertPathToUTF8(dir_entry.path()); + env->ThrowStdErrException( + target_error + ? target_error + : std::make_error_code(std::errc::no_such_file_or_directory), + "cp", + entry_str.c_str()); + return false; + } + } + + if (copy_as_symlink) { if (verbatim_symlinks) { std::filesystem::copy_symlink( dir_entry.path(), dest_file_path, error); @@ -4202,12 +4222,66 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { } } else if (dir_entry.is_directory()) { auto entry_dir_path = src / dir_entry.path().filename(); - std::filesystem::create_directory(dest_file_path); + if (is_symlink && dereference) { + // Mirror the JavaScript walk: create the destination only when it + // does not exist, otherwise recurse into the existing path. + std::error_code dest_error; + const bool dest_exists = + std::filesystem::exists(dest_file_path, dest_error); + if (dest_error) { + env->ThrowStdErrException(dest_error, "cp", dest_str.c_str()); + return false; + } + if (!dest_exists) { + std::filesystem::create_directory(dest_file_path, dest_error); + if (dest_error) { + env->ThrowStdErrException(dest_error, "cp", dest_str.c_str()); + return false; + } + } + } else { + std::filesystem::create_directory(dest_file_path); + } auto success = copy_dir_contents(entry_dir_path, dest_file_path); if (!success) { return false; } } else if (dir_entry.is_regular_file()) { + if (is_symlink && dereference) { + // Only a dereferenced link reaches this branch as a link, so what an + // occupied destination means here is settled the way the JavaScript + // walk settles it: replaced under force, left untouched otherwise. + // Replacing an existing destination unlinks the entry first, which is + // what keeps an existing link there from being written through. + std::error_code dest_error; + const bool dest_exists = + std::filesystem::exists(dest_file_path, dest_error); + if (dest_error) { + env->ThrowStdErrException(dest_error, "cp", dest_str.c_str()); + return false; + } + + if (dest_exists) { + if (!force) { + if (error_on_exist) { + THROW_ERR_FS_CP_EEXIST( + isolate, + "[ERR_FS_CP_EEXIST]: Target already exists: " + "cp returned EEXIST (%s already exists)", + dest_file_path); + return false; + } + continue; + } + + std::filesystem::remove(dest_file_path, dest_error); + if (dest_error) { + env->ThrowStdErrException(dest_error, "cp", dest_str.c_str()); + return false; + } + } + } + std::filesystem::copy_file( dir_entry.path(), dest_file_path, file_copy_opts, error); if (error) { diff --git a/test/parallel/test-fs-cp-sync-dereference-nested-symlink.mjs b/test/parallel/test-fs-cp-sync-dereference-nested-symlink.mjs new file mode 100644 index 000000000000..7d9726d46933 --- /dev/null +++ b/test/parallel/test-fs-cp-sync-dereference-nested-symlink.mjs @@ -0,0 +1,167 @@ +// This tests that cpSync dereferences symlinks found inside the copied tree, +// not only a symlink passed as src. +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 { basename, join } from 'node:path'; +import tmpdir from '../common/tmpdir.js'; + +tmpdir.refresh(); + +const src = nextdir(); +const target = nextdir(); +const dest = nextdir(); + +mkdirSync(src, { recursive: true }); +mkdirSync(join(target, 'dir'), { recursive: true }); +writeFileSync(join(target, 'file.txt'), 'file', 'utf8'); +writeFileSync(join(target, 'dir', 'nested.txt'), 'nested', 'utf8'); +// Relative, as in the report: the link is resolved against its own directory. +symlinkSync(join('..', basename(target), 'file.txt'), join(src, 'link-to-file')); +symlinkSync(join(target, 'dir'), join(src, 'link-to-dir'), 'dir'); + +cpSync(src, dest, mustNotMutateObjectDeep({ dereference: true, recursive: true })); + +assert(!lstatSync(join(dest, 'link-to-file')).isSymbolicLink()); +assert.strictEqual(readFileSync(join(dest, 'link-to-file'), 'utf8'), 'file'); + +assert(!lstatSync(join(dest, 'link-to-dir')).isSymbolicLink()); +assert.strictEqual(readFileSync(join(dest, 'link-to-dir', 'nested.txt'), 'utf8'), 'nested'); + +// A dangling link has no target to copy. +const dangling = nextdir(); +mkdirSync(dangling, { recursive: true }); +symlinkSync(join(target, 'missing.txt'), join(dangling, 'link')); +assert.throws( + () => cpSync(dangling, nextdir(), + mustNotMutateObjectDeep({ dereference: true, recursive: true })), + { code: 'ENOENT' }, +); + +// A symlink cycle fails with ELOOP instead of recursing indefinitely. +const looping = nextdir(); +mkdirSync(looping, { recursive: true }); +symlinkSync(looping, join(looping, 'loop'), 'dir'); +assert.throws( + () => cpSync(looping, nextdir(), + mustNotMutateObjectDeep({ dereference: true, recursive: true })), + { code: 'ELOOP' }, +); + +// Under force, an existing destination link is replaced rather than written +// through. Whether replacement happens at all still follows force and +// errorOnExist. +function withDestLink() { + const outside = nextdir(); + const from = nextdir(); + const to = nextdir(); + mkdirSync(outside, { recursive: true }); + mkdirSync(from, { recursive: true }); + mkdirSync(to, { recursive: true }); + writeFileSync(join(outside, 'untouched.txt'), 'untouched', 'utf8'); + symlinkSync(join(target, 'file.txt'), join(from, 'entry')); + symlinkSync(join(outside, 'untouched.txt'), join(to, 'entry')); + return { outside, from, to }; +} + +{ + const { outside, from, to } = withDestLink(); + cpSync(from, to, mustNotMutateObjectDeep({ dereference: true, recursive: true })); + assert(!lstatSync(join(to, 'entry')).isSymbolicLink()); + assert.strictEqual(readFileSync(join(to, 'entry'), 'utf8'), 'file'); + assert.strictEqual(readFileSync(join(outside, 'untouched.txt'), 'utf8'), 'untouched'); +} + +{ + const { outside, from, to } = withDestLink(); + cpSync(from, to, mustNotMutateObjectDeep({ + dereference: true, recursive: true, force: false, + })); + assert(lstatSync(join(to, 'entry')).isSymbolicLink()); + assert.strictEqual(readFileSync(join(outside, 'untouched.txt'), 'utf8'), 'untouched'); +} + +{ + const { outside, from, to } = withDestLink(); + assert.throws( + () => cpSync(from, to, mustNotMutateObjectDeep({ + dereference: true, recursive: true, force: false, errorOnExist: true, + })), + { code: 'ERR_FS_CP_EEXIST' }, + ); + assert(lstatSync(join(to, 'entry')).isSymbolicLink()); + assert.strictEqual(readFileSync(join(outside, 'untouched.txt'), 'utf8'), 'untouched'); +} + +// A link resolving to a directory descends into whatever already occupies the +// destination path: a file there fails the way copying into it fails, and a +// link to a directory is followed and merged into. +{ + const from = nextdir(); + const to = nextdir(); + mkdirSync(from, { recursive: true }); + mkdirSync(to, { recursive: true }); + symlinkSync(join(target, 'dir'), join(from, 'entry'), 'dir'); + writeFileSync(join(to, 'entry'), 'occupied', 'utf8'); + assert.throws( + () => cpSync(from, to, + mustNotMutateObjectDeep({ dereference: true, recursive: true })), + { code: 'ENOTDIR' }, + ); +} + +{ + const existing = nextdir(); + const from = nextdir(); + const to = nextdir(); + mkdirSync(existing, { recursive: true }); + mkdirSync(from, { recursive: true }); + mkdirSync(to, { recursive: true }); + writeFileSync(join(existing, 'kept.txt'), 'kept', 'utf8'); + symlinkSync(join(target, 'dir'), join(from, 'entry'), 'dir'); + symlinkSync(existing, join(to, 'entry'), 'dir'); + + cpSync(from, to, mustNotMutateObjectDeep({ dereference: true, recursive: true })); + + assert(lstatSync(join(to, 'entry')).isSymbolicLink()); + assert.strictEqual(readFileSync(join(existing, 'kept.txt'), 'utf8'), 'kept'); + assert.strictEqual(readFileSync(join(existing, 'nested.txt'), 'utf8'), 'nested'); +} + +// A directory occupying the destination path is an occupied destination like +// any other, so the same force and errorOnExist rules decide its fate. +function withDestDir() { + const from = nextdir(); + const to = nextdir(); + mkdirSync(from, { recursive: true }); + mkdirSync(join(to, 'entry'), { recursive: true }); + symlinkSync(join(target, 'file.txt'), join(from, 'entry')); + return { from, to }; +} + +{ + const { from, to } = withDestDir(); + cpSync(from, to, mustNotMutateObjectDeep({ dereference: true, recursive: true })); + assert(lstatSync(join(to, 'entry')).isFile()); + assert.strictEqual(readFileSync(join(to, 'entry'), 'utf8'), 'file'); +} + +{ + const { from, to } = withDestDir(); + cpSync(from, to, mustNotMutateObjectDeep({ + dereference: true, recursive: true, force: false, + })); + assert(lstatSync(join(to, 'entry')).isDirectory()); +} + +{ + const { from, to } = withDestDir(); + assert.throws( + () => cpSync(from, to, mustNotMutateObjectDeep({ + dereference: true, recursive: true, force: false, errorOnExist: true, + })), + { code: 'ERR_FS_CP_EEXIST' }, + ); + assert(lstatSync(join(to, 'entry')).isDirectory()); +}