Skip to content

install: resolve a bundled file: dependency whose path leaves the package #43125

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

Closed
robobun wants to merge 4 commits into from
Closed

install: resolve a bundled file: dependency whose path leaves the package #43125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b629ae
install: keep bundleDependencies from every dependency group when par…
robobun Sep 17, 2026
b809ae2
install: resolve a bundled file: dependency whose path leaves the pac…
robobun Sep 17, 2026
621154c
ci: retrigger
robobun Sep 17, 2026
0b1d51f
install: shorten two comments
robobun Sep 17, 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: 3 additions & 1 deletion src/install/PackageManager/PackageManagerEnqueue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2909,7 +2909,9 @@ fn get_or_put_resolved_package(
}

// transitive folder dependencies do not have their dependencies resolved
if crate::bin::bin_target_escapes_package_dir(this.lockfile.str(&folder))
// Nothing installs a bundled one, and every reader of the recorded path refuses it.
if !behavior.is_bundled()
&& crate::bin::bin_target_escapes_package_dir(this.lockfile.str(&folder))
&& !this.lockfile.is_trusted_folder_dependency(dependency_id)
{
break 'res FolderResolutionValue::Err(crate::Error::MissingPackageJSON);
Comment thread
robobun marked this conversation as resolved.
Resolved
Expand Down
27 changes: 25 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 @@ -22,8 +22,8 @@ use crate::package_manager_real::package_manager_directories::{
compute_cache_dir_and_subpath, get_temporary_directory,
};
use crate::{
BuntagHashBuf, DependencyID, Features, PackageID, Resolution, buntaghashbuf_make,
initialize_store, invalid_package_id,
BuntagHashBuf, DependencyID, Features, PackageID, Resolution, ResolutionTag,
buntaghashbuf_make, initialize_store, invalid_package_id,
};

#[inline]
Expand All @@ -46,6 +46,25 @@ fn print_resolution_label<'a>(
label
}

/// `bun patch` reads a `file:` package from its folder, so it refuses what the installers refuse.
fn refuse_unsafe_folder_package(lockfile: &Lockfile, pkg: &Package) {
if pkg.resolution.tag != ResolutionTag::Folder {
return;
}
let folder = lockfile.str(pkg.resolution.folder());
if !crate::bin::bin_target_escapes_package_dir(folder)
|| lockfile.is_trusted_folder_package(pkg.meta.id)
{
return;
}
bun_core::pretty_errorln!(
"<r><red>error<r>: refusing to patch <b>{}<r> with unsafe folder path \"{}\"",
bstr::BStr::new(lockfile.str(&pkg.name)),
bstr::BStr::new(folder),
);
Global::crash();
}

#[derive(Default)]
pub struct PatchCommitResult {
pub(crate) patch_key: Box<[u8]>,
Expand Down Expand Up @@ -269,6 +288,7 @@ pub fn do_patch_commit(

// `compute_cache_dir_and_subpath` resolves `pkg.resolution`'s strings against `manager.lockfile`.
manager.lockfile = lockfile;
refuse_unsafe_folder_package(&manager.lockfile, &pkg);
let name = manager.lockfile.str(&pkg.name).to_vec();
let cache_result =
compute_cache_dir_and_subpath(manager, &name, &pkg.resolution, &mut folder_path_buf, None);
Expand Down Expand Up @@ -833,6 +853,8 @@ pub fn prepare_patch(manager: &mut PackageManager) -> Result<(), crate::Error> {
}
};

refuse_unsafe_folder_package(lockfile, &actual_package);

let name = lockfile.str(&package.name).to_vec();
let existing_patchfile_hash: Option<u64> = 'existing_patchfile_hash: {
let mut name_and_version = Vec::new();
Expand Down Expand Up @@ -889,6 +911,7 @@ pub fn prepare_patch(manager: &mut PackageManager) -> Result<(), crate::Error> {

let strbuf = manager.lockfile.buffers.string_bytes.as_slice();
let pkg = *manager.lockfile.packages.get(pkg_id as usize);
refuse_unsafe_folder_package(&manager.lockfile, &pkg);
let pkg_name = pkg.name.slice(strbuf).to_vec();

let existing_patchfile_hash: Option<u64> = 'existing_patchfile_hash: {
Expand Down
127 changes: 46 additions & 81 deletions src/install/npm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::Error;
use crate::bun_json as JSON;
use crate::bun_schema::api;
use bun_alloc::AllocError;
use bun_collections::{HashMap, IdentityContext, StringSet, index_sort};
use bun_collections::{HashMap, IdentityContext, index_sort};
use bun_core::{Global, Output, fmt as bun_fmt};
use bun_core::{MutableString, strings};
use bun_dotenv::Loader as DotEnv;
Expand Down Expand Up @@ -933,8 +933,9 @@ pub mod package_manifest {
// - v0.0.5: added bundled dependencies
// - v0.0.6: changed semver major/minor/patch to each use u64 instead of u32
// - v0.0.7: added version publish times and extended manifest flag for minimum release age
// - v0.0.8: fixed bundled dependencies being dropped from all but one dependency group
const HEADER_BYTES: &'static str =
concat!("#!/usr/bin/env bun\n", "bun-npm-manifest-cache-v0.0.7\n");
concat!("#!/usr/bin/env bun\n", "bun-npm-manifest-cache-v0.0.8\n");

// Field order is hardcoded (descending alignment). Re-verify if the
// layout changes.
Expand Down Expand Up @@ -1972,35 +1973,6 @@ impl PackageManifest {
}
}

/// Fills `set` with the names listed in `bundle(d)Dependencies`; returns whether it was `true` (bundle everything).
fn collect_bundled_deps(
version_obj: Option<&JSON::E::ObjectJSON>,
set: &mut StringSet,
) -> Result<bool, AllocError> {
set.map.clear_retaining_capacity();
let mut bundle_all_deps = false;
if let Some(bundled_deps_value) = version_obj
.and_then(|o| o.get(b"bundleDependencies"))
.or_else(|| version_obj.and_then(|o| o.get(b"bundledDependencies")))
{
match bundled_deps_value {
JSON::E::JsonValue::Boolean(boolean) => {
bundle_all_deps = *boolean;
}
JSON::E::JsonValue::Array(arr) => {
for bundled_dep in arr.get().items() {
let Some(s) = bundled_dep.as_str() else {
continue;
};
set.insert(s)?;
}
}
_ => {}
}
}
Ok(bundle_all_deps)
}

// Keys are pre-hashed string hashes, so don't re-hash them.
type ExternalStringMapDeduper = HashMap<u64, ExternalStringList, IdentityContext<u64>>;

Expand All @@ -2012,6 +1984,16 @@ const DEPENDENCY_GROUPS: [DependencyGroup; 3] = [
DependencyGroup::PEER,
];

/// npm accepts both spellings; `bundleDependencies` is the documented one.
fn bundled_dependencies_field(
version_obj: Option<&JSON::E::ObjectJSON>,
) -> Option<&JSON::E::JsonValue> {
let version_obj = version_obj?;
version_obj
.get(b"bundleDependencies")
.or_else(|| version_obj.get(b"bundledDependencies"))
}

impl PackageManifest {
/// This parses [Abbreviated metadata](https://github.com/npm/registry/blob/master/docs/responses/package-metadata.md#abbreviated-metadata-format)
pub fn parse(
Expand Down Expand Up @@ -2065,9 +2047,6 @@ impl PackageManifest {
let mut version_extern_strings_dedupe_map = ExternalStringMapDeduper::default();
let mut optional_peer_dep_names: Vec<u64> = Vec::new();

let mut bundled_deps_set = StringSet::init();
let mut bundle_all_deps: bool;

let mut bundled_deps_count: usize = 0;

let mut string_builder = Semver::semver_string::Builder {
Expand Down Expand Up @@ -2200,7 +2179,16 @@ impl PackageManifest {
}
}

bundle_all_deps = collect_bundled_deps(version_obj, &mut bundled_deps_set)?;
if let Some(JSON::E::JsonValue::Array(arr)) =
bundled_dependencies_field(version_obj)
{
bundled_deps_count += arr
.get()
.items()
.iter()
.filter(|bundled_dep| bundled_dep.as_str().is_some())
.count();
}

for pair in &DEPENDENCY_GROUPS {
if let Some(obj) = version_obj
Expand All @@ -2211,11 +2199,6 @@ impl PackageManifest {
dependency_sum += properties.len();
for property in properties {
let key = property.key.slice();
if !bundle_all_deps && bundled_deps_set.swap_remove(key) {
// swap remove the dependency name because it could exist in
// multiple behavior groups.
bundled_deps_count += 1;
}
string_builder.count(key);
string_builder.count(property.value.as_str().unwrap_or(b""));
}
Expand Down Expand Up @@ -2404,10 +2387,30 @@ impl PackageManifest {

let version_obj = prop.value.as_object();

bundle_all_deps = collect_bundled_deps(version_obj, &mut bundled_deps_set)?;

let mut package_version: PackageVersion = empty_version;

// `Package::from_npm` matches these against the dependencies of every group.
package_version.bundled_dependencies = match bundled_dependencies_field(version_obj)
{
Some(JSON::E::JsonValue::Boolean(true)) => ExternalPackageNameHashList::INVALID,
Some(JSON::E::JsonValue::Array(arr)) => {
let bundled_deps_begin = bundled_deps_offset;
for bundled_dep in arr.get().items() {
let Some(name) = bundled_dep.as_str() else {
continue;
};
bundled_deps_buf[bundled_deps_offset] =
Semver::semver_string::Builder::string_hash(name);
bundled_deps_offset += 1;
}
ExternalPackageNameHashList::init(
&bundled_deps_buf,
&bundled_deps_buf[bundled_deps_begin..bundled_deps_offset],
)
}
_ => ExternalPackageNameHashList::default(),
};

if let Some(cpu) = version_obj.and_then(|o| o.get(b"cpu")) {
package_version.cpu = negatable_from_json_value::<Architecture>(cpu);
}
Expand Down Expand Up @@ -2629,13 +2632,7 @@ impl PackageManifest {
// For peer deps, fall through with an empty `items`
// slice when `peerDependencies` is absent so that
// `peerDependenciesMeta`-only entries (synthesised
// below) still get a build pass. The fallthrough must
// stay scoped to packages that actually have a
// `peerDependenciesMeta`: the body sets
// `package_version.bundled_dependencies` from this
// iteration's slice of `bundled_deps_buf`, so an
// unconditional empty pass would clobber the value the
// `dependencies` iteration just produced.
// below) still get a build pass.
let items: &[JSON::E::PropertyJSON] = version_obj
.and_then(|o| o.get(pair.prop))
.and_then(|deps| deps.as_object())
Expand Down Expand Up @@ -2684,8 +2681,6 @@ impl PackageManifest {
}
}

let bundled_deps_begin = bundled_deps_offset;

let mut i: usize = 0;

for item in items {
Expand All @@ -2706,15 +2701,6 @@ impl PackageManifest {
version_extern_strings[values_base + i] =
string_builder.append::<ExternalString>(version_str);

if !bundle_all_deps && bundled_deps_set.swap_remove(name_str) {
// SAFETY: bundled_deps_buf sized in counting pass
unsafe {
*bundled_deps_buf.as_mut_ptr().add(bundled_deps_offset) =
all_extern_strings[names_base + i].hash;
}
bundled_deps_offset += 1;
}

if is_peer {
if optional_peer_dep_names
.iter()
Expand Down Expand Up @@ -2814,27 +2800,6 @@ impl PackageManifest {
let this_versions =
&version_extern_strings[values_base..values_base + count];

// Bundled deps are matched against the
// `dependencies`/`optionalDependencies` groups
// only; the peer pass never adds to
// `bundled_deps_buf`. With the meta-only
// synthesis above the peer body now runs even
// when `peerDependencies` is absent, so writing
// here would clobber the value the dependencies
// pass already produced with an empty slice.
if !is_peer {
if bundle_all_deps {
package_version.bundled_dependencies =
ExternalPackageNameHashList::INVALID;
} else {
package_version.bundled_dependencies =
ExternalPackageNameHashList::init(
&bundled_deps_buf,
&bundled_deps_buf[bundled_deps_begin..bundled_deps_offset],
);
}
}

let mut name_list =
ExternalStringList::init(&all_extern_strings, this_names);
let mut version_list =
Expand Down
Loading
Loading