Skip to content

Commit fc2d1f9

Browse files
cursoragentcursoragent
committed
Fix: Preserve stage on swap failure to prevent destroying live vendored copy
The swap_stage_into_place function previously deleted copy_dir before renaming the stage. If the rename failed (due to permissions, file locks, etc.), cleanup_failed_stage then deleted the stage, leaving both the old copy and the new stage destroyed. This violated the B1 contract that a failed rebuild must preserve the previous copy. Changes: - Modified swap_stage_into_place to try rename first (atomic on Unix), only removing copy_dir if the first rename fails, then retrying - Removed cleanup_failed_stage calls after swap failure at both call sites, preserving the stage as the only valid replacement for recovery
1 parent 8f7c911 commit fc2d1f9

9 files changed

Lines changed: 132 additions & 59 deletions

File tree

‎crates/socket-patch-cli/src/commands/get.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,10 +1232,7 @@ pub async fn download_and_apply_patches(
12321232
// status/exit code degrade and it is never auto-applied.
12331233
if files.is_empty() {
12341234
if !params.json && !params.silent {
1235-
eprintln!(
1236-
" [fail] {} (patch has no applicable files)",
1237-
patch.purl
1238-
);
1235+
eprintln!(" [fail] {} (patch has no applicable files)", patch.purl);
12391236
}
12401237
downloaded_patches.push(serde_json::json!({
12411238
"purl": patch.purl,
@@ -3106,7 +3103,10 @@ mod tests {
31063103
// record — the guardrail-triggering condition the download/apply
31073104
// flows now count as failed rather than applied.
31083105
let mut broken = HashMap::new();
3109-
broken.insert("src/lib.rs".to_string(), file_resp(Some(&"e".repeat(64)), None));
3106+
broken.insert(
3107+
"src/lib.rs".to_string(),
3108+
file_resp(Some(&"e".repeat(64)), None),
3109+
);
31103110
let broken_patch = patch_with_files(broken);
31113111
assert!(
31123112
files_for_manifest(&broken_patch).is_empty(),

‎crates/socket-patch-cli/src/commands/scan/hosted.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,9 @@ pub(super) async fn run_redirect(
529529
// deleting the other mode's ledger; reconciliation is deferred (see PR Scope).
530530
// Read after the ledger write above so a non-dry-run reflects this run.
531531
let mut takeover_warnings: Vec<serde_json::Value> = Vec::new();
532-
let superseded = super::classify_overlap_takeover(&args.common.cwd).await.redirect;
532+
let superseded = super::classify_overlap_takeover(&args.common.cwd)
533+
.await
534+
.redirect;
533535
if !superseded.is_empty() {
534536
takeover_warnings.push(serde_json::json!({
535537
"code": super::REDIRECT_SUPERSEDES_VENDORED,

‎crates/socket-patch-cli/src/commands/scan/mod.rs‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -520,11 +520,15 @@ pub(super) async fn classify_overlap_takeover(cwd: &Path) -> OverlapTakeover {
520520
return out;
521521
};
522522
let canon = |p: &str| normalize_purl(strip_purl_qualifiers(p)).into_owned();
523-
let mut vendor_by_purl: std::collections::HashMap<String, &socket_patch_core::vendor::VendorEntry> =
524-
std::collections::HashMap::new();
523+
let mut vendor_by_purl: std::collections::HashMap<
524+
String,
525+
&socket_patch_core::vendor::VendorEntry,
526+
> = std::collections::HashMap::new();
525527
for (key, entry) in &vendor.entries {
526528
vendor_by_purl.entry(canon(key)).or_insert(entry);
527-
vendor_by_purl.entry(canon(&entry.base_purl)).or_insert(entry);
529+
vendor_by_purl
530+
.entry(canon(&entry.base_purl))
531+
.or_insert(entry);
528532
}
529533
// The scan inventory keeps only http(s) `resolved` URLs and DROPS our own
530534
// `file:.socket/vendor/…` specs (see `lock_inventory`), so a
@@ -2095,7 +2099,10 @@ mod tests {
20952099
"hosted flow must not warn when the lock is vendored: {takeover:?}"
20962100
);
20972101
// Truthful direction: vendored won ⇒ the redirect ledger is the stale one.
2098-
assert_eq!(takeover.vendored, vec!["pkg:npm/minimist@1.2.2".to_string()]);
2102+
assert_eq!(
2103+
takeover.vendored,
2104+
vec!["pkg:npm/minimist@1.2.2".to_string()]
2105+
);
20992106
// Pre-fix the hosted flow keyed off the raw overlap, which is non-empty
21002107
// — it WOULD have wrongly told the user to delete the live ledger.
21012108
assert!(!overlapping_ledger_purls(root).await.is_empty());
@@ -2119,7 +2126,10 @@ mod tests {
21192126
"vendored flow must not warn when the lock is hosted: {takeover:?}"
21202127
);
21212128
// Truthful direction: hosted won ⇒ the vendored ledger is the stale one.
2122-
assert_eq!(takeover.redirect, vec!["pkg:npm/minimist@1.2.2".to_string()]);
2129+
assert_eq!(
2130+
takeover.redirect,
2131+
vec!["pkg:npm/minimist@1.2.2".to_string()]
2132+
);
21232133
}
21242134

21252135
#[tokio::test]

‎crates/socket-patch-cli/src/update_notifier.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,9 +445,15 @@ mod tests {
445445
// ceiling — the override never silently changes shipped behavior.
446446
assert_eq!(grace_budget_from(None), Duration::from_millis(500));
447447
assert_eq!(grace_budget_from(Some("")), Duration::from_millis(500));
448-
assert_eq!(grace_budget_from(Some("not-a-number")), Duration::from_millis(500));
448+
assert_eq!(
449+
grace_budget_from(Some("not-a-number")),
450+
Duration::from_millis(500)
451+
);
449452
// A valid value lifts the ceiling (the e2e suite's escape hatch).
450-
assert_eq!(grace_budget_from(Some("30000")), Duration::from_millis(30_000));
453+
assert_eq!(
454+
grace_budget_from(Some("30000")),
455+
Duration::from_millis(30_000)
456+
);
451457
assert_eq!(grace_budget_from(Some("0")), Duration::from_millis(0));
452458
}
453459

‎crates/socket-patch-cli/tests/e2e_vendored_production.rs‎

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,9 @@ fn scan_vendored(cwd: &Path, extra: &[&str]) -> serde_json::Value {
316316
"scan --mode vendored failed (exit {code}).\nstdout:\n{stdout}\nstderr:\n{stderr}"
317317
);
318318
let env: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap_or_else(|e| {
319-
panic!("scan --mode vendored did not emit JSON ({e}).\nstdout:\n{stdout}\nstderr:\n{stderr}")
319+
panic!(
320+
"scan --mode vendored did not emit JSON ({e}).\nstdout:\n{stdout}\nstderr:\n{stderr}"
321+
)
320322
});
321323
assert_eq!(
322324
env["status"].as_str(),
@@ -385,8 +387,16 @@ fn assert_download_uuid(env: &serde_json::Value, uuids: &[&str], leg: &str) {
385387
/// `vendor --revert --json` in `cwd`, asserting success and returning the
386388
/// number of reverted entries.
387389
fn vendor_revert(cwd: &Path, leg: &str) -> u64 {
388-
let (code, stdout, stderr) =
389-
run_socket(cwd, &["vendor", "--revert", "--json", "--cwd", cwd.to_str().unwrap()]);
390+
let (code, stdout, stderr) = run_socket(
391+
cwd,
392+
&[
393+
"vendor",
394+
"--revert",
395+
"--json",
396+
"--cwd",
397+
cwd.to_str().unwrap(),
398+
],
399+
);
390400
assert_eq!(
391401
code, 0,
392402
"{leg}: vendor --revert failed (exit {code}).\nstdout:\n{stdout}\nstderr:\n{stderr}"
@@ -730,7 +740,11 @@ fn npm_package_lock_vendored_install_proof() {
730740
let fresh = tmp.path().join("fresh");
731741
std::fs::create_dir_all(&fresh).unwrap();
732742
std::fs::copy(proj.join("package.json"), fresh.join("package.json")).unwrap();
733-
std::fs::copy(proj.join("package-lock.json"), fresh.join("package-lock.json")).unwrap();
743+
std::fs::copy(
744+
proj.join("package-lock.json"),
745+
fresh.join("package-lock.json"),
746+
)
747+
.unwrap();
734748
copy_dir_recursive(&proj.join(".socket"), &fresh.join(".socket"));
735749

736750
let fresh_cache = tmp.path().join("fresh-npm-cache").display().to_string();
@@ -1327,7 +1341,11 @@ fn pypi_requirements_txt_vendored_install_proof() {
13271341
// from the project root (bare relative paths resolve against the CWD).
13281342
let fresh = tmp.path().join("fresh");
13291343
std::fs::create_dir_all(&fresh).unwrap();
1330-
std::fs::copy(proj.join("requirements.txt"), fresh.join("requirements.txt")).unwrap();
1344+
std::fs::copy(
1345+
proj.join("requirements.txt"),
1346+
fresh.join("requirements.txt"),
1347+
)
1348+
.unwrap();
13311349
copy_dir_recursive(&proj.join(".socket"), &fresh.join(".socket"));
13321350
let fresh_venv = fresh.join(".venv");
13331351
assert!(
@@ -1415,7 +1433,11 @@ fn pypi_uv_lock_vendored_install_proof() {
14151433
}
14161434
let venv = proj.join(".venv");
14171435
let Some(site) = site_packages(&venv) else {
1418-
soft_skip!(LEG, "could not locate site-packages under {}", venv.display());
1436+
soft_skip!(
1437+
LEG,
1438+
"could not locate site-packages under {}",
1439+
venv.display()
1440+
);
14191441
};
14201442
assert!(
14211443
!urllib3_patched(&site),
@@ -1551,7 +1573,9 @@ fn cargo_vendored_install_proof() {
15511573

15521574
// Vendored directory carries the patch; the registry source stays pristine.
15531575
let vendored_lib = proj
1554-
.join(format!(".socket/vendor/cargo/{CARGO_UUID}/{CARGO_NAME}-{CARGO_VERSION}"))
1576+
.join(format!(
1577+
".socket/vendor/cargo/{CARGO_UUID}/{CARGO_NAME}-{CARGO_VERSION}"
1578+
))
15551579
.join("src/lib.rs");
15561580
assert_patched(&vendored_lib, CARGO_MARKER, LEG);
15571581
if let Some(ref lib) = registry_lib {
@@ -1745,7 +1769,9 @@ fn gem_bundler_vendored_known_platform_defect() {
17451769
.map(|v| v == "1" || v.eq_ignore_ascii_case("true"))
17461770
.unwrap_or(false);
17471771

1748-
let applied = env_json["vendor"]["summary"]["applied"].as_u64().unwrap_or(0);
1772+
let applied = env_json["vendor"]["summary"]["applied"]
1773+
.as_u64()
1774+
.unwrap_or(0);
17491775
if code == 0 && applied >= 1 {
17501776
// The CLI now vendors platform gems. Say so loudly — this leg should be
17511777
// promoted to a full delivery proof (bundle install frozen) and this
@@ -1759,10 +1785,13 @@ fn gem_bundler_vendored_known_platform_defect() {
17591785
}
17601786

17611787
// Otherwise: it must be exactly the known `platform_gem_unsupported` failure.
1762-
let events = env_json["vendor"]["events"].as_array().cloned().unwrap_or_default();
1763-
let is_known = events.iter().any(|e| {
1764-
e["action"] == "failed" && e["errorCode"] == "platform_gem_unsupported"
1765-
});
1788+
let events = env_json["vendor"]["events"]
1789+
.as_array()
1790+
.cloned()
1791+
.unwrap_or_default();
1792+
let is_known = events
1793+
.iter()
1794+
.any(|e| e["action"] == "failed" && e["errorCode"] == "platform_gem_unsupported");
17661795
assert!(
17671796
!gem_strict,
17681797
"{LEG}: SOCKET_PATCH_VENDORED_E2E_GEM_STRICT=1 and vendoring {GEM_PURL} did not succeed \
@@ -1809,7 +1838,9 @@ fn golang_vendored_finds_no_free_patches() {
18091838
.expect("write go.mod");
18101839

18111840
let env_json = scan_vendored(&proj, &["--ecosystems", "golang"]);
1812-
let applied = env_json["vendor"]["summary"]["applied"].as_u64().unwrap_or(0);
1841+
let applied = env_json["vendor"]["summary"]["applied"]
1842+
.as_u64()
1843+
.unwrap_or(0);
18131844
assert_eq!(
18141845
applied, 0,
18151846
"{LEG}: golang vendored something, but production publishes no free golang patches. \
@@ -1849,7 +1880,9 @@ fn deno_vendored_is_unsupported() {
18491880
.expect("write deno.json");
18501881

18511882
let env_json = scan_vendored(&proj, &["--ecosystems", "deno"]);
1852-
let applied = env_json["vendor"]["summary"]["applied"].as_u64().unwrap_or(0);
1883+
let applied = env_json["vendor"]["summary"]["applied"]
1884+
.as_u64()
1885+
.unwrap_or(0);
18531886
assert_eq!(
18541887
applied, 0,
18551888
"{LEG}: deno vendored something, but vendored mode is not supported for deno:\n{env_json:#}"

‎crates/socket-patch-cli/tests/ecosystem_dispatch_e2e.rs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -788,10 +788,7 @@ fn rollback_dispatch_branch_maven() {
788788
let fixture = RollbackFixture {
789789
purl: purl.to_string(),
790790
verify_file,
791-
envs: vec![(
792-
"MAVEN_REPO_LOCAL".to_string(),
793-
repo.display().to_string(),
794-
)],
791+
envs: vec![("MAVEN_REPO_LOCAL".to_string(), repo.display().to_string())],
795792
global: false,
796793
};
797794
assert_rollback_restored(root, "maven", &fixture);

‎crates/socket-patch-cli/tests/update_notifier_e2e.rs‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,11 @@ async fn first_eligible_run_checks_and_notices() {
132132
.mount()
133133
.await;
134134

135-
let (code, stdout, stderr) =
136-
run_installed(&install, &["apply"], &eligible_kit_await_fetch(&release.base_url));
135+
let (code, stdout, stderr) = run_installed(
136+
&install,
137+
&["apply"],
138+
&eligible_kit_await_fetch(&release.base_url),
139+
);
137140
assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}");
138141
assert!(
139142
stderr.contains("Update available") && stderr.contains("9.9.9"),
@@ -196,8 +199,11 @@ async fn stale_state_rechecks() {
196199
.await;
197200
write_state(&install.state_dir, STALE, Some(CURRENT), None);
198201

199-
let (code, stdout, stderr) =
200-
run_installed(&install, &["apply"], &eligible_kit_await_fetch(&release.base_url));
202+
let (code, stdout, stderr) = run_installed(
203+
&install,
204+
&["apply"],
205+
&eligible_kit_await_fetch(&release.base_url),
206+
);
201207
assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}");
202208

203209
let state = read_state(&install.state_dir);
@@ -225,8 +231,11 @@ async fn up_to_date_prints_nothing() {
225231
.await;
226232
write_state(&install.state_dir, STALE, Some(CURRENT), None);
227233

228-
let (code, _, stderr) =
229-
run_installed(&install, &["apply"], &eligible_kit_await_fetch(&release.base_url));
234+
let (code, _, stderr) = run_installed(
235+
&install,
236+
&["apply"],
237+
&eligible_kit_await_fetch(&release.base_url),
238+
);
230239
assert_eq!(code, 0);
231240
assert!(
232241
!stderr.contains("Update available"),
@@ -288,8 +297,11 @@ async fn corrupt_state_recovers() {
288297
)
289298
.unwrap();
290299

291-
let (code, stdout, stderr) =
292-
run_installed(&install, &["apply"], &eligible_kit_await_fetch(&release.base_url));
300+
let (code, stdout, stderr) = run_installed(
301+
&install,
302+
&["apply"],
303+
&eligible_kit_await_fetch(&release.base_url),
304+
);
293305
assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}");
294306
assert!(
295307
!stderr.contains("panicked"),
@@ -316,8 +328,11 @@ async fn future_timestamp_tolerated() {
316328
.await;
317329
write_state(&install.state_dir, -48 * HOUR, Some("9.9.9"), None);
318330

319-
let (code, stdout, stderr) =
320-
run_installed(&install, &["apply"], &eligible_kit_await_fetch(&release.base_url));
331+
let (code, stdout, stderr) = run_installed(
332+
&install,
333+
&["apply"],
334+
&eligible_kit_await_fetch(&release.base_url),
335+
);
321336
assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}");
322337
assert_install_pristine(&install);
323338
}

‎crates/socket-patch-core/src/vendor/cargo.rs‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,19 @@ fn stage_dir_for(copy_dir: &Path) -> std::path::PathBuf {
9898
}
9999
}
100100

101-
/// Swap a fully-built stage into place: remove the old copy (if any), then
102-
/// rename the stage over it. The rename is same-directory and only happens
103-
/// after the stage passed every check, so the destroy-then-replace window is
104-
/// as small as the filesystem allows.
101+
/// Swap a fully-built stage into place: rename the stage over the old copy.
102+
/// The rename is same-directory and only happens after the stage passed every
103+
/// check. Try the rename first (may succeed atomically on Unix); if that fails,
104+
/// remove the old copy and retry. If the retry fails, the stage is left intact
105+
/// so a valid replacement exists for recovery.
105106
async fn swap_stage_into_place(stage: &Path, copy_dir: &Path) -> std::io::Result<()> {
106-
remove_tree(copy_dir).await?;
107-
tokio::fs::rename(stage, copy_dir).await
107+
match tokio::fs::rename(stage, copy_dir).await {
108+
Ok(()) => Ok(()),
109+
Err(_) => {
110+
remove_tree(copy_dir).await?;
111+
tokio::fs::rename(stage, copy_dir).await
112+
}
113+
}
108114
}
109115

110116
/// Best-effort removal of an EMPTY `<uuid>/` dir plus the empty
@@ -224,7 +230,6 @@ async fn cargo_service_copy(
224230
);
225231
}
226232
if let Err(e) = swap_stage_into_place(&stage, copy_dir).await {
227-
cleanup_failed_stage(&stage, uuid_dir, false).await;
228233
return hard(
229234
"vendor_prebuilt_write_failed",
230235
format!("cannot move the extracted crate into place: {e}"),
@@ -322,7 +327,6 @@ async fn copy_and_patch(
322327
}
323328
let _ = tokio::fs::remove_file(stage.join(".cargo-checksum.json")).await;
324329
if let Err(e) = swap_stage_into_place(&stage, copy_dir).await {
325-
cleanup_failed_stage(&stage, uuid_dir, unwind_uuid_dir).await;
326330
result.success = false;
327331
result.error = Some(format!("failed to move the rebuilt copy into place: {e}"));
328332
return Err(result);
@@ -1305,7 +1309,9 @@ mod tests {
13051309
"marker must survive"
13061310
);
13071311
assert_eq!(
1308-
tokio::fs::read(root.join(".cargo/config.toml")).await.unwrap(),
1312+
tokio::fs::read(root.join(".cargo/config.toml"))
1313+
.await
1314+
.unwrap(),
13091315
cfg1,
13101316
"config untouched"
13111317
);
@@ -1372,7 +1378,9 @@ mod tests {
13721378
[[package]]\nname = \"cfg-if\"\nversion = \"1.0.4\"\nsource = \"{SOURCE}\"\nchecksum = \"{CHECKSUM}\"\n\n\
13731379
[[package]]\nname = \"cfg-if\"\nversion = \"1.0.4\"\nsource = \"git+https://example.com/fork/cfg-if#abcdef\"\n"
13741380
);
1375-
tokio::fs::write(root.join("Cargo.lock"), &lock).await.unwrap();
1381+
tokio::fs::write(root.join("Cargo.lock"), &lock)
1382+
.await
1383+
.unwrap();
13761384

13771385
let detail = expect_refused(
13781386
run_vendor(PURL, root, &blobs, &pristine, &record, false).await,
@@ -1399,7 +1407,9 @@ mod tests {
13991407
async fn test_refuses_user_entry_through_foreign_socket_dir() {
14001408
let (dir, blobs, pristine, record) = fixture().await;
14011409
let root = dir.path();
1402-
tokio::fs::create_dir_all(root.join(".cargo")).await.unwrap();
1410+
tokio::fs::create_dir_all(root.join(".cargo"))
1411+
.await
1412+
.unwrap();
14031413
let user_cfg = format!(
14041414
"[patch.crates-io]\ncfg-if = {{ path = \"../shared-fork/.socket/vendor/cargo/{UUID2}/cfg-if-1.0.4\" }}\n"
14051415
);

0 commit comments

Comments
 (0)