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

src/install/PackageManager/PackageManagerDirectories.rs

@@ -313,64 +313,75 @@
 #[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()) {
+        match Dir::cwd().make_open_path(&cache_dir.path, Default::default()) {
             Ok(d) => return d,
-            Err(err) => {
+            Err(err) if cache_dir.is_explicit => {
                 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::quote(&cache_dir.path),
                     bun_fmt::s(err.name())
                 );
                 Global::crash();
             }
+            Err(err) => {
+                bun_core::pretty_errorln!(
+                    "<r><yellow>warn<r>: cache directory {} is not creatable: {}, falling back to node_modules/.cache",
+                    bun_fmt::quote(&cache_dir.path),
+                    bun_fmt::s(err.name())
+                );
+                // 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(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();
         }
     }
 }
 
 pub struct CacheDir {
     pub path: Vec<u8>,
     pub is_node_modules: bool,
+    /// True only for `BUN_INSTALL_CACHE_DIR` / bunfig `install.cache.dir`.
+    pub is_explicit: bool,
 }
 
 pub fn fetch_cache_directory_path(env: &mut DotEnvLoader, options: Option<&Options>) -> CacheDir {
     if let Some(dir) = env.get(b"BUN_INSTALL_CACHE_DIR") {
         return CacheDir {
             path: FileSystem::instance().abs(&[dir]).to_vec(),
             is_node_modules: false,
+            is_explicit: true,
         };
     }
 
@@ -379,6 +390,7 @@
             return CacheDir {
                 path: FileSystem::instance().abs(&[opts.cache_directory]).to_vec(),
                 is_node_modules: false,
+                is_explicit: true,
             };
         }
     }
@@ -388,6 +400,7 @@
         return CacheDir {
             path: FileSystem::instance().abs(&parts).to_vec(),
             is_node_modules: false,
+            is_explicit: false,
         };
     }
 
@@ -396,6 +409,7 @@
         return CacheDir {
             path: FileSystem::instance().abs(&parts).to_vec(),
             is_node_modules: false,
+            is_explicit: false,
         };
     }
 
@@ -404,12 +418,14 @@
         return CacheDir {
             path: FileSystem::instance().abs(&parts).to_vec(),
             is_node_modules: false,
+            is_explicit: false,
         };
     }
 
     let fallback_parts: [&[u8]; 1] = [b"node_modules/.bun-cache"];
     CacheDir {
         is_node_modules: true,
+        is_explicit: false,
         path: FileSystem::instance().abs(&fallback_parts).to_vec(),
     }
 }

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,131 @@
+import { describe, expect, test } from "bun:test";
+import { bunEnv, bunExe, isWindows, tempDir } from "harness";
+import { existsSync, symlinkSync } from "node:fs";
+import { join } from "node:path";
+
+describe.skipIf(isWindows)("install cache directory 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"));
+    return { dir, root, dangling: join(root, "dangling"), cwd: join(root, "proj") };
+  }
+
+  function explicitEnv(cacheDir: string) {
+    return {
+      ...bunEnv,
+      BUN_INSTALL_CACHE_DIR: cacheDir,
+      BUN_CONFIG_REGISTRY: "http://127.0.0.1:1/",
+    };
+  }
+
+  test.concurrent("bun install exits with a cache-directory error instead of spinning in mkdirat", async () => {
+    const ctx = setup();
+    using _dir = ctx.dir;
+    const cacheDir = join(ctx.dangling, "cache");
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "install"],
+      cwd: ctx.cwd,
+      env: explicitEnv(cacheDir),
+      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(`error: cache directory "${cacheDir}" is not creatable: ENOENT`),
+    });
+    expect(stderr).not.toContain("ConnectionRefused");
+    expect(exitCode).not.toBe(0);
+  });
+
+  test.concurrent("bunfig install.cache.dir exits with a cache-directory error instead of spinning", async () => {
+    using dir = tempDir("cache-dir-dangling-bunfig", {
+      "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");
+    await Bun.write(
+      join(root, "proj", "bunfig.toml"),
+      `[install]\nregistry = "http://127.0.0.1:1/"\n[install.cache]\ndir = ${JSON.stringify(cacheDir)}\n`,
+    );
+    const env = { ...bunEnv };
+    delete env.BUN_INSTALL_CACHE_DIR;
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "install"],
+      cwd: join(root, "proj"),
+      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(`error: cache directory "${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;
+    const cacheDir = join(ctx.dangling, "cache");
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", `require("any-pkg")`],
+      cwd: ctx.cwd,
+      env: explicitEnv(cacheDir),
+      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(`error: cache directory "${cacheDir}" is not creatable: ENOENT`),
+    });
+    expect(exitCode).not.toBe(0);
+  });
+
+  test.concurrent("implicit $HOME-derived cache dir warns and falls back to node_modules/.cache", async () => {
+    const ctx = setup();
+    using _dir = ctx.dir;
+    const env = { ...bunEnv, BUN_CONFIG_REGISTRY: "http://127.0.0.1:1/" };
+    delete env.BUN_INSTALL_CACHE_DIR;
+    delete env.BUN_INSTALL;
+    delete env.XDG_CACHE_HOME;
+    env.HOME = ctx.dangling;
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "install"],
+      cwd: ctx.cwd,
+      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(`is not creatable: ENOENT, falling back to node_modules/.cache`),
+    });
+    expect(stderr).toContain("warn: cache directory");
+    expect(stderr).toContain("ConnectionRefused");
+    expect(existsSync(join(ctx.cwd, "node_modules", ".cache"))).toBe(true);
+    expect(exitCode).not.toBe(0);
+  });
+});