fetch: reject file: URLs whose path cannot be read

[robobun]

What does this PR do?

fetch() of a file: URL resolved with a 200 Response no matter what the path pointed at. The error only showed up when the body was read:

const res = await fetch("file:///definitely/missing/file");
console.log(res.status, res.ok); // 200 true
await res.text();                // ENOENT: no such file or directory, open '/definitely/missing/file'

Same thing for a directory (fetch("file:///tmp") is a 200, .text() throws EISDIR).

Cause: the file: branch of fetch_impl (src/runtime/webcore/fetch.rs) builds the Response around a Bun.file()-style blob, which does not touch the file until it is read, and hardcodes status_code: 200. Nothing in between looks at the disk.

Fix: after the blob is created, stat the path and reject the fetch() promise instead of building the Response:

• path does not stat (ENOENT, ENOTDIR, EACCES on a parent directory, ELOOP, ...): reject with that error (code, syscall: "stat", path, errno)
• path is a directory: reject with EISDIR (syscall: "read", the error the body read would have produced)
• anything else, including FIFOs and character devices like /dev/null, keeps resolving as before

The rejection is the system error materialized as a TypeError (SystemError::to_type_error_instance), which is the shape every other fetch() rejection has had since #35855 (ValueError::SystemTypeError): err instanceof TypeError holds, and err.code === "ENOENT" checks that today run against the .text() error keep working.

The check only runs for path-backed file blobs. Files embedded in a standalone executable come back from find_or_create_file_from_path as byte-backed blobs and do not exist on disk, so they are left alone (covered by a new compile test).

Why this shape

• Rejecting rather than returning a 404 Response is what fetch() already does for an unresolvable blob: URL a few lines up, what Deno does for a missing file: URL, and what the Fetch spec asks for when a file: fetch cannot be served (a network error, which is a TypeError at the JS level). fetch() has no status to map ENOTDIR, ELOOP or EISDIR onto anyway. This also mirrors the upload direction in the same function, where fetch(url, { body: Bun.file(missing) }) already rejects with the open error up front.
• stat rather than an eager open: nothing here needs the fd (the body reader opens the path itself, and a path-backed store is what keeps response.clone() working), and opening a FIFO or a device just to close it again has side effects, or blocks when a FIFO has no writer. stat has none of that. There is a FIFO test that blocks under an eager blocking open.
• Only failures that guarantee the read would fail are checked. A permission failure on the file's own mode bits is deliberately not pre-checked: access(2) can disagree with open(2) (real vs effective ids, NFS), and a pre-flight must never reject a file the read would have succeeded on. That case still resolves and the EACCES still comes from the body read, exactly as today; a test pins that boundary from both sides.
• The check is synchronous, like Bun.file().size/.exists() and the open/fstat that Bun.serve does when rendering a file-backed Response. It is one stat of a local path.
• The rejection is created with JSPromise::rejected_promise, so an uncaught fetch("file:///missing") is reported as an unhandled rejection (tested). The older helper used by the other early exits in this function suppresses that reporting; switching those call sites is tracked separately.
• url_string is a bare +1 ref that only Response::init releases, so it is now created after the check. The existing leak test for that ref (fetch-leak.test.ts) fetched a path that does not exist, which no longer reaches the code it guards, so it now fetches a file that does exist. While there, its flat 20 MiB bound (over a ~17 MiB raw signal) was replaced by a longer warm-up and an isASAN-branched bound like the other tests in that file. Measured with url_string.clone() temporarily re-added vs fixed: 26-27 vs 8-10 MiB on debug+ASAN (bound 18); on the release build ~2 MiB fixed and 19-21 MiB with the leak approximated by retaining one equal-sized string per call (bound 12).
• The new early return drops the request body the same way the existing resolving return in this branch (and most other early exits in fetch_impl) already does; HTTPRequestBody has no Drop, so a body passed to a file: fetch is retained today regardless of this change. That is a separate pre-existing leak and is being handled on its own.

How did you verify your code works?

New tests in test/js/web/fetch/fetch.test.ts (fetch() file:// that cannot be read), every rejection also asserted to be a TypeError:

• ENOENT for a string, URL and Request input (a sibling that exists still resolves)
• EISDIR for a directory
• EACCES reported by stat for a file under a mode 000 directory (skipped on Windows and as root)
• a mode 000 file still resolves and .text() rejects with the EACCES from open (skipped on Windows and as root)
• a FIFO with no writer resolves without the body being read (skipped on Windows)
• /dev/null resolves with an empty body (skipped on Windows)
• an uncaught rejection prints TypeError: ENOENT ... and exits 1

The ENOENT, EISDIR, EACCES-from-stat and uncaught cases fail on the current release (fetch() resolves) and pass with this change; the other three pass on both and pin the boundary. I ran the block both as root and as an unprivileged user so the two permission tests executed.

compile/EmbeddedFileOutfile in test/bundler/bundler_compile.test.ts (which already builds an executable with an embedded file) now also checks that fetch(pathToFileURL(embeddedPath)) returns the embedded contents; I confirmed by hand that the /$bunfs/... path involved does not exist on disk.

Also run locally with the debug build: the pre-existing fetch() file:// works test, the updated leak test, cargo clippy -p bun_runtime. The rest of fetch.test.ts has the same failures with and without this change in my environment (localhost connections refused, and (with gc) tests exceeding their timeout under the debug build).

[coderabbitai]

Walkthrough

Changes

File URL fetch behavior

Layer / File(s)	Summary
Validate file URL readability src/runtime/webcore/fetch.rs	File-backed blobs are checked with stat before response setup. Directories and stat failures become TypeError rejections.
Exercise unreadable fetch cases test/js/web/fetch/fetch.test.ts	Tests cover missing paths, directories, permission failures, special files, deferred body errors, and uncaught rejection reporting.
Verify valid file URL integrations test/bundler/bundler_compile.test.ts, test/js/web/fetch/fetch-leak.test.ts	Tests verify embedded-file reads and repeated fetches of existing temporary files.

Possibly related PRs

• oven-sh/bun#35988: Modifies fetch error handling to expose system failures through TypeError rejections.
• oven-sh/bun#35998: Adjusts fetch_impl failure handling and TypeError metadata.
• oven-sh/bun#37501: Updates unreadable file-based fetch operations to reject with TypeError while preserving system error details.

Suggested reviewers: jarred-sumner, dylan-conway

🚥 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 and concisely describes the main change: rejecting unreadable file URLs during fetch.
Description check	✅ Passed	The description includes both required sections and provides detailed behavior, rationale, and verification results.

---

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

[robobun]

^{Updated 8:44 AM PT - Aug 11th, 2026}

✅ @robobun, your commit 0ca5f6383c68012a6cd976adee7b1e5aa181f364 passed in Build #92325! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 37425

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

bun-37425 --bun

[robobun]

Status: reproduced on the 1.4.0 release with bun -e 'const r = await fetch("file:///definitely/missing/file"); console.log(r.status); await r.text()' (prints 200, then ENOENT from .text()); same for a directory. The fetch() file:// that cannot be read tests in test/js/web/fetch/fetch.test.ts fail on the release build (fetch() resolves) and pass with this branch. Current head: 0ca5f63 (test-only changes since 96f1f25, which was green in build 92204: leak-test bound branched on ASAN, embedded-file fetch folded into the existing compile test). Waiting on CI. The open question for a maintainer is only the API shape (reject with a TypeError carrying the system error fields, see the comments below).

[claude]

I reviewed this PR and didn't find any bugs. Since it changes user-visible fetch("file://…") semantics (resolve→reject) and picks a specific error shape over alternatives like a 404 Response, a maintainer should confirm the API decision.

What was reviewed:

• file_url_unreadable_error: Blob drop releases the StoreRef on the reject path; path_buffer_pool::get() guard returns on drop; with_path clones so no borrow of the scratch buffer escapes.
• url_string reordering after the early return — no leak of the +1 ref, and the leak test still guards the path it was written for.
• Embedded-file bypass: byte-backed blobs skip the stat via the store::Data::File guard; covered by the new compile test.
• bun_sys::stat accepts the slice_z path on both POSIX and Windows (libuv).

Extended reasoning...

Overview

The PR makes fetch() of a file: URL reject up front when the path cannot be read (does not stat, or is a directory), instead of resolving with a 200 Response whose body reader later throws. It adds a helper file_url_unreadable_error in src/runtime/webcore/fetch.rs, reorders the url_string +1-ref creation past the new early return, updates the fetch(file://) leak test to use an existing file, and adds coverage in fetch.test.ts (ENOENT via string/URL/Request, EISDIR, /dev/null still resolves, unhandled-rejection reporting) plus a bundler_compile test for embedded files.

Security risks

None. The change adds a stat of a local path the caller already controls; no new untrusted-input parsing, no auth/crypto/permissions surface.

Level of scrutiny

Medium-high. The native change is small and I checked it for the review guide's memory-safety rules: the Blob returned by find_or_create_file_from_path holds its store via StoreRef, whose Drop decrements the refcount, so the early-return path does not leak; the pooled path buffer is a Guard returned on drop; err.with_path(path.slice()) clones into a Box so nothing borrows the scratch buffer past the call. The url_string move is correctly justified — it is a bare +1 that only Response::init releases.

However, this is a user-facing behavior change to a public Web API. The author explicitly chose reject-with-system-error over a 404 Response (and over a bare TypeError), and switched to JSPromise::rejected_promise so an uncaught call now reports as an unhandled rejection. The PR description argues each choice well (Deno precedent, Fetch spec network error, symmetry with the blob: branch and the request-body-is-Bun.file(missing) path), but per the repo's review guidance API-shape decisions like this warrant maintainer sign-off rather than bot approval.

Other factors

Test coverage is thorough and follows harness conventions (tempDir, using, drained pipes, exit-code asserted last, skipIf(isWindows) for /dev/null). The leak-test rewrite keeps guarding the same leak (author re-verified by reintroducing .clone()). I also considered whether the synchronous stat on every fetch(file://) is acceptable — the author's rationale (matches Bun.file().size/.exists() and Bun.serve's file rendering) seems sound, but it is one more thing a maintainer may want to weigh.

[robobun]

On the API point above, for whoever reviews this: the shapes I considered and did not pick, so it is easy to ask for one of them instead.

• Returning a 404 Response: fetch() has no status to map ENOTDIR/ELOOP/EISDIR onto, and a missing blob: URL a few lines up already rejects rather than returning a 404, so rejecting keeps the two non-network schemes consistent.
• Rejecting with a TypeError (what Deno does): drops the code/path/syscall that callers currently get from .text() on the same failure. Rejecting with that same system error keeps existing e.code === "ENOENT" handling working, it just fires at fetch().
• Opening the file eagerly (or stat on the thread pool) instead of one synchronous stat: covered in the description; the fd is not needed and nothing else on this path is async today.

Either of the first two is a small change to file_url_unreadable_error if preferred.

[robobun]

Two changes since the first revision, both from self-review:

• The rejection is now the system error materialized as a TypeError (to_type_error_instance), the shape fetch: mark body disturbed when a reader fails; reject network errors as TypeError #35855 established for every other fetch() rejection (ValueError::SystemTypeError). code/errno/syscall/path are unchanged. That also corrects my comment above: a TypeError does not have to drop those fields, and this one does not. The tests assert instanceof TypeError on every rejection.
• Tests now pin the edges of the check: EACCES reported by stat (unsearchable parent directory) rejects; a mode 000 file still resolves and fails from the body read; a FIFO with no writer resolves, which an eager blocking open would hang on. The two permission tests are skipped on Windows and as root; I ran them as an unprivileged user locally.

The remaining pushes only shortened comments. Description updated to match.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes the user-facing behavior of fetch() for file: URLs — and the author has explicitly laid out alternative API shapes (reject vs 404, sync stat vs async) for a reviewer to weigh in on — a maintainer look would still be worthwhile.

What was reviewed:

• file_url_unreadable_error — checked that byte-store / fd-backed blobs and successful stat fall through unchanged, and that the reported path is the blob's own path rather than the \\?\-prefixed scratch buffer.
• The new early return — url_string's +1 ref is now created only after the check passes, and the updated fetch-leak.test.ts still exercises the code path it was written to guard.
• Test coverage — ENOENT/EISDIR/EACCES-from-stat reject; mode-000 file, FIFO, /dev/null, and standalone-embedded files still resolve; unhandled rejection is reported.

Extended reasoning...

Overview

This PR makes fetch() of a file: URL reject up front (with a TypeError carrying code/syscall/path/errno) when the path cannot be stat'd or is a directory, instead of resolving with a 200 Response whose body read later throws. It touches src/runtime/webcore/fetch.rs (a ~25-line helper plus reordering of the url_string assignment), adds a new test block in test/js/web/fetch/fetch.test.ts, adds a compile test for embedded files in bundler_compile.test.ts, and updates fetch-leak.test.ts so its url_string leak guard still reaches the code it protects.

Security risks

None identified. The change adds a synchronous stat of a path the caller already controls; no new untrusted input is parsed and no security check is weakened. The path reported in the error is the same path that was already reported by the deferred .text() failure.

Level of scrutiny

This is a user-visible behavior change to a Web API (fetch). The implementation itself is small and well-tested, but the shape is an API design decision: reject-with-TypeError (chosen here, matching Deno and the blob: branch a few lines up) vs. a 404 Response; and a synchronous stat on the JS thread vs. deferring to the thread pool. The author has explicitly listed these alternatives in a PR comment and framed file_url_unreadable_error as easy to swap if a maintainer prefers a different shape. Per the repo's "API design" review guidance, a maintainer should sign off on that choice — it is not something an automated review should ratify on its own.

Other factors

The test coverage is thorough: it pins both sides of the boundary (what rejects, what still resolves), covers all three input forms (string/URL/Request), skips permission tests correctly on Windows and as root, and adds a compile test proving embedded standalone-executable files (byte-backed stores) are not broken by the new check. The comment-cop bot's inline notes on comment length have all been addressed and marked resolved. Using JSPromise::rejected_promise here (rather than the dangerously_... variant used by neighboring exits) is a deliberate divergence explained in the description and covered by the unhandled-rejection test.

[robobun]

0ca5f63 is test-only: the fetch-leak.test.ts bound is now branched on ASAN with a longer warm-up (numbers in the description), and the embedded-file fetch() assertion moved into compile/EmbeddedFileOutfile, which already builds that fixture, instead of building a second executable per lane.

#37501 is the same error-shape change for the upload direction (a Bun.file() request body that cannot be opened) and touches a different part of fetch_impl; the two are independent and can land in either order.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes user-facing fetch("file://...") behavior (previously-resolving calls now reject) and the author explicitly flagged the API shape — TypeError-with-system-error-fields vs. 404 Response vs. plain TypeError — as an open question for a maintainer, a human sign-off on that choice would be worthwhile.

What was reviewed:

• file_url_unreadable_error control flow: byte-store / fd-backed blobs and non-directory stats fall through to the existing 200 path; only stat failures and directories reject.
• Resource lifetime on the new early return: url_string's +1 ref is now created after the check, and file_blob's StoreRef is released via Drop — no new leak.
• Test coverage pins both edges (FIFO/char-device/mode-000 file still resolve; ENOENT/EISDIR/unsearchable-parent reject) and the embedded-file exemption.
• The updated leak test still exercises the code path it guards (file now exists so url_string is reached).

Extended reasoning...

Overview

The PR adds a synchronous stat pre-check to the file: branch of fetch_impl in src/runtime/webcore/fetch.rs. When the path does not stat or is a directory, fetch() now rejects with a TypeError carrying code/syscall/path/errno, instead of resolving to a 200 Response whose body read later fails. The url_string +1 ref creation is moved after the new early return so it cannot leak. Tests are added/updated in fetch.test.ts (7 new cases covering ENOENT/EISDIR/EACCES-from-stat rejection, FIFO/char-device/mode-000-file resolution, and unhandled-rejection reporting), fetch-leak.test.ts (updated to fetch an existing file so it still reaches url_string, with an isASAN-branched threshold), and bundler_compile.test.ts (embedded files in a standalone executable still resolve).

Security risks

None identified. The change adds a read-only stat of a path that the existing code already opens on body read; no new attack surface. The path comes from the user's own fetch() argument.

Level of scrutiny

This warrants human review because it is a user-facing behavioral change to a core Web API. Code that today does const r = await fetch("file:///maybe-missing"); if (r.ok) ... will now throw at the await. The author laid out three viable API shapes (reject-with-system-TypeError, 404 Response, plain TypeError à la Deno) and explicitly asked for a maintainer to pick one. That is the API-design category the review guidelines reserve for maintainers. The synchronous stat on the JS thread is also a design choice (well-justified against Bun.file().size/Bun.serve precedent, but still a choice).

Other factors

• The implementation itself is small, well-scoped, and thoroughly tested (both the rejecting and non-rejecting boundaries are pinned; the embedded-file exemption is covered).
• I checked that dropping file_blob on the early return releases its Store ref via StoreRef's Drop (src/jsc/webcore_types.rs:123-125), so the new return does not introduce a leak.
• All comment-cop threads are resolved (comments reduced to single lines).
• No prior human reviews; CI build 92325 was in progress at the time of review.