install: replace a file at a command's name behind a symlinked node_modules

src/install/bin.rs

@@ -849,7 +849,7 @@ pub struct Linker<'a> {
     pub rel_buf: &'a mut [u8],
 
     /// Whether `node_modules_path` is a symlink (see `LinkedNodeModules`). `None` means not checked
-    /// yet. Behind a symlink, a file or directory that has a bin's name is kept, and linking fails.
+    /// yet. Behind a symlink, a directory that has a bin's name is kept, and linking fails.
     pub is_behind_link: Option<bool>,
 
     pub err: Option<Error>,
@@ -1361,14 +1361,20 @@ impl<'a> Linker<'a> {
             }
             Err(readlink_err) => match readlink_err.get_errno() {
                 // A file or a directory.
+                sys::Errno::EINVAL if global => {
+                    return Err(Error::Sys(bun_errno::SystemErrno::EEXIST));
+                }
+                // pnpm writes its commands as files, so a file is replaced. `unlink` fails on a directory.
                 sys::Errno::EINVAL
-                    if global
-                        || Self::is_behind_link(
-                            &mut self.is_behind_link,
-                            self.node_modules_path,
-                        ) =>
+                    if Self::is_behind_link(&mut self.is_behind_link, self.node_modules_path) =>
                 {
-                    return Err(Error::Sys(bun_errno::SystemErrno::EEXIST));
+                    // Without the name being what is in the way, the link would fail again.
+                    if errno != sys::Errno::EEXIST {
+                        return Err(err.into());
+                    }
+                    if sys::unlink(abs_dest).is_err() {
+                        return Err(Error::Sys(bun_errno::SystemErrno::EEXIST));
+                    }
                 }
                 sys::Errno::EINVAL => {
                     let _ = sys::delete_tree_absolute(abs_dest.as_bytes());

test/cli/install/symlink-path-traversal.test.ts

@@ -2173,15 +2173,13 @@ describe.concurrent.skipIf(isWindows)("what is behind a link at node_modules", (
   });
 
   // `dep` has a command of its name.
-  it.each(
-    ["hoisted", "isolated"].flatMap(linker =>
-      ["node_modules", "packages/app/node_modules"].flatMap(folder =>
-        [".bin/dep", ".bin/dep/x"].map(path => [linker, folder, path]),
-      ),
-    ),
-  )("is in the way of the link to a command (%s, %s): %s", async (linker, folder, path) => {
+  const linkersAndFolders = ["hoisted", "isolated"].flatMap(linker =>
+    ["node_modules", "packages/app/node_modules"].map(folder => [linker, folder]),
+  );
+
+  it.each(linkersAndFolders)("a directory is in the way of the link to a command (%s, %s)", async (linker, folder) => {
     using root = tempDir("nm-outside-bin", {});
-    const files = { ...MINE, [path]: KEEP };
+    const files = { ...MINE, ".bin/dep/x": KEEP };
     const outside = await plant(String(root), files);
     const repo = await workspace(String(root));
     await symlink(relative(dirname(join(repo, folder)), outside), join(repo, folder));
@@ -2192,4 +2190,19 @@ describe.concurrent.skipIf(isWindows)("what is behind a link at node_modules", (
     expect(stderr).toContain("EEXIST");
     expect(exitCode).toBe(1);
   });
+
+  // pnpm writes its commands as files, so a project that it installed has them.
+  it.each(linkersAndFolders)("a file gives way to the link to a command (%s, %s)", async (linker, folder) => {
+    using root = tempDir("nm-outside-bin-file", {});
+    const outside = await plant(String(root), { ...MINE, ".bin/dep": "#!/bin/sh\n", ".bin/other": KEEP });
+    const repo = await workspace(String(root));
+    await symlink(relative(dirname(join(repo, folder)), outside), join(repo, folder));
+
+    const { stderr, exitCode } = await runBun(repo, ["install", "--ignore-scripts", "--linker", linker]);
+
+    expect(stderr).not.toContain("error:");
+    expect((await lstat(join(outside, ".bin", "dep"))).isSymbolicLink()).toBe(true);
+    expect(await read(outside, { ...MINE, ".bin/other": KEEP })).toEqual({ ...MINE, ".bin/other": KEEP });
+    expect(exitCode).toBe(0);
+  });
 });