node:fs: snapshot resizable-ArrayBuffer-backed path buffers so an option getter cannot resize(0) through the borrow

[robobun]

Repro

const fs = require("node:fs");
const pb = Buffer.from("/tmp/pathlike-resize-test.txt");
const p = new Uint8Array(new ArrayBuffer(pb.length, { maxByteLength: 1 << 16 }));
p.set(pb);
fs.writeFileSync(p, "data", { get flag() { p.buffer.resize(0); return "w"; } });

panic(main thread): Segmentation fault at address 0x728022000000

Every node:fs op that takes a buffer-typed path through PathLike::from_js_with_allocator and then evaluates a later argument is affected, on both paths:

• sync: writeFileSync/appendFileSync (flag/mode/signal/flush), readFileSync (encoding/flag), mkdirSync (recursive/mode), readdirSync, rmSync, openSync, statSync (throwIfNoEntry), ...
• async: any fs.promises.*/callback op, where JS can shrink the buffer between the call returning and the work-pool thread reading the path bytes.
• Bun.file(Uint8Array) + .text()/.arrayBuffer(), which store the PathLike and read it later on a worker.

Cause

PathLike::from_js_with_allocator pins the backing JSC::ArrayBuffer via Buffer::from_js_pinned so transfer()/structuredClone/postMessage cannot detach it (#31221). The pin does not guard ArrayBuffer.prototype.resize: JSC::ArrayBuffer::resize answers a shrink by OSAllocator::protecting the trimmed pages PROT_NONE. The captured (ptr, len) still spans those pages, so the next slice_z → copy_from_slice (sync) or the work-pool thread's read (async) faults.

Node does not crash: it copies buffer paths into an internal string before the syscall.

Fix

When the path buffer is backed by a resizable non-shared ArrayBuffer, snapshot the bytes into an owned MarkedArrayBuffer at capture time (snapshot_resizable_path_buffer in src/runtime/node/types.rs, applied to both the Uint8Array/DataView and ArrayBuffer arms). Paths are already validated to fit in MAX_PATH_BYTES, so the copy is cheap. Fixed-length buffers stay on the zero-copy pinned path. Growable SharedArrayBuffers can only grow in place within the reserved max and keep a stable data pointer, so they stay borrowed.

Because a PathLike::Buffer can now own its allocation, src/jsc/node_path.rs gains:

• Drop for PathLike frees an owned snapshot via MarkedArrayBuffer::destroy (idempotent; no-op for JS-owned backings).
• Clone for PathLike dupes an owned payload instead of borrowing it, so the clone is independently droppable (mirroring the owned-String arm).

The owned snapshot carries value = JSValue::ZERO, so to_thread_safe/unprotect's protect()/unprotect() calls are no-ops for it, and the protect_eat() on the original JS value is skipped (nothing to root once the bytes are copied).

Semantics

Snapshotting at capture time means a resizable-backed path is read as it was when passed, so an option getter that overwrites the bytes in place (without resizing) no longer changes which path is used. That differs from both Node and Bun's non-resizable path (which borrow and see the overwrite). Matching Node's read-after-options order would require reordering path parsing in every args::*::from_js and re-reading the live byteLength from JSC after arbitrary JS ran; that is a much larger refactor than this crash fix and would need to cover every PathLike caller at once. The practical impact is limited to programs that deliberately rewrite the path buffer from inside an option getter. The existing pin-against-transfer() tests continue to pass unchanged (they use fixed-length buffers).

Verification

test/js/node/fs/fs.test.ts spawns a subprocess that exercises writeFileSync (Uint8Array path, flag getter), readFileSync (DataView path, encoding getter), mkdirSync (ArrayBuffer path, recursive getter), fs.promises.rename (async, post-call shrink), a growable SharedArrayBuffer path, and Bun.file(view).text() called twice (stores the owned PathLike, then clones it on the JS thread and again on the worker, covering the new Clone/Drop arms under ASAN). The subprocess segfaults on stock bun and exits 0 with this change. The existing pin tests ("keeps a ... attached") and the readFileSync/mkdirSync/statSync suites pass.

Related: #32189 covered the async side only; this change covers sync and async in one place. #35821 is the sibling fix for StringOrBuffer. #31221 added the pin that guards transfer() but not resize().

---

no test proof · iteration 2 · 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]

Walkthrough

Changes

Resizable ArrayBuffer-backed paths are copied before JavaScript re-entry, with ownership-aware argument handling and explicit buffer cleanup. Filesystem and Bun.file regression tests cover buffer shrinking during operations.

Path buffer stability

Layer / File(s)	Summary
Buffer ownership and cleanup src/jsc/node_path.rs	Owned path buffers clone into independent snapshots, and buffer-backed paths explicitly destroy their buffers on drop.
Resizable buffer parsing src/runtime/node/types.rs	Resizable non-shared path buffers are snapshotted before parsing completes, with eat() or protect_eat() selected according to ownership.
Resizable path regression coverage test/js/node/fs/fs.test.ts	Filesystem and Bun.file operations are tested while resizable path buffers are shrunk during option getters or asynchronous execution.

Possibly related PRs

• oven-sh/bun#35821: Related resizable ArrayBuffer snapshot and ownership handling in StringOrBuffer.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly names the node:fs path-buffer snapshot fix and the resizable-ArrayBuffer borrow issue.
Description check	✅ Passed	It covers the fix and verification, though it uses custom headings instead of the template's exact sections.

---

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

[robobun]

^{Updated 12:11 AM PT - Jul 26th, 2026}

❌ @robobun, your commit 57b32f7 has 2 failures in Build #82021 (All Failures):

• 📦 Binary size — 7 over 0.50 MB

target	this build	canary: main #79916
		size	Δ
❌ bun-darwin-aarch64	58.13 MB	57.58 MB	+564.9 KB
❌ bun-darwin-x64	63.48 MB	62.95 MB	+544.5 KB
❌ bun-linux-x64	72.47 MB	71.95 MB	+528.0 KB
❌ bun-linux-aarch64-musl	64.88 MB	64.32 MB	+576.0 KB
❌ bun-linux-aarch64-android	78.47 MB	77.97 MB	+512.0 KB
❌ bun-windows-x64	80.26 MB	79.70 MB	+571.0 KB
❌ bun-windows-aarch64	70.86 MB	70.34 MB	+533.5 KB

Add [skip size check] to the commit message if this increase is intentional.

• step failed outside runner - exit 1 on 🐧 aarch64 - build-bun
• step failed outside runner - exit 1 on 🐧 x64-android - build-bun
• step failed outside runner - exit 1 on :freebsd: aarch64 - build-bun
• step failed outside runner - exit 1 on :freebsd: x64 - build-bun
• step failed outside runner - exit 1 on 🐧 x64-musl - build-bun

---

🧪   To try this PR locally:

bunx bun-pr 35840

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

bun-35840 --bun

[github-actions]

This PR may be a duplicate of:

1. node:fs: snapshot path buffers backed by resizable ArrayBuffers for async ops #32189 - Also snapshots resizable-ArrayBuffer-backed path buffers in node:fs (async ops only); node:fs: snapshot resizable-ArrayBuffer-backed path buffers so an option getter cannot resize(0) through the borrow #35840 is a superset covering both sync and async

🤖 Generated with Claude Code

[claude]

No issues found, but this reworks ownership semantics for PathLike::Buffer (new destroy() in Drop, conditional deep-copy in Clone, protect_eat → eat when owned) — memory-safety changes on a path every node:fs buffer-path call flows through, so worth a maintainer's eyes.

What was reviewed:

• snapshot_resizable_path_buffer: verified from_string → from_bytes yields owns_buffer=true, value=JSValue::ZERO, pinned=false, so to_thread_safe/unprotect become no-ops and the pin is released before overwrite.
• Drop: destroy() is gated on owns_buffer and idempotent; the only other PathLike::Buffer constructor (node_fs.rs:3540) sets owns_buffer=false, so no new double-free path.
• Clone: owned-arm dupe means both original and clone free independently; borrowed arm unchanged.
• Test fixture covers Uint8Array/DataView/ArrayBuffer arms, sync + async, and the growable-SAB stay-borrowed path.

Extended reasoning...

Overview

Fixes a segfault where an ArrayBuffer.prototype.resize(0) invoked from an option getter (or between an async fs call and its work-pool dispatch) decommits pages that a captured (ptr, len) still spans. The fix snapshots resizable-non-shared-ArrayBuffer-backed path buffers into an owned MarkedArrayBuffer at capture time (src/runtime/node/types.rs), and updates Drop/Clone for PathLike (src/jsc/node_path.rs) so the owned snapshot is freed exactly once and clones are independent. A subprocess-based regression test in test/js/node/fs/fs.test.ts covers all three input arms plus the async path and the SAB carve-out.

Security risks

The bug being fixed is a user-triggerable read of PROT_NONE pages — a DoS, not memory disclosure. The fix itself introduces a new owned-allocation lifecycle: I traced from_string → from_bytes → ArrayBuffer::from_bytes and confirmed the copy has value = JSValue::ZERO (so protect/unprotect/unpin on it are no-ops), owns_buffer = true (so destroy() frees it), and pinned = false. destroy() clears owns_buffer before freeing, so it's idempotent. MarkedArrayBuffer has no Drop of its own, so *buffer = copy in the snapshot helper doesn't run any destructor on the JS-backed original (whose pin was already explicitly released). The one other site constructing PathLike::Buffer (node_fs.rs:3540, the Mkdtemp default) uses owns_buffer: false, so the new destroy() call in Drop is a no-op there.

Level of scrutiny

High. This is squarely in the repo's "Native code: memory safety" category — new allocation ownership on a type held across JS re-entry and thread-pool dispatch, with paired Drop/Clone changes. The reasoning in the PR description is careful and I couldn't find a hole, but the interaction surface (pin vs. protect vs. owns_buffer, ArgumentsSlice::protect_eat bookkeeping, ThreadSafe<T> guard ordering) is exactly where subtle imbalances hide.

Other factors

The test is well-constructed (subprocess isolation so a segfault surfaces as non-zero exit, drains both pipes, asserts a combined {stdout, stderr, exitCode} object) and exercises each JSType arm the fix touches plus the async race and the SAB exemption. CI is still building. Given the ownership-lifecycle changes, deferring to a human reviewer rather than auto-approving.

[robobun]

Diff is ready; the remaining CI red is infrastructure.

• Build #81818 on 913c32a (the fix commit) passed every test lane, including test/js/node/fs/fs.test.ts on all platforms. The single red job was :package: binary-size against stale canary baseline #79916; other PRs on the same base hit the identical delta.
• 8a089b0 (adds a Bun.file(view).text() sub-case to cover the owned-PathLike::Buffer Clone arm) and be4f581 (trims comments) make no src/ semantic change after 913c32a.
• Build #82021 on 57b32f7 (re-roll): 128 jobs passed. The 6 failed jobs are :package: binary-size (same stale #79916 baseline) and five build-bun lanes (linux-aarch64, linux-x64-musl, linux-x64-android, freebsd-x64, freebsd-aarch64) all annotated [pre-existing] "step failed outside runner". Test lanes downstream of those builds went waiting_failed. The [flaky]-tagged tests (proxy-stress-protocol, fetch-backpressure, and fs.test.ts's pre-existing readdir-recursive fd-count check at :3927) passed on retry and are unrelated to path-buffer handling.

The new test (fs path buffers backed by a resizable ArrayBuffer are snapshotted before option getters run) passed on every lane that ran. Self-review addressed; three bot-review passes found no bugs; all review threads resolved.

[claude]

No bugs found, but this touches PathLike's ownership semantics (new Drop/Clone arms for owned MarkedArrayBuffer, protect/pin balancing) and the PR body flags a deliberate Node-compat divergence (snapshot-at-capture vs read-after-options) — worth a maintainer's eyes.

What was reviewed:

• snapshot_resizable_path_buffer: unpins before overwrite, from_string sets owns_buffer=true / value=ZERO so protect()/unprotect() are no-ops on the owned copy.
• Drop for PathLike now calls b.destroy() unconditionally — checked it's idempotent (owns_buffer=false no-op) and the only other PathLike::Buffer construction (MkdirTemp::default) is non-owning.
• eat() vs protect_eat() split matches the owned/borrowed distinction; no unbalanced protect on the owned path.
• Test fixture covers Uint8Array/DataView/ArrayBuffer arms, sync + async, growable SAB stays borrowed, and the owned Clone arm via Bun.file().text() twice.

Extended reasoning...

Overview

Fixes a segfault where a resizable-ArrayBuffer-backed path buffer passed to any node:fs op can be resize(0)'d by an option getter (or between call and work-pool read for async), decommitting pages under a live borrow. The fix snapshots resizable non-shared path buffers into an owned MarkedArrayBuffer at capture time in PathLike::from_js_with_allocator (both the Uint8Array/DataView and ArrayBuffer arms), and adds the matching Drop/Clone handling in src/jsc/node_path.rs so an owned PathLike::Buffer frees itself and clones by dupe rather than borrow.

Security risks

Fixes a user-triggerable crash (DoS surface). No new attack surface introduced — the copy is bounded by the pre-existing Valid::path_buffer check (< MAX_PATH_BYTES), and handle_oom covers allocation failure.

Level of scrutiny

High. This is exactly the "Native code: memory safety" category the repo review guide calls the most-blocked: it changes Drop and Clone for a type that flows through every node:fs path argument, adds a new owned-allocation arm to a struct that previously only borrowed, and rebalances pin/unpin and protect_eat/eat. I traced each path (owned snapshot: value=ZERO so to_thread_safe/unprotect are no-ops, destroy() idempotent via the owns_buffer flag reset; borrowed: unchanged) and found nothing wrong, but the correctness hinges on invariants scattered across array_buffer.rs, CallFrame.rs, and the C++ pin/unpin — the kind of change a maintainer familiar with the JSC ArrayBuffer lifetime should confirm.

Other factors

• The PR body explicitly documents a semantics divergence from Node (a resizable-backed path is snapshotted before option getters run, so an in-place overwrite from a getter no longer changes which path is used). The author argues matching Node would need a much larger refactor across every args::*::from_js; a maintainer should confirm that trade-off is acceptable.
• Sibling PR #32189 covers the async-only case; this PR supersedes it and the two need coordination.
• Test coverage is good (subprocess fixture exercises sync/async, all three JS type arms, SAB, and the owned-Clone path); prior CI run passed all lanes per the author's comment.
• The comment-cop bot flags are all resolved (comments trimmed in be4f581).

[claude]

I didn't find any bugs, but this changes PathLike's ownership model in memory-safety-critical native code and overlaps with #32189, so it's worth a human look.

What was reviewed:

• MarkedArrayBuffer::destroy() gates on owns_buffer and clears it, so the new unconditional b.destroy() in Drop for PathLike is a no-op for JS-owned/borrowed buffers — no double-free.
• from_string → from_bytes yields value = JSValue::ZERO (via ..Default::default()), so to_thread_safe/unprotect's protect()/unprotect() on the owned snapshot are no-ops as claimed.
• MarkedArrayBuffer has no Drop impl, so *buffer = copy in snapshot_resizable_path_buffer doesn't run cleanup on the overwritten JS-borrowed value; the manual unpin() beforehand balances the pin taken by from_js_pinned.

Extended reasoning...

Overview

Fixes a segfault where node:fs ops capture a (ptr, len) view into a resizable-ArrayBuffer-backed path, and a subsequent option getter (or async gap before the work-pool read) calls .resize(0), decommitting the pages under the borrow. The fix snapshots resizable non-shared path buffers into an owned MarkedArrayBuffer at capture time in from_js_with_allocator, and adds the corresponding Clone/Drop handling for the newly-possible owned-Buffer variant of PathLike. Test coverage exercises Uint8Array/DataView/ArrayBuffer paths, sync and async, growable SAB (stays borrowed), and Bun.file()'s stored-then-cloned PathLike.

Security risks

None new. The change hardens against a user-triggerable segfault (effectively a DoS on their own process). The snapshot is bounded by the existing Valid::path_buffer length check (< MAX_PATH_BYTES), so the copy cannot be leveraged for unbounded allocation.

Level of scrutiny

High. This is native memory-lifecycle code at the JSC boundary — Clone/Drop for a type that crosses to work-pool threads, plus a change from protect_eat() to eat() on the owned path. I verified: destroy() is idempotent and gated on owns_buffer; from_string produces owns_buffer=true, pinned=false, value=ZERO, resizable=false; MarkedArrayBuffer has no Drop so the *buffer = copy overwrite doesn't double-release; and the owned-arm Clone dupes rather than borrows so each clone's Drop frees its own allocation. Nothing looked wrong, but per repo guidance this bug class (ownership/refcount balance across every terminal path, cross-thread lifetime) is exactly what human reviewers scrutinize most.

Other factors

• The PR openly documents a semantic divergence from Node (snapshot-at-capture means an in-place overwrite from an option getter no longer changes the path for resizable buffers). That's a design trade-off a maintainer should sign off on, even if the practical impact is negligible.
• The duplicate-PR bot flagged #32189 as a subset (async-only) of this change; a maintainer should decide how to reconcile them.
• Prior CI (build #81818) was green on all test lanes per the author's note; the current build (#81889) is still running.