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 <christian.aurichzm@gmail.com>

Author: christianaurichzm

src/node_file.cc

@@ -4114,7 +4114,27 @@ static void CpSyncCopyDir(const FunctionCallbackInfo<Value>& 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<Value>& 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) {

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());
+}