diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index fcd5b545bd4d..d9b4cd6908fb 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -126,6 +126,38 @@ fn invalid_trusted_dependencies( crate::Error::InvalidPackageJSON } +/// A declared `trustedDependencies` (even `[]`) replaces the default list, so the set becomes `Some` as soon as the field exists. +pub(crate) fn parse_append_trusted_dependencies( + trusted_dependencies: &mut Option, + log: &mut bun_ast::Log, + source: &bun_ast::Source, + json: Expr, + bump: &bun_alloc::Arena, +) -> crate::Result<()> { + let Some(q) = json.as_property(b"trustedDependencies") else { + return Ok(()); + }; + let count = match &q.expr.data { + ExprData::EArray(arr) => arr.items.len_u32() as usize, + ExprData::EArrayJSON(arr) => arr.get().items().len(), + _ => return Err(invalid_trusted_dependencies(log, source, q.loc)), + }; + let trusted = trusted_dependencies.get_or_insert_with(Default::default); + trusted.ensure_unused_capacity(count)?; + if let Some(mut items) = q.expr.as_array() { + while let Some(item) = items.next() { + let Some(name) = item.as_string(bump) else { + return Err(invalid_trusted_dependencies(log, source, q.loc)); + }; + trusted.put_assume_capacity( + semver::string::Builder::string_hash(name) as TruncatedPackageNameHash, + Box::<[u8]>::from(name), + ); + } + } + Ok(()) +} + // `SemverIntType` defaults to `u64`, the only instantiation the lockfile/PM // call sites name unqualified. // @@ -2466,29 +2498,13 @@ impl Package { } if FEATURES.trusted_dependencies { - if let Some(q) = json.as_property(b"trustedDependencies") { - let count = match &q.expr.data { - ExprData::EArray(arr) => arr.items.len_u32() as usize, - ExprData::EArrayJSON(arr) => arr.get().items().len(), - _ => return Err(invalid_trusted_dependencies(log, source, q.loc)), - }; - if lockfile.trusted_dependencies.is_none() { - lockfile.trusted_dependencies = Some(Default::default()); - } - let trusted = lockfile.trusted_dependencies.as_mut().unwrap(); - trusted.ensure_unused_capacity(count)?; - if let Some(mut items) = q.expr.as_array() { - while let Some(item) = items.next() { - let Some(name) = item.as_string(&bump) else { - return Err(invalid_trusted_dependencies(log, source, q.loc)); - }; - trusted.put_assume_capacity( - semver::string::Builder::string_hash(name) as TruncatedPackageNameHash, - Box::<[u8]>::from(name), - ); - } - } - } + parse_append_trusted_dependencies( + &mut lockfile.trusted_dependencies, + log, + source, + json, + &bump, + )?; } if FEATURES.is_main { diff --git a/src/install/migration.rs b/src/install/migration.rs index dd5270fe8424..172b5661560d 100644 --- a/src/install/migration.rs +++ b/src/install/migration.rs @@ -1,8 +1,8 @@ use crate::Error; use bun_ast::{E, ExprData}; use bun_core::strings; -use bun_core::{Output, zstr}; -use bun_paths::PathBuffer; +use bun_core::{Output, ZStr, zstr}; +use bun_paths::{AutoAbsPath, PathBuffer}; use bun_semver::query::token::Wildcard; use bun_semver::{self as Semver, SlicedString}; use bun_sys::{self, Fd, File, O}; @@ -11,8 +11,8 @@ use crate::install::{self as Install, PackageManager, Subcommand}; use crate::lockfile::{ Format as LockfileFormat, LoadResult, LoadResultErr, LoadResultOk, LoadStep, Lockfile, Migrated, }; -use crate::lockfile_real::package::PackageColumns as _; use crate::lockfile_real::package::workspace_map::{MissingWorkspace, NamesArray, WorkspaceMap}; +use crate::lockfile_real::package::{PackageColumns as _, parse_append_trusted_dependencies}; use crate::npm::{self as Npm}; use crate::pnpm; use crate::pnpm::MigratePnpmLockfileError; @@ -64,11 +64,13 @@ pub fn detect_and_load_other_lockfile<'a>( } }; - if matches!(migrate_result, LoadResult::Ok { .. }) { - report_migrated(manager, log, &timer, "package-lock.json"); - } - - return migrate_result; + return finish_migration( + migrate_result, + manager, + log, + &timer, + zstr!("package-lock.json"), + ); } 'yarn: { @@ -88,11 +90,7 @@ pub fn detect_and_load_other_lockfile<'a>( } }; - if matches!(migrate_result, LoadResult::Ok { .. }) { - report_migrated(manager, log, &timer, "yarn.lock"); - } - - return migrate_result; + return finish_migration(migrate_result, manager, log, &timer, zstr!("yarn.lock")); } 'pnpm: { @@ -155,16 +153,81 @@ pub fn detect_and_load_other_lockfile<'a>( } }; - if matches!(migrate_result, LoadResult::Ok { .. }) { - report_migrated(manager, log, &timer, "pnpm-lock.yaml"); - } - - return migrate_result; + return finish_migration( + migrate_result, + manager, + log, + &timer, + zstr!("pnpm-lock.yaml"), + ); } LoadResult::NotFound } +fn finish_migration<'a>( + migrate_result: LoadResult<'a>, + manager: &mut PackageManager, + log: &mut bun_ast::Log, + timer: &std::time::Instant, + lockfile_name: &'static ZStr, +) -> LoadResult<'a> { + let ok = match migrate_result { + LoadResult::Ok(ok) => ok, + other => return other, + }; + if let Err(err) = record_trusted_dependencies(&mut *ok.lockfile, manager, log) { + if !manager.options.log_level.is_silent() && log.has_errors() { + let _ = log.print(std::ptr::from_mut(Output::error_writer())); + Output::flush(); + } + log.reset(); + return LoadResult::Err(LoadResultErr { + step: LoadStep::Migrating, + value: err, + lockfile_path: lockfile_name, + format: LockfileFormat::Text, + }); + } + report_migrated(manager, log, timer, lockfile_name); + LoadResult::Ok(ok) +} + +/// Other lockfiles have no `trustedDependencies`; read the root's and the members' from package.json like `Package::parse_with_json` does, since `bun pm migrate` saves this lockfile as-is. +fn record_trusted_dependencies( + lockfile: &mut Lockfile, + manager: &mut PackageManager, + log: &mut bun_ast::Log, +) -> Result<(), Error> { + let bump = bun_alloc::Arena::new(); + let string_bytes = lockfile.buffers.string_bytes.as_slice(); + let root: &[u8] = b""; + let members = lockfile + .workspace_paths + .values() + .iter() + .map(|path| path.slice(string_bytes)); + for relative_dir in core::iter::once(root).chain(members) { + let mut package_json_path = AutoAbsPath::init_top_level_dir(); + let _ = package_json_path.append(relative_dir); + let _ = package_json_path.append(b"package.json"); + let crate::GetJsonResult::Entry(entry) = manager + .workspace_package_json_cache + .get_with_path(log, package_json_path.slice(), Default::default()) + else { + continue; + }; + parse_append_trusted_dependencies( + &mut lockfile.trusted_dependencies, + log, + &entry.source, + entry.root, + &bump, + )?; + } + Ok(()) +} + /// True when the migrator already printed the version warn/error + upgrade note, so lockfile-load reporters must stay quiet. pub fn reported_unsupported_lockfile_version(err: &LoadResultErr) -> bool { err.step == LoadStep::Migrating && matches!(err.value, Error::UnexpectedLockfileVersion) @@ -229,7 +292,7 @@ fn report_migrated( manager: &PackageManager, log: &mut bun_ast::Log, timer: &std::time::Instant, - lockfile_name: &str, + lockfile_name: &ZStr, ) { if manager.options.log_level.is_silent() { log.reset(); @@ -240,7 +303,10 @@ fn report_migrated( log.reset(); } Output::print_elapsed(timer.elapsed().as_nanos() as f64 / 1_000_000.0); - bun_core::pretty_errorln!(" migrated lockfile from {}", lockfile_name); + bun_core::pretty_errorln!( + " migrated lockfile from {}", + bstr::BStr::new(lockfile_name.as_bytes()) + ); Output::flush(); } diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index aea7f38ebec4..bd4989773c04 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -259,6 +259,205 @@ test.concurrent( }, ); +describe("trustedDependencies survive lockfile migration", () => { + // `electron` is on the default trusted list, `uses-what-bin` is not. Declaring + // `trustedDependencies: ["uses-what-bin"]` therefore has to flip both: the + // migrated lockfile must run uses-what-bin's install script and block electron's. + const integrity = { + "electron": "sha512-GkuwCdn6o8Krsxb3DIIqYP+TAi8Y5jYUadmseZ6nR2op2k5ssdKRYo4JjYDGopa1ACrGAcQuWViz/+vX/WjYnA==", + "uses-what-bin": "sha512-EI+uMDESinRewWTFhsyzibkzFV+j5LmLM7T1jEpb2X82TmzhSQRzCBTBURflt5dGvNEIY7l563P9Su01Tpe++g==", + "what-bin": "sha512-mbvEObM9mSliIzNJ4lJHfx8Zzdcf3v8PCIanJyIbXv6AFuuipK5tirQuM9Oi1yFXgO/YxI1W5zt4AwwLGxmaPA==", + }; + const shasum = { + "electron": "f1b8bc2c23cd7e4f1500669dfaf8757578d2e391", + "uses-what-bin": "78dea365c24435c0faa99dca78ed44c42273b84a", + "what-bin": "934dc0859a1a9ccf90ac341b1d4867f94d8e4d11", + }; + const dependencies = { "electron": "1.0.0", "uses-what-bin": "1.5.0" }; + + function tarball(name: keyof typeof integrity, version: string) { + return `http://localhost:${verdaccio.port}/${name}/-/${name}-${version}.tgz`; + } + + function packageLock() { + return JSON.stringify({ + name: "foo", + version: "1.0.0", + lockfileVersion: 3, + requires: true, + packages: { + "": { name: "foo", version: "1.0.0", dependencies }, + "node_modules/electron": { + version: "1.0.0", + resolved: tarball("electron", "1.0.0"), + integrity: integrity["electron"], + hasInstallScript: true, + }, + "node_modules/uses-what-bin": { + version: "1.5.0", + resolved: tarball("uses-what-bin", "1.5.0"), + integrity: integrity["uses-what-bin"], + hasInstallScript: true, + dependencies: { "what-bin": "1.5.0" }, + }, + "node_modules/what-bin": { + version: "1.5.0", + resolved: tarball("what-bin", "1.5.0"), + integrity: integrity["what-bin"], + bin: { "what-bin": "what-bin.js" }, + }, + }, + }); + } + + function yarnLock() { + return `# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + +electron@1.0.0: + version "1.0.0" + resolved "${tarball("electron", "1.0.0")}#${shasum["electron"]}" + integrity ${integrity["electron"]} + +uses-what-bin@1.5.0: + version "1.5.0" + resolved "${tarball("uses-what-bin", "1.5.0")}#${shasum["uses-what-bin"]}" + integrity ${integrity["uses-what-bin"]} + dependencies: + what-bin "1.5.0" + +what-bin@1.5.0: + version "1.5.0" + resolved "${tarball("what-bin", "1.5.0")}#${shasum["what-bin"]}" + integrity ${integrity["what-bin"]} +`; + } + + function pnpmLock(importer: string) { + return `lockfileVersion: '9.0' + +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + +importers: +${importer} +packages: + + electron@1.0.0: + resolution: {integrity: ${integrity["electron"]}} + + uses-what-bin@1.5.0: + resolution: {integrity: ${integrity["uses-what-bin"]}} + + what-bin@1.5.0: + resolution: {integrity: ${integrity["what-bin"]}} + hasBin: true + +snapshots: + + electron@1.0.0: {} + + uses-what-bin@1.5.0: + dependencies: + what-bin: 1.5.0 + + what-bin@1.5.0: {} +`; + } + + const importerDependencies = ` + dependencies: + electron: + specifier: 1.0.0 + version: 1.0.0 + uses-what-bin: + specifier: 1.5.0 + version: 1.5.0 +`; + + async function runBunPm(env: Record, packageDir: string, subcommand: string) { + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "pm", subcommand], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [out, err, exitCode] = await Promise.all([stdout.text(), stderr.text(), exited]); + expect(err).not.toContain("error:"); + expect(exitCode).toBe(0); + return { out, err }; + } + + // The migrated bun.lock is what gets committed and installed from with + // --frozen-lockfile, which never writes it back, so everything package.json + // declares has to be in it when `bun pm migrate` returns. + async function expectMigrationKeepsTrust(env: Record, packageDir: string, lockfileName: string) { + const { err: migrateErr } = await runBunPm(env, packageDir, "migrate"); + expect(migrateErr).toContain(`migrated lockfile from ${lockfileName}`); + expect(await file(join(packageDir, "bun.lock")).text()).toMatch( + /"trustedDependencies": \[\s*"uses-what-bin",\s*\]/, + ); + + await runBunInstall(env, packageDir, { frozenLockfile: true }); + expect( + await Promise.all([ + exists(join(packageDir, "node_modules", "uses-what-bin", "what-bin.txt")), + exists(join(packageDir, "node_modules", "electron", "preinstall.txt")), + ]), + ).toEqual([true, false]); + + const { out } = await runBunPm(env, packageDir, "untrusted"); + expect(out).toContain("./node_modules/electron @1.0.0".replaceAll("/", sep)); + expect(out).not.toContain("uses-what-bin"); + } + + const rootLockfiles: Record string> = { + "package-lock.json": packageLock, + "yarn.lock": yarnLock, + "pnpm-lock.yaml": () => pnpmLock(`\n .:${importerDependencies}`), + }; + + for (const [lockfileName, contents] of Object.entries(rootLockfiles)) { + test.concurrent(`bun pm migrate from ${lockfileName} keeps the root's trustedDependencies`, async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + + await Promise.all([ + write( + packageJson, + JSON.stringify({ name: "foo", version: "1.0.0", dependencies, trustedDependencies: ["uses-what-bin"] }), + ), + write(join(packageDir, lockfileName), contents()), + ]); + + await expectMigrationKeepsTrust(env, packageDir, lockfileName); + }); + } + + test.concurrent("bun pm migrate from pnpm-lock.yaml keeps a workspace member's trustedDependencies", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + + // Like `bun install`, a list declared by a member counts for the whole + // install: it is recorded and it turns the default trusted list off. + await Promise.all([ + write(packageJson, JSON.stringify({ name: "foo", version: "1.0.0", workspaces: ["packages/*"] })), + write(join(packageDir, "pnpm-workspace.yaml"), "packages:\n - packages/*\n"), + write( + join(packageDir, "packages", "app", "package.json"), + JSON.stringify({ name: "app", version: "1.0.0", dependencies, trustedDependencies: ["uses-what-bin"] }), + ), + write(join(packageDir, "pnpm-lock.yaml"), pnpmLock(`\n .: {}\n\n packages/app:${importerDependencies}`)), + ]); + + await expectMigrationKeepsTrust(env, packageDir, "pnpm-lock.yaml"); + }); +}); + test.concurrent("node-gyp shim directory added to lifecycle script PATH gets a randomized name", async () => { using ctx = await setupTest(); const { packageDir, packageJson, env } = ctx;