From c8f5b9e8763256cbb7c4be91204573136ada3ebb Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 14 Aug 2026 06:59:07 -0700 Subject: [PATCH 1/2] fix(hosted): persist redirect ledger before lockfile writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hosted write path wrote every rewritten lockfile with a plain fs::write and only afterwards persisted the redirect ledger — the file that records each entry's pre-redirect resolved/integrity originals, the only revert data. A mid-loop write failure (read-only path, disk full, Rush multi-lock repos) therefore exited with some locks already redirected and their originals never persisted; a re-run could not recapture them because an already-redirected entry produces no new edit. The originals were permanently lost. Reorder and harden the write path: - Persist the ledger (merged with any existing one) BEFORE mutating any lockfile, so every planned edit's original is durable first. A recorded edit whose lockfile write then fails is harmless — revert would restore bytes the file already has — and a retried run's re-planned edits are deduplicated instead of appended. - Write lockfiles and the ledger with the vendored backend's atomic stage+fsync+rename writer (mode-preserving), so a crash mid-write can no longer leave a torn lockfile; a ledger failure now leaves the project byte-untouched. - Fail closed on an existing-but-corrupt redirect-state.json (e.g. an unresolved merge conflict): the run refuses before touching any lockfile instead of silently replacing the ledger and discarding the recorded originals (new load_redirect_state_strict; read-only consumers keep the lenient loader). - Percent-decode the version in parse_purl_simple like the name, so a canonical encoded purl version (npm 1.2.3%2Bbuild) matches the decoded form lockfiles store instead of silently redirecting nothing. Regression tests: mid-run partial write failure keeps the ledger's originals and leaves the failed lock byte-identical; a corrupt ledger is refused with the project untouched; an unwritable ledger now fails before (not after) the lockfile rewrite; strict-loader unit tests; and purl version decoding. Co-authored-by: Claude Fable 5 --- .../src/commands/scan/hosted.rs | 159 +++++++++++++---- .../tests/in_process_redirect.rs | 162 +++++++++++++++--- .../src/patch/redirect/mod.rs | 9 +- .../src/patch/redirect/state.rs | 71 +++++++- crates/socket-patch-core/src/utils/fs.rs | 5 +- 5 files changed, 337 insertions(+), 69 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index 340bddf3..cecdb45f 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -62,7 +62,13 @@ fn parse_purl_simple(purl: &str) -> Option<(String, String, String)> { let (typ, after) = rest.split_once('/')?; let (coord, version) = after.rsplit_once('@')?; let name = socket_patch_core::utils::purl::percent_decode_purl_component(coord).into_owned(); - Some((typ.to_string(), name, version.to_string())) + // The API serves canonical percent-encoded purls, so the version needs + // decoding just like the coordinate — npm build metadata arrives as + // `1.2.3%2Bbuild` while lockfiles store `1.2.3+build`; an undecoded + // version would silently match no lock entry. + let version = + socket_patch_core::utils::purl::percent_decode_purl_component(version).into_owned(); + Some((typ.to_string(), name, version)) } /// The hosted-mode JSON error envelope, for bail-outs that return before the @@ -462,35 +468,51 @@ pub(super) async fn run_redirect( } if !args.common.dry_run { - for (rel, content) in &rewrite.files { - let path = args.common.cwd.join(rel); - if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); - } - if let Err(e) = std::fs::write(&path, content) { - let message = format!("failed to write {rel}: {e}"); - eprintln!("{message}"); - if args.common.json { - emit_json_error(scan_result.take(), &message); - } - return 1; - } - } - // Ledger (mirrors the vendor state.json shape): recorded edits for a - // future revert + the patch records (file hashes + vulnerabilities) so - // a post-install `socket-patch vex` can attest the redirected patches. - // MERGE with any existing ledger rather than overwriting: an idempotent - // re-run produces no new edits (the lockfile already points at the - // hosted patch), and clobbering the file would lose the original - // pre-redirect values a future revert needs. New edits APPEND (revert - // walks them in reverse); records are keyed by PURL, newest wins. + // Ledger FIRST (mirrors the vendor state.json shape): recorded edits + // for a future revert + the patch records (file hashes + + // vulnerabilities) so a post-install `socket-patch vex` can attest the + // redirected patches. The ledger is the only revert path, so it must + // be durable BEFORE any lockfile is mutated: were the lockfiles + // written first, a mid-loop write failure would leave the + // already-written files redirected with their pre-redirect originals + // never persisted anywhere — and a re-run cannot recapture them (an + // already-redirected entry produces no new edit). Recording a planned + // edit whose lockfile write then fails is harmless in the other + // direction: revert would restore bytes the file already has. + // MERGE with any existing ledger rather than overwriting: an + // idempotent re-run produces no new edits (the lockfile already + // points at the hosted patch), and clobbering the file would lose the + // original pre-redirect values a future revert needs. New edits + // APPEND (revert walks them in reverse), skipping byte-identical + // re-plans from a retried partial failure; records are keyed by PURL, + // newest wins. if !rewrite.edits.is_empty() || !records.is_empty() || !migration_edits.is_empty() { let vendor_dir = args.common.cwd.join(".socket").join("vendor"); let _ = std::fs::create_dir_all(&vendor_dir); - let mut ledger = - socket_patch_core::patch::redirect::load_redirect_state(&args.common.cwd) - .await - .unwrap_or_else(RedirectState::new); + // Fail CLOSED on an existing-but-corrupt ledger (e.g. an + // unresolved merge conflict): silently replacing it would discard + // the recorded originals forever. Refusing HERE — before any + // lockfile write — leaves the project byte-untouched. + let mut ledger = match socket_patch_core::patch::redirect::load_redirect_state_strict( + &args.common.cwd, + ) + .await + { + Ok(Some(ledger)) => ledger, + Ok(None) => RedirectState::new(), + Err(e) => { + let message = format!( + ".socket/vendor/redirect-state.json {e}; refusing to overwrite it \ + — the ledger records the pre-redirect originals a revert needs. \ + Repair or remove the file, then re-run" + ); + eprintln!("{message}"); + if args.common.json { + emit_json_error(scan_result.take(), &message); + } + return 1; + } + }; // Ledgers written before the mode-string rename carry // `"mode": "redirect"`; normalize on rewrite so the on-disk // ledger converges on the documented "hosted" name (the @@ -498,16 +520,20 @@ pub(super) async fn run_redirect( ledger.mode = "hosted".to_string(); // The bun.lockb→bun.lock migration removal precedes the rewrite // edits so `--revert` unwinds it last (after restoring bun.lock). - ledger.edits.extend(migration_edits.iter().cloned()); - ledger.edits.extend(rewrite.edits.iter().cloned()); + for edit in migration_edits.iter().chain(rewrite.edits.iter()) { + if !ledger.edits.contains(edit) { + ledger.edits.push(edit.clone()); + } + } ledger.records.extend(records.clone()); - // The ledger is the only revert path and the VEX record store — - // a swallowed write failure would leave the rewritten lockfiles - // unrevertable while reporting success. - if let Err(e) = std::fs::write( - vendor_dir.join("redirect-state.json"), - format!("{}\n", serde_json::to_string_pretty(&ledger).unwrap()), - ) { + // A swallowed write failure would let the lockfile writes below + // proceed with no revert data persisted while reporting success. + if let Err(e) = socket_patch_core::utils::fs::atomic_write_bytes_preserving_mode( + &vendor_dir.join("redirect-state.json"), + format!("{}\n", serde_json::to_string_pretty(&ledger).unwrap()).as_bytes(), + ) + .await + { let message = format!("failed to write .socket/vendor/redirect-state.json: {e}"); eprintln!("{message}"); if args.common.json { @@ -516,6 +542,28 @@ pub(super) async fn run_redirect( return 1; } } + for (rel, content) in &rewrite.files { + let path = args.common.cwd.join(rel); + if let Some(parent) = path.parent() { + let _ = std::fs::create_dir_all(parent); + } + // Atomic stage+rename, mode-preserving (the vendored backend's + // writer): a bare `fs::write` truncates first, so a crash + // mid-write could leave a torn lockfile behind. + if let Err(e) = socket_patch_core::utils::fs::atomic_write_bytes_preserving_mode( + &path, + content.as_bytes(), + ) + .await + { + let message = format!("failed to write {rel}: {e}"); + eprintln!("{message}"); + if args.common.json { + emit_json_error(scan_result.take(), &message); + } + return 1; + } + } } // Cross-mode takeover: a committed vendored ledger (`.socket/vendor/state.json`) @@ -529,7 +577,9 @@ pub(super) async fn run_redirect( // deleting the other mode's ledger; reconciliation is deferred (see PR Scope). // Read after the ledger write above so a non-dry-run reflects this run. let mut takeover_warnings: Vec = Vec::new(); - let superseded = super::classify_overlap_takeover(&args.common.cwd).await.redirect; + let superseded = super::classify_overlap_takeover(&args.common.cwd) + .await + .redirect; if !superseded.is_empty() { takeover_warnings.push(serde_json::json!({ "code": super::REDIRECT_SUPERSEDES_VENDORED, @@ -676,9 +726,42 @@ pub(super) async fn run_redirect( #[cfg(test)] mod tests { - use super::{build_redirect_json_envelope, REDIRECT_CANDIDATE_FILES}; + use super::{build_redirect_json_envelope, parse_purl_simple, REDIRECT_CANDIDATE_FILES}; use socket_patch_core::constants::npm_family; + #[test] + fn parse_purl_simple_percent_decodes_name_and_version() { + // The API serves canonical percent-encoded purls: npm build metadata + // `1.2.3+build` arrives as `1.2.3%2Bbuild`. Lock entries store the + // decoded form, so an undecoded version silently matches nothing. + assert_eq!( + parse_purl_simple("pkg:npm/foo@1.2.3%2Bbuild"), + Some(( + "npm".to_string(), + "foo".to_string(), + "1.2.3+build".to_string() + )) + ); + // The coordinate keeps decoding too (scoped npm name). + assert_eq!( + parse_purl_simple("pkg:npm/%40scope/name@1.0.0"), + Some(( + "npm".to_string(), + "@scope/name".to_string(), + "1.0.0".to_string() + )) + ); + // Plain versions pass through unchanged. + assert_eq!( + parse_purl_simple("pkg:npm/left-pad@1.3.0"), + Some(( + "npm".to_string(), + "left-pad".to_string(), + "1.3.0".to_string() + )) + ); + } + /// The classic scan object `run` builds for the `--json` path with ≥1 /// discovered package (scannedPackages/totalPatches/… + the `packages` /// enumeration). Mirrors the `serde_json::json!` in `scan::run`. diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index d302aa48..9668e59a 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -1132,11 +1132,12 @@ async fn no_redirectable_patch_leaves_bun_lockb_alone() { ); } -/// A ledger that cannot be written is an ERROR, not a silent success: the -/// lockfile has already been rewritten, and +/// A ledger that cannot be persisted is an ERROR, not a silent success — and +/// it must fail BEFORE any lockfile is mutated: /// `.socket/vendor/redirect-state.json` is the only revert path (and the VEX -/// record store), so swallowing the write failure would leave the repo -/// redirected with no way back while reporting success. +/// record store), so the write path persists the ledger FIRST and only then +/// rewrites lockfiles. When the ledger cannot be persisted the project stays +/// byte-untouched. #[tokio::test] #[serial] async fn unwritable_ledger_fails_the_run() { @@ -1152,12 +1153,118 @@ async fn unwritable_ledger_fails_the_run() { let code = run(redirect_args(tmp.path(), server.uri())).await; assert_eq!(code, 1, "a failed ledger write must flip the exit code"); - // The failure is about the ledger, not the rewrite: the lockfile edit - // landed before the ledger write was attempted. + // Ledger-first ordering: no lockfile may be rewritten when the revert + // data could not be persisted — a rewritten lock with no recorded + // originals would be unrevertable. let lock = std::fs::read_to_string(tmp.path().join("package-lock.json")).unwrap(); assert!( - lock.contains(HOSTED_URL), - "the lockfile rewrite precedes the ledger write; got:\n{lock}" + !lock.contains(HOSTED_URL) && lock.contains("registry.npmjs.org"), + "the ledger write precedes the lockfile rewrite, so a ledger failure \ + must leave the lockfile untouched; got:\n{lock}" + ); +} + +/// Findings hosted-atomicity 1+2: a MID-RUN lockfile write failure (second of +/// two locks unwritable) must never leave the successfully-written first lock +/// redirected with no ledger record of its pre-redirect originals. The ledger +/// is persisted BEFORE the lockfile loop, so every planned edit's original is +/// durable even when a later write fails; the failed lock itself stays +/// byte-untouched (atomic stage+rename, no truncation). +/// +/// unix-only: a read-only directory does not block file creation on Windows. +#[cfg(unix)] +#[tokio::test] +#[serial] +async fn partial_lockfile_write_failure_persists_ledger_originals() { + use std::os::unix::fs::PermissionsExt; + + let server = MockServer::start().await; + mock_discovery(&server).await; + mock_reference(&server).await; + mock_view(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + // Rush repo: two pnpm locks, both resolving the patched package. The + // rewriter output map is a BTreeMap, so the common lock + // (common/config/rush/…) is written before the subspace lock + // (common/config/subspaces/…). + write_rush_project(tmp.path(), false); + let subspace_dir = tmp.path().join("common/config/subspaces/frontend"); + let before_subspace = std::fs::read_to_string(subspace_dir.join("pnpm-lock.yaml")).unwrap(); + std::fs::set_permissions(&subspace_dir, std::fs::Permissions::from_mode(0o555)).unwrap(); + + let code = run(redirect_args(tmp.path(), server.uri())).await; + + std::fs::set_permissions(&subspace_dir, std::fs::Permissions::from_mode(0o755)).unwrap(); + assert_eq!(code, 1, "a mid-run lockfile write failure must exit 1"); + + // The first lock landed before the failure… + let common = + std::fs::read_to_string(tmp.path().join("common/config/rush/pnpm-lock.yaml")).unwrap(); + assert!( + common.contains(HOSTED_URL), + "the common lock was written before the subspace failure; got:\n{common}" + ); + // …so its pre-redirect originals MUST already be in the ledger: without + // them a revert is impossible, and a re-run cannot recapture them (the + // entry is already redirected and produces no new edit). + let ledger = std::fs::read_to_string(tmp.path().join(".socket/vendor/redirect-state.json")) + .expect("the ledger must be persisted before any lockfile is mutated"); + assert!( + ledger.contains("UPSTREAMupstream"), + "the ledger must record the pre-redirect original integrity: {ledger}" + ); + assert!( + ledger.contains("common/config/rush/pnpm-lock.yaml"), + "the ledger must record the edit for the lock that WAS written: {ledger}" + ); + + // The failed lock is byte-untouched — no partial/truncated write. + assert_eq!( + std::fs::read_to_string(subspace_dir.join("pnpm-lock.yaml")).unwrap(), + before_subspace, + "the unwritable lock must stay byte-identical (atomic writes)" + ); +} + +/// Finding hosted-atomicity 3: an EXISTING-but-corrupt redirect ledger (e.g. +/// an unresolved git merge conflict) must be refused, not silently replaced — +/// replacing it discards the pre-redirect originals a future revert needs. +/// The refusal happens before any lockfile write, so the project stays +/// byte-untouched. +#[tokio::test] +#[serial] +async fn corrupt_ledger_is_refused_not_replaced() { + let server = MockServer::start().await; + mock_discovery(&server).await; + mock_reference(&server).await; + mock_view(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + write_project(tmp.path()); + let vendor_dir = tmp.path().join(".socket/vendor"); + std::fs::create_dir_all(&vendor_dir).unwrap(); + let garbage = "<<<<<<< HEAD\n{ \"version\": 1, \"mode\": \"hosted\" }\n=======\n"; + std::fs::write(vendor_dir.join("redirect-state.json"), garbage).unwrap(); + + let code = run(redirect_args(tmp.path(), server.uri())).await; + assert_eq!( + code, 1, + "a corrupt ledger must fail the run, not be replaced" + ); + + // The corrupt file is preserved for the user to repair (it may still + // hold recoverable originals inside the conflict markers). + assert_eq!( + std::fs::read_to_string(vendor_dir.join("redirect-state.json")).unwrap(), + garbage, + "the corrupt ledger must not be overwritten" + ); + // Fail-closed ordering: nothing was rewritten either. + let lock = std::fs::read_to_string(tmp.path().join("package-lock.json")).unwrap(); + assert!( + !lock.contains(HOSTED_URL) && lock.contains("registry.npmjs.org"), + "the refusal must precede any lockfile write; got:\n{lock}" ); } @@ -1942,24 +2049,31 @@ async fn redirect_json_mode_write_failures_emit_error_envelope() { .expect("run socket-patch") } - // Leg 3 — the rewritten lockfile cannot be written back (read-only - // file; the rewriter read it fine moments earlier). - let server = MockServer::start().await; - mock_discovery(&server).await; - mock_reference(&server).await; - mock_view(&server).await; - let tmp = tempfile::tempdir().unwrap(); - write_project(tmp.path()); - let lock = tmp.path().join("package-lock.json"); - let mut perms = std::fs::metadata(&lock).unwrap().permissions(); - perms.set_readonly(true); - std::fs::set_permissions(&lock, perms).unwrap(); - let out = run_leg(tmp.path(), &server).await; - assert_error_envelope(&out, "lockfile-write failure"); + // Leg 3 — the rewritten lockfile cannot be written back (its DIRECTORY + // is read-only, so the atomic stage file cannot be created; the rewriter + // read the lock fine moments earlier). A read-only lock FILE no longer + // fails this leg: the atomic stage+rename replaces it mode-preserved, + // like the vendored backend's writer. unix-only: a read-only directory + // does not block file creation on Windows. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let server = MockServer::start().await; + mock_discovery(&server).await; + mock_reference(&server).await; + mock_view(&server).await; + let tmp = tempfile::tempdir().unwrap(); + write_rush_project(tmp.path(), false); + let lock_dir = tmp.path().join("common/config/rush"); + std::fs::set_permissions(&lock_dir, std::fs::Permissions::from_mode(0o555)).unwrap(); + let out = run_leg(tmp.path(), &server).await; + std::fs::set_permissions(&lock_dir, std::fs::Permissions::from_mode(0o755)).unwrap(); + assert_error_envelope(&out, "lockfile-write failure"); + } // Leg 4 — the revert ledger cannot be persisted: a DIRECTORY squats on - // `.socket/vendor/redirect-state.json`, so `fs::write` fails after the - // lockfile rewrite succeeded. + // `.socket/vendor/redirect-state.json`. The ledger is written BEFORE the + // lockfiles, so the run refuses with the project untouched. let server = MockServer::start().await; mock_discovery(&server).await; mock_reference(&server).await; diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 0ff40eb5..cb08ee9b 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -25,7 +25,9 @@ use crate::vendor::yarn_berry_lock::yarnrc_compression_level; pub mod golang_local; mod state; -pub use state::{load_redirect_state, RedirectState, REDIRECT_STATE_REL}; +pub use state::{ + load_redirect_state, load_redirect_state_strict, RedirectState, REDIRECT_STATE_REL, +}; /// One ecosystem's integrity hashes (mirrors the TS `PatchArtifactIntegrity`). #[derive(Debug, Clone, Default, Deserialize)] @@ -94,7 +96,10 @@ pub struct DepOverride { /// One recorded file edit (mirrors the TS `FileEdit`). `Deserialize` so the /// persisted `redirect-state.json` ledger round-trips (see `redirect::state`). -#[derive(Debug, Clone, Serialize, Deserialize)] +/// `PartialEq` so the ledger merge can skip byte-identical edits a retried +/// run re-plans (the ledger persists BEFORE the lockfile writes, so a +/// failed write's edit is re-planned by the retry). +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct FileEdit { pub path: String, pub kind: String, diff --git a/crates/socket-patch-core/src/patch/redirect/state.rs b/crates/socket-patch-core/src/patch/redirect/state.rs index a3c280b5..f4336e58 100644 --- a/crates/socket-patch-core/src/patch/redirect/state.rs +++ b/crates/socket-patch-core/src/patch/redirect/state.rs @@ -57,11 +57,37 @@ impl Default for RedirectState { /// Load the redirect ledger. Missing OR malformed → `None` (VEX then simply /// has nothing extra to attest, and per-entry verification still fails closed -/// downstream) rather than aborting the command. +/// downstream) rather than aborting the command. READ-ONLY consumers only — +/// a writer about to merge-and-rewrite the file must use +/// [`load_redirect_state_strict`] so a corrupt ledger is refused, not +/// silently replaced. pub async fn load_redirect_state(project_root: &Path) -> Option { + load_redirect_state_strict(project_root) + .await + .ok() + .flatten() +} + +/// Load the redirect ledger, distinguishing ABSENT (`Ok(None)`) from +/// EXISTING-but-unusable (`Err`, with the reason). The hosted writer merges +/// into the existing ledger before rewriting it; treating a corrupt file +/// (e.g. an unresolved git merge conflict) as "absent" would replace it with +/// a fresh ledger, and — because an already-redirected lockfile produces no +/// new edits on a re-run — permanently discard the pre-redirect originals a +/// future revert needs. Writers must refuse on `Err`. +pub async fn load_redirect_state_strict( + project_root: &Path, +) -> Result, String> { let path = project_root.join(REDIRECT_STATE_REL); - let bytes = tokio::fs::read(&path).await.ok()?; - serde_json::from_slice(&bytes).ok() + let bytes = match tokio::fs::read(&path).await { + Ok(bytes) => bytes, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(e) => return Err(format!("cannot be read: {e}")), + }; + match serde_json::from_slice(&bytes) { + Ok(state) => Ok(Some(state)), + Err(e) => Err(format!("is not a valid redirect ledger: {e}")), + } } #[cfg(test)] @@ -170,4 +196,43 @@ mod tests { .unwrap(); assert!(load_redirect_state(tmp.path()).await.is_none()); } + + #[tokio::test] + async fn strict_load_distinguishes_missing_from_malformed() { + // Missing → Ok(None): a first hosted run starts a fresh ledger. + let tmp = tempfile::tempdir().unwrap(); + assert!(matches!( + load_redirect_state_strict(tmp.path()).await, + Ok(None) + )); + + // Malformed (e.g. an unresolved merge conflict) → Err: the writer + // must REFUSE rather than replace the file — replacement would + // discard the pre-redirect originals a future revert needs. + let dir = tmp.path().join(".socket/vendor"); + tokio::fs::create_dir_all(&dir).await.unwrap(); + tokio::fs::write( + dir.join("redirect-state.json"), + b"<<<<<<< HEAD\n{ \"version\": 1 }\n=======\n", + ) + .await + .unwrap(); + let err = load_redirect_state_strict(tmp.path()).await.unwrap_err(); + assert!( + err.contains("not a valid redirect ledger"), + "the refusal must say the file is corrupt, got: {err}" + ); + + // Valid → Ok(Some), same as the lenient loader. + tokio::fs::write( + dir.join("redirect-state.json"), + serde_json::to_string_pretty(&RedirectState::new()).unwrap(), + ) + .await + .unwrap(); + assert!(matches!( + load_redirect_state_strict(tmp.path()).await, + Ok(Some(_)) + )); + } } diff --git a/crates/socket-patch-core/src/utils/fs.rs b/crates/socket-patch-core/src/utils/fs.rs index 020d0472..bcf19bda 100644 --- a/crates/socket-patch-core/src/utils/fs.rs +++ b/crates/socket-patch-core/src/utils/fs.rs @@ -189,8 +189,9 @@ pub(crate) async fn atomic_write_bytes(path: &Path, content: &[u8]) -> std::io:: /// this variant for files the *user* owns and we merely edit (package.json, /// Gemfile, …), matching npm's write-file-atomic. The patch engine keeps the /// plain writer: `restore_file_permissions` re-applies pre-patch mode + uid/gid -/// itself after the rename. -pub(crate) async fn atomic_write_bytes_preserving_mode( +/// itself after the rename. `pub` (not `pub(crate)`): the CLI's hosted +/// redirect writes user-owned lockfiles through it too. +pub async fn atomic_write_bytes_preserving_mode( path: &Path, content: &[u8], ) -> std::io::Result<()> { From 4c2c255a7a2b404141693cfcb52becaab5b72e47 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 14 Aug 2026 20:07:40 -0400 Subject: [PATCH 2/2] test(redirect): pin corrupt-refuse before bun migrate A project with an unreadable redirect-state.json and a bun.lockb must come out of a refused run byte-identical. The refusal already precedes the bun.lockb auto-migration, but nothing pinned that ordering, and the migration is the one write that happens before every rewriter: run it first and the user loses their binary lockfile to a run that redirects nothing and exits 1. The new test grants a redirectable npm override (the migration's gate) and puts a fake bun that would migrate on PATH, so a surviving bun.lockb proves the ordering rather than a skipped gate. Moving the ledger load back below the migration fails it. Co-authored-by: Claude Fable 5 --- .../tests/in_process_redirect.rs | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index e20e706a..07ea8126 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -1290,6 +1290,94 @@ async fn no_redirectable_patch_leaves_bun_lockb_alone() { ); } +/// The corrupt-ledger refusal must fire BEFORE the bun.lockb auto-migration, +/// not after it. The migration deletes the binary lock and writes a text one, +/// so running it ahead of the refusal would convert the project's lockfile +/// format and then exit 1 without recording the migration or redirecting +/// anything — the "byte-untouched on refusal" promise broken by the one write +/// that precedes every rewriter. A redirectable npm override is granted here +/// (the migration's gate) and a fake `bun` that WOULD migrate sits on PATH, so +/// the surviving bun.lockb proves the ordering rather than a skipped gate. +#[cfg(unix)] +#[tokio::test] +#[serial] +async fn corrupt_ledger_refuses_before_the_bun_lockb_migration() { + let server = MockServer::start().await; + mock_discovery(&server).await; + mock_reference(&server).await; + mock_view(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + std::fs::write( + tmp.path().join("package.json"), + format!( + r#"{{ "name": "consumer", "version": "0.0.0", "dependencies": {{ "{NAME}": "^{VERSION}" }} }}"# + ), + ) + .unwrap(); + let pkg = tmp.path().join("node_modules").join(NAME); + std::fs::create_dir_all(&pkg).unwrap(); + std::fs::write( + pkg.join("package.json"), + format!(r#"{{ "name": "{NAME}", "version": "{VERSION}" }}"#), + ) + .unwrap(); + std::fs::write(tmp.path().join("bun.lockb"), b"BUN-BINARY-PLACEHOLDER").unwrap(); + + // A torn ledger: parseable as neither the vendor nor the redirect shape. + let ledger_path = tmp.path().join(".socket/vendor/redirect-state.json"); + std::fs::create_dir_all(ledger_path.parent().unwrap()).unwrap(); + let corrupt_bytes = b"{\"mode\":\"hosted\",\"edits\":[{\"path\":\"bun.lo"; + std::fs::write(&ledger_path, corrupt_bytes).unwrap(); + + let bin_dir = tmp.path().join("fakebin"); + std::fs::create_dir_all(&bin_dir).unwrap(); + let shim = bin_dir.join("bun"); + std::fs::write( + &shim, + "#!/bin/sh\n\ + echo '{ \"lockfileVersion\": 1, \"packages\": {} }' > bun.lock\n\ + rm -f bun.lockb\n\ + exit 0\n", + ) + .unwrap(); + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&shim, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + let orig_path = std::env::var("PATH").unwrap_or_default(); + // SAFETY: single-threaded #[serial] test; PATH restored below. + unsafe { + std::env::set_var("PATH", format!("{}:{orig_path}", bin_dir.display())); + } + + let code = run(redirect_args(tmp.path(), server.uri())).await; + + unsafe { + std::env::set_var("PATH", orig_path); + } + assert_eq!(code, 1, "a corrupt ledger must flip the exit code"); + assert_eq!( + std::fs::read(tmp.path().join("bun.lockb")).ok().as_deref(), + Some(b"BUN-BINARY-PLACEHOLDER".as_slice()), + "the binary lock must be byte-untouched: the refusal precedes the migration" + ); + assert!( + !tmp.path().join("bun.lock").exists(), + "no text lock may be created by a run that refused before redirecting" + ); + // The malformed ledger is quarantined (never deleted), so recovery of the + // pre-redirect originals it may still hold stays possible. + let quarantined = tmp + .path() + .join(".socket/vendor/redirect-state.json.corrupt"); + assert_eq!( + std::fs::read(&quarantined).unwrap(), + corrupt_bytes, + "the malformed ledger is moved aside verbatim" + ); +} + /// An unusable ledger is an ERROR, not a silent success: /// `.socket/vendor/redirect-state.json` is the only revert path (and the VEX /// record store). A DIRECTORY squatting on the ledger path makes it