install: fix default-trusted lifecycle scripts being blocked after yarn.lock migration

src/install/dependency.rs

@@ -417,7 +417,7 @@ pub(crate) fn is_scp_like_path(dependency: &[u8]) -> bool {
 ///
 /// This also checks for a github url that ends with ".tar.gz"
 #[inline]
-fn is_github_tarball_path(dependency: &[u8]) -> bool {
+pub(crate) fn is_github_tarball_path(dependency: &[u8]) -> bool {
     if is_tarball(dependency) {
         return true;
     }

src/install/lockfile.rs

@@ -3159,7 +3159,9 @@ impl Lockfile {
         ) else {
             return false;
         };
-        url == canonical_url.as_slice()
+        // Older yarn.lock migrations stored yarn's `#<sha1>` on the URL. A fragment is
+        // never sent to the registry, so it cannot change which tarball is downloaded.
+        crate::yarn::Entry::url_without_hash(url) == canonical_url.as_slice()
     }
 
     fn declared_by_root_or_workspace(&self, alias: &[u8], resolution: &Resolution) -> bool {

src/install/yarn.rs

@@ -129,10 +129,13 @@ impl<'a> Entry<'a> {
     }
 
     pub(crate) fn is_git_dependency(version: &[u8]) -> bool {
+        if let Some(github_path) = version.strip_prefix(b"https://github.com/") {
+            // An archive download's `#` is yarn's tarball hash, not a commit.
+            return !dependency::is_github_tarball_path(Entry::url_without_hash(github_path));
+        }
         version.starts_with(b"git+")
             || version.starts_with(b"git://")
             || version.starts_with(b"github:")
-            || version.starts_with(b"https://github.com/")
     }
 
     pub(crate) fn is_npm_alias(version: &[u8]) -> bool {
@@ -143,6 +146,14 @@ impl<'a> Entry<'a> {
         version.starts_with(b"https://") && version.ends_with(b".tgz")
     }
 
+    /// yarn v1 writes tarball `resolved` fields as `<url>#<sha1 of the tarball>`.
+    pub(crate) fn url_without_hash(resolved: &[u8]) -> &[u8] {
+        match strings::index_of_char_usize(resolved, b'#') {
+            Some(hash_idx) => &resolved[..hash_idx],
+            None => resolved,
+        }
+    }
+
     pub(crate) fn is_workspace_dependency(version: &[u8]) -> bool {
         version.starts_with(b"workspace:") || version == b"*"
     }
@@ -954,31 +965,26 @@ pub(crate) fn migrate_yarn_lockfile<'a>(
 
         package_id_to_yarn_idx[package_id as usize] = yarn_idx;
 
+        let resolved_url: Option<&[u8]> = entry.resolved.as_deref().map(Entry::url_without_hash);
+
         let name_to_use: &[u8] = 'blk: {
             if entry.commit.is_some() && entry.git_repo_name.is_some() {
                 break 'blk entry.git_repo_name.as_deref().unwrap();
-            } else if let Some(resolved) = entry.resolved.as_deref() {
-                if is_direct_url_dep
-                    || Entry::is_remote_tarball(resolved)
-                    || resolved.ends_with(b".tgz")
+            } else if let (true, Some(resolved)) = (is_direct_url_dep, resolved_url) {
+                // https://registry.npmjs.org/package/-/package-version.tgz
+                if strings::index_of(resolved, b"registry.npmjs.org/").is_some()
+                    || strings::index_of(resolved, b"registry.yarnpkg.com/").is_some()
                 {
-                    // https://registry.npmjs.org/package/-/package-version.tgz
-                    if strings::index_of(resolved, b"registry.npmjs.org/").is_some()
-                        || strings::index_of(resolved, b"registry.yarnpkg.com/").is_some()
-                    {
-                        if let Some(separator_idx) = strings::index_of(resolved, b"/-/") {
-                            if let Some(registry_idx) = strings::index_of(resolved, b"registry.") {
-                                let after_registry = &resolved[registry_idx..];
-                                if let Some(domain_slash) = strings::index_of(after_registry, b"/")
-                                {
-                                    let package_start = registry_idx + domain_slash + 1;
-                                    let extracted_name = &resolved[package_start..separator_idx];
-                                    break 'blk extracted_name;
-                                }
+                    if let Some(separator_idx) = strings::index_of(resolved, b"/-/") {
+                        if let Some(registry_idx) = strings::index_of(resolved, b"registry.") {
+                            let after_registry = &resolved[registry_idx..];
+                            if let Some(domain_slash) = strings::index_of(after_registry, b"/") {
+                                let package_start = registry_idx + domain_slash + 1;
+                                let extracted_name = &resolved[package_start..separator_idx];
+                                break 'blk extracted_name;
                             }
                         }
                     }
-                    break 'blk base_name;
                 }
             }
             break 'blk base_name;
@@ -1048,29 +1054,31 @@ pub(crate) fn migrate_yarn_lockfile<'a>(
                     }
                 }
                 break 'blk Resolution::default();
-            } else if let Some(resolved) = entry.resolved.as_deref() {
+            } else if let Some(resolved) = resolved_url {
                 if is_direct_url_dep {
                     break 'blk Resolution::init(ResolutionValue::RemoteTarball(
                         sbuf!().append(resolved)?,
                     ));
                 }
 
-                // Yarn v1 lockfiles legitimately contain entries without an integrity field
-                // (workspace deps, file:, codeload tarballs), so migration intentionally
-                // accepts off-registry tarball URLs without integrity instead of failing.
-                if Entry::is_remote_tarball(resolved) || resolved.ends_with(b".tgz") {
-                    break 'blk Resolution::init(ResolutionValue::RemoteTarball(
-                        sbuf!().append(resolved)?,
-                    ));
-                }
-
                 let version = sbuf!().append(entry.version)?;
                 let result =
                     Semver::Version::parse(version.sliced(this.buffers.string_bytes.as_slice()));
                 if !result.valid {
+                    // Yarn v1 lockfiles legitimately contain entries without an integrity field
+                    // (workspace deps, file:, codeload tarballs), so migration intentionally
+                    // accepts off-registry tarball URLs without integrity instead of failing.
+                    if Entry::is_remote_tarball(resolved) || resolved.ends_with(b".tgz") {
+                        break 'blk Resolution::init(ResolutionValue::RemoteTarball(
+                            sbuf!().append(resolved)?,
+                        ));
+                    }
                     break 'blk Resolution::default();
                 }
 
+                // `has_trusted_dependency` compares this URL with the canonical registry
+                // tarball URL, so it must be the bare URL a fresh install records: no
+                // `#sha1`, and no RemoteTarball just because the URL ends in `.tgz`.
                 let is_default_registry = resolved.starts_with(b"https://registry.yarnpkg.com/")
                     || resolved.starts_with(b"https://registry.npmjs.org/");
 

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

@@ -418,6 +418,179 @@ test.concurrent("default trusted dependencies require the canonical registry tar
   expect(await exited).toBe(0);
 });
 
+describe("default trusted dependencies after yarn.lock migration", () => {
+  const electronIntegrity =
+    "sha512-GkuwCdn6o8Krsxb3DIIqYP+TAi8Y5jYUadmseZ6nR2op2k5ssdKRYo4JjYDGopa1ACrGAcQuWViz/+vX/WjYnA==";
+  // `all-lifecycle-scripts@1.0.0`: a different package with preinstall, install
+  // and postinstall scripts of its own, used to record a tarball that is not
+  // electron's under the default-trusted name `electron`.
+  const otherIntegrity =
+    "sha512-hgU56juWYnFOQ3byQuydEgugxd+iWvaWfaoGvly4k/AxehC3dhM6IhXoDc3K7b/n1mP/II8hGIjI+LmxXFNMlw==";
+
+  // Installs `electron@1.0.0` (on the default trusted list) from a yarn.lock
+  // that resolves it to `resolved`, and returns the migrated bun.lock.
+  async function installFromYarnLock(ctx: TestCtx, resolved: string, integrity: string) {
+    const { packageDir, packageJson, env } = ctx;
+    await Promise.all([
+      writeFile(
+        packageJson,
+        JSON.stringify({
+          name: "foo",
+          version: "1.0.0",
+          dependencies: {
+            "electron": "1.0.0",
+          },
+        }),
+      ),
+      writeFile(
+        join(packageDir, "yarn.lock"),
+        `# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+
+
+electron@1.0.0:
+  version "1.0.0"
+  resolved "${resolved}"
+  integrity ${integrity}
+`,
+      ),
+    ]);
+
+    await using proc = spawn({
+      cmd: [bunExe(), "install"],
+      cwd: packageDir,
+      stdout: "pipe",
+      stdin: "ignore",
+      stderr: "pipe",
+      env,
+    });
+
+    const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(err).toContain("migrated lockfile from yarn.lock");
+    expect(err).not.toContain("error:");
+    expect(await exists(join(packageDir, "node_modules", "electron", "package.json"))).toBeTrue();
+    expect(exitCode).toBe(0);
+    return { out, lockfile: await file(join(packageDir, "bun.lock")).text() };
+  }
+
+  // yarn v1 writes the tarball's sha1 as a URL fragment after registry tarball
+  // URLs. Other producers of the format (bun's own `--yarn` output, registries
+  // without a shasum) omit it. Both shapes describe the canonical registry
+  // tarball for electron@1.0.0, so both must keep the default-trusted grant
+  // after migration, exactly like an install that never had a yarn.lock.
+  test.concurrent.each([
+    ["with a sha1 fragment", "#f1b8bc2c23cd7e4f1500669dfaf8757578d2e391"],
+    ["without a fragment", ""],
+  ])("scripts run for the canonical registry tarball URL %s", async (_, fragment) => {
+    using ctx = await setupTest();
+    const canonicalUrl = `http://localhost:${verdaccio.port}/electron/-/electron-1.0.0.tgz`;
+
+    const { out, lockfile } = await installFromYarnLock(ctx, `${canonicalUrl}${fragment}`, electronIntegrity);
+
+    expect(out).not.toContain("Blocked");
+    expect(await exists(join(ctx.packageDir, "node_modules", "electron", "preinstall.txt"))).toBeTrue();
+    // The migrated entry is an npm package whose tarball URL is the canonical
+    // one for the configured registry, with yarn's fragment dropped.
+    expect(lockfile).toContain(`"electron": ["electron@1.0.0", "${canonicalUrl}", {}, "${electronIntegrity}"]`);
+  });
+
+  test.concurrent("scripts stay blocked when the yarn.lock points the name at another tarball", async () => {
+    using ctx = await setupTest();
+    // Same registry, but another package's tarball recorded under the
+    // default-trusted name. Migration must keep that URL, so the canonical URL
+    // check still denies the default grant.
+    const otherUrl = `http://localhost:${verdaccio.port}/all-lifecycle-scripts/-/all-lifecycle-scripts-1.0.0.tgz`;
+
+    const { lockfile } = await installFromYarnLock(
+      ctx,
+      `${otherUrl}#91cd0bd6a450b21db0078b9118c54bc0a27fccb7`,
+      otherIntegrity,
+    );
+
+    expect(lockfile).toContain(`"electron": ["electron@1.0.0", "${otherUrl}", {}, "${otherIntegrity}"]`);
+    const electronDir = join(ctx.packageDir, "node_modules", "electron");
+    expect(
+      await Promise.all([
+        exists(join(electronDir, "install.js")),
+        exists(join(electronDir, "preinstall.txt")),
+        exists(join(electronDir, "install.txt")),
+        exists(join(electronDir, "postinstall.txt")),
+      ]),
+    ).toEqual([true, false, false, false]);
+  });
+
+  // Earlier versions of the migrator wrote yarn's `#sha1` into bun.lock itself.
+  // Those lockfiles are still out there, so the canonical URL check has to look
+  // past the fragment (which never reaches the registry) while still rejecting
+  // a URL whose path is not electron's tarball.
+  test.concurrent.each([
+    [
+      "runs scripts for the canonical tarball",
+      "electron/-/electron-1.0.0.tgz#f1b8bc2c23cd7e4f1500669dfaf8757578d2e391",
+      electronIntegrity,
+      true,
+    ],
+    [
+      "keeps blocking another tarball",
+      "all-lifecycle-scripts/-/all-lifecycle-scripts-1.0.0.tgz#91cd0bd6a450b21db0078b9118c54bc0a27fccb7",
+      otherIntegrity,
+      false,
+    ],
+  ])("a bun.lock already migrated with the #sha1 suffix %s", async (_, tarball, integrity, scriptsRun) => {
+    using ctx = await setupTest();
+    const { packageDir, packageJson, env } = ctx;
+    const url = `http://localhost:${verdaccio.port}/${tarball}`;
+
+    await Promise.all([
+      writeFile(
+        packageJson,
+        JSON.stringify({
+          name: "foo",
+          version: "1.0.0",
+          dependencies: {
+            "electron": "1.0.0",
+          },
+        }),
+      ),
+      writeFile(
+        join(packageDir, "bun.lock"),
+        `{
+  "lockfileVersion": 1,
+  "workspaces": {
+    "": {
+      "name": "foo",
+      "dependencies": {
+        "electron": "1.0.0",
+      },
+    },
+  },
+  "packages": {
+    "electron": ["electron@1.0.0", "${url}", {}, "${integrity}"],
+  }
+}
+`,
+      ),
+    ]);
+
+    await using proc = spawn({
+      cmd: [bunExe(), "install"],
+      cwd: packageDir,
+      stdout: "pipe",
+      stdin: "ignore",
+      stderr: "pipe",
+      env,
+    });
+
+    const [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]);
+    expect(err).not.toContain("error:");
+    const electronDir = join(packageDir, "node_modules", "electron");
+    expect(
+      await Promise.all([exists(join(electronDir, "package.json")), exists(join(electronDir, "preinstall.txt"))]),
+    ).toEqual([true, scriptsRun]);
+    expect(exitCode).toBe(0);
+  });
+});
+
 test.concurrent("binary lockfile trusted dependency entries require an exact name match", async () => {
   using ctx = await setupTest();
   const { packageDir, packageJson, env } = ctx;

test/cli/install/migration/__snapshots__/yarn-lock-migration.test.ts.snap

@@ -2935,7 +2935,7 @@ exports[`bun pm migrate for existing yarn.lock yarn-cli-repo: yarn-cli-repo 1`]
 
     "class-utils/define-property/is-descriptor/is-data-descriptor/kind-of": ["kind-of@3.2.2", "", { "dependencies": { "is-buffer": "^1.1.5" } }, "sha1-MeohpzS6ubuw8yRm2JOupR5KPGQ="],
 
-    "commitizen/inquirer/cli-cursor/restore-cursor/onetime": ["onetime@1.1.0", "http://registry.npmjs.org/onetime/-/onetime-1.1.0.tgz#a1f7838f8314c516f05ecefcbc4ccfe04b4ed789", {}, "sha1-ofeDj4MUxRbwXs78vEzP4EtO14k="],
+    "commitizen/inquirer/cli-cursor/restore-cursor/onetime": ["onetime@1.1.0", "http://registry.npmjs.org/onetime/-/onetime-1.1.0.tgz", {}, "sha1-ofeDj4MUxRbwXs78vEzP4EtO14k="],
 
     "eslint/inquirer/chalk/supports-color/has-flag": ["has-flag@3.0.0", "", {}, "sha1-tdRU3CGZriJWmfNGfloH87lVuv0="],
 
@@ -3154,7 +3154,7 @@ exports[`bun pm migrate for existing yarn.lock yarn-stuff: yarn-stuff 1`] = `
 
     "reg": ["abbrev@1.1.1", "", {}, "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q=="],
 
-    "remote": ["abbrev@https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz#f8f2c887ad10bf67f634f005b6987fed3179aac8", {}],
+    "remote": ["abbrev@https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", {}],
 
     "symlink": ["symlink@file:abbrev-link-target", {}],