Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/bun_core/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ pub enum Error {
AccessDenied,
#[error("WriteFailed")]
WriteFailed,
#[error("CurrentWorkingDirectoryUnlinked")]
CurrentWorkingDirectoryUnlinked,
#[error(transparent)]
Alloc(#[from] bun_alloc::AllocError),
}
Expand All @@ -45,6 +47,7 @@ impl Error {
Self::FileNotFound => "FileNotFound",
Self::AccessDenied => "AccessDenied",
Self::WriteFailed => "WriteFailed",
Self::CurrentWorkingDirectoryUnlinked => "CurrentWorkingDirectoryUnlinked",
Self::Alloc(_) => "OutOfMemory",
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/bun_core/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4165,6 +4165,9 @@ fn getcwd_len(buf: &mut PathBuffer) -> crate::CrateResult<usize> {
unsafe {
let p = libc::getcwd(buf.0.as_mut_ptr().cast(), buf.0.len());
if p.is_null() {
if crate::ffi::errno() == libc::ENOENT {
return Err(crate::CrateError::CurrentWorkingDirectoryUnlinked);
}
return Err(crate::CrateError::Unexpected);
}
Ok(libc::strlen(p))
Expand Down
3 changes: 1 addition & 2 deletions src/resolver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,7 @@ pub mod fs {
Some(d) => DirnameStore::instance().append_slice(d)?,
None => {
let mut buf = bun_paths::PathBuffer::default();
let n = bun_sys::getcwd(&mut buf[..])?;
DirnameStore::instance().append_slice(&buf[..n])?
DirnameStore::instance().append_slice(bun_core::getcwd(&mut buf)?.as_bytes())?
}
};
// Seed the lower-tier `bun_paths::fs::FileSystem` singleton with the
Expand Down
6 changes: 2 additions & 4 deletions src/runtime/cli/Arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -834,8 +834,7 @@
let base: &[u8] = if bun_paths::is_absolute(cwd_arg) {
b"/"
} else {
let len = bun_sys::getcwd(&mut *outbuf)?;
&outbuf[..len]
bun_core::getcwd(&mut outbuf)?.as_bytes()

Check warning on line 837 in src/runtime/cli/Arguments.rs

View check run for this annotation

Claude / Claude Code Review

bun install --cwd . from a deleted cwd still prints the generic ENOENT fallback

The PR description says "Every other bun_sys::getcwd caller handles its error locally," but bun_install's own `--cwd` parser at `src/install/PackageManager/CommandLineArguments.rs:1554` still calls `bun_sys::getcwd(&mut buf[..])?` and propagates ENOENT to `handle_root_error` — package-manager commands skip `arguments::parse` because `USES_GLOBAL_OPTIONS[InstallCommand]` is false. It's the structural twin of the `--cwd` site fixed here, so `bun install --cwd .` from a deleted directory still prin

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The PR description says "Every other bun_sys::getcwd caller handles its error locally," but bun_install's own --cwd parser at src/install/PackageManager/CommandLineArguments.rs:1554 still calls bun_sys::getcwd(&mut buf[..])? and propagates ENOENT to handle_root_error — package-manager commands skip arguments::parse because USES_GLOBAL_OPTIONS[InstallCommand] is false. It's the structural twin of the --cwd site fixed here, so bun install --cwd . from a deleted directory still prints the generic "Bun could not find a file" fallback; switching that call to bun_core::getcwd would close the class.

Extended reasoning...

What the bug is

The PR converts three bun_sys::getcwd call sites to bun_core::getcwd so that a deleted cwd surfaces CurrentWorkingDirectoryUnlinked at handle_root_error and prints the actionable "The current working directory was deleted…" hint. The PR description justifies stopping at three sites with: "Every other bun_sys::getcwd caller handles its error locally and is unchanged." That claim is incorrect for the package-manager --cwd parser, which still propagates a bare ENOENT all the way to handle_root_error and hits the generic fallback this PR set out to eliminate.

The specific code path

Package-manager subcommands (install, add, remove, update, link, pm, …) do not go through the modified arguments::parse in src/runtime/cli/Arguments.rs. USES_GLOBAL_OPTIONS[Tag::InstallCommand] is false (src/options_types/command_tag.rs:266), so create_context_data() at src/runtime/cli/mod.rs:1136 skips arguments::parse entirely for the whole family. Instead these commands parse --cwd inside bun_install::CommandLineArguments::parse:

// src/install/PackageManager/CommandLineArguments.rs:1549-1555
if let Some(cwd_) = args.option(b"--cwd") {
    let mut buf = PathBuffer::uninit();
    let mut buf2 = PathBuffer::uninit();
    let final_path: &mut bun_core::ZStr = if !cwd_.is_empty() && cwd_[0] == b'.' {
        let cwd_len = bun_sys::getcwd(&mut buf[..])?;   // ← still bun_sys, propagates via ?

Why the existing changes don't cover it

  • The Arguments.rs:837 conversion never runs for install-family commands (gated out by USES_GLOBAL_OPTIONS).
  • The resolver/lib.rs conversion (FileSystem::init(None)) is reached later, inside PackageManager::init (src/install/PackageManager.rs:1498), but the --cwd . branch fails before PackageManager::init is called — so the resolver-side fix never fires on this path.
  • The final diff no longer touches src/sys/Error.rs (commit 4c99108 "Produce CurrentWorkingDirectoryUnlinked only from bun_core::getcwd" removed the cross-cutting to_named_core_err approach), so bun_sys::getcwd still surfaces a bare errno.

Step-by-step proof

  1. mkdir /tmp/gone && cd /tmp/gone && rmdir /tmp/gone && bun install --cwd .
  2. USES_GLOBAL_OPTIONS[InstallCommand] == falsecreate_context_data skips arguments::parse; dispatch calls install()bun_install::CommandLineArguments::parse.
  3. --cwd is ".", so cwd_[0] == b'.' → line 1554 calls bun_sys::getcwd(&mut buf[..]), which fails with errno == ENOENT.
  4. ? converts via From<bun_sys::Error> for bun_install::Error (src/install/error.rs:405-408) → bun_install::Error::Sys(SystemErrno::ENOENT).
  5. InstallCommand::handle_error (src/runtime/cli/install_command.rs:29-44) matches only InstallFailed/InvalidPackageJSON, so the error propagates as bun_runtime::Error::Install(...)mainhandle_root_error.
  6. name() chain: runtime::Error::Install(e).name()install::Error::Sys(e).name() (src/install/error.rs:381) → <&str>::from(SystemErrno::ENOENT)"ENOENT".
  7. handle_root_error matches the b"ENOENT" arm at src/crash_handler/lib.rs:1423 and prints:
    ENOENT: Bun could not find a file, and the code that produces this error is missing a better error.
    
    — the exact generic fallback this PR targets, not the CurrentWorkingDirectoryUnlinked hint.

Impact

Low. The trigger requires the combination of (a) a deleted cwd and (b) an explicit relative --cwd argument to a package-manager subcommand. No crash or incorrect behavior; the user just gets the unhelpful message instead of the hint. But per REVIEW.md ("Fix the whole class in the same PR — same-class sites are ONE concern, not scope creep"), this is a same-class sibling of the three converted sites: it resolves a relative --cwd against getcwd() and propagates the failure to handle_root_error, exactly like the Arguments.rs:837 twin. And the "intentionally excluded" rationale in the PR description is factually wrong for this site — it does not handle its error locally.

Fix

One-line: switch src/install/PackageManager/CommandLineArguments.rs:1554 from bun_sys::getcwd to bun_core::getcwd, matching the pattern applied at Arguments.rs:837:

let cwd = bun_core::getcwd(&mut buf)?.as_bytes();

(and either update the PR description's exclusion claim, or drop it).

};
let out = resolve_path::join_abs::<platform::Loose>(base, cwd_arg);
// `chdir` wants a NUL-terminated path; `join_abs` returns a borrowed
Expand Down Expand Up @@ -869,8 +868,7 @@
// Everything else (install/test/build/...) must not silently act on
// whatever project happens to live above the executable.
let mut temp = PathBuffer::uninit();
let len = bun_sys::getcwd(&mut *temp)?;
Box::<[u8]>::from(&temp[..len])
Box::<[u8]>::from(bun_core::getcwd(&mut temp)?.as_bytes())
};

// Not gated on .BunxCommand: bunx skips Arguments.parse entirely
Expand Down
4 changes: 1 addition & 3 deletions test/bundler/bun-build-compile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -758,10 +758,8 @@ describe("compiled binary in a deleted cwd", () => {
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

// The entry never runs (VM init aborts first), the ENOENT surfaces, and the
// process exits 1 — a crash would terminate via a signal, never exit 1.
expect(stdout).toBe("");
expect(stderr).toContain("ENOENT");
expect(stderr).toContain("The current working directory was deleted");
expect(exitCode).toBe(1);
},
60_000,
Expand Down
40 changes: 40 additions & 0 deletions test/cli/run/run-crash-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,46 @@ describe.if(isPosix)("terminal signal reflects the crash cause", () => {
});
});

// POSIX-only: Windows refuses to remove a directory that is any process's cwd.
describe.if(isPosix)("cwd deleted before startup", () => {
test.concurrent.each(["install", "test"])("bun %s prints the cwd-deleted hint", async cmd => {
using dir = tempDir("cwd-unlinked", {});
const gone = String(dir);

await using proc = Bun.spawn({
cmd: ["/bin/sh", "-c", `cd "${gone}" && rmdir "${gone}" && exec "${bunExe()}" '${cmd}'`],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect({ stdout, stderr, exitCode }).toEqual({
stdout: "",
stderr: expect.stringContaining("The current working directory was deleted"),
exitCode: 1,
});
expect(stderr).not.toContain("Bun could not find a file");
});

test.concurrent("bun -e boots via the exe-dir fallback instead", async () => {
using dir = tempDir("cwd-unlinked-run", {});
const gone = String(dir);

await using proc = Bun.spawn({
cmd: ["/bin/sh", "-c", `cd "${gone}" && rmdir "${gone}" && exec "${bunExe()}" -e 'console.log(1)'`],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout).toBe("1\n");
expect(stderr).toBe("");
expect(exitCode).toBe(0);
});
});

// Windows: the VEH handler must walk the stack from the fault CONTEXT record
// (RtlVirtualUnwind), not from inside the handler. When the fault is in an
// external DLL the old RtlCaptureStackBackTrace path could stop at
Expand Down