diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index ce41397fa..072b4943d 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -1637,25 +1637,32 @@ async fn vendored_takeover( None }; // The takeover refusal (if any) for one candidate: bun gates every - // npm purl, berry and vlt only their own vendored entries. A refused - // purl is never dispatched (see the loop), so its wiring is not a - // write target here. + // npm purl, berry and vlt only their own vendored entries. Berry also + // runs the rewriter's per-dep grant gate (a grant without the berry + // cache checksum is skipped by the rewriter, so reverting first would + // leave the package in neither mode). A refused purl is never + // dispatched (see the loop), so its wiring is not a write target here. let takeover_refusal = |c: &Candidate, entry: Option<&socket_patch_core::vendor::VendorEntry>| - -> Option<&socket_patch_core::patch::redirect::RewriteWarning> { + -> Option { if !c.purl.starts_with("pkg:npm/") { return None; } + let berry = entry.is_some_and(berry_entry); bun_takeover_refusal - .as_ref() + .clone() + .or_else(|| berry_takeover_refusal.clone().filter(|_| berry)) .or_else(|| { - berry_takeover_refusal - .as_ref() - .filter(|_| entry.is_some_and(berry_entry)) + berry + .then(|| { + socket_patch_core::patch::redirect::preflight_yarn_berry_hosted_dep(&c.dep) + .err() + }) + .flatten() }) .or_else(|| { vlt_takeover_refusal - .as_ref() + .clone() .filter(|_| entry.is_some_and(vlt_entry)) }) }; @@ -1684,8 +1691,10 @@ async fn vendored_takeover( if let Some(entry) = ledger_entry { if let Some(warning) = takeover_refusal(candidate, Some(entry)) { refused.push(purl.clone()); - if !out.pre_warnings.iter().any(|w| w["code"] == warning.code) { - out.pre_warnings.push(serde_json::json!(warning)); + // Project-level refusals repeat per purl; report each once. + let warning = serde_json::json!(warning); + if !out.pre_warnings.contains(&warning) { + out.pre_warnings.push(warning); } continue; } @@ -1827,8 +1836,8 @@ async fn vendored_takeover( for purl in &refused { if let Some((c, entry)) = takeover.iter().find(|(c, _)| &c.purl == purl) { let reason = takeover_refusal(c, entry.as_ref()) - .map_or("vendored_revert_failed", |w| w.code.as_str()); - skipped.push(SkippedPatch::new(purl, &c.dep.patch_uuid, reason)); + .map_or_else(|| "vendored_revert_failed".to_string(), |w| w.code); + skipped.push(SkippedPatch::new(purl, &c.dep.patch_uuid, &reason)); } } // Purls leaving the rewrite set: refused takeovers, plus the dry-run diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 4eae3aab2..8f1abd734 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -2347,17 +2347,37 @@ pub(crate) async fn vendor_records_reusing( // whose upstream entry cannot be restored is REFUSED; the cargo // backend's `hosted_redirect_live` guard backstops the rest. if let Some(pin) = hosted_pin_of(candidate) { + let origins = crate::commands::rollback::patch_server_origins(common); + let restore_opts = socket_patch_core::patch::redirect::upstream::RestoreOptions { + dry_run: common.dry_run, + offline: common.offline, + patch_server_origins: origins.clone(), + bun_lockb: true, + }; // The refusal the berry backend would raise after the // restore, raised HERE instead — the same `failed` event, // code and detail, in the dry run and the wet run alike — // so the hosted wiring stays untouched. if candidate.starts_with("pkg:npm/") { - let refusal = berry_takeover_refusal + let project = berry_takeover_refusal .get_or_init(|| { socket_patch_core::vendor::yarn_berry_vendor_preflight(&common.cwd) }) - .await; - if let Some((code, detail)) = refusal { + .await + .clone(); + let refusal = match project { + Some(refusal) => Some(refusal), + None => { + socket_patch_core::vendor::yarn_berry_vendor_target_preflight( + &common.cwd, + candidate, + pin, + &restore_opts, + ) + .await + } + }; + if let Some((code, detail)) = &refusal { has_errors = true; env.record( PatchEvent::new(PatchAction::Failed, candidate.clone()) @@ -2367,7 +2387,6 @@ pub(crate) async fn vendor_records_reusing( continue; } } - let origins = crate::commands::rollback::patch_server_origins(common); let vlt_lock = socket_patch_core::utils::fs::read_regular_to_string( &common .cwd @@ -2388,12 +2407,7 @@ pub(crate) async fn vendor_records_reusing( let restore = socket_patch_core::patch::redirect::upstream::restore_upstream( &common.cwd, std::slice::from_ref(pin), - &socket_patch_core::patch::redirect::upstream::RestoreOptions { - dry_run: common.dry_run, - offline: common.offline, - patch_server_origins: origins, - bun_lockb: true, - }, + &restore_opts, ) .await; let refusal = restore diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index 461b6d41a..a70c1fb87 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -1031,6 +1031,13 @@ async fn berry_mixed_line_endings_fail_closed_with_code() { /// reference carrying the yarn-berry-zip checksum, the patch view) for the /// berry takeover legs. Returns the hosted tarball URL. async fn mount_berry_hosted_api(server: &wiremock::MockServer) -> String { + mount_berry_hosted_api_opts(server, true).await +} + +/// [`mount_berry_hosted_api`], with the grant's `yarn-berry-zip` artifact +/// (the `yarnBerry10c0` cache checksum) present or not. Vendored mode only +/// uses the `tarball` artifact, so a vendorable grant can lack it. +async fn mount_berry_hosted_api_opts(server: &wiremock::MockServer, berry_zip: bool) -> String { use wiremock::matchers::{method, path, path_regex}; use wiremock::{Mock, ResponseTemplate}; let org = "test-org"; @@ -1062,17 +1069,18 @@ async fn mount_berry_hosted_api(server: &wiremock::MockServer) -> String { }))) .mount(server) .await; + let mut artifacts = vec![json!({ "kind": "tarball", "url": hosted_url, + "integrity": { "sha512": "sha512-unused-by-berry==" } })]; + if berry_zip { + artifacts.push(json!({ "kind": "yarn-berry-zip", "url": hosted_url, + "integrity": { "yarnBerry10c0": format!("10c0/{}", "7".repeat(128)) } })); + } Mock::given(method("POST")) .and(path(format!("/v0/orgs/{org}/patches/package"))) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "results": { UUID: { "status": "granted", "url": hosted_url, "purl": PURL, - "artifacts": [ - { "kind": "tarball", "url": hosted_url, - "integrity": { "sha512": "sha512-unused-by-berry==" } }, - { "kind": "yarn-berry-zip", "url": hosted_url, - "integrity": { "yarnBerry10c0": format!("10c0/{}", "7".repeat(128)) } } - ], + "artifacts": artifacts, "registryOverride": null }} }))) @@ -1588,6 +1596,161 @@ async fn berry_takeovers_refuse_before_reverting_the_old_mode() { } } +/// #468: a vendored→hosted takeover whose grant has no `yarnBerry10c0` +/// cache checksum (vendored mode never needs it) must keep the package +/// vendored. The berry rewriter skips such a dep with +/// `redirect_yarn_berry_missing_checksum`; reverting the vendored wiring +/// first left it patched in neither mode while the run exited 0 announcing +/// "now fully hosted". +#[tokio::test] +async fn berry_vendored_to_hosted_takeover_keeps_vendored_without_berry_checksum() { + let server = wiremock::MockServer::start().await; + mount_berry_hosted_api_opts(&server, false).await; + let code = "redirect_yarn_berry_missing_checksum"; + for dry in [true, false] { + let ctx = format!("dry={dry}"); + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + stage_berry_project(root, BERRY_WIN_PKG, &berry_win_lock()); + let (exit, env) = vendor_cli(root, &[]); + assert_eq!(exit, 0, "{ctx}: vendor: {env:#}"); + let before = berry_wiring_snapshot(root); + let extra: &[&str] = if dry { &["--dry-run"] } else { &[] }; + let (_, env) = hosted_scan_cli_with(root, &server.uri(), extra); + let text = env.to_string(); + assert!(text.contains(code), "{ctx}: refused with {code}: {env:#}"); + for announced in [ + "redirect_takeover_reverted_vendored", + "redirect_would_revert_vendored", + ] { + assert!( + !text.contains(announced), + "{ctx}: no takeover ({announced}): {env:#}" + ); + } + assert_eq!(env["redirect"]["redirected"], 0, "{ctx}: {env:#}"); + let skipped = env["redirect"]["skipped"] + .as_array() + .cloned() + .unwrap_or_default(); + assert!( + skipped + .iter() + .any(|s| s["purl"] == PURL && s["reason"] == code), + "{ctx}: the purl is skipped with the gate's code: {env:#}" + ); + assert_eq!( + berry_wiring_snapshot(root), + before, + "{ctx}: the vendored wiring, ledger and artifact stay byte-identical" + ); + } +} + +/// #369: a hosted→vendored takeover must run the berry backend's +/// per-package gates (another locked version of the name, a user-authored +/// `resolutions` override) BEFORE restoring the upstream registry entry. +/// Restoring first left the package patched in neither mode: the hosted +/// redirect was gone and vendoring then refused with +/// `vendor_override_conflict`. +#[tokio::test] +async fn berry_hosted_to_vendored_takeover_runs_package_gates_first() { + let server = wiremock::MockServer::start().await; + mount_berry_hosted_api(&server).await; + // The upstream entry the takeover's restore reads: the gates are + // evaluated on the restored files, so the restore itself must succeed. + mount_npm_registry( + &server, + "left-pad", + "1.3.0", + npm_tgz("left-pad", "1.3.0", ORIG_INDEX), + ) + .await; + type Break = fn(&Path); + // A workspace member's lock entry for another version of the name: a + // name-keyed `resolutions` entry would move it too. + let other_version: Break = |root| { + let lock = std::fs::read_to_string(root.join("yarn.lock")).unwrap(); + let extra = format!( + "\n\"left-pad@npm:1.1.3\":\n version: 1.1.3\n \ + resolution: \"left-pad@npm:1.1.3\"\n checksum: 10c0/{}\n \ + languageName: node\n linkType: hard\n", + "5".repeat(128) + ); + std::fs::write(root.join("yarn.lock"), lock + &extra).unwrap(); + }; + // A user-authored range override for the name, merged into any + // `resolutions` table the hosted wiring already wrote. + let user_resolution: Break = |root| { + let pkg = std::fs::read_to_string(root.join("package.json")).unwrap(); + let mut pkg: Value = serde_json::from_str(&pkg).unwrap(); + let table = pkg + .as_object_mut() + .unwrap() + .entry("resolutions") + .or_insert_with(|| json!({})); + table + .as_object_mut() + .unwrap() + .insert("left-pad".into(), json!("^1.0.0")); + let text = serde_json::to_string_pretty(&pkg).unwrap() + "\n"; + std::fs::write(root.join("package.json"), text).unwrap(); + }; + for (label, breakage) in [ + ("other locked version", other_version), + ("user resolutions", user_resolution), + ] { + for dry in [true, false] { + let ctx = format!("{label} dry={dry}"); + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + stage_berry_project(root, BERRY_WIN_PKG, &berry_win_lock()); + let (exit, env) = hosted_scan_cli_with(root, &server.uri(), &[]); + assert_eq!(exit, 0, "{ctx}: hosted scan: {env:#}"); + assert_eq!(env["redirect"]["redirected"], 1, "{ctx}: {env:#}"); + breakage(root); + let before = berry_wiring_snapshot(root); + // The hosted pin's origin must count as the patch server, or + // the vendor run never sees it as a takeover. + let uri = server.uri(); + let mut args = vec![ + "vendor", + "--json", + "--cwd", + root.to_str().unwrap(), + "--patch-server-url", + &uri, + ]; + if dry { + args.push("--dry-run"); + } + let env = online_env(&uri, &uri); + let env: Vec<(&str, &str)> = env.iter().map(|(k, v)| (*k, v.as_str())).collect(); + let (exit, stdout, stderr) = run_cli(root, &args, &env); + let text = format!("{stdout}\n{stderr}"); + assert_ne!(exit, 0, "{ctx}: the refusal fails the run: {text}"); + assert!( + text.contains("vendor_override_conflict"), + "{ctx}: refused with the gate's code: {text}" + ); + for announced in [ + "vendor_takeover_reverted_redirect", + "vendor_would_revert_redirect", + ] { + assert!( + !text.contains(announced), + "{ctx}: no takeover ({announced}): {text}" + ); + } + assert_eq!( + berry_wiring_snapshot(root), + before, + "{ctx}: the hosted lock edits stay byte-identical" + ); + } + } +} + // ───────────────────────────────────────────────────────────────────── // 9. offline with no local source // ───────────────────────────────────────────────────────────────────── diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index ceed30f70..86fc5ae1f 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -3233,6 +3233,32 @@ fn berry_cache_key(content: &str) -> Option { /// compares the file with its own majority-normalized re-render and fails /// (YN0028), while a plain install rewrites every minority line — so it is /// refused untouched, `yarn install` normalizes it first. +/// The grant prerequisite for creating a new yarn berry hosted pin: a dep +/// whose grant carries no `yarnBerry10c0` cache checksum cannot be redirected +/// (berry verifies the converted cache zip, and only the service can compute +/// that checksum). +/// +/// Exposed for the vendored→hosted mode takeover, like +/// [`preflight_yarn_berry_hosted`]: vendored mode only uses the `tarball` +/// artifact, so a vendorable patch can lack the berry checksum, and the +/// takeover must keep such a package vendored instead of reverting it and +/// then skipping the redirect. +/// Keep this unconditional gate at the takeover boundary: a lock-aware +/// rewriter may retain an already complete pin's stored checksum. +pub fn preflight_yarn_berry_hosted_dep(dep: &DepOverride) -> Result<(), RewriteWarning> { + if dep.integrity.yarn_berry10c0.is_some() { + return Ok(()); + } + Err(RewriteWarning { + code: "redirect_yarn_berry_missing_checksum".into(), + detail: format!( + "{}@{} has no yarnBerry10c0 cache checksum", + full_name(dep), + dep.version + ), + }) +} + pub fn preflight_yarn_berry_hosted(lock: &str, yarnrc: Option<&str>) -> Result<(), RewriteWarning> { if !is_berry_lock(lock) { return Ok(()); diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs b/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs index 4e0eaf2ac..ea0fe324f 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs @@ -337,6 +337,10 @@ pub struct RestoreOutcome { pub reverted_files: Vec, /// Advisory `(code, detail)` pairs. pub warnings: Vec<(&'static str, String)>, + /// The new text of each root-relative text file the restore rewrote (or + /// would, on a dry run); `None` for a file it removed. Lets a caller + /// evaluate the restored project before anything is written. + pub staged_text: BTreeMap>, /// A write failure after every pin resolved: some files may have /// landed. `None` on a clean flush (and always on a dry run). pub flush_error: Option, @@ -649,6 +653,7 @@ pub async fn restore_upstream( pins: pins_out, reverted_files: reverted_files.into_iter().collect(), warnings: result.warnings, + staged_text: changed, flush_error, } } diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index d72edf292..d9355dfab 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -128,7 +128,7 @@ pub use verify::{ }; // The hosted→vendored takeover refuses a berry project the backend would // refuse BEFORE it reverts the hosted redirect. -pub use yarn_berry_lock::yarn_berry_vendor_preflight; +pub use yarn_berry_lock::{yarn_berry_vendor_preflight, yarn_berry_vendor_target_preflight}; use std::collections::{HashMap, HashSet}; use std::path::Path; diff --git a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs index dcd692c95..bb7b01909 100644 --- a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs @@ -1103,6 +1103,64 @@ pub async fn yarn_berry_vendor_preflight(project_root: &Path) -> Option<(&'stati .and_then(into_pair) } +/// The per-package twin of [`yarn_berry_vendor_preflight`] for the +/// hosted→vendored takeover: the backend's `resolutions` conflict gate and +/// its lock-entry gates for `purl` (another version of the name, a +/// non-npm protocol, a mixed-descriptor or duplicate entry). Evaluated on +/// the files as the takeover's restore of `pin` will leave them — a dry-run +/// [`restore_upstream`] supplies the restored `package.json` / `yarn.lock` +/// text, so hosted wiring socket-patch itself wrote (a lock rewrite, or a +/// Socket-owned `resolutions` pin) is never mistaken for a user override. +/// Returns `(code, detail)`, exactly the refusal the backend would raise +/// after the restore; `None` when it would not refuse, when the restore +/// itself would refuse (the takeover reports that), or when the files are +/// unreadable (which the backend reports itself). +/// +/// [`restore_upstream`]: crate::patch::redirect::upstream::restore_upstream +pub async fn yarn_berry_vendor_target_preflight( + project_root: &Path, + purl: &str, + pin: &crate::patch::redirect::upstream::HostedPin, + opts: &crate::patch::redirect::upstream::RestoreOptions, +) -> Option<(&'static str, String)> { + use super::npm_flavor::{detect_npm_lock_flavor, NpmLockFlavor}; + use crate::patch::redirect::upstream::{restore_upstream, RestoreOptions}; + if !matches!( + detect_npm_lock_flavor(project_root).await, + Ok((NpmLockFlavor::YarnBerry, _)) + ) { + return None; + } + let (name, version) = super::npm_common::parse_npm_purl(purl)?; + let dry = RestoreOptions { + dry_run: true, + ..opts.clone() + }; + let restore = restore_upstream(project_root, std::slice::from_ref(pin), &dry).await; + if restore.refused().next().is_some() { + return None; + } + let pkg_bytes = match restore.staged_text.get(PACKAGE_JSON) { + Some(Some(text)) => text.clone().into_bytes(), + Some(None) => return None, + None => read_regular_to_bytes(&project_root.join(PACKAGE_JSON)) + .await + .ok()?, + }; + let pkg = parse_json_manifest(&pkg_bytes).ok()?; + if let Err(outcome) = resolutions_gate(pkg.as_object()?, &name, &version) { + if let VendorOutcome::Refused { code, detail } = *outcome { + return Some((code, detail)); + } + } + let lock_text = match restore.staged_text.get(YARN_LOCK) { + Some(Some(text)) => text.clone(), + Some(None) => return None, + None => read_yarn_lock(project_root).await.ok()?, + }; + scan_berry_target(&scan_blocks(&lock_text), &name, &version).err() +} + /// Commit the pair in contract order — package.json first, yarn.lock second /// — unwinding package.json to its original bytes when the lock write fails /// (a resolutions entry without its lock counterpart would let a plain