Fix double-deref of path when S3Client.presign throws

[robobun]

What

Bun.S3Client.presign("foo") (and the other static/instance helpers that construct a temporary S3 blob from a path) crashed with reached unreachable code in debug builds and SIGFPE in release when the underlying signRequest failed (e.g. missing credentials).

Repro:

Bun.S3Client.presign("foo"); // no S3 credentials configured

Why

Blob.Store.initS3 consumes the caller's PathLike reference: it copies the struct by value and calls toThreadSafe() on the copy, which derefs the original WTFStringImpl when a new thread-safe impl is created. After construction, the caller's PathLike is stale.

The static/instance helpers in S3File.zig and S3Client.zig kept an errdefer path.deinit() in scope across both construction and the follow-up call (getPresignUrlFrom, writeFileInternal, etc.). When that follow-up call threw, defer blob.deinit() released the store's ref and then the errdefer derefed the same (already-consumed) string a second time, tripping hasAtLeastOneRef().

Fix

After the blob is successfully constructed, clear the local path so the errdefer becomes a no-op — the same pattern already used in Blob.findOrCreateFileFromPath. The errdefer still runs for errors thrown before ownership transfers.

Fuzzer fingerprint: 1f40a92dc166c1e8

[robobun]

^{Updated 7:35 PM PT - Apr 13th, 2026}

❌ @autofix-ci[bot], your commit 886b1cf has 2 failures in Build #45556 (All Failures):

• test/js/bun/webview/webview-chrome.test.ts - code 1 on 🍎 14 aarch64
• test/js/bun/webview/webview-chrome.test.ts - code 1 on 🍎 13 aarch64
• test/cli/install/bun-install-lifecycle-scripts.test.ts - code 1 on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 29279

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

bun-29279 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2852904f-3fdf-49c1-b68f-1fe419635a53

📥 Commits

Reviewing files that changed from the base of the PR and between ccbaed9 and 886b1cf.

📒 Files selected for processing (3)

• src/bun.js/webcore/S3Client.zig
• src/bun.js/webcore/S3File.zig
• test/js/bun/s3/s3-presign-missing-credentials.test.ts

---

Walkthrough

Modified S3Client and S3File implementations to reset path variables to empty after constructing S3-backed blobs, preventing stale path retention. Added test verifying S3 presign methods throw errors when credentials are missing.

Changes

Cohort / File(s)	Summary
S3 Path Management Updates src/bun.js/webcore/S3Client.zig, src/bun.js/webcore/S3File.zig	Converted path and path_or_blob variables from const to var across presign, exists, size, stat, write, and unlink methods to explicitly reset them to empty bun.PathString after S3-backed blob construction, ensuring variables don't retain previously parsed values through deferred cleanup.
S3 Credential Validation Test test/js/bun/s3/s3-presign-missing-credentials.test.ts	New test file validating that all S3 presign entry points throw errors when credentials are missing, running inline script across multiple presign variants in isolated subprocess environment with AWS and S3 environment variables stripped.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the specific bug being fixed: a double-dereference issue in S3Client.presign, which directly matches the main change across the modified files.
Description check	✅ Passed	The description fully adheres to the template with both required sections: 'What' explains the crash behavior and repro, and 'Why' provides root cause analysis and the fix rationale.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[github-actions]

This PR may be a duplicate of:

1. fix(s3): don't double-free path when S3Client static ops throw after blob creation #29081 - Same double-free fix in S3 static methods using a sentinel value to neutralize errdefer after blob construction
2. Fix double-free in S3 static methods when path is passed as string #28592 - Fixes same double-free by cloning the path so both store and caller own independent copies
3. Fix double-free of path in S3 static methods on error paths #28495 - Same sentinel-value approach applied to both S3Client.zig and S3File.zig error paths
4. Fix crash in S3 presign with missing credentials #28423 - Fixes same S3 presign crash with missing credentials by bypassing blob construction entirely
5. Fix use-after-free in S3 Store.initS3 PathLike refcounting #28417 - Fixes same use-after-free by transferring path ownership so callers no longer errdefer path.deinit()

🤖 Generated with Claude Code

[robobun]

Duplicate of #28495, which already applies the same fix to both S3Client.zig and S3File.zig. Closing in favor of that PR.

Fuzzer fingerprint 1f40a92dc166c1e8 is the same underlying crash.

[claude]

🔴 The S3Client.file() instance method was missed by this PR's double-deref fix — it still uses const path with errdefer path.deinit() and calls constructS3FileWithS3CredentialsAndOptions(), which can throw after initS3 has already consumed the path's WTFStringImpl reference, triggering the same use-after-free crash. Apply the same fix as the sibling methods: change const path to var path and add path = .{ .string = bun.PathString.empty }; after the successful Blob.new(...) call.

Extended reasoning...

What the bug is and how it manifests

The PR correctly identifies and fixes the double-deref pattern in six instance methods of S3Client (presign, exists, size, stat, write, unlink) and six static functions in S3File.zig. However, the file() instance method at S3Client.zig lines 128–142 was left unfixed. It still declares const path: jsc.Node.PathLike with errdefer path.deinit() in scope, and calls constructS3FileWithS3CredentialsAndOptions() without clearing path after construction — the exact pattern this PR set out to eliminate.

The specific code path that triggers it

constructS3FileWithS3CredentialsAndOptions() does the following in order:

1. Calls bun.handleOom(Blob.Store.initS3(path, ...)) or initS3WithReferencedCredentials(path, ...). Both functions copy the PathLike struct by value and call toThreadSafe() on the copy, which may call orig.deref() on the original WTFStringImpl (when a new thread-safe impl is created). At this point the caller's path is stale.
2. Installs errdefer store.deinit().
3. Calls try opts.getTruthyComptime(globalObject, "type") — can throw a JSError (e.g., if a getter throws).
4. Calls try file_type.toSlice(globalObject, ...) — also can throw a JSError.

Why existing code doesn't prevent it

bun.handleOom cannot return an error, so step 1 always completes and ownership of the path reference transfers to the store before the try-able operations in steps 3–4 run. The PR fixed all other methods by clearing path after step 1, but missed file(). Because path is declared const, the clearing assignment cannot be written without first changing it to var.

What the impact would be

If step 3 or 4 throws a JSError, the inner errdefer store.deinit() runs (decrementing and freeing the path ref held by the store), then file()'s errdefer path.deinit() runs on the already-freed WTFStringImpl — a use-after-free. In debug builds this trips hasAtLeastOneRef(); in release builds it is a SIGFPE or silent heap corruption, exactly as described in the PR.

How to fix it

Same pattern as every other fixed method:

1. Change const path: jsc.Node.PathLike → var path: jsc.Node.PathLike at line 132.
2. Add path = .{ .string = bun.PathString.empty }; immediately after the successful Blob.new(...) call (before return blob.toJS(globalThis)).

Step-by-step proof

1. User calls new Bun.S3Client({}).file(jsStringVar, { get type() { throw new Error('oops') } }) where jsStringVar is backed by a non-thread-safe JS string (a WTF StringImpl with refcount 1).
2. file() parses the path → path holds a slice_with_underlying_string PathLike referencing that StringImpl (refcount 2).
3. constructS3FileWithS3CredentialsAndOptions() is called; getCredentialsWithOptions succeeds.
4. Blob.Store.initS3(path, ...) copies path, calls toThreadSafe() on the copy → allocates a new thread-safe StringImpl, calls orig.deref() → StringImpl refcount drops to 1. The store now holds the new ref. The caller's path points to the now-stale original.
5. errdefer store.deinit() is armed inside constructS3FileWithS3CredentialsAndOptions.
6. opts.getTruthyComptime(globalObject, "type") invokes the JS getter → throws → JSError propagates.
7. errdefer store.deinit() fires → StringImpl refcount → 0 → freed.
8. Error propagates back to file() → errdefer path.deinit() fires → calls deref() on the already-freed StringImpl → use-after-free / crash.