crash_handler: do not run the crash report upload in the crashed process's cwd

[robobun]

Problem

• On Windows, once a bun process has crashed with crash reporting enabled (release builds on Windows and macOS, canary builds, or BUN_CRASH_REPORT_URL), the directory that was its cwd cannot be removed for roughly a second after the parent has already seen it exit: rmSync fails with EBUSY: resource busy or locked, rm '<cwd>'. With a release binary, any test that crashes a child inside a tempDir cwd fails at disposal with an opaque SuppressedError instead of its real assertion; tooling that deletes a scratch cwd right after a crash gets the same EBUSY. Normal exits, process.exit() and process.abort() do not show this, only the crash handler path.
• Cause: report() in src/crash_handler/lib.rs records the report by spawning a fire-and-forget child (powershell.exe ... Invoke-RestMethod <url>/ack on Windows, curl on POSIX) and the crashing process exits right after spawning it. That child inherited the crashing process's cwd: CreateProcessW was called with a null lpCurrentDirectory, and the fork()ed curl child never changed directory. Windows refuses to delete a directory while it is any process's cwd, so the uploader pins the directory until PowerShell has started and finished its request.
• POSIX has the same inheritance. It does not block rm there, but the uploader still holds the user's directory for as long as it runs (visible as its /proc/<pid>/cwd, and it keeps that mount busy). Two tests already carried per-test workarounds for the Windows symptom: the retrying rmSync in the system-PowerShell test in test/cli/run/run-crash-handler.test.ts and the "no cwd override" comment in test/cli/run/crash-report-command-char.test.ts.

Fix

• Windows: pass the system directory as lpCurrentDirectory. report() already looks it up to build the absolute powershell.exe path; it always exists and is not something a user is about to delete, and the command line contains nothing relative. This is the fixing line (sysdir.as_ptr() in the CreateProcessW call); the rest of that hunk is the updated SAFETY comment.
• POSIX: chdir("/") in the forked child before execve. chdir is async-signal-safe, and the curl path comes from which() resolved against an absolute cwd, so it is absolute and unaffected by the change of directory. Same defect in the sibling branch, fixed the same way.
• Why here: the upload child is the only thing that outlives the crashing process and it needs nothing from the cwd, so giving it its own directory removes the hold without delaying the crash exit or pushing a workaround into every caller that cleans up after a crashed bun.
• Tests, in test/cli/run/run-crash-handler.test.ts. A shared helper crashes a child in a given cwd and has a local Bun.serve hold the /ack request open, so the uploader is known to be fully started and still alive when the check runs; no timing is involved:
  • Windows (new): rmSync of the crashed child's cwd succeeds while the upload is in flight. Unfixed (1.4.0-canary.1 on Windows Server 2019) it fails with EBUSY on 3/3 runs; with this branch's debug build it passes on every run (8 so far).
  • Linux (new): no process has the crashed child's cwd (/proc/*/cwd). Unfixed (1.4.0) it fails with holders ["curl"]; fixed it passes.
  • The existing "runs the system PowerShell, not a powershell.exe in the working directory" test now goes through the same helper: using dir, decoy powershell.exe in the cwd, and the ack awaited directly. The ack is the discriminating assertion (the decoy is a copy of bun and can never ack, so a regression times out), which is also what test(crash_handler): await the PowerShell ack instead of racing a 2s sleep #36550 changes this test to rely on; its Bun.sleep(2000) race, the retrying rmSync cleanup and the expect(stderr).toContain(url) check (the URL is printed before report() runs, so it passes regardless of which PowerShell was started, and the automatic crash reporter tests assert it on every platform) are gone. This supersedes test(crash_handler): await the PowerShell ack instead of racing a 2s sleep #36550; crash_handler: put the executable's debug id in the trace string #38838 carries the same one-line change and will need a trivial conflict resolution against this.
  • The stale comment in crash-report-command-char.test.ts is removed.
• Other runs: both test files pass with the debug build on Linux (21 pass) and on Windows x64 (11 pass in run-crash-handler.test.ts, the rest platform-skipped); cargo check -p bun_crash_handler passes for aarch64-apple-darwin, x86_64-pc-windows-msvc, aarch64-pc-windows-msvc and x86_64-unknown-freebsd.

Background

• Crash report upload: when reporting is enabled, the crash handler prints a bun.report trace URL to stderr and spawns a child that requests <trace url>/ack so the report gets recorded, then terminates the process immediately without waiting for that child. Debug builds only report when BUN_CRASH_REPORT_URL is set, which is how the tests point the upload at a local server.
• Windows cwd handle: every Windows process keeps an open handle to its current directory for its whole lifetime, and RemoveDirectory fails with a sharing violation (surfaced by libuv as EBUSY) while any process has the directory as its cwd. CreateProcessW's lpCurrentDirectory sets the child's cwd; null means "same as the parent". A process's own cwd handle is released when it exits, so after the parent has observed bun's exit the uploader is the only holder left.
• Decoy test: CreateProcessW with a bare image name searches the parent's current directory before the system directories, which is why a powershell.exe planted in the crashing process's cwd detects a regression to a bare name (Robustness pass across install, css, ffi, crypto, spawn, shell, and node compat #36165 switched to the absolute path). lpCurrentDirectory does not change that search (it uses the parent's cwd), so the test keeps its meaning after this change.
• which() (src/which/lib.rs): resolves a bare binary name against $PATH, prefixing relative $PATH entries with the cwd it is given, so with an absolute cwd every result is an absolute path.

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/run/run-crash-handler.test.ts

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9e7d4861-194e-431d-aea6-aa519015f924

📥 Commits

Reviewing files that changed from the base of the PR and between 8437683 and 0c4ca81.

📒 Files selected for processing (3)

• src/crash_handler/lib.rs
• test/cli/run/crash-report-command-char.test.ts
• test/cli/run/run-crash-handler.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: reproduced and fixed, waiting on CI.

• Reproduced on Windows Server 2019 with 1.4.0-canary.1: after a child bun crashes with reporting enabled, rmSync of its cwd fails with EBUSY while the crash reporter's PowerShell child is still running (3/3 runs with the new test). On Linux the released 1.4.0 shows the same inheritance: the curl child has the crashed process's cwd (/proc/<pid>/cwd).
• Fix is in src/crash_handler/lib.rs report(): lpCurrentDirectory = system directory on Windows, chdir("/") before execve on POSIX.
• Tests in test/cli/run/run-crash-handler.test.ts fail on the unfixed binaries on both platforms and pass with this branch's debug build on Linux and Windows x64. The existing system-PowerShell test was folded into the same helper (see the description), which supersedes test(crash_handler): await the PowerShell ack instead of racing a 2s sleep #36550.

[claude]

LGTM — two-line native change (pass sysdir as lpCurrentDirectory on Windows; chdir("/") in the forked child on POSIX) with clean test coverage on both platforms.

What was reviewed:

• Verified sysdir is zero-initialized and length-checked after GetSystemDirectoryW, so it is a valid NUL-terminated wide string for lpCurrentDirectory.
• Confirmed chdir is async-signal-safe and the curl path from which() (seeded with getcwd()) is absolute, so the child's cwd change cannot affect the exec.
• Checked the new tests await the actual /ack connection (no sleep-for-condition), use a local port: 0 server, and clean up via bounded polling only on the failure path.
• Confirmed the dedented system-PowerShell test body is behaviorally identical apart from switching to using dir, which this fix now makes safe.

Extended reasoning...

Overview

This PR changes the crash-report uploader spawned by report() in src/crash_handler/lib.rs so it does not inherit the crashing process's cwd. On Windows, CreateProcessW's lpCurrentDirectory is switched from NULL to the already-computed sysdir buffer (the system directory used to build the absolute PowerShell path). On POSIX, a libc::chdir(c"/") is inserted in the forked child before execve. The rest is comments (doc comment on report(), updated SAFETY comment) and tests: a new two-test describe block in run-crash-handler.test.ts covering Windows (rmSync succeeds while the upload is in flight) and Linux (no /proc/*/cwd points at the crashed cwd), the existing system-PowerShell test dedented and switched to using dir now that its retry-cleanup workaround is obsolete, and a stale explanatory comment removed from crash-report-command-char.test.ts.

Security risks

None. The Windows change points the child at a directory the code already trusts and reads (it builds the PowerShell path from it); the POSIX change moves a fire-and-forget curl child to /. Nothing user-controllable is introduced, no new inputs are parsed, and the code only runs after the process has already crashed.

Level of scrutiny

Low-to-medium. The native change is two effective lines in a cold, post-crash path with no hot-path or memory-lifetime implications. I verified the invariants each line depends on directly in the surrounding code: sysdir is [0u16; 300] (zero-initialized) and gated by sysdir_len == 0 || sysdir_len >= sysdir.len() before use, so sysdir[sysdir_len] is guaranteed 0 and sysdir.as_ptr() is a valid NUL-terminated LPCWSTR. On POSIX, chdir is on the POSIX async-signal-safe list, c"/" is a static C string, and argv[0] is absolute because which() was called with getcwd() output as its cwd argument. Ignoring chdir's return is harmless — failure just leaves the pre-fix behavior.

Other factors

The tests are well-constructed against the repo's review criteria: they await the observable condition (the held /ack request) rather than sleeping, use a local Bun.serve({ port: 0 }), isolate in tempDir, and the Windows test's cleanup poll is registered in finally and never throws over the real assertion. The PR description documents unfixed/fixed runs on both platforms plus cargo check across four targets. No CODEOWNERS entry covers src/crash_handler/, there are no outstanding reviewer comments, and the bug-hunting system found nothing.

[robobun]

^{Updated 11:05 AM PT - Aug 15th, 2026}

❌ @robobun, your commit 0c4ca81 has some failures in Build #98276 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38979

That installs a local version of the PR into your bun-38979 executable, so you can run:

bun-38979 --bun

[claude]

Beyond the inline nit, I verified the two load-bearing lines: sysdir is guaranteed NUL-terminated for lpCurrentDirectory (zero-initialized [0u16; 300] and the sysdir_len >= sysdir.len() early-return leaves at least one trailing zero), and on POSIX the curl path from which() is absolute because it's resolved against getcwd(), so chdir("/") before execve cannot break the exec. The held-open /ack request in the new tests pins the uploader alive without any timing dependency.

Extended reasoning...

The core fix is two lines and both check out against the surrounding code in report(). The one open item is the inline nit: commit 9976e49 reverted the system-PowerShell test's using conversion but the PR description still claims it, and the now-unexplained retrying rmSync remains. The comment-cop flag at lib.rs:2788 is also still on the thread.

[claude]

LGTM — the concern from my earlier review (retrying rmSync left behind after its comment was deleted) is addressed in fec570d: the system-PowerShell test now goes through the shared helper with using dir, and the comment-cop paragraph was trimmed in 0c4ca81.

What was reviewed:

• sysdir is zero-initialized and length-checked (sysdir_len < 300), so sysdir.as_ptr() is a valid NUL-terminated wide string for lpCurrentDirectory.
• chdir is async-signal-safe and the curl path from which(getcwd(), ...) is absolute, so the chdir("/") before execve cannot break the exec.
• is_reporting_enabled() returns early on BUN_CRASH_REPORT_URL, so the helper not clearing GITHUB_ACTIONS/CI (unlike the old test) does not affect whether the upload fires.
• The helper awaits both proc.exited and the ack before running inspect(), so the Windows rmSync and Linux /proc/*/cwd checks run against a live uploader with no timing dependence.

Extended reasoning...

Overview

Two behavioral lines in src/crash_handler/lib.rs report(): on Windows, CreateProcessW's lpCurrentDirectory changes from null to sysdir.as_ptr() (the system directory already looked up to build the absolute PowerShell path); on POSIX, libc::chdir(c"/".as_ptr()) is inserted between fork() and execve() in the child. The rest is a SAFETY-comment update, ~120 net test lines in run-crash-handler.test.ts (a shared helper, two new platform-gated tests, and the existing system-PowerShell test folded into the helper), and a stale-comment deletion in crash-report-command-char.test.ts.

Security risks

None identified. The Windows change moves the uploader's cwd from user-controlled to GetSystemDirectoryW(); the command line is unchanged and contains nothing relative. The POSIX change moves the forked child's cwd to /; chdir is on the POSIX async-signal-safe list and c"/" is a static C string. The curl argv is built before the fork from an absolute path (which() given getcwd()), so the directory change cannot alter what is exec'd.

Level of scrutiny

Moderate. The crash handler runs post-fault and must stay async-signal-safe, but the delta is two lines whose invariants are verifiable locally: sysdir is [0u16; 300] with sysdir_len == 0 || sysdir_len >= sysdir.len() guarding the return, so the trailing NUL at sysdir[sysdir_len] is guaranteed by zero-init; and the POSIX branch already only calls async-signal-safe functions between fork and execve. The larger test churn is a net de-flake — the old Bun.sleep(2000) race and retrying rmSync are gone, replaced by a helper that holds the /ack request open so the uploader is provably alive during inspection.

Other factors

All prior feedback is addressed and resolved: my earlier nit about the orphaned retrying rmSync was fixed in fec570d (test now uses using dir via the shared helper), and both comment-cop hits were fixed in 0c4ca81 (one-line note at the argument site). I confirmed is_reporting_enabled() (src/crash_handler/lib.rs:2744) short-circuits on a non-empty BUN_CRASH_REPORT_URL before any CI check, so dropping GITHUB_ACTIONS: undefined / CI: undefined from the refactored test's env is harmless. run-crash-handler.test.ts is not listed in test/expectations.txt, so the new tests will run in CI. The PR description documents unfixed/fixed runs on both platforms and cargo check across four targets.