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 <christian.aurichzm@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65731
Fixes: https://github.com/nodejs/node/issues/59168
Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

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(

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