Skip to content

Keep Go hosted pins after go mod tidy drops the zip line - #1748

Open
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
fix/golang-tidy-gomod-only-pin
Open

Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
fix/golang-tidy-gomod-only-pin

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

A root module requires b, b requires a, nothing imports a package from a, and the root has the hosted replace a v => patch.socket.dev/gopatch/<uuid> <sver>. Here go mod tidy keeps only the socket module's /go.mod h1: go.sum line. It drops the zip line because Go keeps just the /go.mod line for a module that provides no loaded package. go build -mod=readonly still succeeds, but VEX discovery required both lines, so socket-patch vex skipped the patch as package_not_found instead 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)

  • The /go.mod line 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 supplies locked_integrity as before.
  • If the zip line is missing, the ref has no locked_integrity and integrity_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.
  • MVS also keeps the /go.mod line 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.
  • Sum files: the root go.sum. With a go.work, it also reads go.work.sum and each in-root use member's go.sum, because Go verifies against all of them and a member's go.sum may hold the zip line.
  • Parsing follows Go's rules:
    • A line of other than three tokens anywhere in a sum file makes that file unusable, so there is no pin.
    • Conflicting hashes, or a non-dirhash h1:, for the socket module+version mean no pin.
    • Hashes of other algorithms are ignored.
  • The sum files are loaded once per discovery, and only when a hosted replace exists.

Writer (patch/redirect/mod.rs)

  • The writer still writes both lines and still refuses with redirect_golang_not_in_module_graph.
  • New: a re-run over a project whose existing hosted replace already targets the same module+version, and whose go.sum holds exactly the expected /go.mod line and no zip line, leaves go.sum alone and still confirms the uuid. Before, it re-added the zip line, and the next go mod tidy removed it again.

Docs

  • The golang row of CLI_CONTRACT.md and the confirmation paragraph are updated.
  • The module docs are updated.
  • The integrity_required contract in discover/mod.rs now allows the choice to depend on lock state, as gem already does.

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. Only lockfile_basis_ok, which VEX uses, depended on both lines.
  • Upstream restore still reinserts the upstream module's two lines. That path writes rather than reads, so it is left as is.

Tests

  • cargo test -p socket-patch-core --lib filtered to vex::, golang, go_sum, tidy and workspace: all pass. New unit tests cover:
    • the tidy /go.mod-only pin, in both go.mod and go.work flavors
    • an unselected-version pin being inert
    • the zip line coming from a workspace member's go.sum
    • the go.sum parsing rules
    • the conflicting /go.mod hash cases
    • holds_only_go_mod_line
    • a rewriter re-run over a tidied pair
  • e2e_vex_lockfile golang 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 regression tidy_pruned_zip_line_still_pins_the_hosted_replace, which:
    • builds a go 1.16 graph and runs go mod tidy
    • asserts that the zip line is pruned and the /go.mod line kept
    • builds on a fresh machine with a bogus GOSUMDB
    • checks that a rewriter re-run changes nothing
    • checks that discovery still reports the hosted ref with lockfile_basis_ok()
  • e2e_golang_hosted_state (29) and e2e_golang_workspace_build (16) pass.
  • Negative control: with the old reader, the new e2e cell fails with package_not_found, and the toolchain test fails its lockfile_basis_ok assertion.
  • cargo clippy --locked -p socket-patch-core -p socket-patch-cli --all-features -- -D warnings (the CI command) is clean.
  • /code-review high ran on the first commit. Its findings are addressed in the second commit.

🤖 Generated with Claude Code


Note

Medium Risk
Changes VEX/discovery attestation rules for hosted Go pins and go.sum interpretation; incorrect edge-case handling could attest or skip patches wrongly, but scope is golang hosted lockfile logic with extensive new tests.

Overview
Fixes manifest-less VEX and discovery for hosted Go patches when go mod tidy removes the socket module’s zip h1: line but keeps the /go.mod line (transitive replace with no imported packages).

Discovery now treats a well-formed /go.mod pin as sufficient: integrity_required is false in that state so lockfile_basis_ok() allows lockfile attestation without the zip line; when the zip line exists, behavior is unchanged. It also reads go.work.sum and in-root workspace member go.sum files, applies stricter go.sum parsing (malformed lines, conflicting hashes), and marks /go.mod-only pins inert on pre-1.17 graphs when MVS selects another version.

Hosted redirect re-runs skip re-inserting the zip line when the replace already matches and go.sum is exactly the tidy go.mod-only pair (holds_only_go_mod_line), avoiding tidy churn.

Docs update the golang hosted confirmation and discovery-table rules; integrity_required may depend on lock state (like gems).

Reviewed by Cursor Bugbot for commit 14057c8. Configure here.


Generated by Claude Code

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

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but none of the reported issues met your autofix severity threshold.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 14057c8. Configure here.

Comment thread crates/socket-patch-core/src/vex/discover/golang.rs
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 27, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at head 14057c8.

  • CI: 50/50 checks on 14057c8 passed, skipped or neutral.
  • Bugbot reviewed 14057c8 and raised one finding, "Member zips skew inert detection" (vex/discover/golang.rs:246). I refuted it in the thread and resolved it: in workspace mode go's build list takes every member's requirements, so a member's higher zip line really does make a /go.mod-only pin inert. Reading member go.sum files only ever withholds an attestation; it never attests an unbuilt patch.
  • Reviewer focus: how inert-pin detection (go_build_list::select over root plus member zip lines) should behave in workspace mode, and the tidy re-run skip in rewrite_golang (redirect/mod.rs).
  • Not announced in Slack this run (Slack send unavailable).

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. When go mod tidy prunes the socket module's zip h1: line from go.sum (a transitive hosted replace whose packages nobody imports), VEX discovery used to report the patch as package_not_found. Now the /go.mod line alone counts as the hosted pin, with no locked integrity, and discovery also reads go.work.sum and the go.sum of each workspace member. The writer stops re-adding the zip line on a re-run over such a tidied go.sum.

Risk: low. The only new attestation path covers modules whose packages go doesn't compile: if a package were loaded, -mod=readonly would fail with a missing go.sum entry, so it can't hide unpatched code. The go.sum parsing rules (malformed file, conflicting hashes, non-dirhash h1:) all fail closed. Go locked_integrity was never compared against server data, so leaving it out loses no real check.

Look here

Verified

  • Read the full diff. It doesn't touch CHANGELOG.md and has no debug leftovers.
  • Checked CRLF, blank lines, BOM (fails closed), trailing whitespace and Windows use paths.
  • Sum files are read once, and only when a hosted replace exists.
  • The tests would fail on the old reader and writer.
  • cargo test -p socket-patch-core --lib -- golang go_sum go_build_list: 177 passed. I didn't run the Go-toolchain e2e suites locally; CI ran them.
  • CI on 14057c8: ci-ok and clippy green. Bugbot: neutral; its one thread is resolved, and the author explained it as a false positive.

Changes I made: none.

Open questions (non-blocking; can be follow-ups):

  1. go ≥ 1.17 with a workspace: the inert check uses only the root require. Suppose a member requires and imports M v1.5.0, while a dependency's /go.mod line pins socket M v1.4.2. Then M@v1.4.2 is attested even though the build compiles v1.5.0. The purl is different, so the real component is still reported, but the doc's "inert if another version is selected" isn't fully true in that case.
  2. In workspace mode the root go.sum is read even without use ., and go doesn't read it then.
  3. GoSumLine::extra_tokens in go_sum_edit.rs seems to have no reader left.

Auto-merge is armed, so approving sends this PR straight to the merge queue.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant