diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index d7b8f728e..c6d0bf7f6 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -402,6 +402,7 @@ async fn run_check(args: &ApplyArgs, manifest_path: &Path) -> i32 { | GoDrift::WrongReplacePath { purl, .. } | GoDrift::ResolvedVersionMismatch { purl, .. } | GoDrift::UnprovenSelection { purl, .. } + | GoDrift::UnprovenStaleSum { purl, .. } | GoDrift::ReplaceTargetOnly { purl, .. } => purl.clone(), GoDrift::OrphanReplace { module } => module.clone(), }; diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index 78f8111d5..0265f0048 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -1044,6 +1044,11 @@ async fn generate_vex( {version} is ignored and the build links the unpatched module", entry.module ), + Some(Selection::Unproven) => format!( + "nothing shows the build selects {} {version}: {}", + entry.module, + socket_patch_core::vendor::go_build_list::UNPROVEN_REASON + ), Some(Selection::Unknown) if socket_patch_core::vendor::go_workspace::resolve(&root) != socket_patch_core::vendor::go_workspace::GoWork::None => diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index 5a9bfa51d..e98aab90c 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -2215,3 +2215,170 @@ fn goflags_modfile_keeps_the_go_mod_replace_from_the_build() { chmod_writable(tmp.path()); } + +// ── a go.sum line a downgrade left behind (#1862) ── + +/// A go 1.16 consumer of the upstream at `from`, downgraded to `to` by +/// `go get`, which lowers the require but keeps `from`'s go.sum lines +/// until `go mod tidy`. +fn downgraded_consumer(consumer: &Path, from: &str, to: &str, env: &[(&str, &str)]) { + std::fs::write( + consumer.join("go.mod"), + format!("module example.com/consumer\n\ngo 1.16\n\nrequire {UMOD} {from}\n"), + ) + .unwrap(); + for args in [vec!["mod", "tidy"], vec!["get", &format!("{UMOD}@{to}")]] { + let out = go(consumer, &args, env); + assert!( + out.status.success(), + "go {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + } + let go_mod = std::fs::read_to_string(consumer.join("go.mod")).unwrap(); + assert!(go_mod.contains(&format!("{UMOD} {to}")), "{go_mod}"); + let sum = std::fs::read_to_string(consumer.join("go.sum")).unwrap(); + assert!( + sum.contains(&format!("{UMOD} {from} h1:")), + "go get must keep the stale zip line:\n{sum}" + ); +} + +/// Publish the upstream at `version` with a greeting of `greeting`. +fn publish_upstream(tmp: &Path, version: &str, greeting: &str) { + publish( + tmp, + UMOD, + version, + &[ + ("go.mod", &format!("module {UMOD}\n\ngo 1.16\n")), + ( + "lib.go", + &format!( + "package upstream\n\nfunc Greeting() string {{ return \"{greeting}\" }}\n" + ), + ), + ], + ); +} + +/// #1862 case 1: downgraded below the patched version, the build links +/// the older module. The stale zip line at the patched version must not +/// pass for proof it is built: apply refuses, and a replace written +/// before the downgrade fails `apply --check` (with or without the module +/// cache to read the graph from) and is left out of VEX. +#[test] +fn downgrade_below_the_patch_leaves_its_replace_unattested() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(downgrade-below)") { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let (consumer, modcache, proxy_url) = stage_with(tmp.path(), "1.16"); + publish_upstream(tmp.path(), "v0.9.0", "OLDER"); + let env = go_env(modcache.to_str().unwrap(), &proxy_url); + let readonly = readonly_env(&env); + let empty_cache = tmp.path().join("empty-modcache"); + std::fs::create_dir_all(&empty_cache).unwrap(); + + // A fresh apply after the downgrade refuses and writes nothing. + downgraded_consumer(&consumer, UVER, "v0.9.0", &env); + write_patch(&consumer); + let (code, so, se) = apply_json(&consumer, &modcache); + assert_ne!(code, 0, "apply must refuse the inert replace:\n{so}\n{se}"); + assert!( + so.contains(&format!("the build selects {UMOD} v0.9.0")), + "{so}\n{se}" + ); + assert!(replace_lines(&consumer.join("go.mod")).is_empty()); + // Applied before the downgrade: the replace is inert afterwards. + std::fs::write( + consumer.join("go.mod"), + format!("module example.com/consumer\n\ngo 1.16\n\nrequire {UMOD} {UVER}\n"), + ) + .unwrap(); + let tidy = go(&consumer, &["mod", "tidy"], &env); + assert!(tidy.status.success()); + let (code, so, se) = apply_json(&consumer, &modcache); + assert_eq!(code, 0, "apply at the required version:\n{so}\n{se}"); + assert!(go_out(&consumer, &readonly).contains("OUT: PATCHED")); + let socket_replace = replace_lines(&consumer.join("go.mod")); + assert_eq!(socket_replace.len(), 1, "{socket_replace:?}"); + let get = go(&consumer, &["get", &format!("{UMOD}@v0.9.0")], &env); + assert!( + get.status.success(), + "{}", + String::from_utf8_lossy(&get.stderr) + ); + // The directory replace dropped the original's go.sum lines; an + // untidied go.sum from before the apply still carries them. + let mut sum = std::fs::read_to_string(consumer.join("go.sum")).unwrap(); + if !sum.contains(&format!("{UMOD} {UVER} h1:")) { + let pristine = tmp.path().join("pristine"); + consumer_module( + &pristine, + "example.com/pristine", + modcache.to_str().unwrap(), + &proxy_url, + ); + sum.push_str(&std::fs::read_to_string(pristine.join("go.sum")).unwrap()); + std::fs::write(consumer.join("go.sum"), sum).unwrap(); + } + assert_eq!(replace_lines(&consumer.join("go.mod")), socket_replace); + assert!(go_out(&consumer, &readonly).contains("OUT: OLDER")); + + let (code, so, se) = check(&consumer, &modcache); + assert_eq!( + code, 1, + "apply --check must flag the inert replace:\n{so}\n{se}" + ); + assert!( + format!("{so}{se}").contains("is not the version the build selects (v0.9.0)"), + "{so}\n{se}" + ); + let (code, so, se) = check(&consumer, &empty_cache); + assert_eq!(code, 1, "without the cache too:\n{so}\n{se}"); + assert!( + format!("{so}{se}").contains("nothing shows the build selects the patched version"), + "{so}\n{se}" + ); + let r = VexRun { + offline: true, + product: Some("pkg:golang/example.com/consumer".into()), + ..VexRun::default() + } + .env("GOMODCACHE", &modcache) + .env("GOFLAGS", ""); + let out = run_vex(&vex_e2e_common::binary(), &consumer, &r); + assert_absent(out.doc.as_ref(), UPURL); + vex_e2e_common::assert_not_attested(&out.envelope, UPURL, "go_version_not_selected"); + + chmod_writable(tmp.path()); +} + +/// #1862 case 2: downgraded to the patched version, the build links it. +/// The stale zip line above it must not refuse the patch. +#[test] +fn downgrade_to_the_patch_applies_and_checks_in_sync() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(downgrade-to)") { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let (consumer, modcache, proxy_url) = stage_with(tmp.path(), "1.16"); + publish_upstream(tmp.path(), "v1.1.0", "NEWER"); + let env = go_env(modcache.to_str().unwrap(), &proxy_url); + let readonly = readonly_env(&env); + + downgraded_consumer(&consumer, "v1.1.0", UVER, &env); + assert!(go_out(&consumer, &readonly).contains("OUT: PRISTINE")); + write_patch(&consumer); + let (code, so, se) = apply_json(&consumer, &modcache); + assert_eq!( + code, 0, + "apply must patch the selected version:\n{so}\n{se}" + ); + assert!(go_out(&consumer, &readonly).contains("OUT: PATCHED")); + let (code, so, se) = check(&consumer, &modcache); + assert_eq!(code, 0, "apply --check:\n{so}\n{se}"); + + chmod_writable(tmp.path()); +} diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 09901663b..61613b1eb 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -565,14 +565,23 @@ async fn read_go_workspace( } } // Module mode: whether a module-to-module replace is in effect - // can turn on modules only the dependencies require (#1840). + // can turn on modules only the dependencies require (#1840), + // and which version the build selects on a go.sum zip line a + // downgrade may have left behind (#1862). let mut module_graph = None; if loaded == Loaded::None { touched.push(root.join("go.mod")); + touched.push(root.join("go.sum")); let go_mod = crate::utils::fs::read_regular_to_string(&root.join("go.mod")).await; + let go_sum = crate::utils::fs::read_regular_to_string(&root.join("go.sum")) + .await + .ok(); if let Ok(go_mod) = go_mod { let go_mod = crate::vendor::go_mod_edit::normalize_for_read(&go_mod); - if crate::vendor::go_build_list::may_need_module_graph(&go_mod) { + if crate::vendor::go_build_list::may_need_module_graph( + &go_mod, + go_sum.as_deref(), + ) { touched.extend( crate::vendor::go_build_list::module_graph_reads(&go_mod) .into_iter() diff --git a/crates/socket-patch-core/src/patch/redirect/golang_local.rs b/crates/socket-patch-core/src/patch/redirect/golang_local.rs index e2afd5b57..c17d9f7be 100644 --- a/crates/socket-patch-core/src/patch/redirect/golang_local.rs +++ b/crates/socket-patch-core/src/patch/redirect/golang_local.rs @@ -88,6 +88,15 @@ pub enum Drift { purl: String, patched_version: String, }, + /// A go 1.16-or-older go.mod requires the module below a go.sum zip + /// line, and the module cache lacks a go.mod its graph needs. The line + /// is a dependency's raise or one `go get` left behind after a + /// downgrade, so nothing shows the build selects the patched version + /// (#1862). + UnprovenStaleSum { + purl: String, + patched_version: String, + }, /// go.mod builds the patched version only as the target of another /// module's `replace` (`replace => `). go /// doesn't apply a replace to a replacement, so the Socket `replace` is @@ -147,6 +156,15 @@ impl std::fmt::Display for Drift { replacement directory nor the module cache (`go mod download` fetches it), \ and go may link the UNPATCHED module" ), + Drift::UnprovenStaleSum { + purl, + patched_version, + } => write!( + f, + "{purl}: nothing shows the build selects the patched version \ + {patched_version}: {}; go may link the UNPATCHED module", + go_build_list::UNPROVEN_REASON + ), Drift::ReplaceTargetOnly { purl, patched_version, @@ -724,6 +742,10 @@ pub async fn verify_go_redirect_state( patched_version: version.to_string(), }) } + Some(Selection::Unproven) => drifts.push(Drift::UnprovenStaleSum { + purl: purl.clone(), + patched_version: version.to_string(), + }), Some(Selection::ReplaceTarget(via)) => drifts.push(Drift::ReplaceTargetOnly { purl: purl.clone(), patched_version: version.to_string(), @@ -833,6 +855,10 @@ pub(crate) async fn selection_refusal( replace to a replacement: its replace would be ignored and the build would \ link the unpatched module; nothing written" )), + Selection::Unproven => Some(format!( + "nothing shows the build selects {module} {version}: {}; nothing written", + go_build_list::UNPROVEN_REASON + )), Selection::Selected | Selection::Unknown => None, } } diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 0c778ac74..ee983649e 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -9829,6 +9829,8 @@ fn rewrite_golang_target( "the go.work workspace selects" } else if scan.required.as_deref() == Some(selected.as_str()) { "go.mod requires" + } else if graph.as_ref().is_some_and(|g| g.complete) { + "the module graph selects" } else { "go.sum shows the build selects" }; @@ -9894,6 +9896,7 @@ fn rewrite_golang_target( return false; } else if selection != go_build_list::Selection::Selected && (in_workspace && selection == go_build_list::Selection::Unknown + || selection == go_build_list::Selection::Unproven || prior.is_none_or(|e| e.version.as_deref() != Some(dep.version.as_str()))) { // Not selected by go.mod's require, no go.sum zip line at this @@ -9902,10 +9905,19 @@ fn rewrite_golang_target( // module cache). Its replace would be inert, and confirming it // would attest a patch no build links. A workspace graph that // could not be read proves nothing either way, so there even our - // own pin is no proof the build selects this version. + // own pin is no proof the build selects this version. Nor is it + // over a go.sum zip line above go.mod's require that a downgrade + // may have left behind (#1862). result.warnings.push(RewriteWarning { code: "redirect_golang_not_in_module_graph".into(), - detail: if in_workspace && selection == go_build_list::Selection::Unknown { + detail: if selection == go_build_list::Selection::Unproven { + format!( + "{fname}@{}: nothing shows the build selects this version — {}; \ + nothing redirected", + dep.version, + go_build_list::UNPROVEN_REASON + ) + } else if in_workspace && selection == go_build_list::Selection::Unknown { format!( "{fname}@{}: nothing shows the go.work workspace building this version \ — a go.mod its module graph needs is not in the module cache (`go mod \ diff --git a/crates/socket-patch-core/src/vendor/go_build_list.rs b/crates/socket-patch-core/src/vendor/go_build_list.rs index e3f9bcaf0..e4aa9a562 100644 --- a/crates/socket-patch-core/src/vendor/go_build_list.rs +++ b/crates/socket-patch-core/src/vendor/go_build_list.rs @@ -27,6 +27,13 @@ //! [`Selection::Unknown`]: a directory `replace` makes `go mod tidy` drop //! the original's go.sum lines, so absence proves nothing. //! +//! A zip line above go.mod's own `require` is either a dependency's +//! raise or a line `go get M@` left behind (go keeps the old +//! version's lines until `go mod tidy`), and the two files can't tell +//! which. Then the answer is MVS over the module graph read from the +//! module cache ([`module_graph`]), and [`Selection::Unproven`] when +//! that graph can't be read in full (#1862). +//! //! Either way, a zip line is no evidence when go.mod names that //! module@version as the target of a module-to-module `replace` //! (`replace other.com/fork v1.0.0 => example.com/upstream v1.0.0`): go @@ -73,6 +80,12 @@ pub enum Selection { /// go doesn't chain replaces, so a replace of the module itself is /// inert and the build links the unpatched code (#1823). ReplaceTarget(String), + /// An unpruned go.mod requires the module below a go.sum zip line, + /// at or under the patched version, and the module graph could not be + /// read in full. The line is a dependency's raise or one a downgrade + /// left behind, so nothing shows which version the build selects + /// (#1862). Fail closed: write, audit or attest nothing for it. + Unproven, } impl Selection { @@ -165,6 +178,20 @@ pub fn select_in( None => no_evidence(Selection::NotInGraph), }; } + if zip_above_require(required, zips) { + // MVS over every go.mod the build reads is go's own answer. + match graph.filter(|g| g.complete).map(|g| g.selected.get(module)) { + Some(Some(s)) if s == version => return Selection::Selected, + Some(Some(s)) => return Selection::Other(s.clone()), + Some(None) => return no_evidence(Selection::NotInGraph), + // MVS never selects below go.mod's own require, so a patched + // version under it is not built either way. + None if required.is_some_and(|r| go_semver_cmp(version, r).is_lt()) => { + return Selection::Other(required.unwrap_or_default().to_string()) + } + None => return Selection::Unproven, + } + } let highest = required .into_iter() .chain(zips.iter().copied()) @@ -197,23 +224,58 @@ fn replaces_targeting<'a>( }) } +/// Whether an unpruned graph's go.sum holds a zip line for the module +/// above go.mod's own require of it: a dependency's raise, or a line a +/// downgrade left behind, which go.mod and go.sum can't tell apart. +fn zip_above_require(required: Option<&str>, zips: &[&str]) -> bool { + required.is_some_and(|r| zips.iter().any(|z| go_semver_cmp(z, r).is_gt())) +} + +/// Why a [`Selection::Unproven`] answer proves nothing, for messages. +pub const UNPROVEN_REASON: &str = "go.sum has a zip line above go.mod's require (a \ + dependency's raise, or a line a downgrade left until `go mod tidy`), and a go.mod its \ + module graph needs is not in the module cache (`go mod download` fetches it)"; + /// Whether [`select`]'s answer for `module@version` can turn on the /// [`ModuleGraph`]: an unpruned go.mod with a module-to-module replace -/// targeting it. Only then is the graph worth reading. -pub fn needs_module_graph(go_mod: &str, module: &str, version: &str) -> bool { - !prunes_module_graph(go_mod) - && replaces_targeting(&parse_replace_entries(go_mod), module, version) - .next() - .is_some() +/// targeting it, or one whose `require` of it sits below a go.sum zip +/// line (#1862). Only then is the graph worth reading. +pub fn needs_module_graph(go_mod: &str, go_sum: Option<&str>, module: &str, version: &str) -> bool { + if prunes_module_graph(go_mod) { + return false; + } + let zips: Vec<&str> = go_sum + .map(|text| { + go_sum_lines(text) + .filter_map(|l| l.zip_version_of(module)) + .collect() + }) + .unwrap_or_default(); + let required = parse_required_versions(go_mod).remove(module); + replaces_targeting(&parse_replace_entries(go_mod), module, version) + .next() + .is_some() + || zip_above_require(required.as_deref(), &zips) } /// Whether any answer of [`select`] over `go_mod` can turn on the /// [`ModuleGraph`] ([`needs_module_graph`] for every module). -pub fn may_need_module_graph(go_mod: &str) -> bool { - !prunes_module_graph(go_mod) - && parse_replace_entries(go_mod) - .iter() - .any(|e| !e.socket_owned() && e.rhs_module.is_some() && e.rhs_version.is_some()) +pub fn may_need_module_graph(go_mod: &str, go_sum: Option<&str>) -> bool { + if prunes_module_graph(go_mod) { + return false; + } + let required = parse_required_versions(go_mod); + parse_replace_entries(go_mod) + .iter() + .any(|e| !e.socket_owned() && e.rhs_module.is_some() && e.rhs_version.is_some()) + || go_sum.is_some_and(|text| { + go_sum_lines(text).any(|l| { + !l.go_mod + && required + .get(l.module) + .is_some_and(|r| go_semver_cmp(l.version, r).is_gt()) + }) + }) } /// The module whose module-to-module `replace` in `go_mod` targets exactly @@ -298,10 +360,11 @@ pub fn module_graph_reads(go_mod: &str) -> Vec { pub async fn module_graph_for( project_root: &Path, go_mod: &str, + go_sum: Option<&str>, module: &str, version: &str, ) -> Option { - if !needs_module_graph(go_mod, module, version) { + if !needs_module_graph(go_mod, go_sum, module, version) { return None; } Some(module_graph(project_root, go_mod).await) @@ -339,7 +402,7 @@ pub async fn project_selection( let go_sum = read_regular_to_string(&project_root.join("go.sum")) .await .ok(); - let graph = module_graph_for(project_root, &go_mod, module, version).await; + let graph = module_graph_for(project_root, &go_mod, go_sum.as_deref(), module, version).await; Some(selection_in( &go_mod, go_sum.as_deref(), @@ -399,11 +462,10 @@ mod tests { #[test] fn unpruned_require_is_a_lower_bound() { let m = format!("module c\n\ngo 1.16\n\nrequire {UP} v1.0.0\n"); + // A zip line above it is a dependency's raise or one a downgrade + // left behind: only the module graph tells which (#1862). let s = sum(&[&format!("{UP} v1.0.1")]); - assert_eq!( - selection(&m, Some(&s), UP, "v1.0.0"), - Selection::Other("v1.0.1".into()) - ); + assert_eq!(selection(&m, Some(&s), UP, "v1.0.0"), Selection::Unproven); // A tidied directory replace drops the original's go.sum lines. assert_eq!(selection(&m, Some(""), UP, "v1.0.0"), Selection::Selected); // Only a lower bound below the patched version: can't tell. @@ -648,11 +710,92 @@ mod tests { ), ] { let m = format!("module c\n\n{go}\n\n{replace}"); - assert_eq!(needs_module_graph(&m, UP, "v1.0.0"), needed, "{m}"); - assert_eq!(may_need_module_graph(&m), needed, "{m}"); + assert_eq!(needs_module_graph(&m, None, UP, "v1.0.0"), needed, "{m}"); + assert_eq!(may_need_module_graph(&m, None), needed, "{m}"); } let m = format!("module c\n\ngo 1.16\n\n{fork}"); - assert!(!needs_module_graph(&m, UP, "v1.1.0")); + assert!(!needs_module_graph(&m, None, UP, "v1.1.0")); + } + + /// #1862: `go get M@` lowers go.mod's require but keeps the old + /// version's go.sum lines until `go mod tidy`. + fn downgraded(from: &str, to: &str) -> (String, String) { + let m = format!("module c\n\ngo 1.16\n\nrequire {UP} {to}\n"); + let s = sum(&[ + &format!("{UP} {to}"), + &format!("{UP} {to}/go.mod"), + &format!("{UP} {from}"), + &format!("{UP} {from}/go.mod"), + ]); + (m, s) + } + + /// #1862 case 1: downgraded below the patched version, the stale zip + /// line at the patched version is no proof it is built. + #[test] + fn stale_zip_line_above_require_does_not_prove_the_patched_version() { + let (m, s) = downgraded("v1.0.0", "v0.9.0"); + assert_eq!(selection(&m, Some(&s), UP, "v1.0.0"), Selection::Unproven); + // MVS over the module graph settles it: the build selects v0.9.0. + let g = graph(&[(UP, "v0.9.0")], true); + assert_eq!( + selection_in(&m, Some(&s), Some(&g), UP, "v1.0.0"), + Selection::Other("v0.9.0".into()) + ); + // An incomplete graph proves nothing. + let g = graph(&[(UP, "v0.9.0")], false); + assert_eq!( + selection_in(&m, Some(&s), Some(&g), UP, "v1.0.0"), + Selection::Unproven + ); + assert!(needs_module_graph(&m, Some(&s), UP, "v1.0.0")); + assert!(may_need_module_graph(&m, Some(&s))); + } + + /// #1862 case 2: downgraded to the patched version, the stale zip line + /// above it is no proof the build selects another. + #[test] + fn stale_zip_line_above_require_does_not_refuse_the_required_version() { + let (m, s) = downgraded("v1.1.0", "v1.0.0"); + assert_eq!(selection(&m, Some(&s), UP, "v1.0.0"), Selection::Unproven); + let g = graph(&[(UP, "v1.0.0")], true); + assert_eq!( + selection_in(&m, Some(&s), Some(&g), UP, "v1.0.0"), + Selection::Selected + ); + assert!(needs_module_graph(&m, Some(&s), UP, "v1.0.0")); + } + + /// A dependency's raise looks the same in go.mod and go.sum, and the + /// graph tells the two apart. + #[test] + fn module_graph_confirms_a_dependency_raise() { + let (m, s) = downgraded("v1.0.1", "v1.0.0"); + let g = graph(&[(UP, "v1.0.1")], true); + assert_eq!( + selection_in(&m, Some(&s), Some(&g), UP, "v1.0.1"), + Selection::Selected + ); + assert_eq!( + selection_in(&m, Some(&s), Some(&g), UP, "v1.0.0"), + Selection::Other("v1.0.1".into()) + ); + // A patched version below go.mod's require is never selected: the + // files say so without the graph, and the graph names the version. + assert_eq!( + selection(&m, Some(&s), UP, "v0.9.0"), + Selection::Other("v1.0.0".into()) + ); + let g = graph(&[(UP, "v1.0.0")], true); + assert_eq!( + selection_in(&m, Some(&s), Some(&g), UP, "v0.9.0"), + Selection::Other("v1.0.0".into()) + ); + // Zip lines at or below require, or a pruned graph, need no graph. + let tidy = sum(&[&format!("{UP} v1.0.0")]); + assert!(!may_need_module_graph(&m, Some(&tidy))); + let pruned = m.replace("go 1.16", "go 1.21"); + assert!(!may_need_module_graph(&pruned, Some(&s))); } /// Without the graph, go.mod's require of a lower version of the diff --git a/crates/socket-patch-core/src/vex/discover/golang.rs b/crates/socket-patch-core/src/vex/discover/golang.rs index a44a3dbc6..e1a617793 100644 --- a/crates/socket-patch-core/src/vex/discover/golang.rs +++ b/crates/socket-patch-core/src/vex/discover/golang.rs @@ -283,11 +283,26 @@ async fn extract_with(ctx: &DiscoverCtx<'_>, out: &mut Discovery, workspaces: bo .iter() .flat_map(|(_, entries)| entries) .any(|e| e.owner == Some(ReplaceOwner::Hosted)); + let mut sums = SumPins::default(); + let mut root_sum = None; + if any_hosted { + match &workspace { + Some(ws) => sums = SumPins::read(ctx, out, &ws.sum_files).await, + None => { + root_sum = ctx.read_text(GO_SUM, out).await; + if let Some(text) = &root_sum { + sums.add(text, true); + } + } + } + } // Module mode: whether a module-to-module replace is in effect can turn - // on modules only the dependencies require (#1840). + // on modules only the dependencies require (#1840), and which version + // the build selects on a go.sum zip line a downgrade may have left + // behind (#1862). let mut module_graph = None; if let (Some(text), None) = (&go_mod, &workspace) { - if go_build_list::may_need_module_graph(text) { + if go_build_list::may_need_module_graph(text, root_sum.as_deref()) { if let Some(root) = ctx.disk_root_reading(go_build_list::module_graph_reads(text)) { module_graph = Some(go_build_list::module_graph(root, text).await); } @@ -298,14 +313,6 @@ async fn extract_with(ctx: &DiscoverCtx<'_>, out: &mut Discovery, workspaces: bo workspace.as_ref().map(|ws| &ws.model), module_graph.as_ref(), ); - let mut sums = SumPins::default(); - if any_hosted { - let sum_files = match &workspace { - Some(ws) => ws.sum_files.clone(), - None => vec![GO_SUM.to_string()], - }; - sums = SumPins::read(ctx, out, &sum_files).await; - } for (file, entries) in &files { extract_file(out, file, entries, &graph, &sums); } @@ -375,6 +382,15 @@ impl<'a> Graph<'a> { } } + /// Whether a complete module-mode graph shows the build selects a + /// version of `module` other than `version`. + fn module_graph_selects_other(&self, module: &str, version: &str) -> bool { + self.module_graph + .filter(|g| g.complete) + .and_then(|g| g.selected.get(module)) + .is_some_and(|s| s != version) + } + /// Whether the workspace module graph proves the build selects exactly /// `module@version`. fn selects(&self, module: &str, version: &str) -> bool { @@ -533,8 +549,12 @@ fn extract_file( let pin = sums.pin(rhs, rhs_version); // Outside a workspace the socket module's zip line shows it // is the one built; in one, another member may raise the - // module above the replaced version all the same. - let proven = matches!(pin, SumPin::Zip(_)) && graph.workspace.is_none(); + // module above the replaced version all the same. A complete + // module graph outranks it: the line may be one a downgrade + // or a later raise left behind (#1862). + let proven = matches!(pin, SumPin::Zip(_)) + && graph.workspace.is_none() + && !graph.module_graph_selects_other(&entry.module, version); // A module go builds only as another module's replacement // is never redirected by its own replace, whatever the sum // files hold: go doesn't chain replaces (#1823). @@ -552,6 +572,22 @@ fn extract_file( ); continue; } + // A go.sum zip line above go.mod's require that a downgrade + // may have left behind, with no module graph to settle it: + // the socket module's own zip line may be as stale (#1862). + if selected == Selection::Unproven { + out.diag( + DIAG_REF_INVALID, + file, + format!( + "{file}: replace {} {version} is unproven: {}, so nothing \ + shows the build selects {version}", + entry.module, + go_build_list::UNPROVEN_REASON + ), + ); + continue; + } if let Some(other) = selected.other_version().filter(|_| !proven) { out.diag( DIAG_REF_INVALID, @@ -2157,4 +2193,59 @@ mod tests { None ); } + /// #1862: in a go 1.16 module the Socket module's zip line can outlive + /// the build's use of it (a later raise or downgrade). A complete + /// module graph outranks it, and without one a zip line above go.mod's + /// require proves nothing. + #[test] + fn module_graph_outranks_a_stale_socket_zip_line() { + use super::*; + let sver = "v1.4.2-socketpatch.1"; + let go_mod = format!( + "module example.com/app\n\ngo 1.16\n\nrequire {MODULE} {VERSION}\n\n\ + replace {MODULE} {VERSION} => {} {sver}\n", + gopatch(UUID_A) + ); + let mut sums = SumPins::default(); + sums.add( + &format!("{}{MODULE} v1.5.0 {ZIP_H1}\n", go_sum_for(UUID_A, sver)), + true, + ); + let entries = go_mod_edit::parse_replace_entries(&go_mod); + let cases = [ + (Some(true), "the build selects v1.5.0"), + (Some(false), "is unproven"), + (None, "is unproven"), + ]; + for (complete, why) in cases { + let mg = complete.map(|complete| ModuleGraph { + selected: [(MODULE.to_string(), "v1.5.0".to_string())] + .into_iter() + .collect(), + complete, + }); + let graph = Graph::new(Some(&go_mod), None, mg.as_ref()); + let mut out = Discovery::default(); + extract_file(&mut out, GO_MOD, &entries, &graph, &sums); + assert!(out.refs.is_empty(), "{complete:?}: {:?}", out.refs); + assert_eq!(out.diagnostics.len(), 1, "{:?}", out.diagnostics); + assert!( + out.diagnostics[0].detail.contains(why), + "{complete:?}: {}", + out.diagnostics[0].detail + ); + } + // The graph selecting the replaced version keeps the patch. + let mg = ModuleGraph { + selected: [(MODULE.to_string(), VERSION.to_string())] + .into_iter() + .collect(), + complete: true, + }; + let graph = Graph::new(Some(&go_mod), None, Some(&mg)); + let mut out = Discovery::default(); + extract_file(&mut out, GO_MOD, &entries, &graph, &sums); + assert_eq!(out.refs.len(), 1, "{:?}", out.diagnostics); + assert_eq!(out.refs[0].purl, PURL); + } }