fix(redirect): make the cargo hosted redirect transactional and fail-closed

[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-fable-5

What

Makes the hosted cargo redirect rewriter (scan --mode hosted / --redirect) transactional and fail-closed per dependency: a dep now counts as redirected — Cargo.lock repoint, .cargo/config.toml registry block, ledger record, redirected count, VEX attestation — only if its Cargo.toml pin fully landed across every occurrence. If any occurrence cannot be pinned, the dep is skipped entirely with one clear warning and nothing is written for it.

Rewriter changes (socket-patch-core/src/patch/redirect/mod.rs):

• All occurrences, all sections (A1): the manifest pin now lands on every declaration of the crate across [dependencies], [dev-dependencies], [build-dependencies], target-specific tables, and [workspace.dependencies] — not just the first regex match. A first-match-only rewrite gave the same dep different sources per section, which cargo rejects at manifest-parse time (bricking every cargo command; reproduced with real cargo in the audit).
• Multi-line table form (A2): [dependencies.<name>] tables gain an inserted registry = "…" line instead of being reported not-found while the lock was still repointed (which broke --locked builds and silently dropped the patch on re-resolve).
• Rename-aware matching (A5): an entry whose key matches but declares package = "<other>" is not the patched crate (pinning it pointed a foreign package at the single-crate socket registry — hard resolution failure); the patched crate consumed under an alias key (iffy = { package = "cfg-if" }) is found and pinned.
• UUID supersession (A4): an existing registry = "socket-patch-<uuid>" pin (canonical-uuid grammar) is ours and is replaced in place when a newer patch uuid is selected — previously it was classified as a foreign registry, leaving Cargo.toml on the old uuid while Cargo.lock moved to the new one (lock out of sync under --locked; superseded patch resolved otherwise; VEX attested the new one). A pin to any other registry remains a refusal.
• Transactional skip (A2/A3): unrewritable occurrences (dotted keys, path/git deps, foreign registry pins, table-form blocks carrying registry-index, unsatisfiable workspace = true inheritance, unsupported spellings) skip the dep entirely: no lock edit, no config block, no record, no VEX — one redirect_cargo_toml_dep_unrewritable warning. A Cargo.lock that lacks the dep's [[package]] block also skips the dep (manifest-only rewrites desync manifest and lock). workspace = true inheritors are satisfied by pinning the [workspace.dependencies] entry in the same manifest.
• Comment-proof idempotence + self-heal (A6): the [registries.…] presence check now requires an uncommented header with an uncommented, exact index = "…" line. A commented-out or degraded managed block is restored/regenerated on re-run instead of being "detected" by raw substring and left broken while the run reported success.
• Blank-line preservation (A7): the plain-version rewrite is line-scoped; the old \s*$ regex consumed the entry's trailing newline and deleted the following blank line, violating the byte-preservation invariant. Trailing # comments on plain-version entries are now preserved too.
• Input validation (A8): service-supplied strings are validated before interpolation into raw TOML: patch_uuid must match the canonical lowercase uuid grammar (reuses path_safety::is_canonical_uuid, same as the vendored flow), index_url must be a quote/backslash/control-free sparse+http(s):// URL, and the cksum must be exactly 64 lowercase hex chars. A hostile value (e.g. a ]+newline uuid injecting [source.crates-io] replace-with = …) is refused with a warning.
• Empty-string cksum (A9): cargoCksumSha256: "" is treated as missing (matching the TS twin's falsy check) — the dep is skipped with redirect_cargo_missing_cksum instead of writing checksum = "" into Cargo.lock, which hard-fails the next cargo fetch --locked.
• A cargo dep whose override kind is not cargo-sparse now warns (redirect_cargo_missing_override, matching the TS twin) instead of vanishing silently.

CLI changes (socket-patch-cli/src/commands/scan/hosted.rs):

• Confirmation keys off the rewrite outcome, never substring presence (A3/A3b): RewriteResult gains confirmed_cargo_uuids, populated only for deps whose pin fully landed (or was already in place). The hosted confirmed-gate uses that set for cargo purls instead of text.contains(index_url) — the config block contains the index URL while pinning nothing, so a config-block-only rewrite could previously confirm, persist a ledger record, and emit an assume_applied VEX statement for a patch no build would ever use (the cargo crawler surfaces machine-wide $CARGO_HOME crates, so this was reachable for crates the project doesn't even depend on).

Why

The bug class is silent broken success: scan --mode hosted exited 0, printed Redirected 1 package(s), recorded ledger entries, and attested patches via --vex assume_applied while the project was left unbuildable (duplicate-source manifests, manifest/lock splits under --locked), silently unpatched (locks re-resolving to crates.io), or falsely attested (config-block-only "redirects"). All audit findings were confirmed and most reproduced with real cargo. Fixes findings A1, A2, A3, A3b, A4, A5, A6, A7, A8, A9 of the cargo hosted-rewriter audit.

Fail-closed refusals implemented instead of rewrites (each yields a warning, a skipped dep, and zero writes — never a broken success):

• Dotted-key declarations other than <crate>.workspace = true (e.g. serde.version = "1").
• workspace = true inheritance when the (root) manifest has no [workspace.dependencies] entry for the crate — covers virtual-workspace roots whose deps live in member manifests the hosted flow does not rewrite today.
• path/git dependencies and foreign registry pins.
• Table-form blocks that carry a registry-index key — inserting registry = "…" next to it makes cargo reject the dependency specification as ambiguous at manifest-parse time.
• Single-quoted version strings and other unsupported spellings.

Known deferrals (documented, not silent):

• Superseded [registries.socket-patch-<old-uuid>] config blocks are left in place when a newer uuid takes over (inert once nothing references them; removing user-edited config regions wholesale felt riskier than the cosmetic leftover). The manifest/lock/ledger all move to the new uuid.
• Member manifests of cargo workspaces are still not rewrite candidates; such deps now refuse (see above) instead of half-redirecting.

Testing

• New shared golden fixtures under crates/socket-patch-core/tests/fixtures/redirect/cargo/cargo/: two-sections (A1 + the A7 blank-line case), table-form (A2), renamed (A5), supersede (A4), rerun (idempotent no-op), commented-config (A6 restore). The existing basic fixture is byte-identical to before.
• 20 new/updated unit tests in patch/redirect/mod.rs covering every finding, including the hostile-input refusals (A8), empty-cksum skip (A9), transactional skips, the table-form registry-index refusal, workspace inheritance both ways, target-specific tables, and rename-awareness. Existing cargo tests updated to use a canonical uuid (the rewriter now validates the grammar).
• New in-process CLI tests in in_process_redirect.rs, adapted from the audit repro probes and flipped red→green:
  • cargo_granted_but_nothing_pinned_is_not_confirmed_or_attested — the cargo analogue of no_lockfile_redirect_is_not_attested: granted patch for a crate the project doesn't declare → no config block, no ledger, no VEX, exit 1 on requested attestation (previously: inert config block + redirected: 1 + ledger + VEX, exit 0).
  • cargo_table_form_without_lock_is_pinned_and_attested — the audit's config-only repro scenario now lands a real manifest pin and legitimately attests.
• Suites: cargo test -p socket-patch-core --all-features redirect, cargo test -p socket-patch-cli --all-features --test in_process_redirect --test e2e_redirect_cargo_build (includes the real cargo fetch --locked/build e2e), cargo clippy --workspace --all-features -- -D warnings, and the full cargo test --workspace --all-features — all green.

Cross-repo note: the depscan TS byte-twin (workspaces/app/src/patches/registry-rewrite/cargo.ts) shares these golden fixtures and is ported in a companion depscan PR; the depscan submodule pin will need a bump after this merges so the TS golden suite consumes the new fixtures.

---

Note

High Risk
Changes hosted redirect confirmation, ledger, and VEX attestation for cargo—security-sensitive supply-chain behavior—and rewrites manifest/lock/config with fail-closed validation; incorrect logic could falsely attest patches or break builds.

Overview
Makes hosted cargo redirects transactional per dependency: a patch counts as redirected, ledgered, and VEX-attested only when its Cargo.toml pin fully lands (all sections, all occurrences) together with lock/config updates—or when that pin is already in place. Any unrewritable occurrence, missing lock entry, or undeclared dep skips the entire dep with warnings and zero writes (no inert [registries.…] block).

The cargo rewriter is reworked around plan-then-commit: validate uuid/index URL/checksum before TOML interpolation; pin every [dependencies] / dev/build / target / workspace table (including [dependencies.name] table form and rename-aware package =); supersede prior socket-patch-<uuid> pins; restore commented or degraded registry config blocks; refuse path/git, foreign registry, and hostile inputs.

Hosted confirmation for pkg:cargo/ now uses RewriteResult.confirmed_cargo_uuids instead of substring matches on index URLs—fixing false redirected: 1, ledger, and assume_applied VEX when nothing in the build actually pins the patch.

Tests: new golden fixtures, many unit tests, and in-process CLI tests for “granted but not pinned” vs table-form without lock.

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

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: CRLF plain versions refuse redirect
  • Added \r? before $ in the regex pattern to optionally match carriage returns from CRLF line endings.

Or push these changes by commenting:

@cursor push fc3bda3158

Preview (fc3bda3158)

diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs
--- a/crates/socket-patch-core/src/patch/redirect/mod.rs
+++ b/crates/socket-patch-core/src/patch/redirect/mod.rs
@@ -1031,7 +1031,7 @@
             // swallowed it).
             let c = regex::escape(crate_name);
             let line_re =
-                Regex::new(&format!(r#"^(\s*(?:{c}|"{c}")\s*=\s*)"([^"]+)"([ \t]*(?:#.*)?)$"#))
+                Regex::new(&format!(r#"^(\s*(?:{c}|"{c}")\s*=\s*)"([^"]+)"([ \t]*(?:#.*)?)\r?$"#))
                     .unwrap();
             let Some(m) = line_re.captures(raw) else {
                 pending.push(Pending::Refuse(
@@ -1943,7 +1943,7 @@
     }
 }
 
-// ── composer.lock ────────────────────────────────────────────────────────────
+// ── composer.lock ──────────────────────────────────────────���─────────────────
 fn rewrite_composer_lock(
     files: &BTreeMap<String, String>,
     overrides: &[DepOverride],

_{You can send follow-ups to the cloud agent here.}

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

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

[cursor]

CRLF plain versions refuse redirect

High Severity

The A7 plain-version matcher only allows spaces or tabs after the closing quote, so a carriage return left on the line after splitting on \n fails the match. That occurrence is treated as unrewritable and the whole dependency is skipped. Typical Windows or core.autocrlf Cargo.toml files use the crate = "1.0" spelling, so hosted cargo redirect does not land.

 

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

[cursor]

Table form misses registry-index

Medium Severity

Table-form entries pin by inserting registry whenever no registry key is present and never look for registry-index. Inline tables already refuse that key as a foreign registry. A [dependencies.<name>] block that only has registry-index still gets a socket registry line, then lock, config, confirmation, and VEX can proceed.

Additional Locations (1)

• crates/socket-patch-core/src/patch/redirect/mod.rs#L996-L1000

 

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

[cursor]

Commented headers duplicate registry tables

Medium Severity

The managed [registries.socket-patch-…] presence check requires the header line to equal the header after trim. An uncommented header with a trailing # comment does not match, so a second table is appended. Duplicate TOML tables break cargo config parse on re-run, while the dep can still be confirmed as redirected.

 

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