install: don't leak package extraction temp directories into $TMPDIR

src/install/PackageManager/patchPackage.rs

@@ -371,6 +371,7 @@ pub fn do_patch_commit(
                 random_tempdir.as_bytes(),
                 sys::RenameOptions {
                     move_fallback: true,
+                    ..Default::default()
                 },
             )
             .is_err()
@@ -426,6 +427,7 @@ pub fn do_patch_commit(
                 patch_tag_tmpname.as_bytes(),
                 sys::RenameOptions {
                     move_fallback: true,
+                    ..Default::default()
                 },
             ) {
                 bun_core::warn!(
@@ -458,7 +460,7 @@ pub fn do_patch_commit(
                         random_tempdir.as_bytes(),
                         new_folder_handle.fd,
                         b"node_modules",
-                        sys::RenameOptions { move_fallback: true },
+                        sys::RenameOptions { move_fallback: true, ..Default::default() },
                     ) {
                         bun_core::warn!("failed renaming nested node_modules folder, this may cause issues: {}", e);
                     }
@@ -470,7 +472,7 @@ pub fn do_patch_commit(
                         patch_tag_tmpname.as_bytes(),
                         new_folder_handle.fd,
                         patch_tag,
-                        sys::RenameOptions { move_fallback: true },
+                        sys::RenameOptions { move_fallback: true, ..Default::default() },
                     ) {
                         bun_core::warn!("failed renaming the bun patch tag, this may cause issues: {}", e);
                     }
@@ -621,6 +623,7 @@ pub fn do_patch_commit(
         path_in_patches_dir,
         sys::RenameOptions {
             move_fallback: true,
+            ..Default::default()
         },
     ) {
         Output::err(e, "failed renaming patch file to patches dir", ());

src/install/extract_tarball.rs

@@ -258,6 +258,11 @@ impl ExtractTarball {
         let mut resolved: &'static [u8] = b"";
         let tmpname =
             FileSystem::tmpname(tmpname_suffix, &mut tmpname_buf.0, bun_core::fast_random())?;
+        // Delete the temp dir if extraction fails before it's renamed into the
+        // cache; defused on success.
+        let tmpdir_cleanup = scopeguard::guard((), |()| {
+            let _ = Dir::borrow(&self.temp_dir).delete_tree(tmpname.as_bytes());
+        });
         {
             let extract_destination = match bun_sys::make_path::make_open_path(
                 tmpdir,
@@ -468,7 +473,11 @@ impl ExtractTarball {
             }
         }
 
-        self.move_to_cache_directory(log, tmpname, name, basename, resolved)
+        let result = self.move_to_cache_directory(log, tmpname, name, basename, resolved);
+        if result.is_ok() {
+            scopeguard::ScopeGuard::into_inner(tmpdir_cleanup);
+        }
+        result
     }
 
     /// Rename the freshly-extracted temp directory into the cache, read
@@ -544,6 +553,27 @@ impl ExtractTarball {
             }
             let cache_dir = Dir::borrow(&self.cache_dir);
 
+            // An existing npm cache entry without package.json is invalid (the
+            // same check `package_missing_from_cache` uses), so no concurrent
+            // install reads from it. Delete it so the fresh copy below replaces
+            // it instead of being kept as an equivalent existing destination.
+            if self.resolution.tag == ResolutionTag::Npm {
+                let mut folder_name_z_buf = PathBuffer::uninit();
+                folder_name_z_buf[..folder_name.len()].copy_from_slice(folder_name);
+                folder_name_z_buf[folder_name.len()] = 0;
+                let folder_name_z = ZStr::from_buf(&folder_name_z_buf, folder_name.len());
+                if sys::directory_exists_at(cache_dir.fd(), folder_name_z).unwrap_or(false) {
+                    let mut json_buf = PathBuffer::uninit();
+                    let json_z = path::resolve_path::join_z_buf::<path::platform::Auto>(
+                        &mut json_buf.0,
+                        &[folder_name, b"package.json"],
+                    );
+                    if !sys::exists_at(cache_dir.fd(), json_z) {
+                        let _ = cache_dir.delete_tree(folder_name);
+                    }
+                }
+            }
+
             // e.g. @next
             // if it's a namespace package, we need to make sure the @name folder exists
             let create_subdir = basename.len() != name.len() && !self.resolution.tag.is_git();
@@ -609,39 +639,22 @@ impl ExtractTarball {
                                     | sys::Errno::PERM
                                     | sys::Errno::BUSY
                                     | sys::Errno::EXIST => {
-                                        // before we attempt to delete the destination, let's close the source dir.
                                         let _ = sys::close(dir_to_move);
 
-                                        // We tried to move the folder over
-                                        // but it didn't work!
-                                        // so instead of just simply deleting the folder
-                                        // we rename it back into the temp dir
-                                        // and then delete that temp dir
-                                        // The goal is to make it more difficult for an application to reach this folder
-                                        let mut tempdest_buf = PathBuffer::uninit();
-                                        tempdest_buf[0..tmpname.len()]
-                                            .copy_from_slice(tmpname.as_bytes());
-                                        tempdest_buf[tmpname.len()..][0..4]
-                                            .copy_from_slice(&[b't', b'm', b'p', 0]);
-                                        let tempdest =
-                                            ZStr::from_buf(&tempdest_buf, tmpname.len() + 3);
-                                        let mut folder_name_z_buf = PathBuffer::uninit();
-                                        folder_name_z_buf[0..folder_name.len()]
-                                            .copy_from_slice(folder_name);
-                                        folder_name_z_buf[folder_name.len()] = 0;
-                                        let folder_name_z =
-                                            ZStr::from_buf(&folder_name_z_buf, folder_name.len());
-                                        match sys::renameat(
+                                        // A concurrent install sharing the cache published
+                                        // this entry first. Keep it (it may already have
+                                        // readers) and drop our equivalent copy instead of
+                                        // renaming it out from under them.
+                                        if sys::directory_exists_at_w(
                                             Fd::from_std_dir(cache_dir),
-                                            folder_name_z,
-                                            Fd::from_std_dir(tmpdir),
-                                            tempdest,
-                                        ) {
-                                            bun_sys::Result::Err(_) => {}
-                                            bun_sys::Result::Ok(_) => {
-                                                let _ = tmpdir.delete_tree(tempdest.as_bytes());
-                                            }
+                                            path_to_use,
+                                        )
+                                        .unwrap_or(false)
+                                        {
+                                            let _ = tmpdir.delete_tree(tmpname.as_bytes());
+                                            break;
                                         }
+
                                         retries += 1;
                                         // 10ms, 20ms, 40ms, 80ms — long enough
                                         // for a concurrent close to land,
@@ -683,9 +696,9 @@ impl ExtractTarball {
                 //
                 // By:
                 // 1. Rename from temporary directory to cache directory and fail if it already exists
-                // 2a. If the rename fails, swap the cache directory with the temporary directory version
-                // 2b. Delete the temporary directory version ONLY if we're not using a provided temporary directory
-                // 3. If rename still fails, fallback to racily deleting the cache directory version and then renaming the temporary directory version again.
+                // 2. If the rename fails because the destination exists, a concurrent install
+                //    published an equivalent copy first: keep it and delete the temporary
+                //    directory version (`keep_existing_destination`).
                 //
 
                 if create_subdir {
@@ -701,6 +714,7 @@ impl ExtractTarball {
                     folder_name,
                     sys::RenameatConcurrentlyOptions {
                         move_fallback: true,
+                        keep_existing_destination: true,
                     },
                 ) {
                     log.add_error_fmt(

src/install/patch_install.rs

@@ -452,6 +452,12 @@ impl PatchTask {
 
         let system_tmpdir = self.tempdir;
 
+        // Delete the temp dir on failure; after a successful rename into the
+        // cache this is a no-op.
+        scopeguard::defer! {
+            let _ = sys::Dir::borrow(&system_tmpdir).delete_tree(tempdir_name.as_bytes());
+        }
+
         let pkg_name = patch.pkgname;
 
         let dummy_node_modules = crate::package_installer::NodeModulesFolder {
@@ -606,7 +612,7 @@ impl PatchTask {
             cache_dir_subpath_z,
             sys::RenameOptions {
                 move_fallback: true,
-                ..Default::default()
+                keep_existing_destination: true,
             },
         ) {
             log.add_error_fmt_opts(

src/sys/lib.rs

@@ -9225,7 +9225,13 @@ pub fn exists(path: &[u8]) -> bool {
 /// retries; on EXDEV falls back to the slow open+copy path. Only opens the
 /// source inside the EXDEV branch.
 pub fn move_file_z(from_dir: Fd, filename: &ZStr, to_dir: Fd, destination: &ZStr) -> Maybe<()> {
-    match renameat_concurrently_without_fallback(from_dir, filename, to_dir, destination) {
+    match renameat_concurrently_without_fallback(
+        from_dir,
+        filename,
+        to_dir,
+        destination,
+        Default::default(),
+    ) {
         Ok(()) => Ok(()),
         // allow over-writing an empty directory
         Err(e) if e.get_errno() == E::EISDIR => {
@@ -9302,6 +9308,12 @@ pub fn renameat_z(from_dir: impl AsFd, from: &ZStr, to_dir: impl AsFd, to: &ZStr
 #[derive(Default, Clone, Copy)]
 pub struct RenameatConcurrentlyOptions {
     pub move_fallback: bool,
+    /// If the destination already exists (a concurrent process published an
+    /// equivalent tree first, e.g. a shared package cache entry), keep it and
+    /// delete the source instead of atomically replacing it. Replacing would
+    /// yank entries out from under processes still reading the existing tree,
+    /// and swapping it into the source leaks the temp directory (#33977).
+    pub keep_existing_destination: bool,
 }
 /// Alias: `bun_install` call sites spell this `RenameOptions`.
 pub type RenameOptions = RenameatConcurrentlyOptions;
@@ -9328,7 +9340,7 @@ pub fn renameat_concurrently(
     to: &ZStr,
     opts: RenameatConcurrentlyOptions,
 ) -> Maybe<()> {
-    match renameat_concurrently_without_fallback(from_dir_fd, from, to_dir_fd, to) {
+    match renameat_concurrently_without_fallback(from_dir_fd, from, to_dir_fd, to, opts) {
         Ok(()) => Ok(()),
         Err(e) => {
             if opts.move_fallback && e.get_errno() == E::EXDEV {
@@ -9348,7 +9360,15 @@ pub fn renameat_concurrently_without_fallback(
     from: &ZStr,
     to_dir_fd: Fd,
     to: &ZStr,
+    opts: RenameatConcurrentlyOptions,
 ) -> Maybe<()> {
+    let delete_source = || {
+        if from_dir_fd.is_valid() {
+            let _ = Dir::borrow(&from_dir_fd).delete_tree(from.as_bytes());
+        } else {
+            let _ = delete_tree_absolute(from.as_bytes());
+        }
+    };
     'attempt: {
         {
             // Happy path: the folder doesn't exist in the cache dir, so we can
@@ -9373,6 +9393,14 @@ pub fn renameat_concurrently_without_fallback(
                 Ok(()) => break 'attempt,
             };
 
+            if opts.keep_existing_destination && matches!(err.get_errno(), E::EEXIST | E::ENOTEMPTY)
+            {
+                // Another process won the race; its tree is equivalent to
+                // ours and may already have readers, so drop our copy.
+                delete_source();
+                break 'attempt;
+            }
+
             // Windows doesn't have any equivalent of renameat with swap
             #[cfg(not(windows))]
             {
@@ -9401,6 +9429,21 @@ pub fn renameat_concurrently_without_fallback(
         }
 
         //  sad path: let's try to delete the folder and then rename it
+        if opts.keep_existing_destination {
+            // The errno didn't tell us whether the destination exists (e.g.
+            // EOPNOTSUPP when the filesystem lacks RENAME_NOREPLACE); check
+            // before deleting a tree another process may be reading. Windows
+            // `exists_at` is file-only, so ask for the directory explicitly.
+            let dir_fd = if to_dir_fd.is_valid() {
+                to_dir_fd
+            } else {
+                Fd::cwd()
+            };
+            if directory_exists_at(dir_fd, to).unwrap_or(false) {
+                delete_source();
+                break 'attempt;
+            }
+        }
         if to_dir_fd.is_valid() {
             let _ = Dir::borrow(&to_dir_fd).delete_tree(to.as_bytes());
         } else {
@@ -9742,6 +9785,7 @@ mod owned_handle_tests {
             b"sub",
             RenameatConcurrentlyOptions {
                 move_fallback: true,
+                ..Default::default()
             },
         )
         .expect("rename");
@@ -9757,6 +9801,51 @@ mod owned_handle_tests {
         let _ = close(root);
         let _ = Dir::open(&tmp).map(|d| d.delete_tree(b"."));
     }
+
+    /// With `keep_existing_destination`, losing the publish race keeps the
+    /// destination untouched and deletes the source instead of swapping the
+    /// two (which stranded the swapped-out tree in the temp dir, #33977).
+    #[test]
+    fn renameat_concurrently_keep_existing_destination() {
+        let _g = crate::file::tests::FD_TEST_LOCK.lock();
+        let mut tmp = std::env::temp_dir().as_os_str().as_encoded_bytes().to_vec();
+        tmp.extend_from_slice(b"/bun_sys_renameat_keep_test");
+        let _ = open_dir_at(Fd::cwd(), &tmp).map(close);
+        let _ = mkdir_recursive_at(Fd::cwd(), &tmp);
+        let root = open_dir_at(Fd::cwd(), &tmp).expect("open root");
+        let _ = mkdir_recursive_at(root, b"from/sub");
+        let _ = mkdir_recursive_at(root, b"to/sub");
+        let to_dir = open_dir_at(root, b"to").expect("open to");
+        File::write_file(root, ZStr::from_static(b"to/sub/winner\0"), b"").expect("marker");
+
+        renameat_concurrently_a(
+            root,
+            b"from/sub",
+            to_dir,
+            b"sub",
+            RenameatConcurrentlyOptions {
+                move_fallback: true,
+                keep_existing_destination: true,
+            },
+        )
+        .expect("rename");
+
+        // The existing destination survives with its contents...
+        assert!(
+            exists_at(to_dir, ZStr::from_static(b"sub/winner\0")),
+            "existing destination was replaced"
+        );
+        // ...and the source was cleaned up rather than left (or swapped)
+        // behind. Windows `exists_at` is file-only, so check the directory.
+        assert!(
+            !directory_exists_at(root, ZStr::from_static(b"from/sub\0")).unwrap_or(false),
+            "source left behind"
+        );
+
+        let _ = close(to_dir);
+        let _ = close(root);
+        let _ = Dir::open(&tmp).map(|d| d.delete_tree(b"."));
+    }
 }
 
 #[cfg(all(test, windows))]