From d7ae5a4a1f55d809c96611d70804a40e004f10da 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. A link whose target cannot be reached has nothing to copy, and uv_fs_stat() reports the underlying filesystem error. When following a link, preserve the existing force and errorOnExist behavior for occupied destinations. Under force, remove an existing destination first so copying does not write through a destination symlink. Signed-off-by: Christian Aurich --- src/node_file.cc | 67 +++++- ...-fs-cp-sync-dereference-nested-symlink.mjs | 192 ++++++++++++++++++ 2 files changed, 258 insertions(+), 1 deletion(-) 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 0179ad12c9d8..02e83a72e04e 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -4857,7 +4857,24 @@ CpError CopyDirRecursive(const std::filesystem::path& src_path, auto dest_file_path = dest / dir_entry.path().filename(); auto dest_str = ConvertPathToUTF8(dest); - if (dir_entry.is_symlink(error)) { + // With dereference, links that resolve to a directory or a regular file + // fall through to the branches below, which follow symlinks. A link + // whose target cannot be reached has nothing to copy, and stat() reports + // why. std::filesystem::status() does not: it folds ENOTDIR into + // not_found, and on Windows its error_code carries a Win32 value where + // an errno is expected. + const bool is_symlink = dir_entry.is_symlink(error); + if (is_symlink && options.dereference) { + uv_fs_t req; + auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); }); + auto entry_str = ConvertPathToUTF8(dir_entry.path()); + int rc = uv_fs_stat(nullptr, &req, entry_str.c_str(), nullptr); + if (rc < 0) { + return CpError::Uv(rc, "stat", entry_str); + } + } + + if (is_symlink && !options.dereference) { if (options.verbatim_symlinks) { std::filesystem::copy_symlink( dir_entry.path(), dest_file_path, error); @@ -4960,6 +4977,19 @@ CpError CopyDirRecursive(const std::filesystem::path& src_path, if (options.fresh_destination) { CpError made = MakeFreshDirectory(dest_file_path); if (made.kind != CpError::kNone) return made; + } else if (is_symlink) { + // Mirror the JavaScript walk: create the destination only when it + // does not exist, otherwise recurse into the existing path. + created = !std::filesystem::exists(dest_file_path, error); + if (error) { + return CpError::Std(error, ConvertPathToUTF8(dest_file_path)); + } + if (created) { + std::filesystem::create_directory(dest_file_path, error); + if (error) { + return CpError::Std(error, ConvertPathToUTF8(dest_file_path)); + } + } } else { created = std::filesystem::create_directory(dest_file_path, error); if (error) { @@ -4984,6 +5014,41 @@ CpError CopyDirRecursive(const std::filesystem::path& src_path, if (stamped.kind != CpError::kNone) return stamped; } } else if (dir_entry.is_regular_file(error)) { + if (is_symlink && !options.fresh_destination) { + // 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) { + return CpError::Std(dest_error, ConvertPathToUTF8(dest_file_path)); + } + + if (dest_exists) { + if (!options.force) { + if (options.error_on_exist) { + return {CpError::kEexist, + 0, + "cp", + SPrintF("[ERR_FS_CP_EEXIST]: Target already exists: " + "cp returned EEXIST (%s already exists)", + dest_file_path), + {}}; + } + continue; + } + + std::filesystem::remove(dest_file_path, dest_error); + if (dest_error) { + return CpError::Std(dest_error, + ConvertPathToUTF8(dest_file_path)); + } + } + } + bool copied = true; if (options.fresh_destination) { CpError fresh = CopyFileFresh( 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..9ec9b002bbe8 --- /dev/null +++ b/test/parallel/test-fs-cp-sync-dereference-nested-symlink.mjs @@ -0,0 +1,192 @@ +// 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' }, +); + +// The error for a target that cannot be reached is the one stat() gives, which +// is the error the JavaScript walk reports for the same tree. A filter keeps +// the copy out of the native walker. +{ + const unreachable = nextdir(); + mkdirSync(unreachable, { recursive: true }); + writeFileSync(join(unreachable, 'file'), 'file', 'utf8'); + symlinkSync(join(unreachable, 'file', 'child'), join(unreachable, 'link')); + + const codeOf = (opts) => { + try { + cpSync(unreachable, nextdir(), opts); + return 'no error'; + } catch (err) { + return err.code; + } + }; + const native = codeOf({ dereference: true, recursive: true }); + const javascript = codeOf({ dereference: true, recursive: true, filter: () => true }); + assert.strictEqual(native, javascript); + assert.notStrictEqual(native, 'no error'); +} + +// 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, so a file sitting there fails the copy. The exact error is +// platform-dependent; what holds everywhere is that the copy fails and the +// file is left as it was. +{ + 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 })), + { name: 'Error' }, + ); + assert.strictEqual(readFileSync(join(to, 'entry'), 'utf8'), 'occupied'); +} + +{ + 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()); +}