fs: give directories created by cpSync the source directory's mode

The C++ fast path that fs.cpSync() takes when no filter is given created
the destination directories with default permissions, so a 0700
directory came out of the copy as 0755 (with the default umask). The
JavaScript implementation, which fs.cp(), fs.promises.cp() and
fs.cpSync() with a filter still use, chmod()s every directory it
creates to the mode of its source, and so did cpSync before the port.

Set the source directory's permissions on each directory the copy
creates (the destination root included); directories that already
exist keep theirs, as before.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #65488
Refs: #58461
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>

Author: codebytere

src/node_file.cc

@@ -3791,6 +3791,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo<Value>& args) {
   auto dest_path = dest.ToPath();
 
   std::error_code error;
+  const bool dest_existed = std::filesystem::exists(dest_path, error);
   std::filesystem::create_directories(dest_path, error);
   if (error) {
     return env->ThrowStdErrException(error, "cp", *dest);
@@ -3910,11 +3911,28 @@ 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);
+        const bool created =
+            std::filesystem::create_directory(dest_file_path, error);
+        if (error) {
+          env->ThrowStdErrException(
+              error, "cp", ConvertPathToUTF8(dest_file_path).c_str());
+          return false;
+        }
         auto success = copy_dir_contents(entry_dir_path, dest_file_path);
         if (!success) {
           return false;
         }
+        // A directory created by the copy gets the mode of its source once
+        // its contents are in (the source may be read-only).
+        if (created) {
+          std::filesystem::permissions(
+              dest_file_path, dir_entry.status().permissions(), error);
+          if (error) {
+            env->ThrowStdErrException(
+                error, "cp", ConvertPathToUTF8(dest_file_path).c_str());
+            return false;
+          }
+        }
       } else if (dir_entry.is_regular_file()) {
         std::filesystem::copy_file(
             dir_entry.path(), dest_file_path, file_copy_opts, error);
@@ -3939,7 +3957,13 @@ static void CpSyncCopyDir(const FunctionCallbackInfo<Value>& args) {
     return true;
   };
 
-  copy_dir_contents(src_path, dest_path);
+  if (copy_dir_contents(src_path, dest_path) && !dest_existed) {
+    std::filesystem::permissions(
+        dest_path, std::filesystem::status(src_path).permissions(), error);
+    if (error) {
+      return env->ThrowStdErrException(error, "cp", *dest);
+    }
+  }
 }
 
 BindingData::FilePathIsFileReturnType BindingData::FilePathIsFile(

test/parallel/test-fs-cp-sync-directory-mode.mjs

@@ -0,0 +1,64 @@
+// This tests that cpSync gives the directories it creates the mode of the
+// corresponding source directory, as cp does.
+import { mustNotMutateObjectDeep, isWindows, skip } from '../common/index.mjs';
+import { nextdir } from '../common/fs.js';
+import assert from 'node:assert';
+import { chmodSync, cpSync, mkdirSync, statSync, writeFileSync, promises } from 'node:fs';
+import { join } from 'node:path';
+import { isMainThread } from 'node:worker_threads';
+import tmpdir from '../common/tmpdir.js';
+
+if (isWindows)
+  skip('directory modes are not meaningful on Windows');
+if (!isMainThread)
+  skip('process.umask() is not available in workers');
+
+tmpdir.refresh();
+const mask = process.umask(0o022);
+
+const src = nextdir();
+mkdirSync(join(src, 'private', 'inner'), { recursive: true, mode: 0o700 });
+mkdirSync(join(src, 'shared'), { mode: 0o775 });
+writeFileSync(join(src, 'private', 'inner', 'file'), 'x', { mode: 0o600 });
+
+function modes(root) {
+  return ['.', 'private', 'private/inner', 'shared', 'private/inner/file']
+    .map((p) => (statSync(join(root, p)).mode & 0o777).toString(8));
+}
+
+const destSync = nextdir();
+cpSync(src, destSync, mustNotMutateObjectDeep({ recursive: true }));
+assert.deepStrictEqual(modes(destSync), modes(src));
+
+const destAsync = nextdir();
+await promises.cp(src, destAsync, { recursive: true });
+assert.deepStrictEqual(modes(destAsync), modes(src));
+
+// A read-only source directory can still be copied; its copy ends up read-only too.
+{
+  const roSrc = nextdir();
+  mkdirSync(join(roSrc, 'sub'), { recursive: true });
+  writeFileSync(join(roSrc, 'sub', 'file'), 'x');
+  chmodSync(join(roSrc, 'sub'), 0o555);
+  chmodSync(roSrc, 0o555);
+  const readOnly = [roSrc, join(roSrc, 'sub')];
+  for (const copy of [(dest) => cpSync(roSrc, dest, { recursive: true }),
+                      (dest) => promises.cp(roSrc, dest, { recursive: true })]) {
+    const dest = nextdir();
+    await copy(dest);
+    assert.strictEqual(statSync(join(dest, 'sub', 'file')).size, 1);
+    assert.deepStrictEqual(
+      [dest, join(dest, 'sub')].map((p) => (statSync(p).mode & 0o777).toString(8)), ['555', '555']);
+    readOnly.push(dest, join(dest, 'sub'));
+  }
+  // Let tmpdir clean up.
+  for (const dir of readOnly) chmodSync(dir, 0o755);
+}
+
+// An existing destination directory keeps its own mode.
+const existing = nextdir();
+mkdirSync(existing, { mode: 0o711 });
+cpSync(src, existing, mustNotMutateObjectDeep({ recursive: true }));
+assert.strictEqual((statSync(existing).mode & 0o777).toString(8), '711');
+assert.deepStrictEqual(modes(existing).slice(1), modes(src).slice(1));
+process.umask(mask);