From 40b66fe52cf91f28e3b45f58203efd0dd2d5bcfe Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 18:23:39 +0000 Subject: [PATCH 1/5] Start fix for #422 Assisted-by: Claude Code:claude-opus-5-5 From ac0b52fcc53d4c040ac451809388e7af7ed48b80 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 18:39:04 +0000 Subject: [PATCH 2/5] Warn when global Go patches miss installed tools A global Go apply patches the module cache in place, which only changes what later builds compile. Tools already built from the unpatched module with `go install` kept running the vulnerable code while apply reported success and `vex -g` attested not_affected. apply now reads the build info of the binaries in $GOBIN / GOPATH bin dirs with `go version -m` and warns go_installed_tool_stale for each tool built from a patched module before it was patched, naming the `go install @` that rebuilds it. `vex -g` omits the patch with the same code until every such tool is rebuilt. Fixes #422 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 10 +- crates/socket-patch-cli/src/commands/apply.rs | 47 ++ crates/socket-patch-cli/src/commands/vex.rs | 68 +++ .../tests/e2e_golang_build.rs | 132 +++++ .../src/utils/go_installed_tools.rs | 455 ++++++++++++++++++ crates/socket-patch-core/src/utils/mod.rs | 1 + docs/ecosystems.md | 21 + 7 files changed, 733 insertions(+), 1 deletion(-) create mode 100644 crates/socket-patch-core/src/utils/go_installed_tools.rs diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index e2bc9b3d9..9a08f0e2a 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -283,7 +283,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc **Hosted-state visibility (`redirectState`, additive/MINOR).** Every non-hosted-mode, non-vendored-mode `scan --json` SUCCESS envelope (report-only, `--mode agent`/`--sync`, and the zero-discovery envelope) carries an additive top-level `redirectState` object whenever the project's lockfiles pin ≥ 1 hosted patch: `{ mode, records: [{purl, uuid}], wiringLive: [purl] }`. It is a descriptive STATE block, not a warning. `mode` is the constant `"hosted"`. **v5.0 (MAJOR shape change)**: hosted mode keeps no ledger, so `records` lists the hosted pins lockfile discovery finds (one per `(purl, uuid)`, the same discovery `vex` uses: a hosted URL counts only on `https://patch.socket.dev` or the `--patch-server-url` origin), and the v4 `ledger` and `records[].ledgerKey` keys are gone. Each record's `purl` is CANONICALIZED (qualifiers stripped, percent-decoded — e.g. `pkg:npm/@scope/pkg@1.0.0`, `pkg:gem/nokogiri@1.13.3`) to the same spelling `wiringLive` carries, so the join is a plain string compare. `wiringLive` is the subset of those pins among this run's *counted* purls (post-`--ecosystems`-filter) — computed once per run, the same set that feeds `hosted_wiring_retained`. A record missing from `wiringLive` is still wired; it just was not crawled/queried this run (an `--ecosystems` filter, a zero discovery). The key is omitted when no lockfile pins a hosted patch (and under `--global`), and error envelopes (the `--offline` refusal, all-batches-failed) are deliberately minimal and never carry it. A pre-v5 `.socket/vendor/redirect-state.json` is not read. Hosted-mode runs carry their own `redirect` payload and `details.mode: "hosted"` events instead, and vendored-mode runs carry the takeover warnings (their takeovers may restore pins mid-run) — neither duplicates a pre-run snapshot that could go stale. -**Agent-flow run-level warnings (additive).** An agent-mode apply (`--mode agent` / `--sync`, `--json`) may add a top-level `warnings[]` array of `{code, detail}` entries to the scan envelope (absent when none fired; each is also mirrored to stderr unless `--silent`). They surface cross-mode state the apply cannot change — never a status or exit-code change (hosted refusals set the precedent: exit 0 + warning). Codes (stable; new codes are additive/MINOR): `vendored_ownership_retained` — vendor-owned package(s) were skipped before download (each is also a `skipped` / `vendored` event); the detail names the purls and the migration path (`remove `, or `vendor --revert` which unwinds every vendored package, then re-run). `hosted_wiring_retained` — the lockfiles still pin scanned package(s) to a hosted patch (the agent run does not unwind hosted wiring — as of v5.0 that is `socket-patch rollback`'s job, which restores the upstream registry entries, or `remove ` per package); the detail names the purls and the options (stay `--mode hosted`, migrate via `scan --mode vendored`, or `socket-patch rollback`). The warning keys on the hosted pins lockfile discovery finds at scan time, so a flow that restored the upstream entries retires it. The human path prints the same `hosted_wiring_retained` text to stderr after an apply; the vendored counterpart is already covered by its per-package `[skip] … (vendored …)` lines. `ownership_not_restored` (v5.0; `apply` and `rollback` `warnings[]` alike) — a file WAS patched (or restored) but its ownership could not be put back to the original uid/gid (the mode is still restored last); the detail is `: : patched, but ownership could not be restored to uid N gid M: ` and the human line `Warning: ` (stderr, muted by `--silent`); never a status or exit change. `cargo_build_cache_stale` (v5.0; `apply` and `rollback` `warnings[]` alike) — a cargo crate's bytes changed but its compiled copy in a project build directory could not be invalidated (a fingerprint directory that could not be removed, or a `build.build-dir` with a `{workspace-path-hash}` template); the detail names the crates to `cargo clean -p` before the next build, which may otherwise link the stale code; never a status or exit change. +**Agent-flow run-level warnings (additive).** An agent-mode apply (`--mode agent` / `--sync`, `--json`) may add a top-level `warnings[]` array of `{code, detail}` entries to the scan envelope (absent when none fired; each is also mirrored to stderr unless `--silent`). They surface cross-mode state the apply cannot change — never a status or exit-code change (hosted refusals set the precedent: exit 0 + warning). Codes (stable; new codes are additive/MINOR): `vendored_ownership_retained` — vendor-owned package(s) were skipped before download (each is also a `skipped` / `vendored` event); the detail names the purls and the migration path (`remove `, or `vendor --revert` which unwinds every vendored package, then re-run). `hosted_wiring_retained` — the lockfiles still pin scanned package(s) to a hosted patch (the agent run does not unwind hosted wiring — as of v5.0 that is `socket-patch rollback`'s job, which restores the upstream registry entries, or `remove ` per package); the detail names the purls and the options (stay `--mode hosted`, migrate via `scan --mode vendored`, or `socket-patch rollback`). The warning keys on the hosted pins lockfile discovery finds at scan time, so a flow that restored the upstream entries retires it. The human path prints the same `hosted_wiring_retained` text to stderr after an apply; the vendored counterpart is already covered by its per-package `[skip] … (vendored …)` lines. `ownership_not_restored` (v5.0; `apply` and `rollback` `warnings[]` alike) — a file WAS patched (or restored) but its ownership could not be put back to the original uid/gid (the mode is still restored last); the detail is `: : patched, but ownership could not be restored to uid N gid M: ` and the human line `Warning: ` (stderr, muted by `--silent`); never a status or exit change. `go_installed_tool_stale` (v5.0; `apply`) — a `--global` Go apply patched a module cache copy that a tool installed with `go install` was built from before the patch; the detail names the binary and the `go install @` remedy (or, when `go` is not on `PATH`, the directories that could not be checked); never a status or exit change. `cargo_build_cache_stale` (v5.0; `apply` and `rollback` `warnings[]` alike) — a cargo crate's bytes changed but its compiled copy in a project build directory could not be invalidated (a fingerprint directory that could not be removed, or a `build.build-dir` with a `{workspace-path-hash}` template); the detail names the crates to `cargo clean -p` before the next build, which may otherwise link the stale code; never a status or exit change. `scan --prune` opts into garbage collection. When set, `scan` removes manifest entries for packages no longer present in the crawl, then deletes orphan blob files, and every obsolete diff and package archive, from `.socket/`. A blob is an orphan when no patch left in the manifest references it: the afterHash and beforeHash blobs of every remaining patch are kept, the same retention policy `repair` uses (the beforeHash blobs are an offline rollback's only restore data, and `repair` downloads afterHash blobs only). Diff and legacy package archives are never kept, even for a remaining patch: nothing reads them any more. Off by default (v3.0) so a temporary uninstall doesn't silently destroy manifest state. Only entries whose ecosystem this run actually crawled are eligible: a `pkg:/` with no crawler in this build (a newer CLI's ecosystem in the committed manifest) is exempt — the crawl never looked for them, so their absence is not evidence of removal (same fail-safe as the `--ecosystems` filter, which narrows the query but never the prune's installed set). A Cargo entry is also exempt while its agent-mode copy is still patched: the project crawl looks up only the crates `Cargo.lock` resolves, but a crate the lock bumped or dropped keeps its patched copy in the machine-wide `$CARGO_HOME/registry/src` cache (nothing deletes it), and the entry holds the only blobs that can restore that copy. The wet pass keeps it with a `cargo_cache_patch_kept` warning naming the `socket-patch rollback ` that restores the copy and drops the entry; the `--dry-run` preview reports no event (and no warning) for it. The pass also reconciles vendored state (runs FIRST, under ONE apply-lock acquisition shared with the manifest prune — lock contention skips the whole pass without failing the scan; `--lock-timeout` is honored and a lock I/O error is reported rather than swallowed; the existence gate — a manifest file OR a vendor ledger file, both cheap stats; an emptied ledger is deleted on save, so its presence is its content proxy — runs BEFORE the lock, so a bare project never gets a `.socket/`; in the vendored scan arms the pass runs AFTER the vendor step): (a) ledger entries still tracked by a manifest record (manifest-mode entries written by standalone `vendor`) whose patch is gone from the manifest are reverted — `detached` entries (every `scan`/`get --mode vendored` entry, v5.0) have no manifest record to lose and are exempt from this leg; (b) EVERY ledger entry whose dependency is no longer in the lockfile graph is reverted and any manifest entry it still had dropped (v5.0: the check is about the lockfile, not the manifest, so embedded-record entries are no longer exempt; a missing or undeterminable lockfile keeps the entry, fail-safe); and (c) orphan `.socket/vendor//` dirs with no ledger entry are swept. The prune never deletes a zero-patch `.socket/manifest.json` (its `{"patches": {}}` + `setup` block stay). Under `--json` (v5.0, MAJOR) the pass is recorded into the scan envelope: each pruned manifest entry is a `removed` event with `details.manifest: true`, each reverted vendored entry a `removed` event (`errorCode: "vendor_reverted"`, `details.mode: "vendored"`), each drift-kept entry a `skipped` / `vendor_revert_kept` event and each entry whose revert failed a `skipped` / `vendor_revert_failed` event (a GC revert failure never fails the run); a `--dry-run` preview reports the removals as `verified`. The artifact sweep is the envelope's `gc` (`summary.bytesFreed` mirrors it) and the orphan vendor dirs swept are the top-level `removedVendorOrphanDirs` count. The pass's warnings join the top-level `warnings[]`: `vendor_state_write_failed` / `manifest_write_failed` (entries were reverted but the ledger or manifest rewrite failed), `cleanup_failed` (an orphan sweep failed mid-way), `cargo_cache_patch_kept` (see above), and the reinstall advisories of the vendored reverts (`vendor_bun_reinstall_required`, `vendor_vlt_reinstall_required`, `vendor_pypi_reinstall_required`; a revert's other warnings, such as `vendor_lock_entry_removed` or a drift keep's, are not repeated). A pass skipped at the lock (`lock_held` | `lock_io`) has no `gc` and no events, only a `gc_skipped` warning whose detail ends with the code. Human mode prints `GC: skipped: .`, one `GC: .` line per warning, and `GC: failed to revert N vendored entries: …` (singular for one). A drift keep is an entry the revert deliberately preserved (`vendor_artifact_kept` — undo the drift and re-run `vendor --revert` to finish); the preview cannot see drift (backends return before the wiring replay on dry runs), so its `verified` reverts may over-promise what a wet run will actually reclaim. @@ -659,6 +659,14 @@ are patched by `socket-patch apply` like every other ecosystem: would be ignored. One already written (a later `go get` bump) fails `apply --check` with a resolved-version mismatch, and `vex` omits the patch, agent, vendored or hosted, with `go_version_not_selected` (#391, #392, #509). + With `--global` / `--global-prefix` there is no project to redirect: `apply` patches the module + cache copy in place, which changes only what later builds compile. A tool already built from the + unpatched module with `go install` (in `$GOBIN`, else each `GOPATH` entry's `bin/`, or `/bin` + for a cache at `/pkg/mod`) keeps running that code, so `apply` adds a + `go_installed_tool_stale` warning per tool naming its `go install @` remedy, and + `vex -g` omits the patch with `go_installed_tool_stale` until every such tool is newer than the + patched files. Without `go` on `PATH` the tools can't be inspected: the same warning code says so, + and VEX is not withheld (#422). ### sbt / Mill / scala-cli caches in agent mode diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 1f915eb2f..0649a1dd0 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -1394,6 +1394,19 @@ pub(crate) async fn run_locked( .into_iter() .map(|(code, detail)| RunWarning { code, detail }), ); + // A global Go apply patches the module cache in place: a + // tool already built from it with `go install` keeps the + // unpatched code until it is rebuilt (#422). + if args.common.is_global() { + let (patched, dirs) = rewritten_go_modules(&results); + let (warnings, _) = + socket_patch_core::utils::go_installed_tools::audit(patched, dirs).await; + cache_warnings.extend( + warnings + .into_iter() + .map(|(code, detail)| RunWarning { code, detail }), + ); + } run_warnings.extend(cache_warnings.iter().cloned()); } @@ -2793,6 +2806,40 @@ async fn apply_patches_inner( }) } +/// The Go modules a (global, in-place) apply rewrote in the module cache, +/// and their cache copies, for the installed-tool check (#422). +fn rewritten_go_modules( + results: &[ApplyResult], +) -> ( + Vec, + Vec, +) { + use socket_patch_core::utils::go_installed_tools::{patched_at, PatchedModule}; + let mut patched = Vec::new(); + let mut dirs = Vec::new(); + for r in results { + if !r.success || r.files_patched.is_empty() || r.package_path == VENDOR_OWNED_MARKER { + continue; + } + if Ecosystem::from_purl(&r.package_key) != Some(Ecosystem::Golang) { + continue; + } + let Some((module, version)) = parse_golang_purl(&r.package_key) else { + continue; + }; + let dir = PathBuf::from(&r.package_path); + let at = patched_at(&dir, r.files_patched.iter().map(String::as_str)) + .unwrap_or_else(std::time::SystemTime::now); + patched.push(PatchedModule { + module: module.into_owned(), + version: version.into_owned(), + patched_at: at, + }); + dirs.push(dir); + } + (patched, dirs) +} + /// One Maven base purl for [`apply_maven_base`]. struct MavenBase<'a> { args: &'a ApplyArgs, diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index f3c399bff..427ca5d8b 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -889,6 +889,9 @@ async fn generate_vex( warnings, ) .await; + if common.is_global() { + withhold_stale_go_tools(&mut outcome, manifest, &copies, common, warnings).await; + } outcome }; @@ -1403,6 +1406,63 @@ async fn withhold_unpatched_jvm_copies( } } +/// A global Go patch rewrites the module cache in place, so a tool built +/// from it with `go install` before the patch still runs the unpatched code +/// however intact the cache copy is (#422): never attest it while such a +/// tool remains. `go` missing from `PATH` (nothing can be checked) is said, +/// not withheld. +async fn withhold_stale_go_tools( + outcome: &mut VerifyOutcome, + manifest: &PatchManifest, + copies: &HashMap>, + common: &GlobalArgs, + warnings: &mut Vec, +) { + use socket_patch_core::utils::go_installed_tools::{audit, patched_at, PatchedModule}; + use socket_patch_core::utils::purl::parse_golang_purl; + let mut patched = Vec::new(); + let mut dirs = Vec::new(); + for purl in &outcome.applied { + let (Some(record), Some(paths), Some((module, version))) = ( + manifest.patches.get(purl), + copies.get(purl), + parse_golang_purl(purl), + ) else { + continue; + }; + for dir in paths { + let Some(at) = patched_at(dir, record.files.keys().map(String::as_str)) else { + continue; + }; + patched.push(PatchedModule { + module: module.to_string(), + version: version.to_string(), + patched_at: at, + }); + dirs.push(dir.clone()); + } + } + let (notes, stale) = audit(patched, dirs).await; + for (code, detail) in notes { + note_warning(warnings, common, &code, detail); + } + let stale: HashSet<(String, String)> = + stale.into_iter().map(|t| (t.module, t.version)).collect(); + let (withheld, kept): (Vec, Vec) = std::mem::take(&mut outcome.applied) + .into_iter() + .partition(|purl| { + parse_golang_purl(purl) + .is_some_and(|(m, v)| stale.contains(&(m.into_owned(), v.into_owned()))) + }); + outcome.applied = kept; + outcome + .failed + .extend(withheld.into_iter().map(|purl| FailedPatch { + purl, + reason: GO_INSTALLED_TOOL_STALE.to_string(), + })); +} + /// Record a run-level advisory the way `update`/`vendor` do: stderr /// (`Warning: `) in human mode, and into `warnings` so the `--json` /// envelope — which silences stderr — carries it in `warnings[]` instead. @@ -1782,6 +1842,10 @@ fn join_and(items: &[&str]) -> String { /// overrides, ignores or rejects (#393, #458, #531). const GO_WORKSPACE_OVERRIDE: &str = "go_workspace_override"; +/// Omission tag for a global Go patch while a tool built from the unpatched +/// module with `go install` remains (#422). +const GO_INSTALLED_TOOL_STALE: &str = socket_patch_core::utils::go_installed_tools::STALE_WARNING; + /// Omission tag for a Go patch whose version-pinned replace names a version /// the build list does not select (#391, #392, #509). const GO_VERSION_NOT_SELECTED: &str = "go_version_not_selected"; @@ -1944,6 +2008,10 @@ fn omission_phrase(reason: &str) -> &'static str { "the go.work in effect overrides, ignores or conflicts with its go.mod replace, \ so the build does not link the patched module" } + GO_INSTALLED_TOOL_STALE => { + "a tool installed with `go install` was built from the unpatched module and keeps \ + running it until it is rebuilt" + } GO_VERSION_NOT_SELECTED => { "go.mod and go.sum show the build selects another version of the module, so its \ version-pinned replace is ignored and the build links the unpatched module" diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index ed0c08425..5ea1af61c 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -982,3 +982,135 @@ fn go_work_member_replaces_must_not_conflict() { chmod_writable(tmp.path()); } + +// ── global mode: tools already built with `go install` (#422) ── + +const TOOL: &str = "example.com/tool"; +const TOOL_PURL: &str = "pkg:golang/example.com/tool@v1.0.0"; + +/// Run the `go install`ed tool and return its trimmed output. +fn run_tool(gobin: &Path) -> String { + let out = Command::new(gobin.join("tool")).output().expect("run tool"); + String::from_utf8_lossy(&out.stdout).trim().to_string() +} + +/// `go install example.com/tool@v1.0.0` into `gobin`, from the file proxy. +fn install_tool(tmp: &Path, modcache: &str, proxy_url: &str, gobin: &Path) { + let mut env = go_env(modcache, proxy_url); + // `go install pkg@version` refuses `-mod=mod`. + env.retain(|(k, _)| *k != "GOFLAGS"); + env.push(("GOBIN", gobin.to_str().unwrap())); + let out = go(tmp, &["install", &format!("{TOOL}@{UVER}")], &env); + assert!( + out.status.success(), + "go install: {}", + String::from_utf8_lossy(&out.stderr) + ); +} + +/// `vex -g --json` over `dir` for the installed tool's product. +fn global_vex(dir: &Path, modcache: &Path, gobin: &Path) -> vex_e2e_common::VexOutcome { + let r = VexRun { + offline: true, + product: Some(TOOL_PURL.into()), + extra_args: vec!["--global".into()], + ..VexRun::default() + } + .env("GOMODCACHE", modcache) + .env("GOBIN", gobin) + .env("GOFLAGS", ""); + run_vex(&vex_e2e_common::binary(), dir, &r) +} + +/// #422: a global apply patches the module cache in place, which a tool +/// already built from it with `go install` never sees. apply names the +/// tool and its `go install` remedy, and `vex -g` refuses to attest until +/// the tool is rebuilt — after which it runs the patched code and attests. +#[test] +fn global_apply_flags_tools_built_from_the_unpatched_module() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(global-go-install)") { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let (_consumer, modcache, proxy_url) = stage(tmp.path()); + let mc = modcache.to_str().unwrap(); + publish( + tmp.path(), + TOOL, + UVER, + &[ + ( + "go.mod", + &format!("module {TOOL}\n\ngo 1.21\n\nrequire {UMOD} {UVER}\n"), + ), + ( + "main.go", + &format!( + "package main\n\nimport (\n\t\"fmt\"\n\t\"{UMOD}\"\n)\n\nfunc main() {{ fmt.Println(\"TOOL:\", upstream.Greeting()) }}\n" + ), + ), + ], + ); + let gobin = tmp.path().join("gobin"); + install_tool(tmp.path(), mc, &proxy_url, &gobin); + assert_eq!(run_tool(&gobin), "TOOL: PRISTINE"); + + // Outside any Go project: the manifest is the only project state. + let outside = tmp.path().join("outside"); + std::fs::create_dir_all(&outside).unwrap(); + write_patch(&outside); + let (code, stdout, stderr) = common::run_with_env( + &outside, + &[ + "apply", + "--global", + "--offline", + "--ecosystems", + "golang", + "--json", + "--cwd", + outside.to_str().unwrap(), + ], + &[("GOMODCACHE", mc), ("GOBIN", gobin.to_str().unwrap())], + ); + assert_eq!(code, 0, "apply -g: {stdout}\n{stderr}"); + let lib = modcache.join(format!("{UMOD}@{UVER}")).join("lib.go"); + assert_eq!(std::fs::read_to_string(&lib).unwrap(), PATCHED_LIB); + // The cache is patched; the installed binary is not. + assert_eq!(run_tool(&gobin), "TOOL: PRISTINE"); + let env = common::envelope::parse_json_envelope(&stdout); + let stale: Vec<&serde_json::Value> = env["warnings"] + .as_array() + .into_iter() + .flatten() + .filter(|w| w["code"] == "go_installed_tool_stale") + .collect(); + assert_eq!(stale.len(), 1, "{stdout}"); + let detail = stale[0]["detail"].as_str().unwrap(); + assert!( + detail.contains(&format!("go install {TOOL}@{UVER}")) + && detail.contains(&format!("{UMOD}@{UVER}")), + "{detail}" + ); + + let out = global_vex(&outside, &modcache, &gobin); + assert_absent(out.doc.as_ref(), UPURL); + vex_e2e_common::assert_not_attested(&out.envelope, UPURL, "go_installed_tool_stale"); + assert!( + warnings_of(&out.stdout).contains(&"go_installed_tool_stale".to_string()), + "{out}" + ); + + // The remedy the warning names rebuilds the tool from the patched cache. + install_tool(tmp.path(), mc, &proxy_url, &gobin); + assert_eq!(run_tool(&gobin), "TOOL: PATCHED"); + let out = global_vex(&outside, &modcache, &gobin); + assert!( + !warnings_of(&out.stdout).contains(&"go_installed_tool_stale".to_string()), + "{out}" + ); + assert_eq!(out.code, Some(0), "{out}"); + assert_attested(out.doc(), UPURL, UUID, Marker::Applied, &[(GHSA, &[CVE])]); + + chmod_writable(&modcache); +} diff --git a/crates/socket-patch-core/src/utils/go_installed_tools.rs b/crates/socket-patch-core/src/utils/go_installed_tools.rs new file mode 100644 index 000000000..096ccd898 --- /dev/null +++ b/crates/socket-patch-core/src/utils/go_installed_tools.rs @@ -0,0 +1,455 @@ +//! Go tools already built from a module a global apply patched in place +//! (#422). +//! +//! `--global` / `--global-prefix` patches `$GOMODCACHE/@/` +//! in place. That changes what the NEXT build compiles; a tool installed +//! earlier with `go install pkg@version` is a statically linked binary that +//! keeps the vulnerable code until it is rebuilt. Go records the module +//! versions a binary was built from in its build info, so +//! `go version -m ` names every installed tool that embeds a patched +//! `module@version`. A binary older than the patched files was built from +//! the unpatched bytes; `go install` of the same version rebuilds it from +//! the patched cache (Go's build cache keys on source content). +//! +//! Only the directories `go install` writes to are searched: `$GOBIN`, else +//! `bin/` of every `GOPATH` entry (default `~/go`), plus `/bin` for a +//! module cache at `/pkg/mod` (a `--global-prefix` tree). A binary +//! copied elsewhere is out of reach; the warning says so. + +use std::path::{Path, PathBuf}; +use std::time::{Duration, SystemTime}; + +/// Run-warning code, and the VEX omission reason for the same condition. +pub const STALE_WARNING: &str = "go_installed_tool_stale"; + +/// A `go version -m` budget per directory: it only reads files. +const VERSION_BUDGET: Duration = Duration::from_secs(60); + +/// One Go binary's build info, as `go version -m` prints it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GoBinary { + pub path: PathBuf, + /// The main package path (`path` line), e.g. `example.com/tool/cmd/x`. + pub main_package: Option, + /// The main module's version (`mod` line); `(devel)` for a local build. + pub main_version: Option, + /// Every `(module, version)` the binary compiled from the module cache: + /// the main module and each dependency, after `=>` replacements. + pub modules: Vec<(String, String)>, +} + +/// A module whose cache copy a global apply rewrote. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PatchedModule { + pub module: String, + pub version: String, + /// When the patched files were written: a binary older than this was + /// built from the unpatched bytes. + pub patched_at: SystemTime, +} + +/// An installed tool still running the unpatched module. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct StaleTool { + pub binary: PathBuf, + pub module: String, + pub version: String, + /// `go install @`, or `None` for a binary built from a + /// local checkout (no version to reinstall). + pub reinstall: Option, +} + +impl StaleTool { + /// The warning detail: which tool, why, and the remedy. + pub fn detail(&self) -> String { + let remedy = match &self.reinstall { + Some(cmd) => format!("run `{cmd}` to rebuild it"), + None => "rebuild it from its source".to_string(), + }; + format!( + "{} was built with `go install` from the unpatched {}@{} and keeps running that \ + code; patching the module cache only changes future builds: {remedy}", + self.binary.display(), + self.module, + self.version + ) + } +} + +/// The warning detail when `go` itself can't be run to inspect `dirs`. +pub fn unchecked_detail(patched: &[PatchedModule], dirs: &[PathBuf]) -> String { + let modules: Vec = patched + .iter() + .map(|p| format!("{}@{}", p.module, p.version)) + .collect(); + let dirs: Vec = dirs.iter().map(|d| d.display().to_string()).collect(); + format!( + "could not run `go version -m` to check the tools installed in {} for {}; a tool \ + built with `go install` before this patch keeps running the unpatched code until \ + you rebuild it with `go install @`", + dirs.join(", "), + modules.join(", ") + ) +} + +/// The newest modification time among `files` under `dir` (the patched +/// files of a module cache copy), or `None` when none can be read. +pub fn patched_at<'a>(dir: &Path, files: impl IntoIterator) -> Option { + files + .into_iter() + .filter_map(|f| std::fs::metadata(dir.join(f)).ok()?.modified().ok()) + .max() +} + +/// The directories `go install` writes to, for module cache copies at +/// `module_dirs`, reading the process environment. +pub fn bin_dirs(module_dirs: &[&Path]) -> Vec { + bin_dirs_with( + module_dirs, + &crate::utils::go_env::go_env, + crate::utils::fs::home_dir(), + ) +} + +/// [`bin_dirs`] with an injected Go setting lookup and home directory. +pub fn bin_dirs_with( + module_dirs: &[&Path], + go_env: &dyn Fn(&str) -> Option, + home: Option, +) -> Vec { + let mut dirs: Vec = Vec::new(); + let mut push = |p: PathBuf| { + if p.is_absolute() && !dirs.contains(&p) { + dirs.push(p); + } + }; + if let Some(gobin) = go_env("GOBIN").filter(|v| !v.is_empty()) { + push(PathBuf::from(gobin)); + } else { + match go_env("GOPATH").filter(|v| !v.is_empty()) { + Some(gopath) => std::env::split_paths(&gopath) + .filter(|p| !p.as_os_str().is_empty()) + .for_each(|p| push(p.join("bin"))), + None => { + if let Some(home) = home { + push(home.join("go").join("bin")); + } + } + } + } + // A cache at `/pkg/mod/@`: `` is the + // GOPATH that `go install` used with it. + for dir in module_dirs { + if let Some(root) = dir + .ancestors() + .find(|a| a.ends_with(Path::new("pkg").join("mod"))) + .and_then(|m| m.parent()?.parent()) + { + push(root.join("bin")); + } + } + dirs +} + +/// Parse `go version -m` output: one `: ` header per +/// binary, then tab-indented `path` / `mod` / `dep` / `=>` / `build` lines. +pub fn parse_version_m(output: &str) -> Vec { + let mut out: Vec = Vec::new(); + for line in output.lines() { + let Some(rest) = line.strip_prefix('\t') else { + // `: go1.x` — the path itself may contain `: `, the Go + // version never does. + if let Some((path, _go)) = line.rsplit_once(": ") { + out.push(GoBinary { + path: PathBuf::from(path), + main_package: None, + main_version: None, + modules: Vec::new(), + }); + } + continue; + }; + let Some(bin) = out.last_mut() else { + continue; + }; + let fields: Vec<&str> = rest.split('\t').collect(); + match fields.as_slice() { + ["path", pkg, ..] => bin.main_package = Some(pkg.to_string()), + ["mod", module, version, ..] => { + bin.main_version = Some(version.to_string()); + bin.modules.push((module.to_string(), version.to_string())); + } + ["dep", module, version, ..] => { + bin.modules.push((module.to_string(), version.to_string())) + } + // The module on the line before was replaced: the binary + // compiled the replacement, never the original. A directory + // replacement (empty version) never matches a patched module. + ["=>", module, version, ..] => { + bin.modules.pop(); + bin.modules.push((module.to_string(), version.to_string())); + } + ["=>", ..] => { + bin.modules.pop(); + } + _ => {} + } + } + out +} + +/// The installed Go binaries under `dirs`, or `None` when `go` is not on +/// `PATH` (nothing can be checked). Missing directories are skipped. +/// Blocking. +pub fn installed_binaries(dirs: &[PathBuf]) -> Option> { + let go = crate::utils::process::resolve_tool("go")?; + let mut out = Vec::new(); + for dir in dirs.iter().filter(|d| d.is_dir()) { + let mut cmd = crate::utils::process::command_for(&go); + // `go version` reads files only; keep a `toolchain` line in an + // ambient go.mod or GOTOOLCHAIN from fetching another toolchain. + cmd.arg("version") + .arg("-m") + .arg(dir) + .current_dir(dir) + .env("GOTOOLCHAIN", "local"); + let Ok(output) = crate::utils::process::output_within(cmd, VERSION_BUDGET) else { + continue; + }; + out.extend(parse_version_m(&String::from_utf8_lossy(&output.stdout))); + } + Some(out) +} + +/// Every installed tool in `binaries` built from one of `patched` before it +/// was patched. +pub fn stale_tools(binaries: &[GoBinary], patched: &[PatchedModule]) -> Vec { + stale_tools_with(binaries, patched, &|p| { + std::fs::metadata(p).ok()?.modified().ok() + }) +} + +/// [`stale_tools`] with an injected binary mtime lookup. +pub fn stale_tools_with( + binaries: &[GoBinary], + patched: &[PatchedModule], + built_at: &dyn Fn(&Path) -> Option, +) -> Vec { + let mut out = Vec::new(); + for bin in binaries { + for p in patched { + if !bin + .modules + .iter() + .any(|(m, v)| *m == p.module && *v == p.version) + { + continue; + } + // An unreadable mtime is no evidence of a rebuild. + if built_at(&bin.path).is_some_and(|t| t > p.patched_at) { + continue; + } + let reinstall = match (&bin.main_package, &bin.main_version) { + (Some(pkg), Some(ver)) if ver.starts_with('v') => { + Some(format!("go install {pkg}@{ver}")) + } + _ => None, + }; + out.push(StaleTool { + binary: bin.path.clone(), + module: p.module.clone(), + version: p.version.clone(), + reinstall, + }); + } + } + out +} + +/// Check the installed tools for every patched module, whose cache copies +/// are `module_dirs`: the run warnings `(code, detail)` to report, and the +/// stale tools themselves. +pub async fn audit( + patched: Vec, + module_dirs: Vec, +) -> (Vec<(String, String)>, Vec) { + if patched.is_empty() { + return (Vec::new(), Vec::new()); + } + crate::utils::fs::run_blocking(move || { + let dirs: Vec<&Path> = module_dirs.iter().map(PathBuf::as_path).collect(); + audit_sync(&patched, &bin_dirs(&dirs)) + }) + .await +} + +/// [`audit`] over the given `go install` directories. Blocking. +pub fn audit_sync( + patched: &[PatchedModule], + dirs: &[PathBuf], +) -> (Vec<(String, String)>, Vec) { + match installed_binaries(dirs) { + Some(binaries) => { + let stale = stale_tools(&binaries, patched); + let warnings = stale + .iter() + .map(|t| (STALE_WARNING.to_string(), t.detail())) + .collect(); + (warnings, stale) + } + None if dirs.iter().any(|d| dir_has_entries(d)) => ( + vec![(STALE_WARNING.to_string(), unchecked_detail(patched, dirs))], + Vec::new(), + ), + None => (Vec::new(), Vec::new()), + } +} + +fn dir_has_entries(dir: &Path) -> bool { + std::fs::read_dir(dir).is_ok_and(|mut rd| rd.next().is_some()) +} + +#[cfg(test)] +mod tests { + use super::*; + + const OUTPUT: &str = "\ +/home/u/go/bin/tool: go1.24.7 +\tpath\texample.com/tool +\tmod\texample.com/tool\tv1.0.0\th1:abc= +\tdep\texample.com/upstream\tv1.0.0\th1:def= +\tdep\texample.com/other\tv0.3.0\th1:ghi= +\t=>\texample.com/fork\tv0.3.1\th1:jkl= +\tdep\texample.com/local\tv1.2.0\t +\t=>\t../local\t\t +\tbuild\t-compiler=gc +/home/u/go/bin/dev: go1.24.7 +\tpath\texample.com/dev/cmd/dev +\tmod\texample.com/dev\t(devel)\t +\tdep\texample.com/upstream\tv1.0.0\th1:def= +"; + + fn bins() -> Vec { + parse_version_m(OUTPUT) + } + + fn patched(module: &str, version: &str, at: u64) -> PatchedModule { + PatchedModule { + module: module.into(), + version: version.into(), + patched_at: SystemTime::UNIX_EPOCH + Duration::from_secs(at), + } + } + + #[test] + fn parses_build_info_with_replacements() { + let b = bins(); + assert_eq!(b.len(), 2); + assert_eq!(b[0].path, PathBuf::from("/home/u/go/bin/tool")); + assert_eq!(b[0].main_package.as_deref(), Some("example.com/tool")); + assert_eq!(b[0].main_version.as_deref(), Some("v1.0.0")); + assert_eq!( + b[0].modules, + vec![ + ("example.com/tool".into(), "v1.0.0".into()), + ("example.com/upstream".into(), "v1.0.0".into()), + ("example.com/fork".into(), "v0.3.1".into()), + ("../local".into(), "".into()), + ] + ); + assert_eq!(b[1].main_version.as_deref(), Some("(devel)")); + } + + #[test] + fn a_tool_built_before_the_patch_is_stale_with_its_reinstall_command() { + let at = |_: &Path| Some(SystemTime::UNIX_EPOCH + Duration::from_secs(100)); + let stale = stale_tools_with( + &bins(), + &[patched("example.com/upstream", "v1.0.0", 200)], + &at, + ); + assert_eq!(stale.len(), 2); + assert_eq!( + stale[0].reinstall.as_deref(), + Some("go install example.com/tool@v1.0.0") + ); + assert!(stale[0] + .detail() + .contains("run `go install example.com/tool@v1.0.0`")); + // A local build has no version to reinstall. + assert_eq!(stale[1].reinstall, None); + assert!(stale[1].detail().contains("rebuild it from its source")); + } + + #[test] + fn a_tool_rebuilt_after_the_patch_is_not_stale() { + let at = |_: &Path| Some(SystemTime::UNIX_EPOCH + Duration::from_secs(300)); + assert!(stale_tools_with( + &bins(), + &[patched("example.com/upstream", "v1.0.0", 200)], + &at + ) + .is_empty()); + } + + #[test] + fn only_the_patched_version_and_unreplaced_modules_match() { + let at = |_: &Path| None; + let b = bins(); + assert!( + stale_tools_with(&b, &[patched("example.com/upstream", "v1.0.1", 1)], &at).is_empty() + ); + // Replaced away: the binary never compiled example.com/other. + assert!(stale_tools_with(&b, &[patched("example.com/other", "v0.3.0", 1)], &at).is_empty()); + assert_eq!( + stale_tools_with(&b, &[patched("example.com/fork", "v0.3.1", 1)], &at).len(), + 1 + ); + // The main module itself. + assert_eq!( + stale_tools_with(&b, &[patched("example.com/tool", "v1.0.0", 1)], &at).len(), + 1 + ); + } + + #[cfg(unix)] + #[test] + fn bin_dirs_follow_gobin_then_gopath_then_home_and_the_cache_root() { + let home = Some(PathBuf::from("/home/u")); + let cache = PathBuf::from("/opt/gp/pkg/mod/example.com/upstream@v1.0.0"); + let env = |vars: Vec<(&'static str, &'static str)>| { + move |k: &str| { + vars.iter() + .find(|(n, _)| *n == k) + .map(|(_, v)| v.to_string()) + } + }; + assert_eq!( + bin_dirs_with(&[&cache], &env(vec![("GOBIN", "/gobin")]), home.clone()), + vec![PathBuf::from("/gobin"), PathBuf::from("/opt/gp/bin")] + ); + assert_eq!( + bin_dirs_with(&[], &env(vec![("GOPATH", "/a:/b")]), home.clone()), + vec![PathBuf::from("/a/bin"), PathBuf::from("/b/bin")] + ); + assert_eq!( + bin_dirs_with( + &[Path::new("/home/u/go/pkg/mod/x@v1.0.0")], + &env(Vec::new()), + home + ), + vec![PathBuf::from("/home/u/go/bin")] + ); + } + + #[test] + fn unchecked_detail_names_dirs_modules_and_remedy() { + let d = unchecked_detail( + &[patched("example.com/upstream", "v1.0.0", 1)], + &[PathBuf::from("/home/u/go/bin")], + ); + assert!(d.contains("/home/u/go/bin")); + assert!(d.contains("example.com/upstream@v1.0.0")); + assert!(d.contains("go install @")); + } +} diff --git a/crates/socket-patch-core/src/utils/mod.rs b/crates/socket-patch-core/src/utils/mod.rs index 127f2c121..027373f61 100644 --- a/crates/socket-patch-core/src/utils/mod.rs +++ b/crates/socket-patch-core/src/utils/mod.rs @@ -9,6 +9,7 @@ pub mod env_compat; pub mod failpoint; pub mod fs; pub(crate) mod go_env; +pub mod go_installed_tools; pub mod group_commit; pub(crate) mod http; pub(crate) mod line_endings; diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 60a2a8fce..34c5759a4 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -1415,6 +1415,27 @@ 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. +### Global mode: tools built with `go install` + +With `--global` / `--global-prefix` there is no project to redirect, so `apply` +patches the module cache copy (`$GOMODCACHE/@/`) in place. That +changes what the next build compiles, and nothing else: a tool installed earlier +with `go install pkg@version` is a static binary that keeps running the unpatched +code. After a global apply, socket-patch reads the build info of the binaries in +`$GOBIN` (else `bin/` of each `GOPATH` entry, default `~/go/bin`, plus +`/bin` for a cache at `/pkg/mod`) with `go version -m`, and warns +`go_installed_tool_stale` for each one built from a patched module before it was +patched, with the command that rebuilds it: + +```sh +go install example.com/tool@v1.0.0 # rebuilds from the patched cache +``` + +`vex -g` omits the patch with the same code while such a tool remains. A binary +copied out of those directories can't be found; rebuild it the same way. While +the cache copy stays patched, `go mod verify` reports `dir has been modified` +for it until `rollback -g`. + ### Files go derives from go.mod Two files the go command derives from `go.mod` can disagree with a new or removed From 88158957e0d489646c375f3b8877d290be028278 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 18:59:32 +0000 Subject: [PATCH 3/5] Harden the go install staleness check - Date a patched copy by its record files with the API's package/ prefix stripped, so real API patches are checked by vex too. - Say so when `go version -m` fails or times out on a directory, instead of reporting a clean check. - Search the user's own GOBIN / GOPATH bin only for their own module cache, not for a --global-prefix tree; use the outermost pkg/mod. - Keep warning on re-runs over an already-patched cache. - Offer no `go install` command for a +dirty local build. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/src/commands/apply.rs | 36 ++- crates/socket-patch-cli/src/commands/vex.rs | 24 +- .../tests/e2e_golang_build.rs | 60 +++-- .../src/utils/go_installed_tools.rs | 234 +++++++++++++----- 4 files changed, 232 insertions(+), 122 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 0649a1dd0..7cb1de0d1 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -1396,9 +1396,10 @@ pub(crate) async fn run_locked( ); // A global Go apply patches the module cache in place: a // tool already built from it with `go install` keeps the - // unpatched code until it is rebuilt (#422). + // unpatched code until it is rebuilt (#422). An embedded + // `--vex` checks again and says so itself. if args.common.is_global() { - let (patched, dirs) = rewritten_go_modules(&results); + let (patched, dirs) = patched_go_modules(&results); let (warnings, _) = socket_patch_core::utils::go_installed_tools::audit(patched, dirs).await; cache_warnings.extend( @@ -2806,36 +2807,29 @@ async fn apply_patches_inner( }) } -/// The Go modules a (global, in-place) apply rewrote in the module cache, -/// and their cache copies, for the installed-tool check (#422). -fn rewritten_go_modules( +/// The Go modules a (global, in-place) apply left patched in the module +/// cache, and their cache copies, for the installed-tool check (#422). +/// Already-patched copies count too: a tool built before an earlier run +/// is just as stale. +fn patched_go_modules( results: &[ApplyResult], ) -> ( Vec, Vec, ) { - use socket_patch_core::utils::go_installed_tools::{patched_at, PatchedModule}; + use socket_patch_core::utils::go_installed_tools::PatchedModule; let mut patched = Vec::new(); let mut dirs = Vec::new(); for r in results { - if !r.success || r.files_patched.is_empty() || r.package_path == VENDOR_OWNED_MARKER { + if !r.success || r.package_path == VENDOR_OWNED_MARKER { continue; } - if Ecosystem::from_purl(&r.package_key) != Some(Ecosystem::Golang) { - continue; - } - let Some((module, version)) = parse_golang_purl(&r.package_key) else { - continue; - }; let dir = PathBuf::from(&r.package_path); - let at = patched_at(&dir, r.files_patched.iter().map(String::as_str)) - .unwrap_or_else(std::time::SystemTime::now); - patched.push(PatchedModule { - module: module.into_owned(), - version: version.into_owned(), - patched_at: at, - }); - dirs.push(dir); + let files = r.files_verified.iter().map(|f| f.file.as_str()); + if let Some(module) = PatchedModule::of_copy(&r.package_key, &dir, files) { + patched.push(module); + dirs.push(dir); + } } (patched, dirs) } diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index 427ca5d8b..5da27f357 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -1418,28 +1418,21 @@ async fn withhold_stale_go_tools( common: &GlobalArgs, warnings: &mut Vec, ) { - use socket_patch_core::utils::go_installed_tools::{audit, patched_at, PatchedModule}; + use socket_patch_core::utils::go_installed_tools::{audit, PatchedModule}; use socket_patch_core::utils::purl::parse_golang_purl; let mut patched = Vec::new(); let mut dirs = Vec::new(); for purl in &outcome.applied { - let (Some(record), Some(paths), Some((module, version))) = ( - manifest.patches.get(purl), - copies.get(purl), - parse_golang_purl(purl), - ) else { + let (Some(record), Some(paths)) = (manifest.patches.get(purl), copies.get(purl)) else { continue; }; for dir in paths { - let Some(at) = patched_at(dir, record.files.keys().map(String::as_str)) else { - continue; - }; - patched.push(PatchedModule { - module: module.to_string(), - version: version.to_string(), - patched_at: at, - }); - dirs.push(dir.clone()); + // An attested copy's patched files are there to date it. + let files = record.files.keys().map(String::as_str); + if let Some(module) = PatchedModule::of_copy(purl, dir, files) { + patched.push(module); + dirs.push(dir.clone()); + } } } let (notes, stale) = audit(patched, dirs).await; @@ -2202,6 +2195,7 @@ mod tests { WIRING_CONFLICT, GO_WORKSPACE_OVERRIDE, GO_VERSION_NOT_SELECTED, + GO_INSTALLED_TOOL_STALE, ] { assert_ne!( omission_phrase(tag), diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index 5ea1af61c..12c82373e 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -1059,39 +1059,45 @@ fn global_apply_flags_tools_built_from_the_unpatched_module() { let outside = tmp.path().join("outside"); std::fs::create_dir_all(&outside).unwrap(); write_patch(&outside); - let (code, stdout, stderr) = common::run_with_env( - &outside, - &[ - "apply", - "--global", - "--offline", - "--ecosystems", - "golang", - "--json", - "--cwd", - outside.to_str().unwrap(), - ], - &[("GOMODCACHE", mc), ("GOBIN", gobin.to_str().unwrap())], - ); - assert_eq!(code, 0, "apply -g: {stdout}\n{stderr}"); + let apply = || { + let (code, stdout, stderr) = common::run_with_env( + &outside, + &[ + "apply", + "--global", + "--offline", + "--ecosystems", + "golang", + "--json", + "--cwd", + outside.to_str().unwrap(), + ], + &[("GOMODCACHE", mc), ("GOBIN", gobin.to_str().unwrap())], + ); + assert_eq!(code, 0, "apply -g: {stdout}\n{stderr}"); + let env = common::envelope::parse_json_envelope(&stdout); + let stale: Vec = env["warnings"] + .as_array() + .into_iter() + .flatten() + .filter(|w| w["code"] == "go_installed_tool_stale") + .map(|w| w["detail"].as_str().unwrap().to_string()) + .collect(); + stale + }; + let stale = apply(); let lib = modcache.join(format!("{UMOD}@{UVER}")).join("lib.go"); assert_eq!(std::fs::read_to_string(&lib).unwrap(), PATCHED_LIB); // The cache is patched; the installed binary is not. assert_eq!(run_tool(&gobin), "TOOL: PRISTINE"); - let env = common::envelope::parse_json_envelope(&stdout); - let stale: Vec<&serde_json::Value> = env["warnings"] - .as_array() - .into_iter() - .flatten() - .filter(|w| w["code"] == "go_installed_tool_stale") - .collect(); - assert_eq!(stale.len(), 1, "{stdout}"); - let detail = stale[0]["detail"].as_str().unwrap(); + assert_eq!(stale.len(), 1, "{stale:?}"); assert!( - detail.contains(&format!("go install {TOOL}@{UVER}")) - && detail.contains(&format!("{UMOD}@{UVER}")), - "{detail}" + stale[0].contains(&format!("go install {TOOL}@{UVER}")) + && stale[0].contains(&format!("{UMOD}@{UVER}")), + "{stale:?}" ); + // A re-run over the already-patched cache still names the tool. + assert_eq!(apply(), stale); let out = global_vex(&outside, &modcache, &gobin); assert_absent(out.doc.as_ref(), UPURL); diff --git a/crates/socket-patch-core/src/utils/go_installed_tools.rs b/crates/socket-patch-core/src/utils/go_installed_tools.rs index 096ccd898..5d29c8496 100644 --- a/crates/socket-patch-core/src/utils/go_installed_tools.rs +++ b/crates/socket-patch-core/src/utils/go_installed_tools.rs @@ -76,7 +76,8 @@ impl StaleTool { } } -/// The warning detail when `go` itself can't be run to inspect `dirs`. +/// The warning detail for `dirs` whose tools `go version -m` could not +/// list (`go` not on `PATH`, or the run failed or timed out). pub fn unchecked_detail(patched: &[PatchedModule], dirs: &[PathBuf]) -> String { let modules: Vec = patched .iter() @@ -92,13 +93,28 @@ pub fn unchecked_detail(patched: &[PatchedModule], dirs: &[PathBuf]) -> String { ) } -/// The newest modification time among `files` under `dir` (the patched -/// files of a module cache copy), or `None` when none can be read. -pub fn patched_at<'a>(dir: &Path, files: impl IntoIterator) -> Option { - files - .into_iter() - .filter_map(|f| std::fs::metadata(dir.join(f)).ok()?.modified().ok()) - .max() +impl PatchedModule { + /// The patched module of `purl` whose cache copy is `dir`, dated by the + /// newest of the patch record's `files` there (keys may carry the + /// API's `package/` prefix), or `None` for a non-Go purl or when no + /// patched file can be read. + pub fn of_copy<'a>( + purl: &str, + dir: &Path, + files: impl IntoIterator, + ) -> Option { + let (module, version) = crate::utils::purl::parse_golang_purl(purl)?; + let patched_at = files + .into_iter() + .map(crate::patch::apply::normalize_file_path) + .filter_map(|f| std::fs::metadata(dir.join(f)).ok()?.modified().ok()) + .max()?; + Some(Self { + module: module.into_owned(), + version: version.into_owned(), + patched_at, + }) + } } /// The directories `go install` writes to, for module cache copies at @@ -112,6 +128,10 @@ pub fn bin_dirs(module_dirs: &[&Path]) -> Vec { } /// [`bin_dirs`] with an injected Go setting lookup and home directory. +/// +/// The user's own `$GOBIN` / `GOPATH` bin directories are searched only +/// for copies in the user's own module cache: a `--global-prefix` tree is +/// some other cache, which the user's tools were never built from. pub fn bin_dirs_with( module_dirs: &[&Path], go_env: &dyn Fn(&str) -> Option, @@ -123,26 +143,31 @@ pub fn bin_dirs_with( dirs.push(p); } }; - if let Some(gobin) = go_env("GOBIN").filter(|v| !v.is_empty()) { - push(PathBuf::from(gobin)); - } else { - match go_env("GOPATH").filter(|v| !v.is_empty()) { - Some(gopath) => std::env::split_paths(&gopath) - .filter(|p| !p.as_os_str().is_empty()) - .for_each(|p| push(p.join("bin"))), - None => { - if let Some(home) = home { - push(home.join("go").join("bin")); - } - } + let gopaths: Vec = 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"))); + if user_cache.is_some_and(|c| module_dirs.iter().any(|d| d.starts_with(&c))) { + match go_env("GOBIN").filter(|v| !v.is_empty()) { + Some(gobin) => push(PathBuf::from(gobin)), + None => gopaths.iter().for_each(|g| push(g.join("bin"))), } } // A cache at `/pkg/mod/@`: `` is the - // GOPATH that `go install` used with it. + // GOPATH that `go install` used with it. The outermost `pkg/mod`: a + // module path may itself contain one. for dir in module_dirs { if let Some(root) = dir .ancestors() - .find(|a| a.ends_with(Path::new("pkg").join("mod"))) + .filter(|a| a.ends_with(Path::new("pkg").join("mod"))) + .last() .and_then(|m| m.parent()?.parent()) { push(root.join("bin")); @@ -198,14 +223,26 @@ pub fn parse_version_m(output: &str) -> Vec { out } -/// The installed Go binaries under `dirs`, or `None` when `go` is not on -/// `PATH` (nothing can be checked). Missing directories are skipped. -/// Blocking. -pub fn installed_binaries(dirs: &[PathBuf]) -> Option> { - let go = crate::utils::process::resolve_tool("go")?; - let mut out = Vec::new(); - for dir in dirs.iter().filter(|d| d.is_dir()) { - let mut cmd = crate::utils::process::command_for(&go); +/// What `go version -m` found under a set of directories. +#[derive(Debug, Default)] +pub struct Inspection { + pub binaries: Vec, + /// Directories holding files whose tools could not be listed: `go` is + /// not on `PATH`, or its run failed or timed out. + pub unchecked: Vec, +} + +/// The installed Go binaries under `dirs`. Missing and empty directories +/// are skipped. Blocking. +pub fn installed_binaries(dirs: &[PathBuf]) -> Inspection { + let mut out = Inspection::default(); + let go = crate::utils::process::resolve_tool("go"); + for dir in dirs.iter().filter(|d| dir_has_entries(d)) { + let Some(go) = &go else { + out.unchecked.push(dir.clone()); + continue; + }; + let mut cmd = crate::utils::process::command_for(go); // `go version` reads files only; keep a `toolchain` line in an // ambient go.mod or GOTOOLCHAIN from fetching another toolchain. cmd.arg("version") @@ -213,12 +250,14 @@ pub fn installed_binaries(dirs: &[PathBuf]) -> Option> { .arg(dir) .current_dir(dir) .env("GOTOOLCHAIN", "local"); - let Ok(output) = crate::utils::process::output_within(cmd, VERSION_BUDGET) else { - continue; - }; - out.extend(parse_version_m(&String::from_utf8_lossy(&output.stdout))); + match crate::utils::process::output_within(cmd, VERSION_BUDGET) { + Ok(output) if output.status.success() => out + .binaries + .extend(parse_version_m(&String::from_utf8_lossy(&output.stdout))), + _ => out.unchecked.push(dir.clone()), + } } - Some(out) + out } /// Every installed tool in `binaries` built from one of `patched` before it @@ -235,7 +274,7 @@ pub fn stale_tools_with( patched: &[PatchedModule], built_at: &dyn Fn(&Path) -> Option, ) -> Vec { - let mut out = Vec::new(); + let mut out: Vec = Vec::new(); for bin in binaries { for p in patched { if !bin @@ -249,18 +288,24 @@ pub fn stale_tools_with( if built_at(&bin.path).is_some_and(|t| t > p.patched_at) { continue; } + // A local build has no version to reinstall: `(devel)`, or + // (Go 1.24+) a VCS-stamped `+dirty` one. let reinstall = match (&bin.main_package, &bin.main_version) { - (Some(pkg), Some(ver)) if ver.starts_with('v') => { + (Some(pkg), Some(ver)) if ver.starts_with('v') && !ver.ends_with("+dirty") => { Some(format!("go install {pkg}@{ver}")) } _ => None, }; - out.push(StaleTool { + let tool = StaleTool { binary: bin.path.clone(), module: p.module.clone(), version: p.version.clone(), reinstall, - }); + }; + // Two cache copies of one module@version: one tool, one line. + if !out.contains(&tool) { + out.push(tool); + } } } out @@ -288,21 +333,19 @@ pub fn audit_sync( patched: &[PatchedModule], dirs: &[PathBuf], ) -> (Vec<(String, String)>, Vec) { - match installed_binaries(dirs) { - Some(binaries) => { - let stale = stale_tools(&binaries, patched); - let warnings = stale - .iter() - .map(|t| (STALE_WARNING.to_string(), t.detail())) - .collect(); - (warnings, stale) - } - None if dirs.iter().any(|d| dir_has_entries(d)) => ( - vec![(STALE_WARNING.to_string(), unchecked_detail(patched, dirs))], - Vec::new(), - ), - None => (Vec::new(), Vec::new()), + let inspection = installed_binaries(dirs); + let stale = stale_tools(&inspection.binaries, patched); + let mut warnings: Vec<(String, String)> = stale + .iter() + .map(|t| (STALE_WARNING.to_string(), t.detail())) + .collect(); + if !inspection.unchecked.is_empty() { + warnings.push(( + STALE_WARNING.to_string(), + unchecked_detail(patched, &inspection.unchecked), + )); } + (warnings, stale) } fn dir_has_entries(dir: &Path) -> bool { @@ -416,7 +459,6 @@ mod tests { #[test] fn bin_dirs_follow_gobin_then_gopath_then_home_and_the_cache_root() { let home = Some(PathBuf::from("/home/u")); - let cache = PathBuf::from("/opt/gp/pkg/mod/example.com/upstream@v1.0.0"); let env = |vars: Vec<(&'static str, &'static str)>| { move |k: &str| { vars.iter() @@ -424,22 +466,96 @@ mod tests { .map(|(_, v)| v.to_string()) } }; + let user = Path::new("/home/u/go/pkg/mod/example.com/upstream@v1.0.0"); assert_eq!( - bin_dirs_with(&[&cache], &env(vec![("GOBIN", "/gobin")]), home.clone()), - vec![PathBuf::from("/gobin"), PathBuf::from("/opt/gp/bin")] + bin_dirs_with(&[user], &env(vec![("GOBIN", "/gobin")]), home.clone()), + vec![PathBuf::from("/gobin"), PathBuf::from("/home/u/go/bin")] ); assert_eq!( - bin_dirs_with(&[], &env(vec![("GOPATH", "/a:/b")]), home.clone()), + bin_dirs_with( + &[Path::new("/a/pkg/mod/x@v1.0.0")], + &env(vec![("GOPATH", "/a:/b")]), + home.clone() + ), vec![PathBuf::from("/a/bin"), PathBuf::from("/b/bin")] ); + assert_eq!( + bin_dirs_with(&[user], &env(Vec::new()), home.clone()), + vec![PathBuf::from("/home/u/go/bin")] + ); + // A GOMODCACHE outside GOPATH: the user's tools, not `/bin`. + assert_eq!( + bin_dirs_with( + &[Path::new("/cache/x@v1.0.0")], + &env(vec![("GOMODCACHE", "/cache")]), + home.clone() + ), + vec![PathBuf::from("/home/u/go/bin")] + ); + // A --global-prefix tree: never the user's own tools. + assert_eq!( + bin_dirs_with( + &[Path::new("/img/gp/pkg/mod/x@v1.0.0")], + &env(vec![("GOBIN", "/gobin")]), + home.clone() + ), + vec![PathBuf::from("/img/gp/bin")] + ); + // A module path that itself contains `pkg/mod`: the outermost one. assert_eq!( bin_dirs_with( - &[Path::new("/home/u/go/pkg/mod/x@v1.0.0")], + &[Path::new("/img/gp/pkg/mod/github.com/foo/pkg/mod/x@v1.0.0")], &env(Vec::new()), home ), - vec![PathBuf::from("/home/u/go/bin")] + vec![PathBuf::from("/img/gp/bin")] + ); + } + + #[test] + fn a_dirty_local_build_gets_no_reinstall_command() { + let mut b = bins(); + b[0].main_version = Some("v1.2.4-0.20240101120000-abcdef123456+dirty".into()); + let stale = stale_tools_with( + &b[..1], + &[patched("example.com/upstream", "v1.0.0", 1)], + &|_| None, + ); + assert_eq!(stale[0].reinstall, None); + } + + #[test] + fn a_copy_is_dated_by_its_patched_files_with_or_without_the_package_prefix() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("lib.go"), "x").unwrap(); + let purl = "pkg:golang/example.com/upstream@v1.0.0"; + for key in ["lib.go", "package/lib.go"] { + let p = PatchedModule::of_copy(purl, dir.path(), [key]).expect(key); + assert_eq!( + (p.module.as_str(), p.version.as_str()), + ("example.com/upstream", "v1.0.0") + ); + } + assert_eq!(PatchedModule::of_copy(purl, dir.path(), ["gone.go"]), None); + assert_eq!( + PatchedModule::of_copy("pkg:npm/x@1.0.0", dir.path(), ["lib.go"]), + None + ); + } + + #[test] + fn an_unlistable_directory_is_reported_unchecked() { + let dir = tempfile::tempdir().unwrap(); + // Not a Go binary: `go version -m` on the directory lists nothing, + // which is a clean check; a missing `go` is not. + std::fs::write(dir.path().join("tool"), "#!/bin/sh\n").unwrap(); + let (warnings, stale) = audit_sync( + &[patched("example.com/upstream", "v1.0.0", 1)], + &[dir.path().to_path_buf(), dir.path().join("missing")], ); + assert!(stale.is_empty()); + let go_on_path = crate::utils::process::resolve_tool("go").is_some(); + assert_eq!(warnings.len(), usize::from(!go_on_path), "{warnings:?}"); } #[test] From ba1875d46ee087d269c2c95339dbd0d004833a59 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 19:11:27 +0000 Subject: [PATCH 4/5] Check go install tools in dry runs and bare caches The installed-tool check only reads, so a dry run over an already-patched module cache now names the stale tools too. A --global-prefix pointing straight at a module cache, with no /pkg/mod layout to find its go install directory, is now reported instead of silently skipped. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 7 +- crates/socket-patch-cli/src/commands/apply.rs | 30 +++++---- .../tests/e2e_golang_build.rs | 29 ++++---- .../src/utils/go_installed_tools.rs | 67 +++++++++++++++---- 4 files changed, 92 insertions(+), 41 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 9a08f0e2a..34eb3691d 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -283,7 +283,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc **Hosted-state visibility (`redirectState`, additive/MINOR).** Every non-hosted-mode, non-vendored-mode `scan --json` SUCCESS envelope (report-only, `--mode agent`/`--sync`, and the zero-discovery envelope) carries an additive top-level `redirectState` object whenever the project's lockfiles pin ≥ 1 hosted patch: `{ mode, records: [{purl, uuid}], wiringLive: [purl] }`. It is a descriptive STATE block, not a warning. `mode` is the constant `"hosted"`. **v5.0 (MAJOR shape change)**: hosted mode keeps no ledger, so `records` lists the hosted pins lockfile discovery finds (one per `(purl, uuid)`, the same discovery `vex` uses: a hosted URL counts only on `https://patch.socket.dev` or the `--patch-server-url` origin), and the v4 `ledger` and `records[].ledgerKey` keys are gone. Each record's `purl` is CANONICALIZED (qualifiers stripped, percent-decoded — e.g. `pkg:npm/@scope/pkg@1.0.0`, `pkg:gem/nokogiri@1.13.3`) to the same spelling `wiringLive` carries, so the join is a plain string compare. `wiringLive` is the subset of those pins among this run's *counted* purls (post-`--ecosystems`-filter) — computed once per run, the same set that feeds `hosted_wiring_retained`. A record missing from `wiringLive` is still wired; it just was not crawled/queried this run (an `--ecosystems` filter, a zero discovery). The key is omitted when no lockfile pins a hosted patch (and under `--global`), and error envelopes (the `--offline` refusal, all-batches-failed) are deliberately minimal and never carry it. A pre-v5 `.socket/vendor/redirect-state.json` is not read. Hosted-mode runs carry their own `redirect` payload and `details.mode: "hosted"` events instead, and vendored-mode runs carry the takeover warnings (their takeovers may restore pins mid-run) — neither duplicates a pre-run snapshot that could go stale. -**Agent-flow run-level warnings (additive).** An agent-mode apply (`--mode agent` / `--sync`, `--json`) may add a top-level `warnings[]` array of `{code, detail}` entries to the scan envelope (absent when none fired; each is also mirrored to stderr unless `--silent`). They surface cross-mode state the apply cannot change — never a status or exit-code change (hosted refusals set the precedent: exit 0 + warning). Codes (stable; new codes are additive/MINOR): `vendored_ownership_retained` — vendor-owned package(s) were skipped before download (each is also a `skipped` / `vendored` event); the detail names the purls and the migration path (`remove `, or `vendor --revert` which unwinds every vendored package, then re-run). `hosted_wiring_retained` — the lockfiles still pin scanned package(s) to a hosted patch (the agent run does not unwind hosted wiring — as of v5.0 that is `socket-patch rollback`'s job, which restores the upstream registry entries, or `remove ` per package); the detail names the purls and the options (stay `--mode hosted`, migrate via `scan --mode vendored`, or `socket-patch rollback`). The warning keys on the hosted pins lockfile discovery finds at scan time, so a flow that restored the upstream entries retires it. The human path prints the same `hosted_wiring_retained` text to stderr after an apply; the vendored counterpart is already covered by its per-package `[skip] … (vendored …)` lines. `ownership_not_restored` (v5.0; `apply` and `rollback` `warnings[]` alike) — a file WAS patched (or restored) but its ownership could not be put back to the original uid/gid (the mode is still restored last); the detail is `: : patched, but ownership could not be restored to uid N gid M: ` and the human line `Warning: ` (stderr, muted by `--silent`); never a status or exit change. `go_installed_tool_stale` (v5.0; `apply`) — a `--global` Go apply patched a module cache copy that a tool installed with `go install` was built from before the patch; the detail names the binary and the `go install @` remedy (or, when `go` is not on `PATH`, the directories that could not be checked); never a status or exit change. `cargo_build_cache_stale` (v5.0; `apply` and `rollback` `warnings[]` alike) — a cargo crate's bytes changed but its compiled copy in a project build directory could not be invalidated (a fingerprint directory that could not be removed, or a `build.build-dir` with a `{workspace-path-hash}` template); the detail names the crates to `cargo clean -p` before the next build, which may otherwise link the stale code; never a status or exit change. +**Agent-flow run-level warnings (additive).** An agent-mode apply (`--mode agent` / `--sync`, `--json`) may add a top-level `warnings[]` array of `{code, detail}` entries to the scan envelope (absent when none fired; each is also mirrored to stderr unless `--silent`). They surface cross-mode state the apply cannot change — never a status or exit-code change (hosted refusals set the precedent: exit 0 + warning). Codes (stable; new codes are additive/MINOR): `vendored_ownership_retained` — vendor-owned package(s) were skipped before download (each is also a `skipped` / `vendored` event); the detail names the purls and the migration path (`remove `, or `vendor --revert` which unwinds every vendored package, then re-run). `hosted_wiring_retained` — the lockfiles still pin scanned package(s) to a hosted patch (the agent run does not unwind hosted wiring — as of v5.0 that is `socket-patch rollback`'s job, which restores the upstream registry entries, or `remove ` per package); the detail names the purls and the options (stay `--mode hosted`, migrate via `scan --mode vendored`, or `socket-patch rollback`). The warning keys on the hosted pins lockfile discovery finds at scan time, so a flow that restored the upstream entries retires it. The human path prints the same `hosted_wiring_retained` text to stderr after an apply; the vendored counterpart is already covered by its per-package `[skip] … (vendored …)` lines. `ownership_not_restored` (v5.0; `apply` and `rollback` `warnings[]` alike) — a file WAS patched (or restored) but its ownership could not be put back to the original uid/gid (the mode is still restored last); the detail is `: : patched, but ownership could not be restored to uid N gid M: ` and the human line `Warning: ` (stderr, muted by `--silent`); never a status or exit change. `go_installed_tool_stale` (v5.0; `apply`, dry runs included) — a `--global` Go apply patched a module cache copy that a tool installed with `go install` was built from before the patch; the detail names the binary and the `go install @` remedy (or, when they can't be inspected, what could not be checked); never a status or exit change. `cargo_build_cache_stale` (v5.0; `apply` and `rollback` `warnings[]` alike) — a cargo crate's bytes changed but its compiled copy in a project build directory could not be invalidated (a fingerprint directory that could not be removed, or a `build.build-dir` with a `{workspace-path-hash}` template); the detail names the crates to `cargo clean -p` before the next build, which may otherwise link the stale code; never a status or exit change. `scan --prune` opts into garbage collection. When set, `scan` removes manifest entries for packages no longer present in the crawl, then deletes orphan blob files, and every obsolete diff and package archive, from `.socket/`. A blob is an orphan when no patch left in the manifest references it: the afterHash and beforeHash blobs of every remaining patch are kept, the same retention policy `repair` uses (the beforeHash blobs are an offline rollback's only restore data, and `repair` downloads afterHash blobs only). Diff and legacy package archives are never kept, even for a remaining patch: nothing reads them any more. Off by default (v3.0) so a temporary uninstall doesn't silently destroy manifest state. Only entries whose ecosystem this run actually crawled are eligible: a `pkg:/` with no crawler in this build (a newer CLI's ecosystem in the committed manifest) is exempt — the crawl never looked for them, so their absence is not evidence of removal (same fail-safe as the `--ecosystems` filter, which narrows the query but never the prune's installed set). A Cargo entry is also exempt while its agent-mode copy is still patched: the project crawl looks up only the crates `Cargo.lock` resolves, but a crate the lock bumped or dropped keeps its patched copy in the machine-wide `$CARGO_HOME/registry/src` cache (nothing deletes it), and the entry holds the only blobs that can restore that copy. The wet pass keeps it with a `cargo_cache_patch_kept` warning naming the `socket-patch rollback ` that restores the copy and drops the entry; the `--dry-run` preview reports no event (and no warning) for it. The pass also reconciles vendored state (runs FIRST, under ONE apply-lock acquisition shared with the manifest prune — lock contention skips the whole pass without failing the scan; `--lock-timeout` is honored and a lock I/O error is reported rather than swallowed; the existence gate — a manifest file OR a vendor ledger file, both cheap stats; an emptied ledger is deleted on save, so its presence is its content proxy — runs BEFORE the lock, so a bare project never gets a `.socket/`; in the vendored scan arms the pass runs AFTER the vendor step): (a) ledger entries still tracked by a manifest record (manifest-mode entries written by standalone `vendor`) whose patch is gone from the manifest are reverted — `detached` entries (every `scan`/`get --mode vendored` entry, v5.0) have no manifest record to lose and are exempt from this leg; (b) EVERY ledger entry whose dependency is no longer in the lockfile graph is reverted and any manifest entry it still had dropped (v5.0: the check is about the lockfile, not the manifest, so embedded-record entries are no longer exempt; a missing or undeterminable lockfile keeps the entry, fail-safe); and (c) orphan `.socket/vendor//` dirs with no ledger entry are swept. The prune never deletes a zero-patch `.socket/manifest.json` (its `{"patches": {}}` + `setup` block stay). Under `--json` (v5.0, MAJOR) the pass is recorded into the scan envelope: each pruned manifest entry is a `removed` event with `details.manifest: true`, each reverted vendored entry a `removed` event (`errorCode: "vendor_reverted"`, `details.mode: "vendored"`), each drift-kept entry a `skipped` / `vendor_revert_kept` event and each entry whose revert failed a `skipped` / `vendor_revert_failed` event (a GC revert failure never fails the run); a `--dry-run` preview reports the removals as `verified`. The artifact sweep is the envelope's `gc` (`summary.bytesFreed` mirrors it) and the orphan vendor dirs swept are the top-level `removedVendorOrphanDirs` count. The pass's warnings join the top-level `warnings[]`: `vendor_state_write_failed` / `manifest_write_failed` (entries were reverted but the ledger or manifest rewrite failed), `cleanup_failed` (an orphan sweep failed mid-way), `cargo_cache_patch_kept` (see above), and the reinstall advisories of the vendored reverts (`vendor_bun_reinstall_required`, `vendor_vlt_reinstall_required`, `vendor_pypi_reinstall_required`; a revert's other warnings, such as `vendor_lock_entry_removed` or a drift keep's, are not repeated). A pass skipped at the lock (`lock_held` | `lock_io`) has no `gc` and no events, only a `gc_skipped` warning whose detail ends with the code. Human mode prints `GC: skipped: .`, one `GC: .` line per warning, and `GC: failed to revert N vendored entries: …` (singular for one). A drift keep is an entry the revert deliberately preserved (`vendor_artifact_kept` — undo the drift and re-run `vendor --revert` to finish); the preview cannot see drift (backends return before the wiring replay on dry runs), so its `verified` reverts may over-promise what a wet run will actually reclaim. @@ -665,8 +665,9 @@ are patched by `socket-patch apply` like every other ecosystem: for a cache at `/pkg/mod`) keeps running that code, so `apply` adds a `go_installed_tool_stale` warning per tool naming its `go install @` remedy, and `vex -g` omits the patch with `go_installed_tool_stale` until every such tool is newer than the - patched files. Without `go` on `PATH` the tools can't be inspected: the same warning code says so, - and VEX is not withheld (#422). + patched files. When the tools can't be inspected (`go` not on `PATH`, `go version -m` failing, + or a bare `--global-prefix` cache with no known `go install` directory) the same warning code says + so, and VEX is not withheld (#422). ### sbt / Mill / scala-cli caches in agent mode diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 7cb1de0d1..a898f0f76 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -1394,22 +1394,24 @@ pub(crate) async fn run_locked( .into_iter() .map(|(code, detail)| RunWarning { code, detail }), ); - // A global Go apply patches the module cache in place: a - // tool already built from it with `go install` keeps the - // unpatched code until it is rebuilt (#422). An embedded - // `--vex` checks again and says so itself. - if args.common.is_global() { - let (patched, dirs) = patched_go_modules(&results); - let (warnings, _) = - socket_patch_core::utils::go_installed_tools::audit(patched, dirs).await; - cache_warnings.extend( - warnings - .into_iter() - .map(|(code, detail)| RunWarning { code, detail }), - ); - } run_warnings.extend(cache_warnings.iter().cloned()); } + // A global Go apply patches the module cache in place: a tool + // already built from it with `go install` keeps the unpatched + // code until it is rebuilt (#422). Read-only, so a dry run + // over an already-patched cache says so too. An embedded + // `--vex` checks again and says so itself. + if args.common.is_global() { + let (patched, dirs) = patched_go_modules(&results); + let (warnings, _) = + socket_patch_core::utils::go_installed_tools::audit(patched, dirs).await; + let warnings: Vec = warnings + .into_iter() + .map(|(code, detail)| RunWarning { code, detail }) + .collect(); + run_warnings.extend(warnings.iter().cloned()); + cache_warnings.extend(warnings); + } // Run-level advisories + best-effort fallback-home skips on the // human path: one gated stderr line each. `--silent` is diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index 12c82373e..b1507fb39 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -1059,19 +1059,21 @@ fn global_apply_flags_tools_built_from_the_unpatched_module() { let outside = tmp.path().join("outside"); std::fs::create_dir_all(&outside).unwrap(); write_patch(&outside); - let apply = || { + let apply_with = |extra: &[&str]| { + let mut args = vec![ + "apply", + "--global", + "--offline", + "--ecosystems", + "golang", + "--json", + "--cwd", + outside.to_str().unwrap(), + ]; + args.extend_from_slice(extra); let (code, stdout, stderr) = common::run_with_env( &outside, - &[ - "apply", - "--global", - "--offline", - "--ecosystems", - "golang", - "--json", - "--cwd", - outside.to_str().unwrap(), - ], + &args, &[("GOMODCACHE", mc), ("GOBIN", gobin.to_str().unwrap())], ); assert_eq!(code, 0, "apply -g: {stdout}\n{stderr}"); @@ -1085,6 +1087,7 @@ fn global_apply_flags_tools_built_from_the_unpatched_module() { .collect(); stale }; + let apply = || apply_with(&[]); let stale = apply(); let lib = modcache.join(format!("{UMOD}@{UVER}")).join("lib.go"); assert_eq!(std::fs::read_to_string(&lib).unwrap(), PATCHED_LIB); @@ -1096,8 +1099,10 @@ fn global_apply_flags_tools_built_from_the_unpatched_module() { && stale[0].contains(&format!("{UMOD}@{UVER}")), "{stale:?}" ); - // A re-run over the already-patched cache still names the tool. + // A re-run over the already-patched cache still names the tool, a + // dry run included (the check is read-only). assert_eq!(apply(), stale); + assert_eq!(apply_with(&["--dry-run"]), stale); let out = global_vex(&outside, &modcache, &gobin); assert_absent(out.doc.as_ref(), UPURL); diff --git a/crates/socket-patch-core/src/utils/go_installed_tools.rs b/crates/socket-patch-core/src/utils/go_installed_tools.rs index 5d29c8496..bc213ece9 100644 --- a/crates/socket-patch-core/src/utils/go_installed_tools.rs +++ b/crates/socket-patch-core/src/utils/go_installed_tools.rs @@ -117,9 +117,20 @@ impl PatchedModule { } } +/// Where to look for the tools built from a set of module cache copies. +#[derive(Debug, Default, PartialEq, Eq)] +pub struct BinDirs { + /// The directories `go install` writes to, deduplicated. + pub dirs: Vec, + /// Copies in a cache that is neither the user's own nor at + /// `/pkg/mod` (a bare `--global-prefix` module cache): no + /// `go install` directory is known for them. + pub unlocated: Vec, +} + /// The directories `go install` writes to, for module cache copies at /// `module_dirs`, reading the process environment. -pub fn bin_dirs(module_dirs: &[&Path]) -> Vec { +pub fn bin_dirs(module_dirs: &[&Path]) -> BinDirs { bin_dirs_with( module_dirs, &crate::utils::go_env::go_env, @@ -136,8 +147,9 @@ pub fn bin_dirs_with( module_dirs: &[&Path], go_env: &dyn Fn(&str) -> Option, home: Option, -) -> Vec { +) -> BinDirs { let mut dirs: Vec = Vec::new(); + let mut unlocated: Vec = Vec::new(); let mut push = |p: PathBuf| { if p.is_absolute() && !dirs.contains(&p) { dirs.push(p); @@ -154,7 +166,8 @@ pub fn bin_dirs_with( .filter(|v| !v.is_empty()) .map(PathBuf::from) .or_else(|| gopaths.first().map(|g| g.join("pkg").join("mod"))); - if user_cache.is_some_and(|c| module_dirs.iter().any(|d| d.starts_with(&c))) { + 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()) { Some(gobin) => push(PathBuf::from(gobin)), None => gopaths.iter().for_each(|g| push(g.join("bin"))), @@ -171,9 +184,24 @@ pub fn bin_dirs_with( .and_then(|m| m.parent()?.parent()) { push(root.join("bin")); + } else if !in_user_cache(dir) { + unlocated.push(dir.to_path_buf()); } } - dirs + BinDirs { dirs, unlocated } +} + +/// The warning detail for cache copies no `go install` directory is known +/// for. +pub fn unlocated_detail(dirs: &[PathBuf]) -> String { + let dirs: Vec = dirs.iter().map(|d| d.display().to_string()).collect(); + format!( + "no `go install` directory is known for the module cache copies at {} (not under \ + your GOMODCACHE or a `/pkg/mod`), so the tools built from them were not \ + checked; a tool built with `go install` before this patch keeps running the \ + unpatched code until you rebuild it with `go install @`", + dirs.join(", ") + ) } /// Parse `go version -m` output: one `: ` header per @@ -323,7 +351,12 @@ pub async fn audit( } crate::utils::fs::run_blocking(move || { let dirs: Vec<&Path> = module_dirs.iter().map(PathBuf::as_path).collect(); - audit_sync(&patched, &bin_dirs(&dirs)) + let bins = bin_dirs(&dirs); + let (mut warnings, stale) = audit_sync(&patched, &bins.dirs); + if !bins.unlocated.is_empty() { + warnings.push((STALE_WARNING.to_string(), unlocated_detail(&bins.unlocated))); + } + (warnings, stale) }) .await } @@ -468,7 +501,7 @@ mod tests { }; let user = Path::new("/home/u/go/pkg/mod/example.com/upstream@v1.0.0"); assert_eq!( - bin_dirs_with(&[user], &env(vec![("GOBIN", "/gobin")]), home.clone()), + bin_dirs_with(&[user], &env(vec![("GOBIN", "/gobin")]), home.clone()).dirs, vec![PathBuf::from("/gobin"), PathBuf::from("/home/u/go/bin")] ); assert_eq!( @@ -476,11 +509,12 @@ mod tests { &[Path::new("/a/pkg/mod/x@v1.0.0")], &env(vec![("GOPATH", "/a:/b")]), home.clone() - ), + ) + .dirs, vec![PathBuf::from("/a/bin"), PathBuf::from("/b/bin")] ); assert_eq!( - bin_dirs_with(&[user], &env(Vec::new()), home.clone()), + bin_dirs_with(&[user], &env(Vec::new()), home.clone()).dirs, vec![PathBuf::from("/home/u/go/bin")] ); // A GOMODCACHE outside GOPATH: the user's tools, not `/bin`. @@ -489,7 +523,8 @@ mod tests { &[Path::new("/cache/x@v1.0.0")], &env(vec![("GOMODCACHE", "/cache")]), home.clone() - ), + ) + .dirs, vec![PathBuf::from("/home/u/go/bin")] ); // A --global-prefix tree: never the user's own tools. @@ -498,7 +533,8 @@ mod tests { &[Path::new("/img/gp/pkg/mod/x@v1.0.0")], &env(vec![("GOBIN", "/gobin")]), home.clone() - ), + ) + .dirs, vec![PathBuf::from("/img/gp/bin")] ); // A module path that itself contains `pkg/mod`: the outermost one. @@ -506,10 +542,17 @@ mod tests { bin_dirs_with( &[Path::new("/img/gp/pkg/mod/github.com/foo/pkg/mod/x@v1.0.0")], &env(Vec::new()), - home - ), + home.clone() + ) + .dirs, vec![PathBuf::from("/img/gp/bin")] ); + // A bare --global-prefix module cache: nowhere known to look. + let bare = Path::new("/img/cache/x@v1.0.0"); + let found = bin_dirs_with(&[bare], &env(vec![("GOBIN", "/gobin")]), home); + assert_eq!(found.dirs, Vec::::new()); + assert_eq!(found.unlocated, vec![bare.to_path_buf()]); + assert!(unlocated_detail(&found.unlocated).contains("/img/cache/x@v1.0.0")); } #[test] From d06bf792ec9fafc1bd92abd1e0bf438b1f334097 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 19:24:51 +0000 Subject: [PATCH 5/5] Preview stale go tools on a first dry run A first `apply -g --dry-run` dated the module copy by its unpatched files, which are older than any tool built from them, so the preview missed the warning the real run gives. Copies this run writes, or would write, now count as patched now. A relative --global-prefix is resolved against the working directory, and a copy with no usable bin directory is reported instead of skipped. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/src/commands/apply.rs | 17 +++++++-- .../tests/e2e_golang_build.rs | 6 +++- .../src/utils/go_installed_tools.rs | 36 +++++++++++++++++-- 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index a898f0f76..3d951a0d9 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -2812,7 +2812,7 @@ async fn apply_patches_inner( /// The Go modules a (global, in-place) apply left patched in the module /// cache, and their cache copies, for the installed-tool check (#422). /// Already-patched copies count too: a tool built before an earlier run -/// is just as stale. +/// is just as stale. A dry run reports what the real run would. fn patched_go_modules( results: &[ApplyResult], ) -> ( @@ -2827,8 +2827,19 @@ fn patched_go_modules( continue; } let dir = PathBuf::from(&r.package_path); - let files = r.files_verified.iter().map(|f| f.file.as_str()); - if let Some(module) = PatchedModule::of_copy(&r.package_key, &dir, files) { + // Files this run writes (or, dry, would write) are patched now; an + // already-patched copy is dated by its files on disk. + let writes = !r.files_patched.is_empty() + || r.files_verified + .iter() + .any(|f| f.status == VerifyStatus::Ready); + let module = if writes { + PatchedModule::written_now(&r.package_key) + } else { + let files = r.files_verified.iter().map(|f| f.file.as_str()); + PatchedModule::of_copy(&r.package_key, &dir, files) + }; + if let Some(module) = module { patched.push(module); dirs.push(dir); } diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index b1507fb39..ad8515f65 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -1088,8 +1088,12 @@ fn global_apply_flags_tools_built_from_the_unpatched_module() { stale }; let apply = || apply_with(&[]); - let stale = apply(); + // The preview names the tool the real run will leave stale. + let preview = apply_with(&["--dry-run"]); let lib = modcache.join(format!("{UMOD}@{UVER}")).join("lib.go"); + assert_eq!(std::fs::read_to_string(&lib).unwrap(), PRISTINE_LIB); + let stale = apply(); + assert_eq!(preview, stale); assert_eq!(std::fs::read_to_string(&lib).unwrap(), PATCHED_LIB); // The cache is patched; the installed binary is not. assert_eq!(run_tool(&gobin), "TOOL: PRISTINE"); diff --git a/crates/socket-patch-core/src/utils/go_installed_tools.rs b/crates/socket-patch-core/src/utils/go_installed_tools.rs index bc213ece9..56c2e9567 100644 --- a/crates/socket-patch-core/src/utils/go_installed_tools.rs +++ b/crates/socket-patch-core/src/utils/go_installed_tools.rs @@ -94,6 +94,18 @@ pub fn unchecked_detail(patched: &[PatchedModule], dirs: &[PathBuf]) -> String { } impl PatchedModule { + /// The patched module of `purl` whose files this run writes (or, in a + /// dry run, would write) now: every tool already built from it is + /// stale. `None` for a non-Go purl. + pub fn written_now(purl: &str) -> Option { + let (module, version) = crate::utils::purl::parse_golang_purl(purl)?; + Some(Self { + module: module.into_owned(), + version: version.into_owned(), + patched_at: SystemTime::now(), + }) + } + /// The patched module of `purl` whose cache copy is `dir`, dated by the /// newest of the patch record's `files` there (keys may carry the /// API's `package/` prefix), or `None` for a non-Go purl or when no @@ -131,8 +143,14 @@ pub struct BinDirs { /// The directories `go install` writes to, for module cache copies at /// `module_dirs`, reading the process environment. pub fn bin_dirs(module_dirs: &[&Path]) -> BinDirs { + // A relative `--global-prefix` resolves against the working directory. + let module_dirs: Vec = module_dirs + .iter() + .map(|d| std::path::absolute(d).unwrap_or_else(|_| d.to_path_buf())) + .collect(); + let module_dirs: Vec<&Path> = module_dirs.iter().map(PathBuf::as_path).collect(); bin_dirs_with( - module_dirs, + &module_dirs, &crate::utils::go_env::go_env, crate::utils::fs::home_dir(), ) @@ -183,7 +201,11 @@ pub fn bin_dirs_with( .last() .and_then(|m| m.parent()?.parent()) { - push(root.join("bin")); + if root.is_absolute() { + push(root.join("bin")); + } else if !in_user_cache(dir) { + unlocated.push(dir.to_path_buf()); + } } else if !in_user_cache(dir) { unlocated.push(dir.to_path_buf()); } @@ -549,10 +571,18 @@ mod tests { ); // A bare --global-prefix module cache: nowhere known to look. let bare = Path::new("/img/cache/x@v1.0.0"); - let found = bin_dirs_with(&[bare], &env(vec![("GOBIN", "/gobin")]), home); + let found = bin_dirs_with(&[bare], &env(vec![("GOBIN", "/gobin")]), home.clone()); assert_eq!(found.dirs, Vec::::new()); assert_eq!(found.unlocated, vec![bare.to_path_buf()]); assert!(unlocated_detail(&found.unlocated).contains("/img/cache/x@v1.0.0")); + // A relative root has no usable bin directory: reported, not dropped. + let rel = Path::new("gp/pkg/mod/x@v1.0.0"); + let found = bin_dirs_with(&[rel], &env(Vec::new()), home.clone()); + assert_eq!(found.dirs, Vec::::new()); + assert_eq!(found.unlocated, vec![rel.to_path_buf()]); + // bin_dirs itself resolves it against the working directory. + let cwd = std::env::current_dir().unwrap(); + assert!(bin_dirs(&[rel]).dirs.contains(&cwd.join("gp").join("bin"))); } #[test]