Fix Go module cache default read two ways (#1834)

crates/socket-patch-core/src/crawlers/go_crawler.rs

@@ -178,33 +178,16 @@ impl GoCrawler {
     // Private helpers
     // ------------------------------------------------------------------
 
-    /// Get `GOMODCACHE`, falling back to `$GOPATH/pkg/mod/` or `$HOME/go/pkg/mod/`.
-    /// Each setting resolves as go resolves it: the environment, then the
-    /// `go env -w` file ([`crate::utils::go_env`]), so a cache configured
-    /// with `go env -w GOMODCACHE=…` is the one crawled (#344).
+    /// The module cache go itself uses ([`crate::utils::go_env::gomodcache`]):
+    /// `GOMODCACHE`, else the first `GOPATH` entry's `pkg/mod`, else
+    /// `$HOME/go/pkg/mod`. Each setting resolves as go resolves it: the
+    /// environment, then the `go env -w` file ([`crate::utils::go_env`]), so
+    /// a cache configured with `go env -w GOMODCACHE=…` is the one crawled
+    /// (#344). `None` when go has no module cache.
     pub(crate) fn get_gomodcache() -> Option<PathBuf> {
-        if let Some(cache) = crate::utils::go_env::go_env("GOMODCACHE") {
-            return Some(PathBuf::from(cache));
-        }
-        if let Some(gopath) = crate::utils::go_env::go_env("GOPATH") {
-            // GOPATH may list several directories separated by the OS path
-            // separator (`:` on Unix, `;` on Windows). Go uses the FIRST
-            // entry for the module cache, so split rather than treating the
-            // whole value as a single path.
-            if let Some(first) = std::env::split_paths(&gopath).find(|p| !p.as_os_str().is_empty())
-            {
-                return Some(first.join("pkg").join("mod"));
-            }
-        }
-        // A set-but-empty HOME/USERPROFILE counts as unset, matching the
-        // GOMODCACHE and GOPATH guards above: honoring `""` would yield the
-        // RELATIVE path `go/pkg/mod`, pointing the crawl at a directory
-        // inside the user's project instead of a real module cache.
-        Some(
-            crate::utils::fs::home_dir()?
-                .join("go")
-                .join("pkg")
-                .join("mod"),
+        crate::utils::go_env::gomodcache(
+            &crate::utils::go_env::go_env,
+            crate::utils::fs::home_dir().as_deref(),
         )
     }
 
@@ -1555,21 +1538,32 @@ mod tests {
         // No `go env -w` file: the developer's own must not leak in.
         let _goenv = EnvGuard::set("GOENV", "off");
 
-        // Case A: empty GOMODCACHE falls through to GOPATH, and the empty
-        // FIRST GOPATH entry is skipped in favor of the next non-empty one
-        // (GOPATH is an OS-separator-delimited list; Go uses the first
-        // usable entry for the module cache).
-        let gopath_list = std::env::join_paths(["".as_ref(), gopath_a.path().as_os_str()]).unwrap();
+        // Case A: empty GOMODCACHE falls through to GOPATH. go reads only
+        // the FIRST GOPATH entry for the module cache, so a non-empty first
+        // entry locates it and an empty one leaves go with no module cache
+        // at all ("module cache not found"): never the next entry (#1834).
         let _gomodcache = EnvGuard::set("GOMODCACHE", "");
-        let _gopath = EnvGuard::set("GOPATH", gopath_list.to_str().unwrap());
         let _home = EnvGuard::set("HOME", "/nonexistent-home-unused");
         let _userprofile = EnvGuard::unset("USERPROFILE");
+        let first_a =
+            std::env::join_paths([gopath_a.path().as_os_str(), home_b.path().as_os_str()]).unwrap();
+        let _gopath = EnvGuard::set("GOPATH", first_a.to_str().unwrap());
         assert_eq!(
             GoCrawler::get_gomodcache(),
             Some(gopath_a.path().join("pkg").join("mod")),
-            "empty GOMODCACHE must fall through to GOPATH, skipping the \
-             empty first GOPATH entry"
+            "empty GOMODCACHE must fall through to the first GOPATH entry"
         );
+        for list in [
+            std::env::join_paths(["".as_ref(), gopath_a.path().as_os_str()]).unwrap(),
+            std::env::join_paths(["", ""]).unwrap(),
+        ] {
+            let _gopath = EnvGuard::set("GOPATH", list.to_str().unwrap());
+            assert_eq!(
+                GoCrawler::get_gomodcache(),
+                None,
+                "an empty first GOPATH entry ({list:?}) leaves go no module cache"
+            );
+        }
 
         // Case B: GOMODCACHE and GOPATH both set-but-empty fall all the way
         // through to the $HOME/go/pkg/mod default.
@@ -1589,6 +1583,12 @@ mod tests {
             None,
             "all-empty env must yield None, never a CWD-relative path"
         );
+
+        // Case D: go refuses a relative GOMODCACHE ("must be absolute
+        // path"), so there is no cache to crawl rather than one under CWD.
+        let _home_d = EnvGuard::set("HOME", home_b.path().to_str().unwrap());
+        let _relative = EnvGuard::set("GOMODCACHE", "rel/cache");
+        assert_eq!(GoCrawler::get_gomodcache(), None);
     }
 
     /// #344: GOMODCACHE / GOPATH written with `go env -w` (the `$GOENV`

crates/socket-patch-core/src/utils/go_env.rs

@@ -17,7 +17,7 @@
 //! layer. Upstream Go ships it with the same values as the built-in
 //! defaults, and locating it would mean running `go`.
 
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 /// The effective value of Go setting `key`: a non-empty environment variable
 /// wins, else the Go environment file's non-empty value, else `None` (the
@@ -37,6 +37,46 @@ pub(crate) fn go_env_with(key: &str, env: impl Fn(&str) -> Option<String>) -> Op
     lookup_in_env_file(&text, key).filter(|v| !v.is_empty())
 }
 
+/// go's GOPATH list (`cfg.BuildContext.GOPATH`): the setting split on the
+/// OS list separator, empty entries kept, or go's default `$HOME/go` when
+/// it is unset or empty. `go_env` resolves a setting as [`go_env`] does.
+pub(crate) fn gopath_entries(
+    go_env: &dyn Fn(&str) -> Option<String>,
+    home: Option<&Path>,
+) -> Vec<PathBuf> {
+    match go_env("GOPATH").filter(|v| !v.is_empty()) {
+        Some(gopath) => std::env::split_paths(&gopath).collect(),
+        None => home.map(|h| h.join("go")).into_iter().collect(),
+    }
+}
+
+/// go's module cache (`cfg.GOMODCACHE`): GOMODCACHE, else `pkg/mod` under
+/// the FIRST [`gopath_entries`] entry, the one place both the crawler and
+/// the installed-tool audit read it from (#1834). `None` wherever go has
+/// no usable module cache, never a path under the working directory:
+/// * an empty first entry (`GOPATH=:/a`): go reads only that entry, so it
+///   reports "module cache not found";
+/// * a relative or `~`-prefixed GOPATH entry anywhere in the list: go
+///   refuses to run at all, even with GOMODCACHE set;
+/// * a relative cache path, which go refuses too.
+pub(crate) fn gomodcache(
+    go_env: &dyn Fn(&str) -> Option<String>,
+    home: Option<&Path>,
+) -> Option<PathBuf> {
+    let gopath = gopath_entries(go_env, home);
+    let refused = |p: &PathBuf| {
+        !p.as_os_str().is_empty() && (!p.is_absolute() || p.to_string_lossy().starts_with('~'))
+    };
+    if gopath.iter().any(refused) {
+        return None;
+    }
+    let cache = match go_env("GOMODCACHE").filter(|v| !v.is_empty()) {
+        Some(cache) => PathBuf::from(cache),
+        None => gopath.into_iter().next()?.join("pkg").join("mod"),
+    };
+    cache.is_absolute().then_some(cache)
+}
+
 /// What a `GOFLAGS` setting makes go do about `-modfile`.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(crate) enum GoflagsModfile {
@@ -184,6 +224,49 @@ mod tests {
         assert_eq!(lookup_in_env_file(text, "GONOPROXY"), None);
     }
 
+    /// One GOPATH / GOMODCACHE rule for every caller, checked against go
+    /// 1.24's `go env GOMODCACHE` and `go mod download` (#1834).
+    #[cfg(unix)]
+    #[test]
+    fn gomodcache_follows_go() {
+        let home = Some(Path::new("/home/u"));
+        let cache = |pairs: &[(&str, &str)]| gomodcache(&env_of(pairs), home);
+        let p = |s: &str| Some(PathBuf::from(s));
+        // GOMODCACHE set: it wins.
+        assert_eq!(cache(&[("GOMODCACHE", "/c"), ("GOPATH", "/a")]), p("/c"));
+        // Multi-entry GOPATH: only the first entry counts.
+        assert_eq!(cache(&[("GOPATH", "/a:/b")]), p("/a/pkg/mod"));
+        // No GOPATH: go's default `$HOME/go`.
+        assert_eq!(cache(&[]), p("/home/u/go/pkg/mod"));
+        assert_eq!(gomodcache(&env_of(&[]), None), None);
+        // An empty first entry: go has no module cache, not the next entry
+        // and not the home default.
+        assert_eq!(cache(&[("GOPATH", ":")]), None);
+        assert_eq!(cache(&[("GOPATH", ":/b")]), None);
+        // Empty settings count as unset.
+        assert_eq!(
+            cache(&[("GOMODCACHE", ""), ("GOPATH", "")]),
+            p("/home/u/go/pkg/mod")
+        );
+        // Empty entries elsewhere in the list are ignored.
+        assert_eq!(cache(&[("GOPATH", "/a::/b")]), p("/a/pkg/mod"));
+        // go refuses relative paths: a relative cache, and a relative or
+        // `~` GOPATH entry anywhere, even beside GOMODCACHE.
+        assert_eq!(cache(&[("GOMODCACHE", "rel")]), None);
+        assert_eq!(cache(&[("GOPATH", "rel:/b")]), None);
+        assert_eq!(cache(&[("GOPATH", "/a:rel")]), None);
+        assert_eq!(cache(&[("GOPATH", "/a:~/x")]), None);
+        assert_eq!(cache(&[("GOMODCACHE", "/c"), ("GOPATH", "/a:rel")]), None);
+        assert_eq!(gomodcache(&env_of(&[]), Some(Path::new("rel"))), None);
+
+        let entries = |pairs: &[(&str, &str)]| gopath_entries(&env_of(pairs), home);
+        assert_eq!(
+            entries(&[("GOPATH", "/a::/b")]),
+            ["/a", "", "/b"].map(PathBuf::from)
+        );
+        assert_eq!(entries(&[]), [PathBuf::from("/home/u/go")]);
+    }
+
     /// `-modfile` read from GOFLAGS as go reads it (#1799).
     #[test]
     fn goflags_modfile_reads_the_last_modfile() {

crates/socket-patch-core/src/utils/go_installed_tools.rs

@@ -173,20 +173,16 @@ pub fn bin_dirs_with(
             dirs.push(p);
         }
     };
-    let gopaths: Vec<PathBuf> = match go_env("GOPATH").filter(|v| !v.is_empty()) {
-        Some(gopath) => std::env::split_paths(&gopath)
-            .filter(|p| !p.as_os_str().is_empty())
-            .collect(),
-        None => home.map(|h| h.join("go")).into_iter().collect(),
-    };
-    // go's own default: GOMODCACHE, else the first GOPATH's pkg/mod.
-    let user_cache = go_env("GOMODCACHE")
-        .filter(|v| !v.is_empty())
-        .map(PathBuf::from)
-        .or_else(|| gopaths.first().map(|g| g.join("pkg").join("mod")));
+    // An empty setting counts as unset, as `go_env` already reports it.
+    let go_env = &|k: &str| go_env(k).filter(|v| !v.is_empty());
+    let user_cache = crate::utils::go_env::gomodcache(go_env, home.as_deref());
+    let gopaths: Vec<PathBuf> = crate::utils::go_env::gopath_entries(go_env, home.as_deref())
+        .into_iter()
+        .filter(|p| !p.as_os_str().is_empty())
+        .collect();
     let in_user_cache = |d: &Path| user_cache.as_ref().is_some_and(|c| d.starts_with(c));
     if module_dirs.iter().any(|d| in_user_cache(d)) {
-        match go_env("GOBIN").filter(|v| !v.is_empty()) {
+        match go_env("GOBIN") {
             Some(gobin) => push(PathBuf::from(gobin)),
             None => gopaths.iter().for_each(|g| push(g.join("bin"))),
         }
@@ -549,6 +545,23 @@ mod tests {
             .dirs,
             vec![PathBuf::from("/home/u/go/bin")]
         );
+        // An empty first GOPATH entry leaves go no module cache, so a
+        // copy under a later entry is not in the user's cache: no GOBIN
+        // (#1834).
+        assert_eq!(
+            bin_dirs_with(
+                &[Path::new("/b/pkg/mod/x@v1.0.0")],
+                &env(vec![("GOPATH", ":/b"), ("GOBIN", "/gobin")]),
+                home.clone()
+            )
+            .dirs,
+            vec![PathBuf::from("/b/bin")]
+        );
+        // An empty GOBIN counts as unset.
+        assert_eq!(
+            bin_dirs_with(&[user], &env(vec![("GOBIN", "")]), home.clone()).dirs,
+            vec![PathBuf::from("/home/u/go/bin")]
+        );
         // A --global-prefix tree: never the user's own tools.
         assert_eq!(
             bin_dirs_with(

crates/socket-patch-core/tests/crawler_go_e2e.rs

@@ -394,7 +394,7 @@ async fn get_module_cache_paths_home_go_pkg_mod_fallback() {
 }
 
 /// A set-but-EMPTY `HOME` must count as unset, matching the empty guard on
-/// `GOMODCACHE` and the empty-entry filter on `GOPATH`: honoring `""` makes
+/// `GOMODCACHE` and `GOPATH` (`utils::go_env::gomodcache`): honoring `""` makes
 /// the last-resort fallback return the RELATIVE path `go/pkg/mod`, pointing
 /// every crawl and lookup at `<cwd>/go/pkg/mod` inside the user's project
 /// instead of a real module cache.