Skip to content

Commit e0916be

Browse files
Fix Go stale go.sum line after a downgrade (#1862) (#1893)
* Start fix for #1840 Assisted-by: Claude Code:claude-opus-5-5 * Refuse Go patches built only via a dep's fork In a go 1.16 module, when a dependency imports a fork that the root go.mod replaces with the patched upstream module, apply reported the patch applied, apply --check said in sync and vex attested not_affected, while go build linked the unpatched code. The root go.mod of an unpruned graph does not list modules that only a dependency requires, but its replace still applies to them. The replace-target check only trusted a fork listed in the root require, so it missed this shape. Now, in an unpruned graph, such a replace counts as in effect unless go.mod requires the patched module itself. Fixes #1840 Assisted-by: Claude Code:claude-opus-5-5 * Read the go 1.16 module graph for fork replaces The first fix guessed that a fork replace in a go 1.16 module is in effect whenever go.mod doesn't list the fork. That refused working patches when go mod tidy kept a replace nothing uses, and the hosted rewriter then removed a working Socket replace. It also missed a fork a dependency raises to the replaced version. Now, when an unpruned go.mod has a module-to-module replace, the module graph is read from the module cache, the same walk go.work projects use, and the fork's selected version decides. apply, apply --check, the hosted rewriter and VEX discovery all use it. When the cache lacks go.mod files, the safe answer stays: the patch is not reported built. Refs #1840 Assisted-by: Claude Code:claude-opus-5-5 * Declare the go.mod files the graph walk reads Reading the go 1.16 module graph went around the project view, which ended read recording for every project: VEX discovery asked for the raw disk root even with no Go files, so discovery could no longer be reused between passes. Now the graph is read only when go.mod needs it, and the root go.mod and any directory replacement's go.mod are declared as reads, so a change to them still invalidates the cache. Refs #1840 Assisted-by: Claude Code:claude-opus-5-5 * Keep a hosted Go pin on an unproven fork replace Without every go.mod of a go 1.16 module graph in the module cache, a fork replace is only suspected of bypassing the Socket pin. That guess now refuses a new pin but no longer removes an existing one that may be working. A partial graph is no longer trusted as proof that the fork is at a lower version. Socket's own hosted replaces no longer trigger a graph walk. Refs #1840 Assisted-by: Claude Code:claude-opus-5-5 * Start fix for #1862 Assisted-by: Claude Code:claude-opus-5-5 * Don't trust a go.sum line a downgrade left In a go 1.16-or-older module, `go get M@<lower>` lowers go.mod's require but keeps the old version's go.sum lines until `go mod tidy`. socket-patch took the highest go.sum zip line as the version the build selects, so after a downgrade apply wrote an unused replace that apply --check and vex then reported as patched, or refused a patch for the version the build really uses (#1862). When go.mod requires a module below one of its go.sum zip lines, read the module graph from the module cache and use the version MVS picks. When the cache can't answer, fail closed: apply refuses with a remedy, apply --check reports drift, and vex and the hosted rewriter don't attest or pin the patch. Assisted-by: Claude Code:claude-opus-5-5 * Test Go downgrades that leave go.sum lines End-to-end with a real go toolchain, both shapes from #1862: a downgrade below the patched version (apply refuses, an earlier replace fails apply --check with and without the module cache, and vex leaves it out), and a downgrade to the patched version (apply patches it and the build links the patch). Assisted-by: Claude Code:claude-opus-5-5 * Let a full Go module graph outrank go.sum Follow-ups from review of the #1862 fix: - VEX discovery let a Socket module's own go.sum zip line vouch for a hosted replace even when a full module graph showed the build picks another version. That line can be just as stale, so the graph now wins. - A patch below go.mod's require reported a leftover go.sum version as the one the build selects; it now reports go.mod's require, or the graph's answer when the graph is read. - The hosted warning says "the module graph selects" when that is where the answer came from. - The "unproven" explanation is written once and shared by apply, apply --check, vex and the hosted rewriter. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Mikola Lysenko <noreply@anthropic.com>
1 parent 5f84319 commit e0916be

8 files changed

Lines changed: 504 additions & 44 deletions

File tree

‎crates/socket-patch-cli/src/commands/apply.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,7 @@ async fn run_check(args: &ApplyArgs, manifest_path: &Path) -> i32 {
403403
| GoDrift::WrongReplacePath { purl, .. }
404404
| GoDrift::ResolvedVersionMismatch { purl, .. }
405405
| GoDrift::UnprovenSelection { purl, .. }
406+
| GoDrift::UnprovenStaleSum { purl, .. }
406407
| GoDrift::ReplaceTargetOnly { purl, .. } => purl.clone(),
407408
GoDrift::OrphanReplace { module } => module.clone(),
408409
};

‎crates/socket-patch-cli/src/commands/vex.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,6 +1044,11 @@ async fn generate_vex(
10441044
{version} is ignored and the build links the unpatched module",
10451045
entry.module
10461046
),
1047+
Some(Selection::Unproven) => format!(
1048+
"nothing shows the build selects {} {version}: {}",
1049+
entry.module,
1050+
socket_patch_core::vendor::go_build_list::UNPROVEN_REASON
1051+
),
10471052
Some(Selection::Unknown)
10481053
if socket_patch_core::vendor::go_workspace::resolve(&root)
10491054
!= socket_patch_core::vendor::go_workspace::GoWork::None =>

‎crates/socket-patch-cli/tests/e2e_golang_build.rs‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2215,3 +2215,170 @@ fn goflags_modfile_keeps_the_go_mod_replace_from_the_build() {
22152215

22162216
chmod_writable(tmp.path());
22172217
}
2218+
2219+
// ── a go.sum line a downgrade left behind (#1862) ──
2220+
2221+
/// A go 1.16 consumer of the upstream at `from`, downgraded to `to` by
2222+
/// `go get`, which lowers the require but keeps `from`'s go.sum lines
2223+
/// until `go mod tidy`.
2224+
fn downgraded_consumer(consumer: &Path, from: &str, to: &str, env: &[(&str, &str)]) {
2225+
std::fs::write(
2226+
consumer.join("go.mod"),
2227+
format!("module example.com/consumer\n\ngo 1.16\n\nrequire {UMOD} {from}\n"),
2228+
)
2229+
.unwrap();
2230+
for args in [vec!["mod", "tidy"], vec!["get", &format!("{UMOD}@{to}")]] {
2231+
let out = go(consumer, &args, env);
2232+
assert!(
2233+
out.status.success(),
2234+
"go {args:?}: {}",
2235+
String::from_utf8_lossy(&out.stderr)
2236+
);
2237+
}
2238+
let go_mod = std::fs::read_to_string(consumer.join("go.mod")).unwrap();
2239+
assert!(go_mod.contains(&format!("{UMOD} {to}")), "{go_mod}");
2240+
let sum = std::fs::read_to_string(consumer.join("go.sum")).unwrap();
2241+
assert!(
2242+
sum.contains(&format!("{UMOD} {from} h1:")),
2243+
"go get must keep the stale zip line:\n{sum}"
2244+
);
2245+
}
2246+
2247+
/// Publish the upstream at `version` with a greeting of `greeting`.
2248+
fn publish_upstream(tmp: &Path, version: &str, greeting: &str) {
2249+
publish(
2250+
tmp,
2251+
UMOD,
2252+
version,
2253+
&[
2254+
("go.mod", &format!("module {UMOD}\n\ngo 1.16\n")),
2255+
(
2256+
"lib.go",
2257+
&format!(
2258+
"package upstream\n\nfunc Greeting() string {{ return \"{greeting}\" }}\n"
2259+
),
2260+
),
2261+
],
2262+
);
2263+
}
2264+
2265+
/// #1862 case 1: downgraded below the patched version, the build links
2266+
/// the older module. The stale zip line at the patched version must not
2267+
/// pass for proof it is built: apply refuses, and a replace written
2268+
/// before the downgrade fails `apply --check` (with or without the module
2269+
/// cache to read the graph from) and is left out of VEX.
2270+
#[test]
2271+
fn downgrade_below_the_patch_leaves_its_replace_unattested() {
2272+
if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(downgrade-below)") {
2273+
return;
2274+
}
2275+
let tmp = tempfile::tempdir().unwrap();
2276+
let (consumer, modcache, proxy_url) = stage_with(tmp.path(), "1.16");
2277+
publish_upstream(tmp.path(), "v0.9.0", "OLDER");
2278+
let env = go_env(modcache.to_str().unwrap(), &proxy_url);
2279+
let readonly = readonly_env(&env);
2280+
let empty_cache = tmp.path().join("empty-modcache");
2281+
std::fs::create_dir_all(&empty_cache).unwrap();
2282+
2283+
// A fresh apply after the downgrade refuses and writes nothing.
2284+
downgraded_consumer(&consumer, UVER, "v0.9.0", &env);
2285+
write_patch(&consumer);
2286+
let (code, so, se) = apply_json(&consumer, &modcache);
2287+
assert_ne!(code, 0, "apply must refuse the inert replace:\n{so}\n{se}");
2288+
assert!(
2289+
so.contains(&format!("the build selects {UMOD} v0.9.0")),
2290+
"{so}\n{se}"
2291+
);
2292+
assert!(replace_lines(&consumer.join("go.mod")).is_empty());
2293+
// Applied before the downgrade: the replace is inert afterwards.
2294+
std::fs::write(
2295+
consumer.join("go.mod"),
2296+
format!("module example.com/consumer\n\ngo 1.16\n\nrequire {UMOD} {UVER}\n"),
2297+
)
2298+
.unwrap();
2299+
let tidy = go(&consumer, &["mod", "tidy"], &env);
2300+
assert!(tidy.status.success());
2301+
let (code, so, se) = apply_json(&consumer, &modcache);
2302+
assert_eq!(code, 0, "apply at the required version:\n{so}\n{se}");
2303+
assert!(go_out(&consumer, &readonly).contains("OUT: PATCHED"));
2304+
let socket_replace = replace_lines(&consumer.join("go.mod"));
2305+
assert_eq!(socket_replace.len(), 1, "{socket_replace:?}");
2306+
let get = go(&consumer, &["get", &format!("{UMOD}@v0.9.0")], &env);
2307+
assert!(
2308+
get.status.success(),
2309+
"{}",
2310+
String::from_utf8_lossy(&get.stderr)
2311+
);
2312+
// The directory replace dropped the original's go.sum lines; an
2313+
// untidied go.sum from before the apply still carries them.
2314+
let mut sum = std::fs::read_to_string(consumer.join("go.sum")).unwrap();
2315+
if !sum.contains(&format!("{UMOD} {UVER} h1:")) {
2316+
let pristine = tmp.path().join("pristine");
2317+
consumer_module(
2318+
&pristine,
2319+
"example.com/pristine",
2320+
modcache.to_str().unwrap(),
2321+
&proxy_url,
2322+
);
2323+
sum.push_str(&std::fs::read_to_string(pristine.join("go.sum")).unwrap());
2324+
std::fs::write(consumer.join("go.sum"), sum).unwrap();
2325+
}
2326+
assert_eq!(replace_lines(&consumer.join("go.mod")), socket_replace);
2327+
assert!(go_out(&consumer, &readonly).contains("OUT: OLDER"));
2328+
2329+
let (code, so, se) = check(&consumer, &modcache);
2330+
assert_eq!(
2331+
code, 1,
2332+
"apply --check must flag the inert replace:\n{so}\n{se}"
2333+
);
2334+
assert!(
2335+
format!("{so}{se}").contains("is not the version the build selects (v0.9.0)"),
2336+
"{so}\n{se}"
2337+
);
2338+
let (code, so, se) = check(&consumer, &empty_cache);
2339+
assert_eq!(code, 1, "without the cache too:\n{so}\n{se}");
2340+
assert!(
2341+
format!("{so}{se}").contains("nothing shows the build selects the patched version"),
2342+
"{so}\n{se}"
2343+
);
2344+
let r = VexRun {
2345+
offline: true,
2346+
product: Some("pkg:golang/example.com/consumer".into()),
2347+
..VexRun::default()
2348+
}
2349+
.env("GOMODCACHE", &modcache)
2350+
.env("GOFLAGS", "");
2351+
let out = run_vex(&vex_e2e_common::binary(), &consumer, &r);
2352+
assert_absent(out.doc.as_ref(), UPURL);
2353+
vex_e2e_common::assert_not_attested(&out.envelope, UPURL, "go_version_not_selected");
2354+
2355+
chmod_writable(tmp.path());
2356+
}
2357+
2358+
/// #1862 case 2: downgraded to the patched version, the build links it.
2359+
/// The stale zip line above it must not refuse the patch.
2360+
#[test]
2361+
fn downgrade_to_the_patch_applies_and_checks_in_sync() {
2362+
if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(downgrade-to)") {
2363+
return;
2364+
}
2365+
let tmp = tempfile::tempdir().unwrap();
2366+
let (consumer, modcache, proxy_url) = stage_with(tmp.path(), "1.16");
2367+
publish_upstream(tmp.path(), "v1.1.0", "NEWER");
2368+
let env = go_env(modcache.to_str().unwrap(), &proxy_url);
2369+
let readonly = readonly_env(&env);
2370+
2371+
downgraded_consumer(&consumer, "v1.1.0", UVER, &env);
2372+
assert!(go_out(&consumer, &readonly).contains("OUT: PRISTINE"));
2373+
write_patch(&consumer);
2374+
let (code, so, se) = apply_json(&consumer, &modcache);
2375+
assert_eq!(
2376+
code, 0,
2377+
"apply must patch the selected version:\n{so}\n{se}"
2378+
);
2379+
assert!(go_out(&consumer, &readonly).contains("OUT: PATCHED"));
2380+
let (code, so, se) = check(&consumer, &modcache);
2381+
assert_eq!(code, 0, "apply --check:\n{so}\n{se}");
2382+
2383+
chmod_writable(tmp.path());
2384+
}

‎crates/socket-patch-core/src/hosted/engine.rs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -575,14 +575,23 @@ async fn read_go_workspace(
575575
}
576576
}
577577
// Module mode: whether a module-to-module replace is in effect
578-
// can turn on modules only the dependencies require (#1840).
578+
// can turn on modules only the dependencies require (#1840),
579+
// and which version the build selects on a go.sum zip line a
580+
// downgrade may have left behind (#1862).
579581
let mut module_graph = None;
580582
if loaded == Loaded::None {
581583
touched.push(root.join("go.mod"));
584+
touched.push(root.join("go.sum"));
582585
let go_mod = crate::utils::fs::read_regular_to_string(&root.join("go.mod")).await;
586+
let go_sum = crate::utils::fs::read_regular_to_string(&root.join("go.sum"))
587+
.await
588+
.ok();
583589
if let Ok(go_mod) = go_mod {
584590
let go_mod = crate::vendor::go_mod_edit::normalize_for_read(&go_mod);
585-
if crate::vendor::go_build_list::may_need_module_graph(&go_mod) {
591+
if crate::vendor::go_build_list::may_need_module_graph(
592+
&go_mod,
593+
go_sum.as_deref(),
594+
) {
586595
touched.extend(
587596
crate::vendor::go_build_list::module_graph_reads(&go_mod)
588597
.into_iter()

‎crates/socket-patch-core/src/patch/redirect/golang_local.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,15 @@ pub enum Drift {
8888
purl: String,
8989
patched_version: String,
9090
},
91+
/// A go 1.16-or-older go.mod requires the module below a go.sum zip
92+
/// line, and the module cache lacks a go.mod its graph needs. The line
93+
/// is a dependency's raise or one `go get` left behind after a
94+
/// downgrade, so nothing shows the build selects the patched version
95+
/// (#1862).
96+
UnprovenStaleSum {
97+
purl: String,
98+
patched_version: String,
99+
},
91100
/// go.mod builds the patched version only as the target of another
92101
/// module's `replace` (`replace <via> => <module> <version>`). go
93102
/// doesn't apply a replace to a replacement, so the Socket `replace` is
@@ -147,6 +156,15 @@ impl std::fmt::Display for Drift {
147156
replacement directory nor the module cache (`go mod download` fetches it), \
148157
and go may link the UNPATCHED module"
149158
),
159+
Drift::UnprovenStaleSum {
160+
purl,
161+
patched_version,
162+
} => write!(
163+
f,
164+
"{purl}: nothing shows the build selects the patched version \
165+
{patched_version}: {}; go may link the UNPATCHED module",
166+
go_build_list::UNPROVEN_REASON
167+
),
150168
Drift::ReplaceTargetOnly {
151169
purl,
152170
patched_version,
@@ -742,6 +760,10 @@ pub async fn verify_go_redirect_state(
742760
patched_version: version.to_string(),
743761
})
744762
}
763+
Some(Selection::Unproven) => drifts.push(Drift::UnprovenStaleSum {
764+
purl: purl.clone(),
765+
patched_version: version.to_string(),
766+
}),
745767
Some(Selection::ReplaceTarget(via)) => drifts.push(Drift::ReplaceTargetOnly {
746768
purl: purl.clone(),
747769
patched_version: version.to_string(),
@@ -851,6 +873,10 @@ pub(crate) async fn selection_refusal(
851873
replace to a replacement: its replace would be ignored and the build would \
852874
link the unpatched module; nothing written"
853875
)),
876+
Selection::Unproven => Some(format!(
877+
"nothing shows the build selects {module} {version}: {}; nothing written",
878+
go_build_list::UNPROVEN_REASON
879+
)),
854880
Selection::Selected | Selection::Unknown => None,
855881
}
856882
}

‎crates/socket-patch-core/src/patch/redirect/mod.rs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9795,6 +9795,8 @@ fn rewrite_golang_target(
97959795
"the go.work workspace selects"
97969796
} else if scan.required.as_deref() == Some(selected.as_str()) {
97979797
"go.mod requires"
9798+
} else if graph.as_ref().is_some_and(|g| g.complete) {
9799+
"the module graph selects"
97989800
} else {
97999801
"go.sum shows the build selects"
98009802
};
@@ -9860,6 +9862,7 @@ fn rewrite_golang_target(
98609862
return false;
98619863
} else if selection != go_build_list::Selection::Selected
98629864
&& (in_workspace && selection == go_build_list::Selection::Unknown
9865+
|| selection == go_build_list::Selection::Unproven
98639866
|| prior.is_none_or(|e| e.version.as_deref() != Some(dep.version.as_str())))
98649867
{
98659868
// Not selected by go.mod's require, no go.sum zip line at this
@@ -9868,10 +9871,19 @@ fn rewrite_golang_target(
98689871
// module cache). Its replace would be inert, and confirming it
98699872
// would attest a patch no build links. A workspace graph that
98709873
// could not be read proves nothing either way, so there even our
9871-
// own pin is no proof the build selects this version.
9874+
// own pin is no proof the build selects this version. Nor is it
9875+
// over a go.sum zip line above go.mod's require that a downgrade
9876+
// may have left behind (#1862).
98729877
result.warnings.push(RewriteWarning {
98739878
code: "redirect_golang_not_in_module_graph".into(),
9874-
detail: if in_workspace && selection == go_build_list::Selection::Unknown {
9879+
detail: if selection == go_build_list::Selection::Unproven {
9880+
format!(
9881+
"{fname}@{}: nothing shows the build selects this version — {}; \
9882+
nothing redirected",
9883+
dep.version,
9884+
go_build_list::UNPROVEN_REASON
9885+
)
9886+
} else if in_workspace && selection == go_build_list::Selection::Unknown {
98759887
format!(
98769888
"{fname}@{}: nothing shows the go.work workspace building this version \
98779889
— a go.mod its module graph needs is not in the module cache (`go mod \

0 commit comments

Comments
 (0)