Skip to content

fix(scan): scope takeover cleanup per package, warn on hosted --prune - #191

Merged
Mikola Lysenko (mikolalysenko) merged 8 commits into
mainfrom
fix/takeover-remediation-safety
Aug 15, 2026
Merged

fix(scan): scope takeover cleanup per package, warn on hosted --prune#191
Mikola Lysenko (mikolalysenko) merged 8 commits into
mainfrom
fix/takeover-remediation-safety

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Problem

Four audit findings (2026-08-13 adversarial audit of origin/main 4e5288e) around the mode-takeover warning and hosted-mode flag handling:

  1. 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 blanket vendor --revert (mass-reverting unrelated packages). Concrete failure: redirect packages A and B, vendor A, follow the warning literally → vex stops attesting B and B's pre-redirect lock fragment is unrecoverable.

  2. scan --mode hosted --prune silently drops --prune (low) — both hosted terminals return before the GC blocks, so a bot migrating from --mode agent --prune stops pruning forever with exit 0, no warning, and a JSON envelope that simply omits the gc key.

  3. Takeover detection blind to degraded ledgers (low) — a hosted run where every record fetch failed persists a ledger with edits but an empty records map; overlapping_ledger_purls derived the overlap exclusively from record keys, so a later vendored takeover of those same packages emitted no warning at all.

  4. Hosted direction proof hardcoded patch.socket.dev and could not see berry/bun wiring (low) — liveness was proved only by the inventory resolved URL containing the default hostname, so non-default patch hosts (staging, --patch-server-url deployments) were unprovable, and yarn-berry (inventory resolved always None) plus bun (URL 3-tuples skipped by the inventory) were structurally unprovable.

No findings were skipped. Note: the in-flight branch work/mode-takeover-fix is content-identical to what is already merged as PR #164 and covers neither blind spot, so nothing here duplicates it.

Fix

  1. mode_takeover_detail now gives per-package, non-destructive cleanup. Hosted direction: run socket-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 under records in redirect-state.json, with an explicit warning against deleting the file (it holds other packages' live records plus the pre-redirect originals in edits).

  2. --prune with --mode hosted stays accepted (CLI_CONTRACT documents it as an orthogonal knob that never conflicts, so a hard error would break existing pipelines) but now emits an explicit redirect_prune_ignored warning on stderr (human path, once in run) and in the JSON redirect.warnings[], including the zero-discovery envelope. CLI_CONTRACT.md and the arg docs name the code.

  3. overlapping_ledger_purls falls back to matching vendored purls against the recorded FileEdit keys (name, name@version, /name suffix) when records is empty but edits exist. The fallback only activates on the degraded shape, so the well-formed-ledger path is unchanged, and the direction gate in classify_overlap_takeover still requires live-lock proof before anything is reported.

  4. Hosted liveness is now proved host-agnostically by the record's patch uuid: in the inventory resolved URL on any host (patch.socket.dev kept 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 as vendored_wiring_live (extracted as is_safe_project_rel_file). Unprovable direction still stays silent rather than guessing.

Testing

  • cargo test -p socket-patch-cli --lib — 373 passed
  • cargo test -p socket-patch-cli --test cli_parse_scan — 48 passed
  • cargo test -p socket-patch-cli --test in_process_scan — 23 passed
  • cargo test -p socket-patch-cli --test in_process_redirect — 24 passed

New 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_records
  • hosted_direction_provable_on_non_default_patch_host
  • hosted_direction_provable_for_bun_url_tuple
  • hosted_direction_provable_for_berry_archive_url
  • vendored_path_uuid_does_not_prove_hosted (guard: a vendored-path uuid must not prove hosted)
  • hosted_prune_emits_explicit_ignored_warning (subprocess --json contract: warning present, no gc key, 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 remediationmode_takeover_detail no longer tells users to delete whole redirect-state.json or entire .socket/vendor/<eco>/ trees. Hosted wins: use socket-patch remove <purl> per listed package; vendored wins: remove only those packages’ records entries, keeping other live redirects and edits revert data.

Hosted + --prunescan --mode hosted --prune (and --sync) still succeeds but emits redirect_prune_ignored on stderr and in JSON redirect.warnings[], including the zero-discovery hosted envelope, so migrations from agent sync jobs don’t silently stop GC.

Overlap / direction detectionoverlapping_ledger_purls can match vendored packages when the redirect ledger has edits but empty records (failed record fetch). Hosted liveness is proved by patch UUID in inventory resolved URLs (any host, not only patch.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.

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>

@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 2 potential issues.

Fix All in Cursor

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.

Comment thread crates/socket-patch-cli/src/commands/scan/mod.rs
Comment thread crates/socket-patch-cli/src/commands/scan/mod.rs
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>
@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit ac43f6b into main Aug 15, 2026
41 of 42 checks passed
@mikolalysenko
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>
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Aug 15, 2026
* fix(cargo): fully migrate hosted<->vendored modes

A cargo mode takeover used to exit 0 while leaving the project
unbuildable in both modes (audit findings C1-C7):

- hosted scan over a vendored crate left the vendored [patch.crates-io]
  entry behind, so every `cargo build --locked` failed (C1);
- vendor over a hosted redirect left the Cargo.toml
  registry = "socket-patch-..." pin, which [patch.crates-io] cannot
  apply over - unbuildable online and offline (C2), silently via the
  plain `vendor` command (C7);
- a vendored->hosted->vendored round trip overwrote the unrecoverable
  crates.io lock originals in the vendored ledger with the hosted
  sparse-index URL + patched checksum, so revert restored a dead
  grant-tokenized URL (C3);
- the takeover classifier could never prove hosted for cargo
  (lock-inventory resolved is None), so it INVERTED the direction and
  told users to delete the LIVE redirect ledger (C4/C4b);
- the displaced vendored entry was unreclaimable by GC/prune forever
  (no cargo in-use probe, C5);
- the remediation text offered deleting the .socket/vendor/<eco>/ tree
  while [patch.crates-io] still referenced it - hard resolution
  failure on every cargo command (C6).

A takeover now leaves the project FULLY in the new mode, or refuses:

- hosted scan reverts each claimed purl's vendored state per purl
  (lock originals restored, [patch] entry dropped, tree + ledger entry
  removed) before redirecting, and refuses the purl when the vendored
  ledger is missing/corrupt;
- every vendored flow (vendor, scan --mode vendored) reverts the
  purl's hosted edits from the redirect ledger FileEdits first
  (Cargo.toml pin, Cargo.lock source/checksum, registries block) and
  drops the ledger record; drifted files refuse fail-closed, and the
  cargo backend refuses (hosted_redirect_live) when hosted wiring is
  live with no ledger to revert it;
- the classifier gains a cargo lock-shape probe (socket-patch registry
  index vs detached-with-[patch]), so the direction follows the lock;
- persist_vendor_entry carries the cargo lock originals forward on
  re-vendor, keeping the crates.io fragment as the ledger's only home;
- dispatch_in_use_one gains the cargo probe so GC/prune reclaims
  genuinely stale vendored entries;
- both takeover remediation texts now recommend only the safe
  sequence (vendor --revert / re-run the vendored flow) and warn
  against hand-deleting ledgers or the vendor tree.

Red-then-green: the new real-cargo migration suite
(tests/mode_migration_cargo.rs, adapted from the audit probes) fails
all four scenarios on the previous code and passes with this change,
proving fresh-checkout `cargo build --locked` in the terminal state of
every migration direction.

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

* fix(takeover): stage cargo revert writes

revert_cargo_redirect_purl unwound the ledger's edits newest-first and
wrote each inverse to disk as it went, so a later drifted edit returned
Err with the earlier ones already written. Reverting a hosted redirect
whose Cargo.toml a third party had re-pinned restored Cargo.lock to
crates.io and then refused on the manifest, leaving the crate pinned to
the socket-patch registry with a lock that no longer resolves there --
a project that builds in neither mode. The ledger still recorded the
redirect as live and the caller reported the package as untouched
("cannot vendor over the live hosted redirect"), so nothing pointed at
the half-reverted files and every retry refused on the same drift.

Each inverse is now resolved against a staged view of the files and
nothing is written until all of them resolve. Reads go through the
staged map, which keeps the two orderings the unwind depends on: a
re-redirect chain still sees the previous step's result, and the
registry block still sees the wiring reverts when it probes whether
anything references it. Only an I/O fault during the final flush can
still stop mid-set, and that surfaces as Err naming the file.

The new unit test drifts only Cargo.toml, which unwinds after
Cargo.lock, and asserts all three files are byte-identical after the
refusal; it fails on the previous code with Cargo.lock already
rewritten.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* fix(scan): lead takeover cleanup with auto-reconcile

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>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants