paths: terminate recursive mkdir walk when a confirmed parent still yields ENOENT

src/install/PackageManager/PackageManagerDirectories.rs

@@ -313,50 +313,48 @@
 #[cold]
 #[inline(never)]
 unsafe fn ensure_cache_directory(this: *mut PackageManager) -> Dir {
-    loop {
-        // SAFETY: field projections through the caller-provided provenance
-        // root; see fn safety contract. Project `enable` narrowly so callers
-        // may hold borrows into disjoint `options` sub-fields.
-        if unsafe { (*this).options.enable.contains(Enable::CACHE) } {
-            // SAFETY: caller-provided provenance root; `env_mut()` itself
-            // encapsulates the BackRef deref + singleton-liveness invariant.
-            let env = unsafe { &*this }.env_mut();
-            // SAFETY: shared read of `options`; disjoint from `cache_directory_path`.
-            let cache_dir = fetch_cache_directory_path(env, Some(unsafe { &(*this).options }));
-            // SAFETY: see fn safety contract.
-            unsafe { (*this).cache_directory_path = ZBox::from_bytes(&cache_dir.path) };
-
-            match Dir::cwd().make_open_path(&cache_dir.path, Default::default()) {
-                Ok(d) => return d,
-                Err(_) => {
-                    // SAFETY: narrow `&mut enable` projection; disjoint from
-                    // any `&options.{registries,scope}` the caller may hold.
-                    unsafe { (*this).options.enable.set(Enable::CACHE, false) };
-                    // SAFETY: see fn safety contract.
-                    unsafe { (*this).cache_directory_path = ZBox::from_bytes(b"") };
-                    continue;
-                }
-            }
-        }
-
+    // SAFETY: field projections through the caller-provided provenance
+    // root; see fn safety contract. Project `enable` narrowly so callers
+    // may hold borrows into disjoint `options` sub-fields.
+    if unsafe { (*this).options.enable.contains(Enable::CACHE) } {
+        // SAFETY: caller-provided provenance root; `env_mut()` itself
+        // encapsulates the BackRef deref + singleton-liveness invariant.
+        let env = unsafe { &*this }.env_mut();
+        // SAFETY: shared read of `options`; disjoint from `cache_directory_path`.
+        let cache_dir = fetch_cache_directory_path(env, Some(unsafe { &(*this).options }));
         // SAFETY: see fn safety contract.
-        unsafe {
-            (*this).cache_directory_path =
-                ZBox::from_bytes(path::resolve_path::join_abs_string::<path::platform::Auto>(
-                    FileSystem::instance().top_level_dir(),
-                    &[b"node_modules", b".cache"],
-                ))
-        };
+        unsafe { (*this).cache_directory_path = ZBox::from_bytes(&cache_dir.path) };
 
-        match Dir::cwd().make_open_path(b"node_modules/.cache", Default::default()) {
-            Ok(d) => return d,
+        return match Dir::cwd().make_open_path(&cache_dir.path, Default::default()) {
+            Ok(d) => d,
             Err(err) => {
                 bun_core::pretty_errorln!(
-                    "<r><red>error<r>: bun is unable to write files: {}",
+                    "<r><red>error<r>: cache directory \"{}\" is not creatable: {}",
+                    bun_fmt::s(&cache_dir.path),
                     bun_fmt::s(err.name())
                 );
                 Global::crash();
             }
+        };
+    }
+
+    // SAFETY: see fn safety contract.
+    unsafe {
+        (*this).cache_directory_path =
+            ZBox::from_bytes(path::resolve_path::join_abs_string::<path::platform::Auto>(
+                FileSystem::instance().top_level_dir(),
+                &[b"node_modules", b".cache"],
+            ))
+    };
+
+    match Dir::cwd().make_open_path(b"node_modules/.cache", Default::default()) {
+        Ok(d) => d,
+        Err(err) => {
+            bun_core::pretty_errorln!(
+                "<r><red>error<r>: bun is unable to write files: {}",
+                bun_fmt::s(err.name())
+            );
+            Global::crash();
         }
     }
 }

src/paths/component_iterator.rs

@@ -193,8 +193,8 @@ pub enum MakePathStep<E> {
     Created,
     /// Directory already exists (`EEXIST`). Walk advances forward.
     Exists,
-    /// A parent is missing (`ENOENT`). Walk steps back one component;
-    /// if there is no previous component the carried error is returned.
+    /// A parent is missing (`ENOENT`). Returns the error if the walk has
+    /// already advanced or there is no previous component; else steps back.
     NotFound(E),
 }
 
@@ -217,14 +217,17 @@ pub fn make_path_with<'a, T: PathChar, E>(
     let Some(mut comp) = it.last() else {
         return Ok(());
     };
+    let mut advanced = false;
     loop {
         match mkdir(comp.path)? {
             MakePathStep::Created | MakePathStep::Exists => {
+                advanced = true;
                 comp = match it.next() {
                     Some(c) => c,
                     None => return Ok(()),
                 };
             }
+            MakePathStep::NotFound(e) if advanced => return Err(e),
             MakePathStep::NotFound(e) => {
                 comp = match it.previous() {
                     Some(c) => c,
@@ -389,4 +392,50 @@ mod tests {
         assert_eq!(it.next().unwrap().name, b"c");
         assert!(it.next().is_none());
     }
+
+    #[test]
+    fn make_path_terminates_when_parent_exists_but_child_is_enoent() {
+        // `/a/b` is a dangling symlink: EEXIST itself, ENOENT for any child.
+        let it = ComponentIterator::init(&b"/a/b/c"[..], PathFormat::Posix).unwrap();
+        let mut calls = 0u32;
+        let r = make_path_with(it, |p| {
+            calls += 1;
+            assert!(calls < 100, "runaway loop");
+            match p {
+                b"/a" | b"/a/b" => Ok(MakePathStep::Exists),
+                b"/a/b/c" => Ok(MakePathStep::NotFound(())),
+                _ => unreachable!(),
+            }
+        });
+        assert!(r.is_err());
+        // leaf ENOENT → parent EEXIST → leaf ENOENT → stop.
+        assert_eq!(calls, 3);
+    }
+
+    #[test]
+    fn make_path_walks_back_then_forward() {
+        // `/a` exists, `/a/b` and `/a/b/c` do not.
+        let it = ComponentIterator::init(&b"/a/b/c"[..], PathFormat::Posix).unwrap();
+        let mut created: Vec<&[u8]> = vec![];
+        let mut calls = 0u32;
+        let r = make_path_with::<u8, ()>(it, |p| {
+            calls += 1;
+            assert!(calls < 100, "runaway loop");
+            if p == b"/a" {
+                return Ok(MakePathStep::Exists);
+            }
+            if created.iter().any(|c| *c == p) {
+                return Ok(MakePathStep::Exists);
+            }
+            let parent = &p[..p.iter().rposition(|b| *b == b'/').unwrap()];
+            if parent == b"/a" || created.iter().any(|c| *c == parent) {
+                created.push(p);
+                Ok(MakePathStep::Created)
+            } else {
+                Ok(MakePathStep::NotFound(()))
+            }
+        });
+        assert!(r.is_ok());
+        assert_eq!(created, vec![&b"/a/b"[..], &b"/a/b/c"[..]]);
+    }
 }

test/cli/install/bun-install-cache-dir.test.ts

@@ -0,0 +1,70 @@
+import { describe, expect, test } from "bun:test";
+import { bunEnv, bunExe, isWindows, tempDir } from "harness";
+import { symlinkSync } from "node:fs";
+import { join } from "node:path";
+
+describe.skipIf(isWindows)("BUN_INSTALL_CACHE_DIR inside a dangling symlink", () => {
+  function setup() {
+    const dir = tempDir("cache-dir-dangling", {
+      "proj/package.json": JSON.stringify({
+        name: "x",
+        version: "1.0.0",
+        dependencies: { "any-pkg": "1.0.0" },
+      }),
+    });
+    const root = String(dir);
+    symlinkSync(join(root, "does-not-exist"), join(root, "dangling"));
+    const cacheDir = join(root, "dangling", "cache");
+    return {
+      dir,
+      cacheDir,
+      env: {
+        ...bunEnv,
+        BUN_INSTALL_CACHE_DIR: cacheDir,
+        BUN_CONFIG_REGISTRY: "http://127.0.0.1:1/",
+      },
+      cwd: join(root, "proj"),
+    };
+  }
+
+  test.concurrent("bun install exits with a cache-directory error instead of spinning in mkdirat", async () => {
+    const ctx = setup();
+    using _dir = ctx.dir;
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "install"],
+      cwd: ctx.cwd,
+      env: ctx.env,
+      stdout: "pipe",
+      stderr: "pipe",
+      timeout: 15_000,
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect({ signalCode: proc.signalCode, stdout, stderr }).toMatchObject({
+      signalCode: null,
+      stderr: expect.stringContaining(`cache directory "${ctx.cacheDir}" is not creatable: ENOENT`),
+    });
+    expect(stderr).not.toContain("ConnectionRefused");
+    expect(exitCode).not.toBe(0);
+  });
+
+  test.concurrent("runtime auto-install exits with a cache-directory error instead of spinning", async () => {
+    const ctx = setup();
+    using _dir = ctx.dir;
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", `require("any-pkg")`],
+      cwd: ctx.cwd,
+      env: ctx.env,
+      stdout: "pipe",
+      stderr: "pipe",
+      timeout: 15_000,
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect({ signalCode: proc.signalCode, stdout, stderr }).toMatchObject({
+      signalCode: null,
+      stderr: expect.stringContaining(`cache directory "${ctx.cacheDir}" is not creatable: ENOENT`),
+    });
+    expect(exitCode).not.toBe(0);
+  });
+});