diff --git a/docs/pm/cli/add.mdx b/docs/pm/cli/add.mdx index 15b4b286eb8e..74570b804982 100644 --- a/docs/pm/cli/add.mdx +++ b/docs/pm/cli/add.mdx @@ -231,6 +231,8 @@ bun add zod@https://registry.npmjs.org/zod/-/zod-3.21.4.tgz } ``` +A tarball URL can carry credentials, such as `https://user:password@example.com/zod-3.21.4.tgz`. Bun sends them as an `Authorization: Basic` header and requests the URL without them, like npm. The URL, credentials included, is written to `package.json` and to the lockfile. + --- diff --git a/docs/pm/isolated-installs.mdx b/docs/pm/isolated-installs.mdx index c34d0cdf3c26..0f322aa295ba 100644 --- a/docs/pm/isolated-installs.mdx +++ b/docs/pm/isolated-installs.mdx @@ -85,6 +85,8 @@ node_modules/ └── package-name -> .bun/package@1.0.0/node_modules/package # Symlinks ``` +The part of a store directory name after `@` is the package's resolution. For a registry package, that is its version. For a folder, tarball, git, or GitHub dependency, Bun derives it from the path or URL (plus the commit), so it can be long. Bun writes at most 80 bytes of it: it cuts a longer resolution to at most 63 bytes and appends `+` and 16 hex digits derived from the full resolution. The path of a package inside the store is therefore at most `34 + 2 * + 80` characters longer than your project directory (17 more when the package has peer dependencies). This matters on Windows, where a path longer than 260 characters still works for Bun itself, but not as the working directory of the package's lifecycle scripts. + ### Resolution algorithm 1. **Central store** — Bun installs all packages in `node_modules/.bun/package@version/` directories diff --git a/src/install/NetworkTask.rs b/src/install/NetworkTask.rs index 615177ac1646..620edd60a07f 100644 --- a/src/install/NetworkTask.rs +++ b/src/install/NetworkTask.rs @@ -408,6 +408,40 @@ fn count_auth(header_builder: &mut HeaderBuilder, scope: &npm::registry::Scope) header_builder.count("npm-auth-type", "legacy"); } +/// Splits `http://user:pass@host/pkg.tgz` into `user:pass` and +/// `http://host/pkg.tgz`. `None` when the authority has no `@`; the `@` of a +/// scoped package in the path (`/@scope/pkg/-/pkg.tgz`) is not one. +fn split_url_userinfo(url: &[u8]) -> Option<(&[u8], Box<[u8]>)> { + let authority_start = strings::index_of(url, b"://")? + b"://".len(); + let rest = &url[authority_start..]; + let authority = &rest[..strings::index_of_any(rest, b"/?#").unwrap_or(rest.len())]; + let at = strings::last_index_of_char(authority, b'@')?; + + let mut without_userinfo = Vec::with_capacity(url.len() - (at + 1)); + without_userinfo.extend_from_slice(&url[..authority_start]); + without_userinfo.extend_from_slice(&rest[at + 1..]); + Some((&rest[..at], without_userinfo.into_boxed_slice())) +} + +/// `Basic base64(userinfo)`, the header npm sends for credentials embedded in a +/// tarball URL: minipass-fetch (`getNodeRequestOptions` in `lib/request.js`) +/// hands the URL's `username:password` to node's `auth` option as is, so +/// nothing is percent-decoded here either, and a userinfo without a `:` is a +/// username with an empty password. +fn basic_authorization_from_userinfo(userinfo: &[u8]) -> Vec { + const SCHEME: &[u8] = b"Basic "; + let mut user_pass = Vec::with_capacity(userinfo.len() + 1); + user_pass.extend_from_slice(userinfo); + if !strings::contains_char(userinfo, b':') { + user_pass.push(b':'); + } + let mut value = vec![0u8; SCHEME.len() + bun_core::base64::encode_len(&user_pass)]; + value[..SCHEME.len()].copy_from_slice(SCHEME); + let encoded_len = bun_core::base64::encode(&mut value[SCHEME.len()..], &user_pass); + value.truncate(SCHEME.len() + encoded_len); + value +} + #[derive(thiserror::Error, Debug, strum::IntoStaticStr)] pub enum ForManifestError { #[error("OutOfMemory")] @@ -784,6 +818,21 @@ impl NetworkTask { return Err(ForTarballError::InvalidURL); } + // `"dep": "https://user:pass@host/dep.tgz"`: the credentials become a + // header, as npm sends them, and the URL is requested without them. + // They cannot stay in the URL: `bun_url` keeps the userinfo in `origin`, + // and the HTTP client compares origins to decide whether `Authorization` + // follows a redirect, so a redirect to the same host would lose it. + let url_authorization: Option> = match split_url_userinfo(&self.url_buf) { + Some((userinfo, url_without_userinfo)) => { + let value = + (!userinfo.is_empty()).then(|| basic_authorization_from_userinfo(userinfo)); + self.url_buf = url_without_userinfo; + value + } + None => None, + }; + // Only attach the registry `Authorization` header when the tarball URL // origin matches the configured registry scope origin. The npm manifest // is registry-controlled, so a malicious registry could otherwise point @@ -815,9 +864,23 @@ impl NetworkTask { count_auth(&mut header_builder, scope); } + // Same precedence as npm, where node derives `Authorization` from the + // URL only when the request does not carry one already: credentials + // configured for the registry win over the ones embedded in the URL. + let url_authorization = match url_authorization { + Some(value) if header_builder.header_count == 0 => { + header_builder.count("Authorization", &value); + Some(value) + } + _ => None, + }; + let header_buf: &'static [u8] = if header_builder.header_count > 0 { header_builder.allocate()?; - append_auth(&mut header_builder, scope); + match &url_authorization { + Some(value) => header_builder.append("Authorization", value), + None => append_auth(&mut header_builder, scope), + } debug_assert_eq!(header_builder.content.len, header_builder.content.cap); self.header_buf = header_builder.content.move_to_slice(); // SAFETY: `self.header_buf` outlives the request; it is freed when the slot returns to the pool. diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index db8f7c3058d3..72aa9715ba81 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -1903,31 +1903,18 @@ fn enqueue_local_tarball( // other dependencies (e.g. `appendPackage` / `StringBuilder.allocate` // in `Package.fromNPM`). let mut abs_buf = PathBuffer::uninit(); - let (tarball_path, normalize): (&[u8], bool) = 'tarball_path: { - let workspace_pkg_id = this - .lockfile - .get_workspace_pkg_if_workspace_dep(dependency_id); - if workspace_pkg_id == invalid_package_id { - break 'tarball_path (path, true); - } - - let workspace_res = this.lockfile.packages.items_resolution()[workspace_pkg_id as usize]; - if workspace_res.tag != ResolutionTag::Workspace { - break 'tarball_path (path, true); - } - - // Construct an absolute path to the tarball. - // Normally tarball paths are always relative to the root directory, but if a - // workspace depends on a tarball path, it should be relative to the workspace. - let workspace_str = *workspace_res.workspace(); - let workspace_path = workspace_str.slice(this.lockfile.buffers.string_bytes.as_slice()); - let joined = Path::resolve_path::join_abs_string_buf::( - FileSystem::instance().top_level_dir(), - &mut abs_buf, - &[workspace_path, path], - ); - break 'tarball_path (joined, false); - }; + let (tarball_path, normalize): (&[u8], bool) = + match local_tarball_base_dir(&this.lockfile, dependency_id, path) { + None => (path, true), + Some(base_dir) => ( + Path::resolve_path::join_abs_string_buf::( + FileSystem::instance().top_level_dir(), + &mut abs_buf, + &[base_dir, path], + ), + false, + ), + }; // Build the `Task` value *before* claiming a hive slot — the `.expect()`s // below can unwind, and `Task` carries drop glue. See `enqueue_git_clone`. @@ -1978,6 +1965,33 @@ fn enqueue_local_tarball( unsafe { &raw mut (*task).threadpool_task } } +/// The workspace or `file:` folder directory that `path` is relative to; `None` is the top-level dir. +fn local_tarball_base_dir<'a>( + lockfile: &'a Lockfile::Lockfile, + dependency_id: DependencyID, + path: &[u8], +) -> Option<&'a [u8]> { + let declared = &lockfile.buffers.dependencies[dependency_id as usize].version; + let declared_by_parent = declared.tag == dependency::version::Tag::Tarball + && matches!( + &declared.tarball().uri, + dependency::tarball::Uri::Local(declared_path) if lockfile.str(declared_path) == path + ); + if !declared_by_parent { + // Overrides, resolutions and catalogs are all written in the root package.json. + return None; + } + + let declarer = lockfile.get_parent_pkg_of_dependency(dependency_id)?; + let declarer_res = &lockfile.packages.items_resolution()[declarer as usize]; + let base_dir = match declarer_res.tag { + ResolutionTag::Workspace => declarer_res.workspace(), + ResolutionTag::Folder => declarer_res.folder(), + _ => return None, + }; + Some(lockfile.str(base_dir)) +} + fn update_name_and_name_hash_from_version_replacement( lockfile: &Lockfile::Lockfile, original_name: SemverString, diff --git a/src/install/PackageManagerTask.rs b/src/install/PackageManagerTask.rs index 18d868675bb0..82d62768f65d 100644 --- a/src/install/PackageManagerTask.rs +++ b/src/install/PackageManagerTask.rs @@ -699,12 +699,7 @@ pub struct GitCheckoutRequest { pub struct LocalTarballRequest { pub(crate) tarball: ExtractTarball, - /// Path to read the tarball from. May be the same as `tarball.url` (when - /// `normalize` is true) or an absolute path joined with a workspace - /// directory. Computed on the main thread in `enqueueLocalTarball` because - /// resolving it requires reading `lockfile.packages` / `string_bytes`, - /// which can be reallocated concurrently by the main thread while this - /// task runs on a ThreadPool worker. + /// Resolved by `enqueue_local_tarball` on the main thread; the worker must not read the lockfile. pub(crate) tarball_path: StringOrTinyString, /// When true, `tarball_path` is a user-provided path resolved relative to /// cwd. When false, it is already an absolute path. diff --git a/src/install/isolated_install/Store.rs b/src/install/isolated_install/Store.rs index 25be802357ff..1dbe118ce2b0 100644 --- a/src/install/isolated_install/Store.rs +++ b/src/install/isolated_install/Store.rs @@ -7,6 +7,7 @@ use bstr::BStr; use bun_alloc::AllocError; use bun_collections::{ArrayHashMap, MultiArrayList}; use bun_semver::String as SemverString; +use bun_wyhash::Wyhash; use crate::lockfile::{Lockfile, package}; use crate::{Dependency, DependencyID, INVALID_DEPENDENCY_ID, PackageID, Resolution}; @@ -351,7 +352,58 @@ pub mod entry { } } + /// Max bytes of resolution (the text after `name@`; a folder path or git/tarball + /// URL otherwise makes it arbitrarily long) in an entry name. The package directory + /// under the entry is the cwd of its lifecycle scripts, which Windows' `CreateProcess` + /// rejects past MAX_PATH (ENOENT) although bun's own file I/O accepts such paths. + /// 80 keeps versions and `github+owner+repo+` verbatim. + const MAX_RESOLUTION_LEN: usize = 80; + /// Longer resolutions become `+<16 hex wyhash of the whole text>`, + /// `MAX_RESOLUTION_LEN` bytes at most. + const CUT_RESOLUTION_LEN: usize = MAX_RESOLUTION_LEN - "+".len() - 16; + + /// The first `MAX_RESOLUTION_LEN` bytes written, plus the length and hash of all of them. + struct ResolutionSink { + buf: [u8; MAX_RESOLUTION_LEN], + len: usize, + hasher: Wyhash, + } + + impl fmt::Write for ResolutionSink { + fn write_str(&mut self, s: &str) -> fmt::Result { + let bytes = s.as_bytes(); + if let Some(room) = self.buf.get_mut(self.len..) { + let n = bytes.len().min(room.len()); + room[..n].copy_from_slice(&bytes[..n]); + } + self.len += bytes.len(); + self.hasher.update(bytes); + Ok(()) + } + } + + fn write_resolution(f: &mut fmt::Formatter<'_>, resolution: fmt::Arguments<'_>) -> fmt::Result { + let mut sink = ResolutionSink { + buf: [0; MAX_RESOLUTION_LEN], + len: 0, + hasher: Wyhash::init(0), + }; + fmt::write(&mut sink, resolution)?; + + if sink.len <= MAX_RESOLUTION_LEN { + return f.write_str(bun_core::str_utf8(&sink.buf[..sink.len]).ok_or(fmt::Error)?); + } + + let mut cut = CUT_RESOLUTION_LEN; + while !bun_core::strings::is_on_char_boundary(&sink.buf, cut) { + cut -= 1; + } + f.write_str(bun_core::str_utf8(&sink.buf[..cut]).ok_or(fmt::Error)?)?; + write!(f, "+{:016x}", sink.hasher.final_()) + } + /// `name@version` (or `name@file+path` / `name@root`) without the `+peerhash` suffix. + /// The resolution part is bounded by [`MAX_RESOLUTION_LEN`]. pub struct StoreKeyFormatter<'a> { name: SemverString, resolution: &'a Resolution, @@ -380,20 +432,15 @@ pub mod entry { } crate::resolution::Tag::Folder => { let folder = *pkg_res.folder(); - write!( + write!(f, "{}@", pkg_name.fmt_store_path(string_buf))?; + write_resolution( f, - "{}@file+{}", - pkg_name.fmt_store_path(string_buf), - folder.fmt_store_path(string_buf), + format_args!("file+{}", folder.fmt_store_path(string_buf)), ) } _ => { - write!( - f, - "{}@{}", - pkg_name.fmt_store_path(string_buf), - pkg_res.fmt_store_path(string_buf), - ) + write!(f, "{}@", pkg_name.fmt_store_path(string_buf))?; + write_resolution(f, format_args!("{}", pkg_res.fmt_store_path(string_buf))) } } } diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index da5691de7ee9..2b63cb6b73d2 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -782,6 +782,16 @@ impl Lockfile { self.get_workspace_pkg_if_workspace_dep(id) != invalid_package_id } + /// `None` for the edges `enqueue_dependency_to_root` appends outside of any package. + pub(crate) fn get_parent_pkg_of_dependency(&self, id: DependencyID) -> Option { + for (pkg_id, dependencies) in self.packages.items_dependencies().iter().enumerate() { + if dependencies.contains(id) { + return Some(PackageID::try_from(pkg_id).expect("int cast")); + } + } + None + } + pub(crate) fn get_workspace_pkg_if_workspace_dep(&self, id: DependencyID) -> PackageID { let packages = self.packages.slice(); let resolutions = packages.items_resolution(); diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index 583fceefe6c7..43017dd2293f 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -854,6 +854,223 @@ describe.concurrent("bun-install", () => { expect(exitCode).toBe(0); }); + // A tarball URL with credentials in it is downloaded the way npm downloads + // it: the userinfo becomes `Authorization: Basic base64(user:pass)` and the + // request goes to the URL without it (`NetworkTask::for_tarball`). + describe.concurrent("credentials embedded in a tarball URL", () => { + const tgz = join(import.meta.dir, "registry", "packages", "no-deps", "no-deps-1.0.0.tgz"); + const tarballPath = "/cdn/no-deps-1.0.0.tgz"; + const basic = (userPass: string) => `Basic ${Buffer.from(userPass).toString("base64")}`; + const installed = { + stdout: expect.stringContaining("1 package installed"), + stderr: expect.stringContaining("Saved lockfile"), + exitCode: 0, + }; + + type Received = { url: string; authorization: string | null }; + + function recording(received: Received[], handler: (req: Request, server: { port: number }) => Response) { + return (req: Request, server: { port: number }) => { + received.push({ url: req.url, authorization: req.headers.get("authorization") }); + return handler(req, server); + }; + } + + // Serves `tgz` to `.tgz` requests carrying exactly `authorization` and + // answers 401 to the others. A request under `/redirect/` is first + // redirected to `redirectTo`, or to the same file under `/cdn/`. + function serveTarball(received: Received[], authorization: string | null, redirectTo?: string) { + return Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch: recording(received, (req, server) => { + const { pathname } = new URL(req.url); + if (pathname.startsWith("/redirect/")) { + const name = pathname.slice("/redirect/".length); + return Response.redirect(redirectTo ?? `http://127.0.0.1:${server.port}/cdn/${name}`, 302); + } + if (req.headers.get("authorization") !== authorization) { + return new Response("unauthorized", { status: 401 }); + } + return new Response(file(tgz)); + }), + }); + } + + // `bun install` of a project whose only dependency `no-deps` is `dependency`. + async function install(dependency: string, files: Record = {}, args: string[] = []) { + using dir = tempDir("tarball-url-credentials", { + "package.json": JSON.stringify({ name: "app", version: "1.0.0", dependencies: { "no-deps": dependency } }), + ...files, + }); + await using proc = spawn({ + cmd: [bunExe(), "install", ...args], + cwd: String(dir), + env: { ...env, BUN_INSTALL_CACHE_DIR: join(String(dir), ".cache") }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + // Each row is the userinfo of the dependency URL and the `user:pass` the + // header must encode. It is sent as written: like npm (checked with npm + // 11), a missing password is sent as an empty one and percent-encoding is + // left alone. npm would percent-encode the second colon of the last row + // because it serializes the URL first. + it.each([ + ["a username and a password", "carol:s3cret", "carol:s3cret", []], + ["a username and a password, isolated linker", "carol:s3cret", "carol:s3cret", ["--linker", "isolated"]], + ["a username only", "carol", "carol:", []], + ["a password only", ":s3cret", ":s3cret", []], + ["a percent-encoded password", "carol:s3%40cret", "carol:s3%40cret", []], + ["a password containing a colon", "carol:s3:cret", "carol:s3:cret", []], + ])("sends %s as Basic authorization", async (_, userinfo, userPass, args) => { + const authorization = basic(userPass); + const received: Received[] = []; + await using server = serveTarball(received, authorization); + + const result = await install(`http://${userinfo}@127.0.0.1:${server.port}${tarballPath}`, {}, args); + + expect({ received, ...result }).toEqual({ + received: [{ url: `http://127.0.0.1:${server.port}${tarballPath}`, authorization }], + ...installed, + }); + }); + + it("does not take the @ of a scoped package path for credentials", async () => { + const received: Received[] = []; + await using server = serveTarball(received, null); + const scopedPath = "/@scope/no-deps/-/no-deps-1.0.0.tgz"; + + const result = await install(`http://127.0.0.1:${server.port}${scopedPath}`); + + expect({ received, ...result }).toEqual({ + received: [{ url: `http://127.0.0.1:${server.port}${scopedPath}`, authorization: null }], + ...installed, + }); + }); + + it("keeps the credentials across a redirect within the host", async () => { + const received: Received[] = []; + await using server = serveTarball(received, basic("carol:s3cret")); + + const result = await install(`http://carol:s3cret@127.0.0.1:${server.port}/redirect/no-deps-1.0.0.tgz`); + + expect({ received, ...result }).toEqual({ + received: [ + { url: `http://127.0.0.1:${server.port}/redirect/no-deps-1.0.0.tgz`, authorization: basic("carol:s3cret") }, + { url: `http://127.0.0.1:${server.port}${tarballPath}`, authorization: basic("carol:s3cret") }, + ], + ...installed, + }); + }); + + it("drops the credentials on a redirect to another host", async () => { + // The same machine, reached under a hostname other than the one the + // credentials were written for. This host serves the tarball regardless. + const otherHostReceived: Received[] = []; + await using otherHost = Bun.serve({ + port: 0, + fetch: recording(otherHostReceived, () => new Response(file(tgz))), + }); + const received: Received[] = []; + await using server = serveTarball(received, null, `http://localhost:${otherHost.port}${tarballPath}`); + + const result = await install(`http://carol:s3cret@127.0.0.1:${server.port}/redirect/no-deps-1.0.0.tgz`); + + expect({ received, otherHostReceived, ...result }).toEqual({ + received: [ + { url: `http://127.0.0.1:${server.port}/redirect/no-deps-1.0.0.tgz`, authorization: basic("carol:s3cret") }, + ], + otherHostReceived: [{ url: `http://localhost:${otherHost.port}${tarballPath}`, authorization: null }], + ...installed, + }); + }); + + it("reports a rejected download by the URL without the credentials", async () => { + const received: Received[] = []; + await using server = serveTarball(received, basic("carol:s3cret")); + + const result = await install(`http://carol:wrong@127.0.0.1:${server.port}${tarballPath}`); + + expect({ received, ...result }).toEqual({ + received: [{ url: `http://127.0.0.1:${server.port}${tarballPath}`, authorization: basic("carol:wrong") }], + stdout: expect.stringContaining("bun install v1."), + stderr: expect.stringContaining(`error: GET http://127.0.0.1:${server.port}${tarballPath} - 401`), + exitCode: 1, + }); + }); + + // A registry whose manifest puts credentials into `dist.tarball`. As with + // npm, the credentials configured for the registry take precedence; the + // URL's are used when the registry has none. + describe.concurrent("in the dist.tarball URL of a registry manifest", () => { + const token = "registry-token"; + const distPath = "/no-deps/-/no-deps-1.0.0.tgz"; + + function serveRegistry(received: Received[], tarballAuthorization: string | null) { + return Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch: recording(received, (req, server) => { + const { pathname } = new URL(req.url); + if (pathname === "/no-deps") { + return Response.json({ + name: "no-deps", + "dist-tags": { latest: "1.0.0" }, + versions: { + "1.0.0": { + name: "no-deps", + version: "1.0.0", + dist: { tarball: `http://dist:d1st@127.0.0.1:${server.port}${distPath}` }, + }, + }, + }); + } + if (pathname === distPath && req.headers.get("authorization") === tarballAuthorization) { + return new Response(file(tgz)); + } + return new Response("unauthorized", { status: 401 }); + }), + }); + } + + it("sends the registry's credentials when it has some", async () => { + const received: Received[] = []; + await using registry = serveRegistry(received, `Bearer ${token}`); + + const result = await install("1.0.0", { + ".npmrc": `registry=http://127.0.0.1:${registry.port}/\n//127.0.0.1:${registry.port}/:_authToken=${token}\n`, + }); + + expect({ received, ...result }).toEqual({ + received: [ + { url: `http://127.0.0.1:${registry.port}/no-deps`, authorization: `Bearer ${token}` }, + { url: `http://127.0.0.1:${registry.port}${distPath}`, authorization: `Bearer ${token}` }, + ], + ...installed, + }); + }); + + it("sends the URL's credentials when the registry has none", async () => { + const received: Received[] = []; + await using registry = serveRegistry(received, basic("dist:d1st")); + + const result = await install("1.0.0", { ".npmrc": `registry=http://127.0.0.1:${registry.port}/\n` }); + + expect({ received, ...result }).toEqual({ + received: [ + { url: `http://127.0.0.1:${registry.port}/no-deps`, authorization: null }, + { url: `http://127.0.0.1:${registry.port}${distPath}`, authorization: basic("dist:d1st") }, + ], + ...installed, + }); + }); + }); + }); + it("--silent suppresses verbose output even when RUNNER_DEBUG is set", async () => { using dir = tempDir("install-silent-verbose", { "package.json": JSON.stringify({ name: "app", dependencies: {} }), @@ -10644,6 +10861,111 @@ it("fails when a transitive file: dependency's folder does not exist", async () expect(exitCode).toBe(1); }); +describe.concurrent("file: tarball declared by a file: folder dependency", () => { + // `bar-0.0.2.tgz` is planted at the path the declaration means and + // `baz-0.0.3.tgz` at the other candidate path, so reading the tarball + // relative to the wrong directory installs `baz` instead of failing with ENOENT. + const expected = readFileSync(join(import.meta.dir, "bar-0.0.2.tgz")); + const decoy = readFileSync(join(import.meta.dir, "baz-0.0.3.tgz")); + + const fixture = (root: object, lib: object, tarballs: Record) => ({ + "package.json": JSON.stringify({ + name: "my-app", + version: "1.0.0", + dependencies: { lib: "file:./vendor/lib" }, + ...root, + }), + "vendor/lib/package.json": JSON.stringify({ name: "lib", version: "1.0.0", main: "index.js", ...lib }), + "vendor/lib/index.js": `const pkg = require("tool/package.json"); module.exports = pkg.name + "@" + pkg.version;`, + ...tarballs, + }); + + // The first install resolves `tool` from vendor/lib/package.json and reads + // the tarball in the process. The second one starts from the lockfile with + // an empty cache, so it has to read the tarball again from the path recorded + // there; both have to pick the same file. + async function installAndRequireLib(projectDir: string, linker: "hoisted" | "isolated") { + const cacheDir = join(projectDir, ".bun-cache"); + const installed: string[] = []; + + for (const args of [["install"], ["install", "--frozen-lockfile"]]) { + await Promise.all([ + rm(join(projectDir, "node_modules"), { recursive: true, force: true }), + rm(cacheDir, { recursive: true, force: true }), + ]); + + await using install = spawn({ + cmd: [bunExe(), ...args, `--linker=${linker}`], + cwd: projectDir, + stdout: "pipe", + stderr: "pipe", + env: { ...env, BUN_INSTALL_CACHE_DIR: cacheDir }, + }); + const [installErr, installOut, installExit] = await Promise.all([ + install.stderr.text(), + install.stdout.text(), + install.exited, + ]); + expect(installErr).not.toContain("error:"); + expect(installOut).toContain("2 packages installed"); + expect(installExit).toBe(0); + + await using run = spawn({ + cmd: [bunExe(), "-e", `console.log(require("lib"))`], + cwd: projectDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [runErr, runOut, runExit] = await Promise.all([run.stderr.text(), run.stdout.text(), run.exited]); + expect(runErr).toBe(""); + expect(runExit).toBe(0); + installed.push(runOut.trim()); + } + + return { installed, lockfile: await file(join(projectDir, "bun.lock")).text() }; + } + + for (const linker of ["hoisted", "isolated"] as const) { + it(`is read relative to the folder (${linker} linker)`, async () => { + using dir = tempDir( + "folder-dep-tarball", + fixture( + {}, + { dependencies: { tool: "file:./tool.tgz" } }, + { "vendor/lib/tool.tgz": expected, "tool.tgz": decoy }, + ), + ); + + const { installed, lockfile } = await installAndRequireLib(String(dir), linker); + expect(installed).toEqual(["bar@0.0.2", "bar@0.0.2"]); + // The lockfile records the path as declared; the name in front of it is + // read from the tarball that was extracted. + expect(lockfile).toContain('"lib": ["lib@file:vendor/lib", { "dependencies": { "tool": "file:./tool.tgz" } }]'); + expect(lockfile).toContain('"tool": ["bar@./tool.tgz", {}, "sha512-'); + }); + } + + it("is read relative to the project when a root override supplies the path", async () => { + // `overrides` can only be written in the root package.json, so the path it + // contains means the project directory even though the dependency it is + // applied to is declared by vendor/lib/package.json. + using dir = tempDir( + "folder-dep-tarball-override", + fixture( + { overrides: { tool: "file:./tool.tgz" } }, + { dependencies: { tool: "^1.0.0" } }, + { "tool.tgz": expected, "vendor/lib/tool.tgz": decoy }, + ), + ); + + const { installed, lockfile } = await installAndRequireLib(String(dir), "hoisted"); + expect(installed).toEqual(["bar@0.0.2", "bar@0.0.2"]); + expect(lockfile).toContain('"lib": ["lib@file:vendor/lib", { "dependencies": { "tool": "^1.0.0" } }]'); + expect(lockfile).toContain('"tool": ["bar@./tool.tgz", {}, "sha512-'); + }); +}); + it("does not extract a local file: tarball outside the temp dir for a dependency alias containing '..' path segments", async () => { // For `file:` tarball dependencies, the dependency alias (the key in // `dependencies`) is used to derive the temporary extraction folder name. diff --git a/test/cli/install/bun-workspaces.test.ts b/test/cli/install/bun-workspaces.test.ts index 48903d67ec9f..bbfe6925b439 100644 --- a/test/cli/install/bun-workspaces.test.ts +++ b/test/cli/install/bun-workspaces.test.ts @@ -734,6 +734,47 @@ describe("relative tarballs", async () => { }, }); }); + // The tarball path is written in the root package.json, so it is relative to + // the root even though the dependency it ends up satisfying is declared by + // the workspace (#25835 for overrides, #25752 for catalogs). The workspace + // gets a different tarball at the same relative path, so reading it relative + // to the workspace installs `baz` instead of failing. + for (const [source, root, specifier] of [ + ["override", { overrides: { bar: "file:./bar.tgz" } }, "^0.0.2"], + ["catalog entry", { catalogs: { vendored: { bar: "file:./bar.tgz" } } }, "catalog:vendored"], + ] as const) { + test.concurrent(`from a root ${source} applied to a workspace dependency`, async () => { + using ctx = await setupTest(); + const { packageDir, env } = ctx; + await Promise.all([ + write(join(packageDir, "package.json"), JSON.stringify({ name: "foo", workspaces: ["pkgs/*"], ...root })), + write( + join(packageDir, "pkgs", "pkg1", "package.json"), + JSON.stringify({ name: "pkg1", dependencies: { bar: specifier } }), + ), + cp(join(import.meta.dir, "bar-0.0.2.tgz"), join(packageDir, "bar.tgz")), + ]); + await cp(join(import.meta.dir, "baz-0.0.3.tgz"), join(packageDir, "pkgs", "pkg1", "bar.tgz")); + + // The second install starts from the lockfile and an empty cache, so it + // reads the tarball again from the path recorded there. + for (const frozenLockfile of [false, true]) { + await Promise.all([ + rm(join(packageDir, "node_modules"), { recursive: true, force: true }), + rm(env.BUN_INSTALL_CACHE_DIR, { recursive: true, force: true }), + ]); + + await runBunInstall(env, packageDir, { frozenLockfile }); + + expect(await file(join(packageDir, "node_modules", "bar", "package.json")).json()).toEqual({ + name: "bar", + version: "0.0.2", + }); + } + + expect(await file(join(packageDir, "bun.lock")).text()).toContain('"bar": ["bar@./bar.tgz", {}, "sha512-'); + }); + } // Regression test for a data race where the `.local_tarball` task callback // (running on a ThreadPool worker) read `lockfile.packages` and diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index 59ddce9484b1..65c31c833473 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -1,10 +1,11 @@ import { file, spawn, write } from "bun"; import { afterAll, beforeAll, describe, expect, test } from "bun:test"; -import { existsSync, lstatSync, readlinkSync, statSync } from "fs"; +import { existsSync, lstatSync, readFileSync, readlinkSync, statSync } from "fs"; import { mkdir, readlink, rm, symlink } from "fs/promises"; import { VerdaccioRegistry, bunEnv, bunExe, readdirSorted, runBunInstall, tempDir } from "harness"; import { createRequire } from "module"; import { basename, dirname, join } from "path"; +import { pathToFileURL } from "url"; const registry = new VerdaccioRegistry(); @@ -31,6 +32,14 @@ function urlHash(url: string): string { return Bun.hash(url).toString(16).padStart(16, "0"); } +// `@`, with the resolution cut and hashed past MAX_RESOLUTION_LEN (see "long store entry names"). +const MAX_RESOLUTION_LEN = 80; +const CUT_RESOLUTION_LEN = MAX_RESOLUTION_LEN - "+".length - 16; +function storeEntryName(name: string, resolution: string): string { + if (resolution.length <= MAX_RESOLUTION_LEN) return `${name}@${resolution}`; + return `${name}@${resolution.slice(0, CUT_RESOLUTION_LEN)}+${urlHash(resolution)}`; +} + beforeAll(async () => { await registry.start(); }); @@ -2096,6 +2105,310 @@ test("tarball URL with query string resolves at runtime", async () => { expect(exitCode).toBe(0); }); +// The resolution part of a store entry name (`@`) embeds +// folder paths, tarball URLs and git URLs verbatim. `write_resolution` in +// src/install/isolated_install/Store.rs bounds it: a resolution longer than +// MAX_RESOLUTION_LEN bytes is cut and suffixed with `+` and the wyhash of the +// full text. Unbounded, the entry's absolute path passes MAX_PATH on Windows, +// where the package's lifecycle scripts then fail to spawn with ENOENT, and a +// resolution longer than NAME_MAX cannot be created at all. +describe("long store entry names", () => { + async function storeEntries(packageDir: string): Promise { + return (await readdirSorted(join(packageDir, "node_modules", ".bun"))).filter(entry => entry !== "node_modules"); + } + + test("a resolution at the limit is kept verbatim, a longer one is cut and hashed", async () => { + // `file+` plus this folder name is exactly MAX_RESOLUTION_LEN bytes; the + // other three folders are one byte longer. + const atLimit = Buffer.alloc(MAX_RESOLUTION_LEN - "file+".length, "a").toString(); + const pastLimit = `${atLimit}b`; + // Two resolutions of the same package that only differ after the cut + // point: the hash is what keeps their entries apart. + const sharedPrefix1 = `${atLimit}1`; + const sharedPrefix2 = `${atLimit}2`; + + const { packageJson, packageDir } = await registry.createTestDir({ + bunfigOpts: { linker: "isolated" }, + files: { + [`${atLimit}/package.json`]: JSON.stringify({ name: "at-limit", version: "1.0.0" }), + [`${pastLimit}/package.json`]: JSON.stringify({ name: "past-limit", version: "1.0.0" }), + [`${sharedPrefix1}/package.json`]: JSON.stringify({ name: "shared-prefix", version: "1.0.0" }), + [`${sharedPrefix2}/package.json`]: JSON.stringify({ name: "shared-prefix", version: "2.0.0" }), + }, + }); + await write( + packageJson, + JSON.stringify({ + name: "test-long-store-entry-names", + dependencies: { + "at-limit": `file:./${atLimit}`, + "past-limit": `file:./${pastLimit}`, + "shared-prefix-1": `file:./${sharedPrefix1}`, + "shared-prefix-2": `file:./${sharedPrefix2}`, + }, + }), + ); + + await runBunInstall(bunEnv, packageDir); + + const pastLimitEntry = storeEntryName("past-limit", `file+${pastLimit}`); + expect(pastLimitEntry).toMatch(/^past-limit@file\+a{58}\+[0-9a-f]{16}$/); + const expectedEntries = [ + `at-limit@file+${atLimit}`, + pastLimitEntry, + storeEntryName("shared-prefix", `file+${sharedPrefix1}`), + storeEntryName("shared-prefix", `file+${sharedPrefix2}`), + ].sort(); + expect(await storeEntries(packageDir)).toEqual(expectedEntries); + + expect(readlinkSync(join(packageDir, "node_modules", "past-limit"))).toBe( + join(".bun", pastLimitEntry, "node_modules", "past-limit"), + ); + expect( + await Promise.all( + ["at-limit", "past-limit", "shared-prefix-1", "shared-prefix-2"].map(alias => + file(join(packageDir, "node_modules", alias, "package.json")).json(), + ), + ), + ).toEqual([ + { name: "at-limit", version: "1.0.0" }, + { name: "past-limit", version: "1.0.0" }, + { name: "shared-prefix", version: "1.0.0" }, + { name: "shared-prefix", version: "2.0.0" }, + ]); + + // The cut name is a pure function of the resolution, so the next install + // finds the same entries again. + await runBunInstall(bunEnv, packageDir, { savesLockfile: false }); + expect(await storeEntries(packageDir)).toEqual(expectedEntries); + }); + + test("the peer hash is appended after the cut resolution", async () => { + const folder = Buffer.alloc(MAX_RESOLUTION_LEN, "p").toString(); + const { packageJson, packageDir } = await registry.createTestDir({ + bunfigOpts: { linker: "isolated" }, + files: { + [`${folder}/package.json`]: JSON.stringify({ + name: "has-peer", + version: "1.0.0", + peerDependencies: { "no-deps": "1.0.0" }, + }), + }, + }); + await write( + packageJson, + JSON.stringify({ + name: "test-long-store-entry-name-with-peer", + dependencies: { "has-peer": `file:./${folder}`, "no-deps": "1.0.0" }, + }), + ); + + await runBunInstall(bunEnv, packageDir); + + // `+7347ae2d86f1441a` is the hash of the peer set `no-deps@1.0.0`. + const entry = `${storeEntryName("has-peer", `file+${folder}`)}+7347ae2d86f1441a`; + expect(await storeEntries(packageDir)).toEqual([entry, "no-deps@1.0.0"]); + expect( + await file(join(packageDir, "node_modules", ".bun", entry, "node_modules", "no-deps", "package.json")).json(), + ).toEqual({ name: "no-deps", version: "1.0.0" }); + }); + + test("a cut that would split a multi-byte character backs up to the character boundary", async () => { + // `file+x` is 6 bytes and every character after it is 2 bytes wide, so + // byte 63 of the resolution falls inside a character and the cut ends at + // byte 62. Only the shape is asserted: how non-ASCII bytes are spelled in + // the name is a separate matter (#32304), the boundary handling is not. + const folder = `x${Buffer.alloc(40 * 2, "\u00e9").toString()}`; + const { packageJson, packageDir } = await registry.createTestDir({ + bunfigOpts: { linker: "isolated" }, + files: { [`${folder}/package.json`]: JSON.stringify({ name: "non-ascii", version: "1.0.0" }) }, + }); + await write( + packageJson, + JSON.stringify({ name: "test-non-ascii-store-entry-name", dependencies: { "non-ascii": `file:./${folder}` } }), + ); + + await runBunInstall(bunEnv, packageDir); + + const entries = await storeEntries(packageDir); + expect(entries).toEqual([expect.stringMatching(/^non-ascii@file\+x.*\+[0-9a-f]{16}$/)]); + const [entry] = entries; + const cutResolution = entry.slice("non-ascii@".length, -"+0123456789abcdef".length); + expect(Buffer.byteLength(cutResolution)).toBe(CUT_RESOLUTION_LEN - 1); + expect(readlinkSync(join(packageDir, "node_modules", "non-ascii"))).toBe( + join(".bun", entry, "node_modules", "non-ascii"), + ); + expect(await file(join(packageDir, "node_modules", "non-ascii", "package.json")).json()).toEqual({ + name: "non-ascii", + version: "1.0.0", + }); + }); + + test("a local tarball and a folder in a deep directory install when their paths are longer than NAME_MAX", async () => { + // Every directory on the way is short; only the resolution, which is the + // whole relative path (257 bytes here), would make the entry name longer + // than NAME_MAX. Unbounded, the install fails with ENAMETOOLONG. + const segment = Buffer.alloc(85, "d").toString(); + const deep = `${segment}/${segment}/${segment}`; + const { packageJson, packageDir } = await registry.createTestDir({ + bunfigOpts: { linker: "isolated" }, + files: { + [`${deep}/bar-0.0.2.tgz`]: readFileSync(join(import.meta.dir, "bar-0.0.2.tgz")), + [`${deep}/pkg/package.json`]: JSON.stringify({ name: "folder-pkg", version: "1.0.0" }), + }, + }); + await write( + packageJson, + JSON.stringify({ + name: "test-deep-local-tarball-and-folder", + dependencies: { + "bar": `file:./${deep}/bar-0.0.2.tgz`, + "folder-pkg": `file:./${deep}/pkg`, + }, + }), + ); + + await runBunInstall(bunEnv, packageDir); + + const tarballEntry = storeEntryName("bar", `.+${deep.replaceAll("/", "+")}+bar-0.0.2.tgz`); + const folderEntry = storeEntryName("folder-pkg", `file+${deep.replaceAll("/", "+")}+pkg`); + expect(tarballEntry).toMatch(/^bar@\.\+d{61}\+[0-9a-f]{16}$/); + expect(folderEntry).toMatch(/^folder-pkg@file\+d{58}\+[0-9a-f]{16}$/); + const expectedEntries = [tarballEntry, folderEntry].sort(); + expect(await storeEntries(packageDir)).toEqual(expectedEntries); + + const bunDir = join(packageDir, "node_modules", ".bun"); + expect( + await Promise.all([ + readlink(join(packageDir, "node_modules", "bar")), + readlink(join(packageDir, "node_modules", "folder-pkg")), + // The `.bun/node_modules` fallback directory links to the cut name too. + readlink(join(bunDir, "node_modules", "bar")), + file(join(packageDir, "node_modules", "bar", "package.json")).json(), + file(join(packageDir, "node_modules", "folder-pkg", "package.json")).json(), + ]), + ).toEqual([ + join(".bun", tarballEntry, "node_modules", "bar"), + join(".bun", folderEntry, "node_modules", "folder-pkg"), + join("..", tarballEntry, "node_modules", "bar"), + { name: "bar", version: "0.0.2" }, + { name: "folder-pkg", version: "1.0.0" }, + ]); + + await runBunInstall(bunEnv, packageDir, { savesLockfile: false }); + expect(await storeEntries(packageDir)).toEqual(expectedEntries); + }); + + test("a tarball URL longer than NAME_MAX installs, also into the global store", async () => { + const tarball = file(join(import.meta.dir, "registry", "packages", "no-deps", "no-deps-1.0.0.tgz")); + const segment = Buffer.alloc(255, "t").toString(); + + using server = Bun.serve({ + port: 0, + fetch(req) { + if (new URL(req.url).pathname !== `/${segment}/no-deps.tgz`) { + return new Response("Not found", { status: 404 }); + } + return new Response(tarball, { headers: { "Content-Type": "application/octet-stream" } }); + }, + }); + const url = `http://localhost:${server.port}/${segment}/no-deps.tgz`; + + const { packageJson, packageDir } = await registry.createTestDir({ + bunfigOpts: { linker: "isolated", globalStore: true }, + files: { "index.mjs": `import pkg from "no-deps";\nconsole.log(pkg.version);` }, + }); + await write(packageJson, JSON.stringify({ name: "test-long-tarball-url", dependencies: { "no-deps": url } })); + + await runBunInstall(bunEnv, packageDir); + + const entry = storeEntryName("no-deps", url.replaceAll(/[/:]/g, "+")); + expect(entry).toMatch(/^no-deps@http\+\+\+localhost\+\d+\+t+\+[0-9a-f]{16}$/); + expect(await storeEntries(packageDir)).toEqual([entry]); + + const localEntry = join(packageDir, "node_modules", ".bun", entry); + expect(lstatSync(localEntry).isSymbolicLink()).toBe(true); + expect(entryStoreName(readlinkSync(localEntry))).toMatch( + new RegExp(`^${entry.replaceAll("+", "\\+")}-[0-9a-f]{16}$`), + ); + + await using proc = spawn({ + cmd: [bunExe(), "index.mjs"], + env: bunEnv, + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("1.0.0\n"); + expect(exitCode).toBe(0); + }); + + // The reported case: a git dependency's resolution is its repository URL + // (here: a path inside the temp directory) plus the commit, and the entry is + // the cwd its lifecycle scripts are spawned with. + test.skipIf(!gitExecutable)("a trusted git dependency with a long URL runs its lifecycle scripts", async () => { + const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } }); + const repoDir = join(packageDir, Buffer.alloc(60, "r").toString()); + const gitEnv = { + ...bunEnv, + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: join(packageDir, "gitconfig"), + GIT_AUTHOR_NAME: "bun-test", + GIT_AUTHOR_EMAIL: "test@bun.sh", + GIT_COMMITTER_NAME: "bun-test", + GIT_COMMITTER_EMAIL: "test@bun.sh", + }; + async function git(...args: string[]): Promise { + await using proc = spawn({ + cmd: [gitExecutable!, ...args], + cwd: repoDir, + env: gitEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(err).not.toContain("fatal:"); + expect(exitCode).toBe(0); + return out; + } + + await Promise.all([ + write(join(packageDir, "gitconfig"), ""), + write( + join(repoDir, "package.json"), + JSON.stringify({ + name: "git-dep", + version: "1.0.0", + scripts: { postinstall: `${bunExe()} -e "require('fs').writeFileSync('postinstall-ran.txt', 'ran')"` }, + }), + ), + ]); + await git("init", "-q"); + await git("add", "package.json"); + await git("commit", "-qm", "init", "--no-gpg-sign"); + const sha = (await git("rev-parse", "HEAD")).trim(); + + const repoUrl = pathToFileURL(repoDir).href; + await write( + packageJson, + JSON.stringify({ + name: "test-long-git-url", + dependencies: { "git-dep": `git+${repoUrl}` }, + trustedDependencies: ["git-dep"], + }), + ); + + await runBunInstall(gitEnv, packageDir); + + const entry = storeEntryName("git-dep", `git+${repoUrl.replaceAll(/[/:]/g, "+")}+${sha}`); + expect(entry).toMatch(/^git-dep@git\+file\+\+\+\+.*\+[0-9a-f]{16}$/); + expect(await storeEntries(packageDir)).toEqual([entry]); + expect(await file(join(packageDir, "node_modules", "git-dep", "postinstall-ran.txt")).text()).toBe("ran"); + }); +}); + // The store entry of a tarball or git dependency is named after its URL, and // that name ends up in every path under the entry (realpaths, stack traces, // `bun pm` output). Credentials travel in the userinfo (`user:token@`) or the @@ -2290,7 +2603,10 @@ describe("store entry names of URL dependencies", () => { await runBunInstall(env, project); - const entry = `git-dep@git+http+++127.0.0.1+${server.port}+repo.git${hashed ? `+${urlHash(repo)}` : ""}+${sha}`; + const entry = storeEntryName( + "git-dep", + `git+http+++127.0.0.1+${server.port}+repo.git${hashed ? `+${urlHash(repo)}` : ""}+${sha}`, + ); expect(await storeEntries(project)).toEqual([entry]); expect(readlinkSync(join(project, "node_modules", "git-dep"))).toBe( join(".bun", entry, "node_modules", "git-dep"),