install: reject package names and aliases longer than 255 bytes

src/install/PackageInstaller.rs

@@ -367,10 +367,14 @@ fn abs_node_modules_path(
 /// A dependency alias becomes the install destination inside `node_modules`
 /// (the existing entry is renamed aside, deleted, and re-created). Reject
 /// anything that could escape `node_modules`: empty names, `.`/`..`
-/// components, absolute paths, drive letters, backslashes, NUL bytes, and any
-/// separator other than the single `/` in a scoped name (`@scope/name`).
+/// components, absolute paths, drive letters, backslashes, NUL bytes, any
+/// separator other than the single `/` in a scoped name (`@scope/name`), and
+/// names longer than `MAX_INSTALL_FOLDER_NAME_LEN`.
 pub(crate) fn alias_is_safe_install_target(alias: &[u8]) -> bool {
-    if alias.is_empty() || alias.len() >= MAX_PATH_BYTES || strings::contains_any(alias, b"\\:\0") {
+    if alias.is_empty()
+        || alias.len() > crate::dependency::MAX_INSTALL_FOLDER_NAME_LEN
+        || strings::contains_any(alias, b"\\:\0")
+    {
         return false;
     }
 
@@ -1254,11 +1258,22 @@ impl<'a> PackageInstaller<'a> {
 
         // The alias is used as a path relative to `node_modules` for delete,
         // rename, and create operations. Refuse anything that could escape it.
-        if !alias_is_safe_install_target(alias.slice(string_buf!())) {
+        // An npm package's own name becomes the cache folder name, so it gets
+        // the same check no matter which lockfile or manifest it came from.
+        let unsafe_name = if !alias_is_safe_install_target(alias.slice(string_buf!())) {
+            Some(alias)
+        } else if resolution.tag == resolution::Tag::Npm
+            && !crate::dependency::is_safe_install_folder_name(pkg_name.slice(string_buf!()))
+        {
+            Some(pkg_name)
+        } else {
+            None
+        };
+        if let Some(unsafe_name) = unsafe_name {
             if log_level != Options::LogLevel::Silent {
                 bun_core::pretty_errorln!(
                     "<r><red>error<r>: refusing to install dependency with unsafe name <b>{}<r>",
-                    bstr::BStr::new(alias.slice(string_buf!())),
+                    bstr::BStr::new(unsafe_name.slice(string_buf!())),
                 );
             }
             self.summary.fail += 1;

src/install/dependency.rs

@@ -531,11 +531,19 @@ pub fn is_scoped_package_name(name: &[u8]) -> Result<bool, PackageNameError> {
     Err(PackageNameError::InvalidPackageName)
 }
 
+/// Longest package name or dependency alias the installers accept. Both
+/// linkers print `node_modules/<name>` and the cache folder name into
+/// fixed-size path buffers without a length check, so a name has to be
+/// bounded well below `MAX_PATH_BYTES` before it reaches them. 255 is
+/// `NAME_MAX` and is above npm's 214 byte limit on a package name.
+pub(crate) const MAX_INSTALL_FOLDER_NAME_LEN: usize = 255;
+
 /// A dependency name/alias becomes a directory under `node_modules/`. Names
 /// come from untrusted `package.json` / manifest keys, so reject anything that
-/// could resolve outside that directory. `@scope/name` stays valid.
+/// could resolve outside that directory, and anything longer than
+/// `MAX_INSTALL_FOLDER_NAME_LEN`. `@scope/name` stays valid.
 pub(crate) fn is_safe_install_folder_name(name: &[u8]) -> bool {
-    if name.is_empty() {
+    if name.is_empty() || name.len() > MAX_INSTALL_FOLDER_NAME_LEN {
         return false;
     }
 

src/install/isolated_install.rs

@@ -2100,6 +2100,39 @@ pub(crate) fn install_isolated_packages(
             );
         }
 
+        // Validate every package name and dependency alias as a
+        // `node_modules/<name>` component before the first task starts. A
+        // running task builds the store paths of its dependencies on a worker
+        // thread, so a check inside the task-start loop below would race it.
+        for (entry_index, entry_deps) in entry_dependencies.iter().enumerate() {
+            let node_id = entry_node_ids[entry_index];
+            let pkg_id = node_pkg_ids[node_id.get() as usize];
+            let name = pkg_names[pkg_id as usize].slice(string_buf);
+
+            let mut unsafe_folder_name: Option<&[u8]> = None;
+            if !name.is_empty() && !crate::package_installer::alias_is_safe_install_target(name) {
+                unsafe_folder_name = Some(name);
+            } else {
+                for dep in entry_deps.slice() {
+                    let dep_name = lockfile_ro.buffers.dependencies[dep.dep_id as usize]
+                        .name
+                        .slice(string_buf);
+                    if !crate::package_installer::alias_is_safe_install_target(dep_name) {
+                        unsafe_folder_name = Some(dep_name);
+                        break;
+                    }
+                }
+            }
+            if let Some(name) = unsafe_folder_name {
+                Output::err_generic(
+                    "\"{}\" is not a valid install folder name",
+                    (BStr::new(name),),
+                );
+                Output::flush();
+                Global::exit(1);
+            }
+        }
+
         // add the pending task count upfront
         installer
             .manager_mut()
@@ -2115,35 +2148,6 @@ pub(crate) fn install_isolated_packages(
             let pkg_name_hash = pkg_name_hashes[pkg_id as usize];
             let pkg_res: Resolution = pkg_resolutions[pkg_id as usize];
 
-            // Validate the package name and every dependency alias as
-            // `node_modules/<name>` components before any filesystem work.
-            {
-                let mut unsafe_folder_name: Option<&[u8]> = None;
-                let name = pkg_name.slice(string_buf);
-                if !name.is_empty() && !crate::package_installer::alias_is_safe_install_target(name)
-                {
-                    unsafe_folder_name = Some(name);
-                } else {
-                    for dep in entry_dependencies[entry_id.get() as usize].slice() {
-                        let dep_name = lockfile_ro.buffers.dependencies[dep.dep_id as usize]
-                            .name
-                            .slice(string_buf);
-                        if !crate::package_installer::alias_is_safe_install_target(dep_name) {
-                            unsafe_folder_name = Some(dep_name);
-                            break;
-                        }
-                    }
-                }
-                if let Some(name) = unsafe_folder_name {
-                    Output::err_generic(
-                        "\"{}\" is not a valid install folder name",
-                        (BStr::new(name),),
-                    );
-                    Output::flush();
-                    Global::exit(1);
-                }
-            }
-
             match pkg_res.tag {
                 ResolutionTag::Root => {
                     if dep_id == invalid_dependency_id {

test/cli/install/bun-install-registry.test.ts

@@ -9426,6 +9426,79 @@ test("rejects package names containing relative path components in bun.lock", as
   expect(await exited).toBe(0);
 });
 
+describe.each(["hoisted", "isolated"])("package names longer than 255 bytes in bun.lock (%s)", linker => {
+  // The name from a bun.lock `packages` entry is copied into fixed-size path
+  // buffers by both linkers: the hoisted linker when it prints the cache
+  // folder name, the isolated linker when a worker thread builds the store
+  // path of a dependency. A name longer than 255 bytes (NAME_MAX, above npm's
+  // 214 byte limit) is rejected when the lockfile is parsed, before either runs.
+  async function installWithName(nameLength: number) {
+    const name = "e" + Buffer.alloc(nameLength - "evil".length, "A").toString() + "vil";
+    const tarballUrl = `${registryUrl()}no-deps/-/no-deps-1.0.0.tgz`;
+    await Promise.all([
+      write(
+        packageJson,
+        JSON.stringify({
+          name: "foo",
+          version: "1.0.0",
+          dependencies: {
+            "no-deps": "1.0.0",
+          },
+        }),
+      ),
+      write(
+        join(packageDir, "bun.lock"),
+        JSON.stringify({
+          lockfileVersion: 1,
+          configVersion: 1,
+          workspaces: {
+            "": {
+              name: "foo",
+              dependencies: {
+                "no-deps": "1.0.0",
+              },
+            },
+          },
+          packages: {
+            "no-deps": [
+              `${name}@1.0.0`,
+              tarballUrl,
+              {},
+              "sha512-v4w12JRjUGvfHDUP8vFDwu0gUWu04j0cv9hLb1Abf9VdaXu4XcrddYFTMVBVvmldKViGWH7jrb6xPJRF0wq6gw==",
+            ],
+          },
+        }),
+      ),
+    ]);
+
+    await using proc = spawn({
+      cmd: [bunExe(), "install", "--frozen-lockfile", "--linker", linker],
+      cwd: packageDir,
+      stdout: "pipe",
+      stderr: "pipe",
+      env,
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    return { stdout, stderr, exitCode };
+  }
+
+  test("a name of 214 bytes (npm's limit) installs", async () => {
+    const { stdout, stderr, exitCode } = await installWithName(214);
+
+    expect(stderr).not.toContain("error:");
+    expect(stdout).toContain("1 package installed");
+    expect(exitCode).toBe(0);
+  });
+
+  test.each([256, 3000, 1_000_000])("a name of %d bytes is rejected", async nameLength => {
+    const { stdout, stderr, exitCode } = await installWithName(nameLength);
+
+    expect(stderr).toContain("Invalid package name");
+    expect(stdout).not.toContain("installed");
+    expect(exitCode).toBe(1);
+  });
+});
+
 test("rejects npm aliases whose manifest URL resolves to a different host than the registry", async () => {
   // The manifest URL is built by joining the registry URL with the package
   // name. WHATWG URL joining treats "\" like "/" for http(s) schemes, so a