install: preserve empty trustedDependencies across install + lockfile round-trip

[robobun]

What

Reported in #31026 (comment) — companion fix for the docs PR #31027.

Setting "trustedDependencies": [] in package.json blocked lifecycle scripts on the first install but silently stopped working on every subsequent one. With the default allow list re-applying under the user's nose, packages like bcrypt, electron, and esbuild had their postinstalls run even though the user had asked to trust nothing.

Repro (before this PR)

$ cat package.json
{ "trustedDependencies": [], "dependencies": { "bcrypt": "5.1.1" } }

$ bun install
Blocked 1 postinstall. Run `bun pm untrusted` for details.    ← correct

$ bun pm untrusted
Found 0 untrusted dependencies with scripts.                  ← disagrees with install!

$ rm -rf node_modules && bun install
58 packages installed                                         ← no "Blocked" message
$ find node_modules/bcrypt -name '*.node'
node_modules/bcrypt/lib/binding/napi-v3/bcrypt_lib.node       ← postinstall RAN

Root cause

Two separate layers dropped the signal:

1. Text lockfile writer (src/install/lockfile/bun.lock.rs) only emitted "trustedDependencies" when at least one installed dep matched the set. With [] (or any list whose names aren't yet installed), nothing matched, the key was dropped, and reloading yielded trusted_dependencies = None.

   has_trusted_dependency (src/install/lockfile.rs):

   if let Some(trusted_dependencies) = &self.trusted_dependencies {
       return trusted_dependencies.contains(&hash);   // explicit list — empty means trust nothing
   }
   resolution.tag == ResolutionTag::Npm && default_trusted_dependencies::has(name)
   //                                      ^^^ fallback — "use Bun's defaults"

   The two code paths depend on Some vs None, so losing Some(empty) quietly flips the contract.

2. Install-time diff (src/install/lockfile/Package.rs, Diff::generate): a None → Some(empty) transition on trusted_dependencies — adding "trustedDependencies": [] to an existing project with no dependency changes — produced zero added/removed entries, so has_diffs() returned false and the lockfile was never rewritten, leaving the stale None on disk.

Fix

• bun.lock.rs: emit trustedDependencies whenever the user defined it, even as []. trusted_dependencies.is_some() iff the key was present; None still means "use Bun's defaults".
• Package.rs: in Diff::generate case 4, flag the None → Some({}) transition via a new trusted_dependencies_changed summary field so has_diffs() reports it and the lockfile is rewritten to persist the [].

Verification

New regression tests in test/cli/install/bun-install-lifecycle-scripts.test.ts, covering both a fresh-install round-trip and editing trustedDependencies: [] onto an existing project with no dep changes. Uses a file: dep so the lockfile round-trip can be verified without a registry.

• Fails on main: lockfile is missing the "trustedDependencies": [] line after install.
• Passes with this PR: both installs write the key and the lockfile survives the round-trip.

The full repro from the issue comment (bcrypt + rm -rf node_modules && bun install + bun pm untrusted) also works end-to-end with this build.

Related

Companion to #31027 (docs clarification). Together these land the behavior the docs describe.

[robobun]

^{Updated 7:15 AM PT - May 28th, 2026}

❌ @alii, your commit bb0b233 has some failures in Build #58621 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 31197

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

bun-31197 --bun

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Changes enable explicit empty trustedDependencies: [] to persist through lockfile round-trips. A new diff flag detects a None→Some(empty) transition, the serializer emits the field when present (including []), and regression tests validate persistence, round-trip, blocking semantics, and removal.

Changes

Trusted Dependencies Empty Set Persistence

Layer / File(s)	Summary
Diff detection for trusted dependencies changes src/install/lockfile/Package.rs	DiffSummary adds trusted_dependencies_changed to flag the None → Some(empty) transition. has_diffs() includes this flag. Diff::generate sets it when lockfile.trusted_dependencies moves from None to Some(...).
Lockfile serialization of trusted dependencies src/install/lockfile/bun.lock.rs	The serializer now checks lockfile.trusted_dependencies.is_some() and emits "trustedDependencies": [] when the explicit set is present but yields no matches, instead of omitting the key.
Round-trip and semantics regression tests test/cli/install/bun-install-lifecycle-scripts.test.ts	Adds tempDir import and multiple concurrent regression tests validating that trustedDependencies: [] is written to bun.lock, survives reinstall, blocks default-trusted packages when persisted, can be added to an existing project to force a lockfile update, and is removed from bun.lock when deleted from package.json.

Possibly related PRs:

• oven-sh/bun#31218: Also touches lockfile trustedDependencies handling; changes the underlying trusted-dependency representation/matching while this PR changes persistence/serialization of an explicit empty set.

Suggested reviewers:

• alii

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: preserving empty trustedDependencies across install and lockfile round-trip, which is the primary focus of this PR.
Description check	✅ Passed	The description provides both required sections: a comprehensive 'What' explaining the bug, reproduction steps, root cause analysis, and the fix; and a detailed 'How did you verify' section with regression tests and verification approach.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/cli/install/bun-install-lifecycle-scripts.test.ts`:
- Around line 3227-3229: The test currently consumes stderr and exitCode
together (const [stderr, exitCode] = await Promise.all([proc.stderr.text(),
proc.exited])) which checks exitCode before reading stdout; update the spawn
checks to also read proc.stdout.text() and assert stdout contents before
asserting exitCode. Specifically, change to await
Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]) (assigning to
stderr, stdout, exitCode), add the appropriate expect on stdout (e.g.,
expect(stdout).not.toContain("error:") or other existing stdout assertions) and
then assert expect(exitCode).toBe(0); apply the same change to both blocks that
use proc, stderr, and exitCode in this test file.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6998c84f-2f99-4a42-bc01-fe1cfed3b827

📥 Commits

Reviewing files that changed from the base of the PR and between 832edbc and 2d53c66.

📒 Files selected for processing (3)

• src/install/PackageManager/install_with_manager.rs
• src/install/lockfile/bun.lock.rs
• test/cli/install/bun-install-lifecycle-scripts.test.ts

[robobun]

Added the end-to-end test from the review: install electron (a default-trusted package with a preinstall script) with trustedDependencies: [], then rm -rf node_modules and reinstall. The postinstall must stay blocked on every install.

• Fails on main: the reinstall re-runs electron's preinstall (the dropped empty array makes the defaults re-apply).
• Passes with this PR: [] round-trips through bun.lock and the postinstall stays blocked.

Also covered the Some → None case (deleting the key returns a package to the default allow list). Fix in #31197.

[alii]

Did a thorough pass on this — walked the writer, parser, the full Diff transition matrix, the binary/migration paths, workspaces, every trust consumer, --frozen-lockfile, and parity with #31027. The fix is correct and sufficient — behavior now matches the documented contract (omitted → defaults, ["a"] → only a, [] → trust nothing).

Both halves are load-bearing:

• Writer (bun.lock.rs): the first install now persists [], so it reloads as Some(empty) instead of the key vanishing.
• Diff flag (Package.rs trusted_dependencies_changed): handles the None → Some({}) transition — adding [] to an existing project, or upgrading from a lockfile written by the buggy version. Without it had_any_diffs stays false, so package.json's Some({}) is never copied into the in-memory lockfile and the lockfile is never rewritten. It's genuinely non-redundant since generate_meta_hash excludes trusted_dependencies.

Also nice that this brings the text lockfile in line with what bun.lockb already did via HAS_EMPTY_TRUSTED_DEPENDENCIES_TAG. Parser is already symmetric ([] ⟷ Some(empty)), has_trusted_dependency already treats Some(empty) as "trust nothing" (never falls through to the default list), and it merges clean with main despite the #31218 TrustedDependenciesSet change — no rebase needed.

One real ask (non-blocking): the tests don't prove the security property

Both added tests use a file: dep and assert only that bun.lock contains "trustedDependencies": []. But a file: dep is a non-Npm tag, and the default-allow-list fallback only applies to Npm-tag packages — so under both Some(empty) and None a file: dep is "not trusted." These tests guard serialization, but they can't actually exercise the regression (a default-trusted npm package's postinstall re-running after reload).

Could you add one end-to-end test — essentially automating the repro in the description? electron is already on the default-trusted list and has a registry fixture with a preinstall script, and this file already uses the exists(node_modules/electron/preinstall.txt) pattern — so: install electron with trustedDependencies: [], reload/re-install, assert preinstall.txt does not exist. That fails on main (defaults re-apply → preinstall runs) and passes here, which is the property that actually matters. A Some → None (delete-the-key) case dropping the key from bun.lock would be a nice bonus.

Two pre-existing things I noticed while in here (out of scope, just flagging): Some({the full default set}) → None isn't detected as a diff, and --frozen-lockfile won't flag a stale trustedDependencies key because eql/meta-hash both exclude it.

[alii]

@robobun ^

[mizulu]

@robobun wake up and read the comments.

[robobun]

Added the end-to-end test you asked for in f2f8927 — it automates the repro with electron (on the default allow list, with a preinstall script):

• Install electron with trustedDependencies: [] → first install blocks the postinstall and persists "trustedDependencies": [] in bun.lock.
• rm -rf node_modules + reinstall → the postinstall stays blocked and preinstall.txt is never written.

That second install is where main regresses: the dropped empty array makes the reload fall back to the default allow list, so electron's preinstall runs again. Verified it fails on main (preinstall.txt appears after reinstall) and passes here.

Also added the Some → None bonus: deleting the trustedDependencies key drops it from bun.lock and returns electron to the default allow list, which is the correct behavior for that transition.

Review nits addressed (343104e); reinstall step asserts the deterministic ground truth — electron's preinstall.txt stays absent — rather than the Blocked N postinstall summary line that the text lockfile doesn't emit on a reinstall (c67d21a).

Status (after the latest main merge, bb0b233): all 16 test-bun lanes are green and bun-install-lifecycle-scripts.test.ts appears in zero failure annotations. The finished build has no ERROR annotations — the only failure status is the overall buildkite rollup, which flips when flaky tests consume retries during the run (those retries passed). The flaky tests are all unrelated to this PR (leak tests, s3.*, webview-chrome, napi, update_interactive_install). The diff is green and ready to merge.

The two pre-existing gaps you flagged (Some({full default set}) → None not detected as a diff, and --frozen-lockfile not flagging a stale key since eql/meta-hash exclude trustedDependencies) are out of scope here — happy to follow up in a separate PR if you want them closed.

[alii]

@robobun two small review nits still open on test/cli/install/bun-install-lifecycle-scripts.test.ts:

1. both spawn blocks set stdout: "pipe" but only destructure { stderr, exited } — pull stdout in too (or drop the pipe).
2. the two tempDir-based tests are declared test.concurrent but don't go through setupTest.

Can you tidy those up and push? Otherwise this looks ready.

[robobun]

Both tidied up in 343104e:

1. Both spawn blocks in the removal test now destructure { stdout, stderr, exited } and assert stdout before the exit code.
2. The two tempDir tests now go through acquireSlot() / releaseSlot() (wrapped in try/finally) so they respect the file-level semaphore — they keep skipping setupTest() since they do not need the registry.

Resolved both review threads.