diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index d52add4f8..6ee4a5952 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1436,6 +1436,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `redirect_unattributable` | `redirect.skipped[].reason` | scan/get `--mode hosted`: the rewriters would pin the patch, but lockfile discovery over the result reads that pin as contested (another lock or requirements file resolves the same version elsewhere, or the pin is not one the package manager consumes), so `vex`, `rollback`, `remove` and `vendor` would refuse it. The candidate is left out of the rewrite, so nothing is written for it; the detail carries discovery's findings. Exit code unchanged. | | `redirect_pin_lockless` | `redirect.warnings[]` (warning) | scan/get `--mode hosted` (nuget, cargo): the pin was written without a lockfile that records its version, so `vex` cannot attest it and `rollback` / `remove` / `vendor` refuse it as unattributable. The detail names the lockfile to create (`dotnet restore --use-lock-file`, `cargo generate-lockfile`) before re-running the hosted scan. | | `redirect_pypi_stale_install` | `redirect.warnings[]` (warning) | Hosted Python redirect: readable installed files differ from patched hashes. Read-only, repeated on re-scan, and excludes the package from same-run VEX. See the "Python stale-install guard" section. | +| `redirect_nuget_stale_global_package` / `vendor_nuget_stale_global_package` | `redirect.warnings[]` / the vendor result's `warnings` (warning) | scan `--mode hosted` / `vendor` / `scan --mode vendored` (nuget, v5.0 #352): NuGet's global packages folder (`NUGET_PACKAGES`, else `~/.nuget/packages`) already holds the patched package extracted from other bytes (its `.nupkg.metadata` `contentHash` is not the patched one). A patch keeps the upstream id and version and NuGet restores a package already in that folder without asking any source, so `dotnet restore` would keep the upstream bytes (silently without a lock, NU1403 against the re-pinned lock with one). The detail names the directory and the remedy: delete it, then `dotnet restore` (`dotnet nuget locals global-packages --clear` also works but empties the whole machine-wide folder, and the detail says so); CI caches of that folder must drop it too. Read-only like the gem guard (the folder is shared machine-wide), re-fired on every re-run until the copy is gone, skipped on `--dry-run`; a hosted purl it flags is excluded from the same run's `--vex` `assume_applied` and reported stale. A dir without `.nupkg.metadata` (a legacy `packages/` folder) is never judged. | | `redirect_gem_stale_install` | `redirect.warnings[]` (warning) | scan `--mode hosted` (gem): a stale UNPATCHED materialization (installed gem, or committed archive in bundler's cache dir — `vendor/cache` unless `cache_path` moves it) that `bundle install` will reuse instead of fetching the redirected patch; the detail carries the verified remedy. Full rules and flavors: the "Gem stale-install guard" section. | | `redirect_gem_version_not_locked` | `redirect.warnings[]` (warning) | scan `--mode hosted` (gem): the crawled gem version is installed on the machine but no `GEM` section of the project's lock resolves it (another project's copy in the shared gem home). The gem is skipped and the Gemfile and lock stay byte-identical. | | `redirect_gem_no_lockfile` | `redirect.warnings[]` (warning) | scan `--mode hosted` (gem): the project has a `Gemfile` / `gems.rb` but no lock, so the version it resolves is unknown (#1125). Every gem is skipped and the Gemfile stays byte-identical; run `bundle lock` (or `bundle install`), commit the lock, and re-run. | diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index 93cc628b7..d143ea90e 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -21,6 +21,7 @@ use crate::commands::vex::generate_vex_from_manifest_path; use super::{discover_selected, ScanArgs}; +mod nuget; mod python; mod takeover; @@ -1420,6 +1421,15 @@ pub(crate) async fn run_redirect_selected( .await }; + // NuGet global packages folder probe (#352): a copy extracted from the + // upstream bytes shadows the Socket source. Read-only, like the gem + // probe; skipped on --dry-run for the same reason. + let nuget_stale = if common.dry_run { + StaleInstallOutcome::default() + } else { + nuget::stale_install_warnings(common, &confirmed, &done.overrides).await + }; + // vlt warm-tree heal: stale installed copies of the Socket-owned nodes // are invalidated (classified only on a dry run or // with --no-vlt-install-cleanup), and every confirmed vlt purl whose @@ -1579,6 +1589,7 @@ pub(crate) async fn run_redirect_selected( .map(|(purl, _)| purl.clone()) .filter(|purl| { !gem_stale.stale_purls.contains(purl) + && !nuget_stale.stale_purls.contains(purl) && !python_stale.stale_purls.contains(purl) && !vlt_stale.stale_purls.contains(purl) }) @@ -1589,6 +1600,7 @@ pub(crate) async fn run_redirect_selected( params.known_stale = python_stale .stale_purls .iter() + .chain(&nuget_stale.stale_purls) .chain(&vlt_stale.stale_purls) .cloned() .collect(); @@ -1618,6 +1630,7 @@ pub(crate) async fn run_redirect_selected( let mut warnings: Vec = socket_patch_core::hosted::render::rewrite_warnings_json(&engine_warnings); warnings.extend(gem_stale.warnings.iter().cloned()); + warnings.extend(nuget_stale.warnings.iter().cloned()); warnings.extend(python_stale.warnings.iter().cloned()); warnings.extend(vlt_stale.warnings.iter().cloned()); warnings.extend(takeover_pre_warnings.iter().cloned()); diff --git a/crates/socket-patch-cli/src/commands/scan/hosted/nuget.rs b/crates/socket-patch-cli/src/commands/scan/hosted/nuget.rs new file mode 100644 index 000000000..bbaa9cb3f --- /dev/null +++ b/crates/socket-patch-cli/src/commands/scan/hosted/nuget.rs @@ -0,0 +1,88 @@ +//! Read-only check of NuGet's global packages folder for hosted redirects +//! (#352). +//! +//! A hosted NuGet patch keeps the upstream id and version, and NuGet +//! restores a package already extracted into its global packages folder +//! (`NUGET_PACKAGES`, else `~/.nuget/packages`) without asking any source. +//! A copy extracted from the upstream bytes therefore shadows the Socket +//! source: without a lock the restore silently keeps the unpatched bytes, +//! with one it fails NU1403. Like the gem stale-install guard, nothing is +//! deleted: the remedy is prescribed, and the purl is withheld from the +//! same-run VEX attestation. + +use socket_patch_core::crawlers::NuGetCrawler; +use socket_patch_core::patch::redirect::DepOverride; +use socket_patch_core::utils::purl::strip_purl_qualifiers; +use socket_patch_core::vendor::nuget_feed::{extracted_content_hash, stale_global_package_detail}; + +use super::StaleInstallOutcome; + +/// Warn for every confirmed NuGet redirect whose package the global +/// packages folder already holds extracted from bytes other than the +/// patched ones. A dir without `.nupkg.metadata` (a legacy `packages/` +/// folder) is never judged: there is no positive evidence. +pub(super) async fn stale_install_warnings( + common: &crate::args::GlobalArgs, + confirmed: &[(String, String)], + overrides: &[DepOverride], +) -> StaleInstallOutcome { + let mut out = StaleInstallOutcome::default(); + // (purl, the patched package's NuGet content hash) + let candidates: Vec<(&String, String)> = confirmed + .iter() + .filter(|(purl, _)| purl.starts_with("pkg:nuget/")) + .filter_map(|(purl, uuid)| { + let sha512 = overrides + .iter() + .find(|o| o.ecosystem == "nuget" && &o.patch_uuid == uuid)? + .integrity + .sha512 + .as_deref()?; + Some(( + purl, + sha512.strip_prefix("sha512-").unwrap_or(sha512).to_string(), + )) + }) + .collect(); + if candidates.is_empty() { + return out; + } + let crawler = NuGetCrawler::new(); + let Ok(paths) = crawler + .get_nuget_package_paths(&common.crawler_options()) + .await + else { + return out; + }; + let purls: Vec = candidates + .iter() + .map(|(purl, _)| strip_purl_qualifiers(purl).to_string()) + .collect(); + for path in &paths { + let Ok(found) = crawler.find_by_purls(path, &purls).await else { + continue; + }; + for (purl, patched) in &candidates { + let Some(pkg) = found.get(strip_purl_qualifiers(purl)) else { + continue; + }; + let Some(cached) = extracted_content_hash(&pkg.path).await else { + continue; + }; + if cached == *patched || out.stale_purls.contains(*purl) { + continue; + } + out.warnings.push(serde_json::json!({ + "code": "redirect_nuget_stale_global_package", + "detail": stale_global_package_detail( + &pkg.name, + &pkg.version, + &pkg.path, + "the Socket source", + ), + })); + out.stale_purls.insert((*purl).clone()); + } + } + out +} diff --git a/crates/socket-patch-cli/tests/e2e_nuget_dotnet_build.rs b/crates/socket-patch-cli/tests/e2e_nuget_dotnet_build.rs index c6df01926..b18f014e5 100644 --- a/crates/socket-patch-cli/tests/e2e_nuget_dotnet_build.rs +++ b/crates/socket-patch-cli/tests/e2e_nuget_dotnet_build.rs @@ -773,37 +773,60 @@ fn nuget_hosted_dotnet_restore_then_manifestless_vex() { let backend = Backend::start(HOSTED_UUID, &pristine, &patched, Some(&nupkg)); let uri = backend.uri(); - // `scan --mode hosted --vex`: the real rewriter + the in-run VEX. - let embedded = fixture.join("scan.vex.json"); - let (code, env, stderr) = socket_patch( - &fixture, - &store_fx, - &[ - "scan", - "--mode", - "hosted", - "--json", - "--yes", - "--api-url", - &uri, - "--org", - ORG, - "--api-token", - "fake-token", - "--patch-server-url", - &uri, - "--vex", - embedded.to_str().unwrap(), - "--vex-product", - PRODUCT, - ], - ); + // `scan --mode hosted`: the real rewriter. The fixture restore left the + // UPSTREAM copy in the global packages folder, which NuGet would restore + // instead of asking the Socket source (#352): the run says so, names + // the directory to delete, and keeps saying so on re-runs until it is. + let hosted_args = [ + "scan", + "--mode", + "hosted", + "--json", + "--yes", + "--api-url", + &uri, + "--org", + ORG, + "--api-token", + "fake-token", + "--patch-server-url", + &uri, + ]; + let (code, env, stderr) = socket_patch(&fixture, &store_fx, &hosted_args); assert_eq!( code, Some(0), "SDK {sdk} scan --mode hosted: {env:#}\n{stderr}" ); assert_eq!(env["redirect"]["redirected"], 1, "{env:#}"); + let stale_dir = pkg_dir(&store_fx); + let warned = env.to_string(); + assert!( + warned.contains("redirect_nuget_stale_global_package") + && warned.contains(&stale_dir.display().to_string()), + "SDK {sdk}: the warm global packages folder is reported: {env:#}" + ); + // The prescribed remedy, then the idempotent re-run with the in-run VEX. + std::fs::remove_dir_all(&stale_dir).unwrap(); + let embedded = fixture.join("scan.vex.json"); + let mut vex_args = hosted_args.to_vec(); + vex_args.extend([ + "--vex", + embedded.to_str().unwrap(), + "--vex-product", + PRODUCT, + ]); + let (code, env, stderr) = socket_patch(&fixture, &store_fx, &vex_args); + assert_eq!( + code, + Some(0), + "SDK {sdk} scan --mode hosted --vex: {env:#}\n{stderr}" + ); + assert!( + !env.to_string() + .contains("redirect_nuget_stale_global_package"), + "SDK {sdk}: nothing stale once the copy is gone: {env:#}" + ); let doc: Value = serde_json::from_slice(&std::fs::read(&embedded).unwrap()).unwrap(); assert_attested(&doc, PURL, HOSTED_UUID, Marker::Redirected, &vulns()); let config = std::fs::read_to_string(fixture.join("nuget.config")).unwrap(); @@ -916,6 +939,13 @@ fn nuget_vendored_dotnet_restore_then_manifestless_vex() { Some(0), "SDK {sdk} scan --mode vendored: {env:#}\n{stderr}" ); + // The fixture restore's UPSTREAM copy in the global packages folder + // would shadow the vendored feed on this machine (#352): reported. + assert!( + env.to_string() + .contains("vendor_nuget_stale_global_package"), + "SDK {sdk}: the warm global packages folder is reported: {env:#}" + ); let doc: Value = serde_json::from_slice(&std::fs::read(&embedded).unwrap()).unwrap(); assert_attested(&doc, PURL, VENDORED_UUID, Marker::Vendored, &vulns()); let artifact = fixture.join(format!(".socket/vendor/nuget/{VENDORED_UUID}/{NUPKG_NAME}")); diff --git a/crates/socket-patch-core/src/vendor/nuget_feed.rs b/crates/socket-patch-core/src/vendor/nuget_feed.rs index 52b9b6868..aba5aaedd 100644 --- a/crates/socket-patch-core/src/vendor/nuget_feed.rs +++ b/crates/socket-patch-core/src/vendor/nuget_feed.rs @@ -390,6 +390,25 @@ pub async fn vendor_nuget( // originals, and re-recording here would clobber them. if config_wired { if in_sync { + // The warning keeps firing on re-runs until the stale copy is + // gone (#352). + let mut warnings = Vec::new(); + if let (Some(cached), Some(bytes)) = ( + extracted_content_hash(installed_dir).await, + read_zip_artifact(&nupkg_path).await, + ) { + if cached != sha512_base64_of(&bytes) { + warnings.push(VendorWarning::new( + "vendor_nuget_stale_global_package", + stale_global_package_detail( + name, + version, + installed_dir, + "the vendored feed", + ), + )); + } + } // A dir vendored before the re-include existed gains it now. if !dry_run { let _ = write_uuid_gitignore(&uuid_dir).await; @@ -397,7 +416,7 @@ pub async fn vendor_nuget( return done( already_patched_result(purl, &nupkg_path, &record.files), None, - Vec::new(), + warnings, ); } // Wired but the committed nupkg is missing/stale: rebuild the ARTIFACT @@ -625,11 +644,23 @@ pub async fn vendor_nuget( format!( "no project under the root restores into a {PACKAGES_LOCK} (or a \ packages..lock.json); the vendored feed serves {name} from the patched \ - copy but its contentHash is not pinned" + copy but its contentHash is not pinned, so nothing rejects an unpatched copy \ + restored from elsewhere (a warm global packages folder)" ), )); } + // A warm global packages folder shadows the vendored feed (#352): the + // crawler found the package there, extracted from other bytes. + if let Some(cached) = extracted_content_hash(installed_dir).await { + if cached != new_hash { + warnings.push(VendorWarning::new( + "vendor_nuget_stale_global_package", + stale_global_package_detail(name, version, installed_dir, "the vendored feed"), + )); + } + } + // ── marker + ledger entry ──────────────────────────────────────────── let base_purl = build_nuget_purl(name, version); let marker = VendorMarker::new("nuget", &base_purl, record, vendored_at); @@ -676,6 +707,43 @@ pub async fn vendor_nuget( done(result, Some(entry), warnings) } +// ── warm global packages folder (#352) ────────────────────────────────────── + +/// The `contentHash` NuGet recorded in `/.nupkg.metadata` when it +/// extracted a package into its global packages folder (`dir` is +/// `///`). `None` when there is none (not a +/// global-packages-folder dir, or unreadable). +pub async fn extracted_content_hash(dir: &Path) -> Option { + let text = read_regular_to_string(&dir.join(".nupkg.metadata")) + .await + .ok()?; + let doc: Value = serde_json::from_str(crate::formats::text::strip_bom(&text)).ok()?; + doc.get("contentHash") + .and_then(Value::as_str) + .map(str::to_string) +} + +/// Why and how to drop a stale copy of `id version` from NuGet's global +/// packages folder: NuGet restores a package already in that folder +/// without asking any source, so a patch that keeps the upstream id and +/// version is shadowed there — silently unpatched without a lock, NU1403 +/// against the re-pinned lock with one. `how` names what now serves the +/// patch (the vendored feed, the Socket source). +pub fn stale_global_package_detail(id: &str, version: &str, dir: &Path, how: &str) -> String { + format!( + "{id} {version} is now served by {how}, but NuGet's global packages folder already holds \ + the UNPATCHED copy at {} — NuGet restores from that folder before asking any source, so \ + `dotnet restore` keeps the upstream bytes (or fails NU1403 against the re-pinned lock). \ + Delete that directory (NuGet downloads the patched package again on the next restore) \ + and run `dotnet restore`; `dotnet nuget locals global-packages --clear` works too, but \ + empties the WHOLE folder, every package of every project on this machine. Other \ + machines and CI runners that restore a cached global packages folder must drop that \ + entry too (key the CI cache on packages.lock.json with no \ + fallback restore key)", + dir.display() + ) +} + /// The ledger entry for a vendored nupkg: `wiring` is the config + lock /// records on a full vendor, only the re-pinned lock record on an /// artifact-only rebuild (see the hot path). @@ -2290,6 +2358,62 @@ mod tests { ); } + /// #352: the crawler found the package in NuGet's global packages + /// folder, extracted from the upstream bytes; NuGet would restore that + /// copy before asking the vendored feed, so the run says so (first run + /// and the in-sync re-run alike), and stays quiet once it is patched. + #[tokio::test] + async fn warm_global_packages_folder_is_reported() { + let (dir, blobs, installed, record) = fixture(true, None).await; + let root = dir.path(); + let metadata = |hash: &str| { + format!("{{\"version\":2,\"contentHash\":\"{hash}\",\"source\":\"https://api.nuget.org/v3/index.json\"}}") + }; + tokio::fs::write(installed.join(".nupkg.metadata"), metadata("UPSTREAM==")) + .await + .unwrap(); + let (result, _entry, warnings) = + unwrap_done(run_vendor(root, &blobs, &installed, &record, false).await); + assert!(result.success, "{:?}", result.error); + let stale: Vec<&VendorWarning> = warnings + .iter() + .filter(|w| w.code == "vendor_nuget_stale_global_package") + .collect(); + assert_eq!(stale.len(), 1, "{warnings:?}"); + assert!( + stale[0].detail.contains(&installed.display().to_string()) + && stale[0] + .detail + .contains("dotnet nuget locals global-packages --clear"), + "{}", + stale[0].detail + ); + let (_r, _e, rerun) = + unwrap_done(run_vendor(root, &blobs, &installed, &record, false).await); + assert!( + rerun + .iter() + .any(|w| w.code == "vendor_nuget_stale_global_package"), + "{rerun:?}" + ); + // Extracted from the vendored bytes: nothing to report. + let nupkg = tokio::fs::read(root.join(copy_rel())).await.unwrap(); + tokio::fs::write( + installed.join(".nupkg.metadata"), + metadata(&sha512_base64_of(&nupkg)), + ) + .await + .unwrap(); + let (_r, _e, quiet) = + unwrap_done(run_vendor(root, &blobs, &installed, &record, false).await); + assert!( + !quiet + .iter() + .any(|w| w.code == "vendor_nuget_stale_global_package"), + "{quiet:?}" + ); + } + #[tokio::test] async fn happy_path_wires_config_lock_and_artifact() { let (dir, blobs, installed, record) = fixture(true, None).await;