Skip to content

Fix Go patch delivery, audits, and reinstalls - #1698

Open
Mikola Lysenko (mikolalysenko) wants to merge 10 commits into
mainfrom
fix/go-patch-delivery-e2e
Open

Mikola Lysenko (mikolalysenko) wants to merge 10 commits into
mainfrom
fix/go-patch-delivery-e2e

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

The pgx/v5 smoke test exposed local Go patches that could be attested while Go built vulnerable bytes from native vendor/, a missing warning in agent get, and rollback that stopped building after go mod tidy removed upstream checksums. Reapplying an intact agent patch after deleting the module cache also failed with package_not_installed. The production Go canaries passed without proving delivery.

Changes

  • Verify the native vendor tree Go selects before VEX attestation and in apply --check / vendor --check. Cover workspace markers, quoted and double-hyphen GOFLAGS, unused replacements, package selection, ignored files, embedded assets, and dependency-only patches.
  • Reuse an intact project-local agent copy after module-cache deletion, checking patch hashes, selected version, and replacement wiring. Refuse corrupt copies without deleting them or claiming success.
  • Forward Go consumer synchronization warnings through nested agent operations.
  • Save upstream checksum lines beside local artifacts and restore them on rollback. Refuse checksum conflicts while preserving the active replacement and copy, including orphan reconciliation. Preserve dry-run behavior and checksum backups across mode changes.
  • Replace empty production canaries with the real github.com/jackc/pgx/v5@v5.7.0 patch d8941a4c-4bcd-4756-b0aa-239ec9c81b0f: require a delivery event and reject the malformed decoder input without a panic. Exercise get/scan, fresh caches, cache removal, same-version go get, go mod download/tidy/verify, consumer tests, repeated native vendoring, offline rollback after tidy, and explicit cold-cache agent failure followed by recovery. Hosted/vendored workspace tests also exercise go work sync and go work vendor when supported.

The cold-cache false applied result from the older smoke-test revision was already fixed on current main; this PR adds a live regression guard for the corrected failure behavior.

Validation

  • Real production pgx patch: agent and vendored get/scan/workspace/native-vendor smoke cases passed, including fresh checkouts and rollback after tidy. Live reinstall canaries passed on macOS Go 1.24.13 and 1.26.3 and on Linux ARM64 as UID 1000 with Go 1.21.13, 1.24.13, and 1.26.3. Native-vendor tampering is rejected by audit and VEX in both modes.
  • Controlled hosted pgx delivery passed with a local Go proxy using the real patched artifact, including fresh readonly builds, both file hashes, manifest-free VEX, and wrong-checksum rejection. This is not a production hosted pass.
  • Go E2E suites passed on macOS ARM64 with Go 1.24.13 and 1.26.3 and Linux ARM64 as UID 1000 with Go 1.18.10, 1.21.13, 1.24.13, and 1.26.3 (109 checks per four-suite run; Go 1.18 uses compatible synthetic fixtures because pgx requires Go 1.21). The compatibility workflow is disabled in GitHub, and dispatch was refused; these are local runs, not GitHub compatibility-job passes.
  • Focused core checks, formatting, and the CI-equivalent workspace/all-features Clippy command passed. The final macOS Go 1.26 run passed all 109 checks; the final core run passed 459 unit tests plus the matching integration checks.
  • Draft PR checks are green on 5c4066702 (32 settled checks). Heavy draft-gated jobs were validated locally. Two high-effort Codex CLI reviews were run; confirmed findings were fixed and given regression coverage.

Review findings not fixed

/code-review max verified 15 findings; all are fixed in 6341c0c (core no longer depends on glob: go_native_vendor.rs ports Go's path.Match). These verified findings are deferred to follow-ups:

  • .upstream-go.sum lines are not forgotten after a vendored/hosted takeover or a hand-deleted directive.
  • go.sum restore accepts any h1: prefix.
  • CLI_CONTRACT.md still says a Go revert never touches go.sum.
  • New e2e tests don't pin GOFLAGS/GOENV.
  • The production Go helper needs -modcacherw.
  • The reuse loop iterates the manifest unsorted.
  • ensure_replace_entry errors drop the go.mod path.

A later /code-review high found 10 more (unverified). Fixed in 4eec336 and 67abc02: conflicting checksums merged into the backup, duplicate apply --check drift for one modules.txt cause, go.sum written before a failed directive edit, files with two or malformed //go:build lines treated as vendored, a bare module path reported as a PURL, and an eager selection probe. Deferred as efficiency/cleanup follow-ups:

  • vex refetches the patched go.mod blob on any native vendor failure, not only a missing go.mod.
  • verify_native_vendor(_tree) re-resolves GOFLAGS, modules.txt and replace entries per purl.
  • The read-entries / drop / forget sequence is duplicated across four call sites.
  • save reads and conflict-checks the go-patches backup twice.

Kept on purpose: a machine-wide GOFLAGS=-mod=mod disables the native-vendor check, as this PR's own test expects.

Rollout dependency

Companion backend PR: https://github.com/SocketDev/depscan/pull/28901

Met 2026-10-29. The backend fix is deployed (depscan deploy/prod/2026-10-26T19-24-34Z), and the pgx patch was regenerated in production. patch.socket.dev/gopatch/d8941a4c-…/v5@v5.7.0-socketpatch.1 now resolves, and the strict hosted canary golang_hosted_pgx_delivery passes in hosted-e2e on 67abc02cc. Rebuilding Go patches that already published a hosted module is handled separately in SocketDev/depscan#29337.

Existing checkouts created by older CLI releases without a checksum backup may still need go mod download <module>@<version> after rollback; newly patched checkouts restore their saved checksums offline.


Note

High Risk
Changes attestation and audit gates for Go (VEX, CI check) and mutates go.mod/go.sum rollback semantics; mistakes could block valid builds or attest unpatched vendor trees.

Overview
Go patching and verification now treat native vendor/ as part of the truth: apply --check, vendor --check, and vex can fail with go_vendor_content_mismatch when the tree a default build compiles does not carry the patch, even if the .socket replace copy is fine. Apply also emits consolidated consumer-sync + native-vendor warnings via go_sync_warnings, and apply --check skips duplicate drift when modules.txt already explains the same module.

Agent/local Go workflow changes: stale go-patches redirects are reconciled before the apply loop; orphans that cannot be dropped (e.g. go.sum conflicts with saved upstream checksums) are kept and surface as failed apply results while other patches still run. reuse_go_redirect reuses a committed copy after GOMODCACHE loss when hashes and wiring match. .upstream-go.sum beside local copies backs up upstream checksum lines for offline rollback after go mod tidy; conflicting pins refuse rollback/reconcile without tearing down active replaces. Nested scan/get no longer duplicates warnings already on the envelope when recording apply outcomes.

Production and fixture E2E coverage replaces empty Go canaries with a real pgx/v5 delivery proof plus broad tests for native-vendor drift, workspace vendoring, checksum-conflict orphans, and cold-cache agent behavior.

Reviewed by Cursor Bugbot for commit de070ed. Configure here.


Generated by Claude Code

Verify the native vendor tree selected by Go before attesting patches.
Account for workspaces, GOFLAGS, selected packages, embedded assets,
and dependency-only fixes. Forward consumer synchronization warnings
through nested agent get operations.

Preserve upstream checksum lines with local patch artifacts and restore
them on rollback after tidy. Refuse checksum conflicts without removing
an active replacement or its copy, including orphan reconciliation.

Replace empty production Go canaries with pgx/v5 execution proofs for
hosted, vendored, and agent delivery, fresh caches, cold agent failure
and recovery, and rollback. Add real Go regressions for native drift
and the lifecycle failures reproduced by the smoke repository.

Assisted-by: Codex:GPT-6
Reuse verified project-local agent copies after clearing the Go module
cache. Refuse corrupted copies without an upstream rebuild source.
Exercise get, scan, dependency maintenance, repeated native vendoring,
and workspace synchronization with the patched pgx decoder.

Assisted-by: Codex:gpt-6-astra
@mikolalysenko Mikola Lysenko (mikolalysenko) changed the title Fix Go patch audits, rollback, and delivery E2Es Fix Go patch delivery, audits, and reinstalls Oct 23, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: draft CI is green on 5c406670 (6 passed, 31 skipped/draft-gated; heavy jobs were validated locally per the description). I'm not marking this ready because the description says it must stay a draft until the backend companion SocketDev/depscan#28901 is deployed and the legacy pgx patch is regenerated, and the strict production hosted canary must pass first.

Question for a human: once depscan#28901 is deployed and the pgx /v5 hosted module is published, mark this ready for review (full CI will run then). Until that happens, the burn-down agent will leave this PR alone.


Generated by Claude Code

@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.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/patch/redirect/golang_local.rs Outdated
Comment thread crates/socket-patch-core/src/vendor/go_sum_backup.rs
Comment thread crates/socket-patch-core/src/vendor/go_consumer_sync.rs Outdated
Resolve conflicts with main's advisory-warning refactor (#1722) and the
global Go tool tests (#1717). apply.rs keeps main's advisory_warning_codes
loop (it covers ownership_not_restored too) and drops main's
nested_skip_warnings, because this branch already reports those skips
through run_warnings. e2e_golang_build.rs keeps both sides' new tests.

Co-Authored-By: Claude <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: merged origin/main (8b32765) into this branch at e3eb1e0 to clear the merge conflict.

  • apply.rs: kept main's advisory_warning_codes loop (Fix stale Python bytecode after apply (#819) #1722), which also emits ownership_not_restored, and dropped main's nested_skip_warnings, since this branch already reports nested user-patch skips through run_warnings.
  • tests/e2e_golang_build.rs: kept both sides' new tests (this branch's rollback/vendor tests and main's global-tool tests from Fix global Go apply leaving installed tools stale (#422) #1717).
  • Local: cargo check -p socket-patch-cli --all-targets clean; cargo test -p socket-patch-cli --lib 934 passed; --test e2e_golang_build 33 passed (Go 1.24).

Draft hold and agent:needs-human unchanged; awaiting CI on the new head.


Generated by Claude Code

Apply the 15 verified findings from /code-review max on #1698, including
porting Go's path.Match into go_native_vendor.rs so core drops the glob dep.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…fix/go-patch-delivery-e2e

# Conflicts:
#	crates/socket-patch-cli/src/commands/apply.rs
…-e2e

# Conflicts:
#	crates/socket-patch-cli/tests/e2e_golang_build.rs
#	crates/socket-patch-cli/tests/e2e_vendor_golang_build.rs
#	crates/socket-patch-core/src/vendor/go_consumer_sync.rs
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: adding agent:needs-human. This draft is blocked on external state, not code: hosted-e2e (strict production hosted canary) on ba1e504 still fails because the legacy pgx patch has to be regenerated in production after SocketDev/depscan#28901. Question: has the pgx patch been regenerated in production? If yes, re-run hosted-e2e; once it is green, a human marks this ready. I am not closing it or marking it ready.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: marking this agent:needs-human.

On ba1e504, every check passes except hosted-e2e (and ci-ok, which depends on it). golang_hosted_pgx_delivery fails all 3 attempts with redirect_golang_unsupported: production publishes no hosted Go module for github.com/jackc/pgx/v5@v5.7.0. The same check is green on main, so this is not a main-wide failure. This PR's strict canary can't pass until production state changes, and no code change here can fix that.

Question: can someone regenerate/publish the pgx/v5@v5.7.0 patch as a hosted Go module in production? If that won't happen soon, should the strict hosted pgx canary stay out of this PR so the rest can land? Once it's published, re-running hosted-e2e should make this green.


Generated by Claude Code

Resolve conflicts with main's go.work replace hosting (#1763, #1764,
#1765). ensure_replace_entry now validates and backs up go.sum before
writing the replace into main's replace host (go.work or go.mod), and
drop_replace_entry restores the saved go.sum lines before removing the
replace from go.work and go.mod. The cleanup fallback uses the same
go.work-aware removal now that edit_go_mod is gone. The workspace e2e
test keeps this branch's reinstall steps on main's tail(&Tail{..}) shape.

Co-Authored-By: Claude <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: merged origin/main (ecd4924) into this branch at 7353cc0 to clear the merge conflict.

  • go_mod_edit.rs: main now writes replaces into its replace host (go.work or go.mod, Write shared Go patch replaces into go.work #1764). This branch's go.sum validation and backup run before that write, and drop_replace_entry restores go.sum before removing the replace from go.work and go.mod. edit_go_mod is gone on main, so the cleanup fallback uses a new go.work-aware remove_from_replace_files helper.
  • go_consumer_sync.rs: kept this branch's GOFLAGS -mod parsing and took main's removal of the workspace_file doc comment.
  • e2e_golang_workspace_build.rs: kept the reinstall_workspace steps on main's tail(&Tail{..}) shape.
  • Local checks: cargo check -p socket-patch-core -p socket-patch-cli --all-targets passes; core --lib go tests pass (987); --test e2e_golang_workspace_build passes (30).

agent:needs-human stays: the hosted pgx Go module question is still open.


Generated by Claude Code

save() unioned live go.sum lines with an existing .upstream-go.sum, so a
different h1 for the same module and version was kept beside the old one,
and a rollback after go mod tidy wrote both back. Refuse the conflict at
save time, and refuse a backup that already holds conflicting lines at
restore time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- apply --check: skip the native vendor check for a module whose
  modules.txt drift the audit already reports, so one cause gives one
  drift (matches go_sync_warnings).
- drop_replace_entry: plan the go.sum restore, remove the directive, and
  only then write go.sum, so a failed go.mod/go.work edit leaves both
  unchanged.
- Native vendor: treat a file with two //go:build lines or an
  unparseable expression as omitted, as imports.ShouldBuild does.
- reconcile: report a failed drop of a version-less directive under a
  PURL built from its copy path, not the bare module path.
- Only probe selection when replace_refusal did not already refuse.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Production pgx/v5 hosted module is live after the patch was regenerated: hosted-e2e on 67abc02cc passes, including the strict canary golang_hosted_pgx_delivery. Removed agent:needs-human. All review threads are resolved; the /code-review high findings were fixed in 4eec33615 and 67abc02cc (deferred items are listed in the description). The remaining red check is a Gradle e2e job whose Depot runner dropped twice; it's been rerun. This needs an approval from someone other than the author.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

1 similar comment
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@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.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/patch/redirect/golang_local.rs Outdated
Comment thread crates/socket-patch-core/src/vendor/go_consumer_sync.rs
reuse_go_redirect reported an intact agent copy as already patched from
redirect_in_sync alone, so a go.work workspace whose root go.mod mirror
of the replace had gone missing stayed unrepaired after the module cache
was deleted, and a GOWORK=off build kept linking the unpatched module.
apply_go_redirect's hot path already wrote the mirror back. Share that
repair between both paths, skipping it on a dry run.

Co-Authored-By: Claude <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@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.

✅ 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 de070ed. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 30, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at de070edc5152c166aa55b3cd4c23fda193cec85e.

  • CI: 31/31 run checks green (50 skipped by path filters); no main-wide failures.
  • Bugbot: reviewed de070ed and found no issues. No review threads are open: the mirror-repair finding was fixed in this head and the -mod= finding was refuted earlier.
  • Mergeable: yes, with no conflicts. CHANGELOG.md is untouched. Only human approval is needed.
  • Slack: not announced, because Slack sending isn't available in this run. The next run will retry.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief: de070ed

What it does. Go directory-replace modes (agent go-patches and vendor) now back up the replaced module's go.sum lines in .upstream-go.sum. Rollback, reconcile and failure cleanup restore those lines offline after go mod tidy drops them, and refuse when the saved checksum conflicts. A new verifier checks Go's native vendor/ tree for vex, apply --check and vendor --check. apply can also reuse a committed go-patches copy after the module cache is cleared, and reconcile failures are now reported instead of ignored.

Risk: medium. It touches 22 files (about 3,300 lines), adds new failure paths to apply/revert/reconcile, and re-implements about 1,000 lines of Go behaviour (path.Match, //go:embed, build constraints). If that code differs from Go, the result is a false drift report, never a wrong attestation.

Look here

Verified

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • Core unit tests for go_sum_backup, go_native_vendor, go_consumer_sync, go_mod_edit, golang_local and vendor::golang: 178 passed.
  • CLI e2e with real Go 1.24.7: the 6 new e2e_golang_build tests, native_go_vendor_drift_and_tidy_rollback and golang_get_uuid_hosted_day2_machine_builds all pass. The new tests check behaviour that doesn't exist on main.
  • Paths taken from go.mod and the manifest are validated (is_safe_relative_subpath, are_safe_redirect_coords). CHANGELOG.md is untouched.
  • CI: 31/31 run checks are green (50 skipped by path filters). Bugbot found no issues on de070ed, and no review threads are open.
  • I didn't run the live production tests, which are #[ignore] and need network access.

Changes I made: none.

Open questions (non-blocking, fine as follow-ups)

  1. apply --check and vendor --check pass an empty known map. A hosted Go patch that changes go.mod, in a natively vendored repo with an empty cache and no committed blobs, could report "cannot verify the patched go.mod requirements". vex fetches the blob and avoids this.
  2. .upstream-go.sum can be left behind in two cases: save runs before a go.mod edit that is then refused, and vendor takes over a go-patches redirect. The leftover file is harmless, but it stays committed.
  3. expected_text compares a known hash case-sensitively, while patched_blob ignores case.
  4. run_locked now passes every non-stage warning on to nested callers. golang_hosted_pgx_delivery now fails outright when production doesn't serve the pgx artifact. Both are deliberate, but neither is mentioned in the description.

Auto-merge is armed, so approving sends this 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.

2 participants