Skip to content

Commit d3d3154

Browse files
Fix Go replace-target patches reported applied (#1823) (#1824)
* Start fix for #1823 Assisted-by: Claude Code:claude-opus-5-5 * Test Go patch of a module used only via replace Add an end-to-end regression for #1823: when go.mod replaces a fork with the patched module (`replace other.com/fork => upstream v1.0.0`), go records the upstream's go.sum lines but never applies a replace of the upstream itself. apply, apply --check and vex all reported the patch as in effect while go linked the unpatched code. Assisted-by: Claude Code:claude-opus-5-5 * Refuse Go patches for replace-only modules A module the build uses only as the target of another module's replace (`replace other.com/fork => example.com/upstream v1.0.0`) has its go.sum zip line under its own path, so the build-list check took it as built. go doesn't chain replaces, so a Socket replace of it is silently ignored: apply reported success, apply --check passed and vex attested not_affected while the build stayed unpatched. The shared build-list check now ignores a zip line that a module-to-module replace in go.mod explains (go refuses a build that uses one module@version under two paths, so it can't be the module's own) and reports the module as a replace target. apply and vendor refuse it, apply --check flags it, vex leaves it out, the hosted rewriter refuses it (redirect_golang_not_in_module_graph) and VEX discovery treats an existing hosted pin of it as inert. Fixes #1823 Assisted-by: Claude Code:claude-opus-5-5 * Only count a fork replace that is in effect go mod tidy keeps unused replace directives. A fork replace of a module go.mod doesn't require, one pinned to a version other than the required one, or a version-less one that a versioned replace of the same module shadows doesn't change what go builds, so it must not discount the patched module's own go.sum line. Also stop quoting a version-less directive in the refusal messages, which misquoted a versioned replace, and cover the agent-mode drift and refusal with a unit test. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 21911cf commit d3d3154

8 files changed

Lines changed: 498 additions & 35 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,8 @@ async fn run_check(args: &ApplyArgs, manifest_path: &Path) -> i32 {
361361
| GoDrift::MissingReplace { purl }
362362
| GoDrift::WrongReplacePath { purl, .. }
363363
| GoDrift::ResolvedVersionMismatch { purl, .. }
364-
| GoDrift::UnprovenSelection { purl, .. } => purl.clone(),
364+
| GoDrift::UnprovenSelection { purl, .. }
365+
| GoDrift::ReplaceTargetOnly { purl, .. } => purl.clone(),
365366
GoDrift::OrphanReplace { module } => module.clone(),
366367
};
367368
drifts.push((id, "go_redirect_drift".to_string(), d.to_string()));

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,6 +1037,12 @@ async fn generate_vex(
10371037
ignored and the build links the unpatched module",
10381038
entry.module
10391039
),
1040+
Some(Selection::ReplaceTarget(via)) => format!(
1041+
"the build uses {} {version} only as the replacement of {via}, and go \
1042+
doesn't apply a replace to a replacement, so the replace pinned to \
1043+
{version} is ignored and the build links the unpatched module",
1044+
entry.module
1045+
),
10401046
Some(Selection::Unknown)
10411047
if socket_patch_core::vendor::go_workspace::resolve(&root)
10421048
!= socket_patch_core::vendor::go_workspace::GoWork::None =>

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

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,6 +1191,138 @@ fn go_work_member_requiring_a_newer_version_makes_the_replace_inert() {
11911191
chmod_writable(tmp.path());
11921192
}
11931193

1194+
// ── a module built only as another module's replacement (#1823) ──
1195+
1196+
const FORK: &str = "other.com/fork";
1197+
1198+
/// Point the consumer at `FORK`, replaced by the upstream module
1199+
/// (`replace_line`), and tidy it: go.sum then holds the upstream's lines
1200+
/// under the upstream's own path, but the build list holds only `FORK`.
1201+
fn fork_consumer(consumer: &Path, go_directive: &str, replace_line: &str, env: &[(&str, &str)]) {
1202+
std::fs::write(
1203+
consumer.join("go.mod"),
1204+
format!(
1205+
"module example.com/consumer\n\n{go_directive}\n\nrequire {FORK} {UVER}\n\n{replace_line}\n"
1206+
),
1207+
)
1208+
.unwrap();
1209+
std::fs::write(
1210+
consumer.join("main.go"),
1211+
format!(
1212+
"package main\n\nimport (\n\t\"fmt\"\n\tu \"{FORK}\"\n)\n\nfunc main() {{ fmt.Println(\"OUT:\", u.Greeting()) }}\n"
1213+
),
1214+
)
1215+
.unwrap();
1216+
let tidy = go(consumer, &["mod", "tidy"], env);
1217+
assert!(
1218+
tidy.status.success(),
1219+
"go mod tidy: {}",
1220+
String::from_utf8_lossy(&tidy.stderr)
1221+
);
1222+
}
1223+
1224+
/// #1823: `replace other.com/fork => example.com/upstream v1.0.0` makes go
1225+
/// download the upstream and record its go.sum lines under its own path,
1226+
/// but go doesn't chain replaces, so a replace of the upstream is inert.
1227+
/// apply must refuse instead of reporting the patch applied, and a replace
1228+
/// written before the fork replace appeared must fail `apply --check` and
1229+
/// be left out of VEX. Covers the versioned and versionless fork replace in
1230+
/// a pruned (go 1.21) and an unpruned (go 1.16) graph.
1231+
#[test]
1232+
fn module_built_only_as_a_replace_target_is_not_patched() {
1233+
if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(replace-target)") {
1234+
return;
1235+
}
1236+
for (go_directive, replace_line) in [
1237+
("go 1.21", format!("replace {FORK} {UVER} => {UMOD} {UVER}")),
1238+
("go 1.16", format!("replace {FORK} => {UMOD} {UVER}")),
1239+
] {
1240+
let case = format!("{go_directive} / {replace_line}");
1241+
let tmp = tempfile::tempdir().unwrap();
1242+
let (consumer, modcache, proxy_url) = stage(tmp.path());
1243+
let env = go_env(modcache.to_str().unwrap(), &proxy_url);
1244+
let readonly: Vec<(&str, &str)> = env
1245+
.iter()
1246+
.map(|&(k, v)| {
1247+
if k == "GOFLAGS" {
1248+
(k, "-mod=readonly")
1249+
} else {
1250+
(k, v)
1251+
}
1252+
})
1253+
.collect();
1254+
1255+
// Applied while the consumer still required the upstream directly.
1256+
write_patch(&consumer);
1257+
let (code, so, se) = apply_json(&consumer, &modcache);
1258+
assert_eq!(code, 0, "{case}: direct apply:\n{so}\n{se}");
1259+
assert!(
1260+
go_out(&consumer, &readonly).contains("OUT: PATCHED"),
1261+
"{case}"
1262+
);
1263+
1264+
// Now the upstream is only the fork's replacement: the Socket
1265+
// replace (kept from the earlier apply) is inert.
1266+
let socket_replace = replace_lines(&consumer.join("go.mod"));
1267+
assert_eq!(socket_replace.len(), 1, "{case}: {socket_replace:?}");
1268+
fork_consumer(
1269+
&consumer,
1270+
go_directive,
1271+
&format!("{replace_line}\n\n{}", socket_replace[0]),
1272+
&env,
1273+
);
1274+
let sum = std::fs::read_to_string(consumer.join("go.sum")).unwrap();
1275+
assert!(sum.contains(&format!("{UMOD} {UVER} h1:")), "{case}: {sum}");
1276+
assert!(
1277+
go_out(&consumer, &readonly).contains("OUT: PRISTINE"),
1278+
"{case}"
1279+
);
1280+
let (code, so, se) = check(&consumer, &modcache);
1281+
assert_eq!(
1282+
code, 1,
1283+
"{case}: apply --check must flag the inert replace:\n{so}\n{se}"
1284+
);
1285+
assert!(
1286+
so.contains(&format!("only as the replacement of {FORK}"))
1287+
|| se.contains(&format!("only as the replacement of {FORK}")),
1288+
"{case}:\n{so}\n{se}"
1289+
);
1290+
let r = VexRun {
1291+
offline: true,
1292+
product: Some("pkg:golang/example.com/consumer".into()),
1293+
..VexRun::default()
1294+
}
1295+
.env("GOMODCACHE", &modcache)
1296+
.env("GOFLAGS", "");
1297+
let out = run_vex(&vex_e2e_common::binary(), &consumer, &r);
1298+
assert_absent(out.doc.as_ref(), UPURL);
1299+
vex_e2e_common::assert_not_attested(&out.envelope, UPURL, "go_version_not_selected");
1300+
1301+
// A fresh apply over the fork shape refuses and writes nothing.
1302+
fork_consumer(&consumer, go_directive, &replace_line, &env);
1303+
let (code, so, se) = apply_json(&consumer, &modcache);
1304+
assert_ne!(
1305+
code, 0,
1306+
"{case}: apply must refuse an inert replace:\n{so}\n{se}"
1307+
);
1308+
assert!(
1309+
so.contains(&format!("only as the replacement of {FORK}")),
1310+
"{case}:\n{so}\n{se}"
1311+
);
1312+
assert_eq!(
1313+
replace_lines(&consumer.join("go.mod")),
1314+
vec![replace_line.clone()],
1315+
"{case}"
1316+
);
1317+
assert!(
1318+
go_out(&consumer, &readonly).contains("OUT: PRISTINE"),
1319+
"{case}"
1320+
);
1321+
1322+
chmod_writable(tmp.path());
1323+
}
1324+
}
1325+
11941326
// ── global mode: tools already built with `go install` (#422) ──
11951327

11961328
const TOOL: &str = "example.com/tool";

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

Lines changed: 75 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+
/// go.mod builds the patched version only as the target of another
92+
/// module's `replace` (`replace <via> => <module> <version>`). go
93+
/// doesn't apply a replace to a replacement, so the Socket `replace` is
94+
/// unused and the build links the UNPATCHED code (#1823).
95+
ReplaceTargetOnly {
96+
purl: String,
97+
patched_version: String,
98+
via: String,
99+
},
91100
}
92101

93102
impl std::fmt::Display for Drift {
@@ -138,6 +147,16 @@ impl std::fmt::Display for Drift {
138147
replacement directory nor the module cache (`go mod download` fetches it), \
139148
and go may link the UNPATCHED module"
140149
),
150+
Drift::ReplaceTargetOnly {
151+
purl,
152+
patched_version,
153+
via,
154+
} => write!(
155+
f,
156+
"{purl}: the build uses {patched_version} only as the replacement of \
157+
{via}, and go doesn't apply a replace to a replacement — go would link \
158+
the UNPATCHED module"
159+
),
141160
}
142161
}
143162
}
@@ -584,6 +603,11 @@ pub async fn verify_go_redirect_state(
584603
patched_version: version.to_string(),
585604
})
586605
}
606+
Some(Selection::ReplaceTarget(via)) => drifts.push(Drift::ReplaceTargetOnly {
607+
purl: purl.clone(),
608+
patched_version: version.to_string(),
609+
via,
610+
}),
587611
_ => {}
588612
}
589613

@@ -682,6 +706,12 @@ pub(crate) async fn selection_refusal(
682706
build requires it and no go.sum has a zip line for it): its replace would be \
683707
ignored; nothing written"
684708
)),
709+
Selection::ReplaceTarget(via) => Some(format!(
710+
"the build uses {module} {version} only as the replacement of {via} \
711+
(go.mod's replace of {via}), and go doesn't apply a \
712+
replace to a replacement: its replace would be ignored and the build would \
713+
link the unpatched module; nothing written"
714+
)),
685715
Selection::Selected | Selection::Unknown => None,
686716
}
687717
}
@@ -2070,6 +2100,51 @@ mod tests {
20702100
);
20712101
}
20722102

2103+
/// #1823: a module the build uses only as another module's replacement
2104+
/// is refused by apply and flagged by `apply --check`.
2105+
#[tokio::test]
2106+
async fn test_replace_target_only_module_is_refused_and_flagged() {
2107+
let (dir, blobs, pristine, files, _after) = fixture().await;
2108+
let root = dir.path();
2109+
assert!(apply_fixture(root, &blobs, &pristine, &files).await.success);
2110+
let fork = format!(
2111+
"module example.com/app\n\ngo 1.21\n\nrequire other.com/fork {VERSION}\n\n\
2112+
replace other.com/fork => {MODULE} {VERSION}\n"
2113+
);
2114+
let socket =
2115+
format!("\nreplace {MODULE} {VERSION} => ./.socket/go-patches/{MODULE}@{VERSION}\n");
2116+
tokio::fs::write(root.join("go.mod"), format!("{fork}{socket}"))
2117+
.await
2118+
.unwrap();
2119+
tokio::fs::write(
2120+
root.join("go.sum"),
2121+
format!("{MODULE} {VERSION} h1:BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB=\n"),
2122+
)
2123+
.await
2124+
.unwrap();
2125+
let manifest = manifest_with(&files);
2126+
let desired: HashSet<String> = [PURL.to_string()].into_iter().collect();
2127+
let drifts = verify_go_redirect_state(root, &manifest, &desired)
2128+
.await
2129+
.unwrap_err();
2130+
assert_eq!(
2131+
drifts,
2132+
vec![Drift::ReplaceTargetOnly {
2133+
purl: PURL.into(),
2134+
patched_version: VERSION.into(),
2135+
via: "other.com/fork".into(),
2136+
}]
2137+
);
2138+
assert!(drifts[0].to_string().contains("UNPATCHED"));
2139+
2140+
tokio::fs::write(root.join("go.mod"), &fork).await.unwrap();
2141+
let refusal = selection_refusal(root, MODULE, VERSION).await.unwrap();
2142+
assert!(
2143+
refusal.contains("only as the replacement of other.com/fork"),
2144+
"{refusal}"
2145+
);
2146+
}
2147+
20732148
#[tokio::test]
20742149
async fn test_verify_orphan_replace() {
20752150
let (dir, blobs, pristine, files, _after) = fixture().await;

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

Lines changed: 61 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9686,29 +9686,42 @@ fn rewrite_golang_target(
96869686
let selection = selection.unwrap_or_else(|| {
96879687
let zips = go_sum.zip_versions(fname);
96889688
let zips: Vec<&str> = zips.iter().map(String::as_str).collect();
9689-
go_build_list::select(
9690-
go_mod_edit::prunes_module_graph(go_mod),
9691-
scan.required.as_deref(),
9692-
&zips,
9693-
&dep.version,
9694-
)
9689+
go_build_list::select(go_mod, fname, scan.required.as_deref(), &zips, &dep.version)
96959690
});
9696-
if let Some(selected) = selection.other_version() {
9697-
let basis = if in_workspace {
9698-
"the go.work workspace selects"
9699-
} else if scan.required.as_deref() == Some(selected) {
9700-
"go.mod requires"
9701-
} else {
9702-
"go.sum shows the build selects"
9703-
};
9704-
result.warnings.push(RewriteWarning {
9705-
code: "redirect_golang_version_mismatch".into(),
9691+
// A pin the build provably doesn't use: a selected version other than
9692+
// the patched one, or a module go builds only as the target of another
9693+
// module's replace (go doesn't chain replaces, #1823).
9694+
let inert = match &selection {
9695+
go_build_list::Selection::Other(selected) => {
9696+
let basis = if in_workspace {
9697+
"the go.work workspace selects"
9698+
} else if scan.required.as_deref() == Some(selected.as_str()) {
9699+
"go.mod requires"
9700+
} else {
9701+
"go.sum shows the build selects"
9702+
};
9703+
Some(RewriteWarning {
9704+
code: "redirect_golang_version_mismatch".into(),
9705+
detail: format!(
9706+
"{fname}: {basis} {selected} but the patch targets {} — \
9707+
a version-pinned replace would be silently ignored",
9708+
dep.version
9709+
),
9710+
})
9711+
}
9712+
go_build_list::Selection::ReplaceTarget(via) => Some(RewriteWarning {
9713+
code: "redirect_golang_not_in_module_graph".into(),
97069714
detail: format!(
9707-
"{fname}: {basis} {selected} but the patch targets {} — \
9708-
a version-pinned replace would be silently ignored",
9715+
"{fname}@{}: the build uses this version only as the replacement of \
9716+
{via} (go.mod's replace of {via}), and go doesn't apply a replace to a \
9717+
replacement — its replace would be ignored; nothing redirected",
97099718
dep.version
97109719
),
9711-
});
9720+
}),
9721+
_ => None,
9722+
};
9723+
if let Some(warning) = inert {
9724+
result.warnings.push(warning);
97129725
let stale_hosted = prior.filter(|e| e.owner == Some(go_mod_edit::ReplaceOwner::Hosted));
97139726
if let Some(stale) = stale_hosted {
97149727
if let Ok(Some(new)) =
@@ -23657,6 +23670,35 @@ packages:
2365723670
assert!(out.warnings[0].detail.contains("v1.5.0"));
2365823671
}
2365923672

23673+
/// #1823: a module go builds only as the target of another module's
23674+
/// replace has its go.sum lines under its own path, but go doesn't chain
23675+
/// replaces, so a replace of it would be inert.
23676+
#[test]
23677+
fn golang_replace_target_only_module_skips() {
23678+
for go in ["go 1.21", "go 1.16"] {
23679+
let mut files = golang_files();
23680+
files.insert(
23681+
"go.mod".to_string(),
23682+
format!(
23683+
"module example.com/app\n\n{go}\n\nrequire other.com/fork v1.4.2\n\n\
23684+
replace other.com/fork => github.com/foo/bar v1.4.2\n"
23685+
),
23686+
);
23687+
let ovr = golang_override();
23688+
let out = rewrite_registry_redirect(&files, std::slice::from_ref(&ovr));
23689+
assert!(out.files.is_empty(), "{go}: {:?}", out.files);
23690+
assert_eq!(out.warnings.len(), 1, "{go}: {:?}", out.warnings);
23691+
assert_eq!(out.warnings[0].code, "redirect_golang_not_in_module_graph");
23692+
assert!(
23693+
out.warnings[0]
23694+
.detail
23695+
.contains("only as the replacement of other.com/fork"),
23696+
"{}",
23697+
out.warnings[0].detail
23698+
);
23699+
}
23700+
}
23701+
2366023702
/// A module NOT in the main go.mod's require block may still be resolved
2366123703
/// (transitively) at the patched version — the cross-check only fires on a
2366223704
/// positive mismatch, never on absence.

0 commit comments

Comments
 (0)