Skip to content

Bun.serve: support directory tree routes via { dir: "..." } - #36156

Merged
Jarred-Sumner merged 30 commits into
mainfrom
farm/a0d36689/serve-directory-routes
Jul 29, 2026
Merged

Jarred-Sumner merged 30 commits into
mainfrom
farm/a0d36689/serve-directory-routes

Conversation

@robobun

@robobun robobun commented Jul 27, 2026 •

Copy link
Copy Markdown
Collaborator

Mounting a directory at a wildcard route now serves its contents as static files:

Bun.serve({
  routes: {
    "/static/*": { dir: "./public" },
  },
});

Path resolution

The part of the request URL after the route prefix is:

  1. percent-decoded once (malformed %XX → fallthrough),
  2. rejected if it contains NUL or \,
  3. lexically cleaned of //, ., and .. (any .. that would climb above the root is rejected outright),
  4. opened relative to the open root directory fd.

On Linux step 4 is openat2(RESOLVE_BENEATH | RESOLVE_NO_MAGICLINKS), so any resolution step, including every symlink hop, that would leave the root is rejected by the kernel with EXDEV. This is TOCTOU-free and stronger than the default in nginx/Apache/Caddy/Go http.FileServer, none of which use openat2. On macOS and Windows step 4 is plain openat(dirfd, rel); the lexical clean is the only containment (same as nginx/Caddy default). No fallback is attempted if openat2 is unavailable.

Response

The file body is streamed by FileResponseStream (the same sendfile path FileRoute uses). Headers:

  • Content-Type from the file extension (application/octet-stream fallback)
  • Last-Modified from fstat mtime
  • weak ETag in the nginx/send shape W/"<size-hex>-<mtime-sec-hex>"
  • Accept-Ranges: bytes

Conditional requests follow the RFC 9110 §13.2.2 ladder (If-Match → If-Unmodified-Since → If-None-Match → If-Modified-Since), and single-range Range is handled via the existing RangeRequest helper (206/416 with Content-Range).

A request that resolves to a directory is served index.html from that directory. A miss (including traversal/symlink rejection) yields to the next matching route, so a fetch handler can supply the 404.

mtime cache

A fixed 1024-slot StatHash array keyed by wyhash(subpath) keeps the formatted Last-Modified string around so repeat hits on an unchanged file skip the date formatter.

API

{ dir, style } continues to select the framework router; { dir } alone is the new static directory route. The route path must end in /*.

Tests

17 integration tests cover nested dirs, custom prefixes, index.html, HEAD, ETag/Last-Modified/304, Range 206/416, fallthrough-on-miss, percent-decoding, reload, and the security vectors: ../, ..%2f, %2e%2e/, %252e%252e, %00, %5c, and (Linux-only) symlink escape vs. in-root symlink.

Supersedes the Zig-era jarred/directory-routes branch.


no test proof · iteration 9 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/serve-directory-routes.test.ts

Mounting a directory at a wildcard route now serves its contents as static
files:

    Bun.serve({
      routes: {
        "/static/*": { dir: "./public" },
      },
    });

The request path after the prefix is percent-decoded once, scrubbed of NUL
and backslash, lexically cleaned of "."/"..", and opened relative to the
root directory. On Linux the open is openat2(RESOLVE_BENEATH |
RESOLVE_NO_MAGICLINKS), so any resolution step (including symlink hops)
that would leave the root is rejected by the kernel; other platforms fall
back to openat() with the same lexical containment.

Responses reuse the FileResponseStream path that FileRoute already uses
(sendfile on Linux), with Content-Type from the file extension,
Last-Modified, a weak W/"size-mtime" ETag, If-None-Match /
If-Modified-Since / If-Match / If-Unmodified-Since handling, and single
Range support. A small fixed-size per-path StatHash cache avoids
reformatting the Last-Modified date on repeat hits. Directory requests
serve index.html; misses yield to the next route.

{ dir, style } continues to select the framework router; { dir } alone is
the new static directory route.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds directory-backed Bun.serve() routes with secure path resolution, static-file HTTP behavior, route registration, type declarations, documentation, and tests for routing, caching, traversal, ranges, and lifecycle behavior.

Changes

Directory route serving

Layer / File(s) Summary
Route contracts and construction
packages/bun-types/serve.d.ts, src/runtime/server/mod.rs, src/runtime/server/server_body.rs, docs/runtime/http/routing.mdx
Adds directory route options, the AnyRoute::Directory variant, ownership handling, direct { dir } route construction, and usage documentation.
Safe path resolution and filesystem access
src/paths/resolve_path.rs, src/runtime/server/DirectoryRoute.rs, src/sys/lib.rs, src/sys/linux_syscall.rs
Resolves normalized URL paths beneath the configured directory, serves directory indexes, supports stat caching, and uses root-constrained filesystem opens.
HTTP metadata, conditions, and streaming
src/runtime/server/DirectoryRoute.rs, src/runtime/server/FileRoute.rs
Adds directory-file response streaming and lifecycle handling while centralizing precondition, range, status, and response-header logic.
Static route registration
src/runtime/server/ServerConfig.rs, src/runtime/server/mod.rs
Connects directory routes to static-route callbacks and HTTP/1 and HTTP/3 router registration.
Directory route behavior validation
test/js/bun/http/serve-directory-routes.test.ts
Tests file and index serving, route precedence, HTTP conditions, ranges, traversal and symlink rejection, caching, concurrency, configuration validation, reloads, and path edge cases.

Possibly related PRs

  • oven-sh/bun#36003: Also changes FileRoute.rs status-line emission behavior used by this route implementation.
  • oven-sh/bun#36161: Extends response-path and routing compatibility for HTTP/2 code touched by these server changes.

Suggested reviewers: alii

🚥 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 is concise and accurately summarizes the main change: adding directory-tree routes via { dir }.
Description check ✅ Passed The description clearly explains the feature and includes verification details, though it doesn't use the template headings verbatim.

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

@robobun

robobun commented Jul 27, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 10:05 AM PT - Jul 29th, 2026

@robobun, your commit 72cd1b5 is building: #85248

Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/server_body.rs

@Jarred-Sumner Jarred-Sumner left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use unsafe. Do not use scopeguard. Use File instead of scopeguard. Do not duplicate code between FileRoute and DirectoryRoute - the specifics of range checks should be reused.

Can subpath handling use our existing directory normalization code?

@robobun

robobun commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

Working on it: will extract the fstat/precondition/range/stream path into a shared helper that both FileRoute and DirectoryRoute call, switch the fd handling to bun_sys::File (Drop-based close, no scopeguard), drop the duplicated unsafe scaffolding, and check whether resolve_path::normalize_string can replace the hand-rolled subpath cleaner.

Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread test/js/bun/http/serve-directory-routes.test.ts Outdated
- Share the RFC 9110 precondition ladder, Content-Range writer, and status
  writer between FileRoute and DirectoryRoute (new pub(crate) helpers in
  file_route.rs).
- Use bun_sys::File for fd ownership instead of scopeguard; the request ref
  is released via a small Drop guard rather than open-coded unsafe.
- Drop duplicated unsafe scaffolding: on() is now a safe &self method
  dispatched via BackRef from the uWS trampolines.
- Switch from RESOLVE_BENEATH to RESOLVE_IN_ROOT | NO_MAGICLINKS and add an
  ENOSYS/EPERM/EINVAL fallback to plain openat on kernels without openat2.
- Reuse resolve_path::normalize_string_buf for the .. clean instead of a
  hand-rolled loop.
- Strip the query string before resolving (req.url() is uWS getFullUrl()).
- Convert Windows openat HANDLE fds to libuv fds before streaming.
- Drop the unused root_path field; trim comments.
Comment thread src/runtime/server/DirectoryRoute.rs
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
Comment thread src/runtime/server/FileRoute.rs Outdated
Comment thread src/runtime/server/FileRoute.rs
Comment thread src/sys/lib.rs
@robobun

robobun commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed b01e966 addressing the five findings:

  • %XX-encoded-pchar bypass (🔴): resolve_subpath now scans the raw suffix and rejects any %XX whose decoded byte is an RFC 3986 pchar (unreserved / sub-delims / : / @). Route segments can only consist of pchars on the wire, so this makes percent-decoding the identity on every byte that can influence routing, while still decoding %20, high-bit UTF-8, etc. /static/%61dmin/secret.txt, /static/admi%6E/..., /static/%40user/..., /static/%2Ewell-known/... all return 404. Added to both the unit test and the integration bypass test.
  • ISDIR→index.html bypass (🔴): a non-root subpath that opens as a directory without a trailing slash now returns 301 Location: {path}/ instead of serving {rel}/index.html. The redirected request re-enters uWS routing, so /static/admin → 301 /static/admin/ → /static/admin/*. Location is built from the already-validated path portion of the request-target (starts with url_prefix, first segment non-empty), so it always begins with a single / and cannot be a //host/ protocol-relative URL. The bypass test now creates public/admin/index.html and asserts /static/admin does not leak it.
  • Uppercase extensions → application/octet-stream (🔴): by_extension_no_default now uses EXTENSIONS.get_ascii_case_insensitive(), matching nginx/Apache/Go/send. New test covers .JPG → image/jpeg, .CSS → text/css, .MJS → javascript.
  • Dead || rel == b"." (🟡): removed; resolve_subpath can only deliver the root as rel.is_empty(). The resolve_subpath doc comment now says "validate canonical" rather than "normalize".
  • Stale docs (🟡): routing.mdx and the DirectoryRouteOptions JSDoc now describe rejection (not cleaning) of non-canonical paths, drop the "other platforms rely on the same lexical normalization" sentence, and document the 301 redirect.

29/29 tests pass locally (including bun-serve-file.test.ts 103/103); rust:check-all green on all 10 targets; clippy clean.

Comment thread src/runtime/server/DirectoryRoute.rs
Comment thread src/runtime/server/DirectoryRoute.rs
Comment thread src/runtime/server/FileRoute.rs Outdated
Comment thread docs/runtime/http/routing.mdx Outdated
A request like /static/page.html/ was stripped of its trailing slash by
resolve_subpath() and then served as a regular file, which (1) breaks
relative-URL resolution in the served document and (2) bypasses an exact
overlapping route: /static/secret.pdf/ routes past an exact
"/static/secret.pdf" handler in uWS (the exact node has no child for the
trailing empty segment) and reached the outer wildcard. open_subpath()
now returns None when a regular file is opened with had_trailing_slash,
matching nginx and npm send.

Also:
- build_slash_redirect: bound path.len() against the output buffer and
  truncate query instead of panicking when url_prefix + after_prefix
  exceeds PATH_MAX (resolve_subpath only bounds after_prefix).
- FileRoute: inline the one-line write_status_code wrapper left over
  from the write_any_status extraction.
- docs/types: "percent-encoded ASCII character" over-stated the guard
  (space is ASCII but %20 passes); reword to match is_url_path_literal.
Comment thread src/runtime/server/DirectoryRoute.rs
@robobun

robobun commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed ff44afe addressing the four findings on b01e966:

  • Trailing slash on a regular file (🔴): open_subpath() now returns None when a regular file is opened with had_trailing_slash == true, so /static/page.html/ → 404 (nginx / npm send behavior). Closes the exact-route bypass: the test now has "/static/secret.pdf": auth + public/secret.pdf on disk and asserts /static/secret.pdf/ does not leak the body.
  • build_slash_redirect panic (🟡): now bounds path.len() against out.len() (returns 0, caller 404s) and truncates query to fit instead of an unbounded copy_from_slice. Covered by unit test.
  • FileRoute::write_status_code wrapper (🟡): inlined and deleted.
  • Docs wording (🟡): both surfaces now say "a %XX sequence encoding a character that may appear literally in a path segment" (matching is_url_path_literal) instead of "a percent-encoded ASCII character".

29/29 tests pass locally (+ bun-serve-file.test.ts 103/103); rust:check-all 10/10; clippy clean.

Comment thread src/runtime/server/DirectoryRoute.rs
Comment thread src/runtime/server/DirectoryRoute.rs
- resolve_subpath: decode directly into out and drop the separate
  scratch buffer + trailing copy_from_slice. The two-buffer shape
  existed for normalize_string_buf (removed in c1804c6); now
  validation runs in place, so the extra pool round-trip and memcpy
  per request were dead weight.

- Reject empty segments (//) in the directory route path at config
  time. build_slash_redirect relies on url_prefix having no empty
  first segment so Location cannot be //host/ (protocol-relative),
  and the segment-scan guard in resolve_subpath only validates bytes
  after the prefix. A "//*" route key was accepted and would emit
  Location: //foo/ on a directory redirect; now it throws at serve().
@robobun

robobun commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed d4e5ad8 addressing the two findings on ff44afe:

  • Vestigial scratch buffer (🟡): resolve_subpath now decodes directly into out and the trailing copy_from_slice is gone. The two-buffer shape was for normalize_string_buf (removed in c1804c6); the caller's extra path_buffer_pool::get()/drop() per request is removed.
  • "//*" route key → //host/ redirect (🟡): config-time validation now rejects // in the directory-route path alongside the existing : check, and DirectoryRoute::create debug_assert!s the invariant it relies on. New test covers "//*", "//assets/*", "/a//*".

30/30 tests pass locally (+ bun-serve-file.test.ts 103/103); rust:check-all 10/10; clippy clean.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new findings on d4e5ad8 — the two remaining items from the last pass (config-time // rejection for the open-redirect invariant, and the dead scratch buffer in resolve_subpath) are both addressed. That said, this is ~700 lines of new security-sensitive static-file-serving code plus a new user-facing API surface, so it's worth a human look before merge.

What was reviewed:

  • resolve_subpath canonical-path guard: pchar rejection, %2F/slash-count, ./../empty segments, NUL/\/:, trailing-slash handling on both ISDIR and ISREG.
  • build_slash_redirect: path.len() bound + query truncation now panic-free; // open-redirect closed at config time (server_body.rs + debug_assert).
  • FileRoute refactor: status_for_preconditions/write_content_range/write_any_status are behavior-preserving extractions of the pre-PR inline ladder; bun-serve-file.test.ts still applies.
  • openat2_in_root fallback caches ENOSYS/EPERM/EINVAL/E2BIG and degrades to plain openat — matches the docs' stated Linux-only guarantee.
Extended reasoning...

Overview

Adds "/prefix/*": { dir: "./path" } directory routes to Bun.serve. New file src/runtime/server/DirectoryRoute.rs (~700 lines) implements URL→filesystem resolution, conditional-request handling, Range support, a per-path Last-Modified string cache, and a 301 trailing-slash redirect. FileRoute.rs is refactored to extract three shared helpers (status_for_preconditions, write_content_range, write_any_status). server_body.rs/ServerConfig.rs/mod.rs wire the new AnyRoute::Directory variant into route registration and refcounting. sys/lib.rs + linux_syscall.rs add an openat2(RESOLVE_IN_ROOT|NO_MAGICLINKS) wrapper with an availability-cached fallback. MimeType.rs switches extension lookup to case-insensitive. Docs, .d.ts, and a 700-line integration test file round it out.

Security risks

This is a static file server exposed directly to untrusted HTTP input, so the attack surface is exactly the class REVIEW.md's Security section is about: path traversal, symlink escape, route-precedence bypass (a more-specific auth handler being routed around via encoding tricks), open redirect via Location, and DoS via oversized paths. Over the course of this PR seven distinct route-precedence-bypass and traversal vectors were found and closed (http://x?q/... absolute-form parsing, %2F////./.. normalization, %61dmin pchar-encoding, ISDIR→index.html without trailing slash, ISREG with trailing slash, // in the route key). The final resolve_subpath rejects rather than normalizes, which is the right fail-closed shape, and RESOLVE_IN_ROOT gives kernel-level TOCTOU-free containment on Linux. On macOS/Windows containment is lexical only (documented as such).

Level of scrutiny

High. This is (a) new user-facing API with design choices a maintainer should ratify — {dir} vs {dir, style} disambiguation, statCache option name/default, 301-vs-404 for directory-without-slash, 404-vs-fallthrough on miss; (b) production-critical hot-path code with intrusive refcounting and FFI userdata pointers whose lifetime spans async FileResponseStream completion; and (c) a security boundary where every prior review round found a real bypass. The FileRoute.rs refactor also touches an existing shipping code path (conditional-request evaluation for BunFile routes), so a regression there would affect existing users.

Other factors

Test coverage is thorough for the vectors that were found (29 integration tests including raw-socket adversarial inputs, symlink escape on Linux, route-precedence bypass with an overlapping 401 handler, PATH_MAX boundary probes, and server.reload()), plus Rust unit tests for resolve_subpath and build_slash_redirect. The by_extension_no_default case-insensitivity change affects every caller of that helper, not just DirectoryRoute — I checked that the other callers (bundler output naming, DevServer) already pass lowercase, so it's a pure widening. All prior inline findings on this PR are marked resolved and I verified each fix landed in the current diff.

@Jarred-Sumner Jarred-Sumner left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security review pass. I built the branch and probed a debug build over raw sockets (revision d4e5ad8820), plus a source audit of the Windows path which I have not executed. Everything marked reproduced was hit on the wire against this branch; the Windows item is a source trace that needs confirming on a Windows box before we treat it as settled.

Blockers

# Issue Sev Evidence Fix
1 Overlong UTF-8 traversal on Windows. %C0%AE / %C0%AF / %C1%9C decode to bytes that are not pchars, so resolve_subpath accepts them and sees no .., /, or \ in the decoded bytes. On Windows open_beneath → bun_sys::openat → openat_windows_a → convert_utf8_to_utf16_in_buffer; simdutf rejects the overlong sequence and the WTF-8 fallback decode_wtf8_one (immutable.rs ~2473) has no overlong / continuation check: ((0xC0&0x1F)<<6)|(0xAE&0x3F) = 0x2E ., C0 AF → 0x2F /, C1 9C → 0x5C \. normalize_path_windows then resolves .. textually against the dirfd's NT path, clamped only at the volume root (lib.rs ~6730-6785), and nothing rechecks the result is under dir. So GET /static/%C0%AE%C0%AE%C0%AFsecret.txt opens <dir>\..\secret.txt; add triplets to go anywhere on the volume. Critical (Windows) source-traced, not executed — please confirm: mkdir C:\srv\public, put C:\srv\secret.txt, serve { dir: "C:\\srv\\public" }, request the URL above In resolve_subpath, after decode_into, 404 if decoded is not valid UTF-8 (or reject any decoded byte ≥0x80 outside a valid sequence). Independently, decode_wtf8_one should return U+FFFD for b0 ∈ {C0, C1} and non-continuation b1 — that decoder is pre-existing, but this is the first caller feeding it raw request bytes.
2 301 slash-redirect sends Content-Length twice. write_header_int(b"content-length", 0) then resp.end(b""), and end auto-writes CL because the flag is only set by mark_wrote_content_length_header() / internalEnd. On the wire: content-length: 0\r\nContent-Length: 0. llhttp (Node/undici clients) and nginx-as-upstream hard-reject duplicate CL, so /static/somedir breaks behind common infra. Medium (correctness) reproduced DirectoryRoute.rs:162 — drop the explicit write, or call mark_wrote_content_length_header() after it.
3 PR body / docs don't match the code. Body says "No fallback is attempted if openat2 is unavailable", RESOLVE_BENEATH, 1024 slots, "yields to the next matching route"; code has a cached process-wide fallback to plain openat (sys/lib.rs openat2_in_root), RESOLVE_IN_ROOT, 256 slots, 404-on-miss. routing.mdx / serve.d.ts state the Linux clamp unconditionally and never say symlinks are followed on macOS/Windows/fallback. must-fix (docs) — Reword: clamp applies on Linux 5.6+ where openat2 is permitted; elsewhere (and on fallback) symlinks inside dir are followed like nginx/Go. Add one sentence: distinct URLs can name the same file on case-insensitive / normalizing filesystems, so don't use an overlapping route as an access-control boundary for files inside dir.

Real but small

# Issue Reproduced Suggestion
4 Exact-route bypass via percent-encoded ASCII non-pchars: with "/static/data[1].json": handler (403), GET /static/data%5B1%5D.json → 200 from the dir route; same for notes#2.txt via %232. The comment above the scan says route segments "can only consist of pchars on the wire", but uWS routes any byte >0x20, so literal [ ] { } | ^ # route keys can be side-stepped with their %XX forms. #35249 does not cover this half (it only uppercases hex and encodes bytes ≥0x80). ✅ Either extend the reject set to %XX decoding into 0x21–0x7E, or fix the comment and document the gap. Low-Medium — needs an exact route on a bracket/hash-named file inside the served tree.
5 Same class, non-ASCII: "/static/admin/caf%C3%A9.txt" → 403; %c3%a9, %C3%a9, and raw UTF-8 bytes on the wire all reach the file via the dir route. Platform-independent, but the byte-literal router is pre-existing (same result for a plain Response route on 1.4.0) and #35249 canonicalizes exactly this. ✅ State the dependency / merge order on #35249 in the body.
6 openat2_in_root latches a process-global UNAVAILABLE on the first ENOSYS/EPERM/EINVAL/E2BIG and never logs. ENOSYS/E2BIG are capability errors, but EINVAL/EPERM can be per-path (vfat/exfat/cifs name validation on lookup, fanotify permission-event agents denying a specific file), so a single request can flip the whole process to symlink-following mode. Fallback landing on nginx/Apache/Go default behavior is fine; a per-request errno mutating global security state is not. source Probe once at route creation (open . under root_fd) and latch on that only; treat per-request EINVAL/EPERM as a miss; scoped_log! the fallback once.
7 /static// reaches index.html past an exact "/static/" route (the i==0 && end==0 hatch), contradicting "empty segments are rejected". ✅ Reject when after_prefix == "/", or fix the doc sentence.

Route-precedence claim vs. filesystems (docs, not code)

Reproduced on APFS: GET /static/Admin/secret.txt → 200 past a /static/admin/* 403 guard (case folding); NFD cafe%CC%81.txt past an NFC exact route (normalization). Symlink to /etc/hosts inside dir served on macOS. All of these are filesystem-level aliasing, not transformations resolve_subpath applies, so "the served path is always the path the router matched" is literally true — but it reads like an access-control guarantee it can't be on APFS/NTFS. Same behavior exists on main for any hand-rolled Bun.file wildcard handler, and nginx/Go/Express/Deno all share it; Apache's manual documents this exact bypass and answers it with an operator warning. Covered by the docs sentence in #3; I don't think code should chase this.

Notes for the body (behavior changes riding along)

  • by_extension_no_default is now ASCII-case-insensitive for every Bun.file().type caller, not just this route (.HTML → text/html where it was octet-stream). Intentional and matches mime/nginx, but it's a global change.
  • status_for_preconditions now gates If-Modified-Since on base_status == 200; the old inline block let e.g. a 202 Bun.file route return 304. RFC-correct, but it's shipped-behavior drift.
  • Dotfiles (.env, .git/config) are served like any other file — consistent with nginx/Go/Caddy, opposite of send/serve-static; worth one docs line either way.

Checked and clean

TOCTOU (single opened fd feeds fstat, ETag, Range, and the stream; dir → index.html re-open goes back through open_beneath) · fd ownership on every early return (304/412/416/HEAD all drop File; only the stream path into_raw()s with auto_close) · ref/UAF (ref_() precedes any root_fd use, ResponseGuard/ManuallyDrop/one-shot stream callbacks match FileRoute's contract) · stat-cache reentrancy + accounting · H1 header injection through Location (uWS request-target scanner stops at bytes ≤0x20, and the redirect requires an existing directory) · protocol-relative //host Location · %2F/..%5c/%00/:/ADS/double-decode · request-target length vs PATH_MAX buffer · H1 routing parity (path_and_query mirrors getUrlForRouting).

Test-suite gaps

The .., %00, and %5C cases are unfalsifiable on Linux (deleting the check still 404s via RESOLVE_IN_ROOT or ENOENT on a nonexistent target); /static//<absolute path> (the vector the leading-empty-segment check exists for off-Linux) has no test; the traversal test accepts [404, 400] while the docs promise 404; nothing covers ..%5c, %2E%2E, %C0%AE, or any Windows-specific shape. Happy to send the full corpus table.

Requesting changes for #1 (pending Windows confirmation), #2, and #3.

Comment thread src/runtime/server/DirectoryRoute.rs
Routing is case-sensitive but macOS APFS and Windows NTFS are not, so a
case-varied URL routes to the directory wildcard and the filesystem
case-folds the open. This matches nginx/Caddy/Go/express behavior and
resolve_subpath applies no transformation here (the filesystem does), so
no code change; but the docs claimed "the served path is always the path
the router matched" without qualification, which is misleading. Add a
callout to routing.mdx and a sentence to the DirectoryRouteOptions JSDoc
advising not to gate content inside dir via overlapping routes.
Comment thread src/runtime/server/DirectoryRoute.rs
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
- resolve_subpath: the mount-root special case (i==0 && end==0) also
  fired for after_prefix == b"/" (decoded_len=1, trailing-slash strip
  leaves end=0), so /static// served index.html while /static//a was
  rejected. Hoist the mount-root return to an explicit decoded_len==0
  check before the segment scan; the scan then rejects every empty
  segment unconditionally.

- Skip alt-svc on H3 responses to match FileRoute/StaticRoute/
  RequestContext (the header advertises H3 to H1/H2 clients; writing
  it on an H3 response is harmless per RFC 7838 but redundant).
@robobun

robobun commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed a28e6c9 addressing the two findings on 5276989:

  • /static// served index.html (🟡): the mount-root special case (i == 0 && end == 0) also matched after_prefix == b"/" after the trailing-slash strip. Hoisted the mount-root return to an explicit decoded_len == 0 check before the segment scan; the scan now rejects every empty segment unconditionally. Unit test adds resolve(b"/static//", ...) == None and resolve(b"//", b"/") == None.
  • alt-svc on H3 responses (🟡): now gated on !matches!(resp, AnyResponse::H3(_)) to match FileRoute/StaticRoute/RequestContext.

30/30 tests pass locally; rust:check-all 10/10; clippy clean.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new issues found on a28e6c9, but this adds a security-sensitive user-facing API (serving files from user-controlled URL paths, with platform-dependent containment guarantees), so it warrants a maintainer sign-off before merge.

What was reviewed:

  • resolve_subpath traversal/canonicalization — the lone-slash and //-prefix holes from earlier rounds are closed; unit tests cover the boundary cases.
  • FileRoute refactor into status_for_preconditions/write_content_range — behavior-preserving vs. the pre-extraction ladder.
  • openat2_in_root fallback caching — ENOSYS|EPERM|EINVAL|E2BIG correctly latch to openat; fd ownership on every non-streaming return goes through ResponseGuard/File::drop.
Extended reasoning...

Overview

This PR adds "/prefix/*": { dir: "./path" } to Bun.serve() routes, serving a directory tree as static files. The bulk is a new 713-line src/runtime/server/DirectoryRoute.rs plus a refactor of FileRoute.rs that extracts the RFC 9110 precondition ladder, Content-Range writing, and status writing into shared pub(crate) helpers. Supporting changes: a new openat2(RESOLVE_IN_ROOT|NO_MAGICLINKS) syscall wrapper with cached-unavailability fallback in src/sys/, an AnyRoute::Directory variant wired through mod.rs/ServerConfig.rs/server_body.rs, case-insensitive MIME extension lookup, an off-by-one fix in resolve_path::z(), new .d.ts/docs, and a 708-line integration test file.

Security risks

This is squarely security-sensitive code: it takes untrusted URL bytes, percent-decodes them, and opens files on disk. The threat model has been iterated on extensively across ~15 prior review rounds on this PR (path traversal via ../%2e%2e/%2f, NUL/\\/: injection, route-precedence bypass via encoded pchars or non-canonical segments, protocol-relative Location open-redirect, trailing-slash file bypass, FIFO event-loop hang, symlink escape). All findings from those rounds are marked resolved and the current diff reflects the fixes. The residual risk is the documented one: on macOS/Windows containment is lexical-only (no RESOLVE_IN_ROOT), and case-insensitive filesystems allow a case-varied URL to route past an overlapping handler — both now called out in the docs as a "do not gate content inside dir via overlapping routes" caveat rather than fixed in code. Whether that documented posture is acceptable for a first release is a maintainer call.

Level of scrutiny

High. This is (a) new public API surface with a stability commitment, (b) a network-reachable file-serving path where a single missed edge case is a directory traversal, and (c) a FileRoute refactor whose behavior-preservation matters for existing users. The PR description itself ends with "Ready for maintainer review", and the one human comment so far (Jarred on the stat-cache slot count) was a tuning nit, not a design review. Per the approval guidelines this is not a candidate for bot-only approval regardless of the zero-findings result.

Other factors

Test coverage is thorough (30 integration tests + Rust unit tests covering the resolver), the FileRoute extraction reads as behavior-preserving on inspection, and every prior inline finding has a corresponding fix commit + resolved thread. Deferred follow-ups (implicit .html extension, per-component O_NOFOLLOW on non-Linux, post-open case verification) are enumerated in the PR body. Nothing new to raise; deferring purely on scope and security surface.

128 concurrent connects per batch occasionally hit ConnectionRefused on
Windows aarch64 (listen backlog). The test exercises cache eviction
across >256 paths, not connection concurrency (the separate "concurrent
burst" test covers that), so 32 is sufficient.
Comment thread src/runtime/server/DirectoryRoute.rs Outdated
write_header_int(b"content-length", 0) wrote the header but did not set
HTTP_WROTE_CONTENT_LENGTH_HEADER, so end(b"") wrote a second one. Let
end() emit it once like write_miss and the 412 arm do. The raw() test
helper now returns the header block so the redirect test asserts exactly
one Content-Length header.
@robobun

robobun commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 72cd1b5 addressing the duplicate Content-Length on the 301 redirect: dropped the explicit write_header_int(b"content-length", 0) and let end(b"") emit it once (matching write_miss and the 412 arm). The raw() test helper now returns the raw header block and the redirect test asserts exactly one Content-Length header.

30/30 tests pass locally; rust:check-all 10/10.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new findings on 72cd1b5, but this adds a user-facing Bun.serve API with security-sensitive path handling and warrants a human sign-off on the API shape and containment model.

What was reviewed:

  • resolve_subpath traversal/encoding guards and route-precedence parity (percent-decoded pchars, %2F, ./../empty segments, trailing-slash, absolute-form request-target, //-prefix open-redirect) — all rejected and unit-tested.
  • fd/refcount lifecycle: ResponseGuard releases the route ref on every non-streaming return; open_subpath drops the dir fd before opening index.html; FileResponseStream takes ownership on the streaming path.
  • FileRoute refactor into status_for_preconditions/write_content_range/write_any_status — behavior-equivalent to the removed inline ladder.
  • MimeType::by_extension_no_default is now ASCII-case-insensitive; this affects every caller (bundler, Blob store, FormData, DevServer), not only directory routes.
Extended reasoning...

Overview

Adds "/prefix/*": { dir: "./path" } as a new Bun.serve route value that streams a directory tree. New file src/runtime/server/DirectoryRoute.rs (~710 lines) implements request handling, path validation, conditional/range responses, a per-route stat cache, and the trailing-slash redirect. FileRoute.rs is refactored to share status_for_preconditions(), write_content_range(), and write_any_status() between the two route types. Config parsing in server_body.rs distinguishes { dir } (static directory) from { dir, style } (framework router) and validates the route key. src/sys/ gains openat2_in_root() with an ENOSYS/EPERM fallback to plain openat. MimeType::by_extension_no_default switches to case-insensitive lookup. Docs (routing.mdx), type definitions (serve.d.ts), and a 700-line integration test file are added.

Security risks

This is the classic static-file-server attack surface: path traversal, encoded-slash smuggling, symlink escape, route-precedence bypass, and open redirect on the trailing-slash 301. Across the eight prior review rounds the following were found and fixed: %XX-encoded-pchar bypass of overlapping routes, trailing-slash-on-file bypass of exact routes, //* route key producing a //host/ protocol-relative Location, /static// slipping the empty-segment guard, and a duplicate Content-Length on the 301. The Linux path uses openat2(RESOLVE_IN_ROOT | NO_MAGICLINKS) for kernel-enforced containment; macOS/Windows fall back to plain openat(dirfd, rel) with lexical validation only, which the docs now call out along with the case-insensitive-filesystem caveat. I did not find remaining traversal or fd-leak paths in the current revision.

Level of scrutiny

High. This is new user-facing API on Bun.serve, on the request hot path, with adversarial-input parsing and cross-platform syscall differences. The API shape ({ dir, statCache? }, 404-instead-of-fallthrough on miss, 301 on directory-without-slash, weak ETag scheme) and the decision to make MIME extension lookup case-insensitive globally are design choices a maintainer should confirm.

Other factors

Test coverage is thorough (30 integration tests plus Rust unit tests for resolve_subpath and build_slash_redirect) and each prior finding landed with a regression assertion. The FileRoute precondition refactor is behavior-preserving as far as I traced, and bun-serve-file.test.ts (103 tests) is reported passing. The resolve_path::z() boundary change from > to >= is a correct off-by-one fix (a MAX_PATH_BYTES-length input leaves no room for the NUL terminator). The STAT_CACHE_SLOTS constant in the diff is 256 while the doc string says ~20 KB — consistent.

@Jarred-Sumner
Jarred-Sumner merged commit 0c88489 into main Jul 29, 2026
52 of 56 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the farm/a0d36689/serve-directory-routes branch July 29, 2026 14:20
social4hyq pushed a commit to social4hyq/ohos-bun that referenced this pull request Aug 2, 2026
Upstream's new directory routes (oven-sh#36156) resolve paths with
openat2(RESOLVE_IN_ROOT|NO_MAGICLINKS). On OHOS the kernel kills the
process with uncatchable SIGSYS instead of returning ENOSYS/EPERM, so
Bun.serve({ routes: { dir } }) crashed the whole process
(serve-directory-routes.test.ts SIGSYS on every case).

Apply the same cfg(target_env = "ohos") early-return-ENOSYS guard that
openat2_beneath already has; the sys/lib.rs wrapper then caches
UNAVAILABLE and falls back to plain openat (the documented
seccomp-fallback path). Verified via C probe: openat2 and
name_to_handle_at are SIGSYS-blocked on device; statx/copy_file_range/
sendfile are allowed. [skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants