Repository navigation
Bun.Archive: extract() with a glob rejects at a file or directory entry that the file system refuses - #46646
Bun.Archive: extract() with a glob rejects at a file or directory entry that the file system refuses#46646robobun wants to merge 4 commits into
Conversation
…ry that the file system refuses extract_to_disk_filtered skipped a file or directory entry when its mkdir or open failed, and the promise resolved with a smaller count. extract() without a glob rejects in the same cases for a file entry. - File entry: open first. If the parent directory is missing, make it and open once more. A failure rejects with the error of that system call (code, syscall, path). - Directory entry: mkdir first. The entry is made when mkdir succeeds or when a directory holds the name. Anything else rejects. - A file written with a glob gets the owner-write bit, so a later entry of the same name and a second extract can open it again. - A symlink entry that cannot be made is still skipped, as without a glob. - crate::Error::ReadError and OpenError had this one user and are gone.
|
Updated 5:45 AM PT - Oct 24th, 2026
❌ @robobun, your commit 8f3961e has 2 failures in
🧪 To try this PR locally: bunx bun-pr 46646That installs a local version of the PR into your bun-46646 --bun |
|
Status: ready for review. The diff is green, and CI is red on lanes that this PR does not touch. This PR replaces the draft #44731. CI.
How I reproduced it. Linux x64, the canary of main (
|
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Extraction results and error propagation src/runtime/api/Archive.rs, src/runtime/error.rs |
Filtered extraction returns ExtractResult. Archive-open, directory-open, and header-read failures return ReadError. System errors are converted for promise rejection. The public Error enum no longer contains ReadError or OpenError. |
Filesystem entry handling src/runtime/api/Archive.rs |
Directory extraction creates missing parents. File extraction adds owner-write permission and retries after creating missing parents. Filesystem refusals return system errors, and failed file-data writes return ReadError. |
Extraction behavior tests test/js/bun/archive.test.ts |
Tests cover filesystem failures, glob filtering, parent creation, repeated writes, and symlink cases. |
Suggested reviewers: jarred-sumner, dylan-conway
Priority:
Merge Risk: 
8f396
-
The filtered extraction change has no established merge-blocking issue; normal checks remain appropriate.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies the main change: glob-based archive extraction now rejects refused file or directory operations. Description check The description explains the problem, implementation, scope, trade-offs, and verification results. It uses headings different from the repository template, but it includes equivalent information for b… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains the problem, implementation, scope, trade-offs, and verification results. It uses headings different from the repository template, but it includes equivalent information for both required sections.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
…ry on every platform Windows makes a directory with an open call, so a refused directory entry reported open as its system call there. The directory arm now reports mkdir, as on POSIX. The test expects the Windows error code of a directory entry below a file (ENOENT).
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the normalized_buf[name_len] = 0 write in the Directory arm (bounded by the earlier raw_pathname.len() >= normalized_buf.len() guard, and normalization never lengthens the path) and the removal of crate::Error::ReadError/OpenError (no remaining callers in src/).
Extended reasoning...
The change rewires the glob path of Bun.Archive#extract in src/runtime/api/Archive.rs to surface refused mkdir/open calls as ErrnoException rejections, adds an owner-write bit to extracted file modes, and deletes two unused Error variants in src/runtime/error.rs, with a new table-driven test suite. It touches archive-to-filesystem path handling, which is a security-relevant surface, though the existing is_safe_path gate is unchanged. The confirmed inline findings (mode change for read-only members, Windows syscall/path parity for the parent-mkdir failure, and the still-silent symlink arm) plus the author-flagged open design decisions mean a human should weigh in.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
src/runtime/api/Archive.rs— pre-existing: a caller usingextract(dir, { glob })whose symlink entry is refused still gets a resolved promise with the link silently missing, while file and directory refusals now reject. The SymLink arm at src/runtime/api/Archive.rs:1421-1432 doescontinueon every errno (EACCES, EROFS, ENOSPC, EEXIST) and only retries EPERM/ENOENT. Fix: in this same loop, returnExtractResult::refusedfor a symlink refusal on any errno other than the ones the arm deliberately tolerates, keeping the EPERM skip the author relies on for file systems without symlinks, so a read-only or full destination fails loudly for every entry kind.Why this was flagged
An archive whose matching entries include a symlink is extracted with
extract(dest, { glob: "**" })into a destination that is read-only or out of space, throughExtractContext::do_run->extract_to_disk_filtered(src/runtime/api/Archive.rs:722-729).bun_sys::symlinkatat src/runtime/api/Archive.rs:1421 fails with EACCES or ENOSPC; the arm hitselse { continue; }at src/runtime/api/Archive.rs:1431 and the loop goes on. The promise resolves with a count that omits the link and no error, exactly as on the base branch, while the File and Directory arms in the same match (src/runtime/api/Archive.rs:1365 and 1397) now reject viaExtractResult::refused. The PR text names this as a decision, citing EPERM on file systems without symlinks; that reason covers EPERM only, not EACCES, EROFS, ENOSPC or EEXIST, which the arm swallows as well.Verification: Pre-existing. Trigger:
extract(dest, { glob })where a matching symlink entry hits any errno other than EPERM/ENOENT. At src/runtime/api/Archive.rs:1421-1432 every refusal falls tocontinue, so the function reachesExtractResult::Success(count)at line 1443 and the promise resolves. The test at archive.test.ts:2512-2517 pins exactly this. The base's SymLink arm is byte-identical.
…ntry cannot be made On Windows the error of a failed mkdir names an open call and carries no path, so a file entry whose parent directory could not be made rejected with the open call and the path of the file. Both arms now build that error in one place: the system call is mkdir, and the path is the directory.
|
Reply to the review of
|
Problem
extract(dir, { glob })resolves although an entry was not written: 0 entries into a read-only directory.extract(dir)rejects, anddocs/runtime/archive.mdx:233promises a throw.extract_to_disk_filtered(src/runtime/api/Archive.rs:1333-1373) doesErr(_) => continueafter a failedmake_pathoropenat. install: fix regressions and older bugs found after the package manager consolidation #45307 made its header and write arms reject.Fix
ENOENT. A directory entry needs a directory at its name.code,syscall,path). Files get the owner-write bit, so a second extract can write them.test/js/bun/archive.test.ts, 19 new tests. On main 12 fail as root, 16 as uid 65534. The whole file passes.Background
glob,extract()runsextract_to_disk_filtered. Without it,Archiver::extract_to_dir, whichbun installshares.Downsides
extract(dir)resolves (Notes).extract(dir)still rejects withReadError.openatper directory that the archive does not list. Onefstatatper directory entry that exists. 1,280 bytes of.text.Notes
Decisions to sign off. No maintainer has ruled on these. Each line says where to revert it.
extract(dir)"). The open archive: contain extraction writes within the destination root #31585 skips an entry whose parent is not a real directory, in both call forms. This PR rejects. Revert: the twoExtractResult::refusedexits.extract(dir)resolves on POSIX (5 probed rows, below).extract(dir)counts a directory that it did not make. Revert: the Directory arm.ReadErrorwithout one. Revert:ExtractResult::SysErr.extract(dir)adds0o666to every file mode. Revert:| 0o200.What the rejection looks like. Linux, a user that is not root.
ENOTDIR: not a directory, open '<dest>/d/a.txt'EISDIR: illegal operation on a directory, open '<dest>/b.txt'EACCES: permission denied, mkdir '<dest>/d'EACCES: permission denied, open '<dest>/b.txt'EACCES: permission denied, open '<dest>/b.txt'(the file keeps its content)ENAMETOOLONG: name too long, open '<dest>/<name>'EEXIST: file already exists, mkdir '<dest>/x'ENOSPC: no space left on device, mkdir '<dest>/x'pathis the destination as the caller passed it, joined with the name that the refused call was given. A failed mkdir, of a directory entry or of the parent of a file entry, namesmkdirand the directory on every platform. On Windowsbun_sysmakes a directory with an open call, and its error names that call and carries no path. When the parent directory cannot be made, the rejection is thatmkdirerror and not theENOENTof the open after it. The entries before the refused one stay on disk, and the ones after it are not written.extract(dir)stops in the same way.Not changed. These still resolve.
Archive.rs, the SymLink arm, andcreate_deferred_symlinksinsrc/libarchive/lib.rs).extract(dir)counts it, a glob does not. A rejection there fails on a file system without symlinks. The arm is the code of main, with itsEPERM | ENOENTretry. A test pins the name-taken case.extract(dir)without a glob resolves and counts a directory entry that it cannot make on POSIX (src/libarchive/lib.rs, the Directory arm ofextract_to_dir).TarballStream::make_directoryofbun installdrops it too. Both are outside this PR.is_safe_pathrefuses are skipped by rule.extract(dir)are still a bareReadError. The draft Bun.Archive: extract() rejects with the failed syscall or libarchive's message #44509 is the change for that.The 8 probed rows where the glob form rejects and
extract(dir)resolves (uid 65534, of 41 scenarios).x/and a lonex/y/into a read-only destination,d/over a regular filed,a/b/below a regular filea,x/with no free inodes.extract(dir)resolves and counts each one, and nothing is on disk.l -> nopethenl/a.txt,l -> b.txtthenl/a.txt,l -> nopethenl/sub/. With a glob the symlink is made in archive order, so the entry below it cannot be written.extract(dir)makes symlinks last: the entry becomes a file in a real directoryl, and the link is dropped without an error. On main the glob form resolved and dropped the entry. A test pins one row.extract(dir)resolves.extract(dir), and 1 settles alike with another mode (a 0400 member: 0600 with a glob, 0644 without).Measurements. Local release builds of main
5c6d4e24cdand of this change on top of it, Linux x64.strace,perf,valgrindandbloatyare not installed here. Syscalls come from a ptrace logger (PTRACE_SYSCALLwithPTRACE_GET_SYSCALL_INFO). Allocator calls come from gdb breakpoints on the mimalloc entry points, on the work-pool thread, from the start of the extract job to its return.extract(dir, { glob: "**" })into a fresh destination, main to PR:mkdirat201 to 5,openat201 to 205.mkdirat205 to 5,openat201 to 201.mkdirat201 to 201,openat201 to 401.mkdirat205 to 5,openat201 to 201,fstatat0 to 4.extract(dir), but for the 4fstatat.extract(dir)is the same on both builds.mi_malloc199 to 7 andmi_free6,272 to 6,080. 200 files in 200 directoriesmi_malloc3 to 203 andmi_free6,076 to 6,276.fstatat)..text: 82,231,705 to 82,232,985 bytes (llvm-size).run_on_poolforExtractContext4,111 to 4,494 bytes. The newExtractResult::refusedandmkdir_errorare 402 and 152 bytes. The stripped binary is 107,808,480 bytes on both.extract()job allocation: 152 to 160 bytes (llvm-objdump). Both are in the 160-byte bin of mimalloc (vendor/mimalloc/src/page-queue.c,mi_bin). The error is boxed for that reason.Weighed.
extract_to_dirthe glob form takes that loop's modes (| 0o666: a 0600 member is written as 0644), its counts (it counts entries that it did not make) and its symlink rules, and that loop still resolves after a refused directory entry or symlink. It needs a maintainer's agreement first, and this PR does not block it.create_deferred_symlinks, asextract_to_dirdoes. It removes the three symlink rows above, and it drops the link without an error in each of them. The open archive: refuse symlinks in the destination with openat2(RESOLVE_NO_SYMLINKS) and O_NOFOLLOW_ANY #41750 carries that decision.ReadErrorfor the new exits, asextract(dir)gives. The caller then cannot tell which entry to exclude, and main gave it every other entry.extract_to_dir(EACCES | EPERM | ENOENT). A missing parent answersENOENTon POSIX and on Windows (src/errno/windows_errno.rs), so the file arm retries onENOENTonly.Other open PRs on this function. The first one merges cleanly today. The others conflict with main.
3a3fc4170eit ports this loop with the same threecontinuearms, so this change still applies after it. The two PRs overlap in the exits ofextract_to_disk_filteredand insrc/runtime/error.rs. If Replace libarchive with a tar reader and writer in safe Rust #46638 lands first, I rebase this PR onto it. Its golden tests extract without a glob, so this PR does not change their results.files(glob)names): it edits the loop head, not these arms.{ count: 0 }with a glob for a locked destination, and its mode rule is the opposite of| 0o200. Whichever lands second changes those.continueis a containment skip next to a file-system refusal that rejects.Tests.
make_pathonENOENT, no owner-write bit, no destination inpath.test/js/bun/archive.test.ts: 138 pass, 5 skip, 0 fail as root, and 142 pass, 1 skip, 0 fail as uid 65534.bun run rust:check-all: 12 targets ok.cargo clippy -p bun_runtime: no warning in the changed files.ENOENT,EEXIST,EPERM). It failed the two directory rows that run there: the rejection namedopenas the system call. The second commit makes the directory arm namemkdir, and the second CI run passed on every lane. The third commit does the same for the parent of a file entry, with a row that also runs on Windows. The Windows lanes of the third CI run passed with that row.Review history. The design review picked the in-place edit of the arms over the larger designs above. The self-review of the first build asked for three code changes. Two are in: the directory arm follows one rule (a directory at the name, or a rejection), and the rejection carries the system error. The third, symlinks created last, is rejected for the reason under Weighed. It also asked for the row table of the tests, the rows that pin the retry path, and this list of what is left.
no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/archive.test.ts