Fix Go replace-target patches reported applied (#1823)

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

@@ -361,7 +361,8 @@ async fn run_check(args: &ApplyArgs, manifest_path: &Path) -> i32 {
                         | GoDrift::MissingReplace { purl }
                         | GoDrift::WrongReplacePath { purl, .. }
                         | GoDrift::ResolvedVersionMismatch { purl, .. }
-                        | GoDrift::UnprovenSelection { purl, .. } => purl.clone(),
+                        | GoDrift::UnprovenSelection { purl, .. }
+                        | GoDrift::ReplaceTargetOnly { purl, .. } => purl.clone(),
                         GoDrift::OrphanReplace { module } => module.clone(),
                     };
                     drifts.push((id, "go_redirect_drift".to_string(), d.to_string()));

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

@@ -1037,6 +1037,12 @@ async fn generate_vex(
                      ignored and the build links the unpatched module",
                     entry.module
                 ),
+                Some(Selection::ReplaceTarget(via)) => format!(
+                    "the build uses {} {version} only as the replacement of {via}, and go \
+                     doesn't apply a replace to a replacement, so the replace pinned to \
+                     {version} is ignored and the build links the unpatched module",
+                    entry.module
+                ),
                 Some(Selection::Unknown)
                     if socket_patch_core::vendor::go_workspace::resolve(&root)
                         != socket_patch_core::vendor::go_workspace::GoWork::None =>

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

@@ -1191,6 +1191,138 @@ fn go_work_member_requiring_a_newer_version_makes_the_replace_inert() {
     chmod_writable(tmp.path());
 }
 
+// ── a module built only as another module's replacement (#1823) ──
+
+const FORK: &str = "other.com/fork";
+
+/// Point the consumer at `FORK`, replaced by the upstream module
+/// (`replace_line`), and tidy it: go.sum then holds the upstream's lines
+/// under the upstream's own path, but the build list holds only `FORK`.
+fn fork_consumer(consumer: &Path, go_directive: &str, replace_line: &str, env: &[(&str, &str)]) {
+    std::fs::write(
+        consumer.join("go.mod"),
+        format!(
+            "module example.com/consumer\n\n{go_directive}\n\nrequire {FORK} {UVER}\n\n{replace_line}\n"
+        ),
+    )
+    .unwrap();
+    std::fs::write(
+        consumer.join("main.go"),
+        format!(
+            "package main\n\nimport (\n\t\"fmt\"\n\tu \"{FORK}\"\n)\n\nfunc main() {{ fmt.Println(\"OUT:\", u.Greeting()) }}\n"
+        ),
+    )
+    .unwrap();
+    let tidy = go(consumer, &["mod", "tidy"], env);
+    assert!(
+        tidy.status.success(),
+        "go mod tidy: {}",
+        String::from_utf8_lossy(&tidy.stderr)
+    );
+}
+
+/// #1823: `replace other.com/fork => example.com/upstream v1.0.0` makes go
+/// download the upstream and record its go.sum lines under its own path,
+/// but go doesn't chain replaces, so a replace of the upstream is inert.
+/// apply must refuse instead of reporting the patch applied, and a replace
+/// written before the fork replace appeared must fail `apply --check` and
+/// be left out of VEX. Covers the versioned and versionless fork replace in
+/// a pruned (go 1.21) and an unpruned (go 1.16) graph.
+#[test]
+fn module_built_only_as_a_replace_target_is_not_patched() {
+    if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(replace-target)") {
+        return;
+    }
+    for (go_directive, replace_line) in [
+        ("go 1.21", format!("replace {FORK} {UVER} => {UMOD} {UVER}")),
+        ("go 1.16", format!("replace {FORK} => {UMOD} {UVER}")),
+    ] {
+        let case = format!("{go_directive} / {replace_line}");
+        let tmp = tempfile::tempdir().unwrap();
+        let (consumer, modcache, proxy_url) = stage(tmp.path());
+        let env = go_env(modcache.to_str().unwrap(), &proxy_url);
+        let readonly: Vec<(&str, &str)> = env
+            .iter()
+            .map(|&(k, v)| {
+                if k == "GOFLAGS" {
+                    (k, "-mod=readonly")
+                } else {
+                    (k, v)
+                }
+            })
+            .collect();
+
+        // Applied while the consumer still required the upstream directly.
+        write_patch(&consumer);
+        let (code, so, se) = apply_json(&consumer, &modcache);
+        assert_eq!(code, 0, "{case}: direct apply:\n{so}\n{se}");
+        assert!(
+            go_out(&consumer, &readonly).contains("OUT: PATCHED"),
+            "{case}"
+        );
+
+        // Now the upstream is only the fork's replacement: the Socket
+        // replace (kept from the earlier apply) is inert.
+        let socket_replace = replace_lines(&consumer.join("go.mod"));
+        assert_eq!(socket_replace.len(), 1, "{case}: {socket_replace:?}");
+        fork_consumer(
+            &consumer,
+            go_directive,
+            &format!("{replace_line}\n\n{}", socket_replace[0]),
+            &env,
+        );
+        let sum = std::fs::read_to_string(consumer.join("go.sum")).unwrap();
+        assert!(sum.contains(&format!("{UMOD} {UVER} h1:")), "{case}: {sum}");
+        assert!(
+            go_out(&consumer, &readonly).contains("OUT: PRISTINE"),
+            "{case}"
+        );
+        let (code, so, se) = check(&consumer, &modcache);
+        assert_eq!(
+            code, 1,
+            "{case}: apply --check must flag the inert replace:\n{so}\n{se}"
+        );
+        assert!(
+            so.contains(&format!("only as the replacement of {FORK}"))
+                || se.contains(&format!("only as the replacement of {FORK}")),
+            "{case}:\n{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");
+
+        // A fresh apply over the fork shape refuses and writes nothing.
+        fork_consumer(&consumer, go_directive, &replace_line, &env);
+        let (code, so, se) = apply_json(&consumer, &modcache);
+        assert_ne!(
+            code, 0,
+            "{case}: apply must refuse an inert replace:\n{so}\n{se}"
+        );
+        assert!(
+            so.contains(&format!("only as the replacement of {FORK}")),
+            "{case}:\n{so}\n{se}"
+        );
+        assert_eq!(
+            replace_lines(&consumer.join("go.mod")),
+            vec![replace_line.clone()],
+            "{case}"
+        );
+        assert!(
+            go_out(&consumer, &readonly).contains("OUT: PRISTINE"),
+            "{case}"
+        );
+
+        chmod_writable(tmp.path());
+    }
+}
+
 // ── global mode: tools already built with `go install` (#422) ──
 
 const TOOL: &str = "example.com/tool";

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

@@ -88,6 +88,15 @@ pub enum Drift {
         purl: String,
         patched_version: String,
     },
+    /// go.mod builds the patched version only as the target of another
+    /// module's `replace` (`replace <via> => <module> <version>`). go
+    /// doesn't apply a replace to a replacement, so the Socket `replace` is
+    /// unused and the build links the UNPATCHED code (#1823).
+    ReplaceTargetOnly {
+        purl: String,
+        patched_version: String,
+        via: String,
+    },
 }
 
 impl std::fmt::Display for Drift {
@@ -138,6 +147,16 @@ 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::ReplaceTargetOnly {
+                purl,
+                patched_version,
+                via,
+            } => write!(
+                f,
+                "{purl}: the build uses {patched_version} only as the replacement of \
+                 {via}, and go doesn't apply a replace to a replacement — go would link \
+                 the UNPATCHED module"
+            ),
         }
     }
 }
@@ -584,6 +603,11 @@ pub async fn verify_go_redirect_state(
                     patched_version: version.to_string(),
                 })
             }
+            Some(Selection::ReplaceTarget(via)) => drifts.push(Drift::ReplaceTargetOnly {
+                purl: purl.clone(),
+                patched_version: version.to_string(),
+                via,
+            }),
             _ => {}
         }
 
@@ -682,6 +706,12 @@ pub(crate) async fn selection_refusal(
              build requires it and no go.sum has a zip line for it): its replace would be \
              ignored; nothing written"
         )),
+        Selection::ReplaceTarget(via) => Some(format!(
+            "the build uses {module} {version} only as the replacement of {via} \
+             (go.mod's replace of {via}), and go doesn't apply a \
+             replace to a replacement: its replace would be ignored and the build would \
+             link the unpatched module; nothing written"
+        )),
         Selection::Selected | Selection::Unknown => None,
     }
 }
@@ -2070,6 +2100,51 @@ mod tests {
         );
     }
 
+    /// #1823: a module the build uses only as another module's replacement
+    /// is refused by apply and flagged by `apply --check`.
+    #[tokio::test]
+    async fn test_replace_target_only_module_is_refused_and_flagged() {
+        let (dir, blobs, pristine, files, _after) = fixture().await;
+        let root = dir.path();
+        assert!(apply_fixture(root, &blobs, &pristine, &files).await.success);
+        let fork = format!(
+            "module example.com/app\n\ngo 1.21\n\nrequire other.com/fork {VERSION}\n\n\
+             replace other.com/fork => {MODULE} {VERSION}\n"
+        );
+        let socket =
+            format!("\nreplace {MODULE} {VERSION} => ./.socket/go-patches/{MODULE}@{VERSION}\n");
+        tokio::fs::write(root.join("go.mod"), format!("{fork}{socket}"))
+            .await
+            .unwrap();
+        tokio::fs::write(
+            root.join("go.sum"),
+            format!("{MODULE} {VERSION} h1:BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB=\n"),
+        )
+        .await
+        .unwrap();
+        let manifest = manifest_with(&files);
+        let desired: HashSet<String> = [PURL.to_string()].into_iter().collect();
+        let drifts = verify_go_redirect_state(root, &manifest, &desired)
+            .await
+            .unwrap_err();
+        assert_eq!(
+            drifts,
+            vec![Drift::ReplaceTargetOnly {
+                purl: PURL.into(),
+                patched_version: VERSION.into(),
+                via: "other.com/fork".into(),
+            }]
+        );
+        assert!(drifts[0].to_string().contains("UNPATCHED"));
+
+        tokio::fs::write(root.join("go.mod"), &fork).await.unwrap();
+        let refusal = selection_refusal(root, MODULE, VERSION).await.unwrap();
+        assert!(
+            refusal.contains("only as the replacement of other.com/fork"),
+            "{refusal}"
+        );
+    }
+
     #[tokio::test]
     async fn test_verify_orphan_replace() {
         let (dir, blobs, pristine, files, _after) = fixture().await;

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

@@ -9630,29 +9630,42 @@ fn rewrite_golang_target(
     let selection = selection.unwrap_or_else(|| {
         let zips = go_sum.zip_versions(fname);
         let zips: Vec<&str> = zips.iter().map(String::as_str).collect();
-        go_build_list::select(
-            go_mod_edit::prunes_module_graph(go_mod),
-            scan.required.as_deref(),
-            &zips,
-            &dep.version,
-        )
+        go_build_list::select(go_mod, fname, scan.required.as_deref(), &zips, &dep.version)
     });
-    if let Some(selected) = selection.other_version() {
-        let basis = if in_workspace {
-            "the go.work workspace selects"
-        } else if scan.required.as_deref() == Some(selected) {
-            "go.mod requires"
-        } else {
-            "go.sum shows the build selects"
-        };
-        result.warnings.push(RewriteWarning {
-            code: "redirect_golang_version_mismatch".into(),
+    // A pin the build provably doesn't use: a selected version other than
+    // the patched one, or a module go builds only as the target of another
+    // module's replace (go doesn't chain replaces, #1823).
+    let inert = match &selection {
+        go_build_list::Selection::Other(selected) => {
+            let basis = if in_workspace {
+                "the go.work workspace selects"
+            } else if scan.required.as_deref() == Some(selected.as_str()) {
+                "go.mod requires"
+            } else {
+                "go.sum shows the build selects"
+            };
+            Some(RewriteWarning {
+                code: "redirect_golang_version_mismatch".into(),
+                detail: format!(
+                    "{fname}: {basis} {selected} but the patch targets {} — \
+                     a version-pinned replace would be silently ignored",
+                    dep.version
+                ),
+            })
+        }
+        go_build_list::Selection::ReplaceTarget(via) => Some(RewriteWarning {
+            code: "redirect_golang_not_in_module_graph".into(),
             detail: format!(
-                "{fname}: {basis} {selected} but the patch targets {} — \
-                 a version-pinned replace would be silently ignored",
+                "{fname}@{}: the build uses this version only as the replacement of \
+                 {via} (go.mod's replace of {via}), and go doesn't apply a replace to a \
+                 replacement — its replace would be ignored; nothing redirected",
                 dep.version
             ),
-        });
+        }),
+        _ => None,
+    };
+    if let Some(warning) = inert {
+        result.warnings.push(warning);
         let stale_hosted = prior.filter(|e| e.owner == Some(go_mod_edit::ReplaceOwner::Hosted));
         if let Some(stale) = stale_hosted {
             if let Ok(Some(new)) =
@@ -23408,6 +23421,35 @@ packages:
         assert!(out.warnings[0].detail.contains("v1.5.0"));
     }
 
+    /// #1823: a module go builds only as the target of another module's
+    /// replace has its go.sum lines under its own path, but go doesn't chain
+    /// replaces, so a replace of it would be inert.
+    #[test]
+    fn golang_replace_target_only_module_skips() {
+        for go in ["go 1.21", "go 1.16"] {
+            let mut files = golang_files();
+            files.insert(
+                "go.mod".to_string(),
+                format!(
+                    "module example.com/app\n\n{go}\n\nrequire other.com/fork v1.4.2\n\n\
+                     replace other.com/fork => github.com/foo/bar v1.4.2\n"
+                ),
+            );
+            let ovr = golang_override();
+            let out = rewrite_registry_redirect(&files, std::slice::from_ref(&ovr));
+            assert!(out.files.is_empty(), "{go}: {:?}", out.files);
+            assert_eq!(out.warnings.len(), 1, "{go}: {:?}", out.warnings);
+            assert_eq!(out.warnings[0].code, "redirect_golang_not_in_module_graph");
+            assert!(
+                out.warnings[0]
+                    .detail
+                    .contains("only as the replacement of other.com/fork"),
+                "{}",
+                out.warnings[0].detail
+            );
+        }
+    }
+
     /// A module NOT in the main go.mod's require block may still be resolved
     /// (transitively) at the patched version — the cross-check only fires on a
     /// positive mismatch, never on absence.