fs: validate copyFile paths in the existing C++ binding

Fold getValidatedPath, file URL conversion, and NUL checks into
binding.copyFile so the sync, callback, and promises paths share
one implementation. Remove the extra copyFileSync binding.

Signed-off-by: Cursor Agent <cursoragent@cursor.com>

Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>

Author: cursoragent

lib/fs.js

@@ -3659,8 +3659,6 @@ function copyFile(src, dest, mode, callback) {
   const h = vfsState.handlers;
   if (h !== null && vfsVoid(h.copyFile(src, dest, mode), callback)) return;
 
-  src = getValidatedPath(src, 'src');
-  dest = getValidatedPath(dest, 'dest');
   callback = makeCallback(callback);
 
   const req = new FSReqCallback();
@@ -3682,7 +3680,7 @@ function copyFileSync(src, dest, mode) {
     const result = h.copyFileSync(src, dest, mode);
     if (result !== undefined) return;
   }
-  binding.copyFileSync(src, dest, mode);
+  binding.copyFile(src, dest, mode);
 }
 
 /**

lib/internal/fs/promises.js

@@ -1335,12 +1335,7 @@ async function copyFile(src, dest, mode) {
     if (promise !== undefined) { await promise; return; }
   }
   return await PromisePrototypeThen(
-    binding.copyFile(
-      getValidatedPath(src, 'src'),
-      getValidatedPath(dest, 'dest'),
-      mode,
-      kUsePromises,
-    ),
+    binding.copyFile(src, dest, mode, kUsePromises),
     undefined,
     handleErrorFromBinding,
   );

src/node_file.cc

@@ -2464,9 +2464,10 @@ static bool ContainsNul(const BufferValue& path) {
          std::memchr(path.out(), '\0', path.length()) != nullptr;
 }
 
-// C++ equivalent of getValidatedPath(value, propName): accepts string,
-// Uint8Array/Buffer, or WHATWG URL, rejects embedded NUL bytes, and converts
-// file: URLs to paths. Returns an empty MaybeLocal and throws on failure.
+// C++ equivalent of getValidatedPath(value, propName), used by CopyFile for
+// the sync, callback, and promises paths. Accepts string, Uint8Array/Buffer,
+// or WHATWG URL, rejects embedded NUL bytes, and converts file: URLs to
+// paths. Returns an empty MaybeLocal and throws on failure.
 static MaybeLocal<Value> GetValidatedPath(Environment* env,
                                           Local<Value> input,
                                           const char* prop_name) {
@@ -2536,13 +2537,15 @@ static MaybeLocal<Value> GetValidatedPath(Environment* env,
   return MaybeLocal<Value>();
 }
 
-// Full C++ implementation of fs.copyFileSync(): path validation, mode
-// validation, permission checks, and the copy itself.
-static void CopyFileSync(const FunctionCallbackInfo<Value>& args) {
+// Shared by the sync, callback, and promises copyFile paths. Path validation
+// (string / Uint8Array / file: URL, NUL checks) happens here so JS callers
+// can pass the original arguments through.
+static void CopyFile(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();
 
-  CHECK_GE(args.Length(), 2);  // src, dest[, mode]
+  const int argc = args.Length();
+  CHECK_GE(argc, 3);  // src, dest, flags[, req]
 
   Local<Value> src_val;
   if (!GetValidatedPath(env, args[0], "src").ToLocal(&src_val)) {
@@ -2575,45 +2578,12 @@ static void CopyFileSync(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
-  Local<Value> mode = args.Length() > 2 ? args[2] : Undefined(isolate);
-  int flags;
-  if (!GetValidFileMode(env, mode, UV_FS_COPYFILE).To(&flags)) {
-    return;
-  }
-
-  ToNamespacedPath(env, &src);
-  ToNamespacedPath(env, &dest);
-
-  THROW_IF_INSUFFICIENT_PERMISSIONS(
-      env, permission::PermissionScope::kFileSystemRead, src.ToStringView());
-  THROW_IF_INSUFFICIENT_PERMISSIONS(
-      env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView());
-
-  FSReqWrapSync req_wrap_sync("copyfile", *src, *dest);
-  FS_SYNC_TRACE_BEGIN(copyfile);
-  SyncCallAndThrowOnError(
-      env, &req_wrap_sync, uv_fs_copyfile, *src, *dest, flags);
-  FS_SYNC_TRACE_END(copyfile);
-}
-
-static void CopyFile(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  Isolate* isolate = env->isolate();
-
-  const int argc = args.Length();
-  CHECK_GE(argc, 3);  // src, dest, flags
-
   int flags;
   if (!GetValidFileMode(env, args[2], UV_FS_COPYFILE).To(&flags)) {
     return;
   }
 
-  BufferValue src(isolate, args[0]);
-  CHECK_NOT_NULL(*src);
   ToNamespacedPath(env, &src);
-
-  BufferValue dest(isolate, args[1]);
-  CHECK_NOT_NULL(*dest);
   ToNamespacedPath(env, &dest);
 
   if (argc > 3) {  // copyFile(src, dest, flags, req)
@@ -4409,7 +4379,6 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
   SetMethod(isolate, target, "writeFileUtf8", WriteFileUtf8);
   SetMethod(isolate, target, "realpath", RealPath);
   SetMethod(isolate, target, "copyFile", CopyFile);
-  SetMethod(isolate, target, "copyFileSync", CopyFileSync);
 
   SetMethod(isolate, target, "chmod", Chmod);
   SetMethod(isolate, target, "fchmod", FChmod);
@@ -4538,7 +4507,6 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(WriteFileUtf8);
   registry->Register(RealPath);
   registry->Register(CopyFile);
-  registry->Register(CopyFileSync);
 
   registry->Register(CpSyncCheckPaths);
   registry->Register(CpSyncOverrideFile);

test/parallel/test-fs-copyfile.js

@@ -167,6 +167,14 @@ assert.throws(() => {
   message: 'The URL must be of scheme file',
 });
 
+assert.throws(() => {
+  fs.copyFile(new URL('http://example.com/a'), dest, common.mustNotCall());
+}, {
+  code: 'ERR_INVALID_URL_SCHEME',
+  name: 'TypeError',
+  message: 'The URL must be of scheme file',
+});
+
 if (common.isWindows) {
   ['%2f', '%2F', '%5c', '%5C'].forEach((i) => {
     assert.throws(

typings/internalBinding/fs.d.ts

@@ -72,10 +72,10 @@ declare namespace InternalFSBinding {
   function close(fd: number, req: FSReqCallback): void;
   function close(fd: number): void;
 
-  function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: FSReqCallback): void;
-  function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: undefined, ctx: FSSyncContext): void;
-  function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, usePromises: typeof kUsePromises): Promise<void>;
-  function copyFileSync(src: unknown, dest: unknown, mode?: unknown): void;
+  function copyFile(src: unknown, dest: unknown, mode?: unknown): void;
+  function copyFile(src: unknown, dest: unknown, mode: unknown, req: FSReqCallback): void;
+  function copyFile(src: unknown, dest: unknown, mode: unknown, req: undefined, ctx: FSSyncContext): void;
+  function copyFile(src: unknown, dest: unknown, mode: unknown, usePromises: typeof kUsePromises): Promise<void>;
 
   function cpSyncCheckPaths(src: StringOrBuffer, dest: StringOrBuffer, dereference: boolean, recursive: boolean): void;
   function cpSyncOverrideFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, preserveTimestamps: boolean): void;
@@ -262,7 +262,6 @@ export interface FsBinding {
   chown: typeof InternalFSBinding.chown;
   close: typeof InternalFSBinding.close;
   copyFile: typeof InternalFSBinding.copyFile;
-  copyFileSync: typeof InternalFSBinding.copyFileSync;
   cpSyncCheckPaths: typeof InternalFSBinding.cpSyncCheckPaths;
   cpSyncOverrideFile: typeof InternalFSBinding.cpSyncOverrideFile;
   cpSyncCopyDir: typeof InternalFSBinding.cpSyncCopyDir;