diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index befc92dc6..f34123a42 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1254,10 +1254,10 @@ async fn run_check(args: &VendorArgs) -> i32 { for (key, entry) in entries { let record = entry.record.as_ref().or_else(|| manifest.patches.get(key)); let mut failure = match record { - Some(record) => match vendor::check_vendored_artifact(root, entry, record).await { - vendor::ArtifactHealth::Healthy => None, - health => Some(format!("artifact verification failed: {health:?}")), - }, + Some(record) => { + let health = vendor::check_vendored_artifact(root, entry, record).await; + vendor::describe_artifact_failure(root, entry, &health).await + } None => Some("patch record missing; restore the manifest or vendor ledger".to_string()), }; if failure.is_none() && vendor::jvm::apply::is_jvm_entry(entry) { diff --git a/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs b/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs index 42c8b946a..ae99806d7 100644 --- a/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs +++ b/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs @@ -760,8 +760,7 @@ impl VendoredBackend<'_> { } fn workspace_copy_issue(health: &ArtifactHealth) -> bool { - matches!(health, ArtifactHealth::Corrupt { reason } - if reason == "vendor_workspace_artifact_missing" || reason == "vendor_workspace_artifact_corrupt") + health.is_workspace_copy_issue() } /// Preserve package originals while adopting/rebuilding every member-relative diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index c53d2eaa8..92bd27f74 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -4780,6 +4780,57 @@ async fn vendor_check_fails_when_lock_no_longer_wires_artifact() { ); } +/// REGRESSION (#1467): a missing or changed committed artifact failed +/// `vendor --check` with the Rust Debug text of the health state +/// (`Missing`, `Corrupt { reason: "vendor_sha256_mismatch" }`): no path, +/// no remedy. The reason must name the artifact and point at `repair`; +/// restoring the committed bytes turns the check green again. +#[tokio::test] +async fn vendor_check_names_damaged_artifact_and_repair_remedy() { + let fx = npm_fixture(); + assert_eq!(vendor_run(vendor_args(fx.root())).await, 0, "vendor"); + let rel = fx + .tgz_path() + .strip_prefix(fx.root()) + .unwrap() + .to_string_lossy() + .replace('\\', "/"); + let good = std::fs::read(fx.tgz_path()).unwrap(); + + let mut changed = good.clone(); + changed.extend_from_slice(b"drift"); + std::fs::write(fx.tgz_path(), &changed).unwrap(); + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 1, "{env:#}"); + let event = find_event(&env, "failed", Some("vendor_check_failed")); + assert_eq!( + event["reason"], + format!( + "committed artifact {rel} does not match the sha256 in the vendor ledger \ + (vendor_sha256_mismatch); run `socket-patch repair` to restore it, then commit it" + ), + "{env:#}" + ); + + std::fs::remove_file(fx.tgz_path()).unwrap(); + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 1, "{env:#}"); + let event = find_event(&env, "failed", Some("vendor_check_failed")); + assert_eq!( + event["reason"], + format!( + "committed artifact {rel} is missing (vendor_artifact_missing); run \ + `socket-patch repair` to restore it, then commit it" + ), + "{env:#}" + ); + + std::fs::write(fx.tgz_path(), &good).unwrap(); + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 0, "{env:#}"); + find_event(&env, "verified", Some("vendor_check_ok")); +} + /// REGRESSION (#969): npm >= 11.14 refuses a vendored `file:` tarball /// (EALLOWFILE) under a project `allow-file=none`, so every install of the /// lock fails — but `vendor --check` verified it. It must fail the entry, diff --git a/crates/socket-patch-core/src/vendor/bun_workspace.rs b/crates/socket-patch-core/src/vendor/bun_workspace.rs index dde1060cf..fbdb35c3e 100644 --- a/crates/socket-patch-core/src/vendor/bun_workspace.rs +++ b/crates/socket-patch-core/src/vendor/bun_workspace.rs @@ -62,19 +62,35 @@ fn required_mirrors( /// Check every location an older Bun workspace reader can install from. pub(super) async fn verify(root: &Path, entry: &VendorEntry) -> Result<(), String> { - let mirrors = required_mirrors(root, entry).map_err(|_| "vendor_workspace_artifact_invalid")?; + check(root, entry).await.map_err(|(tag, _)| tag.to_string()) +} + +/// The project-relative path of the first workspace copy [`verify`] rejects, +/// so a failure can name the member copy that is off. +pub(super) async fn failing_mirror(root: &Path, entry: &VendorEntry) -> Option { + check(root, entry).await.err().and_then(|(_, rel)| rel) +} + +/// Why the workspace copies cannot be listed at all (the detail behind +/// `vendor_workspace_artifact_invalid`). +pub(super) fn mirror_list_error(root: &Path, entry: &VendorEntry) -> Option { + required_mirrors(root, entry).err() +} + +async fn check(root: &Path, entry: &VendorEntry) -> Result<(), (&'static str, Option)> { + let mirrors = + required_mirrors(root, entry).map_err(|_| ("vendor_workspace_artifact_invalid", None))?; if mirrors.is_empty() { return Ok(()); } let expected = super::verify::file_sha256_hex(&root.join(&entry.artifact.path)) .await - .ok_or("vendor_artifact_unreadable")?; - for (_, _, path) in mirrors { + .ok_or(("vendor_artifact_unreadable", None))?; + for (_, rel, path) in mirrors { match super::verify::file_sha256_hex(&path).await { Some(actual) if actual == expected => {} - Some(_) => return Err("vendor_workspace_artifact_corrupt".into()), - None if !path.exists() => return Err("vendor_workspace_artifact_missing".into()), - None => return Err("vendor_workspace_artifact_corrupt".into()), + None if !path.exists() => return Err(("vendor_workspace_artifact_missing", Some(rel))), + _ => return Err(("vendor_workspace_artifact_corrupt", Some(rel))), } } Ok(()) diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index ca0b10bab..6e5042c27 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 state::{ }; pub use verify::{ artifact_is_file_shaped, check_vendored_artifact, compute_dir_inventory, - compute_package_dir_inventory, file_sha256_hex, ArtifactHealth, + compute_package_dir_inventory, describe_artifact_failure, file_sha256_hex, ArtifactHealth, }; // The hosted→vendored takeover refuses a berry project the backend would // refuse BEFORE it reverts the hosted redirect. diff --git a/crates/socket-patch-core/src/vendor/redownload.rs b/crates/socket-patch-core/src/vendor/redownload.rs index 76d8f1e65..7358a3c6a 100644 --- a/crates/socket-patch-core/src/vendor/redownload.rs +++ b/crates/socket-patch-core/src/vendor/redownload.rs @@ -52,6 +52,20 @@ const NO_FILE_INVENTORY: &str = "the ledger has no complete file inventory; rest version control, or re-run the vendoring command (`socket-patch vendor`, or `scan --mode \ vendored`) to rebuild it from a verified download"; +/// Why [`restore`] refuses this entry before downloading anything: the +/// ledger records nothing to check a fresh download against. +pub(super) fn ledger_blocks_restore(entry: &VendorEntry) -> Option { + let file_shaped = !super::verify::is_vlt_dir_entry(entry) + && super::verify::artifact_is_file_shaped(&entry.artifact.path); + if file_shaped && entry.artifact.sha256.is_empty() { + return Some(no_archive_sha256()); + } + if !file_shaped && entry.artifact.file_inventory.is_none() { + return Some(NO_FILE_INVENTORY.into()); + } + None +} + /// Restore only the recorded artifact. Project wiring and ledger are never written. pub async fn restore( root: &Path, @@ -101,14 +115,11 @@ pub async fn restore( if crate::utils::containment::linked_level(root, &root.join(&entry.artifact.path)).is_some() { return Err("vendor_path_unsafe: artifact path contains a symlink".into()); } + if let Some(detail) = ledger_blocks_restore(entry) { + return Err(detail); + } let file_shaped = !super::verify::is_vlt_dir_entry(entry) && super::verify::artifact_is_file_shaped(&entry.artifact.path); - if file_shaped && entry.artifact.sha256.is_empty() { - return Err(no_archive_sha256()); - } - if !file_shaped && entry.artifact.file_inventory.is_none() { - return Err(NO_FILE_INVENTORY.into()); - } let socket = root.join(".socket"); tokio::fs::create_dir_all(&socket) .await diff --git a/crates/socket-patch-core/src/vendor/verify.rs b/crates/socket-patch-core/src/vendor/verify.rs index b5ed2f2f3..34fdeecc9 100644 --- a/crates/socket-patch-core/src/vendor/verify.rs +++ b/crates/socket-patch-core/src/vendor/verify.rs @@ -692,6 +692,16 @@ pub enum ArtifactHealth { UnknownFlavor { flavor: String }, } +impl ArtifactHealth { + /// Only a Bun member-relative copy of the canonical tarball is off; + /// `repair` rewrites it from the committed tarball. + pub fn is_workspace_copy_issue(&self) -> bool { + matches!(self, ArtifactHealth::Corrupt { reason } + if reason == "vendor_workspace_artifact_missing" + || reason == "vendor_workspace_artifact_corrupt") + } +} + /// Health-check one vendored artifact against its patch record: the /// per-file afterHash verification of [`verify_vendored_patch_record`] /// (which for dir-shaped artifacts includes the whole-tree fileInventory @@ -767,6 +777,90 @@ pub async fn check_vendored_artifact( } } +/// Say in words why [`check_vendored_artifact`] failed: the cause, the +/// committed path that is off and the step that fixes it, with the stable +/// routing tag kept in parentheses for log searches. `None` for a healthy +/// artifact. The remedy follows what `repair` does for each state: it +/// restores a missing or corrupt artifact, skips a pending re-vendor, and +/// refuses an entry it cannot verify. +pub async fn describe_artifact_failure( + project_root: &Path, + entry: &VendorEntry, + health: &ArtifactHealth, +) -> Option { + let path = &entry.artifact.path; + // `repair` restores a damaged artifact from a verified download (or, + // for a Bun member copy, from the verified canonical tarball); with + // nothing in the ledger to verify against, it refuses. + let repair = match super::redownload::ledger_blocks_restore(entry) { + Some(detail) => format!("`socket-patch repair` cannot restore it: {detail}"), + None => "run `socket-patch repair` to restore it, then commit it".to_string(), + }; + Some(match health { + ArtifactHealth::Healthy => return None, + // A JVM tree also counts as missing when only its marker is gone. + ArtifactHealth::Missing if super::jvm::apply::is_jvm_entry(entry) => format!( + "committed artifact {path} or its tree marker is missing (vendor_artifact_missing); \ + {repair}" + ), + ArtifactHealth::Missing => { + format!("committed artifact {path} is missing (vendor_artifact_missing); {repair}") + } + ArtifactHealth::Corrupt { reason } if health.is_workspace_copy_issue() => { + let mirror = super::bun_workspace::failing_mirror(project_root, entry) + .await + .unwrap_or_else(|| format!("of {path}")); + let cause = if reason == "vendor_workspace_artifact_missing" { + "is missing" + } else { + "has different bytes" + }; + // The canonical tarball was checked against the ledger only + // when the ledger records its sha256. + let canonical = if entry.artifact.sha256.is_empty() { + String::new() + } else { + format!("; the committed artifact {path} it mirrors is intact") + }; + format!("Bun workspace copy {mirror} {cause} ({reason}){canonical}; {repair}") + } + ArtifactHealth::Corrupt { reason } => { + let cause = match reason.as_str() { + "vendor_sha256_mismatch" => "does not match the sha256 in the vendor ledger", + "vendor_hash_mismatch" => "has a patched file that does not match the patch", + "file_not_found" => "is missing a patched file", + "vendor_artifact_unreadable" => "cannot be read", + "vendor_inventory_mismatch" => "has files that differ from its recorded inventory", + _ => "failed verification", + }; + format!("committed artifact {path} {cause} ({reason}); {repair}") + } + ArtifactHealth::StaleUuid => format!( + "the vendor ledger and the patch manifest name different patches for {path} \ + (vendor_uuid_mismatch); a re-vendor is pending: run `socket-patch vendor`" + ), + ArtifactHealth::Unverifiable { reason } + if reason == "vendor_workspace_artifact_invalid" => + { + let detail = super::bun_workspace::mirror_list_error(project_root, entry) + .unwrap_or_else(|| "the workspace copies could not be listed".into()); + format!( + "the Bun workspace copies of {path} cannot be checked: {detail} ({reason}); \ + `socket-patch repair` will not rebuild them until that is fixed" + ) + } + ArtifactHealth::Unverifiable { reason } => format!( + "committed artifact {path} cannot be verified ({reason}), so `socket-patch repair` \ + will not rebuild it; restore .socket/vendor/state.json and .socket/manifest.json \ + from version control, or re-run `socket-patch vendor`" + ), + ArtifactHealth::UnknownFlavor { flavor } => format!( + "{path} was vendored for the npm flavor `{flavor}`, which this socket-patch release \ + does not understand; upgrade socket-patch" + ), + }) +} + /// Plain sha256 hex of a regular file, size-capped; `None` on any read /// failure or cap breach. Public for repair's ledger re-synthesis (the /// rebuilt artifact's recorded sha). Opens once through the shared guarded @@ -939,12 +1033,29 @@ mod tests { ); lock.set_package(id, &rel, &sri).unwrap(); std::fs::write(root.path().join("bun.lockb"), lock.bytes()).unwrap(); + let health = check_vendored_artifact(root.path(), &entry, &record).await; assert_eq!( - check_vendored_artifact(root.path(), &entry, &record).await, + health, ArtifactHealth::Corrupt { reason: "vendor_workspace_artifact_missing".into() } ); + // #1467: the failure names the member copy, says the canonical + // artifact is fine and gives the remedy, never Rust Debug syntax. + let mirror = super::super::bun_workspace::failing_mirror(root.path(), &entry) + .await + .unwrap(); + assert!(mirror.ends_with(&rel) && mirror != rel, "{mirror}"); + assert_eq!( + describe_artifact_failure(root.path(), &entry, &health) + .await + .unwrap(), + format!( + "Bun workspace copy {mirror} is missing (vendor_workspace_artifact_missing); \ + the committed artifact {rel} it mirrors is intact; run `socket-patch repair` \ + to restore it, then commit it" + ) + ); entry.wiring = super::super::bun_workspace::repair(root.path(), &entry, false) .await .unwrap() @@ -960,6 +1071,18 @@ mod tests { .unwrap_err(), "vendor_workspace_artifact_corrupt" ); + let health = check_vendored_artifact(root.path(), &entry, &record).await; + assert_eq!( + describe_artifact_failure(root.path(), &entry, &health) + .await + .unwrap(), + format!( + "Bun workspace copy {} has different bytes (vendor_workspace_artifact_corrupt); \ + the committed artifact {rel} it mirrors is intact; run `socket-patch repair` \ + to restore it, then commit it", + entry.wiring[0].file + ) + ); // The archive's patched member remains readable, but the canonical // fingerprint must take precedence over the damaged member copy. let mut changed = bytes; @@ -971,6 +1094,146 @@ mod tests { reason: "vendor_sha256_mismatch".into() } ); + let health = check_vendored_artifact(root.path(), &entry, &record).await; + assert_eq!( + describe_artifact_failure(root.path(), &entry, &health) + .await + .unwrap(), + format!( + "committed artifact {rel} does not match the sha256 in the vendor ledger \ + (vendor_sha256_mismatch); run `socket-patch repair` to restore it, then \ + commit it" + ) + ); + } + + /// #1467: every failed health state reads as a cause, the committed + /// path and the step `repair` points at for it, never as Rust Debug. + #[tokio::test] + async fn describe_artifact_failure_names_path_and_remedy() { + let root = tempfile::tempdir().unwrap(); + let rel = format!(".socket/vendor/npm/{UUID}/minimist-1.2.2.tgz"); + let mut entry = entry("npm", UUID, &rel); + entry.artifact.sha256 = "ab".repeat(32); + let describe = |health: ArtifactHealth| { + let (root, entry) = (root.path().to_path_buf(), entry.clone()); + async move { describe_artifact_failure(&root, &entry, &health).await } + }; + assert_eq!(describe(ArtifactHealth::Healthy).await, None); + let cases = [ + ( + ArtifactHealth::Missing, + format!( + "committed artifact {rel} is missing (vendor_artifact_missing); run \ + `socket-patch repair` to restore it, then commit it" + ), + ), + ( + ArtifactHealth::Corrupt { + reason: "vendor_hash_mismatch".into(), + }, + format!( + "committed artifact {rel} has a patched file that does not match the patch \ + (vendor_hash_mismatch); run `socket-patch repair` to restore it, then \ + commit it" + ), + ), + ( + ArtifactHealth::StaleUuid, + format!( + "the vendor ledger and the patch manifest name different patches for {rel} \ + (vendor_uuid_mismatch); a re-vendor is pending: run `socket-patch vendor`" + ), + ), + ( + ArtifactHealth::Unverifiable { + reason: "vendor_artifact_path_invalid".into(), + }, + format!( + "committed artifact {rel} cannot be verified (vendor_artifact_path_invalid), \ + so `socket-patch repair` will not rebuild it; restore \ + .socket/vendor/state.json and .socket/manifest.json from version control, \ + or re-run `socket-patch vendor`" + ), + ), + ( + ArtifactHealth::UnknownFlavor { + flavor: "future".into(), + }, + format!( + "{rel} was vendored for the npm flavor `future`, which this socket-patch \ + release does not understand; upgrade socket-patch" + ), + ), + ]; + for (health, want) in cases { + assert_eq!(describe(health).await.unwrap(), want); + } + + // With no recorded sha256, repair has nothing to verify a download + // (or the canonical tarball) against and refuses, so the text says + // so instead of sending the user to it. Without a bun.lockb there is + // no member copy to name, and the canonical is not claimed intact. + entry.artifact.sha256.clear(); + let no_sha = describe_artifact_failure( + root.path(), + &entry, + &ArtifactHealth::Corrupt { + reason: "vendor_workspace_artifact_missing".into(), + }, + ) + .await + .unwrap(); + assert_eq!( + no_sha, + format!( + "Bun workspace copy of {rel} is missing (vendor_workspace_artifact_missing); \ + `socket-patch repair` cannot restore it: {}", + super::super::redownload::ledger_blocks_restore(&entry).unwrap() + ) + ); + assert!(no_sha.contains("no archive SHA-256"), "{no_sha}"); + // A dir-shaped artifact with no file inventory: the same refusal. + let dir = entry_dir_no_inventory(); + let text = describe_artifact_failure(root.path(), &dir, &ArtifactHealth::Missing) + .await + .unwrap(); + assert!( + text.contains("`socket-patch repair` cannot restore it: the ledger has no complete file inventory"), + "{text}" + ); + + // An unreadable bun.lockb: the workspace copies can't be listed, and + // the text carries the real cause rather than a guess. + entry.flavor = Some("bun".into()); + std::fs::write(root.path().join("bun.lockb"), b"not a lockb").unwrap(); + let detail = super::super::bun_workspace::mirror_list_error(root.path(), &entry).unwrap(); + let text = describe_artifact_failure( + root.path(), + &entry, + &ArtifactHealth::Unverifiable { + reason: "vendor_workspace_artifact_invalid".into(), + }, + ) + .await + .unwrap(); + assert_eq!( + text, + format!( + "the Bun workspace copies of {rel} cannot be checked: {detail} \ + (vendor_workspace_artifact_invalid); `socket-patch repair` will not rebuild \ + them until that is fixed" + ) + ); + assert!(!text.contains("Corrupt {"), "{text}"); + } + + fn entry_dir_no_inventory() -> VendorEntry { + entry( + "cargo", + UUID, + &format!(".socket/vendor/cargo/{UUID}/cfg-if-1.0.4"), + ) } /// A cargo copy's inventory taken before tagged versions keeps