fs: reject empty mkdtemp prefix with EINVAL

[robobun]

What

fs.mkdtemp('') and fs.mkdtempSync('') (and the promises variant) now reject with EINVAL instead of creating a bare six-random-character directory in the process cwd.

Repro

import fs from "node:fs";
import fsp from "node:fs/promises";
try { fs.mkdtempSync(""); } catch (e) { console.log("sync:", e.code); }
try { await fsp.mkdtemp(""); } catch (e) { console.log("promises:", e.code); }

Before: creates gQOino-style directories in cwd with no error.
After (and Node v26.3.0): EINVAL for both.

Cause

Bun appends XXXXXX to the prefix and calls mkdtemp(3). With an empty prefix the template is exactly XXXXXX, which mkdtemp(3) accepts, creating a random-named directory in cwd. Node's C++ binding happens to produce a 5-X template for the empty-prefix case (an snprintf size quirk), which mkdtemp(3) rejects with EINVAL. The Node behavior is the useful one: a caller whose prefix resolves to '' (e.g. mkdtemp(cfg.tmpPrefix ?? '')) should get an error rather than litter cwd.

Fix

Return EINVAL from NodeFS::mkdtemp before touching the filesystem when the prefix slice is empty. Applies to sync, callback, and promise forms.

Verification

USE_SYSTEM_BUN=1 bun test test/js/node/fs/fs.test.ts -t "mkdtemp empty prefix"  # 3 fail
bun bd test test/js/node/fs/fs.test.ts -t "mkdtemp empty prefix"                 # 3 pass
bun bd test test/js/node/fs/fs.test.ts -t "mkdtemp"                              # 19 pass

Node parallel tests test-fs-mkdtemp.js, test-fs-mkdtemp-prefix-check.js, test-fs-mkdtempDisposableSync.js, and test-fs-promises-mkdtempDisposable.js all pass.

---

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/fs/fs.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: 9 minutes

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: 0ec4c031-d641-4ac3-b9d9-ab5d6fcf4265

📥 Commits

Reviewing files that changed from the base of the PR and between 5b98630 and fd2dd9c.

📒 Files selected for processing (2)

• src/runtime/node/node_fs.rs
• test/js/node/fs/fs.test.ts

---

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

[robobun]

Reproduced on 1.4.0-canary (system bun creates a random-named dir in cwd; Node v26.3.0 throws EINVAL). Fix returns EINVAL before the syscall when the prefix is empty; err.path matches Node's "XXXXX".

PR: #34908

CI (build 76783): fs.test.ts and the node-parallel mkdtemp tests pass on every lane. Remaining failures are all tagged flaky by the helper and unrelated to this diff (compile-windows-metadata, spawn.test.ts timeout on win-aarch64, require-cache leak on alpine, bun-upgrade ETXTBSY, fetch-gzip/http2 on darwin, etc.). Ready for review.

[robobun]

^{Updated 4:31 AM PT - Jul 21st, 2026}

✅ @robobun, your commit fd2dd9c0693f05badb206b3028c1bc74be02a869 passed in Build #76783! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 34908

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

bun-34908 --bun

[claude]

LGTM — the earlier 5-X err.path note is addressed, and the remaining inline comment is a cosmetic dead-guard cleanup that doesn't affect behavior.

Extended reasoning...

Overview

Adds a 10-line early return in NodeFS::mkdtemp (src/runtime/node/node_fs.rs:6094) that rejects an empty prefix with EINVAL before any syscall, matching Node's observed behavior (including err.path === "XXXXX"). Three tests in test/js/node/fs/fs.test.ts cover the sync, callback, and promises entry points.

Security risks

None. The change short-circuits before touching the filesystem; no new allocation, no user-controlled data reaches a syscall on the new path. The sys::Error construction mirrors the existing error path a few lines below.

Level of scrutiny

Low. This is a narrow Node-compat edge-case fix at the shared NodeFS::mkdtemp layer (all three JS entry points route through it — confirmed there is only one fn mkdtemp in this file). [b'X'; 5].into() correctly targets sys::Error.path: Box<[u8]>. The tests use strong assertions (toMatchObject on code + syscall), the callback test resolves via Promise.withResolvers rather than throwing in the callback, and the .rejects case is awaited.

Other factors

My previous review flagged the 6-X vs 5-X err.path mismatch; the author fixed it in 80f640a and updated the code comment to note Node's snprintf quirk — that thread is resolved. The one new inline nit (the now-unreachable if len > 0 guard) is a REVIEW.md-style cleanup with zero behavioral impact and can be folded in or left as-is.

[claude]

LGTM — the two earlier nits (5-X err.path and the now-dead len > 0 guard) are both addressed.

What was reviewed:

• Confirmed sync/callback/promises all route through the single NodeFS::mkdtemp, so one guard covers every entry point.
• Checked sys::Error.path is Box<[u8]>; [b'X'; 5].into() type-checks and matches Node's observed err.path.
• Tests await properly (.rejects is awaited, callback resolved via withResolvers) and assert specific code/syscall, not bare toThrow().

Extended reasoning...

Overview

Adds an is_empty() early return in NodeFS::mkdtemp (src/runtime/node/node_fs.rs:6094) that surfaces EINVAL with syscall: mkdtemp and path: "XXXXX" when the prefix is empty, matching Node's observed behavior. Drops the now-redundant if len > 0 guard around the prefix copy. Adds three tests in test/js/node/fs/fs.test.ts covering sync, callback, and promises variants.

Security risks

None. This tightens behavior — an input that previously created a directory in cwd now errors before any syscall. No new user-controlled data reaches a syscall path.

Level of scrutiny

Low-to-medium. This is a small, additive Node-compat guard in a well-understood function; the non-empty path is byte-identical to before (modulo the removed dead guard). All three JS entry points funnel through this one Rust function via the binding table (node_fs_binding.rs:378, node_fs.rs:9441), so there are no sibling sites to miss.

Other factors

Two prior review rounds raised (1) 6-X vs 5-X err.path and (2) the newly-dead len > 0 guard; both were fixed in follow-up commits and the threads are resolved. The PR description shows the tests fail under USE_SYSTEM_BUN=1 and pass under the debug build, and the relevant Node parallel tests still pass. The tests follow harness conventions (awaited .rejects, withResolvers for the callback form, specific error-code assertions).