From 384cda6359faf42e938f5a5321027cf0cee90057 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 07:52:34 +0000 Subject: [PATCH 01/10] install: bound tar-header preallocation and remove temp dir on failed extraction A tarball whose ustar header declares a size far larger than the body that follows (e.g. a 155-byte tgz with one entry claiming 8 GiB) made bun install fallocate the declared size on Linux before the truncated body was detected, and the failed extraction's temp directory was never removed. Repeated attempts filled $TMPDIR with one fully-allocated copy per run. Archiver::extract_to_dir now caps the preallocation at the length of the decompressed tar buffer (a tar entry's body is stored inline in the archive stream, so it cannot exceed that). The streaming extractor caps at 64 MiB since the stream length is not known at header time. The buffered ExtractTarball::extract path wraps its temp directory in a TempExtractionDir RAII guard that removes it on every error path and is committed once the directory has been renamed into the cache. --- src/install/TarballStream.rs | 14 ++- src/install/extract_tarball.rs | 34 ++++- src/libarchive/lib.rs | 10 +- .../bun-install-streaming-extract.test.ts | 116 ++++++++++++++++++ 4 files changed, 165 insertions(+), 9 deletions(-) diff --git a/src/install/TarballStream.rs b/src/install/TarballStream.rs index e1ecc6ef4f7d..8bc835db3dc0 100644 --- a/src/install/TarballStream.rs +++ b/src/install/TarballStream.rs @@ -889,13 +889,15 @@ impl TarballStream { #[cfg(any(target_os = "linux", target_os = "android"))] { - let size: usize = usize::try_from(entry.size().max(0)).expect("int cast"); + // The header's size field is attacker-controlled; cap so a + // size lie can't fallocate more than one entry's worth of + // real disk before the truncated body is detected. The + // buffered path bounds this by the decompressed tar length; + // here the stream is incomplete, so use a fixed ceiling. + const PREALLOCATE_CEILING: i64 = 64 * 1024 * 1024; + let size = entry.size().max(0).min(PREALLOCATE_CEILING); if size > 1_000_000 { - let _ = bun_sys::preallocate_file( - fd.native(), - 0, - i64::try_from(size).expect("int cast"), - ); + let _ = bun_sys::preallocate_file(fd.native(), 0, size); } } diff --git a/src/install/extract_tarball.rs b/src/install/extract_tarball.rs index d1e039353b49..55cd7b950f45 100644 --- a/src/install/extract_tarball.rs +++ b/src/install/extract_tarball.rs @@ -184,6 +184,35 @@ pub(crate) fn uses_streaming_extraction() -> bool { .unwrap_or(false) } +/// RAII owner of a temporary extraction directory under `parent`. Removes +/// `parent/name` on drop unless [`commit`](Self::commit) is called after the +/// directory has been renamed into the cache. This keeps failed extractions +/// (decompression errors, truncated tarballs, rename failures) from leaking an +/// extraction directory per attempt in `$TMPDIR`. +struct TempExtractionDir<'a> { + parent: Fd, + name: &'a ZStr, +} + +impl<'a> TempExtractionDir<'a> { + #[inline] + fn new(parent: Fd, name: &'a ZStr) -> Self { + Self { parent, name } + } + + /// Disarm the drop guard after the directory has been renamed away. + #[inline] + fn commit(self) { + core::mem::forget(self); + } +} + +impl Drop for TempExtractionDir<'_> { + fn drop(&mut self) { + let _ = Dir::borrow(&self.parent).delete_tree(self.name.as_bytes()); + } +} + impl ExtractTarball { /// Derive the display name and a filesystem-safe basename for this /// package. Shared by the buffered `extract()` path below and the @@ -259,6 +288,7 @@ impl ExtractTarball { let mut resolved: &'static [u8] = b""; let tmpname = FileSystem::tmpname(tmpname_suffix, &mut tmpname_buf.0, bun_core::fast_random())?; + let tmpdir_guard = TempExtractionDir::new(self.temp_dir, tmpname); { let extract_destination = match bun_sys::make_path::make_open_path( tmpdir, @@ -450,7 +480,9 @@ impl ExtractTarball { } } - self.move_to_cache_directory(log, tmpname, name, basename, resolved) + let result = self.move_to_cache_directory(log, tmpname, name, basename, resolved)?; + tmpdir_guard.commit(); + Ok(result) } /// Rename the freshly-extracted temp directory into the cache, read diff --git a/src/libarchive/lib.rs b/src/libarchive/lib.rs index 4125c91f1399..85f882acc4bc 100644 --- a/src/libarchive/lib.rs +++ b/src/libarchive/lib.rs @@ -1826,11 +1826,17 @@ impl Archiver { // #define MAX_WRITE (1024 * 1024) #[cfg(any(target_os = "linux", target_os = "android"))] { - if size > 1_000_000 { + // The header's size field is attacker-controlled; a + // malicious tarball can claim 8 GiB for a 100-byte body + // and fallocate that much real disk before the short body + // is detected. A tar entry's body is stored inline in the + // archive stream, so it cannot exceed `file_buffer.len()`. + let prealloc = size.min(file_buffer.len()); + if prealloc > 1_000_000 { let _ = bun_sys::preallocate_file( file_handle.native(), 0, - i64::try_from(size).expect("int cast"), + i64::try_from(prealloc).expect("int cast"), ); } } diff --git a/test/cli/install/bun-install-streaming-extract.test.ts b/test/cli/install/bun-install-streaming-extract.test.ts index 9d9a61304a3c..1df4d1df01b1 100644 --- a/test/cli/install/bun-install-streaming-extract.test.ts +++ b/test/cli/install/bun-install-streaming-extract.test.ts @@ -1036,3 +1036,119 @@ test.concurrent.each([ await new Promise(resolve => server.close(() => resolve())); } }); + +// ------------------------------------------------------------------- +// Buffered extract: a tarball whose ustar header declares a size far +// larger than the body that follows must fail to extract without +// leaving its temporary extraction directory behind, and without +// allocating the declared size on disk. The body of a tar entry is +// stored inline in the (already-decompressed) archive stream, so the +// declared size is attacker-controlled and unbounded. +// ------------------------------------------------------------------- +describe("buffered extract: malformed tarball cleanup", () => { + function treeSize(root: string): number { + let total = 0; + for (const entry of readdirSync(root, { withFileTypes: true })) { + const full = join(root, entry.name); + if (entry.isDirectory()) total += treeSize(full); + else if (entry.isFile()) total += statSync(full).size; + } + return total; + } + + function leakedExtractionDirs(root: string): string[] { + // Temp extraction dirs are `.-.` directly under $TMPDIR. + return readdirSync(root, { withFileTypes: true }) + .filter(d => d.isDirectory() && d.name.startsWith(".")) + .map(d => d.name); + } + + async function runInstallIsolated(root: string) { + const tmp = join(root, "bun-tmp"); + const cache = join(root, "bun-cache"); + mkdirSync(tmp, { recursive: true }); + mkdirSync(cache, { recursive: true }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "install", "--linker=hoisted"], + cwd: root, + env: { + ...bunEnv, + BUN_TMPDIR: tmp, + TMPDIR: tmp, + TEMP: tmp, + TMP: tmp, + BUN_INSTALL_CACHE_DIR: cache, + }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode, tmp, cache }; + } + + test("tar header size far exceeding body fails cleanly without leaking temp disk", async () => { + // A valid package.json entry followed by a file entry whose header + // claims 16 MiB but whose body is only 100 bytes; libarchive reports + // a truncated archive once it runs out of input. 16 MiB is well above + // the 1 MB preallocation threshold, so the unfixed build fallocated + // the full declared size on Linux before failing. + const DECLARED = 16 * 1024 * 1024; + const pj = Buffer.from(JSON.stringify({ name: "pkg", version: "1.0.0" })); + const body = Buffer.alloc(100, 0x78); + const tar = Buffer.concat([ + tarHeader("package/package.json", pj.length, "0"), + pj, + pad512(pj.length), + tarHeader("package/big.bin", DECLARED, "0"), + body, + pad512(body.length), + Buffer.alloc(1024, 0), + ]); + const tgz = gzipSync(tar); + // The tarball itself is tiny; the damage is in the declared size. + expect(tgz.length).toBeLessThan(1024); + + using dir = tempDir("tar-size-lie", { + "package.json": JSON.stringify({ + name: "app", + version: "1.0.0", + dependencies: { pkg: "file:./pkg.tgz" }, + }), + }); + writeFileSync(join(String(dir), "pkg.tgz"), tgz); + + // Run twice to confirm the leak does not accumulate per attempt. + let lastStderr = ""; + for (let i = 0; i < 2; i++) { + const { stderr, exitCode, tmp, cache } = await runInstallIsolated(String(dir)); + lastStderr = stderr; + expect(stderr).toContain("extracting tarball"); + expect(exitCode).not.toBe(0); + // The failed extraction's temp directory must be gone. + expect({ leaked: leakedExtractionDirs(tmp) }).toEqual({ leaked: [] }); + // Nothing close to the declared size was left anywhere under the + // test-isolated tmp or cache directories. + expect(treeSize(tmp) + treeSize(cache)).toBeLessThan(1024 * 1024); + } + expect(existsSync(join(String(dir), "node_modules", "pkg"))).toBe(false); + expect(lastStderr).not.toBe(""); + }); + + test("a tarball that fails to decompress does not leak its temp directory", async () => { + // Not a gzip stream at all: the buffered extractor creates its temp + // directory, then the zlib reader fails immediately. + using dir = tempDir("tar-bad-gzip", { + "package.json": JSON.stringify({ + name: "app", + version: "1.0.0", + dependencies: { pkg: "file:./pkg.tgz" }, + }), + }); + writeFileSync(join(String(dir), "pkg.tgz"), Buffer.from("this is not a gzip stream")); + + const { stderr, exitCode, tmp } = await runInstallIsolated(String(dir)); + expect(stderr).toContain("error:"); + expect({ leaked: leakedExtractionDirs(tmp) }).toEqual({ leaked: [] }); + expect(exitCode).not.toBe(0); + }); +}); From d5b1f72ed5154ca2d1a634ed81c1f303086f94e7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 09:06:20 +0000 Subject: [PATCH 02/10] ci: retrigger From ee12660ee837f1f81aac4d79f8a398bf50b1144f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 09:26:00 +0000 Subject: [PATCH 03/10] libarchive: clarify preallocation-bound comment for compressed-input callers --- src/libarchive/lib.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libarchive/lib.rs b/src/libarchive/lib.rs index 85f882acc4bc..c796fb2f5377 100644 --- a/src/libarchive/lib.rs +++ b/src/libarchive/lib.rs @@ -1829,8 +1829,13 @@ impl Archiver { // The header's size field is attacker-controlled; a // malicious tarball can claim 8 GiB for a 100-byte body // and fallocate that much real disk before the short body - // is detected. A tar entry's body is stored inline in the - // archive stream, so it cannot exceed `file_buffer.len()`. + // is detected. Bound by the input buffer: for callers + // that pre-decompress (package extraction, `bun create`) + // a tar entry's body is stored inline in `file_buffer` + // so this is exact; callers that hand a compressed + // buffer to libarchive's gzip filter (`Bun.Archive`) + // under-preallocate here, which is acceptable since + // preallocation is best-effort. let prealloc = size.min(file_buffer.len()); if prealloc > 1_000_000 { let _ = bun_sys::preallocate_file( From a56dc882793f872b1c19fde60b0e69544172db96 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:28:28 +0000 Subject: [PATCH 04/10] install: fix clippy mem_forget and manual_clamp lints TempExtractionDir::commit disarms via Option::take instead of core::mem::forget (forbidden per PORTING.md), and the streaming preallocation bound uses clamp instead of max().min(). --- src/install/TarballStream.rs | 2 +- src/install/extract_tarball.rs | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/install/TarballStream.rs b/src/install/TarballStream.rs index 8bc835db3dc0..88f1cbe0c8a5 100644 --- a/src/install/TarballStream.rs +++ b/src/install/TarballStream.rs @@ -895,7 +895,7 @@ impl TarballStream { // buffered path bounds this by the decompressed tar length; // here the stream is incomplete, so use a fixed ceiling. const PREALLOCATE_CEILING: i64 = 64 * 1024 * 1024; - let size = entry.size().max(0).min(PREALLOCATE_CEILING); + let size = entry.size().clamp(0, PREALLOCATE_CEILING); if size > 1_000_000 { let _ = bun_sys::preallocate_file(fd.native(), 0, size); } diff --git a/src/install/extract_tarball.rs b/src/install/extract_tarball.rs index 55cd7b950f45..e6fbcb16bcb5 100644 --- a/src/install/extract_tarball.rs +++ b/src/install/extract_tarball.rs @@ -191,25 +191,30 @@ pub(crate) fn uses_streaming_extraction() -> bool { /// extraction directory per attempt in `$TMPDIR`. struct TempExtractionDir<'a> { parent: Fd, - name: &'a ZStr, + name: Option<&'a ZStr>, } impl<'a> TempExtractionDir<'a> { #[inline] fn new(parent: Fd, name: &'a ZStr) -> Self { - Self { parent, name } + Self { + parent, + name: Some(name), + } } /// Disarm the drop guard after the directory has been renamed away. #[inline] - fn commit(self) { - core::mem::forget(self); + fn commit(mut self) { + self.name = None; } } impl Drop for TempExtractionDir<'_> { fn drop(&mut self) { - let _ = Dir::borrow(&self.parent).delete_tree(self.name.as_bytes()); + if let Some(name) = self.name { + let _ = Dir::borrow(&self.parent).delete_tree(name.as_bytes()); + } } } From 60086485becd6c25c8c11333f91db5fd9b57a438 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 1 Aug 2026 00:35:44 +0000 Subject: [PATCH 05/10] Update preallocation-bound and test comments after #36541 Package extraction now hands compressed bytes to libarchive for tarballs whose gzip ISIZE trailer reports over 64 MB, so the file_buffer.len() bound is no longer always the decompressed length there; the comment now describes the invariant without naming callers. Test comments updated to match (libarchive fails to open the input, not a separate zlib reader). --- src/libarchive/lib.rs | 14 +++++++------- .../install/bun-install-streaming-extract.test.ts | 7 +++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/libarchive/lib.rs b/src/libarchive/lib.rs index c796fb2f5377..287f345df3ee 100644 --- a/src/libarchive/lib.rs +++ b/src/libarchive/lib.rs @@ -1829,13 +1829,13 @@ impl Archiver { // The header's size field is attacker-controlled; a // malicious tarball can claim 8 GiB for a 100-byte body // and fallocate that much real disk before the short body - // is detected. Bound by the input buffer: for callers - // that pre-decompress (package extraction, `bun create`) - // a tar entry's body is stored inline in `file_buffer` - // so this is exact; callers that hand a compressed - // buffer to libarchive's gzip filter (`Bun.Archive`) - // under-preallocate here, which is acceptable since - // preallocation is best-effort. + // is detected. Bound by the input buffer length: when + // `file_buffer` is already the raw tar (pre-decompressed) + // this is exact; when it is the compressed gzip stream + // (libarchive's filter gunzips on the fly) it + // under-preallocates, which is acceptable since + // preallocation is best-effort. Either way the cap is at + // most what the caller actually supplied. let prealloc = size.min(file_buffer.len()); if prealloc > 1_000_000 { let _ = bun_sys::preallocate_file( diff --git a/test/cli/install/bun-install-streaming-extract.test.ts b/test/cli/install/bun-install-streaming-extract.test.ts index 1df4d1df01b1..cc2962b48ce7 100644 --- a/test/cli/install/bun-install-streaming-extract.test.ts +++ b/test/cli/install/bun-install-streaming-extract.test.ts @@ -1041,9 +1041,8 @@ test.concurrent.each([ // Buffered extract: a tarball whose ustar header declares a size far // larger than the body that follows must fail to extract without // leaving its temporary extraction directory behind, and without -// allocating the declared size on disk. The body of a tar entry is -// stored inline in the (already-decompressed) archive stream, so the -// declared size is attacker-controlled and unbounded. +// allocating the declared size on disk. The declared size is +// attacker-controlled and unbounded relative to the input. // ------------------------------------------------------------------- describe("buffered extract: malformed tarball cleanup", () => { function treeSize(root: string): number { @@ -1136,7 +1135,7 @@ describe("buffered extract: malformed tarball cleanup", () => { test("a tarball that fails to decompress does not leak its temp directory", async () => { // Not a gzip stream at all: the buffered extractor creates its temp - // directory, then the zlib reader fails immediately. + // directory, then libarchive fails to open the input as an archive. using dir = tempDir("tar-bad-gzip", { "package.json": JSON.stringify({ name: "app", From 004800ccba8e432ad6a3dfed519f810af968d7f2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 08:08:58 +0000 Subject: [PATCH 06/10] tar extraction: size nothing from the entry header, take the length from libarchive A tar entry header declares a size. Nothing proves that the archive holds that many bytes, and for a sparse entry it never does. The buffered extractor, the streaming extractor and the glob path of Bun.Archive.extract() each had a write loop of their own. On Linux the first two called fallocate() with the header size. That allocated the declared size before a short body was detected, and it turned each hole of a sparse entry into real blocks. It also hid a second defect: each loop left the file at the end of the last block that carried data and dropped the length that libarchive returns with ARCHIVE_EOF. An entry that ends in a hole lost its tail unless the fallocate had already set the length. - lib::EntryWriter writes the data of one entry for all three loops. A block goes to the offset libarchive gives it. The file gets its length in finish(), from the offset returned at the end of the entry's data. - No extraction path calls preallocate_file. The caps on it are gone with it. - TarballStream holds an EntryWriter in place of its copy of the block writer. - Archive::read_data_to_vec reads an entry into memory in steps. The Plucker that bun create uses for package.json no longer allocates the header size. Tests build GNU sparse members by hand, in the old GNU format and in PAX 1.0, and compare each extracted file with its map and, on Linux, its allocated blocks. --- src/install/TarballStream.rs | 178 +++------ src/libarchive/lib.rs | 345 ++++++++--------- src/runtime/api/Archive.rs | 40 +- test/cli/install/bun-create.test.ts | 54 +++ .../bun-install-streaming-extract.test.ts | 355 ++++++++++++++---- test/js/bun/archive.test.ts | 158 +++++++- 6 files changed, 719 insertions(+), 411 deletions(-) diff --git a/src/install/TarballStream.rs b/src/install/TarballStream.rs index 88f1cbe0c8a5..953d80457cd9 100644 --- a/src/install/TarballStream.rs +++ b/src/install/TarballStream.rs @@ -51,7 +51,7 @@ type OSPathZMut<'a> = &'a mut OSPathSliceZ; enum Phase { /// Call `archive_read_next_header` next. WantHeader, - /// Currently writing the body of `out_fd`; call + /// Currently writing the body of `out`; call /// `archive_read_data_block` next. WantData, /// `archive_read_next_header` returned EOF; we are done. @@ -114,17 +114,11 @@ pub struct TarballStream { phase: Phase, /// Output file for the entry currently being written. `None` while - /// between entries or when the current entry is being skipped. - out_fd: Option, - #[cfg(unix)] - use_pwrite: bool, - use_lseek: bool, - /// Per-entry write cursors, carried across `write_data_block` calls so - /// the sparse-file handling in `close_output_file` matches - /// `Archive.readDataIntoFd` exactly (which tracks these across its own - /// block loop). Reset in `begin_entry` when a new output file is opened. - entry_actual_offset: i64, - entry_final_offset: i64, + /// between entries or when the current entry is being skipped. The + /// writer is the one the buffered extractor uses, kept here so a block + /// can be written on each side of an ARCHIVE_RETRY yield. + out: Option, + write_strategy: lib::WriteStrategy, /// Temp directory files are written into before being renamed into the /// cache. Lazily opened on the first drain so the HTTP thread never @@ -264,12 +258,8 @@ impl TarballStream { archive_holds_reading: false, archive: None, phase: Phase::WantHeader, - out_fd: None, - #[cfg(unix)] - use_pwrite: true, - use_lseek: true, - entry_actual_offset: 0, - entry_final_offset: 0, + out: None, + write_strategy: lib::WriteStrategy::default(), dest: None, tmpname: ZBox::from_bytes(b""), hasher, @@ -540,7 +530,7 @@ impl TarballStream { // `&mut TarballStream` is held across any libarchive call (which may // re-enter `archive_read_callback` and access `*this` via the same // provenance). Transient `&mut *this` for `open_destination` / - // `begin_entry` / `write_data_block` / `close_output_file` is sound: + // `begin_entry` / `write_block` / `finish_output_file` is sound: // those do not call into libarchive. unsafe { if (*this).archive.is_none() { @@ -591,17 +581,15 @@ impl TarballStream { Phase::WantData => { let mut offset: i64 = 0; let Some(block) = archive.next(&mut offset) else { - // End of this entry's data. - (*this).close_output_file(); + // End of this entry's data; `offset` is its length. + (*this).finish_output_file(offset)?; (*this).phase = Phase::WantHeader; continue; }; match block.result { lib::Result::Retry if !(*this).archive_holds_reading => return Ok(()), lib::Result::Ok | lib::Result::Warn => { - if let Some(fd) = (*this).out_fd { - (*this).write_data_block(fd, &block)?; - } + (*this).write_block(&block)?; } _ => { (*this).fail_detail = archive.error_string().to_vec(); @@ -724,20 +712,33 @@ impl TarballStream { Ok(()) } + /// The current entry's data ended at `end`. Gives the output file that + /// length, then closes it. + fn finish_output_file(&mut self, end: i64) -> crate::Result<()> { + let Some(mut out) = self.out.take() else { + return Ok(()); + }; + let result = out.finish(end); + out.fd().close(); + result.map_err(|e| e.to_zig_err().into()) + } + + /// Closes the output file of an entry that did not reach its end. The + /// file keeps the bytes that were written and nothing more. fn close_output_file(&mut self) { - if let Some(fd) = self.out_fd { - // Same trailing-hole handling as `Archive.readDataIntoFd`: - // extend the file to cover the furthest block we were asked - // to write even if the pwrite/lseek fallback path left - // `actual_offset` behind. - if self.entry_final_offset > self.entry_actual_offset { - let _ = bun_sys::ftruncate(fd, self.entry_final_offset); - } - fd.close(); - self.out_fd = None; + if let Some(out) = self.out.take() { + out.fd().close(); } } + fn write_block(&mut self, block: &lib::Block<'_>) -> crate::Result<()> { + let Some(out) = self.out.as_mut() else { + return Ok(()); + }; + out.write(&mut self.write_strategy, block.offset, block.bytes) + .map_err(|e| e.to_zig_err().into()) + } + /// Process one entry header returned by `read_next_header`. Opens the /// output file (or creates the directory/symlink) and transitions to /// `WantData` so the next `step()` iteration starts pulling its body. @@ -782,7 +783,7 @@ impl TarballStream { // npm tarballs only contain files; matching the libarchive path // in Archiver.extractToDir we skip everything else. self.phase = Phase::WantData; - self.out_fd = None; + self.out = None; return Ok(()); } @@ -794,7 +795,7 @@ impl TarballStream { .filter(|s| !s.is_empty()); if tokenizer.next().is_none() { self.phase = Phase::WantData; - self.out_fd = None; + self.out = None; return Ok(()); } // tokenizeScalar.rest() — need byte offset of remainder, not just @@ -812,7 +813,7 @@ impl TarballStream { bun_core::fmt::fmt_os_path(rest, Default::default()), ); self.phase = Phase::WantData; - self.out_fd = None; + self.out = None; return Ok(()); } let normalized = @@ -824,7 +825,7 @@ impl TarballStream { unsafe { OSPathSliceZ::from_raw_mut(norm_buf.as_mut_ptr(), norm_len) }; if path.is_empty() || (path.len() == 1 && path[0] == ('.' as OSPathChar)) { self.phase = Phase::WantData; - self.out_fd = None; + self.out = None; return Ok(()); } // `normalize_buf_t` collapses interior `..` but leaves a leading `..` @@ -838,14 +839,14 @@ impl TarballStream { && (path.len() == 2 || path[2] == bun_paths::SEP as OSPathChar) { self.phase = Phase::WantData; - self.out_fd = None; + self.out = None; return Ok(()); } #[cfg(windows)] { if bun_paths::is_absolute_windows_wtf16(&path[..]) { self.phase = Phase::WantData; - self.out_fd = None; + self.out = None; return Ok(()); } if self.npm_mode { @@ -863,7 +864,7 @@ impl TarballStream { FileKind::Directory => { make_directory(entry, dest, path, path_slice); self.phase = Phase::WantData; - self.out_fd = None; + self.out = None; } FileKind::SymLink => { #[cfg(unix)] @@ -875,7 +876,7 @@ impl TarballStream { )); } self.phase = Phase::WantData; - self.out_fd = None; + self.out = None; } FileKind::File => { #[cfg(windows)] @@ -886,102 +887,17 @@ impl TarballStream { let mode: Mode = Mode::try_from((entry.perm() & 0o777) | 0o666).expect("int cast"); let fd = open_output_file(dest, path, path_slice, mode)?; self.entry_count += 1; - - #[cfg(any(target_os = "linux", target_os = "android"))] - { - // The header's size field is attacker-controlled; cap so a - // size lie can't fallocate more than one entry's worth of - // real disk before the truncated body is detected. The - // buffered path bounds this by the decompressed tar length; - // here the stream is incomplete, so use a fixed ceiling. - const PREALLOCATE_CEILING: i64 = 64 * 1024 * 1024; - let size = entry.size().clamp(0, PREALLOCATE_CEILING); - if size > 1_000_000 { - let _ = bun_sys::preallocate_file(fd.native(), 0, size); - } - } - - self.out_fd = Some(fd); - self.entry_actual_offset = 0; - self.entry_final_offset = 0; + self.out = Some(lib::EntryWriter::new(fd)); self.phase = Phase::WantData; } _ => { self.phase = Phase::WantData; - self.out_fd = None; + self.out = None; } } Ok(()) } - /// Write one data block from `archive_read_data_block`. Mirrors the - /// sparse/pwrite handling in `Archive.readDataIntoFd` but operates on a - /// single block so it can be interleaved with ARCHIVE_RETRY yields. - /// `entry_actual_offset` / `entry_final_offset` persist across calls so - /// `close_output_file` can perform the same trailing `ftruncate` the - /// buffered path does after its block loop. - fn write_data_block(&mut self, fd: Fd, block: &lib::Block) -> crate::Result<()> { - let file = bun_sys::File::borrow(&fd); - let data = block.bytes; - if data.is_empty() { - return Ok(()); - } - - self.entry_final_offset = self - .entry_final_offset - .max(block.offset + i64::try_from(data.len()).expect("int cast")); - - #[cfg(unix)] - { - if self.use_pwrite { - match file.pwrite_all(data, block.offset) { - Ok(_) => { - self.entry_actual_offset = self - .entry_actual_offset - .max(block.offset + i64::try_from(data.len()).expect("int cast")); - return Ok(()); - } - Err(_) => self.use_pwrite = false, - } - } - } - - 'seek: { - if block.offset == self.entry_actual_offset { - break 'seek; - } - if self.use_lseek { - match file.seek_to(u64::try_from(block.offset).expect("int cast")) { - Ok(_) => { - self.entry_actual_offset = block.offset; - break 'seek; - } - Err(_) => self.use_lseek = false, - } - } - if block.offset > self.entry_actual_offset { - let zero_count: usize = - usize::try_from(block.offset - self.entry_actual_offset).expect("int cast"); - match lib::Archive::write_zeros_to_file(file, zero_count) { - lib::Result::Ok => { - self.entry_actual_offset = block.offset; - } - _ => return Err(crate::Error::Fail), - } - } else { - return Err(crate::Error::Fail); - } - } - - match file.write_all(data) { - Ok(_) => { - self.entry_actual_offset += i64::try_from(data.len()).expect("int cast"); - Ok(()) - } - Err(e) => Err(e.to_zig_err().into()), - } - } - /// # Safety /// `this` must be the live pointer returned by `init()`. Frees `*this` /// — caller must not touch it after return. Takes a raw pointer (not @@ -1266,8 +1182,8 @@ impl TarballStream { impl Drop for TarballStream { fn drop(&mut self) { - if let Some(fd) = self.out_fd { - fd.close(); + if let Some(out) = self.out.take() { + out.fd().close(); } if let Some(d) = self.dest { d.close(); diff --git a/src/libarchive/lib.rs b/src/libarchive/lib.rs index 287f345df3ee..9b468d55eed9 100644 --- a/src/libarchive/lib.rs +++ b/src/libarchive/lib.rs @@ -157,6 +157,127 @@ pub mod lib { pub result: Result, } + /// Which calls still work for placing a block in an output file. One per + /// extraction: a call that failed is not tried again for later entries. + pub struct WriteStrategy { + pub pwrite: bool, + pub lseek: bool, + } + + impl Default for WriteStrategy { + fn default() -> Self { + Self { + pwrite: cfg!(unix), + lseek: true, + } + } + } + + /// Writes the data of one archive entry to its output file. + /// + /// The entry header says how long the file is, but nothing proves the + /// archive holds that many bytes, and for a sparse entry it never does. So + /// no call here is sized from the header. A block goes to the offset + /// libarchive gives it, which leaves a hole where the entry has one, and + /// the file gets its length in [`EntryWriter::finish`], from the offset + /// libarchive returns once the entry's data has been read to the end. + pub struct EntryWriter { + fd: Fd, + /// One past the last byte written. + end: i64, + /// The file position, which only `write` and `lseek` move. + cursor: i64, + } + + impl EntryWriter { + pub fn new(fd: Fd) -> Self { + Self { + fd, + end: 0, + cursor: 0, + } + } + + #[inline] + pub fn fd(&self) -> Fd { + self.fd + } + + pub fn write( + &mut self, + strategy: &mut WriteStrategy, + offset: i64, + data: &[u8], + ) -> bun_sys::Maybe<()> { + if data.is_empty() { + return Ok(()); + } + let file = bun_sys::File::borrow(&self.fd); + + #[cfg(unix)] + if strategy.pwrite { + match file.pwrite_all(data, offset) { + Ok(()) => { + self.end = self.end.max(offset + data.len() as i64); + return Ok(()); + } + Err(_) => { + strategy.pwrite = false; + bun_core::debug_warn!( + "libarchive: falling back to write() after pwrite() failure", + ); + } + } + } + + if offset != self.cursor { + // Without lseek, zeros can fill a gap ahead. Nothing goes back. + let forward = offset > self.cursor; + if !forward || strategy.lseek { + if let Err(err) = bun_sys::set_file_offset(self.fd, offset as u64) { + strategy.lseek = false; + if !forward { + return Err(err); + } + Self::write_zeros(file, (offset - self.cursor) as usize)?; + } + } else { + Self::write_zeros(file, (offset - self.cursor) as usize)?; + } + self.cursor = offset; + } + + file.write_all(data)?; + self.cursor += data.len() as i64; + self.end = self.end.max(self.cursor); + Ok(()) + } + + fn write_zeros(file: &bun_sys::File, count: usize) -> bun_sys::Maybe<()> { + // Use a runtime memset (vs `[0u8; _]`) to keep .rodata small. + let mut zero_buf = [0u8; 16 * 1024]; + zero_buf.fill(0); + let mut remaining = count; + while remaining > 0 { + let to_write = &zero_buf[..remaining.min(zero_buf.len())]; + file.write_all(to_write)?; + remaining -= to_write.len(); + } + Ok(()) + } + + /// Call when libarchive reports the end of the entry's data. `end` is + /// the offset it returned with `ARCHIVE_EOF`: the length of the file. + /// When that is past the last byte written, the entry ends in a hole. + pub fn finish(&mut self, end: i64) -> bun_sys::Maybe<()> { + if end > self.end { + bun_sys::ftruncate(self.fd, end)?; + self.end = end; + } + Ok(()) + } + } + impl Archive { pub fn read_new() -> *mut Archive { // SAFETY: FFI call with no preconditions. @@ -208,7 +329,36 @@ pub mod lib { unsafe { archive_read_data(self.as_mut_ptr(), buf.as_mut_ptr().cast(), buf.len()) } } - /// `archive_read_data_block` — returns `None` on EOF. + /// Appends the data of the current entry to `out`, at most `size` + /// bytes. Reads in steps so untrusted entry sizes don't drive + /// allocation. `Ok(false)` means libarchive reported a read error. + pub fn read_data_to_vec( + &self, + size: usize, + out: &mut Vec, + ) -> core::result::Result { + let start = out.len(); + while out.len() - start < size { + let to_read = (size - (out.len() - start)).min(64 * 1024); + out.try_reserve(to_read).map_err(|_| bun_core::AllocError)?; + // SAFETY: `archive_read_data` only writes into the slice; the written prefix is committed below. + let dest = unsafe { &mut bun_core::vec::spare_bytes_mut(out)[..to_read] }; + let read = self.read_data(dest); + if read < 0 { + return Ok(false); + } + if read == 0 { + break; + } + // SAFETY: `archive_read_data` returns exactly the byte count it wrote (`<= to_read`). + unsafe { bun_core::vec::commit_spare(out, usize::try_from(read).expect("int cast")) }; + } + Ok(true) + } + + /// `archive_read_data_block`. Returns `None` at the end of the entry's + /// data; `*offset` is then the logical length of the entry, which is + /// past the last block when the entry ends in a hole. pub fn next(&self, offset: &mut i64) -> Option> { let mut buff: *const c_void = core::ptr::null(); let mut size: usize = 0; @@ -236,117 +386,23 @@ pub mod lib { }) } - pub fn write_zeros_to_file(file: &bun_sys::File, count: usize) -> Result { - // Use a runtime memset (vs `[0u8; _]`) to keep .rodata small. - let mut zero_buf = [0u8; 16 * 1024]; - zero_buf.fill(0); - let mut remaining = count; - while remaining > 0 { - let to_write = &zero_buf[..remaining.min(zero_buf.len())]; - if file.write_all(to_write).is_err() { - return Result::Failed; - } - remaining -= to_write.len(); - } - Result::Ok - } - - /// Reads data from the archive and writes it to the given file - /// descriptor. This is a port of libarchive's - /// `archive_read_data_into_fd` with optimizations: - /// - Uses pwrite when possible to avoid needing lseek for sparse file handling - /// - Falls back to lseek + write if pwrite is not available - /// - Falls back to writing zeros if lseek is not available - /// - Truncates the file to the final size to handle trailing sparse holes - pub(crate) fn read_data_into_fd( - &self, - fd: Fd, - can_use_pwrite: &mut bool, - can_use_lseek: &mut bool, - ) -> Result { - #[cfg(windows)] - { - *can_use_pwrite = false; - } - let mut target_offset: i64 = 0; // Updated by archive.next() — where this block should be written - let mut actual_offset: i64 = 0; // Where we've actually written to (for write() path) - let mut final_offset: i64 = 0; // Furthest point the file must extend to - let file = bun_sys::File::borrow(&fd); - - while let Some(block) = self.next(&mut target_offset) { + /// Reads the data of the current entry and writes it to `fd` through + /// an [`EntryWriter`]. + pub fn read_data_into_fd(&self, fd: Fd, strategy: &mut WriteStrategy) -> Result { + let mut writer = EntryWriter::new(fd); + let mut offset: i64 = 0; + while let Some(block) = self.next(&mut offset) { if block.result != Result::Ok { return block.result; } - let data = block.bytes; - - // Track the furthest point we need to write to (for final truncation) - final_offset = final_offset.max(block.offset + data.len() as i64); - - #[cfg(unix)] - { - // Try pwrite first — it handles sparse files without needing lseek - if *can_use_pwrite { - match file.pwrite_all(data, block.offset) { - Err(_) => { - *can_use_pwrite = false; - bun_core::debug_warn!( - "libarchive: falling back to write() after pwrite() failure", - ); - // Fall through to lseek+write path - } - Ok(()) => { - // pwrite doesn't update file position, but track logical position for fallback - actual_offset = actual_offset.max(block.offset + data.len() as i64); - continue; - } - } - } - } - - // Handle mismatch between actual position and target position - if block.offset != actual_offset { - 'seek: { - if *can_use_lseek { - match bun_sys::set_file_offset(fd, block.offset as u64) { - Err(_) => *can_use_lseek = false, - Ok(()) => { - actual_offset = block.offset; - break 'seek; - } - } - } - - // lseek failed or not available - if block.offset > actual_offset { - // Write zeros to fill the gap - let zero_count = (block.offset - actual_offset) as usize; - let zero_result = Self::write_zeros_to_file(file, zero_count); - if zero_result != Result::Ok { - return zero_result; - } - actual_offset = block.offset; - } else { - // Can't seek backward without lseek - return Result::Failed; - } - } - } - - match file.write_all(data) { - Err(_) => return Result::Failed, - Ok(()) => { - actual_offset += data.len() as i64; - } + if writer.write(strategy, block.offset, block.bytes).is_err() { + return Result::Failed; } } - - // Handle trailing sparse hole by truncating file to final size. - // This extends the file to include any trailing zeros without actually writing them. - if final_offset > actual_offset { - let _ = bun_sys::ftruncate(fd, final_offset); + match writer.finish(offset) { + Ok(()) => Result::Ok, + Err(_) => Result::Failed, } - - Result::Ok } // `self` must be a live archive handle from `archive_{read,write}_new()`. @@ -781,27 +837,12 @@ pub mod lib { b"invalid archive entry size", )); }; - // Read data incrementally so untrusted entry sizes don't drive allocation. let mut buf: Vec = Vec::new(); - while buf.len() < size { - let to_read = (size - buf.len()).min(64 * 1024); - buf.try_reserve(to_read).map_err(|_| bun_core::AllocError)?; - // SAFETY: `archive_read_data` only writes into the slice; the written prefix is committed below. - let dest = unsafe { &mut bun_core::vec::spare_bytes_mut(&mut buf)[..to_read] }; - let read = archive.read_data(dest); - if read < 0 { - return Ok(IteratorResult::init_err( - archive.as_mut_ptr(), - b"failed to read archive data", - )); - } - if read == 0 { - break; - } - // SAFETY: `archive_read_data` returns exactly the byte count it wrote (`<= to_read`). - unsafe { - bun_core::vec::commit_spare(&mut buf, usize::try_from(read).expect("int cast")) - }; + if !archive.read_data_to_vec(size, &mut buf)? { + return Ok(IteratorResult::init_err( + archive.as_mut_ptr(), + b"failed to read archive data", + )); } Ok(IteratorResult::init_res(buf.into_boxed_slice())) } @@ -1426,8 +1467,7 @@ impl Archiver { let mut deferred_symlinks: Vec = Vec::new(); let mut normalized_buf = bun_paths::os_path_buffer_pool::get(); - let mut use_pwrite = cfg!(unix); - let mut use_lseek = true; + let mut write_strategy = lib::WriteStrategy::default(); 'loop_: loop { // SAFETY: archive valid for stream lifetime @@ -1781,16 +1821,11 @@ impl Archiver { for plucker_ in ctx_.pluckers.iter_mut() { if plucker_.filename_hash == h { - plucker_.contents.inflate(size)?; - let cap = plucker_.contents.list.capacity(); - plucker_.contents.list.resize(cap, 0); + plucker_.contents.list.clear(); // SAFETY: archive valid - let read = unsafe { - (*archive).read_data( - plucker_.contents.list.as_mut_slice(), - ) - }; - if read < 0 { + let read_ok = unsafe { &*archive } + .read_data_to_vec(size, &mut plucker_.contents.list)?; + if !read_ok { if options.log { // SAFETY: `archive` is the live // `read_new()` handle this @@ -1812,50 +1847,20 @@ impl Archiver { } return Err(crate::Error::Fail); } - plucker_.contents.inflate( - usize::try_from(read).expect("int cast"), - )?; - plucker_.found = read > 0; + plucker_.found = !plucker_.contents.list.is_empty(); plucker_.fd = *file_handle; *plucked_file = true; continue 'loop_; } } } - // archive_read_data_into_fd reads in chunks of 1 MB - // #define MAX_WRITE (1024 * 1024) - #[cfg(any(target_os = "linux", target_os = "android"))] - { - // The header's size field is attacker-controlled; a - // malicious tarball can claim 8 GiB for a 100-byte body - // and fallocate that much real disk before the short body - // is detected. Bound by the input buffer length: when - // `file_buffer` is already the raw tar (pre-decompressed) - // this is exact; when it is the compressed gzip stream - // (libarchive's filter gunzips on the fly) it - // under-preallocates, which is acceptable since - // preallocation is best-effort. Either way the cap is at - // most what the caller actually supplied. - let prealloc = size.min(file_buffer.len()); - if prealloc > 1_000_000 { - let _ = bun_sys::preallocate_file( - file_handle.native(), - 0, - i64::try_from(prealloc).expect("int cast"), - ); - } - } - let mut retries_remaining: u8 = 5; 'possibly_retry: while retries_remaining != 0 { // SAFETY: archive valid match unsafe { - (*archive).read_data_into_fd( - *file_handle, - &mut use_pwrite, - &mut use_lseek, - ) + (*archive) + .read_data_into_fd(*file_handle, &mut write_strategy) } { lib::Result::Eof => break 'loop_, lib::Result::Ok => break 'possibly_retry, diff --git a/src/runtime/api/Archive.rs b/src/runtime/api/Archive.rs index 7bdf46f1fe52..a448415d8287 100644 --- a/src/runtime/api/Archive.rs +++ b/src/runtime/api/Archive.rs @@ -1325,9 +1325,7 @@ fn extract_to_disk_filtered( let mut count: u32 = 0; let mut entry: *mut lib::Entry = core::ptr::null_mut(); - let mut stack_buf = bun_core::vec::UninitBuf::<{ 64 * 1024 }>::uninit(); - // SAFETY: `archive_read_data` is the only writer of `buf`; each chunk reads back only `buf[..bytes_read]`. - let buf = unsafe { stack_buf.as_bytes_mut() }; + let mut write_strategy = lib::WriteStrategy::default(); while archive.read_next_header(&mut entry).succeeded() { let entry_ref = lib::Entry::opaque_ref(entry); @@ -1413,40 +1411,8 @@ fn extract_to_disk_filtered( Err(_) => continue, }; - let mut write_success = true; - if size > 0 { - // Read archive data and write to file - let mut remaining = size; - while remaining > 0 { - let to_read = remaining.min(buf.len()); - let read = archive.read_data(&mut buf[..to_read]); - if read <= 0 { - write_success = false; - break; - } - let bytes_read: usize = usize::try_from(read).expect("int cast"); - // Write all bytes, handling partial writes - let mut written: usize = 0; - while written < bytes_read { - let w = match bun_sys::write(file_fd, &buf[written..bytes_read]) { - Ok(w) => w, - Err(_) => { - write_success = false; - break; - } - }; - if w == 0 { - write_success = false; - break; - } - written += w; - } - if !write_success { - break; - } - remaining -= bytes_read; - } - } + let write_success = size == 0 + || archive.read_data_into_fd(file_fd, &mut write_strategy) == lib::Result::Ok; let _ = file_fd.close(); if write_success { diff --git a/test/cli/install/bun-create.test.ts b/test/cli/install/bun-create.test.ts index 8132f66e0245..8acdf7da09be 100644 --- a/test/cli/install/bun-create.test.ts +++ b/test/cli/install/bun-create.test.ts @@ -282,6 +282,60 @@ it("reports an error and exits when the template's package.json entry body is tr expect(exitCode).toBe(1); }); +it("does not size the package.json buffer from the size in the template tarball's header", async () => { + // The header declares 2 GiB. The archive holds 26 bytes. + const declared = 2 * 1024 * 1024 * 1024; + const body = Buffer.from('{"name":"lying-template"}\n'); + const header = Buffer.alloc(512); + header.write("pkg/package.json"); + header.write("0000644", 100); + header.write("0000000", 108); + header.write("0000000", 116); + header.write(declared.toString(8).padStart(11, "0"), 124); + header.write("00000000000", 136); + header.write(" ", 148); + header.write("0", 156); + header.write("ustar\0", 257); + header.write("00", 263); + let sum = 0; + for (const b of header) sum += b; + header.write(sum.toString(8).padStart(6, "0") + "\0 ", 148); + const gz = gzipSync(Buffer.concat([header, body, Buffer.alloc(512 - body.length), Buffer.alloc(1024)])); + + using server = Bun.serve({ + tls, + port: 0, + fetch() { + return new Response(gz, { headers: { "content-type": "application/x-gzip" } }); + }, + }); + + // The peak memory of a child is never reported below the peak of the + // process that spawned it. A child that does nothing gives that floor. + await using idle = spawn({ cmd: [bunExe(), "--version"], env, stdout: "ignore", stderr: "ignore" }); + await idle.exited; + const floor = idle.resourceUsage()!.maxRSS; + + await using proc = spawn({ + cmd: [bunExe(), "create", "github.com/owner/lying-template", "dest", "--force", "--no-install", "--no-git"], + cwd: x_dir, + stdout: "pipe", + stderr: "pipe", + env: { + ...env, + NODE_TLS_REJECT_UNAUTHORIZED: "0", + GITHUB_API_DOMAIN: `${server.hostname}:${server.port}`, + }, + }); + + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(err).toContain("Unexpected"); + expect(out).not.toContain("Success!"); + expect(proc.signalCode).toBeNull(); + expect(proc.resourceUsage()!.maxRSS - floor).toBeLessThan(declared / 2); + expect(exitCode).toBe(1); +}); + // GitHandler::wait() used Futex::wait(.., Some(1000)) (1us timeout) in a loop, // issuing ~18k futex syscalls/sec while the git thread ran. POSIX-only: stub // `git` is a shell script and ru_nvcsw is always 0 on Windows. diff --git a/test/cli/install/bun-install-streaming-extract.test.ts b/test/cli/install/bun-install-streaming-extract.test.ts index cc2962b48ce7..05565dda0c8b 100644 --- a/test/cli/install/bun-install-streaming-extract.test.ts +++ b/test/cli/install/bun-install-streaming-extract.test.ts @@ -6,7 +6,7 @@ // the buffered extractor would produce. import { describe, expect, setDefaultTimeout, test } from "bun:test"; -import { bunEnv, bunExe, readdirSorted, tempDir } from "harness"; +import { bunEnv, bunExe, isLinux, readdirSorted, tempDir } from "harness"; import { createHash } from "node:crypto"; import { createWriteStream, existsSync, mkdirSync, readdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; import { createServer, type Server } from "node:http"; @@ -1038,27 +1038,14 @@ test.concurrent.each([ }); // ------------------------------------------------------------------- -// Buffered extract: a tarball whose ustar header declares a size far -// larger than the body that follows must fail to extract without -// leaving its temporary extraction directory behind, and without -// allocating the declared size on disk. The declared size is -// attacker-controlled and unbounded relative to the input. +// Buffered extract: the archive is extracted into a temporary directory +// that is renamed into the cache at the end. A failed extraction must +// remove that directory, or each attempt leaves one more behind. // ------------------------------------------------------------------- -describe("buffered extract: malformed tarball cleanup", () => { - function treeSize(root: string): number { - let total = 0; - for (const entry of readdirSync(root, { withFileTypes: true })) { - const full = join(root, entry.name); - if (entry.isDirectory()) total += treeSize(full); - else if (entry.isFile()) total += statSync(full).size; - } - return total; - } - - function leakedExtractionDirs(root: string): string[] { - // Temp extraction dirs are `.-.` directly under $TMPDIR. - return readdirSync(root, { withFileTypes: true }) - .filter(d => d.isDirectory() && d.name.startsWith(".")) +describe.concurrent("buffered extract: failed extraction", () => { + function leftInTmp(tmp: string): string[] { + return readdirSync(tmp, { withFileTypes: true }) + .filter(d => d.isDirectory()) .map(d => d.name); } @@ -1081,73 +1068,303 @@ describe("buffered extract: malformed tarball cleanup", () => { stdout: "pipe", stderr: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - return { stdout, stderr, exitCode, tmp, cache }; + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + return { stderr, exitCode, tmp }; } - test("tar header size far exceeding body fails cleanly without leaking temp disk", async () => { - // A valid package.json entry followed by a file entry whose header - // claims 16 MiB but whose body is only 100 bytes; libarchive reports - // a truncated archive once it runs out of input. 16 MiB is well above - // the 1 MB preallocation threshold, so the unfixed build fallocated - // the full declared size on Linux before failing. - const DECLARED = 16 * 1024 * 1024; + test("a header that declares more than the archive holds leaves no temp directory", async () => { + // The header declares 16 MiB. The archive ends 100 bytes into the body. const pj = Buffer.from(JSON.stringify({ name: "pkg", version: "1.0.0" })); const body = Buffer.alloc(100, 0x78); - const tar = Buffer.concat([ - tarHeader("package/package.json", pj.length, "0"), - pj, - pad512(pj.length), - tarHeader("package/big.bin", DECLARED, "0"), - body, - pad512(body.length), - Buffer.alloc(1024, 0), - ]); - const tgz = gzipSync(tar); - // The tarball itself is tiny; the damage is in the declared size. - expect(tgz.length).toBeLessThan(1024); + const tgz = gzipSync( + Buffer.concat([ + tarHeader("package/package.json", pj.length, "0"), + pj, + pad512(pj.length), + tarHeader("package/big.bin", 16 * 1024 * 1024, "0"), + body, + pad512(body.length), + Buffer.alloc(1024, 0), + ]), + ); using dir = tempDir("tar-size-lie", { - "package.json": JSON.stringify({ - name: "app", - version: "1.0.0", - dependencies: { pkg: "file:./pkg.tgz" }, - }), + "package.json": JSON.stringify({ name: "app", version: "1.0.0", dependencies: { pkg: "file:./pkg.tgz" } }), }); writeFileSync(join(String(dir), "pkg.tgz"), tgz); - // Run twice to confirm the leak does not accumulate per attempt. - let lastStderr = ""; - for (let i = 0; i < 2; i++) { - const { stderr, exitCode, tmp, cache } = await runInstallIsolated(String(dir)); - lastStderr = stderr; - expect(stderr).toContain("extracting tarball"); - expect(exitCode).not.toBe(0); - // The failed extraction's temp directory must be gone. - expect({ leaked: leakedExtractionDirs(tmp) }).toEqual({ leaked: [] }); - // Nothing close to the declared size was left anywhere under the - // test-isolated tmp or cache directories. - expect(treeSize(tmp) + treeSize(cache)).toBeLessThan(1024 * 1024); + // Twice: a leak would grow by one directory per attempt. + for (let attempt = 0; attempt < 2; attempt++) { + const { stderr, exitCode, tmp } = await runInstallIsolated(String(dir)); + expect(stderr).toContain("Fail extracting tarball from pkg"); + expect(leftInTmp(tmp)).toEqual([]); + expect(exitCode).toBe(1); } expect(existsSync(join(String(dir), "node_modules", "pkg"))).toBe(false); - expect(lastStderr).not.toBe(""); }); - test("a tarball that fails to decompress does not leak its temp directory", async () => { - // Not a gzip stream at all: the buffered extractor creates its temp - // directory, then libarchive fails to open the input as an archive. + test("a tarball that is not gzip leaves no temp directory", async () => { using dir = tempDir("tar-bad-gzip", { + "package.json": JSON.stringify({ name: "app", version: "1.0.0", dependencies: { pkg: "file:./pkg.tgz" } }), + }); + writeFileSync(join(String(dir), "pkg.tgz"), Buffer.from("this is not a gzip stream")); + + const { stderr, exitCode, tmp } = await runInstallIsolated(String(dir)); + expect(stderr).toContain("Fail extracting tarball from pkg"); + expect(leftInTmp(tmp)).toEqual([]); + expect(exitCode).toBe(1); + }); +}); + +// ------------------------------------------------------------------- +// Sparse members, as `tar --sparse` writes them. Such a member stores +// only its data chunks and a map of where they go; `realSize` is the +// length of the extracted file, so a member whose last chunk ends +// before `realSize` ends in a hole. The members are built here by hand +// because CI has no GNU tar on every platform. +// +// Two things must hold for both extractors. The file has the bytes the +// map describes, at its full length: the length comes from the end of +// the entry's data, not from a block that was written. And the holes +// take no disk: no call sizes the file from the header, which for a +// sparse member names far more bytes than the archive holds. +// ------------------------------------------------------------------- +type SparseChunk = { offset: number; data: Buffer }; +type SparseMember = { name: string; size: number; chunks: SparseChunk[] }; + +function sparseMap(realSize: number, chunks: SparseChunk[]): [number, number][] { + const map = chunks.map(c => [c.offset, c.data.length] as [number, number]); + const end = chunks.length ? chunks.at(-1)!.offset + chunks.at(-1)!.data.length : 0; + // GNU tar closes the map with an empty entry at the real size. + if (end < realSize) map.push([realSize, 0]); + return map; +} + +// Old GNU format, the `tar --sparse` default: typeflag 'S', magic "ustar \0", +// four map entries at offset 386, the real size at 483. The size field counts +// the stored bytes only. +function oldGnuSparseMember(name: string, realSize: number, chunks: SparseChunk[]): Buffer[] { + const map = sparseMap(realSize, chunks); + if (map.length > 4) throw new Error("the old GNU header holds four sparse entries"); + const body = Buffer.concat(chunks.map(c => c.data)); + const buf = Buffer.alloc(512, 0); + buf.write(name, 0, 100, "utf8"); + buf.write(octal(0o644, 8), 100); + buf.write(octal(0, 8), 108); + buf.write(octal(0, 8), 116); + buf.write(octal(body.length, 12), 124); + buf.write(octal(0, 12), 136); + buf.fill(" ", 148, 156); + buf.write("S", 156); + buf.write("ustar \0", 257, "latin1"); + map.forEach(([offset, length], i) => { + buf.write(octal(offset, 12), 386 + i * 24); + buf.write(octal(length, 12), 398 + i * 24); + }); + buf.write(octal(realSize, 12), 483); + let sum = 0; + for (let i = 0; i < 512; i++) sum += buf[i]; + buf.write(octal(sum, 8), 148); + return [buf, body, pad512(body.length)]; +} + +// PAX format 1.0, what `tar --sparse --format=posix` writes: an 'x' header +// carries the name and the real size, and the data area starts with the map as +// decimal lines, NUL-padded to a block, followed by the chunks. +function paxSparseMember(name: string, realSize: number, chunks: SparseChunk[]): Buffer[] { + const record = (key: string, value: string | number) => { + let len = 0; + let rec: string; + do { + rec = `${len} ${key}=${value}\n`; + len = Buffer.byteLength(rec); + } while (rec !== `${len} ${key}=${value}\n`); + return rec; + }; + const pax = Buffer.from( + record("GNU.sparse.major", 1) + + record("GNU.sparse.minor", 0) + + record("GNU.sparse.name", name) + + record("GNU.sparse.realsize", realSize), + ); + const map = sparseMap(realSize, chunks); + const mapText = Buffer.from(`${map.length}\n` + map.map(([offset, length]) => `${offset}\n${length}\n`).join("")); + const body = Buffer.concat([mapText, pad512(mapText.length), ...chunks.map(c => c.data)]); + const slash = name.lastIndexOf("/"); + return [ + tarHeader("PaxHeaders.0/sparse", pax.length, "x"), + pax, + pad512(pax.length), + tarHeader(`${name.slice(0, slash)}/GNUSparseFile.0/${name.slice(slash + 1)}`, body.length, "0"), + body, + pad512(body.length), + ]; +} + +// A package of sparse members: both formats, each given size, each layout. +function sparsePackage(sizes: number[], onlyLayouts?: string[]) { + const layouts = (size: number): Record => { + const lastBlock = Math.floor((size - 1) / 512) * 512; + return { + "data-hole": [{ offset: 0, data: Buffer.alloc(512, 0x41) }], + "hole-data": [{ offset: lastBlock, data: Buffer.alloc(size - lastBlock, 0x42) }], + "data-hole-data-hole": [ + { offset: 0, data: Buffer.alloc(512, 0x43) }, + { offset: Math.floor(size / 2 / 512) * 512, data: Buffer.alloc(1024, 0x44) }, + ], + "data-hole-data": [ + { offset: 0, data: Buffer.alloc(512, 0x45) }, + { offset: lastBlock, data: Buffer.alloc(size - lastBlock, 0x46) }, + ], + "hole": [], + }; + }; + const pkgJson = Buffer.from(JSON.stringify({ name: "sparse-pkg", version: "1.0.0" })); + const blocks: Buffer[] = [tarHeader("package/package.json", pkgJson.length, "0"), pkgJson, pad512(pkgJson.length)]; + const members: SparseMember[] = []; + let dataBytes = 0; + for (const [format, build] of [ + ["gnu", oldGnuSparseMember], + ["pax", paxSparseMember], + ] as const) { + for (const size of sizes) { + for (const [layout, chunks] of Object.entries(layouts(size))) { + if (onlyLayouts && !onlyLayouts.includes(layout)) continue; + const name = `${format}-${layout}-${size}.bin`; + blocks.push(...build(`package/${name}`, size, chunks)); + members.push({ name, size, chunks }); + dataBytes += chunks.reduce((n, c) => n + c.data.length, 0); + } + } + } + blocks.push(Buffer.alloc(1024, 0)); + const tgz = gzipSync(Buffer.concat(blocks)); + return { tgz, members, dataBytes, integrity: "sha512-" + createHash("sha512").update(tgz).digest("base64") }; +} + +// Compares each extracted member with what its map describes. Returns the +// members that differ and the disk space the members take. +function checkSparseMembers(root: string, members: SparseMember[]) { + const wrong: { name: string; length: number }[] = []; + let allocated = 0; + for (const { name, size, chunks } of members) { + const expected = Buffer.alloc(size, 0); + for (const c of chunks) c.data.copy(expected, c.offset); + const got = readFileSync(join(root, name)); + if (!got.equals(expected)) wrong.push({ name, length: got.length }); + allocated += statSync(join(root, name)).blocks * 512; + } + return { wrong, allocated }; +} + +describe.concurrent("sparse tar members", () => { + // 300000 is below the size from which the extractors used to preallocate + // the output file (1 MB), 3000000 is above it. + const pkg = sparsePackage([300_000, 3_000_000]); + + test("buffered extract writes each member whole and leaves its holes unallocated", async () => { + using dir = tempDir("sparse-buffered", { "package.json": JSON.stringify({ name: "app", version: "1.0.0", - dependencies: { pkg: "file:./pkg.tgz" }, + dependencies: { "sparse-pkg": "file:./pkg.tgz" }, }), }); - writeFileSync(join(String(dir), "pkg.tgz"), Buffer.from("this is not a gzip stream")); + writeFileSync(join(String(dir), "pkg.tgz"), pkg.tgz); - const { stderr, exitCode, tmp } = await runInstallIsolated(String(dir)); - expect(stderr).toContain("error:"); - expect({ leaked: leakedExtractionDirs(tmp) }).toEqual({ leaked: [] }); - expect(exitCode).not.toBe(0); + const { stderr, exitCode } = await runInstall(String(dir)); + expect(stderr).not.toContain("error:"); + expect(stderr).not.toContain("Streamed "); + + const { wrong, allocated } = checkSparseMembers(join(String(dir), "node_modules", "sparse-pkg"), pkg.members); + expect(wrong).toEqual([]); + // 33 MB of file for about 100 KB of data. Linux is the platform where + // every CI filesystem reports a hole as unallocated. + if (isLinux) expect(allocated).toBeLessThan(pkg.dataBytes + 1024 * 1024); + expect(exitCode).toBe(0); + }); + + test.each([ + ["up to 3 MB", () => pkg], + // Longer than any cap a preallocation could have. One layout, because each + // member is 65 MiB of real disk on a filesystem without holes. + ["65 MiB", () => sparsePackage([65 * 1024 * 1024], ["data-hole"])], + ] as const)("streaming extract writes each member whole and leaves its holes unallocated (%s)", async (_, make) => { + const { tgz, members, dataBytes, integrity } = make(); + + using dir = tempDir("sparse-streamed", { + "package.json": JSON.stringify({ name: "app", version: "1.0.0", dependencies: { "sparse-pkg": "1.0.0" } }), + }); + const tmp = join(String(dir), "bun-tmp"); + const cache = join(String(dir), "bun-cache"); + mkdirSync(tmp); + mkdirSync(cache); + let exited = false; + + await using server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url); + if (url.pathname === "/sparse-pkg") { + return Response.json({ + name: "sparse-pkg", + "dist-tags": { latest: "1.0.0" }, + versions: { + "1.0.0": { + name: "sparse-pkg", + version: "1.0.0", + dist: { integrity, tarball: `${server.url}sparse-pkg/-/sparse-pkg-1.0.0.tgz` }, + }, + }, + }); + } + if (url.pathname.endsWith("/sparse-pkg-1.0.0.tgz")) { + return new Response( + new ReadableStream({ + type: "direct", + async pull(c) { + const half = tgz.length >> 1; + c.write(tgz.subarray(0, half)); + await c.flush(); + // The streaming extractor takes the tarball only when the body + // arrives in more than one piece. Its first drain creates the + // extraction directory, so the rest waits until that exists. + while (!exited && !readdirSync(tmp).some(name => name.endsWith(".sparse-pkg"))) await Bun.sleep(5); + c.write(tgz.subarray(half)); + await c.flush(); + c.close(); + }, + }), + { headers: { "content-type": "application/octet-stream" } }, + ); + } + return new Response("not found", { status: 404 }); + }, + }); + writeFileSync(join(String(dir), "bunfig.toml"), Bun.TOML.stringify({ install: { registry: String(server.url) } })); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "install", "--verbose", "--linker=hoisted"], + cwd: String(dir), + env: { + ...bunEnv, + BUN_TMPDIR: tmp, + TMPDIR: tmp, + BUN_INSTALL_CACHE_DIR: cache, + // Drain on the first piece, so that the extraction directory appears. + BUN_INSTALL_STREAMING_DRAIN_THRESHOLD: "1", + }, + stdout: "pipe", + stderr: "pipe", + }); + void proc.exited.then(() => (exited = true)); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect(stderr).toContain("Streamed "); + + const { wrong, allocated } = checkSparseMembers(join(String(dir), "node_modules", "sparse-pkg"), members); + expect(wrong).toEqual([]); + if (isLinux) expect(allocated).toBeLessThan(dataBytes + 1024 * 1024); + expect(exitCode).toBe(0); }); }); diff --git a/test/js/bun/archive.test.ts b/test/js/bun/archive.test.ts index 7eef7eaf7420..f84d53cefcd4 100644 --- a/test/js/bun/archive.test.ts +++ b/test/js/bun/archive.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isWindows, tempDir } from "harness"; -import { existsSync, readdirSync, rmSync } from "node:fs"; +import { bunEnv, bunExe, isLinux, isWindows, tempDir } from "harness"; +import { existsSync, readdirSync, readFileSync, rmSync, statSync } from "node:fs"; import { join } from "path"; // Minimal ustar tarball builder (pathnames must be <100 bytes). `name` accepts @@ -86,6 +86,70 @@ function buildPaxTarball(entries: Array<{ name: string; data: Buffer | string }> return new Uint8Array(Buffer.concat(parts)); } +// Sparse members, as `tar --sparse` writes them. Such a member stores only its +// data chunks and a map of where they go; `realSize` is the length of the +// extracted file, so a member whose last chunk ends before `realSize` ends in +// a hole. +type SparseChunk = { offset: number; data: Buffer }; + +function sparseMap(realSize: number, chunks: SparseChunk[]): [number, number][] { + const map = chunks.map(c => [c.offset, c.data.length] as [number, number]); + const end = chunks.length ? chunks.at(-1)!.offset + chunks.at(-1)!.data.length : 0; + // GNU tar closes the map with an empty entry at the real size. + if (end < realSize) map.push([realSize, 0]); + return map; +} + +// Old GNU format, the `tar --sparse` default: typeflag 'S', magic "ustar \0", +// four map entries at offset 386, the real size at 483. The size field counts +// the stored bytes only. +function oldGnuSparseEntry(name: string, realSize: number, chunks: SparseChunk[]): Buffer { + const field = (n: number) => n.toString(8).padStart(11, "0") + "\0"; + const map = sparseMap(realSize, chunks); + if (map.length > 4) throw new Error("the old GNU header holds four sparse entries"); + const body = Buffer.concat(chunks.map(c => c.data)); + const h = ustarHeader(name, body.length, "S"); + h.write("ustar \0", 257, "latin1"); + map.forEach(([offset, length], i) => { + h.write(field(offset), 386 + i * 24); + h.write(field(length), 398 + i * 24); + }); + h.write(field(realSize), 483); + h.write(" ", 148); + let sum = 0; + for (let i = 0; i < 512; i++) sum += h[i]; + h.write(sum.toString(8).padStart(6, "0") + "\0 ", 148); + return Buffer.concat([h, body, Buffer.alloc((512 - (body.length % 512)) % 512)]); +} + +// PAX format 1.0, what `tar --sparse --format=posix` writes: an 'x' header +// carries the name and the real size, and the data area starts with the map as +// decimal lines, NUL-padded to a block, followed by the chunks. +function paxSparseEntry(name: string, realSize: number, chunks: SparseChunk[]): Buffer { + const record = (key: string, value: string | number) => { + const body = ` ${key}=${value}\n`; + const bodyBytes = Buffer.byteLength(body); + let len = bodyBytes + 1; + while (String(len).length + bodyBytes !== len) len++; + return `${len}${body}`; + }; + const pax = Buffer.from( + record("GNU.sparse.major", 1) + + record("GNU.sparse.minor", 0) + + record("GNU.sparse.name", name) + + record("GNU.sparse.realsize", realSize), + ); + const map = sparseMap(realSize, chunks); + const mapText = Buffer.from(`${map.length}\n` + map.map(([offset, length]) => `${offset}\n${length}\n`).join("")); + const mapPad = Buffer.alloc((512 - (mapText.length % 512)) % 512); + return Buffer.concat([ + ustarHeader("PaxHeaders/sparse", pax.length, "x"), + pax, + Buffer.alloc((512 - (pax.length % 512)) % 512), + ustarEntry(`GNUSparseFile.0/${name}`, Buffer.concat([mapText, mapPad, ...chunks.map(c => c.data)])), + ]); +} + describe("Bun.Archive", () => { describe("new Archive()", () => { test("creates archive from object with string values", async () => { @@ -1649,8 +1713,9 @@ describe("Bun.Archive", () => { }); describe("sparse files", () => { - // These test sparse tar files created with GNU tar --sparse - // They exercise the pwrite/lseek/writeZeros code paths in readDataIntoFd + // Files with runs of zeros, archived with GNU tar --sparse. The runs were + // not holes on disk, so tar stored each file whole (typeflag '0'): these + // cover a plain member. "sparse members" below has members with a map. const fixturesDir = join(import.meta.dir, "fixtures", "sparse-tars"); test("extracts sparse file with small hole (< 1 tar block)", async () => { @@ -1746,6 +1811,91 @@ describe("Bun.Archive", () => { }); }); + describe("sparse members", () => { + // Both formats, every layout, below and above the size from which + // extraction used to preallocate the output file (1 MB). + const layouts = (size: number): Record => { + const lastBlock = Math.floor((size - 1) / 512) * 512; + return { + "data-hole": [{ offset: 0, data: Buffer.alloc(512, 0x41) }], + "hole-data": [{ offset: lastBlock, data: Buffer.alloc(size - lastBlock, 0x42) }], + "data-hole-data-hole": [ + { offset: 0, data: Buffer.alloc(512, 0x43) }, + { offset: Math.floor(size / 2 / 512) * 512, data: Buffer.alloc(1024, 0x44) }, + ], + "data-hole-data": [ + { offset: 0, data: Buffer.alloc(512, 0x45) }, + { offset: lastBlock, data: Buffer.alloc(size - lastBlock, 0x46) }, + ], + "hole": [], + }; + }; + const members: { name: string; size: number; chunks: SparseChunk[] }[] = []; + const parts: Buffer[] = []; + let dataBytes = 0; + for (const [format, build] of [ + ["gnu", oldGnuSparseEntry], + ["pax", paxSparseEntry], + ] as const) { + for (const size of [300_000, 3_000_000]) { + for (const [layout, chunks] of Object.entries(layouts(size))) { + const name = `${format}-${layout}-${size}.bin`; + parts.push(build(name, size, chunks)); + members.push({ name, size, chunks }); + dataBytes += chunks.reduce((n, c) => n + c.data.length, 0); + } + } + } + parts.push(Buffer.alloc(1024)); + const bytes = new Uint8Array(Buffer.concat(parts)); + + test.each([ + ["without a glob", undefined], + ["with a glob", { glob: "**" }], + ] as const)("extract() writes each member whole and leaves its holes unallocated (%s)", async (_, options) => { + using dir = tempDir("sparse-members", {}); + expect(await new Bun.Archive(bytes).extract(String(dir), options)).toBe(members.length); + + const wrong: { name: string; length: number }[] = []; + let allocated = 0; + for (const { name, size, chunks } of members) { + const expected = Buffer.alloc(size, 0); + for (const c of chunks) c.data.copy(expected, c.offset); + const got = readFileSync(join(String(dir), name)); + if (!got.equals(expected)) wrong.push({ name, length: got.length }); + allocated += statSync(join(String(dir), name)).blocks * 512; + } + // The length of a member comes from the end of its data, not from the + // last block that was written: a member that ends in a hole is whole. + expect(wrong).toEqual([]); + // 33 MB of file for about 100 KB of data. Linux is the platform where + // every CI filesystem reports a hole as unallocated. + if (isLinux) expect(allocated).toBeLessThan(dataBytes + 1024 * 1024); + }); + + test("extract() does not size a file from a header that declares more than the archive holds", async () => { + // The header declares 16 MiB. The archive ends 100 bytes into the body. + const lying = new Uint8Array( + Buffer.concat([ + ustarHeader("big.bin", 16 * 1024 * 1024), + Buffer.alloc(100, 0x78), + Buffer.alloc(412), + Buffer.alloc(1024), + ]), + ); + using dir = tempDir("sparse-lying-header", {}); + await expect(async () => { + await new Bun.Archive(lying).extract(String(dir)); + }).toThrow(); + + // What stays is the part of the body that was there. + const left = join(String(dir), "big.bin"); + const stat = existsSync(left) ? statSync(left) : { size: 0, blocks: 0 }; + expect(stat.size).toBeLessThan(64 * 1024); + expect(stat.blocks * 512).toBeLessThan(64 * 1024); + }); + }); + describe("extract with glob patterns", () => { test("extracts only files matching glob pattern", async () => { const archive = new Bun.Archive({ From 99ef8d2ed8684fa012c98d41ded596a6dc387bd2 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 08:23:55 +0000 Subject: [PATCH 07/10] [autofix.ci] apply automated fixes --- src/libarchive/lib.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libarchive/lib.rs b/src/libarchive/lib.rs index 9b468d55eed9..469df47f42df 100644 --- a/src/libarchive/lib.rs +++ b/src/libarchive/lib.rs @@ -351,7 +351,9 @@ pub mod lib { break; } // SAFETY: `archive_read_data` returns exactly the byte count it wrote (`<= to_read`). - unsafe { bun_core::vec::commit_spare(out, usize::try_from(read).expect("int cast")) }; + unsafe { + bun_core::vec::commit_spare(out, usize::try_from(read).expect("int cast")) + }; } Ok(true) } @@ -1823,8 +1825,10 @@ impl Archiver { if plucker_.filename_hash == h { plucker_.contents.list.clear(); // SAFETY: archive valid - let read_ok = unsafe { &*archive } - .read_data_to_vec(size, &mut plucker_.contents.list)?; + let read_ok = unsafe { &*archive }.read_data_to_vec( + size, + &mut plucker_.contents.list, + )?; if !read_ok { if options.log { // SAFETY: `archive` is the live From 21138f9e11a34654a3ae7b0510f7600532b294f5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 09:23:35 +0000 Subject: [PATCH 08/10] bun_core: remove MutableString::inflate, which has no caller left The Plucker in Archiver::extract_to_dir was its last caller. Archive::read_data_to_vec is used only inside bun_libarchive, so it is pub(crate). --- src/bun_core/string/MutableString.rs | 8 -------- src/libarchive/lib.rs | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/bun_core/string/MutableString.rs b/src/bun_core/string/MutableString.rs index cb2465ad12d3..ecd14d166b33 100644 --- a/src/bun_core/string/MutableString.rs +++ b/src/bun_core/string/MutableString.rs @@ -260,14 +260,6 @@ impl MutableString { unsafe { self.list.set_len(index) }; } - pub fn inflate(&mut self, amount: usize) -> Result<(), AllocError> { - // Callers always overwrite the inflated region, so the - // zero-fill here is technically redundant — but it lowers to a single - // memset and avoids `clippy::uninit_vec` / a `set_len` over uninit bytes. - self.list.resize(amount, 0); - Ok(()) - } - #[inline] pub fn append_char(&mut self, char: u8) -> Result<(), AllocError> { self.list.push(char); diff --git a/src/libarchive/lib.rs b/src/libarchive/lib.rs index 469df47f42df..7e0ed6c75168 100644 --- a/src/libarchive/lib.rs +++ b/src/libarchive/lib.rs @@ -332,7 +332,7 @@ pub mod lib { /// Appends the data of the current entry to `out`, at most `size` /// bytes. Reads in steps so untrusted entry sizes don't drive /// allocation. `Ok(false)` means libarchive reported a read error. - pub fn read_data_to_vec( + pub(crate) fn read_data_to_vec( &self, size: usize, out: &mut Vec, From 7ca87799e893b87f648778bd6221d8c100ee40af Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 22:29:21 +0000 Subject: [PATCH 09/10] tar extraction: fail an entry that cannot be placed, mark sparse files on Windows EntryWriter wrote zeros up to a block's offset when pwrite and lseek both failed. ext4 cannot seek past 16 TiB, so a sparse map with a chunk above that made the writer fill the disk. A block that neither call can place now fails the entry. WriteStrategy loses its lseek flag. On Windows, NTFS gives a hole real clusters unless the file is marked sparse. EntryWriter now sends FSCTL_SET_SPARSE before a seek or an ftruncate leaves a range that no block fills. bun_sys gets set_sparse() for that. Tests: a chunk at 17 TiB, run under a file size limit. The allocation checks now run on Windows too. The streamed sparse tests split the body at a fixed place in package.json and not at half of a deflate stream. --- src/libarchive/lib.rs | 76 +++++++++---------- src/sys/lib.rs | 25 +++++- src/sys/windows/mod.rs | 15 ++++ .../bun-install-streaming-extract.test.ts | 64 +++++++++++----- test/js/bun/archive.test.ts | 47 ++++++++++-- 5 files changed, 164 insertions(+), 63 deletions(-) diff --git a/src/libarchive/lib.rs b/src/libarchive/lib.rs index 7e0ed6c75168..68e0cfb2ab18 100644 --- a/src/libarchive/lib.rs +++ b/src/libarchive/lib.rs @@ -157,36 +157,37 @@ pub mod lib { pub result: Result, } - /// Which calls still work for placing a block in an output file. One per - /// extraction: a call that failed is not tried again for later entries. + /// Whether `pwrite` still works for placing a block in an output file. One + /// per extraction: once it fails, later entries do not try it again. pub struct WriteStrategy { pub pwrite: bool, - pub lseek: bool, } impl Default for WriteStrategy { fn default() -> Self { - Self { - pwrite: cfg!(unix), - lseek: true, - } + Self { pwrite: cfg!(unix) } } } /// Writes the data of one archive entry to its output file. /// - /// The entry header says how long the file is, but nothing proves the - /// archive holds that many bytes, and for a sparse entry it never does. So - /// no call here is sized from the header. A block goes to the offset - /// libarchive gives it, which leaves a hole where the entry has one, and - /// the file gets its length in [`EntryWriter::finish`], from the offset - /// libarchive returns once the entry's data has been read to the end. + /// The entry header says how long the file is, and a sparse map says where + /// each block goes, but nothing proves the archive holds that many bytes, + /// and for a sparse entry it never does. So no write and no allocation + /// here is sized from either. A block goes to the offset libarchive gives + /// it, with `pwrite` or a seek, which leaves a hole where the entry has + /// one. When neither reaches the offset, the entry fails: zeros are never + /// written to get there. The file gets its length in + /// [`EntryWriter::finish`], from the offset libarchive returns once the + /// entry's data has been read to the end. pub struct EntryWriter { fd: Fd, /// One past the last byte written. end: i64, /// The file position, which only `write` and `lseek` move. cursor: i64, + #[cfg(windows)] + marked_sparse: bool, } impl EntryWriter { @@ -195,6 +196,8 @@ pub mod lib { fd, end: 0, cursor: 0, + #[cfg(windows)] + marked_sparse: false, } } @@ -203,6 +206,19 @@ pub mod lib { self.fd } + /// Call before the file gets a range that no block fills. NTFS gives + /// such a range real clusters unless the file is marked sparse. + #[inline] + fn before_hole(&mut self) { + #[cfg(windows)] + if !self.marked_sparse { + self.marked_sparse = true; + // A volume without sparse files refuses the mark. The hole + // then takes disk, as it does on such a volume anywhere. + let _ = bun_sys::set_sparse(self.fd); + } + } + pub fn write( &mut self, strategy: &mut WriteStrategy, @@ -212,7 +228,8 @@ pub mod lib { if data.is_empty() { return Ok(()); } - let file = bun_sys::File::borrow(&self.fd); + let fd = self.fd; + let file = bun_sys::File::borrow(&fd); #[cfg(unix)] if strategy.pwrite { @@ -229,21 +246,14 @@ pub mod lib { } } } + #[cfg(not(unix))] + let _ = strategy; if offset != self.cursor { - // Without lseek, zeros can fill a gap ahead. Nothing goes back. - let forward = offset > self.cursor; - if !forward || strategy.lseek { - if let Err(err) = bun_sys::set_file_offset(self.fd, offset as u64) { - strategy.lseek = false; - if !forward { - return Err(err); - } - Self::write_zeros(file, (offset - self.cursor) as usize)?; - } - } else { - Self::write_zeros(file, (offset - self.cursor) as usize)?; + if offset > self.end { + self.before_hole(); } + bun_sys::set_file_offset(self.fd, offset as u64)?; self.cursor = offset; } @@ -253,24 +263,12 @@ pub mod lib { Ok(()) } - fn write_zeros(file: &bun_sys::File, count: usize) -> bun_sys::Maybe<()> { - // Use a runtime memset (vs `[0u8; _]`) to keep .rodata small. - let mut zero_buf = [0u8; 16 * 1024]; - zero_buf.fill(0); - let mut remaining = count; - while remaining > 0 { - let to_write = &zero_buf[..remaining.min(zero_buf.len())]; - file.write_all(to_write)?; - remaining -= to_write.len(); - } - Ok(()) - } - /// Call when libarchive reports the end of the entry's data. `end` is /// the offset it returned with `ARCHIVE_EOF`: the length of the file. /// When that is past the last byte written, the entry ends in a hole. pub fn finish(&mut self, end: i64) -> bun_sys::Maybe<()> { if end > self.end { + self.before_hole(); bun_sys::ftruncate(self.fd, end)?; self.end = end; } diff --git a/src/sys/lib.rs b/src/sys/lib.rs index c89037345c8d..c6a7533548ae 100644 --- a/src/sys/lib.rs +++ b/src/sys/lib.rs @@ -1366,7 +1366,6 @@ impl Tag { #[cfg(not(windows))] pub(crate) const fchdir: Tag = Tag(102); pub const fchownat: Tag = Tag(103); - #[cfg(not(windows))] pub(crate) const ioctl: Tag = Tag(104); #[cfg(not(windows))] pub(crate) const getrlimit: Tag = Tag(105); @@ -3850,6 +3849,30 @@ mod windows_impl { } Ok(()) } + /// `FSCTL_SET_SPARSE`. NTFS gives every byte below the end of a file real + /// clusters unless the file has this mark, so set it before a seek or an + /// `ftruncate` leaves a range that nothing writes to. + pub fn set_sparse(fd: Fd) -> Maybe<()> { + let mut returned: w::DWORD = 0; + // SAFETY: FFI; fd is a valid HANDLE, the call passes no buffer, and + // `returned` is valid for the call. + let ok = unsafe { + w::kernel32::DeviceIoControl( + fd.native(), + w::FSCTL_SET_SPARSE, + core::ptr::null_mut(), + 0, + core::ptr::null_mut(), + 0, + &mut returned, + core::ptr::null_mut(), + ) + }; + if ok == w::FALSE { + return Err(Error::from_win32(w::Win32Error::get(), Tag::ioctl).with_fd(fd)); + } + Ok(()) + } // ── kernel32 / ntdll arms ──────────────────────────────────────────── pub fn openat(dir: impl AsFd, path: &ZStr, flags: i32, mode: Mode) -> Maybe { diff --git a/src/sys/windows/mod.rs b/src/sys/windows/mod.rs index d2eac80990ea..9dc244b327ec 100644 --- a/src/sys/windows/mod.rs +++ b/src/sys/windows/mod.rs @@ -78,6 +78,19 @@ pub mod kernel32 { dwFlags: DWORD, ) -> BOOL; + /// `lpOverlapped` may be null for a handle opened without + /// `FILE_FLAG_OVERLAPPED`. + pub fn DeviceIoControl( + hDevice: HANDLE, + dwIoControlCode: DWORD, + lpInBuffer: *mut c_void, + nInBufferSize: DWORD, + lpOutBuffer: *mut c_void, + nOutBufferSize: DWORD, + lpBytesReturned: *mut DWORD, + lpOverlapped: LPOVERLAPPED, + ) -> BOOL; + // ── SRW locks / condition variables (`bun_threading` windows arm) ── pub fn ReleaseSRWLockExclusive(SRWLock: *mut SRWLOCK); pub fn SleepConditionVariableSRW( @@ -131,6 +144,8 @@ pub const ENABLE_VIRTUAL_TERMINAL_PROCESSING: DWORD = 0x0004; pub const MOVEFILE_COPY_ALLOWED: DWORD = 0x2; pub const MOVEFILE_REPLACE_EXISTING: DWORD = 0x1; pub const MOVEFILE_WRITE_THROUGH: DWORD = 0x8; +/// `FSCTL_SET_SPARSE` (winioctl.h). +pub const FSCTL_SET_SPARSE: DWORD = 0x0009_00C4; pub use bun_windows_sys::FILETIME; pub use bun_windows_sys::DUPLICATE_SAME_ACCESS; diff --git a/test/cli/install/bun-install-streaming-extract.test.ts b/test/cli/install/bun-install-streaming-extract.test.ts index 05565dda0c8b..c8222470a646 100644 --- a/test/cli/install/bun-install-streaming-extract.test.ts +++ b/test/cli/install/bun-install-streaming-extract.test.ts @@ -6,12 +6,12 @@ // the buffered extractor would produce. import { describe, expect, setDefaultTimeout, test } from "bun:test"; -import { bunEnv, bunExe, isLinux, readdirSorted, tempDir } from "harness"; +import { bunEnv, bunExe, isLinux, isWindows, readdirSorted, tempDir } from "harness"; import { createHash } from "node:crypto"; import { createWriteStream, existsSync, mkdirSync, readdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; import { createServer, type Server } from "node:http"; import { join } from "node:path"; -import { createGzip, gzipSync } from "node:zlib"; +import { createGzip, deflateRawSync, gzipSync } from "node:zlib"; setDefaultTimeout(1000 * 60 * 5); @@ -1222,7 +1222,6 @@ function sparsePackage(sizes: number[], onlyLayouts?: string[]) { const pkgJson = Buffer.from(JSON.stringify({ name: "sparse-pkg", version: "1.0.0" })); const blocks: Buffer[] = [tarHeader("package/package.json", pkgJson.length, "0"), pkgJson, pad512(pkgJson.length)]; const members: SparseMember[] = []; - let dataBytes = 0; for (const [format, build] of [ ["gnu", oldGnuSparseMember], ["pax", paxSparseMember], @@ -1233,28 +1232,53 @@ function sparsePackage(sizes: number[], onlyLayouts?: string[]) { const name = `${format}-${layout}-${size}.bin`; blocks.push(...build(`package/${name}`, size, chunks)); members.push({ name, size, chunks }); - dataBytes += chunks.reduce((n, c) => n + c.data.length, 0); } } } blocks.push(Buffer.alloc(1024, 0)); - const tgz = gzipSync(Buffer.concat(blocks)); - return { tgz, members, dataBytes, integrity: "sha512-" + createHash("sha512").update(tgz).digest("base64") }; + const tar = Buffer.concat(blocks); + + // The gzip stream starts with a stored block that ends inside the data of + // package.json. A body that is split after `firstPiece` bytes then gives + // the streaming extractor a piece that ends there, and every sparse member + // in the other piece: that extractor cannot resume a read that stops inside + // a sparse map. + const cut = 512 + 16; + const stored = Buffer.alloc(5); + stored.writeUInt16LE(cut, 1); + stored.writeUInt16LE(~cut & 0xffff, 3); + const trailer = Buffer.alloc(8); + trailer.writeUInt32LE(Bun.hash.crc32(tar), 0); + trailer.writeUInt32LE(tar.length, 4); + const head = Buffer.concat([Buffer.from([0x1f, 0x8b, 8, 0, 0, 0, 0, 0, 0, 3]), stored, tar.subarray(0, cut)]); + const tgz = Buffer.concat([head, deflateRawSync(tar.subarray(cut)), trailer]); + return { + tgz, + firstPiece: head.length, + members, + integrity: "sha512-" + createHash("sha512").update(tgz).digest("base64"), + }; } // Compares each extracted member with what its map describes. Returns the -// members that differ and the disk space the members take. +// members that differ, the disk space the members take, and the most they +// can take when no hole takes any. That limit exists where the file system +// has sparse files and reports them: every Linux CI file system, and NTFS, +// which rounds a chunk up to 64 KiB. function checkSparseMembers(root: string, members: SparseMember[]) { const wrong: { name: string; length: number }[] = []; let allocated = 0; + let chunkCount = 0; for (const { name, size, chunks } of members) { const expected = Buffer.alloc(size, 0); for (const c of chunks) c.data.copy(expected, c.offset); const got = readFileSync(join(root, name)); if (!got.equals(expected)) wrong.push({ name, length: got.length }); allocated += statSync(join(root, name)).blocks * 512; + chunkCount += chunks.length; } - return { wrong, allocated }; + const allocatedLimit = isLinux || isWindows ? chunkCount * 64 * 1024 + 1024 * 1024 : Infinity; + return { wrong, allocated, allocatedLimit }; } describe.concurrent("sparse tar members", () => { @@ -1276,11 +1300,13 @@ describe.concurrent("sparse tar members", () => { expect(stderr).not.toContain("error:"); expect(stderr).not.toContain("Streamed "); - const { wrong, allocated } = checkSparseMembers(join(String(dir), "node_modules", "sparse-pkg"), pkg.members); + const { wrong, allocated, allocatedLimit } = checkSparseMembers( + join(String(dir), "node_modules", "sparse-pkg"), + pkg.members, + ); expect(wrong).toEqual([]); - // 33 MB of file for about 100 KB of data. Linux is the platform where - // every CI filesystem reports a hole as unallocated. - if (isLinux) expect(allocated).toBeLessThan(pkg.dataBytes + 1024 * 1024); + // 33 MB of file for 24 chunks of data. + expect(allocated).toBeLessThan(allocatedLimit); expect(exitCode).toBe(0); }); @@ -1290,7 +1316,7 @@ describe.concurrent("sparse tar members", () => { // member is 65 MiB of real disk on a filesystem without holes. ["65 MiB", () => sparsePackage([65 * 1024 * 1024], ["data-hole"])], ] as const)("streaming extract writes each member whole and leaves its holes unallocated (%s)", async (_, make) => { - const { tgz, members, dataBytes, integrity } = make(); + const { tgz, firstPiece, members, integrity } = make(); using dir = tempDir("sparse-streamed", { "package.json": JSON.stringify({ name: "app", version: "1.0.0", dependencies: { "sparse-pkg": "1.0.0" } }), @@ -1323,14 +1349,13 @@ describe.concurrent("sparse tar members", () => { new ReadableStream({ type: "direct", async pull(c) { - const half = tgz.length >> 1; - c.write(tgz.subarray(0, half)); + c.write(tgz.subarray(0, firstPiece)); await c.flush(); // The streaming extractor takes the tarball only when the body // arrives in more than one piece. Its first drain creates the // extraction directory, so the rest waits until that exists. while (!exited && !readdirSync(tmp).some(name => name.endsWith(".sparse-pkg"))) await Bun.sleep(5); - c.write(tgz.subarray(half)); + c.write(tgz.subarray(firstPiece)); await c.flush(); c.close(); }, @@ -1362,9 +1387,12 @@ describe.concurrent("sparse tar members", () => { expect(stderr).not.toContain("error:"); expect(stderr).toContain("Streamed "); - const { wrong, allocated } = checkSparseMembers(join(String(dir), "node_modules", "sparse-pkg"), members); + const { wrong, allocated, allocatedLimit } = checkSparseMembers( + join(String(dir), "node_modules", "sparse-pkg"), + members, + ); expect(wrong).toEqual([]); - if (isLinux) expect(allocated).toBeLessThan(dataBytes + 1024 * 1024); + expect(allocated).toBeLessThan(allocatedLimit); expect(exitCode).toBe(0); }); }); diff --git a/test/js/bun/archive.test.ts b/test/js/bun/archive.test.ts index f84d53cefcd4..cd323a51182f 100644 --- a/test/js/bun/archive.test.ts +++ b/test/js/bun/archive.test.ts @@ -1832,7 +1832,6 @@ describe("Bun.Archive", () => { }; const members: { name: string; size: number; chunks: SparseChunk[] }[] = []; const parts: Buffer[] = []; - let dataBytes = 0; for (const [format, build] of [ ["gnu", oldGnuSparseEntry], ["pax", paxSparseEntry], @@ -1842,7 +1841,6 @@ describe("Bun.Archive", () => { const name = `${format}-${layout}-${size}.bin`; parts.push(build(name, size, chunks)); members.push({ name, size, chunks }); - dataBytes += chunks.reduce((n, c) => n + c.data.length, 0); } } } @@ -1868,9 +1866,11 @@ describe("Bun.Archive", () => { // The length of a member comes from the end of its data, not from the // last block that was written: a member that ends in a hole is whole. expect(wrong).toEqual([]); - // 33 MB of file for about 100 KB of data. Linux is the platform where - // every CI filesystem reports a hole as unallocated. - if (isLinux) expect(allocated).toBeLessThan(dataBytes + 1024 * 1024); + // 33 MB of file for 24 chunks of data. A hole takes no disk where the + // file system has sparse files and reports them: every Linux CI file + // system, and NTFS, which rounds a chunk up to 64 KiB. + const chunkCount = members.reduce((n, m) => n + m.chunks.length, 0); + if (isLinux || isWindows) expect(allocated).toBeLessThan(chunkCount * 64 * 1024 + 1024 * 1024); }); test("extract() does not size a file from a header that declares more than the archive holds", async () => { @@ -1894,6 +1894,43 @@ describe("Bun.Archive", () => { expect(stat.size).toBeLessThan(64 * 1024); expect(stat.blocks * 512).toBeLessThan(64 * 1024); }); + + // A sparse map can put a chunk where the file system cannot seek: ext4 + // stops at 16 TiB. The child runs with a file size limit, so a build that + // writes zeros up to the offset stops at the limit and not at a full disk. + test.skipIf(isWindows)("extract() writes nothing to reach an offset the file system cannot seek to", async () => { + const offset = 17 * 2 ** 40; + using dir = tempDir("sparse-far-offset", { + "far.tar": Buffer.concat([ + paxSparseEntry("far.bin", offset + 512, [{ offset, data: Buffer.alloc(512, 0x41) }]), + Buffer.alloc(1024), + ]), + "extract.mjs": ` + import { existsSync, statSync } from "node:fs"; + const settled = await new Bun.Archive(await Bun.file("far.tar").bytes()).extract(".").then( + () => "resolved", + () => "rejected", + ); + const stat = existsSync("far.bin") ? statSync("far.bin") : { blocks: 0 }; + console.log(JSON.stringify({ settled, allocated: stat.blocks * 512 })); + `, + }); + + await using proc = Bun.spawn({ + // A write past the limit raises SIGXFSZ, which must not end the child. + // Without the limit the child must not run. + cmd: ["sh", "-c", `trap '' XFSZ; ulimit -f 65536 && exec "$@"`, "sh", bunExe(), "extract.mjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "inherit", + }); + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + + // The limit is below the offset, so no file system can place the chunk. + expect(JSON.parse(stdout)).toEqual({ settled: "rejected", allocated: 0 }); + expect(exitCode).toBe(0); + }); }); describe("extract with glob patterns", () => { From 79e7e5d95e5ce88167bae87a77af97bca7bf66fe Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 3 Oct 2026 08:36:56 +0000 Subject: [PATCH 10/10] test: a refused write fails the extraction, and two review nits A full disk, a quota or a file size limit refuses a write in the middle of a file. Two tests put a 6,000,000-byte member under a 4 MiB file size limit. Bun.Archive.extract() must reject and leave only the start of the file. A streamed registry install must fail and leave no package in the cache. The test for a header that declares too much now asserts the ReadError rejection. Two install spawns no longer pipe a stdout that nothing reads. --- .../bun-install-streaming-extract.test.ts | 103 +++++++++++++++++- test/js/bun/archive.test.ts | 38 ++++++- 2 files changed, 136 insertions(+), 5 deletions(-) diff --git a/test/cli/install/bun-install-streaming-extract.test.ts b/test/cli/install/bun-install-streaming-extract.test.ts index c8222470a646..6fe021c0855f 100644 --- a/test/cli/install/bun-install-streaming-extract.test.ts +++ b/test/cli/install/bun-install-streaming-extract.test.ts @@ -7,8 +7,17 @@ import { describe, expect, setDefaultTimeout, test } from "bun:test"; import { bunEnv, bunExe, isLinux, isWindows, readdirSorted, tempDir } from "harness"; -import { createHash } from "node:crypto"; -import { createWriteStream, existsSync, mkdirSync, readdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; +import { createHash, randomBytes } from "node:crypto"; +import { + createWriteStream, + existsSync, + mkdirSync, + readdirSync, + readFileSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; import { createServer, type Server } from "node:http"; import { join } from "node:path"; import { createGzip, deflateRawSync, gzipSync } from "node:zlib"; @@ -1065,7 +1074,7 @@ describe.concurrent("buffered extract: failed extraction", () => { TMP: tmp, BUN_INSTALL_CACHE_DIR: cache, }, - stdout: "pipe", + stdout: "ignore", stderr: "pipe", }); const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); @@ -1379,7 +1388,7 @@ describe.concurrent("sparse tar members", () => { // Drain on the first piece, so that the extraction directory appears. BUN_INSTALL_STREAMING_DRAIN_THRESHOLD: "1", }, - stdout: "pipe", + stdout: "ignore", stderr: "pipe", }); void proc.exited.then(() => (exited = true)); @@ -1396,3 +1405,89 @@ describe.concurrent("sparse tar members", () => { expect(exitCode).toBe(0); }); }); + +// ------------------------------------------------------------------- +// A full disk, a quota or a file size limit refuses a write in the +// middle of a file. The write loop then went on with `write()` at the +// start of the file: the install exited 0 with a file that was cut and +// whose first bytes were those of the refused block, and that package +// stayed in the cache for every later install. +// ------------------------------------------------------------------- +test.skipIf(isWindows)("streaming extract fails the install when the file system refuses a write", async () => { + // 6,000,000 bytes where no file can grow past 4 MiB. Random bytes keep the + // tarball above the size from which a registry tarball is streamed. + const big = randomBytes(6_000_000); + const pkgJson = Buffer.from(JSON.stringify({ name: "pk", version: "1.0.0" })); + const tgz = gzipSync( + Buffer.concat([ + tarHeader("package/package.json", pkgJson.length, "0"), + pkgJson, + pad512(pkgJson.length), + tarHeader("package/big.bin", big.length, "0"), + big, + pad512(big.length), + Buffer.alloc(1024, 0), + ]), + ); + const integrity = "sha512-" + createHash("sha512").update(tgz).digest("base64"); + + using dir = tempDir("refused-write-streamed", { + "package.json": JSON.stringify({ name: "app", version: "1.0.0", dependencies: { pk: "1.0.0" } }), + }); + await using server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url); + if (url.pathname === "/pk") { + return Response.json({ + name: "pk", + "dist-tags": { latest: "1.0.0" }, + versions: { + "1.0.0": { name: "pk", version: "1.0.0", dist: { integrity, tarball: `${server.url}pk/-/pk-1.0.0.tgz` } }, + }, + }); + } + if (url.pathname.endsWith("/pk-1.0.0.tgz")) return new Response(tgz); + return new Response("not found", { status: 404 }); + }, + }); + writeFileSync(join(String(dir), "bunfig.toml"), Bun.TOML.stringify({ install: { registry: String(server.url) } })); + const tmp = join(String(dir), "bun-tmp"); + mkdirSync(tmp); + const env = { ...bunEnv, BUN_TMPDIR: tmp, TMPDIR: tmp, BUN_INSTALL_CACHE_DIR: join(String(dir), "bun-cache") }; + const installed = join(String(dir), "node_modules", "pk", "big.bin"); + + { + await using proc = Bun.spawn({ + // 8192 blocks of 512 bytes. A write past the limit raises SIGXFSZ, which + // must not end the child. Without the limit the child must not run. + cmd: ["sh", "-c", `trap '' XFSZ; ulimit -f 8192 && exec "$@"`, "sh", bunExe(), "install", "--linker=hoisted"], + cwd: String(dir), + env, + stdout: "ignore", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).toContain('EFBIG extracting tarball for "pk"'); + expect(existsSync(installed)).toBe(false); + expect(exitCode).toBe(1); + } + + // Without the limit, the same cache gives the whole file: the failed install + // left no package there. + rmSync(join(String(dir), "node_modules"), { recursive: true, force: true }); + rmSync(join(String(dir), "bun.lock"), { force: true }); + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install", "--verbose", "--linker=hoisted"], + cwd: String(dir), + env, + stdout: "ignore", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).toContain("Streamed "); + expect(readFileSync(installed).equals(big)).toBe(true); + expect(exitCode).toBe(0); + } +}); diff --git a/test/js/bun/archive.test.ts b/test/js/bun/archive.test.ts index cd323a51182f..a7ec234fac36 100644 --- a/test/js/bun/archive.test.ts +++ b/test/js/bun/archive.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, isLinux, isWindows, tempDir } from "harness"; +import { randomBytes } from "node:crypto"; import { existsSync, readdirSync, readFileSync, rmSync, statSync } from "node:fs"; import { join } from "path"; @@ -1886,7 +1887,7 @@ describe("Bun.Archive", () => { using dir = tempDir("sparse-lying-header", {}); await expect(async () => { await new Bun.Archive(lying).extract(String(dir)); - }).toThrow(); + }).toThrow("ReadError"); // What stays is the part of the body that was there. const left = join(String(dir), "big.bin"); @@ -1933,6 +1934,41 @@ describe("Bun.Archive", () => { }); }); + // A full disk, a quota or a file size limit refuses a write in the middle of + // a file. The write loop then went on with `write()` at the start of the + // file: the file was cut, its first bytes were those of the refused block, + // and extract() resolved. + test.skipIf(isWindows)("extract() rejects when the file system refuses a write in the middle of a file", async () => { + // 6,000,000 bytes where no file can grow past 4 MiB. Gzip, so that the + // member arrives in many blocks: the first ones fit. + const data = randomBytes(6_000_000); + using dir = tempDir("archive-refused-write", { + "big.tar.gz": Buffer.from(Bun.gzipSync(Buffer.concat([ustarEntry("big.bin", data), Buffer.alloc(1024)]))), + "extract.mjs": ` + const archive = new Bun.Archive(await Bun.file("big.tar.gz").bytes()); + console.log(await archive.extract("out").then(() => "resolved", e => "rejected " + e.message)); + `, + }); + + await using proc = Bun.spawn({ + // 8192 blocks of 512 bytes. A write past the limit raises SIGXFSZ, which + // must not end the child. Without the limit the child must not run. + cmd: ["sh", "-c", `trap '' XFSZ; ulimit -f 8192 && exec "$@"`, "sh", bunExe(), "extract.mjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "inherit", + }); + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + expect(stdout.trim()).toBe("rejected ReadError"); + + // What stays of the file is its start, byte for byte. + const left = join(String(dir), "out", "big.bin"); + const written = existsSync(left) ? readFileSync(left) : Buffer.alloc(0); + expect(written.equals(data.subarray(0, written.length))).toBe(true); + expect(exitCode).toBe(0); + }); + describe("extract with glob patterns", () => { test("extracts only files matching glob pattern", async () => { const archive = new Bun.Archive({