Read sink.start() options before resolving the native sink

[Jarred-Sumner]

What

sink.start(options) on Bun.ArrayBufferSink / FileSink resolved the native sink from this first and then read the options (highWaterMark / asUint8Array / stream / path / fd) via property gets. A getter that calls the sink's own close() frees the payload and start() continued on freed memory (ASan: heap-use-after-free WRITE in ArrayBufferSink::start, READ in FileSink::setup).

js_start now parses the Start config before resolving this.

Repro (before)

const sink = new Bun.ArrayBufferSink(); sink.write("hello");
sink.start({ get highWaterMark() { sink.close(); return 1024; } }); // ASan: heap-use-after-free WRITE of size 8

Tests

test/js/bun/util/arraybuffersink.test.ts and test/js/bun/util/filesink.test.ts — "start() with an option getter that closes the sink/writer throws instead of crashing". Fail on the current ASan canary and debug main; pass with this change.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 27 minutes

Limit details: You’ve used all 1 included review currently available under your plan. You completed 71 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour.

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: 10376759-1241-4a1e-97fd-7cf799411258

📥 Commits

Reviewing files that changed from the base of the PR and between fea1829 and e8aa77b.

📒 Files selected for processing (3)

• src/runtime/webcore/Sink.rs
• test/js/bun/util/arraybuffersink.test.ts
• test/js/bun/util/filesink.test.ts

---

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

[robobun]

^{Updated 8:46 PM PT - Aug 16th, 2026}

❌ @Jarred-Sumner, your commit e8aa77b has 3 failures in Build #99780 (All Failures):

• test/cli/run/require-cache.test.ts - code 1 on 🐧 13 x64-asan
• test/js/web/timers/setInterval.test.js - code 1 on 🐧 13 x64-asan
• test/js/workerd/html-rewriter-leak.test.ts - code 1: SyntaxError: JSON Parse error: Unexpected EOF on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 39393

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

bun-39393 --bun

[claude]

LGTM — textbook "read user options before resolving the native pointer" reordering.

What was reviewed:

• Confirmed streams::Start owns its data (no borrowed slices), so parsing before get_this and dropping config on the detached-throw path is sound and leak-free.
• Checked sibling host-fns (js_write/js_flush/js_end) — none read user-getter option objects after get_this, so js_start was the only affected site.
• Tests spawn subprocesses for crash isolation, cover every option key per sink type, and verify follow-up write() stays detached.

Extended reasoning...

Overview

Reorders JSSink::<T>::js_start in src/runtime/webcore/Sink.rs so streams::Start::from_js_with_runtime_tag (which invokes user property getters on the options object) runs before Self::get_this materializes &mut JSSink<T> from m_sinkPtr. Previously a getter calling sink.close() freed the boxed sink while this still pointed at it, and this.sink.start(config) was a heap UAF. After the reorder, get_this sees from_js_result::DETACHED and throws the standard "already been closed" error. Two subprocess tests are added exercising every getter key (highWaterMark/asUint8Array/stream for ArrayBufferSink, path/fd for FileSink).

Security risks

This is a memory-safety fix (UAF reachable from user JS). The change strictly reduces attack surface; it introduces no new user-controlled paths. Start is a fully-owned enum (BlobSizeType, bools, FileSinkOptions{PathOrFileDescriptor, bool, Mode}), so nothing in config can dangle if get_this subsequently throws — Rust drops it.

Level of scrutiny

Low-to-medium. The functional diff is a pure statement reorder of two independent blocks with no shared bindings; happy-path behavior is byte-identical. The fix is generic over T: JsSinkType, so it covers every sink variant (ArrayBufferSink, FileSink, HTTPServerWritable, NetworkSink, etc.) in one place. I audited the sibling host-fns for the same pattern: js_write only accepts strings/ArrayBuffers (its as_array_buffer/is_string+to_js_string on an actual JSString do not invoke user code), js_flush reads a bare boolean, and js_end reads no arguments — so js_start was correctly identified as the sole site.

Other factors

Tests follow file-local conventions (subprocess spawn with concurrent pipe drain, exact stdout assertion, if (exitCode !== 0) expect(stderr).toBe("") diagnostic before the unconditional expect(exitCode).toBe(0)), match the variant-matrix guidance, and assert the sink remains detached for a subsequent write(). No prior human or bot review comments to address.