Skip to content

install: fail the package with ENAMETOOLONG when the isolated linker walks an entry that does not fit the path buffer #37424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 4 commits into
base: main
Choose a base branch
from
Open

install: fail the package with ENAMETOOLONG when the isolated linker walks an entry that does not fit the path buffer #37424

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
87a9b45
install(isolated): fail the package with ENAMETOOLONG when a walked e…
robobun Aug 11, 2026
1b4890a
test: run the deep folder dependency case on Windows too
robobun Aug 11, 2026
e250881
install(isolated): propagate a failed destination create in FileCopie…
robobun Aug 11, 2026
7d99e1d
install(isolated): keep the Windows FileCopier match in place; shorte…
robobun Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/install/PackageManager/patchPackage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1231,8 +1231,8 @@ fn overwrite_package_in_node_modules_folder(

let mut copier: FileCopier = FileCopier::init(
cached_package_folder.fd,
src_path,
dest_subpath,
src_path.into_checked(),
dest_subpath.into_checked(),
ignore_directories,
)?;

Expand Down
155 changes: 76 additions & 79 deletions src/install/isolated_install/FileCopier.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,20 @@
use core::ptr;

use bun_alloc::AllocError;
#[cfg(not(windows))]
use bun_core::{Global, fmt as bun_fmt};
use bun_paths::path_options::{CheckLength, Kind, PathSeparators};
use bun_paths::{self, OSPathChar, OSPathSlice};
use bun_sys::{self as sys, Dir, E, EntryKind, Fd, walker_skippable, walker_skippable::Walker};

// The path-builder types here use the OS path unit: u8 on POSIX,
// u16 on Windows — encoded via `OSPathChar` so `slice()`/`slice_z()` produce
// the platform-native width. The auto separator mode normalizes `/` → `\` on Windows
// during `from`/`append`, which is load-bearing for the Win32 calls below.
// Length-checked for the same reason as the `Hardlinker` paths: the walker's
// entry paths appended on Windows are not bounded by any path buffer.
Comment thread
robobun marked this conversation as resolved.
Outdated
type AbsPathAutoOs =
bun_paths::AbsPath<OSPathChar, { bun_paths::path_options::PathSeparators::AUTO }>;
type PathAutoOs = bun_paths::Path<
OSPathChar,
{ bun_paths::path_options::Kind::ANY },
{ bun_paths::path_options::PathSeparators::AUTO },
>;
bun_paths::AbsPath<OSPathChar, { PathSeparators::AUTO }, { CheckLength::CHECK }>;
type PathAutoOs =
bun_paths::Path<OSPathChar, { Kind::ANY }, { PathSeparators::AUTO }, { CheckLength::CHECK }>;

pub(crate) struct FileCopier {
pub(crate) src_path: AbsPathAutoOs,
Expand Down Expand Up @@ -110,74 +108,85 @@ impl FileCopier {

// A `path.save()` ResetScope would hold `&mut Path` and keep
// `self.src_path` / `self.dest_subpath` exclusively borrowed
// for the rest of the iteration. Capture the saved length and
// for the rest of the iteration. Capture the saved lengths and
// restore via `set_length` after the body instead.
let src_saved_len = self.src_path.len();
let _ = self.src_path.append(entry.path.as_slice());

let dest_saved_len = self.dest_subpath.len();
let _ = self.dest_subpath.append(entry.path.as_slice());

let result: sys::Result<()> = match entry.kind {
EntryKind::Directory => {
// SAFETY: FFI — both `slice_z()` are NUL-terminated WStrs.
if unsafe {
bun_sys::windows::CreateDirectoryExW(
self.src_path.slice_z().as_ptr(),
self.dest_subpath.slice_z().as_ptr(),
ptr::null_mut(),
)
} == 0
{
let _ = bun_sys::make_path::make_path::<u16>(
&dest_dir,
entry.path.as_slice(),
);
}
sys::Result::Ok(())

let result: sys::Result<()> = 'entry: {
if self.src_path.append(entry.path.as_slice()).is_err()
|| self.dest_subpath.append(entry.path.as_slice()).is_err()
{
break 'entry sys::Result::Err(sys::Error::from_code(
E::ENAMETOOLONG,
sys::Tag::copyfile,
));
}
EntryKind::File => {
match bun_sys::copy_file::copy_file(
self.src_path.slice_z(),
self.dest_subpath.slice_z(),
) {
sys::Result::Ok(()) => sys::Result::Ok(()),
sys::Result::Err(first_err) => {
// Retry after creating the parent directory.
// For root-level files (`index.js`,
// `package.json`, `LICENSE`) `dirname` is
// null and there is no missing parent to
// create — `dest_dir` itself was already
// opened above — so the original error is the
// real failure and must propagate. Silently
// continuing here would let a staged
// global-store entry be renamed into place
// with files missing.
match bun_paths::Dirname::dirname::<u16>(entry.path.as_slice()) {
None => sys::Result::Err(first_err),
Some(entry_dirname) => {
let _ = bun_sys::make_path::make_path::<u16>(
&dest_dir,
entry_dirname,
);
bun_sys::copy_file::copy_file(
self.src_path.slice_z(),
self.dest_subpath.slice_z(),
)

match entry.kind {
EntryKind::Directory => {
// SAFETY: FFI — both `slice_z()` are NUL-terminated WStrs.
if unsafe {
bun_sys::windows::CreateDirectoryExW(
self.src_path.slice_z().as_ptr(),
self.dest_subpath.slice_z().as_ptr(),
ptr::null_mut(),
)
} == 0
{
// CreateDirectoryExW also fails when the directory already
// exists; `make_path` treats that as success and reports
// anything else.
Comment thread
robobun marked this conversation as resolved.
Outdated
bun_sys::make_path::make_path::<u16>(
&dest_dir,
entry.path.as_slice(),
)
} else {
sys::Result::Ok(())
}
}
EntryKind::File => {
match bun_sys::copy_file::copy_file(
self.src_path.slice_z(),
self.dest_subpath.slice_z(),
) {
sys::Result::Ok(()) => sys::Result::Ok(()),
sys::Result::Err(first_err) => {
// Retry after creating the parent directory.
// For root-level files (`index.js`,
// `package.json`, `LICENSE`) `dirname` is
// null and there is no missing parent to
// create — `dest_dir` itself was already
// opened above — so the original error is the
// real failure and must propagate. Silently
// continuing here would let a staged
// global-store entry be renamed into place
// with files missing.
Comment thread
robobun marked this conversation as resolved.
Outdated
match bun_paths::Dirname::dirname::<u16>(entry.path.as_slice())
{
None => sys::Result::Err(first_err),
Some(entry_dirname) => {
let _ = bun_sys::make_path::make_path::<u16>(
&dest_dir,
entry_dirname,
);
bun_sys::copy_file::copy_file(
self.src_path.slice_z(),
self.dest_subpath.slice_z(),
)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
}
}
}
_ => unreachable!(),
}
_ => unreachable!(),
};

self.src_path.set_length(src_saved_len);
self.dest_subpath.set_length(dest_saved_len);

if let sys::Result::Err(err) = result {
return sys::Result::Err(err);
}
result?;
}
#[cfg(not(windows))]
{
Expand All @@ -194,7 +203,7 @@ impl FileCopier {

let dest = match dest_dir.create_file_z(entry.path, Default::default()) {
Ok(f) => f,
Err(_) => 'dest: {
Err(_) => {
if let Some(entry_dirname) =
bun_paths::Dirname::dirname::<OSPathChar>(entry.path)
{
Expand All @@ -203,27 +212,15 @@ impl FileCopier {
entry_dirname,
);
}

match dest_dir.create_file_z(entry.path, Default::default()) {
Ok(f) => break 'dest f,
Err(err) => {
bun_core::pretty_errorln!(
"<r><red>{}<r>: copy file {}",
bstr::BStr::new(err.name()),
bun_fmt::fmt_os_path(entry.path, Default::default()),
);
Global::exit(1);
}
}
dest_dir.create_file_z(entry.path, Default::default())?
}
};

#[cfg(unix)]
{
let stat = match bun_sys::fstat(src.handle()) {
sys::Result::Ok(s) => s,
sys::Result::Err(_) => continue,
};
// `dest` has already been created (or truncated) above, so
// skipping this entry would leave an empty file behind.
Comment thread
robobun marked this conversation as resolved.
Outdated
let stat = bun_sys::fstat(src.handle())?;
// SAFETY: fchmod is safe to call with any fd + mode; errors are ignored (`_ =`).
unsafe {
let _ = bun_sys::c::fchmod(dest.handle().native(), stat.st_mode);
Expand Down
61 changes: 42 additions & 19 deletions src/install/isolated_install/Hardlinker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,28 @@ use bun_sys::{self as sys, EntryKind, Fd, FdExt};
// on Windows — encoded here via the `OSPathChar` type alias so the struct's
// `slice()`/`slice_z()` produce the platform-native width without per-field
// `#[cfg]` divergence.
#[cfg(windows)]
use bun_paths::path_options::AssumeOk as _;
//
// The walker's entry paths get appended to these per entry, and the walker
// opens every directory relative to its parent, so an entry path can be longer
// than any path buffer. `CheckLength::CHECK` turns that into `ENAMETOOLONG`
// for the package (what the hoisted linker reports for the same tree) instead
// of an out-of-bounds write in `append`.
Comment thread
robobun marked this conversation as resolved.
Outdated
use bun_paths::path_options::{CheckLength, Kind, PathSeparators};
use bun_paths::{AbsPath, OSPathChar, OSPathSlice, Path};

type OsAbsPath = AbsPath<OSPathChar, { bun_paths::path_options::PathSeparators::AUTO }>;
type OsPath = Path<
OSPathChar,
{ bun_paths::path_options::Kind::ANY },
{ bun_paths::path_options::PathSeparators::AUTO },
>;
type OsAbsPath = AbsPath<OSPathChar, { PathSeparators::AUTO }, { CheckLength::CHECK }>;
type OsPath = Path<OSPathChar, { Kind::ANY }, { PathSeparators::AUTO }, { CheckLength::CHECK }>;

pub(crate) struct Hardlinker {
pub(crate) src: OsAbsPath,
pub(crate) dest: OsPath,
pub(crate) walker: Walker,
}

fn name_too_long() -> sys::Error {
sys::Error::from_code(sys::E::ENAMETOOLONG, sys::Tag::link)
}

impl Hardlinker {
pub(crate) fn init(
folder_dir: Fd,
Expand Down Expand Up @@ -97,25 +102,27 @@ impl Hardlinker {

// A `path.save()` ResetScope would hold `&mut Path` and keep
// `self.src`/`self.dest` exclusively borrowed for the rest of
// the iteration. Capture the saved length directly and restore
// the iteration. Capture the saved lengths directly and restore
// via `set_length` after the body (and before any error return)
// so the truncation happens on every exit.
let src_saved_len = self.src.len();
// `OsAbsPath`/`OsPath` use `CheckLength::ASSUME`, so `append`'s
// `Err(MaxPathExceeded)` arm is statically unreachable -- see
// `path_options::AssumeOk`.
self.src.append(entry.path.as_slice()).assume_ok();

let dest_saved_len = self.dest.len();
self.dest.append(entry.path.as_slice()).assume_ok();

let err: Option<sys::Error> = 'body: {
if self.src.append(entry.path.as_slice()).is_err()
|| self.dest.append(entry.path.as_slice()).is_err()
{
break 'body Some(name_too_long());
}

match entry.kind {
EntryKind::Directory => {
let _ = sys::make_path::make_path::<u16>(
if let sys::Result::Err(mkdir_err) = sys::make_path::make_path::<u16>(
&sys::Dir::cwd(),
self.dest.slice(),
);
) {
break 'body Some(mkdir_err);
}
}
EntryKind::File => {
let mut destfile_path_buf = bun_paths::w_path_buffer_pool::get();
Expand All @@ -136,6 +143,15 @@ impl Hardlinker {
dest_slice,
]
};
// `join_string_buf_w_same` and `add_nt_path_prefix_if_needed`
// write into the pooled buffers unchecked: the parts, a
// separator per part, the `\??\` prefix and the NUL must fit.
Comment thread
robobun marked this conversation as resolved.
Outdated
let needed_len =
dest_parts.iter().map(|part| part.len() + 1).sum::<usize>()
+ bun_paths::windows::NT_OBJECT_PREFIX.len();
if needed_len > destfile_path_buf2.len() {
break 'body Some(name_too_long());
}
let joined = bun_paths::resolve_path::join_string_buf_w_same::<
bun_paths::platform::Windows,
>(
Expand Down Expand Up @@ -246,12 +262,19 @@ impl Hardlinker {
// `len()` and restore via `set_length()` after the body so the
// truncation runs on every exit.
let dest_saved_len = self.dest.len();
let _ = self.dest.append(entry.path.as_bytes()); // OOM/capacity: fire-and-forget

let err: Option<sys::Error> = 'body: {
if self.dest.append(entry.path.as_bytes()).is_err() {
break 'body Some(name_too_long());
}

match entry.kind {
EntryKind::Directory => {
let _ = Fd::cwd().make_path(self.dest.slice());
if let sys::Result::Err(mkdir_err) =
Fd::cwd().make_path(self.dest.slice())
{
break 'body Some(mkdir_err);
}
}
EntryKind::File => {
match sys::linkat(
Expand Down
16 changes: 8 additions & 8 deletions src/install/isolated_install/Installer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -917,8 +917,8 @@ impl Task {

let mut hardlinker = Hardlinker::init(
folder_dir,
src,
dest,
src.into_checked(),
dest.into_checked(),
&[bun_paths::os_path_literal!("node_modules")],
)?;

Expand Down Expand Up @@ -1029,8 +1029,8 @@ impl Task {

let mut file_copier = FileCopier::init(
folder_dir,
src_path.into_sep::<{ PathSeparators::AUTO }>(),
dest.into_sep::<{ PathSeparators::AUTO }>(),
src_path.into_checked(),
dest.into_checked(),
&[bun_paths::os_path_literal!("node_modules")],
)?;

Expand Down Expand Up @@ -1338,8 +1338,8 @@ impl Task {

let mut hardlinker = Hardlinker::init(
cached_package_dir.unwrap(),
src,
dest_subpath,
src.into_checked(),
dest_subpath.into_checked(),
&[],
)?;

Expand Down Expand Up @@ -1420,8 +1420,8 @@ impl Task {

let mut file_copier = FileCopier::init(
cached_package_dir.unwrap(),
src_path.into_sep::<{ PathSeparators::AUTO }>(),
dest_subpath.into_sep::<{ PathSeparators::AUTO }>(),
src_path.into_checked(),
dest_subpath.into_checked(),
&[],
)?;

Expand Down
Loading