install: stop panicking on workspaces entries longer than the path buffer

src/install/lockfile/Package/WorkspaceMap.rs

@@ -2,7 +2,7 @@ use bstr::BStr;
 use bun_alloc::Arena; // bumpalo::Bump re-export
 use bun_ast as js_ast;
 use bun_collections::StringArrayHashMap;
-use bun_core::{ZStr, strings};
+use bun_core::strings;
 use bun_glob as glob;
 use bun_paths as path;
 use bun_paths::resolve_path;
@@ -117,13 +117,13 @@ impl<'a> NamesArray<'a> {
 
 fn process_workspace_name(
     json_cache: &mut WorkspacePackageJSONCache,
-    abs_package_json_path: &ZStr,
+    abs_package_json_path: &[u8],
     log: &mut bun_ast::Log,
 ) -> crate::Result<Entry> {
     let workspace_json = json_cache
         .get_with_path(
             log,
-            abs_package_json_path.as_bytes(),
+            abs_package_json_path,
             GetJSONOptions {
                 init_reset_store: false,
                 guess_indentation: true,
@@ -159,7 +159,7 @@ fn process_workspace_name(
     bun_output::scoped_log!(
         Lockfile,
         "processWorkspaceName({}) = {}",
-        BStr::new(abs_package_json_path.as_bytes()),
+        BStr::new(abs_package_json_path),
         BStr::new(&entry.name)
     );
 
@@ -215,48 +215,52 @@ impl WorkspaceMap {
                 continue;
             }
 
-            let abs_package_json_path: &ZStr =
-                resolve_path::join_abs_string_buf_z::<path::platform::Auto>(
-                    source.path.name().dir,
-                    filepath_buf,
-                    &[input_path, b"package.json"],
-                );
-
-            // skip root package.json
-            if strings::eql_long(
-                resolve_path::dirname::<path::platform::Auto>(abs_package_json_path.as_bytes()),
+            let processed = match resolve_path::join_abs_string_buf_checked::<path::platform::Auto>(
                 source.path.name().dir,
-                true,
+                filepath_buf,
+                &[input_path, b"package.json"],
             ) {
-                continue;
-            }
+                Some(abs_package_json_path) => {
+                    // skip root package.json
+                    if strings::eql_long(
+                        resolve_path::dirname::<path::platform::Auto>(abs_package_json_path),
+                        source.path.name().dir,
+                        true,
+                    ) {
+                        continue;
+                    }
 
-            let workspace_entry =
-                match process_workspace_name(json_cache, abs_package_json_path, log) {
-                    Ok(e) => e,
-                    Err(err) => {
-                        if err == crate::Error::Sys(bun_errno::SystemErrno::EISDIR)
-                            || err == crate::Error::Sys(bun_errno::SystemErrno::EPERM)
-                            || err == crate::Error::Sys(bun_errno::SystemErrno::ENOENT)
-                        {
-                            let _ = log.add_error_fmt(
-                                Some(source),
-                                arr.item_loc(source, i),
-                                format_args!("Workspace not found \"{}\"", BStr::new(input_path)),
-                            );
-                        } else if err == crate::Error::MissingPackageName {
-                            let _ = log.add_error_fmt(
-                                Some(source),
-                                loc,
-                                format_args!(
-                                    "Missing \"name\" from package.json in {}",
-                                    BStr::new(input_path)
-                                ),
-                            );
-                        } else {
-                            let mut cwd_buf = vec![0u8; MAX_PATH_BYTES];
-                            let cwd_len = bun_sys::getcwd(&mut cwd_buf).expect("unreachable");
-                            let _ = log.add_error_fmt(
+                    process_workspace_name(json_cache, abs_package_json_path, log)
+                        .map(|entry| (abs_package_json_path, entry))
+                }
+                None => Err(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG)),
+            };
+
+            let (abs_package_json_path, workspace_entry) = match processed {
+                Ok(processed) => processed,
+                Err(err) => {
+                    if err == crate::Error::Sys(bun_errno::SystemErrno::EISDIR)
+                        || err == crate::Error::Sys(bun_errno::SystemErrno::EPERM)
+                        || err == crate::Error::Sys(bun_errno::SystemErrno::ENOENT)
+                    {
+                        let _ = log.add_error_fmt(
+                            Some(source),
+                            arr.item_loc(source, i),
+                            format_args!("Workspace not found \"{}\"", BStr::new(input_path)),
+                        );
+                    } else if err == crate::Error::MissingPackageName {
+                        let _ = log.add_error_fmt(
+                            Some(source),
+                            loc,
+                            format_args!(
+                                "Missing \"name\" from package.json in {}",
+                                BStr::new(input_path)
+                            ),
+                        );
+                    } else {
+                        let mut cwd_buf = vec![0u8; MAX_PATH_BYTES];
+                        let cwd_len = bun_sys::getcwd(&mut cwd_buf).expect("unreachable");
+                        let _ = log.add_error_fmt(
                             Some(source),
                             arr.item_loc(source, i),
                             format_args!(
@@ -266,10 +270,10 @@ impl WorkspaceMap {
                                 BStr::new(&cwd_buf[..cwd_len]),
                             ),
                         );
-                        }
-                        continue;
                     }
-                };
+                    continue;
+                }
+            };
 
             if workspace_entry.name.len() == 0 {
                 continue;
@@ -278,7 +282,7 @@ impl WorkspaceMap {
             let rel_input_path = resolve_path::relative_platform::<path::platform::Auto, true>(
                 source.path.name().dir,
                 strings::without_suffix_comptime(
-                    abs_package_json_path.as_bytes(),
+                    abs_package_json_path,
                     const_format::concatcp!(SEP_STR, "package.json").as_bytes(),
                 ),
             );
@@ -333,7 +337,10 @@ impl WorkspaceMap {
                     b"package.json"
                 } else {
                     let parts: [&[u8]; 2] = [user_pattern, b"package.json"];
-                    arena.alloc_slice_copy(resolve_path::join::<path::platform::Auto>(&parts))
+                    let mut spill: Vec<u8> = Vec::new();
+                    arena.alloc_slice_copy(resolve_path::join_spill::<path::platform::Auto>(
+                        &mut spill, &parts,
+                    ))
                 };
 
                 let mut cwd = resolve_path::dirname::<path::platform::Auto>(source.path.text);
@@ -436,22 +443,20 @@ impl WorkspaceMap {
                         BStr::new(entry_dir)
                     );
 
-                    let abs_package_json_path = resolve_path::join_abs_string_buf_z::<
+                    let processed = match resolve_path::join_abs_string_buf_checked::<
                         path::platform::Auto,
                     >(
                         cwd, filepath_buf, &[entry_dir, b"package.json"]
-                    );
-                    let abs_workspace_dir_path: &[u8] = strings::without_suffix_comptime(
-                        abs_package_json_path.as_bytes(),
-                        b"package.json",
-                    );
-
-                    let workspace_entry = match process_workspace_name(
-                        json_cache,
-                        abs_package_json_path,
-                        log,
                     ) {
-                        Ok(e) => e,
+                        Some(abs_package_json_path) => {
+                            process_workspace_name(json_cache, abs_package_json_path, log)
+                                .map(|entry| (abs_package_json_path, entry))
+                        }
+                        None => Err(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG)),
+                    };
+
+                    let (abs_package_json_path, workspace_entry) = match processed {
+                        Ok(processed) => processed,
                         Err(err) => {
                             let entry_base: &[u8] = path::basename(matched_path);
                             if err == crate::Error::Sys(bun_errno::SystemErrno::ENOENT) {
@@ -488,6 +493,8 @@ impl WorkspaceMap {
                         continue;
                     }
 
+                    let abs_workspace_dir_path: &[u8] =
+                        strings::without_suffix_comptime(abs_package_json_path, b"package.json");
                     let workspace_path: &[u8] =
                         resolve_path::relative_platform::<path::platform::Auto, true>(
                             source.path.name().dir,

test/cli/install/bad-workspace.test.ts

@@ -1,7 +1,9 @@
-import { spawnSync } from "bun";
-import { beforeEach, expect, setDefaultTimeout, test } from "bun:test";
+import { spawn, spawnSync } from "bun";
+import { install_test_helpers } from "bun:internal-for-testing";
+import { beforeEach, describe, expect, setDefaultTimeout, test } from "bun:test";
 import { mkdirSync, writeFileSync } from "fs";
-import { bunEnv, bunExe, tempDir, tmpdirSync } from "harness";
+import { bunEnv, bunExe, isLinux, isWindows, tempDir, tmpdirSync } from "harness";
+import { dirname, join } from "path";
 
 let cwd: string;
 
@@ -143,3 +145,156 @@ test("workspace with .\\ should not crash", () => {
   expect(text).not.toContain("panic");
   expect(text).not.toContain("Internal assertion failure");
 });
+
+// Each `workspaces` entry is joined onto the project directory in a path buffer of
+// MAX_PATH_BYTES: 4096 bytes on Linux, 1024 on macOS (on Windows 32767 * 3 + 1, more than
+// any path the OS accepts, so the tests around the buffer size are POSIX only). Glob
+// entries are first joined onto "package.json" in a 4096 byte buffer on every platform.
+const POSIX_PATH_BUFFER_BYTES = isLinux ? 4096 : 1024;
+// Longer than either buffer on every platform.
+const LONG_ENTRY_BYTES = 100_000;
+
+const PKG1 = { "pkgs/pkg1/package.json": JSON.stringify({ name: "pkg1" }) };
+
+function rootPackageJson(workspaces: string[]) {
+  return JSON.stringify({ name: "root", workspaces });
+}
+
+async function runInstall(cwd: string) {
+  await using proc = spawn({
+    cmd: [bunExe(), "install"],
+    cwd,
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  return { stdout, stderr, exitCode };
+}
+
+// Installs and asserts that pkgs/pkg1 is the only workspace package that was found.
+async function expectOnlyPkg1Found(dir: string) {
+  const { stderr, exitCode } = await runInstall(dir);
+  expect(stderr).not.toContain("error:");
+  expect(exitCode).toBe(0);
+  expect(Object.values(install_test_helpers.parseLockfile(dir).workspace_paths)).toEqual(["pkgs/pkg1"]);
+}
+
+describe.concurrent("workspaces entries longer than the path buffer", () => {
+  test("path entry fails with ENAMETOOLONG", async () => {
+    const entry = Buffer.alloc(LONG_ENTRY_BYTES, "a").toString();
+    using dir = tempDir("bad-workspace-long-path", { "package.json": rootPackageJson([entry]) });
+
+    const { stderr, exitCode } = await runInstall(String(dir));
+
+    expect(stderr).toContain(`error: ENAMETOOLONG reading package.json for workspace package "${entry}"`);
+    expect(exitCode).toBe(1);
+  });
+
+  // A relative path of exactly `bytes` bytes made of one letter directory names, so that a
+  // path which fits the buffer is looked up by the OS (ENOENT) instead of exceeding its
+  // limit on the length of a single name.
+  function pathOfLength(bytes: number) {
+    const tail = bytes % 2 === 0 ? "dd" : "d";
+    return Buffer.alloc(bytes - tail.length, "d/").toString() + tail;
+  }
+
+  // The entry is read from `${dir}/${entry}/package.json`. One byte below the buffer size
+  // that path still reaches the OS; from the buffer size on it is rejected before that.
+  test.skipIf(isWindows).each([
+    ["one byte below", -1, "Workspace not found"],
+    ["exactly", 0, "ENAMETOOLONG reading package.json for workspace package"],
+    ["one byte above", 1, "ENAMETOOLONG reading package.json for workspace package"],
+  ])("path entry whose package.json path is %s the path buffer size", async (_, offset, message) => {
+    using dir = tempDir("bad-workspace-path-buffer-edge", {});
+    const prefixBytes = Buffer.byteLength(String(dir)) + "/".length;
+    const entry = pathOfLength(POSIX_PATH_BUFFER_BYTES + offset - prefixBytes - "/package.json".length);
+    writeFileSync(join(String(dir), "package.json"), rootPackageJson([entry]));
+
+    const { stderr, exitCode } = await runInstall(String(dir));
+
+    expect(stderr).toContain(`error: ${message} "${entry}"`);
+    expect(exitCode).toBe(1);
+  });
+
+  test("glob entry is still matched", async () => {
+    // A brace group padded far past the buffer size; its first alternative matches `pkgs`.
+    const entry = "{pkgs," + Buffer.alloc(LONG_ENTRY_BYTES, "x,").toString() + "x}/*";
+    using dir = tempDir("bad-workspace-long-glob", { "package.json": rootPackageJson([entry]), ...PKG1 });
+
+    await expectOnlyPkg1Found(String(dir));
+  });
+
+  test("glob entry matching nothing is skipped like any other glob", async () => {
+    const entry = Buffer.alloc(LONG_ENTRY_BYTES, "a").toString() + "/*";
+    using dir = tempDir("bad-workspace-long-glob-no-match", {
+      "package.json": rootPackageJson(["pkgs/*", entry]),
+      ...PKG1,
+    });
+
+    await expectOnlyPkg1Found(String(dir));
+  });
+
+  // What has to fit the buffer is the normalized path, not the entry as written.
+  test.each([
+    ["path", "pkgs/pkg1"],
+    ["glob", "pkgs/*"],
+  ])("%s entry that only fits the path buffer once normalized resolves", async (_, suffix) => {
+    const entry = Buffer.alloc(LONG_ENTRY_BYTES, "x/../").toString() + suffix;
+    using dir = tempDir("bad-workspace-long-normalized", { "package.json": rootPackageJson([entry]), ...PKG1 });
+
+    await expectOnlyPkg1Found(String(dir));
+  });
+
+  // Directory names for a relative path of exactly `bytes` bytes: "deep" followed by names
+  // of at most NAME_MAX (255) bytes. The last name is at least 128 bytes long so that its
+  // parent directory, used as a cwd below, stays well within PATH_MAX.
+  function deepDirectoryNames(bytes: number) {
+    const names = ["deep"];
+    let remaining = bytes - names[0].length;
+    while (remaining > 256) {
+      const length = remaining - 256 >= 129 ? 255 : remaining - 130;
+      names.push(Buffer.alloc(length, "a").toString());
+      remaining -= "/".length + length;
+    }
+    names.push(Buffer.alloc(remaining - "/".length, "b").toString());
+    return names;
+  }
+
+  // Globs are walked relative to the project directory, so a match can lie deeper than the
+  // buffer holds once the project directory is put in front of it. Windows has no such
+  // depth: the buffer there holds more than the longest path the OS accepts.
+  test.skipIf(isWindows)(
+    "glob match whose absolute package.json path does not fit fails with ENAMETOOLONG",
+    async () => {
+      using dir = tempDir("bad-workspace-deep-glob-match", { "package.json": rootPackageJson(["deep/**"]) });
+      // The directory itself fits PATH_MAX (so it can be created), `<dir>/package.json`
+      // does not fit the buffer.
+      const absoluteDirBytes = POSIX_PATH_BUFFER_BYTES - 6;
+      const names = deepDirectoryNames(absoluteDirBytes - Buffer.byteLength(String(dir)) - "/".length);
+      const workspaceDir = join(String(dir), ...names);
+      expect(Buffer.byteLength(workspaceDir)).toBe(absoluteDirBytes);
+      mkdirSync(workspaceDir, { recursive: true });
+      // Too long to be written by its absolute path: write it relative to the parent directory.
+      await using writer = spawn({
+        cmd: [
+          bunExe(),
+          "-e",
+          `require("fs").writeFileSync(process.argv.at(-1) + "/package.json", JSON.stringify({ name: "deep" }))`,
+          names.at(-1)!,
+        ],
+        cwd: dirname(workspaceDir),
+        env: bunEnv,
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      expect(await writer.stderr.text()).toBe("");
+      expect(await writer.exited).toBe(0);
+
+      const { stderr, exitCode } = await runInstall(String(dir));
+
+      expect(stderr).toContain(`error: ENAMETOOLONG reading package.json for workspace package "${names.join("/")}"`);
+      expect(exitCode).toBe(1);
+    },
+  );
+});