Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 27 additions & 2 deletions src/install/PackageInstaller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1558,15 +1558,40 @@ impl<'a> PackageInstaller<'a> {
installer.cache_dir = Fd::cwd();
}
resolution::Tag::Symlink => {
let directory = package_manager::global_link_dir(self.manager_mut());

let folder_str = *resolution.symlink();
let folder = folder_str.slice(string_buf!());

if folder.is_empty() || (folder.len() == 1 && folder[0] == b'.') {
installer.cache_dir_subpath = ZStr::from_static(b".\0");
installer.cache_dir = Fd::cwd();
} else if crate::dependency::is_link_path(folder) {
if folder.len() >= self.folder_path_buf.len()
|| !self
.lockfile()
.link_target_allowed_for_package(package_id, folder)
{
if log_level != Options::LogLevel::Silent {
bun_core::pretty_errorln!(
"<r><red>error<r>: refusing to link dependency <b>{}<r> to \"{}\": only the root package.json, a workspace, or an override may link to a path outside the project",
bstr::BStr::new(pkg_name.slice(string_buf!())),
bstr::BStr::new(folder),
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
self.summary.fail += 1;
self.increment_tree_install_count(
!IS_PENDING_PACKAGE_INSTALL,
self.current_tree_id,
log_level,
);
return;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
self.folder_path_buf[..folder.len()].copy_from_slice(folder);
self.folder_path_buf[folder.len()] = 0;
installer.cache_dir_subpath =
ZStr::from_buf(&self.folder_path_buf, folder.len());
installer.cache_dir = Fd::cwd();
} else {
let directory = package_manager::global_link_dir(self.manager_mut());
let global_link_dir = package_manager::global_link_dir_path(self.manager_mut());
let buf = self.folder_path_buf.as_mut_slice();
let mut len = 0usize;
Expand Down
8 changes: 6 additions & 2 deletions src/install/PackageManager/PackageManagerDirectories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1009,8 +1009,6 @@ pub fn compute_cache_dir_and_subpath<'a>(
cache_dir = Fd::cwd();
}
ResolutionTag::Symlink => {
let directory = global_link_dir(manager);

// borrowck — `global_link_dir_path` below reborrows
// `manager` mutably, so copy the symlink target out of the lockfile
// string buffer first instead of holding a slice across that call.
Expand All @@ -1022,7 +1020,13 @@ pub fn compute_cache_dir_and_subpath<'a>(
if folder.is_empty() || (folder.len() == 1 && folder[0] == b'.') {
cache_dir_subpath = z_static(b".\0");
cache_dir = Fd::cwd();
} else if crate::dependency::is_link_path(&folder) {
folder_path_buf[..folder.len()].copy_from_slice(&folder);
folder_path_buf[folder.len()] = 0;
cache_dir_subpath = ZStr::from_buf(folder_path_buf, folder.len());
cache_dir = Fd::cwd();
} else {
let directory = global_link_dir(manager);
let global_link_dir = global_link_dir_path(manager);
let ptr = &mut folder_path_buf.0[..];
let mut off = 0usize;
Expand Down
83 changes: 70 additions & 13 deletions src/install/PackageManager/PackageManagerEnqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1419,6 +1419,19 @@ pub fn enqueue_dependency_with_main_and_success_fn(
) {
Ok(v) => v,
Err(crate::Error::MissingPackageJSON) => None,
Err(crate::Error::UnsafeLinkTarget) => {
if dependency.behavior.is_required() {
bun_ast::add_error_pretty!(
this.log_mut(),
None,
bun_ast::Loc::EMPTY,
"Refusing to link dependency \"{}\" to \"{}\": only the root package.json, a workspace, or an override may link to a path outside the project\n\n",
bstr::BStr::new(this.lockfile.str(&name)),
bstr::BStr::new(this.lockfile.str(version.symlink())),
);
}
return Ok(());
}
Comment thread
robobun marked this conversation as resolved.
Err(err) => return Err(err),
};

Expand Down Expand Up @@ -1476,6 +1489,15 @@ pub fn enqueue_dependency_with_main_and_success_fn(
quoted: true
},
);
} else if dependency::is_link_path(this.lockfile.str(version.symlink())) {
bun_ast::add_error_pretty!(
this.log_mut(),
None,
bun_ast::Loc::EMPTY,
"Could not find package.json at \"{}\" for dependency \"{}\"\n\n",
bstr::BStr::new(this.lockfile.str(version.symlink())),
bstr::BStr::new(this.lockfile.str(&name)),
);
Comment thread
claude[bot] marked this conversation as resolved.
} else {
bun_ast::add_error_pretty!(
this.log_mut(),
Expand All @@ -1499,6 +1521,15 @@ pub fn enqueue_dependency_with_main_and_success_fn(
quoted: true
},
);
} else if dependency::is_link_path(this.lockfile.str(version.symlink())) {
bun_ast::add_warning_pretty!(
this.log_mut(),
None,
bun_ast::Loc::EMPTY,
"Could not find package.json at \"{}\" for dependency \"{}\"\n\n",
bstr::BStr::new(this.lockfile.str(version.symlink())),
bstr::BStr::new(this.lockfile.str(&name)),
);
} else {
bun_ast::add_warning_pretty!(
this.log_mut(),
Expand Down Expand Up @@ -2800,21 +2831,47 @@ fn get_or_put_resolved_package(
// reshaped for borrowck — `link_dir` / `symlink_path`
// borrow into `*this`; detach their lifetimes so the
// `&mut PackageManager` reborrow for `get_or_put` does not
// conflict.
// SAFETY: `global_link_dir_path` returns a slice into the lazily-
// initialized `PackageManager.global_link_dir_path` (a `Box<[u8]>`
// set once and never freed); `get_or_put` copies `symlink_path`
// into the lockfile string buffer before any other mutation.
// conflict. `get_or_put` copies `symlink_path` into the lockfile
// string buffer before any other mutation.
Comment thread
robobun marked this conversation as resolved.
// `version.tag == Symlink`.
let link_dir =
unsafe { detach_lifetime(package_manager_real::global_link_dir_path(this)) };
let symlink_path = this.lockfile.str_detached(version.symlink());
let res = FolderResolution::get_or_put(
GlobalOrRelative::Global(link_dir),
version,
symlink_path,
this,
);
let res = if dependency::is_link_path(symlink_path) {
if !this
.lockfile
.link_target_allowed_for_dependency(dependency_id, symlink_path)
{
FolderResolutionValue::Err(crate::Error::UnsafeLinkTarget)
} else {
let mut buf2 = PathBuffer::uninit();
let symlink_path_abs = if bun_paths::is_absolute(symlink_path) {
symlink_path
} else {
Path::resolve_path::join_abs_string_buf::<Path::platform::Auto>(
FileSystem::instance().top_level_dir(),
&mut buf2,
&[symlink_path],
)
};
FolderResolution::get_or_put(
GlobalOrRelative::Relative(dependency::version::Tag::Symlink),
version,
symlink_path_abs,
this,
)
}
} else {
// SAFETY: `global_link_dir_path` returns a slice into the
// lazily-initialized `PackageManager.global_link_dir_path`
// (a `Box<[u8]>` set once and never freed).
let link_dir =
unsafe { detach_lifetime(package_manager_real::global_link_dir_path(this)) };
FolderResolution::get_or_put(
GlobalOrRelative::Global(link_dir),
version,
symlink_path,
this,
)
};

match res {
FolderResolutionValue::Err(err) => Err(err),
Expand Down
7 changes: 5 additions & 2 deletions src/install/PackageManager/add_remove_with_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,11 @@ pub(crate) fn local_relative_path(request: &UpdateRequest) -> Option<(&'static [
dependency::Tag::Symlink => (b"link:", literal.strip_prefix(b"link:")?),
_ => return None,
};
let is_path = path.starts_with(b".")
|| (prefix != b"link:" && !path.is_empty() && !strings::contains(path, b"://"));
let is_path = if prefix == b"link:" {
dependency::is_link_path(path)
} else {
path.starts_with(b".") || (!path.is_empty() && !strings::contains(path, b"://"))
};
(is_path && !path.starts_with(b"//") && !Platform::AUTO.is_absolute(path))
.then_some((prefix, path))
}
Expand Down
45 changes: 43 additions & 2 deletions src/install/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -717,11 +717,12 @@ impl VersionExt for Version {
true,
) || self.npm().eql(rhs.npm(), lhs_buf, rhs_buf)
}
Tag::Folder | Tag::DistTag => self.literal.eql(rhs.literal, lhs_buf, rhs_buf),
Tag::Folder | Tag::DistTag | Tag::Symlink => {
self.literal.eql(rhs.literal, lhs_buf, rhs_buf)
}
Tag::Git => Repository::eql(self.git(), rhs.git(), lhs_buf, rhs_buf),
Tag::Github => Repository::eql(self.github(), rhs.github(), lhs_buf, rhs_buf),
Tag::Tarball => self.tarball().eql(rhs.tarball(), lhs_buf, rhs_buf),
Tag::Symlink => self.symlink().eql(*rhs.symlink(), lhs_buf, rhs_buf),
Tag::Workspace => self.workspace().eql(*rhs.workspace(), lhs_buf, rhs_buf),
Tag::Catalog => self.catalog().eql(*rhs.catalog(), lhs_buf, rhs_buf),
_ => true,
Expand Down Expand Up @@ -1134,6 +1135,46 @@ impl ValueExt for Value {
// Free functions: parse
// ──────────────────────────────────────────────────────────────────────────

/// A `link:` target is a path unless it is a package name (`bun link <name>`).
/// Shared with the pnpm-lock.yaml migration so both agree on stored values.
Comment thread
robobun marked this conversation as resolved.
pub(crate) fn is_link_path(value: &[u8]) -> bool {
!value.is_empty() && !strings::is_npm_package_name(value)
}

/// Stored form of a path-form target (root-relative or absolute): `/`-separated
/// and `./`-prefixed where needed, so `is_link_path` holds when read back.
Comment thread
robobun marked this conversation as resolved.
pub(crate) fn link_path_for_lockfile<'a>(
relative: &[u8],
buf: &'a mut bun_paths::PathBuffer,
) -> Option<&'a [u8]> {
if relative.is_empty() || relative == b"." {
return Some(b".");
}
let already_shaped = relative == b".."
|| relative.starts_with(b"./")
|| relative.starts_with(b"../")
|| (cfg!(windows) && (relative.starts_with(b".\\") || relative.starts_with(b"..\\")))
|| bun_paths::is_absolute(relative);
let prefix_len = if already_shaped { 0 } else { 2 };
let total = prefix_len + relative.len();
if total > buf.len() {
return None;
}
if prefix_len != 0 {
buf[0] = b'.';
buf[1] = b'/';
}
buf[prefix_len..total].copy_from_slice(relative);
#[cfg(windows)]
bun_paths::dangerously_convert_path_to_posix_in_place::<u8>(&mut buf[..total]);
Some(&buf[..total])
}

/// Leaves the project root (`..` or absolute); same rule `file:` uses.
pub(crate) fn link_path_escapes_root(stored: &[u8]) -> bool {
crate::bin::bin_target_escapes_package_dir(stored)
}

#[cfg(windows)]
pub(crate) fn is_windows_abs_path_with_leading_slashes(dep: &[u8]) -> Option<&[u8]> {
let mut i: usize = 0;
Expand Down
3 changes: 3 additions & 0 deletions src/install/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ pub enum Error {
TooRecentVersion,
#[error("MissingPackageJSON")]
MissingPackageJSON,
#[error("UnsafeLinkTarget")]
UnsafeLinkTarget,
#[error("InstallFailed")]
InstallFailed,
#[error("HTTPError")]
Expand Down Expand Up @@ -280,6 +282,7 @@ impl Error {
Self::NoMatchingVersion => "NoMatchingVersion",
Self::TooRecentVersion => "TooRecentVersion",
Self::MissingPackageJSON => "MissingPackageJSON",
Self::UnsafeLinkTarget => "UnsafeLinkTarget",
Self::InstallFailed => "InstallFailed",
Self::HTTPError => "HTTPError",
Self::Failed => "Failed",
Expand Down
41 changes: 30 additions & 11 deletions src/install/isolated_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2080,18 +2080,37 @@ pub(crate) fn install_isolated_packages(
task.installer = installer_backref;
}

// `append_store_path` runs on worker threads via `&Installer` and
// can't take `&mut PackageManager` there, so ensure the
// global link dir once on the main thread before any `.symlink`
// resolution can be reached by a task. Guarded so installs without
// `link:` deps don't touch the global dir.
if pkg_resolutions
.iter()
.any(|r| r.tag == ResolutionTag::Symlink)
// Must precede the first `start_task`: workers symlink `link:` packages
// into dependents (`append_store_path`) and cannot create the global dir.
Comment thread
robobun marked this conversation as resolved.
{
let _ = crate::package_manager_real::directories::global_link_dir_path(
installer.manager_mut(),
);
let mut needs_global_link_dir = false;
for (pkg_id, res) in pkg_resolutions.iter().enumerate() {
if res.tag != ResolutionTag::Symlink {
continue;
}
let target = res.symlink().slice(string_buf);
if !crate::dependency::is_link_path(target) {
needs_global_link_dir = true;
continue;
}
let pkg_id = PackageID::try_from(pkg_id).expect("int cast");
if !lockfile_ro.link_target_allowed_for_package(pkg_id, target) {
Output::err_generic(
"refusing to link dependency <b>{}<r> to \"{}\": only the root package.json, a workspace, or an override may link to a path outside the project",
(
BStr::new(pkg_names[pkg_id as usize].slice(string_buf)),
BStr::new(target),
),
);
Output::flush();
Global::exit(1);
}
}
if needs_global_link_dir {
let _ = crate::package_manager_real::directories::global_link_dir_path(
installer.manager_mut(),
);
}
}

// add the pending task count upfront
Expand Down
36 changes: 22 additions & 14 deletions src/install/isolated_install/Installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2762,21 +2762,29 @@ impl<'a> Installer<'a> {
buf.append(pkg_res.workspace().slice(string_buf));
}
ResolutionTag::Symlink => {
// Lazily ensuring the global link dir would mutate
// `*PackageManager`, but `append_store_path` is
// `&self` and may run on worker
// threads, so the lazy init is hoisted to the main-thread caller
// (`isolated_install::install_packages`, before any `start_task`).
// Reading the cached field here is then equivalent.
let symlink_dir_path: &[u8] = &self.manager().global_link_dir_path;
debug_assert!(
!symlink_dir_path.is_empty(),
"global_link_dir_path must be ensured before tasks start",
);
let symlink = pkg_res.symlink().slice(string_buf);
if crate::dependency::is_link_path(symlink) {
if bun_paths::is_absolute(symlink) {
buf.clear();
}
buf.append(symlink);
} else {
// Lazily ensuring the global link dir would mutate
// `*PackageManager`, but `append_store_path` is
// `&self` and may run on worker
// threads, so the lazy init is hoisted to the main-thread caller
// (`isolated_install::install_packages`, before any `start_task`).
// Reading the cached field here is then equivalent.
Comment thread
robobun marked this conversation as resolved.
let symlink_dir_path: &[u8] = &self.manager().global_link_dir_path;
debug_assert!(
!symlink_dir_path.is_empty(),
"global_link_dir_path must be ensured before tasks start",
);

buf.clear();
buf.append(symlink_dir_path);
buf.append(pkg_res.symlink().slice(string_buf));
buf.clear();
buf.append(symlink_dir_path);
buf.append(symlink);
}
Comment thread
robobun marked this conversation as resolved.
}
_ => {
let pkg_name = pkg_names[pkg_id as usize];
Expand Down
Loading