Repository: denoland/deno
Pull request: fix(npm): handle read-only bin files when setting up node_modules/.bin

## Summary

- Some npm tarballs ship bin files with read+execute but **no write** permission (e.g. `@formatjs/cli@2.15.0` uses mode `555`). `make_executable_if_exists()` was opening these files with `O_RDWR` unconditionally, causing `EACCES` ("Permission denied").
- Fix: open read-only first to check the execute bit. Only reopen with write if chmod is actually needed.

```
$ curl -sL https://registry.npmjs.org/@formatjs/cli/-/cli-2.15.0.tgz | tar tvzf - | grep bin/formatjs
-r-xr-xr-x  0 0      0          81 26 Oct  1985 package/bin/formatjs
```

Closes #29847

Changed files:
filename: libs/npm_installer/bin_entries.rs
patch: @@ -382,11 +382,12 @@ fn make_executable_if_exists(
   path: &Path,
 ) -> Result<bool, std::io::Error> {
   let sys = sys.with_paths_in_errors();
-  let mut open_options = sys_traits::OpenOptions::new();
-  open_options.read = true;
-  open_options.write = true;
-  open_options.truncate = false; // ensure false
-  let mut file = match sys.fs_open(path, &open_options) {
+  // Open read-only first to check existing permissions. Some npm tarballs
+  // ship bin files as read+execute without write (e.g. mode 555), so opening
+  // with O_RDWR would fail with EACCES.
+  let mut read_options = sys_traits::OpenOptions::new();
+  read_options.read = true;
+  let file = match sys.fs_open(path, &read_options) {
     Ok(file) => file,
     Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
       return Ok(false);
@@ -395,8 +396,15 @@ fn make_executable_if_exists(
   };
   let metadata = file.fs_file_metadata()?;
   let mode = metadata.mode()?;
+  drop(file);
+
   if mode & 0o111 == 0 {
-    // if the original file is not executable, make it executable
+    // The file is not executable — reopen with write to set permissions.
+    let mut write_options = sys_traits::OpenOptions::new();
+    write_options.read = true;
+    write_options.write = true;
+    write_options.truncate = false;
+    let mut file = sys.fs_open(path, &write_options)?;
     file.fs_file_set_permissions(mode | 0o111)?;
   }
 
@@ -479,3 +487,107 @@ fn symlink_bin_entry<'a>(
 
   Ok(EntrySetupOutcome::Success)
 }
+
+#[cfg(test)]
+mod test {
+  use std::path::PathBuf;
+
+  use sys_traits::FsCreateDirAll;
+  use sys_traits::FsRemoveDirAll;
+  #[cfg(unix)]
+  use sys_traits::FsWrite;
+
+  use super::*;
+
+  fn test_dir(name: &str) -> (PathBuf, impl Drop) {
+    static COUNTER: std::sync::atomic::AtomicU64 =
+      std::sync::atomic::AtomicU64::new(0);
+    let id = COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+    let sys = sys_traits::impls::RealSys;
+    let dir = sys_traits::EnvTempDir::env_temp_dir(&sys)
+      .unwrap()
+      .join(format!("deno_test_bin_entries_{name}_{id}"));
+    sys.fs_create_dir_all(&dir).unwrap();
+    struct Cleanup(PathBuf);
+    impl Drop for Cleanup {
+      fn drop(&mut self) {
+        let _ = sys_traits::impls::RealSys.fs_remove_dir_all(&self.0);
+      }
+    }
+    let cleanup = Cleanup(dir.clone());
+    (dir, cleanup)
+  }
+
+  #[cfg(unix)]
+  fn write_and_chmod(path: &Path, contents: &[u8], mode: u32) {
+    let sys = sys_traits::impls::RealSys;
+    sys.fs_write(path, contents).unwrap();
+    let mut open_options = sys_traits::OpenOptions::new();
+    open_options.read = true;
+    open_options.write = true;
+    let mut file = sys.fs_open(path, &open_options).unwrap();
+    file.fs_file_set_permissions(mode).unwrap();
+  }
+
+  #[cfg(unix)]
+  fn file_mode(path: &Path) -> u32 {
+    let sys = sys_traits::impls::RealSys;
+    let mut open_options = sys_traits::OpenOptions::new();
+    open_options.read = true;
+    let file = sys.fs_open(path, &open_options).unwrap();
+    let metadata = file.fs_file_metadata().unwrap();
+    metadata.mode().unwrap()
+  }
+
+  /// Regression test for https://github.com/denoland/deno/issues/29847
+  /// Some npm tarballs ship bin files with read+execute but no write
+  /// permission (mode 555). make_executable_if_exists must not fail on these.
+  #[cfg(unix)]
+  #[test]
+  fn make_executable_readonly_bin_file() {
+    let sys = sys_traits::impls::RealSys;
+    let (dir, _cleanup) = test_dir("readonly");
+
+    // Create a file with mode 555 (r-xr-xr-x) — executable but not writable
+    let bin_path = dir.join("my-bin");
+    write_and_chmod(&bin_path, b"#!/bin/sh\necho hi", 0o555);
+
+    // Should succeed without EACCES
+    let result = make_executable_if_exists(&sys, &bin_path);
+    assert!(result.is_ok());
+    assert!(result.unwrap()); // file exists
+
+    // Permissions should still include execute
+    assert_ne!(file_mode(&bin_path) & 0o111, 0);
+  }
+
+  /// Verify make_executable_if_exists adds execute bit when missing.
+  #[cfg(unix)]
+  #[test]
+  fn make_executable_non_executable_file() {
+    let sys = sys_traits::impls::RealSys;
+    let (dir, _cleanup) = test_dir("non_exec");
+
+    // Create a file with mode 644 (rw-r--r--) — no execute bit
+    let bin_path = dir.join("my-bin");
+    write_and_chmod(&bin_path, b"#!/bin/sh\necho hi", 0o644);
+
+    let result = make_executable_if_exists(&sys, &bin_path);
+    assert!(result.is_ok());
+    assert!(result.unwrap());
+
+    // Execute bit should now be set
+    assert_ne!(file_mode(&bin_path) & 0o111, 0);
+  }
+
+  #[test]
+  fn make_executable_nonexistent_file() {
+    let sys = sys_traits::impls::RealSys;
+    let (dir, _cleanup) = test_dir("nonexistent");
+    let bin_path = dir.join("does-not-exist");
+
+    let result = make_executable_if_exists(&sys, &bin_path);
+    assert!(result.is_ok());
+    assert!(!result.unwrap()); // file does not exist
+  }
+}