diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 8846acf60..1bc50f006 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -354,6 +354,21 @@ async fn run_check(args: &ApplyArgs, manifest_path: &Path) -> i32 { drifts.push((id, "go_redirect_drift".to_string(), d.to_string())); } } + // The redirects can be intact while the project no longer + // builds: a committed vendor/modules.txt or the go.mod + // requirements out of step with them (#343, #618). + for issue in socket_patch_core::vendor::go_consumer_sync::audit( + &args.common.cwd, + &HashMap::new(), + ) + .await + { + drifts.push(( + issue.module().to_string(), + issue.code().to_string(), + issue.to_string(), + )); + } } } @@ -422,7 +437,14 @@ async fn run_check(args: &ApplyArgs, manifest_path: &Path) -> i32 { for (_, _, detail) in &drifts { eprintln!(" {detail}"); } - eprintln!("{}", check_remedy(&args.common)); + // A go consumer-sync drift names its own go command; apply + // cannot regenerate vendor/modules.txt or go.sum. + if drifts.iter().any(|(_, code, _)| { + code != socket_patch_core::vendor::go_consumer_sync::VENDOR_MODULES_TXT_CODE + && code != socket_patch_core::vendor::go_consumer_sync::REQUIREMENTS_CODE + }) { + eprintln!("{}", check_remedy(&args.common)); + } } 1 } @@ -2017,6 +2039,7 @@ async fn apply_patches_inner( } } let mut fallback_skips: Vec = Vec::new(); + let mut go_pristine_mods: HashMap = HashMap::new(); // Multi-copy aware: npm nests genuine duplicates of one `name@version` // (nested dupes, diamonds, `file:` dups), so the resolver returns EVERY @@ -2429,7 +2452,16 @@ async fn apply_patches_inner( match try_local_go_apply(purl, pkg_path, patch, &sources, &args.common, policy) .await { - Some(r) => r, + Some(r) => { + // The unpatched go.mod, so the consumer-sync + // audit below can tell requirements the patch + // added from ones upstream always had (#618). + if let Some((module, _)) = parse_golang_purl(purl) { + go_pristine_mods + .insert(module.into_owned(), pkg_path.join("go.mod")); + } + r + } None => { apply_package_patch( purl, @@ -2508,6 +2540,21 @@ async fn apply_patches_inner( // The human summary is printed by `run`, after the per-package list. + // A redirect leaves the project building only if its committed + // `vendor/modules.txt` and its go.mod requirements still agree with the + // `replace` directives (#343, #618): name the regeneration step. + if !args.common.dry_run && eco_in_local_scope(&args.common, Ecosystem::Golang) { + run_warnings.extend( + socket_patch_core::vendor::go_consumer_sync::audit_warnings( + &args.common.cwd, + &go_pristine_mods, + ) + .await + .into_iter() + .map(|(code, detail)| RunWarning { code, detail }), + ); + } + // Note: `apply` deliberately does NOT garbage-collect unused blobs in // `.socket/`. GC is the responsibility of `socket-patch repair` / // `gc` / `scan --prune`. Keeping apply read-only against `.socket/` diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 77859bd16..d58140f45 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -2740,6 +2740,19 @@ pub(crate) async fn rollback_patches_inner( results.push(result); } + // Dropping a `replace` leaves a committed vendor/modules.txt recording + // it, which breaks every vendored build until it is regenerated (#343). + if !common.dry_run + && results + .iter() + .any(|r| r.success && is_local_go(&r.package_key, common)) + { + let none = std::collections::HashMap::new(); + warnings.extend( + socket_patch_core::vendor::go_consumer_sync::audit_warnings(&common.cwd, &none).await, + ); + } + // The restored crates' compiled copies in this project's build cache // still hold the patched code cargo keyed on the package id (#387). if !common.dry_run { diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index 93cc628b7..714ed8f30 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -1615,6 +1615,24 @@ pub(crate) async fn run_redirect_selected( engine_warnings.extend(done.rush_warnings.iter().cloned()); engine_warnings.extend(done.pnpm_warnings.iter().cloned()); engine_warnings.extend(done.npm_warnings.iter().cloned()); + // A hosted Go `replace` breaks a committed vendor/ directory until + // `go mod vendor` re-records it in vendor/modules.txt (#343). + if !common.dry_run + && done + .rewritten + .iter() + .any(|f| f == "go.mod" || f.ends_with("/go.mod")) + { + for (code, detail) in socket_patch_core::vendor::go_consumer_sync::audit_warnings( + &common.cwd, + &std::collections::HashMap::new(), + ) + .await + { + engine_warnings + .push(socket_patch_core::patch::redirect::RewriteWarning { code, detail }); + } + } let mut warnings: Vec = socket_patch_core::hosted::render::rewrite_warnings_json(&engine_warnings); warnings.extend(gem_stale.warnings.iter().cloned()); diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index bfeb93664..ecd618dac 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1213,6 +1213,7 @@ async fn run_check(args: &VendorArgs) -> i32 { } else { None }; + let go_sync_issues = tokio::sync::OnceCell::new(); for (key, entry) in entries { let record = entry.record.as_ref().or_else(|| manifest.patches.get(key)); let mut failure = match record { @@ -1245,6 +1246,26 @@ async fn run_check(args: &VendorArgs) -> i32 { failure = Some(unwired_check_failure(discovery, root, key, entry).await); } } + // The committed copy and its `replace` can be intact while the + // project no longer builds: vendor/modules.txt or the go.mod + // requirements out of step with the `replace` (#343, #618). + if failure.is_none() && entry.ecosystem == "golang" { + if let Some((module, _)) = socket_patch_core::utils::purl::parse_golang_purl(key) { + let issues: Vec = go_sync_issues + .get_or_init(|| async { + socket_patch_core::vendor::go_consumer_sync::audit(root, &HashMap::new()) + .await + }) + .await + .iter() + .filter(|issue| issue.module() == module) + .map(ToString::to_string) + .collect(); + if !issues.is_empty() { + failure = Some(issues.join("; ")); + } + } + } if vendor::jvm::apply::upstream_unverified(entry) { env.warnings.push(RunWarning {code: "vendor_jvm_upstream_unverified".into(), detail: format!("{key}: upstream metadata was accepted offline; run vendor online to verify registry checksums")}); } @@ -3755,6 +3776,22 @@ pub(crate) async fn vendor_records_reusing( } } + // A vendored Go module builds only if the committed vendor/modules.txt + // and the go.mod requirements agree with its `replace` (#343, #618). + if !common.dry_run && records.keys().any(|p| p.starts_with("pkg:golang/")) { + let warnings = socket_patch_core::vendor::go_consumer_sync::audit_warnings( + &common.cwd, + &HashMap::new(), + ) + .await; + for (code, detail) in warnings { + if !common.json && !common.silent { + eprintln!("Warning: {detail}"); + } + env.warnings.push(RunWarning { code, detail }); + } + } + // A rolled-back eject's summary would describe a vendoring that was // undone: the eject prints it itself once it knows the outcome (#1005). match eject { diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index 202abaf9d..c765bc39d 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -354,3 +354,324 @@ fn go_build_links_patch_via_replace_redirect() { // Best-effort: relax perms so the temp cache cleans up. chmod_writable(tmp.path()); } + +// ── consumer sync: vendor/modules.txt (#343) and requirements (#618) ── + +/// Publish `module@version` (its files, `go.mod` included) into the file +/// proxy at `tmp/proxy`. +fn publish(tmp: &Path, module: &str, version: &str, files: &[(&str, &str)]) { + let stage_root = tmp.join("stage-pub"); + let _ = std::fs::remove_dir_all(&stage_root); + let dir = stage_root.join(format!("{module}@{version}")); + std::fs::create_dir_all(&dir).unwrap(); + for (name, body) in files { + std::fs::write(dir.join(name), body).unwrap(); + } + let pxv = tmp.join("proxy").join(module).join("@v"); + std::fs::create_dir_all(&pxv).unwrap(); + std::fs::write( + pxv.join(format!("{version}.info")), + format!("{{\"Version\":\"{version}\"}}"), + ) + .unwrap(); + let go_mod = files.iter().find(|(n, _)| *n == "go.mod").unwrap().1; + std::fs::write(pxv.join(format!("{version}.mod")), go_mod).unwrap(); + let status = Command::new("zip") + .args([ + "-q", + "-r", + pxv.join(format!("{version}.zip")).to_str().unwrap(), + &format!("{module}@{version}"), + ]) + .current_dir(&stage_root) + .status() + .expect("run zip"); + assert!(status.success(), "zip failed"); + let mut list = std::fs::read_to_string(pxv.join("list")).unwrap_or_default(); + list.push_str(&format!("{version}\n")); + std::fs::write(pxv.join("list"), list).unwrap(); +} + +/// The JSON `warnings[]` codes of a `--json` run's stdout. +fn warnings_of(stdout: &str) -> Vec { + common::envelope::codes_in(&common::envelope::parse_json_envelope(stdout)["warnings"]) +} + +/// #343: in a project with a committed `vendor/` (`go mod vendor`), the +/// redirect alone breaks every default build ("inconsistent vendoring"). +/// apply names `go mod vendor`, `apply --check` reports the drift until it +/// is run, and the same holds the other way round after a rollback. +#[test] +fn committed_vendor_dir_needs_go_mod_vendor_after_apply_and_rollback() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(vendor-dir)") { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let (consumer, modcache, proxy_url) = stage(tmp.path()); + let cs = consumer.to_str().unwrap(); + let mc = modcache.to_str().unwrap(); + let mut goenv = go_env(mc, &proxy_url); + let vendored = go(&consumer, &["mod", "vendor"], &goenv); + assert!( + vendored.status.success(), + "go mod vendor: {}", + String::from_utf8_lossy(&vendored.stderr) + ); + // Default flags from here on: go builds from vendor/. + goenv.retain(|(k, _)| *k != "GOFLAGS"); + goenv.push(("GOFLAGS", "")); + let base = go(&consumer, &["build", "./..."], &goenv); + assert!( + base.status.success(), + "{}", + String::from_utf8_lossy(&base.stderr) + ); + + write_patch(&consumer); + let (code, so, se) = run_socket( + &consumer, + &[ + "apply", + "--offline", + "--ecosystems", + "golang", + "--json", + "--cwd", + cs, + ], + &modcache, + ); + assert_eq!(code, 0, "apply failed.\n{so}\n{se}"); + assert!( + warnings_of(&so).contains(&"go_vendor_modules_txt_out_of_sync".to_string()), + "apply must name the vendor/modules.txt regeneration:\n{so}" + ); + let broken = go(&consumer, &["build", "./..."], &goenv); + assert!( + !broken.status.success() + && String::from_utf8_lossy(&broken.stderr).contains("inconsistent vendoring"), + "the fixture reproduces the broken build: {}", + String::from_utf8_lossy(&broken.stderr) + ); + let (code, _so, se) = run_socket( + &consumer, + &["apply", "--check", "--ecosystems", "golang", "--cwd", cs], + &modcache, + ); + assert_eq!( + code, 1, + "apply --check must report the out-of-sync vendor/:\n{se}" + ); + assert!(se.contains("go mod vendor"), "{se}"); + + // The step the warning names fixes the build, and --check agrees. + let revendor = go(&consumer, &["mod", "vendor"], &goenv); + assert!( + revendor.status.success(), + "{}", + String::from_utf8_lossy(&revendor.stderr) + ); + let run = go(&consumer, &["run", "."], &goenv); + assert!( + String::from_utf8_lossy(&run.stdout).contains("OUT: PATCHED"), + "{}{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + let (code, so, se) = run_socket( + &consumer, + &["apply", "--check", "--ecosystems", "golang", "--cwd", cs], + &modcache, + ); + assert_eq!(code, 0, "in sync after go mod vendor:\n{so}\n{se}"); + + // Rollback drops the replace; modules.txt still records it. + let (code, so, se) = run_socket( + &consumer, + &[ + "rollback", + "--offline", + "--ecosystems", + "golang", + "--json", + "--cwd", + cs, + ], + &modcache, + ); + assert_eq!(code, 0, "rollback failed.\n{so}\n{se}"); + assert!( + warnings_of(&so).contains(&"go_vendor_modules_txt_out_of_sync".to_string()), + "rollback must name the vendor/modules.txt regeneration:\n{so}" + ); + let (code, _so, se) = run_socket( + &consumer, + &["apply", "--check", "--ecosystems", "golang", "--cwd", cs], + &modcache, + ); + assert_eq!(code, 1, "stale vendor/modules.txt is drift:\n{se}"); + assert!( + se.contains("no longer has") && se.contains("go mod vendor"), + "{se}" + ); + + chmod_writable(tmp.path()); +} + +/// #618: a patch whose go.mod raises a requirement (the common security-fix +/// shape) leaves the default `-mod=readonly` build failing with "updates +/// to go.mod needed". apply names `go mod tidy`, `apply --check` reports +/// the drift until it is run, and after it the patched bytes build. +#[test] +fn patched_go_mod_requirement_bump_needs_go_mod_tidy() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(requirements)") { + return; + } + let godir = golang_e2e_matrix::go_directive(); + let tmp = tempfile::tempdir().unwrap(); + let t = tmp.path(); + const DEP: &str = "example.com/dep"; + let dep_mod = format!("module {DEP}\n\ngo {godir}\n"); + let dep_src = |tag: &str| { + format!("package dep\n\nfunc Safe(s string) string {{ return \"{tag}-\" + s }}\n") + }; + publish( + t, + DEP, + "v1.0.0", + &[("go.mod", &dep_mod), ("dep.go", &dep_src("OLD"))], + ); + publish( + t, + DEP, + "v1.1.0", + &[("go.mod", &dep_mod), ("dep.go", &dep_src("FIXED"))], + ); + let up_mod = |v: &str| format!("module {UMOD}\n\ngo {godir}\n\nrequire {DEP} {v}\n"); + let up_lib = |tag: &str| { + format!( + "package upstream\n\nimport \"{DEP}\"\n\nfunc Greeting() string {{ return dep.Safe(\"{tag}\") }}\n" + ) + }; + let (before_mod, after_mod) = (up_mod("v1.0.0"), up_mod("v1.1.0")); + let (before_lib, after_lib) = (up_lib("PRISTINE"), up_lib("PATCHED")); + publish( + t, + UMOD, + UVER, + &[("go.mod", &before_mod), ("lib.go", &before_lib)], + ); + + let modcache = t.join("modcache"); + std::fs::create_dir_all(&modcache).unwrap(); + let proxy_url = format!("file://{}", t.join("proxy").display()); + let consumer = t.join("consumer"); + std::fs::create_dir_all(&consumer).unwrap(); + std::fs::write( + consumer.join("go.mod"), + format!("module example.com/consumer\n\ngo {godir}\n\nrequire {UMOD} {UVER}\n"), + ) + .unwrap(); + std::fs::write( + consumer.join("main.go"), + format!( + "package main\n\nimport (\n\t\"fmt\"\n\t\"{UMOD}\"\n)\n\nfunc main() {{ fmt.Println(\"OUT:\", upstream.Greeting()) }}\n" + ), + ) + .unwrap(); + let mc = modcache.to_str().unwrap(); + let mut goenv = go_env(mc, &proxy_url); + let tidy = go(&consumer, &["mod", "tidy"], &goenv); + assert!( + tidy.status.success(), + "{}", + String::from_utf8_lossy(&tidy.stderr) + ); + goenv.retain(|(k, _)| *k != "GOFLAGS"); + goenv.push(("GOFLAGS", "")); + let base = go(&consumer, &["run", "."], &goenv); + assert!( + String::from_utf8_lossy(&base.stdout).contains("OUT: OLD-PRISTINE"), + "{}", + String::from_utf8_lossy(&base.stderr) + ); + + // The patch: lib.go and the go.mod requirement bump. + let socket = consumer.join(".socket"); + std::fs::create_dir_all(socket.join("blobs")).unwrap(); + for body in [&after_lib, &after_mod] { + std::fs::write(socket.join("blobs").join(git_sha256(body.as_bytes())), body).unwrap(); + } + let manifest = serde_json::json!({"patches": {UPURL: { + "uuid": UUID, "exportedAt": "t", + "files": { + "lib.go": {"beforeHash": git_sha256(before_lib.as_bytes()), + "afterHash": git_sha256(after_lib.as_bytes())}, + "go.mod": {"beforeHash": git_sha256(before_mod.as_bytes()), + "afterHash": git_sha256(after_mod.as_bytes())}, + }, + "vulnerabilities": {GHSA: {"cves": [CVE], "summary": "s", "severity": "high", + "description": "d"}}, + "description": "", "license": "", "tier": ""}}}); + std::fs::write(socket.join("manifest.json"), manifest.to_string()).unwrap(); + + let cs = consumer.to_str().unwrap(); + let (code, so, se) = run_socket( + &consumer, + &[ + "apply", + "--offline", + "--ecosystems", + "golang", + "--json", + "--cwd", + cs, + ], + &modcache, + ); + assert_eq!(code, 0, "apply failed.\n{so}\n{se}"); + assert!( + warnings_of(&so).contains(&"go_requirements_out_of_sync".to_string()), + "apply must name the go.mod/go.sum refresh:\n{so}" + ); + let broken = go(&consumer, &["build", "./..."], &goenv); + assert!( + !broken.status.success(), + "the fixture reproduces the readonly failure" + ); + let (code, _so, se) = run_socket( + &consumer, + &["apply", "--check", "--ecosystems", "golang", "--cwd", cs], + &modcache, + ); + assert_eq!( + code, 1, + "apply --check must report the requirement drift:\n{se}" + ); + assert!(se.contains("go mod tidy"), "{se}"); + + let mut tidy_env = goenv.clone(); + tidy_env.retain(|(k, _)| *k != "GOFLAGS"); + tidy_env.push(("GOFLAGS", "-mod=mod")); + let tidy = go(&consumer, &["mod", "tidy"], &tidy_env); + assert!( + tidy.status.success(), + "{}", + String::from_utf8_lossy(&tidy.stderr) + ); + let run = go(&consumer, &["run", "."], &goenv); + assert!( + String::from_utf8_lossy(&run.stdout).contains("OUT: FIXED-PATCHED"), + "{}{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + let (code, so, se) = run_socket( + &consumer, + &["apply", "--check", "--ecosystems", "golang", "--cwd", cs], + &modcache, + ); + assert_eq!(code, 0, "in sync after go mod tidy:\n{so}\n{se}"); + + chmod_writable(tmp.path()); +} diff --git a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs new file mode 100644 index 000000000..5599e40f9 --- /dev/null +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -0,0 +1,616 @@ +//! Does the consumer project still build after a socket `replace` was wired +//! in or removed? (#343, #618) +//! +//! Every Go mode (`apply`, `vendor`, `scan --mode hosted`) redirects a module +//! by editing the consumer's `go.mod` `replace` directives. Two pieces of the +//! consumer's state are derived from `go.mod` and the module graph, and the +//! go command refuses to build when they disagree with it: +//! +//! * **`vendor/modules.txt`** (#343). With a committed `vendor/` directory +//! (`go mod vendor`, or `go work vendor` for a workspace) go builds with +//! `-mod=vendor` and checks that `modules.txt` records every `go.mod` +//! `replace`: "is replaced in go.mod, but not marked as replaced in +//! vendor/modules.txt" after a redirect, and "is marked as replaced in +//! vendor/modules.txt, but not replaced in go.mod" after a rollback. +//! Re-running `go mod vendor` copies the patched tree into `vendor/` and +//! records the replacement. +//! * **Requirements raised by the patched module's own `go.mod`** (#618). A +//! security fix often bumps a dependency of the patched module. go reads +//! the replacement's `go.mod`, so the selected version of that dependency +//! rises above what the consumer's `go.mod` lists, and the default +//! `-mod=readonly` build fails with "updates to go.mod needed" (or +//! "missing go.sum entry" for an added requirement) until `go mod tidy` +//! updates `go.mod` and `go.sum`. +//! +//! socket-patch does not regenerate either file itself: `go mod vendor` and +//! `go mod tidy` need the go toolchain and every module in the graph, and +//! rewrite files the user owns. Instead [`audit`] reports each disagreement +//! with the command that fixes it; the commands surface the result as a run +//! warning, and `apply --check` treats it as drift. + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; + +use crate::utils::fs::read_regular_to_string; +use crate::vendor::go_mod_edit::{self, ReplaceEntry}; + +/// Warning code for a `vendor/modules.txt` that disagrees with a socket +/// `replace` in `go.mod`. +pub const VENDOR_MODULES_TXT_CODE: &str = "go_vendor_modules_txt_out_of_sync"; +/// Warning code for a patched module whose `go.mod` requires a newer (or a +/// new) dependency than the consumer's `go.mod` lists. +pub const REQUIREMENTS_CODE: &str = "go_requirements_out_of_sync"; + +/// One way the consumer's derived Go state disagrees with the socket +/// `replace` directives. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum GoSyncIssue { + /// `modules.txt` lacks the replacement a socket `replace` in `go.mod` + /// declares (`stale: false`), or records a socket replacement `go.mod` + /// no longer has (`stale: true`). + VendorModulesTxt { + module: String, + /// Project-relative path of the `modules.txt` go checks. + modules_txt: String, + /// `go mod vendor` or `go work vendor`. + command: &'static str, + stale: bool, + }, + /// The patched copy of `module` requires `dep` at `required`, above the + /// `listed` version in the consumer's `go.mod` (`None`: not listed). + Requirement { + module: String, + dep: String, + required: String, + listed: Option, + }, +} + +impl GoSyncIssue { + /// The stable warning / drift code. + pub fn code(&self) -> &'static str { + match self { + GoSyncIssue::VendorModulesTxt { .. } => VENDOR_MODULES_TXT_CODE, + GoSyncIssue::Requirement { .. } => REQUIREMENTS_CODE, + } + } + + /// The module the issue is about (the replaced module). + pub fn module(&self) -> &str { + match self { + GoSyncIssue::VendorModulesTxt { module, .. } + | GoSyncIssue::Requirement { module, .. } => module, + } + } +} + +impl std::fmt::Display for GoSyncIssue { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + GoSyncIssue::VendorModulesTxt { + module, + modules_txt, + command, + stale: false, + } => write!( + f, + "{modules_txt} does not record the go.mod `replace` for {module}, so every \ + build using the committed vendor directory fails with \"inconsistent \ + vendoring\"; run `{command}` to copy the patched module into vendor/, then \ + commit it" + ), + GoSyncIssue::VendorModulesTxt { + module, + modules_txt, + command, + stale: true, + } => write!( + f, + "{modules_txt} still records a socket-patch replacement for {module} that \ + go.mod no longer has, so every build using the committed vendor directory \ + fails with \"inconsistent vendoring\"; run `{command}` to restore the \ + unpatched module in vendor/, then commit it" + ), + GoSyncIssue::Requirement { + module, + dep, + required, + listed, + } => { + let listed = match listed { + Some(v) => format!("go.mod lists {v}"), + None => "go.mod does not require it".to_string(), + }; + write!( + f, + "the patched {module} requires {dep} {required} but {listed}, so a \ + default (-mod=readonly) go build fails until go.mod and go.sum are \ + updated; run `go mod tidy`, then commit go.mod and go.sum" + ) + } + } + } +} + +/// Every [`GoSyncIssue`] in the project at `project_root` (the directory +/// holding the consumer `go.mod`). Read-only and offline: reads `go.mod`, +/// the socket copies it points at, and `vendor/modules.txt`. +/// +/// `pristine_go_mods` maps a replaced module to its unpatched `go.mod` +/// (the module-cache copy a run just patched). With it, a requirement the +/// patch ADDED (or raised) that the consumer does not list at all is +/// reported too; without it (`apply --check`, no module cache) only a +/// requirement the consumer lists at a lower version is, since an upstream +/// `go.mod` routinely requires modules a pruned consumer graph never lists. +pub async fn audit( + project_root: &Path, + pristine_go_mods: &HashMap, +) -> Vec { + let Ok(go_mod) = read_regular_to_string(&project_root.join("go.mod")).await else { + return Vec::new(); + }; + let replaces = go_mod_edit::parse_replace_entries(&go_mod); + let mut issues = vendor_modules_txt_issues(project_root, &replaces).await; + issues.extend(requirement_issues(project_root, &go_mod, &replaces, pristine_go_mods).await); + issues +} + +/// [`audit`] as `(code, detail)` warning pairs, for the commands' warning +/// channels. +pub async fn audit_warnings( + project_root: &Path, + pristine_go_mods: &HashMap, +) -> Vec<(String, String)> { + audit(project_root, pristine_go_mods) + .await + .into_iter() + .map(|issue| (issue.code().to_string(), issue.to_string())) + .collect() +} + +/// Where go looks for the vendor directory's `modules.txt`, and the command +/// that regenerates it: the workspace root's `vendor/` (`go work vendor`) +/// when a `go.work` is in effect, else the module's own (`go mod vendor`). +fn modules_txt_location(project_root: &Path) -> (PathBuf, &'static str) { + if let Some(work_dir) = workspace_file(project_root) + .as_deref() + .and_then(Path::parent) + { + return ( + work_dir.join("vendor").join("modules.txt"), + "go work vendor", + ); + } + ( + project_root.join("vendor").join("modules.txt"), + "go mod vendor", + ) +} + +/// The `go.work` go would use for `project_root`: `GOWORK` when it names a +/// file (`off` disables workspaces), else the nearest `go.work` at or above +/// the project. +fn workspace_file(project_root: &Path) -> Option { + match std::env::var("GOWORK") { + Ok(v) if v == "off" => return None, + Ok(v) if !v.is_empty() => return Some(PathBuf::from(v)), + _ => {} + } + let base = std::env::current_dir() + .unwrap_or_default() + .join(project_root); + base.ancestors() + .map(|dir| dir.join("go.work")) + .find(|file| file.is_file()) +} + +/// The replaces a workspace `modules.txt` records: `go work vendor` writes +/// every `use` member's `go.mod` replaces plus the `go.work` ones. Returns +/// `(go.work replaces, every member's replaces)`. +async fn workspace_replaces(project_root: &Path) -> (Vec, Vec) { + let Some(work_file) = workspace_file(project_root) else { + return (Vec::new(), Vec::new()); + }; + let Ok(text) = read_regular_to_string(&work_file).await else { + return (Vec::new(), Vec::new()); + }; + let work_dir = work_file.parent().unwrap_or(Path::new("")); + let mut members = Vec::new(); + for dir in go_mod_edit::parse_use_dirs(&text) { + if let Ok(go_mod) = read_regular_to_string(&work_dir.join(&dir).join("go.mod")).await { + members.extend(go_mod_edit::parse_replace_entries(&go_mod)); + } + } + (go_mod_edit::parse_replace_entries(&text), members) +} + +async fn vendor_modules_txt_issues( + project_root: &Path, + replaces: &[ReplaceEntry], +) -> Vec { + let (path, command) = modules_txt_location(project_root); + let Ok(text) = read_regular_to_string(&path).await else { + return Vec::new(); + }; + let workspace = command == "go work vendor"; + let display = { + let base = std::env::current_dir() + .unwrap_or_default() + .join(project_root); + path.strip_prefix(&base) + .map(|rel| rel.to_string_lossy().replace('\\', "/")) + .unwrap_or_else(|_| path.display().to_string()) + }; + let recorded: Vec = text + .lines() + .filter_map(|line| line.strip_prefix("# ")) + .filter(|body| body.contains("=>")) + .filter_map(go_mod_edit::parse_replace_body) + .collect(); + + // In a workspace, a `go.work` replace of the same module overrides the + // member's, and a recorded replacement may come from any member. + let (work_replaces, member_replaces) = if workspace { + workspace_replaces(project_root).await + } else { + (Vec::new(), Vec::new()) + }; + let overridden_by_work = |entry: &ReplaceEntry| { + work_replaces.iter().any(|w| { + w.module == entry.module && (w.version.is_none() || w.version == entry.version) + }) + }; + + let mut issues = Vec::new(); + for entry in replaces.iter().filter(|e| e.socket_owned()) { + if !overridden_by_work(entry) + && !recorded + .iter() + .any(|r| same_replacement(entry, r, workspace)) + { + issues.push(GoSyncIssue::VendorModulesTxt { + module: entry.module.clone(), + modules_txt: display.clone(), + command, + stale: false, + }); + } + } + for rec in recorded.iter().filter(|r| r.socket_owned()) { + if !replaces + .iter() + .chain(&member_replaces) + .chain(&work_replaces) + .any(|e| same_replacement(e, rec, workspace)) + { + issues.push(GoSyncIssue::VendorModulesTxt { + module: rec.module.clone(), + modules_txt: display.clone(), + command, + stale: true, + }); + } + } + issues +} + +/// Whether a `modules.txt` replacement records the `go.mod` one. go writes +/// the target verbatim from `go.mod`; a workspace vendor directory may +/// record a module's directory replacement relative to the workspace root, +/// so there the recorded path only has to end with the module's. +fn same_replacement(go_mod: &ReplaceEntry, recorded: &ReplaceEntry, workspace: bool) -> bool { + let norm = |p: &str| { + let p = p.replace('\\', "/"); + p.strip_prefix("./").unwrap_or(&p).to_string() + }; + go_mod.module == recorded.module + && go_mod.version == recorded.version + && match (&go_mod.path, &recorded.path) { + (Some(a), Some(b)) => { + let (a, b) = (norm(a), norm(b)); + a == b || (workspace && b.ends_with(&format!("/{a}"))) + } + (None, None) => { + go_mod.rhs_module == recorded.rhs_module + && go_mod.rhs_version == recorded.rhs_version + } + _ => false, + } +} + +async fn requirement_issues( + project_root: &Path, + consumer_go_mod: &str, + replaces: &[ReplaceEntry], + pristine_go_mods: &HashMap, +) -> Vec { + let consumer = + go_mod_edit::parse_required_versions(&go_mod_edit::normalize_for_read(consumer_go_mod)); + let mut issues = Vec::new(); + for entry in replaces.iter().filter(|e| e.socket_owned()) { + // Only a directory copy socket-patch wrote can be read here: a + // hosted (module-to-module) replacement's go.mod lives in the + // module cache, not the project. + let Some(rel) = entry.path.as_deref() else { + continue; + }; + let rel = rel.replace('\\', "/"); + let rel = rel.strip_prefix("./").unwrap_or(&rel); + if !crate::patch::path_safety::is_safe_multi_segment(rel) { + continue; + } + let Ok(copy) = read_regular_to_string(&project_root.join(rel).join("go.mod")).await else { + continue; + }; + let patched = requirements_of(©); + let pristine = match pristine_go_mods.get(&entry.module) { + Some(path) => match read_regular_to_string(path).await { + Ok(text) => Some(requirements_of(&text)), + // A pre-modules package: its copy's go.mod is synthesized + // and requires nothing. + Err(_) => Some(HashMap::new()), + }, + None => None, + }; + let mut deps: Vec<(&String, &String)> = patched.iter().collect(); + deps.sort(); + for (dep, required) in deps { + if *dep == entry.module { + continue; + } + let listed = consumer.get(dep); + let behind = match listed { + Some(listed) => go_semver_cmp(listed, required).is_lt(), + // Unlisted: only a requirement the patch added or raised + // is known to matter. + None => pristine.as_ref().is_some_and(|p| { + p.get(dep) + .is_none_or(|before| go_semver_cmp(before, required).is_lt()) + }), + }; + if behind { + issues.push(GoSyncIssue::Requirement { + module: entry.module.clone(), + dep: dep.clone(), + required: required.clone(), + listed: listed.cloned(), + }); + } + } + } + issues +} + +fn requirements_of(go_mod: &str) -> HashMap { + go_mod_edit::parse_required_versions(&go_mod_edit::normalize_for_read(go_mod)) +} + +fn go_semver_cmp(a: &str, b: &str) -> std::cmp::Ordering { + crate::vendor::go_sum_edit::go_semver_cmp(a, b) +} + +#[cfg(test)] +mod tests { + use super::*; + + const COPY: &str = ".socket/go-patches/example.com/upstream@v1.0.0"; + + fn write(root: &Path, rel: &str, text: &str) { + let path = root.join(rel); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, text).unwrap(); + } + + fn consumer(root: &Path, extra: &str) { + write( + root, + "go.mod", + &format!( + "module example.com/c\n\ngo 1.21\n\nrequire example.com/upstream v1.0.0\n\n\ + require example.com/dep v1.0.0 // indirect\n{extra}" + ), + ); + } + + const REPLACE: &str = "\nreplace example.com/upstream v1.0.0 => \ + ./.socket/go-patches/example.com/upstream@v1.0.0\n"; + + /// #343: `go mod vendor` before the redirect leaves a modules.txt that + /// does not record the socket replace; after `go mod vendor` it does; + /// after a rollback the recorded one is stale. + #[tokio::test] + async fn vendor_modules_txt_must_record_socket_replaces() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + consumer(root, REPLACE); + write( + root, + &format!("{COPY}/go.mod"), + "module example.com/upstream\n", + ); + let none = HashMap::new(); + + // No vendor directory: nothing to sync. + assert_eq!(audit(root, &none).await, vec![]); + + write( + root, + "vendor/modules.txt", + "# example.com/dep v1.0.0\n## explicit; go 1.21\nexample.com/dep\n\ + # example.com/upstream v1.0.0\n## explicit; go 1.21\nexample.com/upstream\n", + ); + let issues = audit(root, &none).await; + assert_eq!( + issues, + vec![GoSyncIssue::VendorModulesTxt { + module: "example.com/upstream".into(), + modules_txt: "vendor/modules.txt".into(), + command: "go mod vendor", + stale: false, + }] + ); + assert_eq!(issues[0].code(), VENDOR_MODULES_TXT_CODE); + assert!(issues[0].to_string().contains("run `go mod vendor`")); + + // What `go mod vendor` writes after the redirect. + write( + root, + "vendor/modules.txt", + &format!( + "# example.com/dep v1.0.0\n## explicit; go 1.21\nexample.com/dep\n\ + # example.com/upstream v1.0.0 => ./{COPY}\n## explicit; go 1.21\n\ + example.com/upstream\n" + ), + ); + assert_eq!(audit(root, &none).await, vec![]); + + // Rolled back: go.mod lost the replace, modules.txt still has it. + consumer(root, ""); + let issues = audit(root, &none).await; + assert!( + matches!( + &issues[..], + [GoSyncIssue::VendorModulesTxt { stale: true, .. }] + ), + "{issues:?}" + ); + assert!(issues[0].to_string().contains("no longer has")); + } + + /// A workspace `modules.txt` records every `use` member's replaces plus + /// `go.work`'s: auditing one member must not call another member's + /// socket replace stale, and a member replace that a `go.work` replace + /// of the same module overrides is not expected in it. + #[tokio::test] + async fn workspace_modules_txt_is_checked_against_every_member() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + consumer(root, REPLACE); + write( + root, + &format!("{COPY}/go.mod"), + "module example.com/upstream\n", + ); + write( + root, + "tools/go.mod", + "module example.com/tools\n\ngo 1.21\n\nrequire example.com/other v1.0.0\n\n\ + replace example.com/other v1.0.0 => ./.socket/go-patches/example.com/other@v1.0.0\n", + ); + write(root, "go.work", "go 1.21\n\nuse (\n\t.\n\t./tools\n)\n"); + // What `go work vendor` writes: both members' replaces. + write( + root, + "vendor/modules.txt", + &format!( + "# example.com/upstream v1.0.0 => ./{COPY}\n## explicit; go 1.21\n\ + example.com/upstream\n\ + # example.com/other v1.0.0 => ./tools/.socket/go-patches/example.com/other@v1.0.0\n\ + ## explicit; go 1.21\nexample.com/other\n" + ), + ); + let none = HashMap::new(); + assert_eq!(audit(root, &none).await, vec![]); + assert_eq!(audit(&root.join("tools"), &none).await, vec![]); + + // go.work replaces the root's patched module itself: modules.txt + // records go.work's replacement, not the member's. + write( + root, + "go.work", + "go 1.21\n\nuse (\n\t.\n\t./tools\n)\n\nreplace example.com/upstream => ../fork\n", + ); + write( + root, + "vendor/modules.txt", + "# example.com/upstream v1.0.0 => ../fork\n## explicit; go 1.21\n\ + example.com/upstream\n\ + # example.com/other v1.0.0 => ./tools/.socket/go-patches/example.com/other@v1.0.0\n", + ); + assert_eq!(audit(root, &none).await, vec![]); + } + + /// A user-authored replace is never ours to report, and a hosted + /// (module-to-module) replace is checked by module + version. + #[tokio::test] + async fn only_socket_replaces_are_checked() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let hosted = "\nreplace example.com/upstream v1.0.0 => \ + patch.socket.dev/gopatch/11111111-2222-4333-8444-555555555555 \ + v1.0.0-socketpatch.1\nreplace example.com/mine => ../mine\n"; + consumer(root, hosted); + write( + root, + "vendor/modules.txt", + "# example.com/upstream v1.0.0\n", + ); + let issues = audit(root, &HashMap::new()).await; + assert_eq!(issues.len(), 1, "{issues:?}"); + assert_eq!(issues[0].module(), "example.com/upstream"); + + write( + root, + "vendor/modules.txt", + "# example.com/upstream v1.0.0 => \ + patch.socket.dev/gopatch/11111111-2222-4333-8444-555555555555 \ + v1.0.0-socketpatch.1\n# example.com/mine => ../mine\n", + ); + assert_eq!(audit(root, &HashMap::new()).await, vec![]); + } + + /// #618: the patched copy's go.mod raising a requirement the consumer + /// lists lower is always reported; one it adds is reported when the + /// pristine go.mod shows it is new. + #[tokio::test] + async fn patched_go_mod_requirements_must_be_in_the_consumer() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + consumer(root, REPLACE); + write( + root, + "cache/go.mod", + "module example.com/upstream\n\ngo 1.21\n\nrequire (\n\texample.com/dep v1.0.0\n\ + \texample.com/testonly v0.1.0\n)\n", + ); + write( + root, + &format!("{COPY}/go.mod"), + "module example.com/upstream\n\ngo 1.21\n\nrequire (\n\texample.com/dep v1.1.0\n\ + \texample.com/testonly v0.1.0\n\texample.com/added v0.2.0\n)\n", + ); + let pristine = HashMap::from([( + "example.com/upstream".to_string(), + root.join("cache/go.mod"), + )]); + + let raised = GoSyncIssue::Requirement { + module: "example.com/upstream".into(), + dep: "example.com/dep".into(), + required: "v1.1.0".into(), + listed: Some("v1.0.0".into()), + }; + let added = GoSyncIssue::Requirement { + module: "example.com/upstream".into(), + dep: "example.com/added".into(), + required: "v0.2.0".into(), + listed: None, + }; + // Without the pristine go.mod (apply --check) only the raised one; + // the unlisted, unchanged `testonly` is never reported. + assert_eq!(audit(root, &HashMap::new()).await, vec![raised.clone()]); + assert_eq!(audit(root, &pristine).await, vec![added, raised.clone()]); + assert!(raised.to_string().contains("run `go mod tidy`")); + assert_eq!(raised.code(), REQUIREMENTS_CODE); + + // After `go mod tidy` the consumer lists both: in sync. + consumer( + root, + &format!( + "{REPLACE}\nrequire (\n\texample.com/dep v1.1.0 // indirect\n\ + \texample.com/added v0.2.0 // indirect\n)\n" + ), + ); + assert_eq!(audit(root, &pristine).await, vec![]); + } +} diff --git a/crates/socket-patch-core/src/vendor/go_mod_edit.rs b/crates/socket-patch-core/src/vendor/go_mod_edit.rs index a74d15209..16fdafe7a 100644 --- a/crates/socket-patch-core/src/vendor/go_mod_edit.rs +++ b/crates/socket-patch-core/src/vendor/go_mod_edit.rs @@ -327,7 +327,7 @@ fn rhs_is_path(tok: &str) -> bool { /// Parse the `module path => target [version]` body of a replace directive /// (the part after the `replace` keyword, or a line inside a `replace ( … )` /// block). Returns `None` if there is no `=>` (not a replace body). -fn parse_replace_body(body: &str) -> Option { +pub(crate) fn parse_replace_body(body: &str) -> Option { let (lhs, rhs) = body.split_once("=>")?; let lhs: Vec<&str> = lhs.split_whitespace().map(unquote).collect(); let rhs: Vec<&str> = rhs.split_whitespace().map(unquote).collect(); @@ -367,6 +367,19 @@ pub fn parse_replace_entries(content: &str) -> Vec { out } +/// Parse a `go.work` file's `use` directives into the member directories +/// they name (as written, unquoted), single-line and block form alike. +pub fn parse_use_dirs(content: &str) -> Vec { + let mut out = Vec::new(); + let _ = for_each_directive_body(content, "use", |_, body| { + if let Some(tok) = body.split_whitespace().next() { + out.push(unquote(tok).to_string()); + } + Ok(()) + }); + out +} + /// Parse `require` directives into `module -> version` (last wins; the module /// graph selects one version per module path). Pure — exposed for the hosted /// rewriter's stale-pin cross-check (a version-pinned `replace` is silently diff --git a/crates/socket-patch-core/src/vendor/go_sum_edit.rs b/crates/socket-patch-core/src/vendor/go_sum_edit.rs index 6b5c774be..0fd80d3fe 100644 --- a/crates/socket-patch-core/src/vendor/go_sum_edit.rs +++ b/crates/socket-patch-core/src/vendor/go_sum_edit.rs @@ -96,7 +96,7 @@ fn go_sum_line_cmp(a: &str, b: &str) -> std::cmp::Ordering { /// `golang.org/x/mod/semver.Compare`: build metadata (`+incompatible`) is /// ignored and an invalid version sorts before every valid one. -fn go_semver_cmp(a: &str, b: &str) -> std::cmp::Ordering { +pub(crate) fn go_semver_cmp(a: &str, b: &str) -> std::cmp::Ordering { let parse = |v: &str| { v.strip_prefix('v') .and_then(|s| semver::Version::parse(s).ok()) diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index 9dd7f1150..d1c3b1bcd 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -63,6 +63,7 @@ pub mod cargo_tag; pub(crate) mod common; pub mod composer_lock; pub mod gem; +pub mod go_consumer_sync; pub mod go_mod_edit; pub mod go_sum_edit; pub mod golang; diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 396da17c5..cfb87b59d 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -1251,6 +1251,19 @@ lines, so `go mod tidy -diff` stays clean after a patch update. This requires a free, publicly retrievable patch reference carrying a `goproxy` override. CLI support does not imply a patch is published for a particular module. + +### Files go derives from go.mod + +Two files the go command derives from `go.mod` can disagree with a new or removed +socket `replace`, and go then refuses to build. socket-patch does not regenerate +them (that needs the go toolchain and the whole module graph); `apply`, `vendor`, +`rollback` and `scan --mode hosted` warn with the command that does, and +`apply --check` / `vendor --check` report the disagreement as drift until it is run: + +| Warning code | When | Fix | +| --- | --- | --- | +| `go_vendor_modules_txt_out_of_sync` | A committed `vendor/` directory (`go mod vendor`; `go work vendor` for a workspace) whose `vendor/modules.txt` does not record a socket `replace`, or still records one that was removed. go builds with `-mod=vendor` and fails with "inconsistent vendoring" | `go mod vendor` (or `go work vendor`), then commit `vendor/` | +| `go_requirements_out_of_sync` | The patched module's own `go.mod` requires a dependency at a version above the one the project's `go.mod` lists (agent and vendored modes; `apply` also reports a requirement the patch added). The default `-mod=readonly` build fails with "updates to go.mod needed" or "missing go.sum entry" | `go mod tidy`, then commit `go.mod` and `go.sum` | Paid hosted Go references are unsupported: embedding credentials in module paths would expose them to module proxies and change module identity. Use vendored mode for those patches; the CLI reports `redirect_golang_unsupported` when the required