From 2d53c66fae51a94bd8a8e79320bdfa02a43efbee Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 21 May 2026 23:23:13 +0000 Subject: [PATCH 1/7] install: text lockfile must preserve empty trustedDependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting "trustedDependencies": [] in package.json blocked lifecycle scripts on the first install — the lockfile's trusted_dependencies was Some(empty_set), so has_trusted_dependency fell through to the empty membership check and correctly returned false for everything. But the text lockfile writer only emitted "trustedDependencies" when at least one installed dep matched the set (bun.lock.rs:401). With an empty set nothing matched, the key was dropped, and reloading yielded trusted_dependencies = None. has_trusted_dependency then fell through to the default allow list and silently re-trusted packages like bcrypt, electron, and esbuild. bun pm untrusted shows the same regression: it only reads the lockfile (not package.json), so after the round-trip it reports 'Found 0 untrusted dependencies with scripts' while bun install was still saying 'Blocked 1 postinstall'. Fix: - bun.lock.rs: emit trustedDependencies whenever the user defined it, even as []. trusted_dependencies.is_some() iff the key was present; None preserves the 'use Bun's defaults' semantics. - install_with_manager.rs: in the !had_any_diffs branch, also clone trusted_dependencies from the freshly-parsed package.json onto the manager's lockfile. The diff only compares dependency lists, so editing trustedDependencies alone (no dep changes) would otherwise get ignored at install time until a dep also changed. Regression test in bun-install-lifecycle-scripts.test.ts uses a file: dep so the lockfile round-trip can be verified without a registry. Reported in https://github.com/oven-sh/bun/issues/31026#issuecomment-4513423599 --- .../PackageManager/install_with_manager.rs | 14 ++++ src/install/lockfile/bun.lock.rs | 29 ++++++--- .../bun-install-lifecycle-scripts.test.ts | 65 +++++++++++++++++++ 3 files changed, 99 insertions(+), 9 deletions(-) diff --git a/src/install/PackageManager/install_with_manager.rs b/src/install/PackageManager/install_with_manager.rs index 29c825ce77a..e01c06eb2bc 100644 --- a/src/install/PackageManager/install_with_manager.rs +++ b/src/install/PackageManager/install_with_manager.rs @@ -279,6 +279,20 @@ pub fn install_with_manager( .scripts .clone_into(&lockfile.buffers.string_bytes, builder); builder.clamp(); + + // Refresh `trustedDependencies` from the latest package.json + // even when no deps changed. The diff loop only compares + // dependency lists; editing `trustedDependencies` (including + // adding or removing entries) would otherwise get ignored + // until a dep also changed. Carry `None` through unchanged so + // "key absent from package.json" still means "use Bun's + // default list". + // PORT NOTE: `ArrayHashMap::clone()` is an inherent fallible + // method, so map by hand (same as the had-diffs branch). + *lf.trusted_dependencies = match &lockfile.trusted_dependencies { + Some(td) => Some(td.clone()?), + None => None, + }; } else { // If you changed packages, we will copy over the new package from the new lockfile let new_dependencies = diff --git a/src/install/lockfile/bun.lock.rs b/src/install/lockfile/bun.lock.rs index 38c1679c852..626b8d63dcf 100644 --- a/src/install/lockfile/bun.lock.rs +++ b/src/install/lockfile/bun.lock.rs @@ -398,17 +398,28 @@ impl Stringifier { tree_sort_buf.sort_by(tree_sort_is_less_than); // PERF(port): std.sort.pdq - if found_trusted_dependencies.len() > 0 { + // Emit `trustedDependencies` whenever the user defined it in + // package.json, even as `[]` or an array of packages that aren't + // installed. The lockfile's `trusted_dependencies` is `Some(set)` + // iff the key was present; `None` means "use Bun's default allow + // list". Dropping the key when the set didn't match any installed + // dep would make a later reload silently fall back to the + // defaults, re-enabling postinstall for packages the user meant + // to block. + if lockfile.trusted_dependencies.is_some() { Self::write_indent(writer, *indent)?; - writer.write_all(b"\"trustedDependencies\": [\n")?; - *indent += 1; - for dep_name in found_trusted_dependencies.values() { - Self::write_indent(writer, *indent)?; - writeln!(writer, "\"{}\",", bstr::BStr::new(dep_name.slice(buf)))?; + if found_trusted_dependencies.is_empty() { + writer.write_all(b"\"trustedDependencies\": [],\n")?; + } else { + writer.write_all(b"\"trustedDependencies\": [\n")?; + *indent += 1; + for dep_name in found_trusted_dependencies.values() { + Self::write_indent(writer, *indent)?; + writeln!(writer, "\"{}\",", bstr::BStr::new(dep_name.slice(buf)))?; + } + Self::dec_indent(writer, indent)?; + writer.write_all(b"],\n")?; } - - Self::dec_indent(writer, indent)?; - writer.write_all(b"],\n")?; } if found_patched_dependencies.len() > 0 { diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index ff62bd618a6..5e83ee79db6 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -11,6 +11,7 @@ import { readdirSorted, runBunInstall, stderrForInstall, + tempDir, } from "harness"; import { join, sep } from "path"; @@ -3187,6 +3188,70 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) { }); } +// https://github.com/oven-sh/bun/issues/31026 +// +// `trustedDependencies: []` is supposed to block lifecycle scripts for every +// package, including the ones in Bun's built-in default allow list. On the +// first install this worked, but the text lockfile writer dropped the empty +// array (it only emitted `"trustedDependencies"` when at least one installed +// dep matched the set). On reload, `lockfile.trusted_dependencies = None` +// made `has_trusted_dependency` fall through to the default list, silently +// re-trusting packages like `bcrypt` / `electron` / `esbuild`. +test.concurrent("trustedDependencies: [] survives a lockfile round-trip", async () => { + // Use a `file:` dep so the test is self-contained (no registry needed). + // The bug we're guarding against is in the lockfile writer/reader, not in + // npm-specific install logic — the writer handles `trusted_dependencies` + // identically regardless of how deps were resolved. + using dir = tempDir("trusted-deps-empty-roundtrip", { + "local-dep/package.json": JSON.stringify({ name: "local-dep", version: "1.0.0" }), + "package.json": JSON.stringify({ + name: "roundtrip", + version: "1.0.0", + trustedDependencies: [], + dependencies: { "local-dep": "file:./local-dep" }, + }), + }); + + const env = { ...baseEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }; + + // First install writes the lockfile. The critical assertion is that the + // empty array survives — without fix A, the key would be dropped entirely. + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env, + stdout: "pipe", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect(exitCode).toBe(0); + } + + const lockfileText = await file(join(String(dir), "bun.lock")).text(); + expect(lockfileText).toContain('"trustedDependencies": []'); + + // Round-trip: reading the lockfile back must yield `Some(empty_set)` and + // not `None`. We can't inspect that directly from userspace, but a second + // install must succeed and the lockfile must still have the empty array + // (proving reader + writer are symmetric). + await rm(join(String(dir), "node_modules"), { recursive: true, force: true }); + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env, + stdout: "pipe", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect(exitCode).toBe(0); + } + expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); +}); + test.concurrent("ignore-scripts is read from npmrc", async () => { using ctx = await setupTest(); const { packageDir, packageJson, env } = ctx; From 52a2c1dcd6f43e03028c19a8a38e6960d05f8d90 Mon Sep 17 00:00:00 2001 From: robobun Date: Thu, 21 May 2026 23:57:29 +0000 Subject: [PATCH 2/7] =?UTF-8?q?install:=20flag=20None=20=E2=86=92=20Some({?= =?UTF-8?q?})=20as=20a=20diff=20on=20trustedDependencies?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up on the review in #31197: the original fix refreshed `lockfile.trusted_dependencies` from package.json unconditionally in the `!had_any_diffs` branch, which made install behaviour correct but left `bun pm untrusted` stale on disk — the lockfile save was still gated on `has_diffs()`, and `None → Some({})` (adding `trustedDependencies: []` to an existing project with no dep changes) was the one transition that produced zero added/removed entries and therefore no diff. Move the signal into `Diff::generate` case 4: when transitioning from "use defaults" (None) to an explicit empty set, flag the summary as changed so `has_diffs()` reports it. The existing had-diffs branch in `install_with_manager.rs` already clones `trusted_dependencies` onto the manager's lockfile and triggers a save, so the `!had_any_diffs`-branch refresh from the prior commit is no longer needed and is removed. Net effect: - Install still blocks postinstalls correctly on every run. - `bun.lock` is rewritten with `"trustedDependencies": []` even when only the key was added (no dep edits). - `bun pm untrusted`, which reads the lockfile directly, now sees the empty set on reload and matches `bun install`'s blocked count. Adds a second regression test covering the edit-on-existing-project scenario and updates the original round-trip test to read stdout before asserting the exit code. --- .../PackageManager/install_with_manager.rs | 14 --- src/install/lockfile/Package.rs | 22 ++++- .../bun-install-lifecycle-scripts.test.ts | 96 ++++++++++++++++++- 3 files changed, 109 insertions(+), 23 deletions(-) diff --git a/src/install/PackageManager/install_with_manager.rs b/src/install/PackageManager/install_with_manager.rs index e01c06eb2bc..29c825ce77a 100644 --- a/src/install/PackageManager/install_with_manager.rs +++ b/src/install/PackageManager/install_with_manager.rs @@ -279,20 +279,6 @@ pub fn install_with_manager( .scripts .clone_into(&lockfile.buffers.string_bytes, builder); builder.clamp(); - - // Refresh `trustedDependencies` from the latest package.json - // even when no deps changed. The diff loop only compares - // dependency lists; editing `trustedDependencies` (including - // adding or removing entries) would otherwise get ignored - // until a dep also changed. Carry `None` through unchanged so - // "key absent from package.json" still means "use Bun's - // default list". - // PORT NOTE: `ArrayHashMap::clone()` is an inherent fallible - // method, so map by hand (same as the had-diffs branch). - *lf.trusted_dependencies = match &lockfile.trusted_dependencies { - Some(td) => Some(td.clone()?), - None => None, - }; } else { // If you changed packages, we will copy over the new package from the new lockfile let new_dependencies = diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index dec11f6a1f1..3190b81625c 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -1000,6 +1000,15 @@ pub struct DiffSummary { ArrayHashMap, pub removed_trusted_dependencies: TrustedDependenciesSet, + /// Flags a `None → Some(empty)` transition on `trusted_dependencies` + /// (i.e. the user just added `"trustedDependencies": []` to package.json + /// on an existing project). Case 2/3/case-4-with-entries already flow + /// into `added_` / `removed_trusted_dependencies` so `has_diffs()` + /// reports them; the one remaining transition that has no elements to + /// put in either set is `None → Some({})`, which still needs a save so + /// the lockfile persists the empty array. + pub trusted_dependencies_changed: bool, + pub patched_dependencies_changed: bool, } @@ -1020,6 +1029,7 @@ impl DiffSummary { || self.catalogs_changed || self.added_trusted_dependencies.count() > 0 || self.removed_trusted_dependencies.count() > 0 + || self.trusted_dependencies_changed || self.patched_dependencies_changed } } @@ -1310,11 +1320,15 @@ impl Diff { summary.added_trusted_dependencies.put(to_trusted, true)?; } - { - // removed - // none - } + // Transitioning from "use defaults" (None) to an explicit set + // — even an empty one — is itself a semantic diff: the user + // just opted out of the default allow list. With an empty + // `to`, the loop above records zero additions, so flag the + // transition here so `has_diffs()` returns true and the + // lockfile is rewritten to persist the `[]`. + summary.trusted_dependencies_changed = true; + // removed: none break 'trusted_dependencies; } } diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index 5e83ee79db6..0509c2b409b 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -3215,7 +3215,7 @@ test.concurrent("trustedDependencies: [] survives a lockfile round-trip", async const env = { ...baseEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }; // First install writes the lockfile. The critical assertion is that the - // empty array survives — without fix A, the key would be dropped entirely. + // empty array survives — without the fix, the key would be dropped. { await using proc = Bun.spawn({ cmd: [bunExe(), "install"], @@ -3224,13 +3224,17 @@ test.concurrent("trustedDependencies: [] survives a lockfile round-trip", async stdout: "pipe", stderr: "pipe", }); - const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + const [stdout, stderr, exitCode] = await Promise.all([ + proc.stdout.text(), + proc.stderr.text(), + proc.exited, + ]); expect(stderr).not.toContain("error:"); + expect(stdout).toContain("1 package installed"); expect(exitCode).toBe(0); } - const lockfileText = await file(join(String(dir), "bun.lock")).text(); - expect(lockfileText).toContain('"trustedDependencies": []'); + expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); // Round-trip: reading the lockfile back must yield `Some(empty_set)` and // not `None`. We can't inspect that directly from userspace, but a second @@ -3245,13 +3249,95 @@ test.concurrent("trustedDependencies: [] survives a lockfile round-trip", async stdout: "pipe", stderr: "pipe", }); - const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + const [stdout, stderr, exitCode] = await Promise.all([ + proc.stdout.text(), + proc.stderr.text(), + proc.exited, + ]); expect(stderr).not.toContain("error:"); + expect(stdout).toContain("1 package installed"); expect(exitCode).toBe(0); } expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); }); +// Follow-up to the above: on an existing project that never had +// `trustedDependencies` in its lockfile, adding `"trustedDependencies": []` +// to package.json alone (no dep changes) must be treated as a diff — the +// `None → Some({})` transition records zero adds/removes, so without an +// explicit flag the diff walker considered it "no changes" and the lockfile +// stayed on disk without the empty array. `bun pm untrusted` (which reads +// only the lockfile) then silently fell back to the default allow list. +test.concurrent( + "trustedDependencies: [] added to an existing project triggers a lockfile save", + async () => { + using dir = tempDir("trusted-deps-added-later", { + "local-dep/package.json": JSON.stringify({ name: "local-dep", version: "1.0.0" }), + "package.json": JSON.stringify({ + name: "added-later", + version: "1.0.0", + dependencies: { "local-dep": "file:./local-dep" }, + }), + }); + + const env = { ...baseEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }; + + // Baseline install: no `trustedDependencies` key. Lockfile is written + // without the key. + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + proc.stdout.text(), + proc.stderr.text(), + proc.exited, + ]); + expect(stderr).not.toContain("error:"); + expect(stdout).toContain("1 package installed"); + expect(exitCode).toBe(0); + } + expect(await file(join(String(dir), "bun.lock")).text()).not.toContain('"trustedDependencies"'); + + // Add `"trustedDependencies": []` — no dep changes. Without the fix the + // diff summary records zero adds/removes, `had_any_diffs` stays false, + // `should_save_lockfile` is false, and the `[]` never reaches disk. + await Bun.write( + join(String(dir), "package.json"), + JSON.stringify({ + name: "added-later", + version: "1.0.0", + trustedDependencies: [], + dependencies: { "local-dep": "file:./local-dep" }, + }), + ); + + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + proc.stdout.text(), + proc.stderr.text(), + proc.exited, + ]); + expect(stderr).not.toContain("error:"); + expect(stdout).not.toContain("error"); + expect(exitCode).toBe(0); + } + + expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); + }, +); + test.concurrent("ignore-scripts is read from npmrc", async () => { using ctx = await setupTest(); const { packageDir, packageJson, env } = ctx; From 6c8a857cab414737bbc121ff9b205077d383a386 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 21 May 2026 23:59:32 +0000 Subject: [PATCH 3/7] [autofix.ci] apply automated fixes --- .../bun-install-lifecycle-scripts.test.ts | 131 ++++++++---------- 1 file changed, 56 insertions(+), 75 deletions(-) diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index 0509c2b409b..64d53f17a04 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -3224,11 +3224,7 @@ test.concurrent("trustedDependencies: [] survives a lockfile round-trip", async stdout: "pipe", stderr: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).not.toContain("error:"); expect(stdout).toContain("1 package installed"); expect(exitCode).toBe(0); @@ -3249,11 +3245,7 @@ test.concurrent("trustedDependencies: [] survives a lockfile round-trip", async stdout: "pipe", stderr: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).not.toContain("error:"); expect(stdout).toContain("1 package installed"); expect(exitCode).toBe(0); @@ -3268,75 +3260,64 @@ test.concurrent("trustedDependencies: [] survives a lockfile round-trip", async // explicit flag the diff walker considered it "no changes" and the lockfile // stayed on disk without the empty array. `bun pm untrusted` (which reads // only the lockfile) then silently fell back to the default allow list. -test.concurrent( - "trustedDependencies: [] added to an existing project triggers a lockfile save", - async () => { - using dir = tempDir("trusted-deps-added-later", { - "local-dep/package.json": JSON.stringify({ name: "local-dep", version: "1.0.0" }), - "package.json": JSON.stringify({ - name: "added-later", - version: "1.0.0", - dependencies: { "local-dep": "file:./local-dep" }, - }), - }); - - const env = { ...baseEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }; +test.concurrent("trustedDependencies: [] added to an existing project triggers a lockfile save", async () => { + using dir = tempDir("trusted-deps-added-later", { + "local-dep/package.json": JSON.stringify({ name: "local-dep", version: "1.0.0" }), + "package.json": JSON.stringify({ + name: "added-later", + version: "1.0.0", + dependencies: { "local-dep": "file:./local-dep" }, + }), + }); - // Baseline install: no `trustedDependencies` key. Lockfile is written - // without the key. - { - await using proc = Bun.spawn({ - cmd: [bunExe(), "install"], - cwd: String(dir), - env, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); - expect(stderr).not.toContain("error:"); - expect(stdout).toContain("1 package installed"); - expect(exitCode).toBe(0); - } - expect(await file(join(String(dir), "bun.lock")).text()).not.toContain('"trustedDependencies"'); + const env = { ...baseEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }; - // Add `"trustedDependencies": []` — no dep changes. Without the fix the - // diff summary records zero adds/removes, `had_any_diffs` stays false, - // `should_save_lockfile` is false, and the `[]` never reaches disk. - await Bun.write( - join(String(dir), "package.json"), - JSON.stringify({ - name: "added-later", - version: "1.0.0", - trustedDependencies: [], - dependencies: { "local-dep": "file:./local-dep" }, - }), - ); + // Baseline install: no `trustedDependencies` key. Lockfile is written + // without the key. + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect(stdout).toContain("1 package installed"); + expect(exitCode).toBe(0); + } + expect(await file(join(String(dir), "bun.lock")).text()).not.toContain('"trustedDependencies"'); + + // Add `"trustedDependencies": []` — no dep changes. Without the fix the + // diff summary records zero adds/removes, `had_any_diffs` stays false, + // `should_save_lockfile` is false, and the `[]` never reaches disk. + await Bun.write( + join(String(dir), "package.json"), + JSON.stringify({ + name: "added-later", + version: "1.0.0", + trustedDependencies: [], + dependencies: { "local-dep": "file:./local-dep" }, + }), + ); - { - await using proc = Bun.spawn({ - cmd: [bunExe(), "install"], - cwd: String(dir), - env, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); - expect(stderr).not.toContain("error:"); - expect(stdout).not.toContain("error"); - expect(exitCode).toBe(0); - } + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect(stdout).not.toContain("error"); + expect(exitCode).toBe(0); + } - expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); - }, -); + expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); +}); test.concurrent("ignore-scripts is read from npmrc", async () => { using ctx = await setupTest(); From 0f0ec9e1472ba8014a79f0b1599a47b4ee9a1fbe Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 22 May 2026 00:28:58 +0000 Subject: [PATCH 4/7] ci: retrigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The debian-13-x64-asan-test-bun failure on the previous CI run hit test/js/bun/http/serve-body-leak.test.ts, which is a well-documented ASAN-flaky memory-leak test (see PR #28301 and test/no-validate-leaksan.txt:274). This PR only touches src/install/lockfile/ and test/cli/install/ — nothing in the HTTP/Bun.serve path. Re-rolling once. From f2f8927d6a4b8c787ce2f67976166dcf4f73f4d9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 27 May 2026 12:41:28 +0000 Subject: [PATCH 5/7] test: prove trustedDependencies: [] blocks default-trusted npm postinstall across reload Add end-to-end coverage using electron (on Bun's built-in default allow list, with a preinstall script) to exercise the actual security property: with trustedDependencies: [], the postinstall stays blocked on the first install AND on reinstall from the persisted lockfile. On main the text lockfile dropped the empty array, so the reload fell back to the default allow list and electron's preinstall ran again. The existing file: round-trip tests guard the lockfile serialization but can't catch this regression: file: deps carry a non-Npm tag, and the default-list fallback only applies to Npm-tag packages, so they are 'not trusted' under both Some(empty) and None. Also adds the Some -> None case: removing the trustedDependencies key drops it from bun.lock and returns the package to the default allow list. --- .../bun-install-lifecycle-scripts.test.ts | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index 64d53f17a04..470cf20ea86 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -3319,6 +3319,154 @@ test.concurrent("trustedDependencies: [] added to an existing project triggers a expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); }); +// End-to-end version of the round-trip test above, exercising the actual +// security property: `electron` is on Bun's built-in default allow list and +// its registry fixture has a `preinstall` script that writes `preinstall.txt`. +// With `trustedDependencies: []` the postinstall must stay blocked on *every* +// install. On `main` the text lockfile dropped the empty array (electron +// didn't match the empty set, so the writer emitted no key), so reloading +// yielded `trusted_dependencies = None`, the default allow list re-applied, +// and electron's preinstall ran on the reinstall. The `file:` tests above +// can't catch this: `file:` deps carry a non-Npm tag, and the default-list +// fallback only applies to Npm-tag packages, so they're "not trusted" under +// both `Some(empty)` and `None`. +test.concurrent("trustedDependencies: [] keeps a default-trusted package blocked across reinstall", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + + await writeFile( + packageJson, + JSON.stringify({ + name: "foo", + version: "1.2.3", + trustedDependencies: [], + dependencies: { + electron: "1.0.0", + }, + }), + ); + + // First install: electron is default-trusted, but `trustedDependencies: []` + // opts out of the default list, so its preinstall must be blocked. + { + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const err = await stderr.text(); + expect(err).toContain("Saved lockfile"); + expect(err).not.toContain("not found"); + expect(err).not.toContain("error:"); + const out = await stdout.text(); + expect(out).toContain("+ electron@1.0.0"); + expect(out).toContain("Blocked 1 postinstall"); + expect(await exited).toBe(0); + assertManifestsPopulated(join(packageDir, ".bun-cache"), verdaccio.registryUrl()); + } + + // The empty array must be persisted in the text lockfile, otherwise the + // reload below sees `None` and falls back to the default allow list. + expect(await file(join(packageDir, "bun.lock")).text()).toContain('"trustedDependencies": []'); + expect(await exists(join(packageDir, "node_modules", "electron", "preinstall.txt"))).toBeFalse(); + + // Reinstall from the persisted lockfile. This is where `main` regresses: + // the dropped key makes the defaults re-apply and electron's preinstall + // runs. With the fix, `[]` round-trips and the postinstall stays blocked. + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + { + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const err = await stderr.text(); + expect(err).not.toContain("not found"); + expect(err).not.toContain("error:"); + const out = await stdout.text(); + expect(out).toContain("Blocked 1 postinstall"); + expect(await exited).toBe(0); + } + + expect(await file(join(packageDir, "bun.lock")).text()).toContain('"trustedDependencies": []'); + expect(await exists(join(packageDir, "node_modules", "electron", "preinstall.txt"))).toBeFalse(); +}); + +// Bonus transition from the review: deleting the `trustedDependencies` key +// from package.json (`Some → None`) must drop it from the text lockfile too, +// so a reload correctly returns to the default allow list. Start from `[]` +// (which blocks electron), then remove the key and confirm electron becomes +// trusted again and the lockfile no longer carries the array. +test.concurrent("removing trustedDependencies drops the key from the lockfile", async () => { + using ctx = await setupTest(); + const { packageDir, packageJson, env } = ctx; + + await writeFile( + packageJson, + JSON.stringify({ + name: "foo", + version: "1.2.3", + trustedDependencies: [], + dependencies: { + electron: "1.0.0", + }, + }), + ); + + { + const { stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const err = await stderr.text(); + expect(err).not.toContain("error:"); + expect(await exited).toBe(0); + } + expect(await file(join(packageDir, "bun.lock")).text()).toContain('"trustedDependencies": []'); + expect(await exists(join(packageDir, "node_modules", "electron", "preinstall.txt"))).toBeFalse(); + + // Remove the key entirely → back to the default allow list. + await writeFile( + packageJson, + JSON.stringify({ + name: "foo", + version: "1.2.3", + dependencies: { + electron: "1.0.0", + }, + }), + ); + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + + { + const { stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const err = await stderr.text(); + expect(err).not.toContain("error:"); + expect(await exited).toBe(0); + } + + const lockAfter = await file(join(packageDir, "bun.lock")).text(); + expect(lockAfter).not.toContain("trustedDependencies"); + expect(await exists(join(packageDir, "node_modules", "electron", "preinstall.txt"))).toBeTrue(); +}); + test.concurrent("ignore-scripts is read from npmrc", async () => { using ctx = await setupTest(); const { packageDir, packageJson, env } = ctx; From 343104eabe7e7d0277a7d5081066845ca049f148 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 27 May 2026 16:57:26 +0000 Subject: [PATCH 6/7] test: read stdout in removal test and gate file: tests on the concurrency semaphore --- .../bun-install-lifecycle-scripts.test.ts | 224 ++++++++++-------- 1 file changed, 123 insertions(+), 101 deletions(-) diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index 470cf20ea86..3d868965139 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -3198,59 +3198,66 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) { // made `has_trusted_dependency` fall through to the default list, silently // re-trusting packages like `bcrypt` / `electron` / `esbuild`. test.concurrent("trustedDependencies: [] survives a lockfile round-trip", async () => { - // Use a `file:` dep so the test is self-contained (no registry needed). - // The bug we're guarding against is in the lockfile writer/reader, not in - // npm-specific install logic — the writer handles `trusted_dependencies` - // identically regardless of how deps were resolved. - using dir = tempDir("trusted-deps-empty-roundtrip", { - "local-dep/package.json": JSON.stringify({ name: "local-dep", version: "1.0.0" }), - "package.json": JSON.stringify({ - name: "roundtrip", - version: "1.0.0", - trustedDependencies: [], - dependencies: { "local-dep": "file:./local-dep" }, - }), - }); + // No registry needed (`file:` dep), so we skip setupTest(), but we still + // gate on the file-level concurrency semaphore like every other test. + await acquireSlot(); + try { + // Use a `file:` dep so the test is self-contained (no registry needed). + // The bug we're guarding against is in the lockfile writer/reader, not in + // npm-specific install logic — the writer handles `trusted_dependencies` + // identically regardless of how deps were resolved. + using dir = tempDir("trusted-deps-empty-roundtrip", { + "local-dep/package.json": JSON.stringify({ name: "local-dep", version: "1.0.0" }), + "package.json": JSON.stringify({ + name: "roundtrip", + version: "1.0.0", + trustedDependencies: [], + dependencies: { "local-dep": "file:./local-dep" }, + }), + }); - const env = { ...baseEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }; + const env = { ...baseEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }; - // First install writes the lockfile. The critical assertion is that the - // empty array survives — without the fix, the key would be dropped. - { - await using proc = Bun.spawn({ - cmd: [bunExe(), "install"], - cwd: String(dir), - env, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).not.toContain("error:"); - expect(stdout).toContain("1 package installed"); - expect(exitCode).toBe(0); - } + // First install writes the lockfile. The critical assertion is that the + // empty array survives — without the fix, the key would be dropped. + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect(stdout).toContain("1 package installed"); + expect(exitCode).toBe(0); + } - expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); + expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); - // Round-trip: reading the lockfile back must yield `Some(empty_set)` and - // not `None`. We can't inspect that directly from userspace, but a second - // install must succeed and the lockfile must still have the empty array - // (proving reader + writer are symmetric). - await rm(join(String(dir), "node_modules"), { recursive: true, force: true }); - { - await using proc = Bun.spawn({ - cmd: [bunExe(), "install"], - cwd: String(dir), - env, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).not.toContain("error:"); - expect(stdout).toContain("1 package installed"); - expect(exitCode).toBe(0); + // Round-trip: reading the lockfile back must yield `Some(empty_set)` and + // not `None`. We can't inspect that directly from userspace, but a second + // install must succeed and the lockfile must still have the empty array + // (proving reader + writer are symmetric). + await rm(join(String(dir), "node_modules"), { recursive: true, force: true }); + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect(stdout).toContain("1 package installed"); + expect(exitCode).toBe(0); + } + expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); + } finally { + releaseSlot(); } - expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); }); // Follow-up to the above: on an existing project that never had @@ -3261,62 +3268,69 @@ test.concurrent("trustedDependencies: [] survives a lockfile round-trip", async // stayed on disk without the empty array. `bun pm untrusted` (which reads // only the lockfile) then silently fell back to the default allow list. test.concurrent("trustedDependencies: [] added to an existing project triggers a lockfile save", async () => { - using dir = tempDir("trusted-deps-added-later", { - "local-dep/package.json": JSON.stringify({ name: "local-dep", version: "1.0.0" }), - "package.json": JSON.stringify({ - name: "added-later", - version: "1.0.0", - dependencies: { "local-dep": "file:./local-dep" }, - }), - }); + // No registry needed (`file:` dep), so we skip setupTest(), but we still + // gate on the file-level concurrency semaphore like every other test. + await acquireSlot(); + try { + using dir = tempDir("trusted-deps-added-later", { + "local-dep/package.json": JSON.stringify({ name: "local-dep", version: "1.0.0" }), + "package.json": JSON.stringify({ + name: "added-later", + version: "1.0.0", + dependencies: { "local-dep": "file:./local-dep" }, + }), + }); - const env = { ...baseEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }; + const env = { ...baseEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }; - // Baseline install: no `trustedDependencies` key. Lockfile is written - // without the key. - { - await using proc = Bun.spawn({ - cmd: [bunExe(), "install"], - cwd: String(dir), - env, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).not.toContain("error:"); - expect(stdout).toContain("1 package installed"); - expect(exitCode).toBe(0); - } - expect(await file(join(String(dir), "bun.lock")).text()).not.toContain('"trustedDependencies"'); + // Baseline install: no `trustedDependencies` key. Lockfile is written + // without the key. + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect(stdout).toContain("1 package installed"); + expect(exitCode).toBe(0); + } + expect(await file(join(String(dir), "bun.lock")).text()).not.toContain('"trustedDependencies"'); - // Add `"trustedDependencies": []` — no dep changes. Without the fix the - // diff summary records zero adds/removes, `had_any_diffs` stays false, - // `should_save_lockfile` is false, and the `[]` never reaches disk. - await Bun.write( - join(String(dir), "package.json"), - JSON.stringify({ - name: "added-later", - version: "1.0.0", - trustedDependencies: [], - dependencies: { "local-dep": "file:./local-dep" }, - }), - ); + // Add `"trustedDependencies": []` — no dep changes. Without the fix the + // diff summary records zero adds/removes, `had_any_diffs` stays false, + // `should_save_lockfile` is false, and the `[]` never reaches disk. + await Bun.write( + join(String(dir), "package.json"), + JSON.stringify({ + name: "added-later", + version: "1.0.0", + trustedDependencies: [], + dependencies: { "local-dep": "file:./local-dep" }, + }), + ); - { - await using proc = Bun.spawn({ - cmd: [bunExe(), "install"], - cwd: String(dir), - env, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).not.toContain("error:"); - expect(stdout).not.toContain("error"); - expect(exitCode).toBe(0); - } + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("error:"); + expect(stdout).not.toContain("error"); + expect(exitCode).toBe(0); + } - expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); + expect(await file(join(String(dir), "bun.lock")).text()).toContain('"trustedDependencies": []'); + } finally { + releaseSlot(); + } }); // End-to-end version of the round-trip test above, exercising the actual @@ -3420,7 +3434,7 @@ test.concurrent("removing trustedDependencies drops the key from the lockfile", ); { - const { stderr, exited } = spawn({ + const { stdout, stderr, exited } = spawn({ cmd: [bunExe(), "install"], cwd: packageDir, stdout: "pipe", @@ -3430,6 +3444,9 @@ test.concurrent("removing trustedDependencies drops the key from the lockfile", }); const err = await stderr.text(); expect(err).not.toContain("error:"); + const out = await stdout.text(); + expect(out).toContain("+ electron@1.0.0"); + expect(out).toContain("Blocked 1 postinstall"); expect(await exited).toBe(0); } expect(await file(join(packageDir, "bun.lock")).text()).toContain('"trustedDependencies": []'); @@ -3449,7 +3466,7 @@ test.concurrent("removing trustedDependencies drops the key from the lockfile", await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); { - const { stderr, exited } = spawn({ + const { stdout, stderr, exited } = spawn({ cmd: [bunExe(), "install"], cwd: packageDir, stdout: "pipe", @@ -3459,6 +3476,11 @@ test.concurrent("removing trustedDependencies drops the key from the lockfile", }); const err = await stderr.text(); expect(err).not.toContain("error:"); + const out = await stdout.text(); + expect(out).toContain("+ electron@1.0.0"); + // Key removed → default allow list re-applies, so electron's preinstall + // runs and nothing is blocked. + expect(out).not.toContain("Blocked"); expect(await exited).toBe(0); } From c67d21a9cf2eff06c964ee5175614defc86136da Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 27 May 2026 17:43:43 +0000 Subject: [PATCH 7/7] test: assert preinstall didn't run instead of the blocked-summary line on reinstall The text lockfile doesn't persist per-package script metadata, so the "Blocked N postinstall" summary is not emitted on a reinstall from bun.lock even though the package stays correctly blocked. Assert the deterministic ground truth (electron's preinstall.txt is absent) instead of the cosmetic summary line, which was failing CI on the reinstall step. --- test/cli/install/bun-install-lifecycle-scripts.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index 3d868965139..86685dfcce9 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -3404,10 +3404,15 @@ test.concurrent("trustedDependencies: [] keeps a default-trusted package blocked expect(err).not.toContain("not found"); expect(err).not.toContain("error:"); const out = await stdout.text(); - expect(out).toContain("Blocked 1 postinstall"); + expect(out).toContain("+ electron@1.0.0"); expect(await exited).toBe(0); } + // The ground truth for "stayed blocked" is that electron's preinstall never + // ran, so `preinstall.txt` is absent. We deliberately don't assert on the + // "Blocked N postinstall" summary here: the text lockfile doesn't persist + // per-package script metadata, so that cosmetic line isn't emitted on a + // reinstall from `bun.lock` even though the package is correctly blocked. expect(await file(join(packageDir, "bun.lock")).text()).toContain('"trustedDependencies": []'); expect(await exists(join(packageDir, "node_modules", "electron", "preinstall.txt"))).toBeFalse(); });