Skip to content
Closed
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
24 changes: 22 additions & 2 deletions src/install/PackageInstaller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1517,11 +1517,31 @@ 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!());

// Rows loaded from bun.lock skip the resolver's check.
if crate::dependency::link_path_escapes_root(folder)
&& !self.lockfile().is_trusted_folder_package(package_id)
{
Comment thread
robobun marked this conversation as resolved.
Resolved
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 a top-level override may link to a path outside the project",
bstr::BStr::new(pkg_name.slice(string_buf!())),
bstr::BStr::new(folder),
);
}
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.
Resolved
}

let directory = package_manager::global_link_dir(self.manager_mut());

if folder.is_empty() || (folder.len() == 1 && folder[0] == b'.') {
installer.cache_dir_subpath = ZStr::from_static(b".\0");
installer.cache_dir = Fd::cwd();
Expand Down
45 changes: 45 additions & 0 deletions src/install/PackageManager/PackageManagerEnqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1597,6 +1597,17 @@ pub fn enqueue_dependency_with_main_and_success_fn(
dependency::version::Tag::Symlink | dependency::version::Tag::Workspace => {
let dependency_tag = version.tag;

// Same trust rule as the `file:` arm below and both installers.
if dependency_tag == dependency::version::Tag::Symlink
&& dependency::link_path_escapes_root(this.lockfile.str(version.symlink()))
&& !this.lockfile.is_trusted_folder_dependency(id)
Comment thread
robobun marked this conversation as resolved.
Resolved
{
if dependency.behavior.is_required() {
reject_link_path_of_remote_package(this, id, dependency, &version);
}
return Ok(());
Comment thread
robobun marked this conversation as resolved.
Resolved
Comment thread
robobun marked this conversation as resolved.
Resolved
Comment thread
claude[bot] marked this conversation as resolved.
Resolved
}
Comment thread
robobun marked this conversation as resolved.
Resolved

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
let _result = match get_or_put_resolved_package(
this,
name_hash,
Expand Down Expand Up @@ -1852,6 +1863,40 @@ fn warn_unmet_peer_dependency(
);
}

#[cold]
#[inline(never)]
fn reject_link_path_of_remote_package(
this: &PackageManager,
dependency_id: DependencyID,
dependency: &Dependency,
version: &dependency::Version,
) {
const REASON: &str = "only the root package.json, a workspace, or a top-level override may link to a path outside the project";
let buf = this.lockfile.buffers.string_bytes.as_slice();
let packages = this.lockfile.packages.slice();
let name = bstr::BStr::new(dependency.name.slice(buf));
let literal = bstr::BStr::new(version.literal.slice(buf));
match this.lockfile.get_parent_pkg_of_dependency(dependency_id) {
Some(declarer) => this.log_mut().add_error_fmt(
None,
bun_ast::Loc::EMPTY,
format_args!(
"refusing to resolve \"{}@{}\" declared by {}@{}: {}",
name,
literal,
bstr::BStr::new(packages.items_name()[declarer as usize].slice(buf)),
packages.items_resolution()[declarer as usize].fmt(buf, bun_fmt::PathSep::Posix),
REASON,
),
),
None => this.log_mut().add_error_fmt(
None,
bun_ast::Loc::EMPTY,
format_args!("refusing to resolve \"{}@{}\": {}", name, literal, REASON),
),
}
}

/// Allocate and initialise an `.extract` Task for an npm tarball.
/// Shared by the buffered path (`enqueueExtractNPMPackage`) and the
/// streaming path (`createExtractTaskForStreaming`) so both produce
Expand Down
13 changes: 11 additions & 2 deletions src/install/PackageManager/install_with_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,8 @@ pub fn install_with_manager(
let off = lf.dependencies.len() as u32;
let len = (new_dependencies.len() + kept_pruned.len()) as u32;
let old_resolutions_list = lf.packages.items_resolutions()[0];
// Stays in the buffer, declared by no package from here on.
let replaced_root_rows = root.dependencies;
lf.packages.items_dependencies_mut()[0] =
lockfile::DependencySlice::new(off, len);
lf.packages.items_resolutions_mut()[0] =
Expand Down Expand Up @@ -501,7 +503,9 @@ pub fn install_with_manager(
if manager.summary.overrides_changed && !all_name_hashes.is_empty() {
let dependencies_len = manager.lockfile.buffers.dependencies.len();
for dependency_i in 0..dependencies_len {
if pinned_rows.is_set_allow_out_of_bound(dependency_i, false) {
if pinned_rows.is_set_allow_out_of_bound(dependency_i, false)
|| replaced_root_rows.contains(dependency_i as u32)
{
continue;
}
let dependency =
Expand Down Expand Up @@ -535,7 +539,9 @@ pub fn install_with_manager(
let dependencies_len = manager.lockfile.buffers.dependencies.len();
for _dep_id in 0..dependencies_len {
let dep_id: DependencyID = u32::try_from(_dep_id).expect("int cast");
if pinned_rows.is_set_allow_out_of_bound(_dep_id, false) {
if pinned_rows.is_set_allow_out_of_bound(_dep_id, false)
|| replaced_root_rows.contains(dep_id)
{
continue;
}
let dep =
Expand Down Expand Up @@ -839,6 +845,9 @@ pub fn install_with_manager(
let save_format = load_result.save_format(&manager.options);

if manager.options.lockfile_only {
if had_errors_before_cleaning_lockfile {
Global::crash();
}
Comment thread
robobun marked this conversation as resolved.
Resolved
// save the lockfile and exit. make sure metahash is generated for binary lockfile
return save_lockfile_only(
manager,
Expand Down
5 changes: 5 additions & 0 deletions src/install/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,11 @@ pub(crate) fn is_safe_install_folder_name(name: &[u8]) -> bool {
true
}

/// Leaves the project root (`..` or absolute); same rule `file:` uses.
pub(crate) fn link_path_escapes_root(target: &[u8]) -> bool {
crate::bin::bin_target_escapes_package_dir(target)
Comment thread
robobun marked this conversation as resolved.
Resolved
}

/// assumes version is valid
pub fn without_build_tag(version: &[u8]) -> &[u8] {
if let Some(plus) = strings::index_of_char(version, b'+') {
Expand Down
31 changes: 31 additions & 0 deletions src/install/isolated_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,36 @@ pub(crate) enum Timings {
Quiet,
}

/// Rows loaded from bun.lock skip the resolver's check.
fn refuse_escaping_links(lockfile: &Lockfile, store: &Store) {
let string_buf = &lockfile.buffers.string_bytes[..];
let pkgs = lockfile.packages.slice();
let pkg_names = pkgs.items_name();
let pkg_resolutions = pkgs.items_resolution();
let node_pkg_ids = store.nodes.items_pkg_id();
for &node_id in store.entries.items_node_id() {
let pkg_id = node_pkg_ids[node_id.get() as usize];
let pkg_res = &pkg_resolutions[pkg_id as usize];
if pkg_res.tag != ResolutionTag::Symlink {
Comment thread
robobun marked this conversation as resolved.
Resolved
continue;
}
let target = pkg_res.symlink().slice(string_buf);
if crate::dependency::link_path_escapes_root(target)
&& !lockfile.is_trusted_folder_package(pkg_id)
{
Output::err_generic(
"refusing to link dependency <b>{}<r> to \"{}\": only the root package.json, a workspace, or a top-level 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);
}
}
}

pub(crate) fn build_store(
manager: &PackageManager,
lockfile: &Lockfile,
Expand Down Expand Up @@ -1163,6 +1193,7 @@ pub(crate) fn install_isolated_packages(
packages_to_install,
timings,
)?;
refuse_escaping_links(&*lockfile, &store);

let global_store_path: Option<Vec<u8>> = if manager.options.enable.global_virtual_store() {
'global_store_path: {
Expand Down
14 changes: 14 additions & 0 deletions src/install/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,20 @@ impl Lockfile {
.contains_name(dep.name_hash, dep.name.slice(buf), buf)
}

/// Does a trusted dependency, installed or filtered out, resolve to folder package `id`?
pub(crate) fn is_trusted_folder_package(&self, id: PackageID) -> bool {
self.buffers
.resolutions
.iter()
.enumerate()
.any(|(dep_id, &pkg_id)| {
pkg_id == id
&& self.is_trusted_folder_dependency(
Comment thread
robobun marked this conversation as resolved.
Resolved
DependencyID::try_from(dep_id).expect("int cast"),
)
})
}

/// Does this tree id belong to a workspace (including workspace root)?
/// TODO(dylan-conway) fix!
pub(crate) fn is_workspace_tree_id(&self, id: tree::Id) -> bool {
Expand Down
Loading
Loading