fix(scan): scope takeover cleanup per package, warn on hosted --prune - #191
Merged
Mikola Lysenko (mikolalysenko) merged 8 commits intoAug 15, 2026
Merged
Conversation
The mode-takeover warning told users to delete the whole redirect ledger (vendored direction) or the whole .socket/vendor/<eco>/ tree (hosted direction). Following it destroyed live data for packages the takeover never touched: other packages' VEX records, the only recorded pre-redirect lockfile originals, and still-live vendored artifacts. The remediation is now per-package and non-destructive — run `socket-patch remove <purl>` per named package (hosted direction) or delete only the named records from the redirect ledger (vendored direction) — and explicitly warns against whole-file/tree deletion and blanket reverts. `scan --mode hosted --prune` silently dropped --prune: both hosted terminals return before the GC blocks, so a bot migrating from `--mode agent --prune` stopped pruning forever with exit 0 and no signal. The flag stays accepted (an orthogonal knob per the CLI contract), but the run now emits an explicit `redirect_prune_ignored` warning on stderr and in the JSON `redirect.warnings[]`, including the zero-discovery envelope. Two takeover blind spots are also closed. A redirect ledger whose every record fetch failed (edits present, records empty) was invisible to overlap detection; the vendored purls are now matched against the recorded edit keys so the stale ledger is still flagged. And hosted liveness was proved only by a hardcoded patch.socket.dev inventory URL — structurally unprovable for yarn-berry (inventory resolved is always None), bun (URL 3-tuples are skipped), and any non-default patch host. It is now proved by the record's patch uuid: in the inventory URL on any host, or in the redirect-edited lockfile text outside a .socket/vendor/<eco>/ path (vendored wiring embeds the same uuid, so only non-vendored occurrences count). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a9f2e45. Configure here.
Wenxin Jiang (Wenxin-Jiang)
approved these changes
Aug 14, 2026
Mikola Lysenko (mikolalysenko)
enabled auto-merge (squash)
August 14, 2026 20:25
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
One conflict, in `scan/mod.rs`'s `classify_overlap_takeover`: this branch adds a redirect-ledger read there (to recover each record's patch uuid and the lockfiles the redirect edited, for the host-agnostic hosted-liveness proof), while main's fail-closed ledger work changed `load_redirect_state` to return `Result<Option<_>, CorruptRedirectState>`. Both sides kept: the new read now goes through `.ok().flatten()`, so a malformed ledger reads as a missing one — matching how main's `overlapping_ledger_purls` already treats corruption on this warnings-only path, and unreachable in practice since a non-empty overlap from that function is this one's precondition. Everything else combined cleanly: main's ledger-aware `updates[]`, dry-run VEX guards and ecosystem filter sit alongside this branch's per-package takeover remediation, the `redirect_prune_ignored` warning on both the human and JSON hosted paths, and the degraded-ledger overlap fallback. Assisted-by: Claude Code:claude-opus-5
The PR branch had its own merge of main pushed while this one was in flight. Both resolved the same `classify_overlap_takeover` conflict the same way — the redirect-ledger consult reads through `.ok().flatten()` now that `load_redirect_state` returns a `Result` — so the only conflict left was the explanatory comment above it. Kept the published wording. Assisted-by: Claude Code:claude-opus-5
Main advanced past the earlier merge (install.socket.dev, fail-closed pnpm v5/v6 lock keys, the gem hosted capstone, pnpm node-linker=pnp detection). One conflict, in `tests/in_process_redirect.rs`: both sides appended to the end of the file — this branch's hosted `--prune` warning contract test, main's cargo hosted-mock helpers and table-form pin test. They are independent, so both were kept. Assisted-by: Claude Code:claude-opus-5
Composer redirect work (#195) landed on main. Same conflict shape as the previous merge, in `tests/in_process_redirect.rs`: both sides append to the end of the file — this branch's hosted `--prune` warning contract test, main's composer redirect fixtures and tests. Independent, so both were kept. Assisted-by: Claude Code:claude-opus-5
Picks up #176 (gem/pypi/maven singleton variant mismatch policy and the variant-scoped rollback before-blob gate). No conflicts: that work sits in apply.rs/rollback.rs and their tests, disjoint from this branch's scan-side takeover remediation and hosted --prune warning. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Both mode-takeover remediations were incomplete in ways that hurt the operator who followed them. The vendored-direction text named only the stale package's `records` entries. When the takeover cleared the LAST record, the leftover `edits` still matched the package through the degraded-ledger fallback, so the identical warning fired on every later run — repeating advice that could no longer be carried out, since `records` was already empty. The text now names the matching `edits` entries too (that package's stale pre-redirect originals, which a later redirect revert would replay over the live vendored wiring), and says why both halves matter. Following it in full now leaves nothing to warn about. The hosted-direction text said `socket-patch remove <purl>` "drops only that entry and its own artifact directory". It also deletes the package's `.socket/manifest.json` entry, so a reader budgeting for a ledger-scoped edit — a bot passing --yes especially — was mis-told the blast radius. The text now states that, places the live hosted patch (recorded in the redirect ledger, which `remove` never touches), notes that in-place file rollback is skipped for vendor-owned packages, and suggests previewing with --dry-run. Detection is unchanged: the degraded-ledger blind spot stays closed, because a hand-cleaned ledger and one left by failed record fetches are indistinguishable from their contents. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
deleted the
fix/takeover-remediation-safety
branch
August 15, 2026 00:13
Mikola Lysenko (mikolalysenko)
added a commit
that referenced
this pull request
Aug 15, 2026
Refines the scan/mod.rs conflict resolution in the preceding merge of #187 and #191, which crossed with this branch's takeover work. The vendored-direction remediation had taken #191's text verbatim, which tells the operator to hand-edit .socket/vendor/redirect-state.json and delete each superseded package's `records` entry plus its matching `edits`. #191 could not have known this branch makes that automatic: re-running `socket-patch vendor` (or `scan --mode vendored`) now reverts the stale hosted edits from the ledger and drops both halves of the entry itself. The text leads with that and keeps #191's by-hand procedure as the fallback, along with its records-AND-edits reasoning and its never-delete-the-ledger-file warning. Restores the comment explaining why the ledger is never offered up for hand deletion. The hosted direction keeps #191's per-package `remove <purl>` guidance; its comment now also records why `vendor --revert` is not offered -- it unwinds every vendored package, including ones still live in the lockfile. Both sides' tests pass together: #191's following_the_vendored_remediation_clears_the_warning and hosted_remediation_states_removes_full_blast_radius alongside this branch's cargo classifier and GC reclaim tests. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Problem
Four audit findings (2026-08-13 adversarial audit of origin/main 4e5288e) around the mode-takeover warning and hosted-mode flag handling:
Destructive takeover remediation (medium) — the takeover warning fires per package, but its cleanup advice was file/tree scoped. Vendored direction told users to delete the whole
.socket/vendor/redirect-state.json, destroying other packages' live VEX records and the only recorded pre-redirect lockfile originals. Hosted direction told users to delete the whole.socket/vendor/<eco>/tree (which holds every vendored uuid dir, including still-live packages) or run a blanketvendor --revert(mass-reverting unrelated packages). Concrete failure: redirect packages A and B, vendor A, follow the warning literally →vexstops attesting B and B's pre-redirect lock fragment is unrecoverable.scan --mode hosted --prunesilently drops--prune(low) — both hosted terminals return before the GC blocks, so a bot migrating from--mode agent --prunestops pruning forever with exit 0, no warning, and a JSON envelope that simply omits thegckey.Takeover detection blind to degraded ledgers (low) — a hosted run where every record fetch failed persists a ledger with edits but an empty
recordsmap;overlapping_ledger_purlsderived the overlap exclusively from record keys, so a later vendored takeover of those same packages emitted no warning at all.Hosted direction proof hardcoded
patch.socket.devand could not see berry/bun wiring (low) — liveness was proved only by the inventoryresolvedURL containing the default hostname, so non-default patch hosts (staging,--patch-server-urldeployments) were unprovable, and yarn-berry (inventoryresolvedalwaysNone) plus bun (URL 3-tuples skipped by the inventory) were structurally unprovable.No findings were skipped. Note: the in-flight branch
work/mode-takeover-fixis content-identical to what is already merged as PR #164 and covers neither blind spot, so nothing here duplicates it.Fix
mode_takeover_detailnow gives per-package, non-destructive cleanup. Hosted direction: runsocket-patch remove <purl>per named package — its revert path is drift-guarded (vendor_lock_entry_drifted), so when hosted has won the lockfile it drops only that ledger entry and its own.socket/vendor/<eco>/<uuid>/dir without touching the live wiring — with an explicit warning against tree deletion and blanket reverts. Vendored direction: delete only the named packages' entries underrecordsinredirect-state.json, with an explicit warning against deleting the file (it holds other packages' live records plus the pre-redirect originals inedits).--prunewith--mode hostedstays accepted (CLI_CONTRACT documents it as an orthogonal knob that never conflicts, so a hard error would break existing pipelines) but now emits an explicitredirect_prune_ignoredwarning on stderr (human path, once inrun) and in the JSONredirect.warnings[], including the zero-discovery envelope. CLI_CONTRACT.md and the arg docs name the code.overlapping_ledger_purlsfalls back to matching vendored purls against the recordedFileEditkeys (name,name@version,/namesuffix) whenrecordsis empty buteditsexist. The fallback only activates on the degraded shape, so the well-formed-ledger path is unchanged, and the direction gate inclassify_overlap_takeoverstill requires live-lock proof before anything is reported.Hosted liveness is now proved host-agnostically by the record's patch uuid: in the inventory
resolvedURL on any host (patch.socket.devkept only as a no-record fallback), or in the redirect-edited lockfile text outside a.socket/vendor/<eco>/path — making berry (::__archiveUrl=bindings) and bun (URL 3-tuples) provable. Vendored wiring embeds the same uuid in its path, so only non-vendored occurrences count (guard test included); ledger-named files pass through the same traversal guard asvendored_wiring_live(extracted asis_safe_project_rel_file). Unprovable direction still stays silent rather than guessing.Testing
cargo test -p socket-patch-cli --lib— 373 passedcargo test -p socket-patch-cli --test cli_parse_scan— 48 passedcargo test -p socket-patch-cli --test in_process_scan— 23 passedcargo test -p socket-patch-cli --test in_process_redirect— 24 passedNew tests:
takeover_detail_remediation_is_per_package_and_non_destructive(verified red against the pre-fix remediation text)overlap_detected_when_redirect_ledger_has_edits_but_no_recordshosted_direction_provable_on_non_default_patch_hosthosted_direction_provable_for_bun_url_tuplehosted_direction_provable_for_berry_archive_urlvendored_path_uuid_does_not_prove_hosted(guard: a vendored-path uuid must not prove hosted)hosted_prune_emits_explicit_ignored_warning(subprocess--jsoncontract: warning present, nogckey, redirect unaffected)🤖 Generated with Claude Code
Note
Medium Risk
Changes warning text and overlap/takeover logic that drives CI bots and cleanup actions; behavior is safer but mis-detection could still mis-route users; no auth or network contract changes.
Overview
Fixes scan mode-takeover warnings and hosted-mode flag handling so automation and operators get accurate signals and safe cleanup guidance.
Per-package takeover remediation —
mode_takeover_detailno longer tells users to delete wholeredirect-state.jsonor entire.socket/vendor/<eco>/trees. Hosted wins: usesocket-patch remove <purl>per listed package; vendored wins: remove only those packages’recordsentries, keeping other live redirects andeditsrevert data.Hosted +
--prune—scan --mode hosted --prune(and--sync) still succeeds but emitsredirect_prune_ignoredon stderr and in JSONredirect.warnings[], including the zero-discovery hosted envelope, so migrations from agent sync jobs don’t silently stop GC.Overlap / direction detection —
overlapping_ledger_purlscan match vendored packages when the redirect ledger has edits but emptyrecords(failed record fetch). Hosted liveness is proved by patch UUID in inventoryresolvedURLs (any host, not onlypatch.socket.dev) or in redirect-edited lockfile text outside vendored paths, covering bun URL tuples and yarn berry::__archiveUrl=bindings; vendored-path UUID matches are excluded. Ledger lockfile reads use a shared path traversal guard.Reviewed by Cursor Bugbot for commit a9f2e45. Configure here.