diff --git a/src/install/dependency.rs b/src/install/dependency.rs index 2f5b38f9b895..d7f0a18c1ecb 100644 --- a/src/install/dependency.rs +++ b/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; } diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index 73d55df73e17..1da0412d895c 100644 --- a/src/install/lockfile.rs +++ b/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 `#` 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 { diff --git a/src/install/yarn.rs b/src/install/yarn.rs index 0d0be5a31f57..1e2d97e87aef 100644 --- a/src/install/yarn.rs +++ b/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 `#`. + 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/"); diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index aea7f38ebec4..6983d8f2a39c 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/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; diff --git a/test/cli/install/migration/__snapshots__/yarn-lock-migration.test.ts.snap b/test/cli/install/migration/__snapshots__/yarn-lock-migration.test.ts.snap index b52a34874284..c0e389462c75 100644 --- a/test/cli/install/migration/__snapshots__/yarn-lock-migration.test.ts.snap +++ b/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", {}], diff --git a/test/cli/install/migration/yarn-lock-migration.test.ts b/test/cli/install/migration/yarn-lock-migration.test.ts index b4744963a17c..dea8db57e6bc 100644 --- a/test/cli/install/migration/yarn-lock-migration.test.ts +++ b/test/cli/install/migration/yarn-lock-migration.test.ts @@ -1547,3 +1547,123 @@ fsevents@^2.3.2: expect(bunLockContent).toContain("@esbuild/darwin-arm64"); }); }); + +describe.concurrent("yarn.lock migration of registry tarball URLs", () => { + const integrity = { + parent: "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==", + leaf: "sha512-LEyx4aLEC3x6T0UguF6YILf+ntvmOaWsVfENmIW0E9H09vKlLDGelMjjSm0jkDHALj8A8quZ/HapKNigzwge+Q==", + }; + + async function migrate(dependencies: Record, yarnLockEntries: string) { + // After migrating, bun fetches manifests from the configured registry to + // fill in bin/os/cpu. Point it at a local server that has nothing so the + // test stays off the network; the migrated resolutions don't depend on it. + using registry = Bun.serve({ + port: 0, + fetch: () => new Response("not found", { status: 404 }), + }); + await using dir = tempDir("yarn-migration-registry-urls", { + "package.json": JSON.stringify({ name: "registry-urls", version: "1.0.0", dependencies }), + "yarn.lock": `# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + +${yarnLockEntries}`, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "pm", "migrate", "-f"], + cwd: String(dir), + env: { ...bunEnv, BUN_CONFIG_REGISTRY: registry.url.href }, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout + stderr).toContain("migrated lockfile from yarn.lock"); + expect(exitCode).toBe(0); + return fs.readFileSync(join(String(dir), "bun.lock"), "utf8"); + } + + test("private registry: yarn's #sha1 suffix is dropped and suffix-less URLs stay npm packages", async () => { + const registry = "https://npm.example.com"; + const bunLock = await migrate( + { parent: "^1.0.0" }, + `parent@^1.0.0: + version "1.2.3" + resolved "${registry}/parent/-/parent-1.2.3.tgz#f8f2c887ad10bf67f634f005b6987fed3179aac8" + integrity ${integrity.parent} + dependencies: + "@scope/leaf" "~2.0.0" + +"@scope/leaf@~2.0.0": + version "2.0.1" + resolved "${registry}/@scope/leaf/-/leaf-2.0.1.tgz" + integrity ${integrity.leaf} +`, + ); + + // Both entries are npm packages whose tarball URL is exactly what the + // registry would hand a fresh install: no `#sha1`, and no remote tarball + // entry (`parent@https://...`) just because yarn omitted the suffix. + expect(bunLock).toContain( + `"parent": ["parent@1.2.3", "${registry}/parent/-/parent-1.2.3.tgz", { "dependencies": { "@scope/leaf": "~2.0.0" } }, "${integrity.parent}"]`, + ); + expect(bunLock).toContain( + `"@scope/leaf": ["@scope/leaf@2.0.1", "${registry}/@scope/leaf/-/leaf-2.0.1.tgz", {}, "${integrity.leaf}"]`, + ); + }); + + test("default registry: a URL without yarn's #sha1 suffix is still the default registry", async () => { + // This is the shape `bun install --yarn` itself writes. + const bunLock = await migrate( + { "@scope/leaf": "~2.0.0", "leaf-alias": "npm:leaf@^1.0.0" }, + `"@scope/leaf@~2.0.0": + version "2.0.1" + resolved "https://registry.yarnpkg.com/@scope/leaf/-/leaf-2.0.1.tgz" + integrity ${integrity.leaf} + +"leaf-alias@npm:leaf@^1.0.0": + version "1.0.0" + resolved "https://registry.npmjs.org/leaf/-/leaf-1.0.0.tgz" + integrity ${integrity.parent} +`, + ); + + expect(bunLock).toContain(`"@scope/leaf": ["@scope/leaf@2.0.1", "", {}, "${integrity.leaf}"]`); + expect(bunLock).toContain(`"leaf-alias": ["leaf@1.0.0", "", {}, "${integrity.parent}"]`); + }); + + test("a dependency declared as a tarball URL stays a remote tarball, without yarn's #sha1 suffix", async () => { + const registryUrl = "https://npm.example.com/leaf/-/leaf-2.0.1.tgz"; + // yarn puts the same `#sha1` after a GitHub archive download, where it must + // not be mistaken for the commit of a git dependency. + const githubUrl = "https://github.com/isaacs/abbrev-js/archive/refs/tags/v1.1.1.tar.gz"; + const bunLock = await migrate( + { "leaf": registryUrl, "gh-tar": githubUrl }, + `"leaf@${registryUrl}": + version "2.0.1" + resolved "${registryUrl}#f8f2c887ad10bf67f634f005b6987fed3179aac8" + +"gh-tar@${githubUrl}": + version "1.1.1" + resolved "${githubUrl}#f8f2c887ad10bf67f634f005b6987fed3179aac8" +`, + ); + + expect(bunLock).toContain(`"leaf": ["leaf@${registryUrl}", {}]`); + expect(bunLock).toContain(`"gh-tar": ["gh-tar@${githubUrl}", {}]`); + }); + + test("a git dependency hosted on github.com is still migrated as git", async () => { + const bunLock = await migrate( + { abbrev: "https://github.com/isaacs/abbrev-js.git" }, + `"abbrev@https://github.com/isaacs/abbrev-js.git": + version "1.1.1" + resolved "https://github.com/isaacs/abbrev-js.git#3f9802e56ff878761a338e43ecacbfed39d2181d" +`, + ); + + expect(bunLock).toContain(`"abbrev": ["abbrev-js@github:isaacs/abbrev-js#3f9802e", {}, ""]`); + }); +});