diff --git a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs index 9f18fb746..d69c31ea1 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs @@ -2137,3 +2137,154 @@ fn pnpm_vendor_keeps_user_workspace_overrides_authoritative() { ); assert_eq!(std::fs::read_to_string(&lock_path).unwrap(), lock_before); } + +/// #957: pnpm 9+ writes a scoped `npm:` alias's target quoted +/// (`version: '@isaacs/string-locale-compare@1.1.0'` in the root +/// importer, `sl: '@isaacs/…@1.1.0'` in a dependent's snapshot). Vendoring +/// can't rewrite that reference, so it must refuse the package — exactly as +/// it refuses the unscoped `npm:left-pad@1.3.0` alias — instead of +/// reporting success over a lock whose frozen install fails with +/// ERR_PNPM_LOCKFILE_MISSING_DEPENDENCY. The lock, package.json and +/// `.socket/vendor` stay untouched, and the untouched lock still +/// frozen-installs. +#[test] +fn pnpm_vendor_refuses_quoted_scoped_alias_references() { + if !has_corepack_pm(PNPM_PRIMARY) { + println!("SKIP: `corepack {PNPM_PRIMARY}` unavailable"); + return; + } + let pm = PNPM_PRIMARY; + const SCOPED: &str = "@isaacs/string-locale-compare"; + const SCOPED_VERSION: &str = "1.1.0"; + let alias = format!("npm:{SCOPED}@{SCOPED_VERSION}"); + + for shape in ["importer", "snapshot"] { + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + let deps = if shape == "importer" { + serde_json::json!({ "sl": alias }) + } else { + // A local tarball dependency that itself aliases the scoped + // package, so the quoted reference lands in its snapshot. + let pkg = tmp.path().join("host").join("package"); + std::fs::create_dir_all(&pkg).unwrap(); + let host = serde_json::json!({ + "name": "host", + "version": "1.0.0", + "dependencies": { "sl": alias }, + }); + std::fs::write(pkg.join("package.json"), host.to_string()).unwrap(); + std::fs::write(pkg.join("index.js"), "module.exports = require('sl');\n").unwrap(); + let tar = Command::new("tar") + .args(["-czf"]) + .arg(proj.join("host-1.0.0.tgz")) + .arg("package") + .current_dir(tmp.path().join("host")) + .output() + .expect("tar runs"); + assert!(tar.status.success(), "{tar:?}"); + serde_json::json!({ "host": "file:./host-1.0.0.tgz" }) + }; + let pkg_doc = serde_json::json!({ + "name": "scoped-alias", + "version": "0.0.0", + "private": true, + "dependencies": deps, + }); + let pkg_before = format!("{}\n", serde_json::to_string_pretty(&pkg_doc).unwrap()); + std::fs::write(proj.join("package.json"), &pkg_before).unwrap(); + + let store = tmp.path().join("pnpm-store"); + let install = corepack( + &proj, + pm, + &["install", "--store-dir", store.to_str().unwrap()], + ); + if !install.status.success() { + assert!(!pnpm_required(), "fixture install failed: {install:?}"); + println!("SKIP: fixture `pnpm install` failed: {install:?}"); + return; + } + let lock_path = proj.join("pnpm-lock.yaml"); + let lock_before = std::fs::read_to_string(&lock_path).unwrap(); + let quoted = format!("'{SCOPED}@{SCOPED_VERSION}'"); + let reference = if shape == "importer" { + format!(" version: {quoted}\n") + } else { + format!(" sl: {quoted}\n") + }; + assert!( + lock_before.contains(&reference), + "{shape}: pnpm wrote the quoted alias reference:\n{lock_before}" + ); + + let installed = if shape == "importer" { + proj.join("node_modules/sl/index.js") + } else { + proj.join(format!( + "node_modules/.pnpm/{}@{SCOPED_VERSION}/node_modules/{SCOPED}/index.js", + SCOPED.replace('/', "+") + )) + }; + let orig = std::fs::read(&installed) + .unwrap_or_else(|e| panic!("{shape}: installed {}: {e}", installed.display())); + let patched: Vec = [MARKER.as_bytes(), orig.as_slice()].concat(); + let purl = format!("pkg:npm/{SCOPED}@{SCOPED_VERSION}"); + stage_patch(&proj, &purl, "package/index.js", &orig, &patched); + let cwd = proj.to_str().unwrap(); + + let (code, stdout, stderr) = + run_socket(&proj, &["vendor", "--json", "--offline", "--cwd", cwd]); + let env = parse_envelope(&stdout); + assert_ne!(code, 0, "{shape}: the refusal fails the run.\n{env}"); + assert_eq!( + env["summary"]["applied"], 0, + "{shape}: a quoted scoped alias must not vendor.\n{env}\nstderr:\n{stderr}" + ); + assert!( + stdout.contains("vendor_lock_entry_unsupported") && stdout.contains("aliased"), + "{shape}: the refusal names the aliased reference: {env}" + ); + assert_eq!( + std::fs::read_to_string(&lock_path).unwrap(), + lock_before, + "{shape}: lock untouched" + ); + assert_eq!( + std::fs::read_to_string(proj.join("package.json")).unwrap(), + pkg_before, + "{shape}: package.json untouched" + ); + assert!( + !proj.join(format!(".socket/vendor/npm/{UUID}")).exists(), + "{shape}: a refused vendor leaves no artifact" + ); + + // The untouched lock still frozen-installs from a fresh checkout. + let fresh = tmp.path().join("fresh"); + std::fs::create_dir_all(&fresh).unwrap(); + for file in ["package.json", "pnpm-lock.yaml"] { + std::fs::copy(proj.join(file), fresh.join(file)).unwrap(); + } + if shape == "snapshot" { + std::fs::copy(proj.join("host-1.0.0.tgz"), fresh.join("host-1.0.0.tgz")).unwrap(); + } + let ci = corepack( + &fresh, + pm, + &[ + "install", + "--frozen-lockfile", + "--store-dir", + store.to_str().unwrap(), + ], + ); + assert!( + ci.status.success(), + "{shape}: fresh frozen install of the untouched lock.\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&ci.stdout), + String::from_utf8_lossy(&ci.stderr), + ); + } +} diff --git a/crates/socket-patch-core/src/crawlers/gradle_cache.rs b/crates/socket-patch-core/src/crawlers/gradle_cache.rs index ef295ee27..afd7c4fba 100644 --- a/crates/socket-patch-core/src/crawlers/gradle_cache.rs +++ b/crates/socket-patch-core/src/crawlers/gradle_cache.rs @@ -70,8 +70,7 @@ pub fn hash_eq(dir_name: &str, sha1_hex: &str) -> bool { /// Whether `bytes` are the pristine download Gradle stored in the hash /// directory `dir_name` (their sha1 names it). pub fn pristine(dir_name: &str, bytes: &[u8]) -> bool { - use sha1::{Digest, Sha1}; - hash_eq(dir_name, &hex::encode(Sha1::digest(bytes))) + hash_eq(dir_name, &crate::utils::digest::sha1_hex_of(bytes)) } /// Whether `path` is a version directory of a `files-2.1` tree @@ -432,8 +431,6 @@ impl DerivedIndex { /// The [`DerivedCopies`] of the jar `jar_leaf` whose pristine bytes /// hash to `pristine_sha1`. pub fn query(&self, jar_leaf: &str, pristine_sha1: &str) -> DerivedCopies { - use sha1::{Digest, Sha1}; - let instrumented = format!("instrumented-{jar_leaf}"); let mut out = DerivedCopies { incomplete: self.incomplete, @@ -460,7 +457,9 @@ impl DerivedIndex { out.stale.push(path.clone()); } else if name == jar_leaf || name == instrumented { match crate::utils::fs::read_regular_to_bytes_sync(path) { - Ok(bytes) if hash_eq(&hex::encode(Sha1::digest(&bytes)), pristine_sha1) => { + Ok(bytes) + if hash_eq(&crate::utils::digest::sha1_hex_of(&bytes), pristine_sha1) => + { out.stale.push(path.clone()) } Ok(_) => out.unknown.push(path.clone()), diff --git a/crates/socket-patch-core/src/patch/jvm_jar.rs b/crates/socket-patch-core/src/patch/jvm_jar.rs index 82d679406..f38a84403 100644 --- a/crates/socket-patch-core/src/patch/jvm_jar.rs +++ b/crates/socket-patch-core/src/patch/jvm_jar.rs @@ -25,8 +25,6 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use crate::crawlers::gradle_cache; use crate::hash::git_sha256::compute_git_sha256_from_bytes; use crate::manifest::schema::PatchFileInfo; @@ -353,12 +351,11 @@ fn unpatched_members( } fn sha256_hex(bytes: &[u8]) -> String { - use sha2::Digest as _; - hex::encode(sha2::Sha256::digest(bytes)) + crate::utils::digest::sha256_hex_of(bytes) } fn sha1_hex(bytes: &[u8]) -> String { - hex::encode(sha1::Sha1::digest(bytes)) + crate::utils::digest::sha1_hex_of(bytes) } /// `/jvm-originals/.jar`. diff --git a/crates/socket-patch-core/src/patch/sidecars/maven.rs b/crates/socket-patch-core/src/patch/sidecars/maven.rs index f2f5a2466..8798bfce6 100644 --- a/crates/socket-patch-core/src/patch/sidecars/maven.rs +++ b/crates/socket-patch-core/src/patch/sidecars/maven.rs @@ -17,8 +17,6 @@ use std::path::{Path, PathBuf}; -use sha1::Digest as _; - use super::{ SidecarAdvisory, SidecarAdvisoryCode, SidecarError, SidecarFile, SidecarFileAction, SidecarPayload, SidecarSeverity, @@ -44,7 +42,7 @@ impl Algo { fn digest(self, bytes: &[u8]) -> String { match self { - Algo::Sha1 => hex::encode(sha1::Sha1::digest(bytes)), + Algo::Sha1 => crate::utils::digest::sha1_hex_of(bytes), Algo::Md5 => hex::encode(md5(bytes)), } } diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock.rs b/crates/socket-patch-core/src/vendor/pnpm_lock.rs index 88777f0c7..7d9d8c32f 100644 --- a/crates/socket-patch-core/src/vendor/pnpm_lock.rs +++ b/crates/socket-patch-core/src/vendor/pnpm_lock.rs @@ -1573,6 +1573,7 @@ fn check_rewritable_refs_with( let Some((dep, _repr, rest)) = parse_key_line(&lines[line], 6) else { continue; }; + let rest = unquote_value(rest); if rest == reg_key || rest.starts_with(&key_peer_prefix) { return refuse("an aliased snapshot reference", rest); } @@ -1584,7 +1585,7 @@ fn check_rewritable_refs_with( } for entry in index.importer_ref_candidates(®_key, name, version) { let dep = entry.dep.as_str(); - if let Some(v) = entry.ver.as_deref() { + if let Some(v) = entry.ver.as_deref().map(unquote_value) { if v == reg_key || v.starts_with(&key_peer_prefix) { return refuse("an aliased importer version", v); } @@ -1615,6 +1616,7 @@ fn check_rewritable_refs_with( let Some((dep, _repr, rest)) = parse_key_line(line, 6) else { continue; }; + let rest = unquote_value(rest); if rest == reg_key || rest.starts_with(&key_peer_prefix) { return refuse("an aliased snapshot reference", rest); } @@ -1639,12 +1641,13 @@ fn check_rewritable_refs_with( continue; } let (spec, ver, f) = dep_field_lines(lines, k + 1, importer.end, 8); - if let Some((_, v)) = ver { + if let Some((_, v)) = &ver { + let v = unquote_value(v); if v == reg_key || v.starts_with(&key_peer_prefix) { - return refuse("an aliased importer version", &v); + return refuse("an aliased importer version", v); } if dep == name && v.starts_with(&val_peer_prefix) { - return refuse("a peer-suffixed importer version", &v); + return refuse("a peer-suffixed importer version", v); } if dep == name && v == version { if let Some((_, s)) = spec { @@ -2755,6 +2758,9 @@ impl LockIndex { .entry(dep.to_string()) .or_default() .push((k, at)); + // Keyed like the scan compares: pnpm quotes a value that + // starts with `@` (a scoped alias target). + let rest = unquote_value(rest); index .first_snapshot_rest .entry(rest.to_string()) @@ -2788,7 +2794,8 @@ impl LockIndex { let (spec, ver, f) = dep_field_lines(lines, k + 1, importer.end, 8); if let Some((_, v)) = &ver { let at = index.importer_deps.len(); - index.first_importer_ver.entry(v.clone()).or_insert(at); + let v = unquote_value(v); + index.first_importer_ver.entry(v.to_string()).or_insert(at); for prefix in paren_prefixes(v) { index .first_importer_ver_paren @@ -2805,7 +2812,7 @@ impl LockIndex { { index .first_importer_catalog - .entry((dep.to_string(), v.clone())) + .entry((dep.to_string(), v.to_string())) .or_insert(at); } index.importer_deps.push(ImporterDep { @@ -7270,6 +7277,93 @@ catalogs: ); } + /// #957: pnpm 9+ single-quotes a value that starts with `@`, so a + /// scoped `npm:` alias reaches the lock as + /// `version: '@scope/pkg@1.1.0'` (root importer) or + /// `sl: '@scope/pkg@1.1.0'` (a dependent's snapshot). Both are + /// references the pair surgery cannot rewrite, and must refuse exactly + /// as the unquoted unscoped alias does — on the scan AND the indexed + /// path, peer-suffixed spellings included. + #[test] + fn quoted_scoped_alias_references_refuse() { + let name = "@isaacs/string-locale-compare"; + let importer = "lockfileVersion: '9.0' + +importers: + + .: + dependencies: + sl: + specifier: npm:@isaacs/string-locale-compare@1.1.0 + version: '@isaacs/string-locale-compare@1.1.0' + +packages: + + '@isaacs/string-locale-compare@1.1.0': + resolution: {integrity: sha512-x} + +snapshots: + + '@isaacs/string-locale-compare@1.1.0': {} +"; + let snapshot = "lockfileVersion: '9.0' + +importers: + + .: + dependencies: + host: + specifier: file:./host-1.0.0.tgz + version: file:host-1.0.0.tgz + +packages: + + '@isaacs/string-locale-compare@1.1.0': + resolution: {integrity: sha512-x} + + host@file:host-1.0.0.tgz: + resolution: {integrity: sha512-y, tarball: file:host-1.0.0.tgz} + version: 1.0.0 + +snapshots: + + '@isaacs/string-locale-compare@1.1.0': {} + + host@file:host-1.0.0.tgz: + dependencies: + sl: '@isaacs/string-locale-compare@1.1.0' +"; + let peer_snapshot = snapshot.replace( + " sl: '@isaacs/string-locale-compare@1.1.0'\n", + " sl: '@isaacs/string-locale-compare@1.1.0(peer@1.0.0)'\n", + ); + let peer_importer = importer.replace( + " version: '@isaacs/string-locale-compare@1.1.0'\n", + " version: \"@isaacs/string-locale-compare@1.1.0(peer@1.0.0)\"\n", + ); + for (text, what) in [ + (importer, "an aliased importer version"), + (snapshot, "an aliased snapshot reference"), + (peer_importer.as_str(), "an aliased importer version"), + (peer_snapshot.as_str(), "an aliased snapshot reference"), + ] { + let lines = split_lines(text); + let index = LockIndex::build(&lines); + for (indexed, path) in [(None, "scan"), (Some(&index), "indexed")] { + let err = check_rewritable_refs_with(&lines, name, "1.1.0", indexed) + .expect_err(&format!("{what} must refuse ({path}):\n{text}")); + assert!(err.contains(what), "{err}"); + assert!( + err.contains("(`@isaacs/string-locale-compare@1.1.0"), + "the refusal names the unquoted reference: {err}" + ); + } + } + // The unscoped control and an unrelated version still behave. + let lines = split_lines(snapshot); + assert!(check_rewritable_refs(&lines, name, "1.0.0").is_ok()); + } + /// KNOWN GAP (coverage-audit suspected bug, needs triage): an importer /// dep entry that lost its `specifier:` field (keeping `version:`) is /// neither refused by `check_rewritable_refs` nor rewritten by @@ -8575,13 +8669,19 @@ snapshots: fn random_value(rng: &mut Lcg) -> String { let name = rng.pick(&NAMES); let version = rng.pick(&VERSIONS); - match rng.next() % 7 { + let value = match rng.next() % 7 { 0 | 1 => version.to_string(), 2 => format!("{version}(peer@1.0.0)"), 3 => format!("{name}@{version}"), 4 => format!("{name}@{version}(x@1.0.0)"), 5 => vendor_spec(rng, name, version), _ => format!("npm:{name}@{version}"), + }; + // pnpm quotes a value starting with `@` (a scoped alias target). + if value.starts_with('@') { + format!("'{value}'") + } else { + value } }