diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 6c96867f593b..c292bbc7f68b 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -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!( + "error: refusing to link dependency {} 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), + ); + } + self.summary.fail += 1; + self.increment_tree_install_count( + !IS_PENDING_PACKAGE_INSTALL, + self.current_tree_id, + log_level, + ); + return; + } + 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; diff --git a/src/install/PackageManager/PackageManagerDirectories.rs b/src/install/PackageManager/PackageManagerDirectories.rs index 4861484c4b21..9ba77e362598 100644 --- a/src/install/PackageManager/PackageManagerDirectories.rs +++ b/src/install/PackageManager/PackageManagerDirectories.rs @@ -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. @@ -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; diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index 5c120f26c777..a7cc125bf858 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -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(()); + } Err(err) => return Err(err), }; @@ -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)), + ); } else { bun_ast::add_error_pretty!( this.log_mut(), @@ -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(), @@ -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. // `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::( + 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), diff --git a/src/install/PackageManager/add_remove_with_filter.rs b/src/install/PackageManager/add_remove_with_filter.rs index 1ae652f46c3e..0e0af100cb42 100644 --- a/src/install/PackageManager/add_remove_with_filter.rs +++ b/src/install/PackageManager/add_remove_with_filter.rs @@ -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)) } diff --git a/src/install/dependency.rs b/src/install/dependency.rs index 2f5b38f9b895..4706de29e25d 100644 --- a/src/install/dependency.rs +++ b/src/install/dependency.rs @@ -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, @@ -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 `). +/// Shared with the pnpm-lock.yaml migration so both agree on stored values. +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. +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::(&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; diff --git a/src/install/error.rs b/src/install/error.rs index 87ce091d1bca..6f478efc042c 100644 --- a/src/install/error.rs +++ b/src/install/error.rs @@ -62,6 +62,8 @@ pub enum Error { TooRecentVersion, #[error("MissingPackageJSON")] MissingPackageJSON, + #[error("UnsafeLinkTarget")] + UnsafeLinkTarget, #[error("InstallFailed")] InstallFailed, #[error("HTTPError")] @@ -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", diff --git a/src/install/isolated_install.rs b/src/install/isolated_install.rs index d89105ac2ead..3ba2ffbe4518 100644 --- a/src/install/isolated_install.rs +++ b/src/install/isolated_install.rs @@ -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. { - 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 {} 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 diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index ce346a116b37..e05bfce361ec 100644 --- a/src/install/isolated_install/Installer.rs +++ b/src/install/isolated_install/Installer.rs @@ -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. + 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); + } } _ => { let pkg_name = pkg_names[pkg_id as usize]; diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index 73d55df73e17..bfaf3318d36d 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -791,6 +791,40 @@ impl Lockfile { invalid_package_id } + /// A path-form `link:` target may leave the project only if the root, a + /// workspace, or a root override declared it; never a transitive package. + pub fn link_target_allowed_for_dependency(&self, id: DependencyID, target: &[u8]) -> bool { + if !dependency::link_path_escapes_root(target) { + return true; + } + if self.is_workspace_dependency(id) { + return true; + } + let buf = self.buffers.string_bytes.as_slice(); + let dep = &self.buffers.dependencies[id as usize]; + self.overrides + .contains_name(dep.name_hash, dep.name.slice(buf), buf) + } + + /// Installer-side check (installs from a lockfile skip resolution): + /// allowed if any dependency resolving to the package is. + pub fn link_target_allowed_for_package(&self, pkg_id: PackageID, target: &[u8]) -> bool { + if !dependency::link_path_escapes_root(target) { + return true; + } + self.buffers + .resolutions + .iter() + .enumerate() + .filter(|(_, resolved)| **resolved == pkg_id) + .any(|(dep_id, _)| { + self.link_target_allowed_for_dependency( + DependencyID::try_from(dep_id).expect("int cast"), + target, + ) + }) + } + /// 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 { diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index fcd5b545bd4d..4027ec0d2aa6 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -1876,6 +1876,46 @@ impl Package { dependency_version.value.folder = string_builder .append::(if relative.is_empty() { b"." } else { relative }); } + dependency::version::Tag::Symlink => { + let symlink = *dependency_version.symlink(); + if dependency::is_link_path(symlink.slice(buf)) { + let mut symlink_buf = PathBuffer::uninit(); + let Some(joined) = + resolve_path::join_abs_string_buf_checked::( + FileSystem::instance().top_level_dir(), + &mut symlink_buf.0, + &[source.path.name().dir, symlink.slice(buf)], + ) + else { + log.add_error_fmt( + source, + value_loc_of(source, key_loc), + format_args!( + "Dependency \"{}\" has an unsafe folder path", + bstr::BStr::new(external_alias.slice(buf)), + ), + ); + return Err(crate::Error::InstallFailed); + }; + let relative = + resolve_path::relative(FileSystem::instance().top_level_dir(), joined); + let mut stored_buf = PathBuffer::uninit(); + let Some(stored) = + dependency::link_path_for_lockfile(relative, &mut stored_buf) + else { + log.add_error_fmt( + source, + value_loc_of(source, key_loc), + format_args!( + "Dependency \"{}\" has an unsafe folder path", + bstr::BStr::new(external_alias.slice(buf)), + ), + ); + return Err(crate::Error::InstallFailed); + }; + dependency_version.value.symlink = string_builder.append::(stored); + } + } dependency::version::Tag::Npm => { if let Some(workspace_version) = workspace_version { let satisfies = @@ -2434,7 +2474,8 @@ impl Package { // If it's a folder or workspace, pessimistically assume we will need a maximum path match dependency::version::Tag::infer(value) { dependency::version::Tag::Folder - | dependency::version::Tag::Workspace => { + | dependency::version::Tag::Workspace + | dependency::version::Tag::Symlink => { string_builder.cap += MAX_PATH_BYTES; } _ => {} diff --git a/src/install/migration.rs b/src/install/migration.rs index dd5270fe8424..beb12d1e1dce 100644 --- a/src/install/migration.rs +++ b/src/install/migration.rs @@ -130,7 +130,7 @@ pub fn detect_and_load_other_lockfile<'a>( } MigratePnpmLockfileError::RelativeLinkDependency => { bun_core::warn!( - "Relative link dependencies aren't supported yet. Please follow along at https://github.com/oven-sh/bun/issues/23026", + "pnpm-lock.yaml migration does not carry over relative link: dependencies; resolving fresh.", ); } MigratePnpmLockfileError::WorkspaceNameMissing => { diff --git a/src/install/pnpm.rs b/src/install/pnpm.rs index abf7916c98f8..2ff6aa6a4e34 100644 --- a/src/install/pnpm.rs +++ b/src/install/pnpm.rs @@ -1036,15 +1036,6 @@ pub(crate) fn migrate_pnpm_lockfile<'a>( ); } - let mut pkg = lockfile::Package { - name: dep.name, - name_hash: dep.name_hash, - resolution: Resolution::init_symlink( - sbuf!(lockfile).append(link_path)?, - ), - ..Default::default() - }; - let mut abs_link_path = bun_paths::AutoAbsPath::init_top_level_dir(); let _ = abs_link_path.join(&[workspace_path, link_path]); // path-buffer overflow unreachable for bounded inputs @@ -1054,11 +1045,38 @@ pub(crate) fn migrate_pnpm_lockfile<'a>( continue; } + // pnpm's value is importer-relative; the stored one is root-relative. + let mut joined_buf = bun_paths::PathBuffer::uninit(); + let root_relative: &[u8] = if bun_paths::is_absolute(link_path) { + link_path + } else { + bun_paths::resolve_path::join_string_buf::< + bun_paths::resolve_path::platform::Posix, + >( + &mut joined_buf.0, &[workspace_path, link_path] + ) + }; + let mut stored_buf = bun_paths::PathBuffer::uninit(); + let Some(stored) = + dependency::link_path_for_lockfile(root_relative, &mut stored_buf) + else { + return Err(invalid_pnpm_lockfile()); + }; + + let mut pkg = lockfile::Package { + name: dep.name, + name_hash: dep.name_hash, + resolution: Resolution::init_symlink( + sbuf!(lockfile).append(stored)?, + ), + ..Default::default() + }; + *pkg_entry.value_ptr = lockfile.append_package_dedupe(&mut pkg)?; } } dependency::VersionTag::Symlink => { - if !strings::is_npm_package_name( + if dependency::is_link_path( dep.version.symlink().slice(string_bytes!(lockfile)), ) { log.add_warning_fmt( diff --git a/src/install/resolvers/folder_resolver.rs b/src/install/resolvers/folder_resolver.rs index d75d0c9c96df..5a680c04ecce 100644 --- a/src/install/resolvers/folder_resolver.rs +++ b/src/install/resolvers/folder_resolver.rs @@ -469,6 +469,20 @@ pub(crate) fn get_or_put( &mut resolver, ); } + dependency::version::Tag::Symlink => 'symlink: { + let mut path = PathBuffer::uninit(); + let Some(folder_path) = dependency::link_path_for_lockfile(rel, &mut path) else { + break 'symlink Err(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG)); + }; + let mut resolver: SymlinkResolver = NewResolver { folder_path }; + break 'symlink read_package_json_from_disk( + manager, + abs, + version, + Features::LINK, + &mut resolver, + ); + } _ => unreachable!(), }, GlobalOrRelative::CacheFolder(_) => 'cache_folder: { diff --git a/test/cli/install/bun-add-filter.test.ts b/test/cli/install/bun-add-filter.test.ts index 80d66f7ecdc8..af5d669d1247 100644 --- a/test/cli/install/bun-add-filter.test.ts +++ b/test/cli/install/bun-add-filter.test.ts @@ -1978,15 +1978,37 @@ test.concurrent.each([ expect(reinstall.exitCode).toBe(0); }); -// `link:` resolves against the global link dir (same failure without --filter); the error shows the per-target spelling. -test.concurrent("a link: path is re-spelled relative to the target before it is resolved", async () => { +// A `link:` value that is not a package name is a path (see bun-link.test.ts): like `file:`, it is relative +// to the cwd and re-spelled per target. A bare name still means a `bun link` registration and is left alone. +test.concurrent.each([["link:./vendor/foo"], ["link:vendor/foo"]])( + "a link: path (%s) is re-spelled relative to the target", + async positional => { + const dir = await makeMonorepo(); + await addVendorFoo(dir); + const before = await allPackageJsonTexts(dir); + + const { stderr, exitCode } = await run(["add", positional, "--filter", "api"], dir, { linker: "hoisted" }); + expect(stderr).not.toContain("error:"); + expect(exitCode).toBe(0); + + await expectAddedOnlyTo(dir, before, ["api"], "foo", "link:../../vendor/foo"); + const { workspaces, packages } = await lockfileJson(dir); + expect(workspaces["packages/api"].dependencies).toStrictEqual({ foo: "link:../../vendor/foo" }); + expect(packages.foo[0]).toBe("foo@link:./vendor/foo"); + expect(await file(join(dir, "node_modules", "foo", "package.json")).json()).toStrictEqual(VENDOR_FOO); + + const frozen = await run(["install", "--frozen-lockfile"], dir, { linker: "hoisted" }); + expect(frozen.stderr).not.toContain("error:"); + expect(frozen.exitCode).toBe(0); + }, +); + +test.concurrent("a link: path that does not exist fails with the per-target spelling and writes nothing", async () => { const dir = await makeMonorepo(); - await addVendorFoo(dir); const before = await allPackageJsonTexts(dir); const { stderr, exitCode } = await run(["add", "link:./vendor/foo", "--filter", "api"], dir); - expect(stderr).toContain('error: Package "link:../../vendor/foo" is not linked'); - expect(stderr).not.toContain('"link:./vendor/foo"'); + expect(stderr).toContain('Could not find package.json at "./vendor/foo" for dependency "link:../../vendor/foo"'); expect(exitCode).toBe(1); expect(await allPackageJsonTexts(dir)).toStrictEqual(before); diff --git a/test/cli/install/bun-link.test.ts b/test/cli/install/bun-link.test.ts index 8a937dad63fd..99f427e7f5e4 100644 --- a/test/cli/install/bun-link.test.ts +++ b/test/cli/install/bun-link.test.ts @@ -1,18 +1,20 @@ import { file, spawn } from "bun"; -import { afterAll, afterEach, beforeAll, beforeEach, expect, it } from "bun:test"; -import { access, mkdir, writeFile } from "fs/promises"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test"; +import { existsSync } from "fs"; +import { access, mkdir, readlink, rm, writeFile } from "fs/promises"; import { bunExe, bunEnv as env, isWindows, readdirSorted, runBunInstall, + tempDir, tmpdirSync, toBeValidBin, toHaveBins, } from "harness"; import { basename, join } from "path"; -import { dummyAfterAll, dummyAfterEach, dummyBeforeAll, dummyBeforeEach, package_dir } from "./dummy.registry"; +import { dummyAfterAll, dummyAfterEach, dummyBeforeAll, dummyBeforeEach, getPort, package_dir } from "./dummy.registry"; beforeAll(dummyBeforeAll); afterAll(dummyAfterAll); @@ -471,3 +473,311 @@ it("should link dependency without crashing", async () => { // This should fail with a non-zero exit code. expect(await exited4).toBe(1); }); + +// https://github.com/oven-sh/bun/issues/4719 +describe.each(["hoisted", "isolated"])("link: with a filesystem path (%s)", linker => { + async function setLinker() { + await writeFile( + join(package_dir, "bunfig.toml"), + `[install]\ncache = false\nregistry = "http://localhost:${getPort()}/"\nsaveTextLockfile = true\nlinker = "${linker}"\n`, + ); + } + + async function checkLink(dep: string, expected: { name: string; version: string }) { + await setLinker(); + const { out, err } = await runBunInstall(env, package_dir); + expect(err).not.toContain("not linked"); + if (linker === "hoisted") expect(out).toContain(`+ ${expected.name}@link:`); + + const target = await readlink(join(package_dir, "node_modules", ...expected.name.split("/"))); + expect(target.replaceAll("\\", "/")).toContain(basename(dep)); + expect(await file(join(package_dir, "node_modules", expected.name, "package.json")).json()).toEqual(expected); + + const second = await runBunInstall(env, package_dir, { frozenLockfile: true }); + expect(second.err).not.toContain("Saved lockfile"); + } + + it("resolves a ./relative path", async () => { + await mkdir(join(package_dir, "lib", "mypkg"), { recursive: true }); + await writeFile( + join(package_dir, "lib", "mypkg", "package.json"), + JSON.stringify({ name: "mypkg", version: "1.0.0" }), + ); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ + name: "root", + dependencies: { mypkg: "link:./lib/mypkg" }, + }), + ); + await checkLink("./lib/mypkg", { name: "mypkg", version: "1.0.0" }); + }); + + it("treats a bare relative path (no ./) as a path, and stores it as ./", async () => { + await mkdir(join(package_dir, "lib", "bare"), { recursive: true }); + await writeFile( + join(package_dir, "lib", "bare", "package.json"), + JSON.stringify({ name: "bare", version: "1.0.0" }), + ); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ name: "root", dependencies: { bare: "link:lib/bare" } }), + ); + await checkLink("lib/bare", { name: "bare", version: "1.0.0" }); + expect(await file(join(package_dir, "bun.lock")).text()).toContain('"bare@link:./lib/bare"'); + }); + + it("resolves a ../relative path", async () => { + await mkdir(join(link_dir, "sibling"), { recursive: true }); + await writeFile( + join(link_dir, "sibling", "package.json"), + JSON.stringify({ name: "sibling-pkg", version: "2.0.0" }), + ); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ + name: "root", + dependencies: { "sibling-pkg": `link:${join("..", basename(link_dir), "sibling").replaceAll("\\", "/")}` }, + }), + ); + await checkLink("sibling", { name: "sibling-pkg", version: "2.0.0" }); + }); + + it("resolves a scoped package path", async () => { + await mkdir(join(package_dir, "packages", "scoped"), { recursive: true }); + await writeFile( + join(package_dir, "packages", "scoped", "package.json"), + JSON.stringify({ name: "@scope/pkg", version: "3.0.0" }), + ); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ + name: "root", + dependencies: { "@scope/pkg": "link:./packages/scoped" }, + }), + ); + await checkLink("./packages/scoped", { name: "@scope/pkg", version: "3.0.0" }); + }); + + it("resolves an absolute path", async () => { + await mkdir(join(link_dir, "abspkg"), { recursive: true }); + await writeFile(join(link_dir, "abspkg", "package.json"), JSON.stringify({ name: "abspkg", version: "4.0.0" })); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ + name: "root", + dependencies: { abspkg: `link:${join(link_dir, "abspkg").replaceAll("\\", "/")}` }, + }), + ); + await checkLink("abspkg", { name: "abspkg", version: "4.0.0" }); + }); + + it("resolves relative to a workspace member", async () => { + await mkdir(join(package_dir, "packages", "foo", "local"), { recursive: true }); + await writeFile( + join(package_dir, "packages", "foo", "local", "package.json"), + JSON.stringify({ name: "localpkg", version: "5.0.0" }), + ); + await writeFile( + join(package_dir, "packages", "foo", "package.json"), + JSON.stringify({ name: "foo", dependencies: { localpkg: "link:./local" } }), + ); + await writeFile(join(package_dir, "package.json"), JSON.stringify({ name: "root", workspaces: ["packages/*"] })); + await setLinker(); + await runBunInstall(env, package_dir); + + const linked = + linker === "hoisted" + ? join(package_dir, "node_modules", "localpkg") + : join(package_dir, "packages", "foo", "node_modules", "localpkg"); + expect(await file(join(linked, "package.json")).json()).toEqual({ name: "localpkg", version: "5.0.0" }); + expect((await readlink(linked)).replaceAll("\\", "/")).toContain("local"); + + const second = await runBunInstall(env, package_dir, { frozenLockfile: true }); + expect(second.err).not.toContain("Saved lockfile"); + }); + + it("errors with the path when package.json is missing", async () => { + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ + name: "root", + dependencies: { missing: "link:./does-not-exist" }, + }), + ); + await setLinker(); + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd: package_dir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [, stderr, exited] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toContain('Could not find package.json at "./does-not-exist"'); + expect(stderr).not.toContain("is not linked"); + expect(stderr).not.toContain("bun link my-pkg-name-from-package-json"); + expect(exited).toBe(1); + }); +}); + +// A path-form link: declared by a package that is not the root or a workspace +// (here: a file: dependency of the project) may only point inside the project. +// Same rule and same fixtures as the transitive file: tests in bun-install.test.ts. +describe.each(["hoisted", "isolated"])("transitive path-form link: (%s)", linker => { + // Called inside each test: the dummy registry only exists after beforeAll. + const bunfig = () => `[install]\ncache = false\nregistry = "http://localhost:${getPort()}/"\nlinker = "${linker}"\n`; + const refusal = "only the root package.json, a workspace, or an override may link to a path outside the project"; + + // Every place either linker could have put the link (node_modules/, + // a nested node_modules, or an isolated store entry), dangling links included. + function linksNamed(project: string, name: string): string[] { + const node_modules = join(project, "node_modules"); + if (!existsSync(node_modules)) return []; + return Array.from( + new Bun.Glob(`**/${name}`).scanSync({ cwd: node_modules, onlyFiles: false, dot: true, followSymlinks: false }), + ); + } + + async function install(cwd: string, ...args: string[]) { + await using proc = spawn({ + cmd: [bunExe(), "install", ...args], + cwd, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { out, err, exitCode }; + } + + it("is refused when it escapes the project (resolve)", async () => { + using dir = tempDir("transitive-link-escape", { + "secret/package.json": JSON.stringify({ name: "loot", version: "1.0.0" }), + "project/bunfig.toml": bunfig(), + "project/package.json": JSON.stringify({ name: "my-app", dependencies: { evil: "file:./evil" } }), + "project/evil/package.json": JSON.stringify({ + name: "evil", + version: "1.0.0", + dependencies: { loot: "link:../../secret" }, + }), + }); + const project = join(String(dir), "project"); + + const { err, exitCode } = await install(project); + expect(err).toContain(refusal); + expect(err).not.toContain("Could not find package.json"); + expect(exitCode).toBe(1); + expect(linksNamed(project, "loot")).toEqual([]); + }); + + it("is refused when it escapes the project (existing lockfile)", async () => { + // The lockfile already carries the escaping resolution, so resolution is + // skipped and the installer is what has to refuse it. + using dir = tempDir("transitive-link-escape-lock", { + "secret/package.json": JSON.stringify({ name: "loot", version: "1.0.0" }), + "project/bunfig.toml": bunfig(), + "project/package.json": JSON.stringify({ name: "my-app", dependencies: { evil: "file:./evil" } }), + "project/evil/package.json": JSON.stringify({ + name: "evil", + version: "1.0.0", + dependencies: { loot: "link:../../secret" }, + }), + "project/bun.lock": JSON.stringify({ + lockfileVersion: 1, + workspaces: { "": { name: "my-app", dependencies: { evil: "file:./evil" } } }, + packages: { + evil: ["evil@file:evil", { dependencies: { loot: "link:../../secret" } }], + loot: ["loot@link:../secret", {}], + }, + }), + }); + const project = join(String(dir), "project"); + + const { err, exitCode } = await install(project); + expect(err).toContain(refusal); + expect(exitCode).toBe(1); + expect(linksNamed(project, "loot")).toEqual([]); + }); + + it("is installed when it stays inside the project", async () => { + using dir = tempDir("transitive-link-inside", { + "bunfig.toml": bunfig(), + "package.json": JSON.stringify({ name: "my-app", dependencies: { lib: "file:./vendor/lib" } }), + "vendor/lib/package.json": JSON.stringify({ + name: "lib", + version: "1.0.0", + main: "index.js", + dependencies: { nested: "link:../nested" }, + }), + "vendor/lib/index.js": `module.exports = require("nested");`, + "vendor/nested/package.json": JSON.stringify({ name: "nested", version: "1.0.0", main: "index.js" }), + "vendor/nested/index.js": `module.exports = "it worked";`, + }); + const project = String(dir); + + for (const args of [[], ["--frozen-lockfile"]]) { + await rm(join(project, "node_modules"), { recursive: true, force: true }); + const { err, exitCode } = await install(project, ...args); + expect(err).not.toContain("error:"); + expect(exitCode).toBe(0); + + await using run = spawn({ + cmd: [bunExe(), "-e", `console.log(require("lib"))`], + cwd: project, + env, + stdout: "pipe", + stderr: "pipe", + }); + const [runOut, runErr, runExit] = await Promise.all([run.stdout.text(), run.stderr.text(), run.exited]); + expect(runErr).toBe(""); + expect(runOut.trim()).toBe("it worked"); + expect(runExit).toBe(0); + } + // The declaring package's importer-relative target is stored root-relative. + expect(await file(join(project, "bun.lock")).text()).toContain('"nested@link:./vendor/nested"'); + }); + + for (const field of ["overrides", "resolutions"]) { + it(`may escape the project when the target comes from root "${field}"`, async () => { + using dir = tempDir("transitive-link-override", { + "shared/package.json": JSON.stringify({ name: "shared", version: "1.0.0", main: "index.js" }), + "shared/index.js": `module.exports = "shared";`, + "project/bunfig.toml": bunfig(), + "project/package.json": JSON.stringify({ + name: "my-app", + dependencies: { "pkg-a": "file:./pkg-a" }, + [field]: { shared: "link:../shared" }, + }), + "project/pkg-a/package.json": JSON.stringify({ + name: "pkg-a", + version: "1.0.0", + main: "index.js", + dependencies: { shared: "1.0.0" }, + }), + "project/pkg-a/index.js": `module.exports = require("shared");`, + }); + const project = join(String(dir), "project"); + + for (const args of [[], ["--frozen-lockfile"]]) { + await rm(join(project, "node_modules"), { recursive: true, force: true }); + const { err, exitCode } = await install(project, ...args); + expect(err).not.toContain(refusal); + expect(err).not.toContain("error:"); + expect(exitCode).toBe(0); + + await using run = spawn({ + cmd: [bunExe(), "-e", `console.log(require("pkg-a"))`], + cwd: project, + env, + stdout: "pipe", + stderr: "pipe", + }); + const [runOut, runErr, runExit] = await Promise.all([run.stdout.text(), run.stderr.text(), run.exited]); + expect(runErr).toBe(""); + expect(runOut.trim()).toBe("shared"); + expect(runExit).toBe(0); + } + }); + } +}); diff --git a/test/cli/install/migration/pnpm-lock-migration.test.ts b/test/cli/install/migration/pnpm-lock-migration.test.ts index 039e8727b2fc..136880c8cba2 100644 --- a/test/cli/install/migration/pnpm-lock-migration.test.ts +++ b/test/cli/install/migration/pnpm-lock-migration.test.ts @@ -261,6 +261,73 @@ snapshots: expect(packageJson).toMatchSnapshot("workspace-pnpm-migration-package-json"); }); + test("file: dependency of a workspace member is stored relative to the root", async () => { + // pnpm writes `link:../utils` relative to the importer; bun.lock stores it + // relative to the root, otherwise the install below looks for /../utils. + await using tmpDir = tempDir("pnpm-migrate-importer-link", { + "package.json": JSON.stringify({ name: "root", private: true, workspaces: ["packages/app"] }), + "packages/app/package.json": JSON.stringify({ name: "app", dependencies: { utils: "file:../utils" } }), + "packages/utils/package.json": JSON.stringify({ name: "utils", version: "1.0.0", main: "index.js" }), + "packages/utils/index.js": `module.exports = "utils";`, + "pnpm-lock.yaml": `lockfileVersion: '9.0' + +importers: + + .: {} + + packages/app: + dependencies: + utils: + specifier: file:../utils + version: link:../utils +`, + }); + + await using migrate = Bun.spawn({ + cmd: [bunExe(), "pm", "migrate"], + cwd: tmpDir, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [, migrateStderr, migrateExit] = await Promise.all([ + migrate.stdout.text(), + migrate.stderr.text(), + migrate.exited, + ]); + expect(migrateStderr).toContain("migrated lockfile from pnpm-lock.yaml"); + expect(migrateExit).toBe(0); + + expect(fs.readFileSync(join(tmpDir, "bun.lock"), "utf8")).toContain('"utils@link:./packages/utils"'); + + await using install = Bun.spawn({ + cmd: [bunExe(), "install", "--frozen-lockfile"], + cwd: tmpDir, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [, installStderr, installExit] = await Promise.all([ + install.stdout.text(), + install.stderr.text(), + install.exited, + ]); + expect(installStderr).not.toContain("error:"); + expect(installExit).toBe(0); + + await using run = Bun.spawn({ + cmd: [bunExe(), "-e", `console.log(require("utils"))`], + cwd: join(tmpDir, "packages", "app"), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [runOut, runErr, runExit] = await Promise.all([run.stdout.text(), run.stderr.text(), run.exited]); + expect(runErr).toBe(""); + expect(runOut.trim()).toBe("utils"); + expect(runExit).toBe(0); + }); + test("pnpm with npm protocol aliases", async () => { await using tmpDir = tempDir("pnpm-migrate-npm-aliases", { "package.json": JSON.stringify(