Improve alias tests per CodeRabbit review

- Add verification that skipped packages don't appear in 'bun pm untrusted' output
- Add comment explaining that skipScriptsFrom accepts both alias and canonical names

This ensures the tests comprehensively verify that skipped packages are not
just excluded from the blocked count, but also from the untrusted dependencies
list entirely.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

src/install/PackageInstaller.zig

@@ -1125,30 +1125,37 @@ pub const PackageInstaller = struct {
                             // these will never be blocked
                         },
                         else => if (!is_trusted and this.metas[package_id].hasInstallScript()) {
-                            // Check if the package actually has scripts. `hasInstallScript` can be false positive if a package is published with
-                            // an auto binding.gyp rebuild script but binding.gyp is excluded from the published files.
-                            var folder_path: bun.AbsPath(.{ .sep = .auto }) = .from(this.node_modules.path.items);
-                            defer folder_path.deinit();
-                            folder_path.append(alias.slice(this.lockfile.buffers.string_bytes.items));
+                            const alias_str = alias.slice(this.lockfile.buffers.string_bytes.items);
+                            const canonical_name = this.names[package_id].slice(this.lockfile.buffers.string_bytes.items);
+                            const should_skip = this.lockfile.shouldSkipLifecycleScripts(alias_str) or
+                                this.lockfile.shouldSkipLifecycleScripts(canonical_name);
 
-                            const count = this.getInstalledPackageScriptsCount(
-                                alias.slice(this.lockfile.buffers.string_bytes.items),
-                                package_id,
-                                resolution.tag,
-                                &folder_path,
-                                log_level,
-                            );
-                            if (count > 0) {
-                                if (log_level.isVerbose()) {
-                                    Output.prettyError("Blocked {d} scripts for: {s}@{}\n", .{
-                                        count,
-                                        alias.slice(this.lockfile.buffers.string_bytes.items),
-                                        resolution.fmt(this.lockfile.buffers.string_bytes.items, .posix),
-                                    });
+                            if (!should_skip) {
+                                // Check if the package actually has scripts. `hasInstallScript` can be false positive if a package is published with
+                                // an auto binding.gyp rebuild script but binding.gyp is excluded from the published files.
+                                var folder_path: bun.AbsPath(.{ .sep = .auto }) = .from(this.node_modules.path.items);
+                                defer folder_path.deinit();
+                                folder_path.append(alias_str);
+
+                                const count = this.getInstalledPackageScriptsCount(
+                                    alias_str,
+                                    package_id,
+                                    resolution.tag,
+                                    &folder_path,
+                                    log_level,
+                                );
+                                if (count > 0) {
+                                    if (log_level.isVerbose()) {
+                                        Output.prettyError("Blocked {d} scripts for: {s}@{}\n", .{
+                                            count,
+                                            alias_str,
+                                            resolution.fmt(this.lockfile.buffers.string_bytes.items, .posix),
+                                        });
+                                    }
+                                    const entry = bun.handleOom(this.summary.packages_with_blocked_scripts.getOrPut(this.manager.allocator, truncated_dep_name_hash));
+                                    if (!entry.found_existing) entry.value_ptr.* = 0;
+                                    entry.value_ptr.* += count;
                                 }
-                                const entry = bun.handleOom(this.summary.packages_with_blocked_scripts.getOrPut(this.manager.allocator, truncated_dep_name_hash));
-                                if (!entry.found_existing) entry.value_ptr.* = 0;
-                                entry.value_ptr.* += count;
                             }
                         },
                     }
@@ -1268,28 +1275,35 @@ pub const PackageInstaller = struct {
             };
 
             if (resolution.tag != .root and is_trusted) {
-                var folder_path: bun.AbsPath(.{ .sep = .auto }) = .from(this.node_modules.path.items);
-                defer folder_path.deinit();
-                folder_path.append(alias.slice(this.lockfile.buffers.string_bytes.items));
+                const alias_str = alias.slice(this.lockfile.buffers.string_bytes.items);
+                const canonical_name = this.names[package_id].slice(this.lockfile.buffers.string_bytes.items);
+                const should_skip = this.lockfile.shouldSkipLifecycleScripts(alias_str) or
+                    this.lockfile.shouldSkipLifecycleScripts(canonical_name);
 
-                if (this.enqueueLifecycleScripts(
-                    alias.slice(this.lockfile.buffers.string_bytes.items),
-                    log_level,
-                    &folder_path,
-                    package_id,
-                    dep.behavior.optional,
-                    resolution,
-                )) {
-                    if (is_trusted_through_update_request) {
-                        this.manager.trusted_deps_to_add_to_package_json.append(
-                            this.manager.allocator,
-                            bun.handleOom(this.manager.allocator.dupe(u8, alias.slice(this.lockfile.buffers.string_bytes.items))),
-                        ) catch |err| bun.handleOom(err);
-                    }
+                if (!should_skip) {
+                    var folder_path: bun.AbsPath(.{ .sep = .auto }) = .from(this.node_modules.path.items);
+                    defer folder_path.deinit();
+                    folder_path.append(alias_str);
 
-                    if (add_to_lockfile) {
-                        if (this.lockfile.trusted_dependencies == null) this.lockfile.trusted_dependencies = .{};
-                        this.lockfile.trusted_dependencies.?.put(this.manager.allocator, truncated_dep_name_hash, {}) catch |err| bun.handleOom(err);
+                    if (this.enqueueLifecycleScripts(
+                        alias_str,
+                        log_level,
+                        &folder_path,
+                        package_id,
+                        dep.behavior.optional,
+                        resolution,
+                    )) {
+                        if (is_trusted_through_update_request) {
+                            this.manager.trusted_deps_to_add_to_package_json.append(
+                                this.manager.allocator,
+                                bun.handleOom(this.manager.allocator.dupe(u8, alias_str)),
+                            ) catch |err| bun.handleOom(err);
+                        }
+
+                        if (add_to_lockfile) {
+                            if (this.lockfile.trusted_dependencies == null) this.lockfile.trusted_dependencies = .{};
+                            this.lockfile.trusted_dependencies.?.put(this.manager.allocator, truncated_dep_name_hash, {}) catch |err| bun.handleOom(err);
+                        }
                     }
                 }
             }

src/install/default-skipped-lifecycle-scripts.txt

@@ -0,0 +1,2 @@
+esbuild
+protobufjs

src/install/lockfile.zig

@@ -24,6 +24,8 @@ workspace_versions: VersionHashMap = .{},
 /// Optional because `trustedDependencies` in package.json might be an
 /// empty list or it might not exist
 trusted_dependencies: ?TrustedDependenciesSet = null,
+/// Optional: packages whose lifecycle scripts should be skipped AND excluded from "blocked" count
+skip_scripts_from: ?TrustedDependenciesSet = null,
 patched_dependencies: PatchedDependenciesMap = .{},
 overrides: OverrideMap = .{},
 catalogs: CatalogMap = .{},
@@ -355,6 +357,7 @@ pub fn loadFromBytes(this: *Lockfile, pm: ?*PackageManager, buf: []u8, allocator
     this.format = FormatVersion.current;
     this.scripts = .{};
     this.trusted_dependencies = null;
+    this.skip_scripts_from = null;
     this.workspace_paths = .{};
     this.workspace_versions = .{};
     this.overrides = .{};
@@ -628,6 +631,7 @@ pub fn cleanWithLogger(
     }
 
     const old_trusted_dependencies = old.trusted_dependencies;
+    const old_skip_scripts_from = old.skip_scripts_from;
     const old_scripts = old.scripts;
     // We will only shrink the number of packages here.
     // never grow
@@ -755,6 +759,7 @@ pub fn cleanWithLogger(
     try cloner.flush();
 
     new.trusted_dependencies = old_trusted_dependencies;
+    new.skip_scripts_from = old_skip_scripts_from;
     new.scripts = old_scripts;
     new.meta_hash = old.meta_hash;
 
@@ -1338,6 +1343,7 @@ pub fn initEmpty(this: *Lockfile, allocator: Allocator) void {
         .scratch = Scratch.init(allocator),
         .scripts = .{},
         .trusted_dependencies = null,
+        .skip_scripts_from = null,
         .workspace_paths = .{},
         .workspace_versions = .{},
         .overrides = .{},
@@ -1738,6 +1744,9 @@ pub fn deinit(this: *Lockfile) void {
     if (this.trusted_dependencies) |*trusted_dependencies| {
         trusted_dependencies.deinit(this.allocator);
     }
+    if (this.skip_scripts_from) |*skip_scripts| {
+        skip_scripts.deinit(this.allocator);
+    }
     this.patched_dependencies.deinit(this.allocator);
     this.workspace_paths.deinit(this.allocator);
     this.workspace_versions.deinit(this.allocator);
@@ -2108,6 +2117,79 @@ pub fn hasTrustedDependency(this: *const Lockfile, name: []const u8) bool {
     return default_trusted_dependencies.has(name);
 }
 
+const max_default_skipped_lifecycle_scripts = 512;
+
+pub const default_skipped_lifecycle_scripts_list: []const []const u8 = brk: {
+    // This file contains a list of packages whose lifecycle scripts should be skipped
+    // and excluded from the "blocked" count message
+    const data = @embedFile("./default-skipped-lifecycle-scripts.txt");
+    @setEvalBranchQuota(999999);
+    var buf: [max_default_skipped_lifecycle_scripts][]const u8 = undefined;
+    var i: usize = 0;
+    var iter = std.mem.tokenizeAny(u8, data, " \r\n\t");
+    while (iter.next()) |package_ptr| {
+        const package = package_ptr[0..].*;
+        buf[i] = &package;
+        i += 1;
+    }
+
+    const Sorter = struct {
+        pub fn lessThan(_: void, lhs: []const u8, rhs: []const u8) bool {
+            return std.mem.order(u8, lhs, rhs) == .lt;
+        }
+    };
+
+    // alphabetical so we don't need to sort later
+    std.sort.pdq([]const u8, buf[0..i], {}, Sorter.lessThan);
+
+    var names: [i][]const u8 = undefined;
+    @memcpy(names[0..i], buf[0..i]);
+    const final = names;
+    break :brk &final;
+};
+
+/// The default list of skipped lifecycle scripts is a static hashmap
+pub const default_skipped_lifecycle_scripts = brk: {
+    const StringHashContext = struct {
+        pub fn hash(_: @This(), s: []const u8) u64 {
+            @setEvalBranchQuota(999999);
+            // truncate to u32 because Lockfile uses the same u32 string hash
+            return @intCast(@as(u32, @truncate(String.Builder.stringHash(s))));
+        }
+        pub fn eql(_: @This(), a: []const u8, b: []const u8) bool {
+            @setEvalBranchQuota(999999);
+            return std.mem.eql(u8, a, b);
+        }
+    };
+
+    var map: StaticHashMap([]const u8, void, StringHashContext, max_default_skipped_lifecycle_scripts) = .{};
+
+    for (default_skipped_lifecycle_scripts_list) |dep| {
+        if (map.len == max_default_skipped_lifecycle_scripts) {
+            @compileError("default-skipped-lifecycle-scripts.txt is too large, please increase 'max_default_skipped_lifecycle_scripts' in lockfile.zig");
+        }
+
+        const entry = map.getOrPutAssumeCapacity(dep);
+        if (entry.found_existing) {
+            @compileError("Duplicate skipped lifecycle script: " ++ dep);
+        }
+    }
+
+    const final = map;
+    break :brk &final;
+};
+
+pub inline fn shouldSkipLifecycleScripts(this: *const Lockfile, name: []const u8) bool {
+    // Check user-specified skipScriptsFrom first (higher precedence)
+    if (this.skip_scripts_from) |skip_scripts| {
+        const hash = @as(u32, @truncate(String.Builder.stringHash(name)));
+        if (skip_scripts.contains(hash)) return true;
+    }
+
+    // Fall back to default skipped list
+    return default_skipped_lifecycle_scripts.has(name);
+}
+
 pub const NameHashMap = std.ArrayHashMapUnmanaged(PackageNameHash, String, ArrayIdentityContext.U64, false);
 pub const TrustedDependenciesSet = std.ArrayHashMapUnmanaged(TruncatedPackageNameHash, void, ArrayIdentityContext, false);
 pub const VersionHashMap = std.ArrayHashMapUnmanaged(PackageNameHash, Semver.Version, ArrayIdentityContext.U64, false);

src/install/lockfile/Package.zig

@@ -1571,6 +1571,36 @@ pub fn Package(comptime SemverIntType: type) type {
                         },
                     }
                 }
+
+                if (json.asProperty("skipScriptsFrom")) |q| {
+                    switch (q.expr.data) {
+                        .e_array => |arr| {
+                            if (lockfile.skip_scripts_from == null) lockfile.skip_scripts_from = .{};
+                            try lockfile.skip_scripts_from.?.ensureUnusedCapacity(allocator, arr.items.len);
+                            for (arr.slice()) |item| {
+                                const name = item.asString(allocator) orelse {
+                                    log.addErrorFmt(source, q.loc, allocator,
+                                        \\skipScriptsFrom expects an array of strings, e.g.
+                                        \\  <r><green>"skipScriptsFrom"<r>: [
+                                        \\    <green>"package_name"<r>
+                                        \\  ]
+                                    , .{}) catch {};
+                                    return error.InvalidPackageJSON;
+                                };
+                                lockfile.skip_scripts_from.?.putAssumeCapacity(@as(TruncatedPackageNameHash, @truncate(String.Builder.stringHash(name))), {});
+                            }
+                        },
+                        else => {
+                            log.addErrorFmt(source, q.loc, allocator,
+                                \\skipScriptsFrom expects an array of strings, e.g.
+                                \\  <r><green>"skipScriptsFrom"<r>: [
+                                \\    <green>"package_name"<r>
+                                \\  ]
+                            , .{}) catch {};
+                            return error.InvalidPackageJSON;
+                        },
+                    }
+                }
             }
 
             if (comptime features.is_main) {

test/cli/install/bun-install-lifecycle-scripts.test.ts

@@ -2921,6 +2921,263 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) {
       expect(await exited).toBe(0);
       assertManifestsPopulated(join(packageDir, ".bun-cache"), verdaccio.registryUrl());
     });
+
+    test("skipScriptsFrom prevents scripts and excludes from blocked count", async () => {
+      const testEnv = forceWaiterThread ? { ...env, BUN_FEATURE_FLAG_FORCE_WAITER_THREAD: "1" } : env;
+      await writeFile(
+        packageJson,
+        JSON.stringify({
+          name: "foo",
+          version: "1.0.0",
+          dependencies: {
+            "all-lifecycle-scripts": "1.0.0",
+            "uses-what-bin": "1.0.0",
+          },
+          skipScriptsFrom: ["all-lifecycle-scripts"],
+        }),
+      );
+
+      var { stdout, stderr, exited } = spawn({
+        cmd: [bunExe(), "install"],
+        cwd: packageDir,
+        stdout: "pipe",
+        stdin: "ignore",
+        stderr: "pipe",
+        env: testEnv,
+      });
+
+      var err = await stderr.text();
+      var out = await stdout.text();
+      expect(err).toContain("Saved lockfile");
+      expect(err).not.toContain("not found");
+      expect(err).not.toContain("error:");
+
+      // Should only show 1 blocked script (uses-what-bin), not 4 (all-lifecycle-scripts + uses-what-bin)
+      expect(out.replace(/\s*\[[0-9\.]+m?s\]$/m, "").split(/\r?\n/)).toEqual([
+        expect.stringContaining("bun install v1."),
+        "",
+        "+ all-lifecycle-scripts@1.0.0",
+        "+ uses-what-bin@1.0.0",
+        "",
+        "2 packages installed",
+        "",
+        "Blocked 1 postinstall. Run `bun pm untrusted` for details.",
+        "",
+      ]);
+      expect(await exited).toBe(0);
+      assertManifestsPopulated(join(packageDir, ".bun-cache"), verdaccio.registryUrl());
+
+      // Verify scripts were skipped for all-lifecycle-scripts
+      const depDir = join(packageDir, "node_modules", "all-lifecycle-scripts");
+      expect(await exists(join(depDir, "preinstall.txt"))).toBeFalse();
+      expect(await exists(join(depDir, "install.txt"))).toBeFalse();
+      expect(await exists(join(depDir, "postinstall.txt"))).toBeFalse();
+      expect(await exists(join(depDir, "preprepare.txt"))).toBeFalse();
+      expect(await exists(join(depDir, "prepare.txt"))).toBeTrue(); // prepare always runs
+      expect(await exists(join(depDir, "postprepare.txt"))).toBeFalse();
+
+      // Verify scripts were blocked for uses-what-bin
+      const whatBinDir = join(packageDir, "node_modules", "uses-what-bin");
+      expect(await exists(join(whatBinDir, "what-bin.txt"))).toBeFalse();
+
+      // Verify skipScriptsFrom has higher precedence than trustedDependencies
+      await writeFile(
+        packageJson,
+        JSON.stringify({
+          name: "foo",
+          version: "1.0.0",
+          dependencies: {
+            "all-lifecycle-scripts": "1.0.0",
+          },
+          trustedDependencies: ["all-lifecycle-scripts"],
+          skipScriptsFrom: ["all-lifecycle-scripts"],
+        }),
+      );
+
+      ({ stdout, stderr, exited } = spawn({
+        cmd: [bunExe(), "install"],
+        cwd: packageDir,
+        stdout: "pipe",
+        stdin: "ignore",
+        stderr: "pipe",
+        env: testEnv,
+      }));
+
+      err = await stderr.text();
+      out = await stdout.text();
+      expect(err).not.toContain("error:");
+
+      // Should not show any blocked scripts message because the only package is in skipScriptsFrom
+      const outLines = out.replace(/\s*\[[0-9\.]+m?s\]$/m, "").split(/\r?\n/);
+      expect(outLines).not.toContain(expect.stringContaining("Blocked"));
+      expect(await exited).toBe(0);
+
+      // Scripts should still be skipped even though it's in trustedDependencies
+      expect(await exists(join(depDir, "preinstall.txt"))).toBeFalse();
+      expect(await exists(join(depDir, "install.txt"))).toBeFalse();
+      expect(await exists(join(depDir, "postinstall.txt"))).toBeFalse();
+
+      // Verify skipped packages don't appear in untrusted list
+      ({ stdout, stderr, exited } = spawn({
+        cmd: [bunExe(), "pm", "untrusted"],
+        cwd: packageDir,
+        stdout: "pipe",
+        stderr: "pipe",
+        env: testEnv,
+      }));
+
+      out = await stdout.text();
+      err = await stderr.text();
+      expect(err).not.toContain("error:");
+      expect(out).not.toContain("all-lifecycle-scripts");
+      expect(await exited).toBe(0);
+    });
+
+    test("skipScriptsFrom works with aliased dependencies (default list)", async () => {
+      const testEnv = forceWaiterThread ? { ...env, BUN_FEATURE_FLAG_FORCE_WAITER_THREAD: "1" } : env;
+      await writeFile(
+        packageJson,
+        JSON.stringify({
+          name: "foo",
+          version: "1.0.0",
+          dependencies: {
+            "my-bundler": "npm:esbuild@0.19.0",
+            "uses-what-bin": "1.0.0",
+          },
+        }),
+      );
+
+      var { stdout, stderr, exited } = spawn({
+        cmd: [bunExe(), "install"],
+        cwd: packageDir,
+        stdout: "pipe",
+        stdin: "ignore",
+        stderr: "pipe",
+        env: testEnv,
+      });
+
+      var err = await stderr.text();
+      var out = await stdout.text();
+      expect(err).toContain("Saved lockfile");
+      expect(err).not.toContain("not found");
+      expect(err).not.toContain("error:");
+
+      // esbuild is in default skip list, so even with alias "my-bundler" it should be skipped
+      // Only uses-what-bin should be blocked
+      expect(out.replace(/\s*\[[0-9\.]+m?s\]$/m, "").split(/\r?\n/)).toEqual([
+        expect.stringContaining("bun install v1."),
+        "",
+        "+ my-bundler@0.19.0",
+        "+ uses-what-bin@1.0.0",
+        "",
+        expect.stringContaining("packages installed"),
+        "",
+        "Blocked 1 postinstall. Run `bun pm untrusted` for details.",
+        "",
+      ]);
+      expect(await exited).toBe(0);
+      assertManifestsPopulated(join(packageDir, ".bun-cache"), verdaccio.registryUrl());
+
+      // Verify esbuild's postinstall was skipped (installed as "my-bundler")
+      const esbuildDir = join(packageDir, "node_modules", "my-bundler");
+      expect(await exists(join(esbuildDir, "install.txt"))).toBeFalse();
+    });
+
+    test("skipScriptsFrom works with aliased dependencies (user-specified)", async () => {
+      const testEnv = forceWaiterThread ? { ...env, BUN_FEATURE_FLAG_FORCE_WAITER_THREAD: "1" } : env;
+      await writeFile(
+        packageJson,
+        JSON.stringify({
+          name: "foo",
+          version: "1.0.0",
+          dependencies: {
+            "my-scripts": "npm:all-lifecycle-scripts@1.0.0",
+            "uses-what-bin": "1.0.0",
+          },
+          skipScriptsFrom: ["all-lifecycle-scripts"],
+        }),
+      );
+
+      var { stdout, stderr, exited } = spawn({
+        cmd: [bunExe(), "install"],
+        cwd: packageDir,
+        stdout: "pipe",
+        stdin: "ignore",
+        stderr: "pipe",
+        env: testEnv,
+      });
+
+      var err = await stderr.text();
+      var out = await stdout.text();
+      expect(err).toContain("Saved lockfile");
+      expect(err).not.toContain("not found");
+      expect(err).not.toContain("error:");
+
+      // all-lifecycle-scripts is in user's skipScriptsFrom, so even with alias it should be skipped
+      // Only uses-what-bin should be blocked
+      expect(out.replace(/\s*\[[0-9\.]+m?s\]$/m, "").split(/\r?\n/)).toEqual([
+        expect.stringContaining("bun install v1."),
+        "",
+        "+ my-scripts@1.0.0",
+        "+ uses-what-bin@1.0.0",
+        "",
+        "2 packages installed",
+        "",
+        "Blocked 1 postinstall. Run `bun pm untrusted` for details.",
+        "",
+      ]);
+      expect(await exited).toBe(0);
+      assertManifestsPopulated(join(packageDir, ".bun-cache"), verdaccio.registryUrl());
+
+      // Verify scripts were skipped for all-lifecycle-scripts (installed as "my-scripts")
+      const scriptsDir = join(packageDir, "node_modules", "my-scripts");
+      expect(await exists(join(scriptsDir, "preinstall.txt"))).toBeFalse();
+      expect(await exists(join(scriptsDir, "install.txt"))).toBeFalse();
+      expect(await exists(join(scriptsDir, "postinstall.txt"))).toBeFalse();
+      expect(await exists(join(scriptsDir, "prepare.txt"))).toBeTrue(); // prepare always runs
+    });
+
+    // skipScriptsFrom accepts either the dependency alias or canonical package name for flexibility
+    test("skipScriptsFrom by alias name", async () => {
+      const testEnv = forceWaiterThread ? { ...env, BUN_FEATURE_FLAG_FORCE_WAITER_THREAD: "1" } : env;
+      await writeFile(
+        packageJson,
+        JSON.stringify({
+          name: "foo",
+          version: "1.0.0",
+          dependencies: {
+            "my-scripts": "npm:all-lifecycle-scripts@1.0.0",
+          },
+          skipScriptsFrom: ["my-scripts"], // Skip by alias name instead of canonical
+        }),
+      );
+
+      var { stdout, stderr, exited } = spawn({
+        cmd: [bunExe(), "install"],
+        cwd: packageDir,
+        stdout: "pipe",
+        stdin: "ignore",
+        stderr: "pipe",
+        env: testEnv,
+      });
+
+      var err = await stderr.text();
+      var out = await stdout.text();
+      expect(err).toContain("Saved lockfile");
+      expect(err).not.toContain("not found");
+      expect(err).not.toContain("error:");
+
+      // Should not show any blocked scripts message
+      const outLines = out.replace(/\s*\[[0-9\.]+m?s\]$/m, "").split(/\r?\n/);
+      expect(outLines).not.toContain(expect.stringContaining("Blocked"));
+      expect(await exited).toBe(0);
+
+      // Verify scripts were skipped
+      const scriptsDir = join(packageDir, "node_modules", "my-scripts");
+      expect(await exists(join(scriptsDir, "preinstall.txt"))).toBeFalse();
+      expect(await exists(join(scriptsDir, "install.txt"))).toBeFalse();
+      expect(await exists(join(scriptsDir, "postinstall.txt"))).toBeFalse();
+    });
   });
 }