Repository: denoland/deno
Pull request: fix(ext/node): implement `fchmod` on windows

Towards #29972

- Also fixes `node:fs.fstat` because it was incorrectly showing the file mode on windows, which is needed by the node compat test.
- Moves the `stat_extra` function from `ext/fs/std_fs.rs` (deno_fs) into `ext/io/lib.rs` (deno_io), because it's more convenient to export it from there letting us to use the function on `deno_fs` and `deno_io`, and currently `deno_fs` had already used `deno_io` as a dependency.
- Allows [parallel/test-fs-chmod-mask.js](https://github.com/nodejs/node/blob/v24.2.0/test/parallel/test-fs-chmod-mask.js) and [parallel/test-fs-chmod.js](https://github.com/nodejs/node/blob/v24.2.0/test/parallel/test-fs-chmod.js) tests to pass.

Changed files:
filename: ext/fs/std_fs.rs
patch: @@ -862,7 +862,7 @@ fn stat(path: &Path) -> FsResult<FsStat> {
   let file = opts.open(path)?;
   let metadata = file.metadata()?;
   let mut fsstat = FsStat::from_std(metadata);
-  stat_extra(&file, &mut fsstat)?;
+  deno_io::stat_extra(&file, &mut fsstat)?;
   Ok(fsstat)
 }
 
@@ -885,132 +885,10 @@ fn lstat(path: &Path) -> FsResult<FsStat> {
   let file = opts.open(path)?;
   let metadata = file.metadata()?;
   let mut fsstat = FsStat::from_std(metadata);
-  stat_extra(&file, &mut fsstat)?;
+  deno_io::stat_extra(&file, &mut fsstat)?;
   Ok(fsstat)
 }
 
-#[cfg(windows)]
-fn stat_extra(file: &std::fs::File, fsstat: &mut FsStat) -> FsResult<()> {
-  use std::os::windows::io::AsRawHandle;
-
-  unsafe fn get_dev(
-    handle: winapi::shared::ntdef::HANDLE,
-  ) -> std::io::Result<u64> {
-    use winapi::shared::minwindef::FALSE;
-    use winapi::um::fileapi::BY_HANDLE_FILE_INFORMATION;
-    use winapi::um::fileapi::GetFileInformationByHandle;
-
-    // SAFETY: winapi calls
-    unsafe {
-      let info = {
-        let mut info =
-          std::mem::MaybeUninit::<BY_HANDLE_FILE_INFORMATION>::zeroed();
-        if GetFileInformationByHandle(handle, info.as_mut_ptr()) == FALSE {
-          return Err(std::io::Error::last_os_error());
-        }
-
-        info.assume_init()
-      };
-
-      Ok(info.dwVolumeSerialNumber as u64)
-    }
-  }
-
-  const WINDOWS_TICK: i64 = 10_000; // 100-nanosecond intervals in a millisecond
-  const SEC_TO_UNIX_EPOCH: i64 = 11_644_473_600; // Seconds between Windows epoch and Unix epoch
-
-  fn windows_time_to_unix_time_msec(windows_time: &i64) -> i64 {
-    let milliseconds_since_windows_epoch = windows_time / WINDOWS_TICK;
-    milliseconds_since_windows_epoch - SEC_TO_UNIX_EPOCH * 1000
-  }
-
-  use windows_sys::Wdk::Storage::FileSystem::FILE_ALL_INFORMATION;
-  use windows_sys::Win32::Foundation::NTSTATUS;
-
-  unsafe fn query_file_information(
-    handle: winapi::shared::ntdef::HANDLE,
-  ) -> Result<FILE_ALL_INFORMATION, NTSTATUS> {
-    use windows_sys::Wdk::Storage::FileSystem::NtQueryInformationFile;
-    use windows_sys::Win32::Foundation::ERROR_MORE_DATA;
-    use windows_sys::Win32::Foundation::RtlNtStatusToDosError;
-    use windows_sys::Win32::System::IO::IO_STATUS_BLOCK;
-
-    // SAFETY: winapi calls
-    unsafe {
-      let mut info = std::mem::MaybeUninit::<FILE_ALL_INFORMATION>::zeroed();
-      let mut io_status_block =
-        std::mem::MaybeUninit::<IO_STATUS_BLOCK>::zeroed();
-      let status = NtQueryInformationFile(
-        handle as _,
-        io_status_block.as_mut_ptr(),
-        info.as_mut_ptr() as *mut _,
-        std::mem::size_of::<FILE_ALL_INFORMATION>() as _,
-        18, /* FileAllInformation */
-      );
-
-      if status < 0 {
-        let converted_status = RtlNtStatusToDosError(status);
-
-        // If error more data is returned, then it means that the buffer is too small to get full filename information
-        // to have that we should retry. However, since we only use BasicInformation and StandardInformation, it is fine to ignore it
-        // since struct is populated with other data anyway.
-        // https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntifs/nf-ntifs-ntqueryinformationfile#remarksdd
-        if converted_status != ERROR_MORE_DATA {
-          return Err(converted_status as NTSTATUS);
-        }
-      }
-
-      Ok(info.assume_init())
-    }
-  }
-
-  // SAFETY: winapi calls
-  unsafe {
-    let file_handle = file.as_raw_handle();
-
-    fsstat.dev = get_dev(file_handle)?;
-
-    if let Ok(file_info) = query_file_information(file_handle) {
-      fsstat.ctime = Some(windows_time_to_unix_time_msec(
-        &file_info.BasicInformation.ChangeTime,
-      ) as u64);
-
-      if file_info.BasicInformation.FileAttributes
-        & winapi::um::winnt::FILE_ATTRIBUTE_REPARSE_POINT
-        != 0
-      {
-        fsstat.is_symlink = true;
-      }
-
-      if file_info.BasicInformation.FileAttributes
-        & winapi::um::winnt::FILE_ATTRIBUTE_DIRECTORY
-        != 0
-      {
-        fsstat.mode |= libc::S_IFDIR as u32;
-        fsstat.size = 0;
-      } else {
-        fsstat.mode |= libc::S_IFREG as u32;
-        fsstat.size = file_info.StandardInformation.EndOfFile as u64;
-      }
-
-      if file_info.BasicInformation.FileAttributes
-        & winapi::um::winnt::FILE_ATTRIBUTE_READONLY
-        != 0
-      {
-        fsstat.mode |=
-          (libc::S_IREAD | (libc::S_IREAD >> 3) | (libc::S_IREAD >> 6)) as u32;
-      } else {
-        fsstat.mode |= ((libc::S_IREAD | libc::S_IWRITE)
-          | ((libc::S_IREAD | libc::S_IWRITE) >> 3)
-          | ((libc::S_IREAD | libc::S_IWRITE) >> 6))
-          as u32;
-      }
-    }
-
-    Ok(())
-  }
-}
-
 fn exists(path: &Path) -> bool {
   #[cfg(unix)]
   {

filename: ext/io/Cargo.toml
patch: @@ -39,3 +39,4 @@ rand.workspace = true
 parking_lot.workspace = true
 windows-sys.workspace = true
 deno_subprocess_windows.workspace = true
+libc.workspace = true

filename: ext/io/lib.rs
patch: @@ -828,29 +828,63 @@ impl crate::fs::File for StdFileResourceInner {
     }
   }
 
-  fn chmod_sync(self: Rc<Self>, _mode: u32) -> FsResult<()> {
+  fn chmod_sync(self: Rc<Self>, mode: u32) -> FsResult<()> {
     #[cfg(unix)]
     {
       use std::os::unix::prelude::PermissionsExt;
       self.with_sync(|file| {
-        Ok(file.set_permissions(std::fs::Permissions::from_mode(_mode))?)
+        Ok(file.set_permissions(std::fs::Permissions::from_mode(mode))?)
+      })
+    }
+    #[cfg(windows)]
+    {
+      self.with_sync(|file| {
+        let mut permissions = file.metadata()?.permissions();
+        if mode & libc::S_IWRITE as u32 > 0 {
+          // clippy warning should only be applicable to Unix platforms
+          // https://rust-lang.github.io/rust-clippy/master/index.html#permissions_set_readonly_false
+          #[allow(clippy::permissions_set_readonly_false)]
+          permissions.set_readonly(false);
+        } else {
+          permissions.set_readonly(true);
+        }
+        file.set_permissions(permissions)?;
+        Ok(())
       })
     }
-    #[cfg(not(unix))]
-    Err(FsError::NotSupported)
   }
-  async fn chmod_async(self: Rc<Self>, _mode: u32) -> FsResult<()> {
+  async fn chmod_async(self: Rc<Self>, mode: u32) -> FsResult<()> {
     #[cfg(unix)]
     {
       use std::os::unix::prelude::PermissionsExt;
       self
         .with_inner_blocking_task(move |file| {
-          Ok(file.set_permissions(std::fs::Permissions::from_mode(_mode))?)
+          Ok(file.set_permissions(std::fs::Permissions::from_mode(mode))?)
         })
         .await
     }
-    #[cfg(not(unix))]
-    Err(FsError::NotSupported)
+    #[cfg(windows)]
+    {
+      self
+        .with_inner_blocking_task(move |file| {
+          let mut permissions = file.metadata()?.permissions();
+          if mode & libc::S_IWRITE as u32 > 0 {
+            // clippy warning should only be applicable to Unix platforms
+            // https://rust-lang.github.io/rust-clippy/master/index.html#permissions_set_readonly_false
+            #[allow(clippy::permissions_set_readonly_false)]
+            permissions.set_readonly(false);
+          } else {
+            permissions.set_readonly(true);
+          }
+          file.set_permissions(permissions)?;
+          Ok(())
+        })
+        .await
+    }
+    #[cfg(not(any(unix, windows)))]
+    {
+      Err(FsError::NotSupported)
+    }
   }
 
   fn chown_sync(
@@ -922,14 +956,46 @@ impl crate::fs::File for StdFileResourceInner {
   }
 
   fn stat_sync(self: Rc<Self>) -> FsResult<FsStat> {
-    self.with_sync(|file| Ok(file.metadata().map(FsStat::from_std)?))
+    #[cfg(unix)]
+    {
+      self.with_sync(|file| Ok(file.metadata().map(FsStat::from_std)?))
+    }
+    #[cfg(windows)]
+    {
+      self.with_sync(|file| {
+        let mut fs_stat = file.metadata().map(FsStat::from_std)?;
+        stat_extra(file, &mut fs_stat)?;
+        Ok(fs_stat)
+      })
+    }
+    #[cfg(not(any(unix, windows)))]
+    {
+      Err(FsError::NotSupported)
+    }
   }
   async fn stat_async(self: Rc<Self>) -> FsResult<FsStat> {
-    self
-      .with_inner_blocking_task(|file| {
-        Ok(file.metadata().map(FsStat::from_std)?)
-      })
-      .await
+    #[cfg(unix)]
+    {
+      self
+        .with_inner_blocking_task(|file| {
+          Ok(file.metadata().map(FsStat::from_std)?)
+        })
+        .await
+    }
+    #[cfg(windows)]
+    {
+      self
+        .with_inner_blocking_task(|file| {
+          let mut fs_stat = file.metadata().map(FsStat::from_std)?;
+          stat_extra(file, &mut fs_stat)?;
+          Ok(fs_stat)
+        })
+        .await
+    }
+    #[cfg(not(any(unix, windows)))]
+    {
+      Err(FsError::NotSupported)
+    }
   }
 
   fn lock_sync(self: Rc<Self>, exclusive: bool) -> FsResult<()> {
@@ -1122,3 +1188,125 @@ pub fn op_print(
       .map_err(JsErrorBox::from_err)
   })
 }
+
+#[cfg(windows)]
+pub fn stat_extra(file: &std::fs::File, fsstat: &mut FsStat) -> FsResult<()> {
+  use std::os::windows::io::AsRawHandle;
+
+  unsafe fn get_dev(
+    handle: winapi::shared::ntdef::HANDLE,
+  ) -> std::io::Result<u64> {
+    use winapi::shared::minwindef::FALSE;
+    use winapi::um::fileapi::BY_HANDLE_FILE_INFORMATION;
+    use winapi::um::fileapi::GetFileInformationByHandle;
+
+    // SAFETY: winapi calls
+    unsafe {
+      let info = {
+        let mut info =
+          std::mem::MaybeUninit::<BY_HANDLE_FILE_INFORMATION>::zeroed();
+        if GetFileInformationByHandle(handle, info.as_mut_ptr()) == FALSE {
+          return Err(std::io::Error::last_os_error());
+        }
+
+        info.assume_init()
+      };
+
+      Ok(info.dwVolumeSerialNumber as u64)
+    }
+  }
+
+  const WINDOWS_TICK: i64 = 10_000; // 100-nanosecond intervals in a millisecond
+  const SEC_TO_UNIX_EPOCH: i64 = 11_644_473_600; // Seconds between Windows epoch and Unix epoch
+
+  fn windows_time_to_unix_time_msec(windows_time: &i64) -> i64 {
+    let milliseconds_since_windows_epoch = windows_time / WINDOWS_TICK;
+    milliseconds_since_windows_epoch - SEC_TO_UNIX_EPOCH * 1000
+  }
+
+  use windows_sys::Wdk::Storage::FileSystem::FILE_ALL_INFORMATION;
+  use windows_sys::Win32::Foundation::NTSTATUS;
+
+  unsafe fn query_file_information(
+    handle: winapi::shared::ntdef::HANDLE,
+  ) -> Result<FILE_ALL_INFORMATION, NTSTATUS> {
+    use windows_sys::Wdk::Storage::FileSystem::NtQueryInformationFile;
+    use windows_sys::Win32::Foundation::ERROR_MORE_DATA;
+    use windows_sys::Win32::Foundation::RtlNtStatusToDosError;
+    use windows_sys::Win32::System::IO::IO_STATUS_BLOCK;
+
+    // SAFETY: winapi calls
+    unsafe {
+      let mut info = std::mem::MaybeUninit::<FILE_ALL_INFORMATION>::zeroed();
+      let mut io_status_block =
+        std::mem::MaybeUninit::<IO_STATUS_BLOCK>::zeroed();
+      let status = NtQueryInformationFile(
+        handle as _,
+        io_status_block.as_mut_ptr(),
+        info.as_mut_ptr() as *mut _,
+        std::mem::size_of::<FILE_ALL_INFORMATION>() as _,
+        18, /* FileAllInformation */
+      );
+
+      if status < 0 {
+        let converted_status = RtlNtStatusToDosError(status);
+
+        // If error more data is returned, then it means that the buffer is too small to get full filename information
+        // to have that we should retry. However, since we only use BasicInformation and StandardInformation, it is fine to ignore it
+        // since struct is populated with other data anyway.
+        // https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntifs/nf-ntifs-ntqueryinformationfile#remarksdd
+        if converted_status != ERROR_MORE_DATA {
+          return Err(converted_status as NTSTATUS);
+        }
+      }
+
+      Ok(info.assume_init())
+    }
+  }
+
+  // SAFETY: winapi calls
+  unsafe {
+    let file_handle = file.as_raw_handle();
+
+    fsstat.dev = get_dev(file_handle)?;
+
+    if let Ok(file_info) = query_file_information(file_handle) {
+      fsstat.ctime = Some(windows_time_to_unix_time_msec(
+        &file_info.BasicInformation.ChangeTime,
+      ) as u64);
+
+      if file_info.BasicInformation.FileAttributes
+        & winapi::um::winnt::FILE_ATTRIBUTE_REPARSE_POINT
+        != 0
+      {
+        fsstat.is_symlink = true;
+      }
+
+      if file_info.BasicInformation.FileAttributes
+        & winapi::um::winnt::FILE_ATTRIBUTE_DIRECTORY
+        != 0
+      {
+        fsstat.mode |= libc::S_IFDIR as u32;
+        fsstat.size = 0;
+      } else {
+        fsstat.mode |= libc::S_IFREG as u32;
+        fsstat.size = file_info.StandardInformation.EndOfFile as u64;
+      }
+
+      if file_info.BasicInformation.FileAttributes
+        & winapi::um::winnt::FILE_ATTRIBUTE_READONLY
+        != 0
+      {
+        fsstat.mode |=
+          (libc::S_IREAD | (libc::S_IREAD >> 3) | (libc::S_IREAD >> 6)) as u32;
+      } else {
+        fsstat.mode |= ((libc::S_IREAD | libc::S_IWRITE)
+          | ((libc::S_IREAD | libc::S_IWRITE) >> 3)
+          | ((libc::S_IREAD | libc::S_IWRITE) >> 6))
+          as u32;
+      }
+    }
+
+    Ok(())
+  }
+}

filename: tests/node_compat/config.toml
patch: @@ -426,6 +426,8 @@
 "parallel/test-file-write-stream5.js" = {}
 "parallel/test-finalization-registry-shutdown.js" = {}
 "parallel/test-fs-buffertype-writesync.js" = {}
+"parallel/test-fs-chmod-mask.js" = {}
+"parallel/test-fs-chmod.js" = {}
 "parallel/test-fs-chown-type-check.js" = {}
 "parallel/test-fs-close.js" = {}
 # TODO(bartlomieju): appears that part of the test that's broke was removed in Node 24.0