Repository navigation
Keep Go hosted pins after go mod tidy drops the zip line - #1748
Conversation
When nothing imports a package from a hosted-patched Go module (it is only a transitive requirement), `go mod tidy` keeps just the socket module's `/go.mod` go.sum line and drops its zip line. The build still succeeds offline, but VEX discovery required both lines, so the patch was skipped as `package_not_found` instead of attested. The `/go.mod` line for the exact socket module+version is now enough to recognise the pin. Without the zip line the ref has no locked integrity and is not integrity-required, so it attests from the lockfile: go compiles none of the module's packages, and the module path carries the patch uuid. A zip line, when present, must still be the single well-formed hash and supplies `locked_integrity`. Conflicting hashes or a malformed line for the socket module+version still mean no pin. The hosted writer is unchanged: it still writes both lines and refuses a module outside the build graph. Assisted-by: Claude Code:claude-opus-5-5
A `/go.mod`-only go.sum line also exists for versions MVS reads but does not select, so in a go < 1.17 graph a `/go.mod`-only hosted pin whose version go.sum shows is not selected is now reported as inert instead of attested. In workspace mode go verifies against go.work.sum and every member's go.sum, so discovery now reads those too (in-root members only) and a member's zip line still supplies the locked integrity. go.sum parsing now follows go: a line of other than three tokens makes the whole file unusable, a non-dirhash `h1:` breaks that module's pin, and hashes of other algorithms are ignored. Re-running the hosted rewrite over a project `go mod tidy` pruned to the `/go.mod` line now leaves go.sum alone instead of re-adding the zip line the next tidy removes again. Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
In a go.work workspace, Go builds the highest version any `use` member requires. A Socket replace whose go.sum held only the `/go.mod` line was still attested when another member required a newer version, so VEX could claim a patch that no build links. Such a pin now attests only when every member's go.mod is inside the project and readable and together they select the replaced version; otherwise it is flagged as inert or left unattested. Pins are also looked up only in the sum files Go actually reads: in a workspace, go.work.sum and the members' go.sum (the root go.sum only when `.` is a member); with GOWORK=off, the root go.sum alone, and the go.work replaces are ignored as Go ignores them. Assisted-by: Claude Code:claude-opus-5-5
|
Burn-down agent: labeled Ready for review at head
Generated by Claude Code |
`go mod tidy` drops the Socket module's zip checksum line while no package of the patched module is imported. If the project later imports it, the build fails with "missing go.sum entry", and re-running `scan --mode hosted` left that state untouched, so the project had no supported way back. A re-run now always writes both go.sum lines, so it repairs the project and the build uses the patched module again. The trade-off is that the next `go mod tidy` may drop the line again while the module stays unimported. Assisted-by: Claude Code:claude-opus-5-5
Final review briefWhat it does. When Risk: low. The only new attestation path covers modules whose packages go doesn't compile: if a package were loaded, Look here
Verified
Changes I made: none. Open questions (non-blocking; can be follow-ups):
Auto-merge is armed, so approving sends this PR straight to the merge queue. Generated by Claude Code |
A go.work build never makes a member's go.mod list the versions it builds, and it adds missing checksums to go.work.sum on its own. So a member's dependency can pull in a newer version of a patched module than any committed file names, and the build links the unpatched module. vex used to attest such a pin from the lockfile whenever the members' go.mod files named the patched version. In a workspace a pin with only the /go.mod line now needs a verifying installed copy, and a newer version in a member's require or in any recorded zip line marks the replace as inert. Docs and the CLI contract say so, and a real-go test covers the member dependency case. Assisted-by: Claude Code:claude-opus-5-5
go only ever appends to go.work.sum: once a member stops depending on whatever raised a module, the zip line of the higher version stays behind. vex treated such a line as proof the workspace builds that version, so it dropped a Go /go.mod-only pin as inert even when go builds the patched version. Only a member's require or the zip lines of a member's go.sum now mark a workspace pin inert. The pin still needs a verifying installed copy to attest. Assisted-by: Claude Code:claude-opus-5-5
A workspace's sum files are only appended to, so the socket zip line a past build wrote to go.work.sum (or a member go.sum) stays after a member starts requiring a higher version of the replaced module. Go then builds that higher version and the replace is inert, but discovery attested the zip-pinned ref from the lockfile anyway, and the CLI's post-gate reads only the root go.mod and go.sum, so vex claimed a patch the build never links. Run the same inert check GoModOnly pins already get on Zip pins in workspace mode. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Final reviewer: pushed Finding. In a
The CLI post-gate ( Fix. In workspace mode, Zip pins now run the same Ran (stable 1.97; the pinned 1.93.1 toolchain is incomplete in this container):
Stacked PR #1763 targets this branch and will pick up the change. bugbot run Generated by Claude Code |
|
bugbot run Generated by Claude Code |
1 similar comment
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Zip pins attest unknown workspace selection
- Added tracking of go.work.sum zip versions separately; when a higher version exists in go.work.sum, the Zip pin now emits the ref without locked_integrity, preventing false attestation.
- ✅ Fixed: One bad member drops all requires
- Changed read_workspace to skip only the bad member instead of setting mains to None, allowing valid members' requires to still be collected and used for selection checks.
Or push these changes by commenting:
@cursor push 2fa97846a0
Preview (2fa97846a0)
diff --git a/crates/socket-patch-core/src/vex/discover/golang.rs b/crates/socket-patch-core/src/vex/discover/golang.rs
--- a/crates/socket-patch-core/src/vex/discover/golang.rs
+++ b/crates/socket-patch-core/src/vex/discover/golang.rs
@@ -249,11 +249,10 @@
go_mod: Option<&str>,
go_work: &str,
) -> (Option<Vec<MainModule>>, Vec<String>) {
- let mut mains = Some(Vec::new());
+ let mut mains = Vec::new();
let mut files = vec![GO_WORK_SUM.to_string()];
for dir in go_mod_edit::parse_use_dirs(go_work) {
let Some(dir) = member_dir(&dir) else {
- mains = None;
continue;
};
let text = if dir.is_empty() {
@@ -265,17 +264,15 @@
None => None,
}
};
- match (text, mains.as_mut()) {
- (Some(text), Some(mains)) => mains.push(MainModule::new(&text)),
- (Some(_), None) => {}
- (None, _) => mains = None,
+ if let Some(text) = text {
+ mains.push(MainModule::new(&text));
}
let sum = member_file(&dir, GO_SUM);
if !files.contains(&sum) {
files.push(sum);
}
}
- (mains, files)
+ (Some(mains), files)
}
/// Normalize a go.mod-grammar file (BOM, simple string literals) and check
@@ -357,7 +354,7 @@
SumPin::Zip(_) => graph.workspace,
SumPin::Unpinned => false,
};
- if check {
+ let workspace_higher = if check {
let req = required.get(&entry.module).map(String::as_str);
// `replaced_purl` resolved one of the two.
let version = entry.version.as_deref().or(req).unwrap_or_default();
@@ -375,9 +372,23 @@
);
continue;
}
- }
+ // In workspace mode, also check go.work.sum for higher versions.
+ // Those lines outlive the requirement that raised the module, so
+ // they don't prove selection: emit the ref but don't attest it.
+ if graph.workspace {
+ let workspace_zips = sums.workspace_zip_versions(&entry.module);
+ workspace_zips
+ .iter()
+ .any(|v| go_semver_cmp(v, version) == Ordering::Greater)
+ } else {
+ false
+ }
+ } else {
+ false
+ };
let (integrity, integrity_required) = match pin {
- SumPin::Zip(h1) => (Some(LockIntegrity::GoH1(h1)), true),
+ SumPin::Zip(h1) if !workspace_higher => (Some(LockIntegrity::GoH1(h1)), true),
+ SumPin::Zip(_) => (None, true),
SumPin::GoModOnly => (None, graph.workspace),
SumPin::Unpinned => (None, true),
};
@@ -536,6 +547,9 @@
unparseable: bool,
/// Every module's zip-line versions (the build-list evidence).
zip_versions: HashMap<String, Vec<String>>,
+ /// go.work.sum zip-line versions: used to detect inert workspace
+ /// replaces but not as selection proof.
+ workspace_zip_versions: HashMap<String, Vec<String>>,
}
/// What the sum files pin for one Socket `module@version`.
@@ -553,7 +567,7 @@
let mut pins = SumPins::default();
for file in files {
if let Some(text) = ctx.read_text(file, out).await {
- pins.add(&text, file != GO_WORK_SUM);
+ pins.add(&text, file != GO_WORK_SUM, file == GO_WORK_SUM);
}
}
pins
@@ -562,7 +576,9 @@
/// `selects`: whether the file's zip lines show what the build selects.
/// go only ever appends to `go.work.sum`, so its lines outlive the
/// requirement that raised a module.
- fn add(&mut self, text: &str, selects: bool) {
+ /// `workspace_sum`: whether this is go.work.sum (used to detect inert
+ /// workspace replaces but not as selection proof).
+ fn add(&mut self, text: &str, selects: bool, workspace_sum: bool) {
// go rejects a whole go.sum with any non-blank line of other than
// three tokens (only `go mod tidy` skips it).
if text
@@ -573,11 +589,19 @@
return;
}
for line in go_sum_lines(text) {
- if !line.go_mod && selects {
- self.zip_versions
- .entry(line.module.to_string())
- .or_default()
- .push(line.version.to_string());
+ if !line.go_mod {
+ if selects {
+ self.zip_versions
+ .entry(line.module.to_string())
+ .or_default()
+ .push(line.version.to_string());
+ }
+ if workspace_sum {
+ self.workspace_zip_versions
+ .entry(line.module.to_string())
+ .or_default()
+ .push(line.version.to_string());
+ }
}
// go ignores hashes of algorithms it does not know.
if !line.module.starts_with(HOSTED_GO_MODULE_PREFIX) || !line.hash.starts_with("h1:") {
@@ -624,6 +648,13 @@
.map(|v| v.iter().map(String::as_str).collect())
.unwrap_or_default()
}
+
+ fn workspace_zip_versions(&self, module: &str) -> Vec<&str> {
+ self.workspace_zip_versions
+ .get(module)
+ .map(|v| v.iter().map(String::as_str).collect())
+ .unwrap_or_default()
+ }
}
/// Root-relative directory of a `go.work` `use` member (`""` for the rootYou can send follow-ups to the cloud agent here.
|
Burn-down agent: CI is green on
I did not push a fix. Stacked PR #1763 ("Read Go workspaces the way go builds them") is based on this branch and removes Question for a human: should #1748 merge first with these two workspace gaps as known issues, to be fixed by #1763? Or should the fixes go into #1748 itself, with the #1763 stack rebased on them? Until that's decided I've added Generated by Claude Code |
read_workspace dropped every main module when any `use` member was outside the root or unreadable, so a readable member requiring a higher version no longer marked a workspace zip pin inert and vex attested a replace go does not build (Bugbot, golang.rs:271). In a workspace the members' requires only ever prove a replace inert, so the readable ones still count when another member cannot be read. Co-Authored-By: Claude <noreply@anthropic.com>
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f534ec7. Configure here.
|
Burn-down agent: not labeled.
Generated by Claude Code |
|
Final reviewer: armed squash auto-merge at f534ec7 so this goes into the merge queue. Approved on the current head by Tanmay182003, Stacked PRs #1763 → #1764 → #1765 sit on this branch and were not armed: their bases aren't Generated by Claude Code |
Main landed lighter fixes for the same go.work gaps this PR models in full: #1752 (version gate reads every member), #1753 (hosted pins outranked by another member), #1767 (stale go.work vendor warning) and the squashed #1748. Resolve them onto this PR's GoWorkspace model instead of keeping two workspace readers: - go_build_list: keep project_selection on GoWorkspace. Drop main's ModuleEvidence / workspace_selection / other_workspace_members and its three on-disk selection tests: GoWorkspace::selection covers those cases from the module graph (go_workspace tests), and without a module cache it answers Unknown by design. - hosted: drop #1753's go_members plumbing (engine RewriteOptions closure, rewrite_golang argument, CLI wiring). The PR's HostedGoWorkspace already refuses a pin another member outranks. Main's e2e test for it now seeds the module cache the workspace graph reads, and passes, including the rescan that removes the pin. - go_consumer_sync: port #1767's modules.txt audit (per-member replace resolution, member_owned, unreadable members skipped) onto go_workspace::load and its shared same_dir_key / lexical_target. - vex discover, CLI_CONTRACT, e2e_golang_hosted_build: keep the PR's stricter rules (a zip pin is checked against the workspace selection too; sum-file zip lines are no evidence of it). Co-Authored-By: Claude <noreply@anthropic.com>

Problem
A root module requires
b,brequiresa, nothing imports a package froma, and the root has the hostedreplace a v => patch.socket.dev/gopatch/<uuid> <sver>. Herego mod tidykeeps only the socket module's/go.mod h1:go.sum line. It drops the zip line because Go keeps just the/go.modline for a module that provides no loaded package.go build -mod=readonlystill succeeds, but VEX discovery required both lines, sosocket-patch vexskipped the patch aspackage_not_foundinstead of attesting it.The new real-toolchain test reproduces this with go1.26.3. A reviewer first found it with go1.18.1.
Change
Reader (
vex/discover/golang.rs)/go.modline for the exact socket module+version is now enough to recognise the hosted pin. When the zip line is present, it must be the one well-formed hash, and it supplieslocked_integrityas before.locked_integrityandintegrity_required = false, so it attests from the lockfile. Go compiles none of the module's packages in this state, and the module path carries the patch uuid./go.modline of versions it reads but does not select. So in a go < 1.17 graph, a/go.mod-only pin is now diagnosed as inert (ref_invalid, no ref) when go.sum shows that another version is selected (go_build_list::select). This matches depscan#29121, which only annotates the selected version.go.sum. With ago.work, it also readsgo.work.sumand each in-rootusemember'sgo.sum, because Go verifies against all of them and a member's go.sum may hold the zip line.h1:, for the socket module+version mean no pin.Writer (
patch/redirect/mod.rs)redirect_golang_not_in_module_graph./go.modline, but the rewriter can't tell whether a package of the module is loaded. Once the project imported the module, the build failed withmissing go.sum entryand re-running hosted could no longer repair it.Docs
CLI_CONTRACT.mdand the confirmation paragraph are updated.integrity_requiredcontract indiscover/mod.rsnow allows the choice to depend on lock state, as gem already does.Review fixes (14fa6d4..bc5f8c4)
/go.mod-only pin was attested whenever the root's ownrequirematched, but go builds a workspace from every member's requirements. In a workspace a/go.mod-only pin is now never the lockfile basis (integrity_required = true). Workspace-wide selection lands in Read Go workspaces the way go builds them #1763.usemembers' go.sum, and the root go.sum only when.is a member. UnderGOWORK=offit reads only the root go.sum and ignores go.work's replace.Stack: this PR → #1763 (workspace model) → #1764 (go.work writer) → #1765 (workspace e2e). depscan bumps the submodule in a PR stacked on SocketDev/depscan#29121.
Other readers checked
list,rollback,remove,repair, the hosted engine and upstream restore (redirect/upstream/golang.rs) find hosted Go pins from go.mod alone. Discovery already emitted the ref whatever go.sum held, so no change was needed. Onlylockfile_basis_ok, which VEX uses, depended on both lines.Tests
cargo test -p socket-patch-core --libfiltered tovex::,golang,go_sum,tidyandworkspace: all pass. New unit tests cover:/go.mod-only pin, in both go.mod and go.work flavors/go.modhash casesholds_only_go_mod_linee2e_vex_lockfilegolang cells (22): cell g gains a/go.mod-line-only case that attests(redirected)without an installed copy.e2e_golang_hosted_build(23), with a new real-toolchain regressiontidy_pruned_zip_line_still_pins_the_hosted_replace, which:go mod tidy/go.modline keptlockfile_basis_ok()e2e_golang_hosted_state(29) ande2e_golang_workspace_build(16) pass.package_not_found, and the toolchain test fails itslockfile_basis_okassertion.cargo clippy --locked -p socket-patch-core -p socket-patch-cli --all-features -- -D warnings(the CI command) is clean./code-review highran on the first commit. Its findings are addressed in the second commit.🤖 Generated with Claude Code
Note
Medium Risk
Changes Go hosted pin recognition and VEX lockfile attestation (security-sensitive), with deliberate stricter rules for workspaces and inert replaces to limit false positives.
Overview
Fixes VEX / lockfile discovery for Go hosted redirects when
go mod tidyleaves only the socket module’s/go.modh1:line ingo.sum(no zip line while nothing imports the patched module). Those pins were treated as unpinned andvexskipped them even thoughgo buildstill succeeds.Discovery (
vex/discover/golang.rs) now treats a single well-formed/go.modline as a valid pin when not in ago.workworkspace (integrity_required = false, attests from the lockfile). With both lines, behavior is unchanged (ziph1:islocked_integrity). It also reads the sum files Go actually verifies (rootgo.sum, plusgo.work.sumand in-root membergo.sumwhen a workspace is active; honorsGOWORK=off), rejects malformed / conflicting sums like Go would, and marks replaces inert when evidence shows the build selects a higher module version (including severalgo.workcases where a/go.mod-only pin must not attest).Hosted rewriter (
patch/redirect/mod.rs) always upserts both sum lines on apply/re-run so a later import does not hitmissing go.sum entryafter tidy pruned the zip line.Docs (
CLI_CONTRACT.md,docs/ecosystems.md) andintegrity_requiredcontract text are updated; unit, VEX e2e, and real-toolchain e2e cover tidy pruning, re-run restore, and workspace non-attestation.Reviewed by Cursor Bugbot for commit f534ec7. Configure here.
Generated by Claude Code