fix(s3): avoid double-freeing path when presign throws after store creation

[robobun]

What does this PR do?

Fixes a crash (debug assertion / heap corruption) when S3 operations throw an error after the blob store has already taken ownership of the path.

Bun.S3Client.presign("foo"); // no credentials in env → panic in debug builds

Root cause

Blob.Store.initS3 moves the PathLike into the store without adding a reference. Callers in S3File.zig and S3Client.zig were written as:

errdefer path.deinit();
var blob = try constructS3File...(path, ...);   // store now owns path
defer blob.deinit();                             // frees path
return try getPresignUrlFrom(&blob, ...);        // can throw

When the final call threw (missing credentials, invalid expiresIn, invalid method, etc.), defer blob.deinit() freed the path via the store's destructor, and then the outer errdefer path.deinit() freed the same PathLike again — over-derefing the WTFStringImpl (latin1 case) or double-freeing the allocation (encoded_slice case), hitting bun.assert in WTFStringImpl.deref.

Fix

Clear the caller's path right after the store takes ownership so the errdefer becomes a no-op. This is the same pattern already used in Blob.findOrCreateFileFromPath.

Applied to: presign, unlink, write, size, exists, stat (static and instance variants).

How did you verify your code works?

• Minimal repro Bun.S3Client.presign("NFC") panics on debug builds before, throws ERR_S3_MISSING_CREDENTIALS cleanly after.
• Added regression test using { expiresIn: -1 } to deterministically hit the post-construction error path regardless of env credentials, covering static, Bun.s3, instance, and non-latin1 path variants.
• Existing S3 validation tests pass.

Fuzzer fingerprint: 5308c20fced315a7

[robobun]

^{Updated 8:08 AM PT - May 11th, 2026}

❌ @robobun, your commit ec8bbc1 has 3 failures in Build #53345 (All Failures):

• test/cli/install/bun-install.test.ts - code 1 on 🐧 25.04 x64
• test/js/bun/s3/s3-storage-class.test.ts - code 1 on 🍎 14 aarch64
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 2019 x64
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 2019 x64-baseline
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 30495

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

bun-30495 --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: 9468a152-3c79-4e94-bc57-c12e97905633

📥 Commits

Reviewing files that changed from the base of the PR and between 1136af3 and ec8bbc1.

📒 Files selected for processing (3)

• src/runtime/webcore/Blob.zig
• src/runtime/webcore/S3Client.zig
• src/runtime/webcore/S3File.zig

---

Walkthrough

This PR fixes a memory safety issue where S3 path variables are double-freed when errors occur after blob construction. Callers and S3File now clear consumed path values after transferring ownership into S3 blobs/stores; constructors also clear stored pathlike on error. A new test suite validates presign error paths and forces GC to detect double-free/refcount issues.

Changes

S3 Path Double-Free Prevention

Layer / File(s)	Summary
S3Client Instance Methods src/runtime/webcore/S3Client.zig	Six instance methods (presign, exists, size, stat, write, unlink) reassign path to an empty PathLike immediately after constructing the S3 blob via S3File.constructS3FileWithS3CredentialsAndOptions, preventing errdefer path.deinit() from deallocating already-consumed path values.
S3Client.staticFile JS parsing src/runtime/webcore/S3Client.zig	staticFile adds an errdefer path.deinit() for the parsed JS path to ensure cleanup on error before calling S3File.constructInternalJS.
S3File Path Ownership Management src/runtime/webcore/S3File.zig	Six path-handling methods and a top-level stat reassign path_or_blob to an empty .path variant after blob construction, ensuring the outer .path errdefer cleanup does not attempt to deinitialize the already-consumed original path value.
Constructor Error Cleanup src/runtime/webcore/S3File.zig	constructS3FileWithS3CredentialsAndOptions and constructS3FileWithS3Credentials clear store.data.s3.pathlike to an empty bun.PathString in their error errdefer before calling store.deinit().
constructInternal Defer src/runtime/webcore/S3File.zig	constructInternal adds an errdefer path.deinit() after parsing JS input so the parsed path is deinitialized if subsequent construction fails.
Blob s3:// Branch Cleanup src/runtime/webcore/Blob.zig	constructBunFile registers an errdefer path.deinit() in the s3:// early-return branch to ensure the parsed path is cleaned up if S3File.constructInternalJS fails.
Test Coverage test/js/bun/s3/s3-presign-error-path-free.test.ts	New test suite verifies S3 presign failures occurring after blob store construction do not cause double-free, covering static and instance presign APIs with both latin1 and non-latin1 paths and a throwing option getter; each test forces GC to surface refcount/double-free issues.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main fix: avoiding double-free of a path in S3 presign when an error occurs after store creation.
Description check	✅ Passed	The PR description fully covers both required sections: 'What does this PR do?' provides detailed context and root cause analysis; 'How did you verify your code works?' documents testing approach and verification steps.
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.

_{✏️ 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 use-after-free in S3 Store.initS3 PathLike refcounting #28417 - Fixes same S3 path double-free by removing errdefer path.deinit() and nullifying path after store takes ownership in S3Client.zig
2. Fix crash in S3 presign with missing credentials #28423 - Fixes same S3 presign crash by reworking path cleanup from errdefer to defer in S3File.zig and S3Client.zig
3. Fix double-free of path in S3 static methods on error paths #28495 - Nullifies path with bun.PathString.empty after store construction in all S3Client.zig static methods — same fix approach
4. Fix double-free in S3 static methods when path is passed as string #28592 - Nullifies path_or_blob.path in S3File.zig for all six S3 operations to prevent the same double-free
5. fix(s3): don't double-free path when S3Client static ops throw after blob creation #29081 - Neutralizes caller's errdefer path.deinit() in S3Client.zig static ops — same ownership-transfer fix
6. fix(s3): double free of path when S3Client operation throws #29656 - Removes errdefer path.deinit() from all S3Client.zig static methods to fix the same double-free
7. s3: fix path double-free when presign throws after blob creation #30351 - Nullifies path_or_blob before constructS3FileInternalStore in S3File.zig — same approach as this PR
8. S3Client: don't double-deref path when a method fails after blob construction #30419 - Nullifies path after constructS3FileWithS3CredentialsAndOptions in S3Client.zig for presign/exists/size/stat
9. s3: don't double-free path when presign throws after blob creation #30467 - Near-identical fix, passes path by pointer so callee can nullify the caller's copy in S3Client.zig

🤖 Generated with Claude Code

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/webcore/S3File.zig`:
- Around line 271-274: constructInternal() currently calls PathLike.fromJS(...)
and then performs operations (like a type getter) that can throw, but there is
no errdefer to deinit the caller-owned PathLike on those failure paths, so a
thrown error bypasses store.deinit() and leaks path; fix by adding an errdefer
that deinitializes the local path (call path.deinit()) immediately after
PathLike.fromJS(...) and before any further throwing calls, and ensure the
existing errdefer that resets store.data.s3.pathlike and calls store.deinit()
remains in place (also mirror the same errdefer pattern for the other occurrence
around lines 318-321), so any early error cleans up the PathLike and prevents
ownership leak while preserving normal ownership handoff on success.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a0f11231-6860-427c-97b9-f117f96576d9

📥 Commits

Reviewing files that changed from the base of the PR and between 6439d14 and 1136af3.

📒 Files selected for processing (2)

• src/runtime/webcore/S3File.zig
• test/js/bun/s3/s3-presign-error-path-free.test.ts

[robobun]

CI status on ec8bbc1:

• ✅ test/js/bun/s3/s3-presign-error-path-free.test.ts passes on all platforms (5 pass, 0 fail)
• ✅ 50 checks green
• ❌ test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts — timeout on windows-2019-x64 / windows-2019-x64-baseline / windows-11-aarch64

The HTTP close-on-abort timeout is a pre-existing Windows flake unrelated to this S3 change — it also fails on other PR builds (e.g. #53335) and there's a fix in progress on the claude/fix-http-emit-close-flake branch. The previous build (#53339) failed only because GitHub returned HTTP 500 during git fetch on two agents.

[robobun]

Superseded by #30567, which implements the *PathLike move-from approach recommended in review here. That approach nulls the caller's path inside the constructor immediately after initS3 consumes it, so it correctly handles all three failure points (before initS3, inside the constructor after initS3, and after the constructor returns) without the leak on the type-getter path.