diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index a7c46c834667..24ad0b4dc089 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -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); diff --git a/src/install/PackageManager/patchPackage.rs b/src/install/PackageManager/patchPackage.rs index 22f89dfd3146..b7865c882226 100644 --- a/src/install/PackageManager/patchPackage.rs +++ b/src/install/PackageManager/patchPackage.rs @@ -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] @@ -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!( + "error: refusing to patch {} 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]>, @@ -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); @@ -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 = 'existing_patchfile_hash: { let mut name_and_version = Vec::new(); @@ -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 = 'existing_patchfile_hash: { diff --git a/src/install/npm.rs b/src/install/npm.rs index 0addc627fc1f..2a27333782a5 100644 --- a/src/install/npm.rs +++ b/src/install/npm.rs @@ -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; @@ -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. @@ -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 { - 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>; @@ -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( @@ -2065,9 +2047,6 @@ impl PackageManifest { let mut version_extern_strings_dedupe_map = ExternalStringMapDeduper::default(); let mut optional_peer_dep_names: Vec = 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 { @@ -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 @@ -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"")); } @@ -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::(cpu); } @@ -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()) @@ -2684,8 +2681,6 @@ impl PackageManifest { } } - let bundled_deps_begin = bundled_deps_offset; - let mut i: usize = 0; for item in items { @@ -2706,15 +2701,6 @@ impl PackageManifest { version_extern_strings[values_base + i] = string_builder.append::(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() @@ -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 = diff --git a/test/cli/install/bun-install-registry.test.ts b/test/cli/install/bun-install-registry.test.ts index 08bdf5c9bd11..dd90f51b4e91 100644 --- a/test/cli/install/bun-install-registry.test.ts +++ b/test/cli/install/bun-install-registry.test.ts @@ -1376,6 +1376,139 @@ describe("bundledDependencies", () => { await check(); }); + test(`(${textLockfile ? "bun.lock" : "bun.lockb"}) bundled names from dependencies and optionalDependencies`, async () => { + // bundled-with-optional@1.0.0 has dependencies { no-deps }, optionalDependencies { a-dep, basic-1 } + // and bundleDependencies [no-deps, a-dep, not-a-dependency]. Its tarball ships no-deps and a-dep. + // Only basic-1 should be installed from the registry; not-a-dependency is declared in no group + // (and does not exist in the registry), so listing it must have no effect. + await write( + packageJson, + JSON.stringify({ + name: "bundled-mixed-groups", + dependencies: { + "bundled-with-optional": "1.0.0", + }, + }), + ); + + await runBunInstall(env, packageDir, { saveTextLockfile: textLockfile }); + + async function check() { + expect(await readdirSorted(join(packageDir, "node_modules"))).toEqual(["basic-1", "bundled-with-optional"]); + const bundledNodeModules = join(packageDir, "node_modules", "bundled-with-optional", "node_modules"); + expect( + await Promise.all([ + file(join(bundledNodeModules, "no-deps", "package.json")).json(), + file(join(bundledNodeModules, "a-dep", "package.json")).json(), + ]), + ).toEqual([ + { name: "no-deps", version: "1.0.0" }, + { name: "a-dep", version: "1.0.1" }, + ]); + } + + await check(); + + let lockfile: string | undefined; + if (textLockfile) { + lockfile = await file(join(packageDir, "bun.lock")).text(); + expect(lockfile.replaceAll(/localhost:\d+/g, "localhost:1234")).toMatchInlineSnapshot(` + "{ + "lockfileVersion": 2, + "configVersion": 1, + "workspaces": { + "": { + "name": "bundled-mixed-groups", + "dependencies": { + "bundled-with-optional": "1.0.0", + }, + }, + }, + "packages": { + "basic-1": ["basic-1@1.0.0", "http://localhost:1234/basic-1/-/basic-1-1.0.0.tgz", {}, "sha512-NW5qBU1Kn7DzCjfVfnAbBBRGuQ7krbBtrnezZwOXutA9NvrCT4SI4EJMog3AGsNeK/1OygErysF8RN/FqDYunA=="], + + "bundled-with-optional": ["bundled-with-optional@1.0.0", "http://localhost:1234/bundled-with-optional/-/bundled-with-optional-1.0.0.tgz", { "dependencies": { "no-deps": "1.0.0" }, "optionalDependencies": { "a-dep": "1.0.1", "basic-1": "1.0.0" } }, "sha512-rq4Jtdsk53QE4T00/KaOP5lyv3d8Gnt4VABtara3PFos+8POKMkRIQPYCXVRRBey7K+ttzkiA5MkHhe+T19eYA=="], + + "bundled-with-optional/a-dep": ["a-dep@1.0.1", "http://localhost:1234/a-dep/-/a-dep-1.0.1.tgz", { "bundled": true }, "sha512-6nmTaPgO2U/uOODqOhbjbnaB4xHuZ+UB7AjKUA3g2dT4WRWeNxgp0dC8Db4swXSnO5/uLLUdFmUJKINNBO/3wg=="], + + "bundled-with-optional/no-deps": ["no-deps@1.0.0", "http://localhost:1234/no-deps/-/no-deps-1.0.0.tgz", { "bundled": true }, "sha512-v4w12JRjUGvfHDUP8vFDwu0gUWu04j0cv9hLb1Abf9VdaXu4XcrddYFTMVBVvmldKViGWH7jrb6xPJRF0wq6gw=="], + } + } + " + `); + } + + // The lockfile must record the bundled edges too: a cold install from it + // must not pull no-deps or a-dep from the registry either. + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + await runBunInstall(env, packageDir, { frozenLockfile: true }); + + await check(); + if (textLockfile) { + expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile!); + } + }); + + test(`(${textLockfile ? "bun.lock" : "bun.lockb"}) bundled name declared in peerDependencies`, async () => { + // bundled-peer@1.0.0 has peerDependencies { no-deps } and bundleDependencies [no-deps], and its + // tarball ships no-deps. The peer is satisfied by the bundled copy, so nothing else is installed. + await write( + packageJson, + JSON.stringify({ + name: "bundled-peer-root", + dependencies: { + "bundled-peer": "1.0.0", + }, + }), + ); + + await runBunInstall(env, packageDir, { saveTextLockfile: textLockfile }); + + async function check() { + expect(await readdirSorted(join(packageDir, "node_modules"))).toEqual(["bundled-peer"]); + expect( + await file( + join(packageDir, "node_modules", "bundled-peer", "node_modules", "no-deps", "package.json"), + ).json(), + ).toEqual({ name: "no-deps", version: "1.0.0" }); + } + + await check(); + + let lockfile: string | undefined; + if (textLockfile) { + lockfile = await file(join(packageDir, "bun.lock")).text(); + expect(lockfile.replaceAll(/localhost:\d+/g, "localhost:1234")).toMatchInlineSnapshot(` + "{ + "lockfileVersion": 2, + "configVersion": 1, + "workspaces": { + "": { + "name": "bundled-peer-root", + "dependencies": { + "bundled-peer": "1.0.0", + }, + }, + }, + "packages": { + "bundled-peer": ["bundled-peer@1.0.0", "http://localhost:1234/bundled-peer/-/bundled-peer-1.0.0.tgz", { "peerDependencies": { "no-deps": "1.0.0" } }, "sha512-rmaahOTDrcPuhmBskBLcKEEfTotY+xH6bDPnQopcNlmdePtEvAbAehSpK8JQAegpBir4omjiFqBy1LFqXQBfgA=="], + + "bundled-peer/no-deps": ["no-deps@1.0.0", "http://localhost:1234/no-deps/-/no-deps-1.0.0.tgz", { "bundled": true }, "sha512-v4w12JRjUGvfHDUP8vFDwu0gUWu04j0cv9hLb1Abf9VdaXu4XcrddYFTMVBVvmldKViGWH7jrb6xPJRF0wq6gw=="], + } + } + " + `); + } + + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + await runBunInstall(env, packageDir, { frozenLockfile: true }); + + await check(); + if (textLockfile) { + expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile!); + } + }); + test(`(${textLockfile ? "bun.lock" : "bun.lockb"}) git dependencies`, async () => { await Promise.all([ write( @@ -1585,6 +1718,189 @@ describe("bundledDependencies", () => { expect(stdout).toBe("bundled-file-dep\n"); expect(exitCode).toBe(0); }); + + // bundled-file@4.0.0 has the shape of a package that `npm publish` makes from a + // monorepo: the `file:` path of its bundled dependency leaves the package, and + // the tarball ships only the bundled copy in its node_modules. The copy is the + // dependency. The install records the path in the lockfile and never follows it. + // bundled-file@5.0.0 also has `optionalDependencies` ("no-deps"). The bundled + // name is in `dependencies`, and the later group must not drop its bundled flag. + describe.each(["hoisted", "isolated"] as const)( + "(%s) bundled file: dependency with a path that leaves the package", + linker => { + test.each([ + ["4.0.0", "1 package installed"], + ["5.0.0", "2 packages installed"], + ])("bundled-file@%s", async (version, installed) => { + await Promise.all([ + registry.writeBunfig(packageDir, { saveTextLockfile: false, linker }), + write( + packageJson, + JSON.stringify({ + name: "bundled-file-outside-root", + dependencies: { + "bundled-file": version, + }, + }), + ), + ]); + + const bundledDepDir = join(packageDir, "node_modules", "bundled-file", "node_modules", "bundled-file-dep"); + + async function check() { + const [pkgJsonStat, indexStat] = await Promise.all([ + lstat(join(bundledDepDir, "package.json")), + lstat(join(bundledDepDir, "index.js")), + ]); + expect([pkgJsonStat.isFile(), indexStat.isFile()]).toEqual([true, true]); + // `../bundled-file-dep`, seen from the installed package + expect(await exists(join(packageDir, "node_modules", "bundled-file-dep"))).toBeFalse(); + + await using proc = spawn({ + cmd: [bunExe(), "-e", `console.log(require("bundled-file"))`], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("bundled-file-dep\n"); + expect(exitCode).toBe(0); + } + + let { out } = await runBunInstall(env, packageDir, { saveTextLockfile: true }); + expect(out).toContain(installed); + await check(); + + const lockfile = await file(join(packageDir, "bun.lock")).text(); + expect(lockfile).toContain( + `"bundled-file/bundled-file-dep": ["bundled-file-dep@file:../bundled-file-dep", { "bundled": true }],`, + ); + + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + + ({ out } = await runBunInstall(env, packageDir, { frozenLockfile: true })); + expect(out).toContain(installed); + await check(); + expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile); + }); + }, + ); + + // bundled-file@3.0.0 declares the same path and does not bundle the + // dependency. A registry package must not link a directory outside itself. + test("file: dependency with a path that leaves the package is refused when it is not bundled", async () => { + await write( + packageJson, + JSON.stringify({ + name: "unbundled-file-outside-root", + dependencies: { + "bundled-file": "3.0.0", + }, + }), + ); + + await using proc = spawn({ + cmd: [bunExe(), "install", "--save-text-lockfile"], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr.split(/\r?\n/).filter(line => line.startsWith("error:"))).toEqual([ + `error: Could not find package.json for "file:../bundled-file-dep" dependency "bundled-file-dep"`, + "error: bundled-file-dep@file:../bundled-file-dep failed to resolve", + ]); + expect(stdout).not.toContain("installed"); + expect(await exists(join(packageDir, "bun.lock"))).toBeFalse(); + expect(exitCode).toBe(1); + }); + + // The path recorded for a bundled dependency must stay unread when another + // dependency resolves to the same package. "mid" is nested next to the bundled + // "inner" and has an optional peer on it, so the hoister binds that peer to the + // recorded `file:../inner`, which names a directory outside the project. + for (const linker of ["hoisted", "isolated"] as const) { + test(`(${linker}) the path of a bundled file: dependency is not followed for another dependent`, async () => { + const packages = [ + { + json: { + name: "outer", + version: "1.0.0", + dependencies: { inner: "file:../inner", mid: "1.0.0" }, + bundleDependencies: ["inner"], + }, + files: { "package/node_modules/inner/package.json": JSON.stringify({ name: "inner", version: "1.0.0" }) }, + }, + { + json: { + name: "mid", + version: "1.0.0", + peerDependencies: { inner: "*" }, + peerDependenciesMeta: { inner: { optional: true } }, + }, + }, + // the root depends on this one, so mid@1.0.0 stays nested under "outer" + { json: { name: "mid", version: "2.0.0" } }, + ]; + const tarballs = new Map(); + for (const { json, files } of packages) { + tarballs.set( + `/${json.name}-${json.version}.tgz`, + await new Bun.Archive( + { "package/package.json": JSON.stringify(json), ...files }, + { compress: "gzip" }, + ).bytes(), + ); + } + using server = Bun.serve({ + port: 0, + fetch(request) { + const { origin, pathname } = new URL(request.url); + const tarball = tarballs.get(pathname); + if (tarball) return new Response(tarball); + const versions = packages.filter(({ json }) => `/${json.name}` === pathname); + if (versions.length === 0) return new Response("not found", { status: 404 }); + return Response.json({ + name: versions[0].json.name, + "dist-tags": { latest: versions.at(-1)!.json.version }, + versions: Object.fromEntries( + versions.map(({ json }) => [ + json.version, + { ...json, dist: { tarball: `${origin}/${json.name}-${json.version}.tgz` } }, + ]), + ), + }); + }, + }); + + using dir = tempDir("bundled-file-outside-peer", { + "inner/package.json": JSON.stringify({ name: "inner", version: "9.9.9" }), + "inner/outside.txt": "outside the project", + "project/package.json": JSON.stringify({ name: "app", dependencies: { outer: "1.0.0", mid: "2.0.0" } }), + "project/bunfig.toml": `[install]\nregistry = "${server.url.href}"\nlinker = "${linker}"\n`, + }); + const projectDir = join(String(dir), "project"); + + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd: projectDir, + stdout: "ignore", + stderr: "pipe", + env: { ...env, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }, + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect(stderr).toContain(`error: refusing to install dependency inner with unsafe folder path "../inner"`); + expect([...new Bun.Glob("**/outside.txt").scanSync({ cwd: projectDir, dot: true, onlyFiles: false })]).toEqual( + [], + ); + expect(exitCode).toBe(1); + }); + } }); describe("optionalDependencies", () => { diff --git a/test/cli/install/bun-patch.test.ts b/test/cli/install/bun-patch.test.ts index 0e61d4526398..aabaf1ad0e51 100644 --- a/test/cli/install/bun-patch.test.ts +++ b/test/cli/install/bun-patch.test.ts @@ -177,6 +177,68 @@ describe("packages whose label is longer than 1024 bytes", () => { }); }); +// bundled-file@4.0.0 bundles a `file:../bundled-file-dep` dependency. The lockfile +// records that path and the install never reads it: the copy in the tarball is +// the dependency. `bun patch` takes a `file:` package from its folder, which for +// this path is a directory outside the project, so it has to refuse. +describe("a bundled file: dependency with a path that leaves the package", () => { + const registry = new VerdaccioRegistry(); + + beforeAll(async () => { + await registry.start(); + }); + + afterAll(() => { + registry.stop(); + }); + + const bundledCopy = "node_modules/bundled-file/node_modules/bundled-file-dep"; + + test.concurrent.each([ + ["bun patch ", ["bundled-file-dep"]], + ["bun patch ", [bundledCopy]], + ["bun patch --commit ", ["--commit", bundledCopy]], + ])("%s", async (_, args) => { + const packageJson = { name: "app", dependencies: { "bundled-file": "4.0.0" } }; + const { packageDir } = await registry.createTestDir({ + bunfigOpts: { linker: "hoisted" }, + files: { + "bundled-file-dep/package.json": JSON.stringify({ name: "bundled-file-dep", version: "9.9.9" }), + "bundled-file-dep/index.js": `module.exports = "outside the project";\n`, + "project/package.json": JSON.stringify(packageJson), + }, + }); + const projectDir = join(packageDir, "project"); + await registry.writeBunfig(projectDir, { linker: "hoisted" }); + + async function runBun(...cmd: string[]) { + await using proc = Bun.spawn({ + cmd: [bunExe(), ...cmd], + cwd: projectDir, + env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(packageDir, ".bun-cache") }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + const install = await runBun("install"); + expect(install.stderr).not.toContain("error:"); + expect(install.exitCode).toBe(0); + + const patch = await runBun("patch", ...args); + expect(patch.stderr).toContain( + `error: refusing to patch bundled-file-dep with unsafe folder path "../bundled-file-dep"`, + ); + expect(patch.exitCode).toBe(1); + expect(await Bun.file(join(projectDir, bundledCopy, "index.js")).text()).toBe( + `module.exports = "bundled-file-dep";\n`, + ); + expect(await Bun.file(join(projectDir, "package.json")).json()).toEqual(packageJson); + }); +}); + describe("bun patch ", async () => { describe("workspace interactions", async () => { /** diff --git a/test/cli/install/registry/packages/bundled-file/bundled-file-3.0.0.tgz b/test/cli/install/registry/packages/bundled-file/bundled-file-3.0.0.tgz new file mode 100644 index 000000000000..97d3ad927f94 Binary files /dev/null and b/test/cli/install/registry/packages/bundled-file/bundled-file-3.0.0.tgz differ diff --git a/test/cli/install/registry/packages/bundled-file/bundled-file-4.0.0.tgz b/test/cli/install/registry/packages/bundled-file/bundled-file-4.0.0.tgz new file mode 100644 index 000000000000..513e146b464f Binary files /dev/null and b/test/cli/install/registry/packages/bundled-file/bundled-file-4.0.0.tgz differ diff --git a/test/cli/install/registry/packages/bundled-file/bundled-file-5.0.0.tgz b/test/cli/install/registry/packages/bundled-file/bundled-file-5.0.0.tgz new file mode 100644 index 000000000000..f273fe3b95ed Binary files /dev/null and b/test/cli/install/registry/packages/bundled-file/bundled-file-5.0.0.tgz differ diff --git a/test/cli/install/registry/packages/bundled-file/package.json b/test/cli/install/registry/packages/bundled-file/package.json index 3190852462f2..2935ae1b7902 100644 --- a/test/cli/install/registry/packages/bundled-file/package.json +++ b/test/cli/install/registry/packages/bundled-file/package.json @@ -2,7 +2,7 @@ "_id": "bundled-file", "name": "bundled-file", "dist-tags": { - "latest": "2.0.0" + "latest": "5.0.0" }, "versions": { "1.0.0": { @@ -33,6 +33,54 @@ "shasum": "cd4c6b205128b53321bad299e60d7c6afb1f6c0e", "tarball": "http://localhost:4873/bundled-file/-/bundled-file-2.0.0.tgz" } + }, + "3.0.0": { + "name": "bundled-file", + "version": "3.0.0", + "dependencies": { + "bundled-file-dep": "file:../bundled-file-dep" + }, + "_id": "bundled-file@3.0.0", + "dist": { + "integrity": "sha512-gQMoo88Glz7mlN93q5VrcTwW7lGobaWASIYXAq+JJ1mVDpVawkKKOkfAkv9CaLlsd/96HhuavU516+ldQvsEig==", + "shasum": "acec9a255f909d6c07f07d0654150ec87594a181", + "tarball": "http://localhost:4873/bundled-file/-/bundled-file-3.0.0.tgz" + } + }, + "4.0.0": { + "name": "bundled-file", + "version": "4.0.0", + "dependencies": { + "bundled-file-dep": "file:../bundled-file-dep" + }, + "bundleDependencies": [ + "bundled-file-dep" + ], + "_id": "bundled-file@4.0.0", + "dist": { + "integrity": "sha512-x18GKenWM1RFAs5LjQF6wyolc2UIuBjZHntGawm9XIaAjzVvKEVFmQZjZYb9QOrVLwGphMWVXVQKUPDnTzYubw==", + "shasum": "6a4539872405e60b6ca02a4f9ed7c69b9dae0a84", + "tarball": "http://localhost:4873/bundled-file/-/bundled-file-4.0.0.tgz" + } + }, + "5.0.0": { + "name": "bundled-file", + "version": "5.0.0", + "dependencies": { + "bundled-file-dep": "file:../bundled-file-dep" + }, + "optionalDependencies": { + "no-deps": "1.0.0" + }, + "bundleDependencies": [ + "bundled-file-dep" + ], + "_id": "bundled-file@5.0.0", + "dist": { + "integrity": "sha512-aOIi7rw4XaXuHYwHE0bvPGmC0lTpzdcSw5bS0BGxXVIDJkG3vW29IiWcTcQg539P7ZEQgZqqFpwC48ZDaHNiDA==", + "shasum": "29cb868fd9146c0c519ac07832aed444550d73a3", + "tarball": "http://localhost:4873/bundled-file/-/bundled-file-5.0.0.tgz" + } } } } \ No newline at end of file diff --git a/test/cli/install/registry/packages/bundled-peer/bundled-peer-1.0.0.tgz b/test/cli/install/registry/packages/bundled-peer/bundled-peer-1.0.0.tgz new file mode 100644 index 000000000000..40e294dbcb58 Binary files /dev/null and b/test/cli/install/registry/packages/bundled-peer/bundled-peer-1.0.0.tgz differ diff --git a/test/cli/install/registry/packages/bundled-peer/package.json b/test/cli/install/registry/packages/bundled-peer/package.json new file mode 100644 index 000000000000..670e3938bcff --- /dev/null +++ b/test/cli/install/registry/packages/bundled-peer/package.json @@ -0,0 +1,44 @@ +{ + "name": "bundled-peer", + "versions": { + "1.0.0": { + "name": "bundled-peer", + "version": "1.0.0", + "peerDependencies": { + "no-deps": "1.0.0" + }, + "bundleDependencies": [ + "no-deps" + ], + "_id": "bundled-peer@1.0.0", + "integrity": "sha512-rmaahOTDrcPuhmBskBLcKEEfTotY+xH6bDPnQopcNlmdePtEvAbAehSpK8JQAegpBir4omjiFqBy1LFqXQBfgA==", + "shasum": "32b15c8ab065db1e6b9383043f0dd002077cb65d", + "dist": { + "integrity": "sha512-rmaahOTDrcPuhmBskBLcKEEfTotY+xH6bDPnQopcNlmdePtEvAbAehSpK8JQAegpBir4omjiFqBy1LFqXQBfgA==", + "shasum": "32b15c8ab065db1e6b9383043f0dd002077cb65d", + "tarball": "http://localhost:4873/bundled-peer/-/bundled-peer-1.0.0.tgz" + }, + "contributors": [] + } + }, + "time": { + "modified": "2024-12-30T18:14:22.143Z", + "created": "2024-12-30T18:14:22.143Z", + "1.0.0": "2024-12-30T18:14:22.143Z" + }, + "users": {}, + "dist-tags": { + "latest": "1.0.0" + }, + "_uplinks": {}, + "_distfiles": {}, + "_attachments": { + "bundled-peer-1.0.0.tgz": { + "shasum": "32b15c8ab065db1e6b9383043f0dd002077cb65d", + "version": "1.0.0" + } + }, + "_rev": "", + "_id": "bundled-peer", + "readme": "" +} diff --git a/test/cli/install/registry/packages/bundled-with-optional/bundled-with-optional-1.0.0.tgz b/test/cli/install/registry/packages/bundled-with-optional/bundled-with-optional-1.0.0.tgz new file mode 100644 index 000000000000..940fd676286e Binary files /dev/null and b/test/cli/install/registry/packages/bundled-with-optional/bundled-with-optional-1.0.0.tgz differ diff --git a/test/cli/install/registry/packages/bundled-with-optional/package.json b/test/cli/install/registry/packages/bundled-with-optional/package.json new file mode 100644 index 000000000000..43c842c3bacb --- /dev/null +++ b/test/cli/install/registry/packages/bundled-with-optional/package.json @@ -0,0 +1,50 @@ +{ + "name": "bundled-with-optional", + "versions": { + "1.0.0": { + "name": "bundled-with-optional", + "version": "1.0.0", + "dependencies": { + "no-deps": "1.0.0" + }, + "optionalDependencies": { + "a-dep": "1.0.1", + "basic-1": "1.0.0" + }, + "bundleDependencies": [ + "no-deps", + "a-dep", + "not-a-dependency" + ], + "_id": "bundled-with-optional@1.0.0", + "integrity": "sha512-rq4Jtdsk53QE4T00/KaOP5lyv3d8Gnt4VABtara3PFos+8POKMkRIQPYCXVRRBey7K+ttzkiA5MkHhe+T19eYA==", + "shasum": "b5dbbcef38296b5a75d4e7441216dff1467d6ebd", + "dist": { + "integrity": "sha512-rq4Jtdsk53QE4T00/KaOP5lyv3d8Gnt4VABtara3PFos+8POKMkRIQPYCXVRRBey7K+ttzkiA5MkHhe+T19eYA==", + "shasum": "b5dbbcef38296b5a75d4e7441216dff1467d6ebd", + "tarball": "http://localhost:4873/bundled-with-optional/-/bundled-with-optional-1.0.0.tgz" + }, + "contributors": [] + } + }, + "time": { + "modified": "2024-12-30T18:14:22.143Z", + "created": "2024-12-30T18:14:22.143Z", + "1.0.0": "2024-12-30T18:14:22.143Z" + }, + "users": {}, + "dist-tags": { + "latest": "1.0.0" + }, + "_uplinks": {}, + "_distfiles": {}, + "_attachments": { + "bundled-with-optional-1.0.0.tgz": { + "shasum": "b5dbbcef38296b5a75d4e7441216dff1467d6ebd", + "version": "1.0.0" + } + }, + "_rev": "", + "_id": "bundled-with-optional", + "readme": "" +} diff --git a/test/cli/install/registry/packages/create-bundled-file-packages.ts b/test/cli/install/registry/packages/create-bundled-file-packages.ts index 836d009ea924..83345a4b7379 100644 --- a/test/cli/install/registry/packages/create-bundled-file-packages.ts +++ b/test/cli/install/registry/packages/create-bundled-file-packages.ts @@ -2,10 +2,12 @@ /** * Generates the `bundled-file` fixture used by bun-install-registry.test.ts. * - * Both versions depend on `bundled-file-dep` through a `file:` spec - * (`file:vendor/bundled-file-dep`) and ship both `vendor/bundled-file-dep/` - * and a copy under `node_modules/bundled-file-dep/`, the way `npm pack` does - * for a bundled `file:` dependency. + * Every version depends on `bundled-file-dep` through a `file:` spec. + * + * 1.0.0 and 2.0.0 point the spec inside the package + * (`file:vendor/bundled-file-dep`) and ship both `vendor/bundled-file-dep/` and + * a copy under `node_modules/bundled-file-dep/`, the way `npm pack` does for a + * bundled `file:` dependency. * * - bundled-file@1.0.0 lists the dependency in `bundleDependencies`. A `file:` * resolution has its `bundled: true` marker at a different index in bun.lock @@ -13,6 +15,19 @@ * the dependency as bundled. * - bundled-file@2.0.0 does not bundle it. The install has to link the * `file:` dependency over the files the tarball already put in place. + * + * 3.0.0 and 4.0.0 point the spec outside the package + * (`file:../bundled-file-dep`), the way a package published from a monorepo + * does. The tarball cannot ship that directory. + * + * - bundled-file@3.0.0 does not bundle it. A registry package must not link a + * directory outside itself, so the install must refuse the path. + * - bundled-file@4.0.0 lists the dependency in `bundleDependencies` and ships + * the copy under `node_modules/bundled-file-dep/`. The copy is the + * dependency, so the install must not refuse the path. + * - bundled-file@5.0.0 is 4.0.0 plus an `optionalDependencies` group. The + * bundled name is in `dependencies`, and the manifest parser must keep it + * bundled when a later dependency group exists. */ import { mkdir, writeFile } from "fs/promises"; @@ -31,28 +46,40 @@ const depIndex = `module.exports = "${depName}";\n`; const versions: Record = {}; let latest = ""; -for (const [version, bundled] of [ - ["1.0.0", true], - ["2.0.0", false], -] as const) { +for (const { version, inside, bundled, optional } of [ + { version: "1.0.0", inside: true, bundled: true }, + { version: "2.0.0", inside: true, bundled: false }, + { version: "3.0.0", inside: false, bundled: false }, + { version: "4.0.0", inside: false, bundled: true }, + { version: "5.0.0", inside: false, bundled: true, optional: true }, +]) { const pkgJson = { name, version, - dependencies: { [depName]: `file:vendor/${depName}` }, + dependencies: { [depName]: inside ? `file:vendor/${depName}` : `file:../${depName}` }, + ...(optional ? { optionalDependencies: { "no-deps": "1.0.0" } } : {}), ...(bundled ? { bundleDependencies: [depName] } : {}), }; const files: Record = { "package/package.json": JSON.stringify(pkgJson, null, 2), "package/index.js": `module.exports = require("${depName}");\n`, - [`package/vendor/${depName}/package.json`]: depPkgJson, - [`package/vendor/${depName}/index.js`]: depIndex, - [`package/node_modules/${depName}/package.json`]: depPkgJson, - [`package/node_modules/${depName}/index.js`]: depIndex, }; + if (inside) { + files[`package/vendor/${depName}/package.json`] = depPkgJson; + files[`package/vendor/${depName}/index.js`] = depIndex; + } + if (inside || bundled) { + files[`package/node_modules/${depName}/package.json`] = depPkgJson; + files[`package/node_modules/${depName}/index.js`] = depIndex; + } + // Bun.Archive stamps the current time on every entry, so a second write + // changes the integrity. Keep a tarball that already exists. const tarball = join(dir, `${name}-${version}.tgz`); - await Bun.Archive.write(tarball, files, { compress: "gzip" }); + if (!(await Bun.file(tarball).exists())) { + await Bun.Archive.write(tarball, files, { compress: "gzip" }); + } const bytes = await Bun.file(tarball).bytes(); versions[version] = {