Skip to content

install: record package.json trustedDependencies when migrating a lockfile - #38773

Closed
robobun wants to merge 3 commits into
mainfrom
farm/655f4ddd/migrate-trusted-dependencies
Closed

robobun wants to merge 3 commits into
mainfrom
farm/655f4ddd/migrate-trusted-dependencies

Conversation

@robobun

@robobun robobun commented Aug 15, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #38193

Problem

  • bun pm migrate writes a bun.lock without the project's trustedDependencies, for all three sources (package-lock.json, yarn.lock, pnpm-lock.yaml). A fresh bun install records them.
  • Cause: none of those lockfiles carry the field, and the migrators only read package.json for other things (migration.rs / npm_lock.rs apply_root_overrides, yarn.rs parse_root_overrides, the importer manifests in pnpm.rs), so Lockfile::trusted_dependencies stays None when package_manager_command.rs saves the migrated lockfile.
  • Effect (the issue's bun pm migrate + bun install --frozen-lockfile + bun pm untrusted flow): the frozen install applies package.json's list in memory and runs the right scripts, but never writes the lockfile back, so bun pm untrusted / bun pm trust (which read the lockfile, pm_trusted_command.rs) report the default trusted list instead of the project's: the trusted package is listed as blocked and the packages the list blocks are not. A plain bun install fixes the lockfile, a CI install never does.
  • Once install: fail --frozen-lockfile on manifest drift and fix the spurious lockfile re-saves behind it #33632 lands (a trustedDependencies list that the lockfile does not record fails --frozen-lockfile), a migrated lockfile would fail bun ci outright without this change.

Fix

  • migration.rs: after any successful migration, record_trusted_dependencies reads the root package.json and every entry of lockfile.workspace_paths (the members each migrator found) through the workspace package.json cache and appends their trustedDependencies to the lockfile. A malformed field fails the migration with the same error bun install prints for it.
  • Package.rs: the parsing moves out of parse_with_json into parse_append_trusted_dependencies, used by both, so a migrated lockfile records exactly what a fresh install records: the union of the root's and the members' lists, and Some(empty) when a list is declared, which is what turns the default list off.
  • Members are covered for package-lock.json and pnpm-lock.yaml. yarn.lock migration does not migrate workspace members at all yet (its own test carries the TODO), so there only the root's list is recorded; a lockfile migrated from a yarn workspace fails --frozen-lockfile anyway and the plain install that follows records the members' lists itself. Whatever adds yarn workspace migration fills workspace_paths like the other two and is picked up here unchanged.
  • In-memory migration during bun install was already corrected by the differ afterwards; it now produces the same lockfile directly.
  • Verified: test/cli/install/bun-install-lifecycle-scripts.test.ts, trustedDependencies survive lockfile migration (4 tests: root list for each of the three sources, plus a pnpm workspace whose member declares the list). Each runs bun pm migrate, checks the field in bun.lock, installs with --frozen-lockfile, checks that uses-what-bin's script ran and default-trusted electron's did not, and checks bun pm untrusted lists only electron. All four fail on the released binary at the bun.lock check and pass with this change.
  • Also run: the rest of that file, test/cli/install/migration/ (remaining failures there need network or are the known 5s debug timeouts of yarn-cli-repo / the pnpm comprehensive case, which pass when run alone), cargo clippy -p bun_install, the source lints.

Background

  • trustedDependencies is the package.json allow-list for dependency lifecycle scripts. When no package.json in the project declares it, bun uses its built-in default list; declaring it (in the root or in any workspace member, even empty) replaces the default list for the whole install. Lockfile::trusted_dependencies is Option for that reason, and bun.lock stores the field so later installs and bun pm untrusted / trust see the same list.
  • Lockfile migration (bun pm migrate, or automatically when bun install finds only a foreign lockfile) builds a Lockfile from the other package manager's file. detect_and_load_other_lockfile dispatches to the three migrators and is where the shared post-step lives.
  • The differ (Diff::generate) compares the loaded lockfile with package.json on every install and applies differences in memory; --frozen-lockfile still does that but never saves, which is why the bug only shows up through the saved file.

…kfile

package-lock.json, yarn.lock and pnpm-lock.yaml do not carry
trustedDependencies, and the migrators never read them from package.json,
so the bun.lock written by `bun pm migrate` lacked the field. A later
install applied the package.json list in memory, but --frozen-lockfile
never writes the lockfile back, so `bun pm untrusted` and `bun pm trust`
kept reading the default trusted list instead of the project's.

Every migration now records the root's and each workspace member's
trustedDependencies the same way Package::parse_with_json does for a
fresh install, sharing the parsing code with it.

Fixes #38193
@coderabbitai

coderabbitai Bot commented Aug 15, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7ec24e68-d01e-426f-8fc5-fb5e73bf2dfa

📥 Commits

Reviewing files that changed from the base of the PR and between 4bf3f36 and 85d575b.

📒 Files selected for processing (3)
  • src/install/lockfile/Package.rs
  • src/install/migration.rs
  • test/cli/install/bun-install-lifecycle-scripts.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Aug 15, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:05 PM PT - Aug 14th, 2026

❌ @robobun, your commit 85d575b has some failures in Build #97010 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 38773

That installs a local version of the PR into your bun-38773 executable, so you can run:

bun-38773 --bun

@robobun

robobun commented Aug 15, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review. CI build 97010 ran 177/179 jobs green; the remaining two (darwin 14 aarch64 - test-bun) expired in the queue without running (the 6 agents on that queue were busy for 5h), and the only annotated tests are unrelated files that passed on retry (napi, issue 07261, importstar_ts, inspect-error-leak, text-loader, and a peer-hoisting case in bun-install-registry on Windows aarch64). Nothing in the diff is platform specific. The two expired jobs can be retried from Buildkite without a new push if a darwin run is wanted before merging.

Reproduced #38193 against the test registry on the released build: bun pm migrate from package-lock.json, yarn.lock and pnpm-lock.yaml all write a bun.lock without trustedDependencies; after bun install --frozen-lockfile, bun pm untrusted lists the package the project trusts and omits the one the list blocks (the install itself already ran the right scripts). With this branch the migrated bun.lock records the field and bun pm untrusted matches the install.

The missing Blocked N postinstalls line mentioned in the issue is a separate, pre-existing gap (bun.lock does not store hasInstallScript), tracked in #35818.

Comment thread src/install/lockfile/Package.rs Outdated
Comment thread src/install/migration.rs Outdated
Comment thread src/install/migration.rs
@dylan-conway

Copy link
Copy Markdown
Member

why does this read trustedDependencies from all workspace package.jsons? isn't it normally the root package.json we read from?

also pnpm trusted dependencies are called onlyBuiltDependencies right?

@robobun

robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

Because that is what bun install reads. Features::WORKSPACE has trusted_dependencies: true (src/install_types/resolver_hooks.rs, same as MAIN), and it has since trustedDependencies was added in #3288, so Package::parse_with_json appends each member's list to lockfile.trusted_dependencies alongside the root's, and the differ copies that union into the loaded lockfile on later installs. I checked on main with a workspace whose root has no trustedDependencies and whose member declares ["uses-what-bin"]: a fresh bun install writes "trustedDependencies": ["uses-what-bin"] to bun.lock, runs uses-what-bin's script and blocks electron. This PR only makes the migrated lockfile match that. If migration read the root alone, a migrated lockfile would differ from a fresh install's for such repos, and with #33632 the first --frozen-lockfile install would fail on it. If root-only is the model you want, the place to change is the install side, and I would make migration follow it in the same change; just say which.

Yes, pnpm's equivalent is onlyBuiltDependencies (pnpm.onlyBuiltDependencies in package.json, or top level in pnpm-workspace.yaml with pnpm 10, which also blocks builds by default like bun does). This PR does not translate it: it only fixes the issue's case, where the user already wrote bun's own trustedDependencies in package.json and bun pm migrate dropped it. Mapping onlyBuiltDependencies to trustedDependencies would go in update_package_json_after_migration in pnpm.rs, next to the existing pnpm.overrides and pnpm.patchedDependencies moves, and the lockfile step added here would then pick it up since it reads package.json after the migrator returns. Happy to add that here or in a follow-up PR, whichever you prefer.

@robobun

robobun commented Sep 8, 2026 •

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #41908, which carries this change (trustedDependencies, including workspace members' lists, recorded by all three migrators) together with its four tests, plus patchedDependencies and configVersion parity. Fixes #38193 there.

#41908 now also uses this PR's shape for it: one post-migration step with the same trustedDependencies parser as bun install, so a malformed field fails the migration with install's error.

@robobun robobun closed this Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

trustedDependencies has no effect after bun pm migrate from pnpm-lock.yaml; scripts silently blocked

3 participants