From ff31677ad00a60f675a14b3440018c9160102da5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 05:23:42 +0000 Subject: [PATCH 1/4] Start fix for #1467 Assisted-by: Claude Code:claude-opus-5-5 From e4cffb46a1446c54d0c16e65af103d5a9bccd58a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 05:34:14 +0000 Subject: [PATCH 2/4] Name the damaged artifact in vendor --check When a committed vendored artifact was missing or changed, vendor --check printed the Rust Debug form of an internal state, such as `Corrupt { reason: "vendor_workspace_artifact_missing" }`, with no path and no next step. CI logs gave users nothing to act on. The reason now says what is wrong, names the committed file (for a Bun bun.lockb workspace, the member copy that is off, and that the root artifact is intact), and gives the fix that matches what repair does: `socket-patch repair` then commit, `socket-patch vendor` for a pending re-vendor, fix state.json for an entry that can't be verified, or upgrade for an unknown npm flavor. The stable tag stays in parentheses so logs remain searchable. Fixes #1467 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/vendor.rs | 8 +- .../tests/in_process_vendor.rs | 51 +++++ .../src/vendor/bun_workspace.rs | 22 ++- crates/socket-patch-core/src/vendor/mod.rs | 2 +- crates/socket-patch-core/src/vendor/verify.rs | 175 +++++++++++++++++- 5 files changed, 246 insertions(+), 12 deletions(-) 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/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index c53d2eaa8..662a5bd24 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`, +/// which then 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..e9628ffdf 100644 --- a/crates/socket-patch-core/src/vendor/bun_workspace.rs +++ b/crates/socket-patch-core/src/vendor/bun_workspace.rs @@ -62,19 +62,29 @@ 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) +} + +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/verify.rs b/crates/socket-patch-core/src/vendor/verify.rs index b5ed2f2f3..3a7286830 100644 --- a/crates/socket-patch-core/src/vendor/verify.rs +++ b/crates/socket-patch-core/src/vendor/verify.rs @@ -767,6 +767,64 @@ 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 { + const REPAIR: &str = "run `socket-patch repair` to restore it, then commit it"; + let path = &entry.artifact.path; + Some(match health { + ArtifactHealth::Healthy => return None, + ArtifactHealth::Missing => { + format!("committed artifact {path} is missing (vendor_artifact_missing); {REPAIR}") + } + ArtifactHealth::Corrupt { reason } if reason.starts_with("vendor_workspace_artifact_") => { + 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" + }; + format!( + "Bun workspace copy {mirror} {cause} ({reason}); the committed artifact {path} \ + it mirrors is intact; {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!( + "committed artifact {path} is for a different patch than the vendor ledger records \ + (vendor_uuid_mismatch); a re-vendor is pending: run `socket-patch vendor`" + ), + ArtifactHealth::Unverifiable { reason } => format!( + "the vendor ledger entry for {path} cannot be verified ({reason}); fix \ + .socket/vendor/state.json, for example by restoring it from version control" + ), + 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 +997,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 +1035,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 +1058,92 @@ 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 entry = entry("npm", UUID, &rel); + 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!( + "committed artifact {rel} is for a different patch than the vendor ledger \ + records (vendor_uuid_mismatch); a re-vendor is pending: run \ + `socket-patch vendor`" + ), + ), + ( + ArtifactHealth::Unverifiable { + reason: "vendor_artifact_path_invalid".into(), + }, + format!( + "the vendor ledger entry for {rel} cannot be verified \ + (vendor_artifact_path_invalid); fix .socket/vendor/state.json, for example \ + by restoring it from version control" + ), + ), + ( + 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); + } + // Without a bun.lockb there is no member copy to name: the text + // still reads as words. + let text = describe(ArtifactHealth::Corrupt { + reason: "vendor_workspace_artifact_missing".into(), + }) + .await + .unwrap(); + assert!( + text.starts_with(&format!("Bun workspace copy of {rel} is missing")), + "{text}" + ); + assert!(!text.contains('{') && !text.contains('"'), "{text}"); } /// A cargo copy's inventory taken before tagged versions keeps From c8c77bfef591c1d5e2cb9ea1163b85cfef8a3245 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 05:41:32 +0000 Subject: [PATCH 3/4] Tighten vendor --check artifact remedies Follow-up from review of the new vendor --check wording: - Only call the root artifact intact for a Bun member-copy failure when the ledger records its sha256; without one it was never checked. - An entry that can't be verified no longer always blames state.json: an unreadable bun.lockb says so, and other cases point at the ledger, the manifest or a re-vendor, and say repair won't rebuild. - A pending re-vendor says the ledger and manifest disagree. - A JVM entry also counts as missing when only its tree marker is gone, so the message names both. - vendor --check and repair now share one check for a Bun member copy failure. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/vendored_backend/repair.rs | 3 +- .../tests/in_process_vendor.rs | 4 +- crates/socket-patch-core/src/vendor/verify.rs | 67 ++++++++++++++----- 3 files changed, 53 insertions(+), 21 deletions(-) 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 662a5bd24..92bd27f74 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -4783,8 +4783,8 @@ 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`, -/// which then turns the check green again. +/// 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(); diff --git a/crates/socket-patch-core/src/vendor/verify.rs b/crates/socket-patch-core/src/vendor/verify.rs index 3a7286830..de355559c 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 @@ -782,10 +792,15 @@ pub async fn describe_artifact_failure( let path = &entry.artifact.path; 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 reason.starts_with("vendor_workspace_artifact_") => { + 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}")); @@ -794,10 +809,14 @@ pub async fn describe_artifact_failure( } else { "has different bytes" }; - format!( - "Bun workspace copy {mirror} {cause} ({reason}); the committed artifact {path} \ - it mirrors is intact; {REPAIR}" - ) + // 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() { @@ -811,12 +830,22 @@ pub async fn describe_artifact_failure( format!("committed artifact {path} {cause} ({reason}); {REPAIR}") } ArtifactHealth::StaleUuid => format!( - "committed artifact {path} is for a different patch than the vendor ledger records \ + "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" => + { + format!( + "the Bun workspace copies of {path} cannot be checked because bun.lockb cannot be \ + read or names an unsafe path ({reason}); restore bun.lockb from version control \ + or re-run `socket-patch vendor`" + ) + } ArtifactHealth::Unverifiable { reason } => format!( - "the vendor ledger entry for {path} cannot be verified ({reason}); fix \ - .socket/vendor/state.json, for example by restoring it from version control" + "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 \ @@ -1104,9 +1133,8 @@ mod tests { ( ArtifactHealth::StaleUuid, format!( - "committed artifact {rel} is for a different patch than the vendor ledger \ - records (vendor_uuid_mismatch); a re-vendor is pending: run \ - `socket-patch vendor`" + "the vendor ledger and the patch manifest name different patches for {rel} \ + (vendor_uuid_mismatch); a re-vendor is pending: run `socket-patch vendor`" ), ), ( @@ -1114,9 +1142,10 @@ mod tests { reason: "vendor_artifact_path_invalid".into(), }, format!( - "the vendor ledger entry for {rel} cannot be verified \ - (vendor_artifact_path_invalid); fix .socket/vendor/state.json, for example \ - by restoring it from version control" + "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`" ), ), ( @@ -1139,9 +1168,13 @@ mod tests { }) .await .unwrap(); - assert!( - text.starts_with(&format!("Bun workspace copy of {rel} is missing")), - "{text}" + assert_eq!( + text, + format!( + "Bun workspace copy of {rel} is missing (vendor_workspace_artifact_missing); run \ + `socket-patch repair` to restore it, then commit it" + ), + "no recorded sha256: the canonical tarball is not claimed intact" ); assert!(!text.contains('{') && !text.contains('"'), "{text}"); } From c8cec59e797dfa0be243a61f2001e43aad5080e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 06:18:42 +0000 Subject: [PATCH 4/4] Don't point at repair when it would refuse vendor --check told users to run `socket-patch repair` for any missing or damaged artifact. When the ledger records no archive SHA-256 (file artifacts) or no file inventory (directory artifacts), repair has nothing to verify a download against and refuses, so that advice left the check red. The message now gives repair's own refusal and its remedy instead. When a Bun workspace's copies can't be listed, the message used to guess at an unreadable bun.lockb. It now carries the real cause (unreadable lock, unsafe path, symlinked member copy, ...). restore() and vendor --check share one check for when repair refuses. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/bun_workspace.rs | 6 ++ .../src/vendor/redownload.rs | 23 +++-- crates/socket-patch-core/src/vendor/verify.rs | 95 +++++++++++++++---- 3 files changed, 99 insertions(+), 25 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/bun_workspace.rs b/crates/socket-patch-core/src/vendor/bun_workspace.rs index e9628ffdf..fbdb35c3e 100644 --- a/crates/socket-patch-core/src/vendor/bun_workspace.rs +++ b/crates/socket-patch-core/src/vendor/bun_workspace.rs @@ -71,6 +71,12 @@ pub(super) async fn failing_mirror(root: &Path, entry: &VendorEntry) -> Option 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))?; 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 de355559c..34fdeecc9 100644 --- a/crates/socket-patch-core/src/vendor/verify.rs +++ b/crates/socket-patch-core/src/vendor/verify.rs @@ -788,17 +788,23 @@ pub async fn describe_artifact_failure( entry: &VendorEntry, health: &ArtifactHealth, ) -> Option { - const REPAIR: &str = "run `socket-patch repair` to restore it, then commit it"; 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}" + {repair}" ), ArtifactHealth::Missing => { - format!("committed artifact {path} is missing (vendor_artifact_missing); {REPAIR}") + 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) @@ -816,7 +822,7 @@ pub async fn describe_artifact_failure( } else { format!("; the committed artifact {path} it mirrors is intact") }; - format!("Bun workspace copy {mirror} {cause} ({reason}){canonical}; {REPAIR}") + format!("Bun workspace copy {mirror} {cause} ({reason}){canonical}; {repair}") } ArtifactHealth::Corrupt { reason } => { let cause = match reason.as_str() { @@ -827,7 +833,7 @@ pub async fn describe_artifact_failure( "vendor_inventory_mismatch" => "has files that differ from its recorded inventory", _ => "failed verification", }; - format!("committed artifact {path} {cause} ({reason}); {REPAIR}") + format!("committed artifact {path} {cause} ({reason}); {repair}") } ArtifactHealth::StaleUuid => format!( "the vendor ledger and the patch manifest name different patches for {path} \ @@ -836,10 +842,11 @@ pub async fn describe_artifact_failure( 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 because bun.lockb cannot be \ - read or names an unsafe path ({reason}); restore bun.lockb from version control \ - or re-run `socket-patch vendor`" + "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!( @@ -1106,7 +1113,8 @@ mod tests { 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 entry = entry("npm", UUID, &rel); + 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 } @@ -1161,22 +1169,71 @@ mod tests { for (health, want) in cases { assert_eq!(describe(health).await.unwrap(), want); } - // Without a bun.lockb there is no member copy to name: the text - // still reads as words. - let text = describe(ArtifactHealth::Corrupt { - reason: "vendor_workspace_artifact_missing".into(), - }) + + // 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!( - "Bun workspace copy of {rel} is missing (vendor_workspace_artifact_missing); run \ - `socket-patch repair` to restore it, then commit it" - ), - "no recorded sha256: the canonical tarball is not claimed intact" + "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('{') && !text.contains('"'), "{text}"); + 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