Keep vendored re-runs idempotent across patch-service outages by mikolalysenko · Pull Request #250 · SocketDev/socket-patch

Mikola Lysenko (mikolalysenko) · 2026-09-24T00:09:01Z

Summary

In #248's CI, the Bun and PDM native matrices failed when patch.socket.dev returned 503. Under --vendor-source auto, a service failure falls back to a local build. A local tarball has the same members as the service's prebuilt one but different bytes, and every npm-family re-run acquired a new artifact before looking at the lock. So when one run got the service copy and the other built locally, the re-run re-vendored: it reported applied instead of already_vendored, rewrote the lock's integrity, and overwrote the committed tarball. The code comments assumed the two artifacts were "byte-identical by determinism", which holds only when both runs use the same source.

The fix: reuse the committed artifact when it verifies. cargo, go, composer, gem, maven and nuget already worked this way. New vendor/reuse.rs, fail-closed; any miss falls back to today's acquisition. A committed artifact is reused only if all of these hold:

the vendor ledger has an entry for this uuid, under a path that is uuid-bound and inside .socket/vendor/;
no path component is a symlink, and the file is not a FIFO or device;
it is within the size cap;
it is read once, and that buffer is checked against the ledger sha256 and size;
a strict, canonical decode succeeds: only package/ members, no link entries, no duplicate or case-fold aliases;
every patched file matches its afterHash;
for wheels, the leaf is bound to the package's name and version, and the wheel is not platform-tagged.

Where it applies:

npm family: one short-circuit in npm_common::stage_patch_pack fixes npm v2/v3, pnpm v9 and legacy, yarn classic and berry, bun.lock and bun.lockb. Their existing in-sync checks then run against the reused bytes. Dry runs probe too, so they agree with real runs.
pypi: a re-scan after a relock re-wires the committed wheel (Verbose advisory vendor_artifact_reused) instead of rebuilding it. PDM's partial-relock check no longer depends on which source built the wheel. When an artifact is missing during an outage, the error now says what to do.
golang: an in-sync re-run passes under service + --offline, like the other directory backends.
Service client: a bounded retry on 5xx, transport errors and a cut-off body (not on 4xx, and never on an integrity failure). Each attempt has a timeout. A per-run circuit breaker stops a single outage from costing a failed request for every package.
CLI: an in-sync run during an outage no longer prints the misleading "building locally instead" warning.

The integrity gaps found along the way are in the separate PR #249.

Test plan

For each npm flavor, four scenarios: service→503, 503→service, 503→503, and an in-sync --vendor-source service run during an outage. Each asserts that the lock and tarball are byte-identical, the result is already_vendored, and there are no POSTs.
Tamper and forgery cases:
- an edited unpatched member with a stale ledger sha;
- a symlink, a FIFO, or a wrong uuid directory;
- non-canonical tar/zip shapes;
- a forged wheel leaf, a platform-tagged wheel, a forged ledger for bun.lock, and a new uuid.
- Each one misses the reuse check and falls back to acquisition.
pypi/PDM relock re-wire tests, and dry-run parity tests.
wiremock and raw-TCP tests for the retry, breaker and timeouts.
CLI end to end: vendor, then re-run with the service returning 503, and get already_vendored, exit 0, and no lock change.
Most new tests were checked against a mutant: the fix was undone and the test failed.
cargo test -p socket-patch-cli -p socket-patch-core --no-fail-fast: 7886 passed, 0 failed. cargo clippy ... -D warnings: clean.
The bun.lockb end-to-end test and the PDM harness rescanReusesWheel check need Bun and PDM, so CI's native matrices are their first run.

Not in this PR

The integrity hardening is in Harden vendored-mode trust in patch-service artifacts #249.
Reuse when state.json is missing or corrupt keeps today's behavior: the lock is re-pinned.
The trust level is the same as repair/vex: an attacker who forges the tarball, the ledger sha, and the lock together is not detected.

🤖 Generated with Claude Code

Note

Medium Risk
Changes core vendoring acquisition and lock pinning across many ecosystems; behavior is fail-closed with heavy test coverage but mistakes could still affect integrity or outage handling.

Overview
Vendoring re-runs stop re-acquiring packages when the ledger already vouches for the committed archive. npm-family flows now probe reuse in stage_patch_pack before any service call or local pack; verified tarballs yield already_vendored with byte-stable locks and tarballs even when patch.socket.dev flips between prebuilt and local bytes. PyPI re-scans after a relock re-wire the committed wheel (vendor_artifact_reused) instead of rebuilding; golang matches cargo/composer by allowing service + --offline on in-sync re-runs.

Reuse is centralized in vendor/reuse.rs with fail-closed checks (uuid-bound path, no symlinks, ledger sha256/size, strict canonical tarball/wheel decode, patched-member afterHashes). read_archive_bytes_to_map_strict and related helpers enforce installer-equivalent archive shape. Staged npm packs can carry verified_bytes so lock wiring cannot diverge from what was hashed.

The API client adds bounded retries (429/5xx, transport, truncated bodies), per-attempt timeouts, and a run-level circuit breaker after two exhausted fetches. Docs/CLI surface reuse (CLI_CONTRACT, verbose vendor_artifact_reused). Large test coverage covers outage flips, tamper cases, and dry-run parity.

^{Reviewed by Cursor Bugbot for commit d6b41c2. Configure here.}

…uffer Split the tarball and wheel readers into a reader-generic core so a caller can hash and decode the same bytes: read_archive_bytes_to_map and read_zip_bytes_to_map keep the existing bomb caps, entry cap and path-safety gate. Widen checked_artifact_path, verify_member_map and MAX_HEALTH_HASH_BYTES to pub(crate) for the upcoming reuse helper. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…ledger New vendor::reuse module: prior_entry finds the ledger entry anchoring a record's uuid (twins must agree on path + sha256), and verify_committed_artifact checks, fail closed: a non-empty record, a canonical uuid-bound path, no symlink below the project root, one FIFO-safe capped read, sha256 (and size) equal to the ledger, and every afterHash inside the members decoded from that same buffer. It is read-only and never touches the network. Shared test tooling for the source-flip tests lives in vendor::test_support. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…new one stage_patch_pack acquired a new artifact on every run (the service prebuilt, else a local deterministic pack) and every npm flavor then compared the lock's digests with those NEW bytes. The prebuilt and the local pack carry the same members in different encodings, so a source flip between runs (a service outage, or its recovery) re-vendored every package: lock integrity rewritten, tarball overwritten, the CLI reporting applied instead of already_vendored. Reuse the committed tarball when the ledger anchors it (sha256 + size) and every afterHash verifies from the same bytes, before the service offline conflict and any network: an in-sync re-run is now a no-op in every --vendor-source mode, including service + --offline and build. Flavor signatures are unchanged; their in-sync checks now run against the reused facts, so a drifted lock is re-pinned to the verified committed bytes. The reused pack reports uuid_dir_preexisted, so a later wiring failure never deletes it. Tests: a shared flip suite (service->503, 503->service, 503->503, service-mode in-sync under outage and offline, service->service with no request, build after service) per flavor: npm v2/v3, pnpm v9, pnpm legacy v5/v6, yarn classic, yarn berry, bun.lock (direct + nested) and bun.lockb (with workspace mirrors). npm and bun cover the fail-closed edges: re-gzipped tarball, edited unpatched member with a stale ledger sha, forged ledger over an edited patched member, symlinked artifact, FIFO, wrong uuid dir, missing ledger, new uuid, relock re-pin, a package.json-rewriting patch, and a digest-less bun 2-tuple heal. The bun.lockb prebuilt->fallback test now deletes the canonical tarball first; its sibling asserts the flip itself is a no-op. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

A relock that restored the registry unit (the vendored wiring dropped) made the next re-scan a Fresh plan, which acquired the wheel anew: the service when reachable, else a local build with a different sha. The lock then pinned whichever source answered, and a wiring failure swept the uuid dir, deleting the wheel the live ledger entry still names. - Fresh-path reuse: when the ledger entry anchors a verified, portable wheel directly under the uuid dir, re-wire those exact bytes (no service call, no build, before the offline conflict) and emit a Verbose vendor_artifact_reused advisory. - A wiring failure never sweeps a reused wheel. - The PDM partial-relock guard matches every sha the patch's wheel is known by (this run's and the ledger's), so it refuses whichever source built the wheel; wire_pdm takes known_patched_sha256. - The in-sync missing-artifact rebuild names the service outage when the local rebuild cannot reproduce a prebuilt pin, instead of only advising revert + re-vendor. - The ledger entry is read once for the rebuild pin, the reuse anchor and the PDM guard. Tests: the analysts' flip regression for poetry, pdm, pipenv, uv, requirements and hatch in both directions; relock re-scan reuse for pdm/uv/poetry both directions (0 requests, first sha pinned); reuse under service + offline; PDM partial relock with the wheel present (reused, not swept) and deleted (prior-sha guard); the outage message; a platform-locked entry not reused; service-mode in-sync under outage. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

golang checked service_offline_conflict before its in-sync hot path, so an already-vendored module refused under --vendor-source service --offline while cargo and composer return already_vendored. Move the check below the hot path, as cargo.rs does; real acquisition still refuses. Also keep the analysts' source-flip regressions for the directory backends (cargo, golang, composer, gem, maven, nuget): a service <-> local flip in both directions is a byte-identical no-op with no request. They pass before and after this change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…aker A single 503 or dropped connection on the package-reference POST or the archive GET decided the source for that package, and every package of an outage paid its own failing round trip. Retry both round trips on transport errors and HTTP 429/500/502/503/ 504: three attempts, exponential delays from 400ms with +/-25% jitter, capped at 4s, honoring Retry-After seconds under the same cap. Auth (401/403), terminal misses (404/410), still-building (408), other 4xx and parse errors are never retried. After two consecutive fetches end in a retryable failure, the rest of the run skips the service without I/O (per client, shared by clones: one CLI run); a Ready, Pending or Unavailable answer resets the count. The auto/service miss policy is unchanged. ApiClient::with_vendor_retry overrides the policy; the shared vendor test config disables retries. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…to end - CLI: vendor_artifact_reused prints only under --verbose (it explains a successful relock re-wire, like vendor_prebuilt_downloaded). - e2e (covgap_commands_vendor): npm package-lock and bun.lock, both flip directions against a mocked service: the re-run exits 0 with applied=0, skipped=1, one already_vendored event, no vendor_prebuilt_unavailable, the lock and tarball byte-identical and no package request; a PDM relock re-scan during an outage re-wires the committed service wheel (vendor_artifact_reused, no request) and rollback restores the relocked bytes exactly. - e2e_bun_lockb: the in-sync rerun repeated against a closed vendor port. - Harnesses: backtest-bun repeats each vendored re-run with SOCKET_VENDOR_URL at a closed port (repeatOutageStableLock, repeatOutageClean); backtest-pdm checks the relock re-scan wires the first scan's patched sha (rescanReusesWheel). - CLI_CONTRACT: --vendor-source governs acquisition, not reuse; the retry and breaker; the vendor_artifact_reused code. CHANGELOG entry. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…e edges A patched member edited under a forged ledger sha is never reused or pinned, and a new record uuid acquires under its own uuid dir while the old uuid's artifact is left alone (the npm_lock twins already exist). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

npm, pnpm, yarn and bun strip the first path segment whatever it is, extract type-'7' entries as files, and let case-variant names overwrite each other on a case-insensitive filesystem. The lenient decoder strips only a literal `package/`, keeps Regular entries and lets the last duplicate win, so a committed tarball could pass every afterHash check (with a recomputed ledger sha) and still install unpatched code; reuse would then keep it, and re-pin a drifted lock to it with no network. Reuse now decodes strictly: every tarball entry under `package/`, only Regular/Directory entries (pax / GNU long-name headers skipped), and no exact or ASCII-case-folded duplicate names; wheels get the same name rules plus no symlink entries. Any violation is a NonCanonical miss, so the run falls back to today's acquisition. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…ew it in dry runs The Fresh-path wheel reuse took its filename from the committed ledger's artifact.path and checked only for `/` and a `.whl` suffix; the wirings splice that name verbatim, so a forged ledger could inject a requirements.txt option line (`--trusted-host ...`) or wire another distribution's wheel. The leaf must now be a well-formed PEP 427 name in the wheel charset whose name and version are this package's. A platform-specific wheel is also skipped when its own filename tags say so, not only when the ledger carries platform_locked: true, so a ledger without the flag no longer re-wires a foreign-OS wheel silently. The probe is read-only and offline, so dry runs now run it too and return a verified preview (plus the reuse note) instead of calling the acquisition, whose `service` + `--offline` refusal made the dry run predict a failure the real run does not have. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

The reuse probe ran only for real runs, so a dry run of an in-sync package under `--vendor-source service --offline` hit the offline refusal while the real run of the same command succeeded as already_vendored. The probe is read-only and offline: dry runs now run it too, and on a hit skip the offline refusal and keep previewing the local build (same result shape as before). Covers every npm flavor, since all of them go through stage_patch_pack. Also pins two reuse details nothing tested: a relock re-pinned from the reused bytes recomputes the dependency mirror from their patched package.json (npm and yarn classic), and a wiring failure after a reuse never unstages the uuid dir holding the committed tarball. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…all bytes After a reuse, yarn berry re-read the tarball with a plain tokio::fs::read and derived hash= and the cache checksum from that second read, so a file swapped between verification and the read could get pinned (or a FIFO could hang the run). The reuse now hands the exact bytes it hashed and verified to the flavor through NpmStagedPack::verified_bytes, and a fresh pack's re-read must still hash to the sha256 the pack recorded, or the run fails and unstages. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…ad error The retry policy promised bounded latency, but no vendor request carried a timeout: a black-holed host cost the OS connect timeout per attempt (about 3 x 75s per package on macOS before the breaker tripped), and a server that accepted and stalled hung forever. VendorRetryPolicy now carries attempt_timeout (30s: the whole POST round trip, and the GET's connect + response headers) and body_timeout (300s: the archive body); a timeout is a retryable transport failure. read_capped gains a typed twin (Truncated / CapExceeded) so the GET's retry decision no longer depends on the error string's prefix: a body cut off mid-transfer is retried, a cap breach is not. Both are now tested against a raw TCP server. The open breaker's reason now reads correctly inside the callers' "patch service request failed (...)" wrapper: "not attempted: the service failed for the previous 2 packages in this run". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…lients Only test_support::service_cfg opted out of the default 3-attempt policy; the golang, cargo, composer, maven, nuget, gem, npm, pypi and service_fetch test helpers still paid ~1.2s of backoff per 503 or closed-port case and silently changed their request counts. They now chain with_vendor_retry(VendorRetryPolicy::none()). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…empt timeouts Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…e-pack The re-run assertions (applied 0, skipped 1, lock unchanged) also held before the fix, since the outage fallback re-packed the same deterministic tarball. Also require exactly one already_vendored event, no vendor_prebuilt_unavailable event, and a byte-identical tarball. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…-wires rescanReusesWheel only checked that the first scan's patched sha came back, which also holds on the pre-fix CLI whenever the source does not flip between scans. When the relock dropped the vendored reference the re-scan must now also report vendor_artifact_reused. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

cursor

Stale Bugbot comment from a previous run.

Mikola Lysenko (mikolalysenko) · 2026-09-24T13:55:08Z

BugBot review

Mikola Lysenko (mikolalysenko) · 2026-09-24T13:55:14Z

Claude (@claude) review

…onger refuses Gate service_offline_conflict on copy_was_ok instead of the wet-only hot path, so the preview matches the real run (Bugbot). Also skip the newline-leaf forged-ledger case on Windows, where such a filename is invalid (os error 123). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

cursor

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

…outage-idempotence Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Mikola Lysenko (mikolalysenko) and others added 17 commits September 23, 2026 17:07

docs(changelog): note canonical-archive reuse, dry-run parity and att…

8651115

…empt timeouts Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

cursor Bot reviewed Sep 24, 2026

View reviewed changes

Comment thread crates/socket-patch-core/src/vendor/golang.rs

Wenxin Jiang (Wenxin-Jiang) approved these changes Sep 24, 2026

View reviewed changes

cursor Bot reviewed Sep 24, 2026

View reviewed changes

Merge origin/main (#249 integrity hardening) into fix/vendor-service-…

fdc2527

…outage-idempotence Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Mikola Lysenko (mikolalysenko) merged commit e024629 into main Sep 24, 2026
199 checks passed

Mikola Lysenko (mikolalysenko) deleted the fix/vendor-service-outage-idempotence branch September 24, 2026 14:44

Mikola Lysenko (mikolalysenko) mentioned this pull request Sep 24, 2026

feat(vex): manifest-less VEX from hosted/vendored lockfiles + npm 12 allow-remote auto-config (v5) #251

Merged

Navigation Menu

Keep vendored re-runs idempotent across patch-service outages - #250

Keep vendored re-runs idempotent across patch-service outages#250
Mikola Lysenko (mikolalysenko) merged 19 commits into
mainfrom
fix/vendor-service-outage-idempotence

Mikola Lysenko (mikolalysenko) commented Sep 24, 2026 •

edited by cursor Bot

Loading

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Sep 24, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Sep 24, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Mikola Lysenko (mikolalysenko) commented Sep 24, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Not in this PR

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Sep 24, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Sep 24, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mikola Lysenko (mikolalysenko) commented Sep 24, 2026 •

edited by cursor Bot

Loading

cursor Bot left a comment •

edited

Loading