diff --git a/crates/socket-patch-cli/tests/e2e_redirect_yarn_berry_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_yarn_berry_build.rs index 0300848f3..39c6368bf 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_yarn_berry_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_yarn_berry_build.rs @@ -885,6 +885,147 @@ async fn berry_get_uuid_hosted_fresh_checkout_installs() { hosted_manifestless_vex_matrix(&fx, HostedDriver::GetUuid); } +/// #1465: a metadata mirror that passes npmjs packuments through unchanged +/// (tarball URLs still on registry.npmjs.org) and 404s every `/-/` tarball +/// path under itself. Real yarn locks the dependency behind it as +/// `left-pad@npm:1.3.0::__archiveUrl=`. A hosted pin then +/// `rollback` must restore that binding: without it a cold install fetches +/// the conventional path under the mirror and fails YN0035 (404). +#[tokio::test(flavor = "multi_thread")] +#[serial_test::serial] +async fn berry_hosted_rollback_keeps_archive_url_behind_passthrough_mirror() { + let Some(fx) = berry_hosted_project("mirror-rollback", false, HostedDriver::Scan).await else { + return; + }; + let server = &fx._server; + + // The mirror: npmjs's packument verbatim; nothing mounted under `/-/`, + // so wiremock answers every tarball path under the mirror with 404. + let mut curl = Command::new("curl"); + curl.args(["-fsSL", &format!("https://registry.npmjs.org/{DEP}")]); + let packument: serde_json::Value = match curl.output() { + Ok(out) if out.status.success() => { + serde_json::from_slice(&out.stdout).expect("packument JSON") + } + other => { + skip!("SKIP berry mirror rollback: npmjs packument unreachable: {other:?}"); + return; + } + }; + let version_doc = packument["versions"][DEP_VERSION].clone(); + let npmjs_tarball = version_doc["dist"]["tarball"].as_str().unwrap().to_string(); + assert!( + npmjs_tarball.starts_with("https://registry.npmjs.org/"), + "{npmjs_tarball}" + ); + Mock::given(method("GET")) + .and(path(format!("/meta/{DEP}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(&packument)) + .mount(server) + .await; + Mock::given(method("GET")) + .and(path(format!("/meta/{DEP}/{DEP_VERSION}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(&version_doc)) + .mount(server) + .await; + let mirror_rc = format!( + "nodeLinker: node-modules\nenableGlobalCache: false\n\ + unsafeHttpWhitelist:\n - \"{}\"\n\ + npmRegistryServer: \"{}/meta\"\n", + fx.host.split(':').next().unwrap_or("127.0.0.1"), + server.uri() + ); + + // What yarn itself locks behind the mirror (the restore's target). + let reference = fx.tmp.path().join("mirror-reference"); + std::fs::create_dir_all(&reference).unwrap(); + std::fs::write(reference.join("package.json"), &fx.registry_pkg).unwrap(); + std::fs::write(reference.join(".yarnrc.yml"), &mirror_rc).unwrap(); + let reference_global = fx.tmp.path().join("mirror-reference-global"); + let install = corepack( + &reference, + yarn_berry(), + &["install"], + &[("YARN_GLOBAL_FOLDER", reference_global.to_str().unwrap())], + ); + assert!( + install.status.success(), + "yarn install behind the mirror:\n{}", + yarn_berry_common::yarn_output(&install) + ); + let mirror_lock = std::fs::read_to_string(reference.join("yarn.lock")).unwrap(); + let bound = mirror_lock + .lines() + .find(|l| { + l.trim_start() + .starts_with(&format!("resolution: \"{DEP}@npm:")) + }) + .unwrap_or_else(|| panic!("no {DEP} resolution in:\n{mirror_lock}")) + .to_string(); + assert!( + bound.contains("::__archiveUrl="), + "yarn binds npmjs's URL behind the mirror: {bound}" + ); + + // The project now resolves through the mirror; the patch service + // supplies the upstream checksum the restore writes back. + std::fs::write(fx.proj.join(".yarnrc.yml"), &mirror_rc).unwrap(); + let checksum = yarn_berry_common::yarn_written_checksum(&mirror_lock).unwrap(); + Mock::given(method("GET")) + .and(path(format!("/upstream/npm/{UUID}.json"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "name": DEP, + "version": DEP_VERSION, + "integrity": version_doc["dist"]["integrity"], + "yarnBerry10c0": checksum, + }))) + .mount(server) + .await; + let origin = server.uri(); + let (code, stdout, stderr) = run_socket( + &fx.proj, + &[ + "rollback", + "--json", + "--yes", + "--patch-server-url", + &origin, + "--cwd", + fx.proj.to_str().unwrap(), + ], + ); + assert_eq!( + code, 0, + "rollback failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let restored = std::fs::read_to_string(fx.proj.join("yarn.lock")).unwrap(); + assert!( + restored.lines().any(|l| l == bound), + "rollback restores yarn's own binding {bound:?}; got:\n{restored}" + ); + + // A cold, immutable install behind the mirror fetches the bound URL. + let fresh = fx.tmp.path().join("mirror-fresh"); + std::fs::create_dir_all(&fresh).unwrap(); + for file in ["package.json", "yarn.lock", ".yarnrc.yml"] { + std::fs::copy(fx.proj.join(file), fresh.join(file)).unwrap(); + } + let fresh_global = fx.tmp.path().join("mirror-fresh-global"); + let ci = corepack( + &fresh, + yarn_berry(), + &["install", "--immutable", "--check-cache"], + &[("YARN_GLOBAL_FOLDER", fresh_global.to_str().unwrap())], + ); + assert!( + ci.status.success(), + "cold `yarn install --immutable` behind the mirror after rollback:\n{}", + yarn_berry_common::yarn_output(&ci) + ); + let installed = std::fs::read(fresh.join("node_modules").join(DEP).join("index.js")).unwrap(); + assert_eq!(installed, fx.orig, "the upstream bytes are back"); +} + /// Negative twin: the hosted URL serves a DIFFERENT tarball while the lock /// pins the real `10c0` checksum — the fresh `--check-cache` install must fail /// with YN0018. diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index d4f7dfe3c..8d7a89ef5 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -5580,6 +5580,86 @@ async fn yarn_berry_rollback_reads_the_tarball_from_the_project_registry() { ); } +/// #1465: a metadata mirror that passes the default registry's documents +/// through unchanged hands out tarball URLs conventional under the default +/// registry (npmjs normally, `SOCKET_NPM_REGISTRY` here), not under itself. +/// Yarn compares only against the package's own registry, so it binds them +/// with `::__archiveUrl=`; rollback and remove must keep that binding, or +/// yarn fetches the conventional path under the mirror, which 404s. +#[tokio::test] +#[serial] +async fn yarn_berry_unwind_keeps_a_default_registry_url_behind_a_mirror() { + for command in ["rollback", "remove"] { + let server = MockServer::start().await; + mock_discovery(&server).await; + let hosted_url = HOSTED_URL.replace("http://patch.test", &server.uri()); + mock_reference_with_berry_url(&server, &hosted_url).await; + mock_view(&server).await; + let integrity = vlt_hosted_common::sha512_sri(&upstream_tarball()); + let advertised = format!( + "{}/npm-registry/{NAME}/-/{NAME}-{VERSION}.tgz", + server.uri() + ); + mock_npm_registry_advertising(&server, &integrity, &advertised).await; + // The project's mirror passes the default registry's document + // through, tarball URL and all. + let mirror = format!("{}/mirror", server.uri()); + Mock::given(method("GET")) + .and(path(format!("/mirror/{NAME}/{VERSION}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "name": NAME, + "version": VERSION, + "dist": { "tarball": advertised, "integrity": integrity }, + }))) + .mount(&server) + .await; + let binding = |t: &str| { + t.replace( + &format!("resolution: \"{NAME}@npm:{VERSION}\""), + &format!( + "resolution: \"{NAME}@npm:{VERSION}::__archiveUrl={}\"", + socket_patch_core::utils::uri::encode_uri_component(&advertised) + ), + ) + }; + + let tmp = tempfile::tempdir().unwrap(); + write_berry_project_spelled(tmp.path(), binding); + std::fs::write( + tmp.path().join(".yarnrc.yml"), + format!("nodeLinker: node-modules\nnpmRegistryServer: \"{mirror}\"\n"), + ) + .unwrap(); + let lock_path = tmp.path().join("yarn.lock"); + let pristine = std::fs::read_to_string(&lock_path).unwrap(); + + let env = run_redirect_subprocess_with( + tmp.path(), + &server.uri(), + &["--patch-server-url", &server.uri()], + ); + assert_eq!(vlt_hosted_common::redirected(&env), 1, "{env:#}"); + + let args: &[&str] = if command == "rollback" { + &["rollback"] + } else { + &["remove", PURL] + }; + let (code, env) = hosted_unwind_json(tmp.path(), &server, &server.uri(), args); + assert_eq!(code, Some(0), "{command}: {env:#}"); + let restored = std::fs::read_to_string(&lock_path).unwrap(); + let checksum = berry_checksum_of(&restored); + assert_eq!( + restored, + pristine.replace( + &format!("10c0/{}", "3".repeat(128)), + &format!("10c0/{checksum}") + ), + "{command} keeps the mirror's __archiveUrl binding" + ); + } +} + /// A pnpm project whose lock records the patched entry as /// `{integrity, tarball: }` — what pnpm writes under /// `lockfile-include-tarball-url`, or for a tarball URL the registry @@ -5726,6 +5806,30 @@ async fn pnpm_rollback_reads_the_npmrc_mirror_document_for_an_offpath_tarball() assert_eq!(pnpm_pin_and_rollback(tmp.path(), &server), pristine); } +/// #1465 for pnpm: a mirror that passes the default registry's documents +/// through hands out URLs conventional under the default registry, not +/// under the mirror. pnpm compares only against the package's own +/// registry, so it records them as `tarball:`; rollback must keep it. +#[tokio::test] +#[serial] +async fn pnpm_rollback_keeps_a_default_registry_tarball_behind_a_mirror() { + let server = MockServer::start().await; + mock_discovery(&server).await; + mock_reference(&server).await; + mock_view(&server).await; + mock_npm_registry(&server, "sha512-UPSTREAMupstream==", None).await; + let advertised = format!( + "{}/npm-registry/{NAME}/-/{NAME}-{VERSION}.tgz", + server.uri() + ); + let mirror = mock_pnpm_mirror(&server, &advertised).await; + + let tmp = tempfile::tempdir().unwrap(); + let pristine = write_pnpm_tarball_project(tmp.path(), &advertised); + std::fs::write(tmp.path().join(".npmrc"), format!("registry={mirror}\n")).unwrap(); + assert_eq!(pnpm_pin_and_rollback(tmp.path(), &server), pristine); +} + /// #919, under `lockfile-include-tarball-url=true`: the restored `tarball:` /// is the mirror's URL pnpm wrote, not the default registry's. #[tokio::test] diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs index 470415d1b..3f4974d8c 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs @@ -438,11 +438,17 @@ async fn restore_classic( /// Whether a package manager derives `tarball` for `name@version` itself, /// and so leaves it out of the lock: it is the conventional URL under the -/// registry the version document came from, or under the project's -/// configured registry (`project_registry`; npmjs when unset), which is -/// the one the package manager compares against. +/// project's configured registry (`project_registry`; npmjs when unset), +/// the only one yarn and pnpm compare against. `from_project` says the +/// version document came from that registry. When it did not (the project +/// resolves from the default registry, or its own could not be read), the +/// default registry's base counts too: `SOCKET_NPM_REGISTRY` stands in for +/// npmjs, and a fallback document is read as the project registry's. A +/// mirror that passes npmjs's URLs through hands out URLs it does not +/// derive, so yarn binds them and pnpm records them (#1465). fn registry_derives_tarball( project_registry: Option<&str>, + from_project: bool, name: &str, version: &str, tarball: &str, @@ -450,24 +456,22 @@ fn registry_derives_tarball( use crate::vendor::registry_fetch::{ npm_registry_base, npm_tarball_is_conventional, DEFAULT_NPM_REGISTRY, }; - [ - npm_registry_base().as_str(), - project_registry.unwrap_or(DEFAULT_NPM_REGISTRY), - ] - .iter() - .any(|base| npm_tarball_is_conventional(base, name, version, tarball)) + let conventional = |base: &str| npm_tarball_is_conventional(base, name, version, tarball); + conventional(project_registry.unwrap_or(DEFAULT_NPM_REGISTRY)) + || (!from_project && conventional(&npm_registry_base())) } /// The `npm:` locator yarn berry writes for `name@version` resolved from the /// registry: bare when yarn derives `tarball` itself, else bound to it with -/// `::__archiveUrl=`. +/// `::__archiveUrl=` ([`registry_derives_tarball`]). fn berry_registry_locator( project_registry: Option<&str>, + from_project: bool, name: &str, version: &str, tarball: &str, ) -> String { - if registry_derives_tarball(project_registry, name, version, tarball) { + if registry_derives_tarball(project_registry, from_project, name, version, tarball) { format!("{name}@npm:{version}") } else { format!( @@ -898,11 +902,13 @@ async fn restore_berry( continue; } }; - let Some(dist) = dists.get(&(name.clone(), version.clone())).map(|d| &d.dist) else { + let Some(found) = dists.get(&(name.clone(), version.clone())) else { continue; }; + let dist = &found.dist; let locator = berry_registry_locator( registries.get(&name).and_then(Option::as_deref), + found.from_project, &name, &version, &dist.tarball, @@ -1480,8 +1486,14 @@ fn pnpm_lock_tarball_evidence( None => return Some(false), Some(tarball) if tarball.starts_with("file:") || is_hosted(tarball) => {} Some(tarball) => { + // No document was read: the lock's URL came from the + // registry pnpm names, the default one when it names none. + let registry = registry(name); + let own = registry + .as_deref() + .is_some_and(|r| non_default_registry(r).is_some()); derived |= - registry_derives_tarball(registry(name).as_deref(), name, version, tarball); + registry_derives_tarball(registry.as_deref(), own, name, version, tarball); } } } @@ -1831,9 +1843,10 @@ pub(crate) async fn restore_pnpm_locks( if result.refused.contains_key(uuid) { continue; } - let Some(dist) = dists.get(&(name.clone(), version.clone())).map(|d| &d.dist) else { + let Some(found) = dists.get(&(name.clone(), version.clone())) else { continue; }; + let dist = &found.dist; let Some(integrity) = dist.integrity.as_deref() else { result.refuse( uuid, @@ -1845,6 +1858,7 @@ pub(crate) async fn restore_pnpm_locks( // a URL it cannot derive from the registry (#557). let derived = registry_derives_tarball( policy.registry(name).as_deref(), + found.from_project, name, version, &dist.tarball, @@ -2739,9 +2753,10 @@ mod tests { use super::{ berry_lookup_registry, berry_registry_locator, bun_env_registry, bun_lookup_registry, bun_registry_slot, bun_tarball_url, expand_url_env, modules_yaml_pnpm_major, - non_default_registry, package_json_pnpm_major, pnpm_include_tarball, pnpm_lookup_registry, - pnpm_tarball_guess_warning, registry_derives_tarball, rush_json_pnpm_major, rush_lock_root, - split_userinfo, yaml_top_level_value, yarn_expand_env, PnpmTarballGuess, ProjectDist, + non_default_registry, package_json_pnpm_major, pnpm_include_tarball, + pnpm_lock_tarball_evidence, pnpm_lookup_registry, pnpm_tarball_guess_warning, + registry_derives_tarball, rush_json_pnpm_major, rush_lock_root, split_userinfo, + yaml_top_level_value, yarn_expand_env, PnpmTarballGuess, ProjectDist, }; use crate::patch::redirect::upstream::client::NpmDist; @@ -3784,6 +3799,7 @@ mod tests { let scope = rc_lookup(Some(rc), "@s/a"); assert!(registry_derives_tarball( scope.as_deref(), + true, "@s/a", "1.0.0", "https://s.example/@s/a/-/a-1.0.0.tgz" @@ -4053,6 +4069,7 @@ mod tests { // the package manager derives them, so they stay out of the lock. assert!(registry_derives_tarball( Some("https://r.example/npm/"), + true, "a", "1.0.0", "https://r.example/npm/a/-/a-1.0.0.tgz" @@ -4060,16 +4077,70 @@ mod tests { // No configured registry means npmjs (yarnpkg is its alias). assert!(registry_derives_tarball( None, + false, "@s/p", "2.0.0", "https://registry.yarnpkg.com/@s%2fp/-/p-2.0.0.tgz" )); assert!(!registry_derives_tarball( Some("https://r.example/npm"), + true, "a", "1.0.0", "https://cdn.example/files/a-1.0.0.tgz" )); + // #1465: a mirror passing npmjs documents through hands out npmjs + // URLs; the package manager compares only against the mirror, so + // it does not derive them (yarn binds them, pnpm records them). + assert!(!registry_derives_tarball( + Some("https://r.example/npm"), + true, + "a", + "1.0.0", + "https://registry.npmjs.org/a/-/a-1.0.0.tgz" + )); + assert!(!registry_derives_tarball( + Some("https://r.example/npm"), + true, + "@s/p", + "2.0.0", + "https://registry.npmjs.org/@s/p/-/p-2.0.0.tgz" + )); + // A fallback to the default registry's document (the mirror could + // not be read) guesses the mirror serves the conventional path. + assert!(registry_derives_tarball( + Some("https://r.example/npm"), + false, + "a", + "1.0.0", + "https://registry.npmjs.org/a/-/a-1.0.0.tgz" + )); + // A project that names npmjs (or yarn's alias for it) explicitly + // resolves from the default registry. + assert!(registry_derives_tarball( + Some("https://registry.yarnpkg.com"), + false, + "a", + "1.0.0", + "https://registry.npmjs.org/a/-/a-1.0.0.tgz" + )); + } + + /// #1465: behind a mirror that passes npmjs's documents through, pnpm + /// records the npmjs URLs as `tarball:` because it cannot derive them + /// from the mirror, so they say nothing about + /// lockfile-include-tarball-url. Without the mirror they prove it on. + #[test] + fn npmjs_tarballs_behind_a_mirror_are_no_tarball_setting_evidence() { + let lock = "lockfileVersion: '9.0'\n\npackages:\n\n a@1.0.0:\n \ + resolution: {integrity: sha512-AAAA, tarball: \ + https://registry.npmjs.org/a/-/a-1.0.0.tgz}\n"; + let mirror = |_: &str| Some("https://r.example/npm".to_string()); + assert_eq!(pnpm_lock_tarball_evidence(lock, mirror, |_| false), None); + assert_eq!( + pnpm_lock_tarball_evidence(lock, |_| None, |_| false), + Some(true) + ); } #[test] @@ -4077,6 +4148,7 @@ mod tests { assert_eq!( berry_registry_locator( Some("https://r.example"), + true, "a", "1.0.0", "https://r.example/a/-/a-1.0.0.tgz" @@ -4086,12 +4158,24 @@ mod tests { assert_eq!( berry_registry_locator( Some("https://r.example"), + true, "a", "1.0.0", "https://cdn.example/f/a.tgz" ), "a@npm:1.0.0::__archiveUrl=https%3A%2F%2Fcdn.example%2Ff%2Fa.tgz" ); + // #1465: npmjs's URL behind a mirror stays bound. + assert_eq!( + berry_registry_locator( + Some("https://r.example"), + true, + "a", + "1.0.0", + "https://registry.npmjs.org/a/-/a-1.0.0.tgz" + ), + "a@npm:1.0.0::__archiveUrl=https%3A%2F%2Fregistry.npmjs.org%2Fa%2F-%2Fa-1.0.0.tgz" + ); } #[test]