From bae4d4d981fcbef226e3a34673f1d02fe1510014 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 01:23:08 +0000 Subject: [PATCH 1/7] Start fix for #1756 Assisted-by: Claude Code:claude-opus-5-5 From 1f8728ab58d27fd39c695989413de0d28b4bf666 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 01:41:16 +0000 Subject: [PATCH 2/7] Accept npm's LF shebang on CRLF bin files On Linux and macOS npm's bin-links rewrites a bin file whose first line is "#!...\r\n" to end in "\n". socket-patch compared the installed bytes against the tarball's hashes, so a pristine npm install of such a bin failed apply as drift, an overwrite with consent wrote back a shebang env can't run ("node\r": exit 127), hosted vex omitted a correctly patched install, and vendored vex warned the tree was out of sync. The hash checks now also accept npm's form of the original and the patched content, and apply and rollback write the bytes in that form, so the bin keeps running. An overwrite with consent of an npm bin with an LF shebang writes the patched content the same way. Fixes #1756 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_npm_build.rs | 138 +++++++++ crates/socket-patch-core/src/patch/apply.rs | 273 +++++++++++++++++- crates/socket-patch-core/src/patch/mod.rs | 1 + crates/socket-patch-core/src/patch/npm_bin.rs | 167 +++++++++++ .../socket-patch-core/src/patch/rollback.rs | 108 ++++++- crates/socket-patch-core/src/vex/verify.rs | 26 ++ 6 files changed, 701 insertions(+), 12 deletions(-) create mode 100644 crates/socket-patch-core/src/patch/npm_bin.rs diff --git a/crates/socket-patch-cli/tests/e2e_redirect_npm_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_npm_build.rs index bc451f1df..f09dbb836 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_npm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_npm_build.rs @@ -1160,3 +1160,141 @@ async fn npm_get_ghsa_hosted_narrows_and_installs() { let (fresh, installed) = fresh_install_patched(&fx); manifestless_tail(&fx, &fresh, installed, &[]); } + +/// Regression (#1756): on Linux and macOS npm's `bin-links` rewrites a +/// `bin` file's CRLF shebang to LF at install. Agent-mode `apply` patches +/// that install (it was refused as drift), keeps the bin runnable (a +/// written-back `#!/usr/bin/env node\r` makes `env` exit 127), re-applies +/// as a no-op, `vex` attests it, and `rollback` restores npm's form of the +/// original. Offline: the package is a local tarball. +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread")] +#[ignore = "wall-bound real-npm install; runs on the e2e CI matrix leg"] +async fn npm_agent_patches_crlf_shebang_bin_npm_rewrote() { + const SUITE: &str = "npm_agent_patches_crlf_shebang_bin_npm_rewrote"; + if npm_e2e_common::npm_version().is_none() { + npm_e2e_common::skip(SUITE, "npm not on PATH"); + return; + } + let before: &[u8] = b"#!/usr/bin/env node\r\nconsole.log(\"hi\");\r\n"; + let after: &[u8] = b"#!/usr/bin/env node\r\nconsole.log(\"hi\");\r\n// SOCKET-PATCHED\r\n"; + // What npm leaves on disk for each: the first line's `\r` dropped. + let installed_before: &[u8] = b"#!/usr/bin/env node\nconsole.log(\"hi\");\r\n"; + let installed_after: &[u8] = + b"#!/usr/bin/env node\nconsole.log(\"hi\");\r\n// SOCKET-PATCHED\r\n"; + + let tmp = tempfile::tempdir().unwrap(); + let pkg = tmp.path().join("pkg"); + std::fs::create_dir_all(&pkg).unwrap(); + std::fs::write( + pkg.join("package.json"), + r#"{"name":"crlfbin","version":"1.0.0","bin":{"crlfbin":"cli.js"}}"#, + ) + .unwrap(); + std::fs::write(pkg.join("cli.js"), before).unwrap(); + std::fs::write( + tmp.path().join("crlfbin-1.0.0.tgz"), + npm_e2e_common::npm_pack_like(&pkg), + ) + .unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + std::fs::write( + proj.join("package.json"), + r#"{"name":"p","version":"1.0.0","dependencies":{"crlfbin":"file:../crlfbin-1.0.0.tgz"}}"#, + ) + .unwrap(); + let cache = tmp.path().join("cache"); + let out = npm_e2e_common::npm( + &proj, + &[ + "install", + "--no-audit", + "--no-fund", + "--cache", + cache.to_str().unwrap(), + ], + ); + assert!( + out.status.success(), + "{}", + npm_e2e_common::output_text(&out) + ); + let cli = proj.join("node_modules/crlfbin/cli.js"); + assert_eq!( + std::fs::read(&cli).unwrap(), + installed_before, + "npm's fixBin" + ); + + let (b, a) = (git_sha256(before), git_sha256(after)); + let blobs = proj.join(".socket/blobs"); + std::fs::create_dir_all(&blobs).unwrap(); + std::fs::write(blobs.join(&b), before).unwrap(); + std::fs::write(blobs.join(&a), after).unwrap(); + std::fs::write( + proj.join(".socket/manifest.json"), + format!( + r#"{{"patches":{{"pkg:npm/crlfbin@1.0.0":{{"uuid":"e2f99e5d-3afd-5cbd-be80-6a0102bcd6c2", +"exportedAt":"2026-01-01T00:00:00Z","files":{{"package/cli.js":{{"beforeHash":"{b}","afterHash":"{a}"}}}}, +"vulnerabilities":{{"GHSA-xxxx-yyyy-zzzz":{{"cves":["CVE-2099-0001"],"summary":"t","severity":"high","description":"d"}}}}, +"description":"d","license":"MIT","tier":"free"}}}}}}"# + ), + ) + .unwrap(); + + let run_bin = || { + std::process::Command::new(proj.join("node_modules/.bin/crlfbin")) + .current_dir(&proj) + .output() + .unwrap() + }; + + let (code, stdout, stderr) = run_socket(&proj, &["apply", "--offline"]); + assert_eq!(code, 0, "apply\nstdout:\n{stdout}\nstderr:\n{stderr}"); + assert!( + !stderr.contains("did not match"), + "no drift warning: {stderr}" + ); + assert_eq!(std::fs::read(&cli).unwrap(), installed_after); + let ran = run_bin(); + assert!( + ran.status.success(), + "{}", + npm_e2e_common::output_text(&ran) + ); + assert_eq!(String::from_utf8_lossy(&ran.stdout).trim(), "hi"); + + let (code, stdout, stderr) = run_socket(&proj, &["apply", "--offline"]); + assert_eq!(code, 0, "re-apply\nstdout:\n{stdout}\nstderr:\n{stderr}"); + assert_eq!(std::fs::read(&cli).unwrap(), installed_after); + + let doc = proj.join("vex.json"); + let (code, stdout, stderr) = run_socket( + &proj, + &[ + "vex", + "--offline", + "-O", + doc.to_str().unwrap(), + "--product", + "pkg:npm/p@1.0.0", + ], + ); + assert_eq!(code, 0, "vex\nstdout:\n{stdout}\nstderr:\n{stderr}"); + let vex = std::fs::read_to_string(&doc).unwrap(); + assert!( + vex.contains("CVE-2099-0001") && vex.contains("not_affected"), + "{vex}" + ); + + let (code, stdout, stderr) = run_socket(&proj, &["rollback", "--offline"]); + assert_eq!(code, 0, "rollback\nstdout:\n{stdout}\nstderr:\n{stderr}"); + assert_eq!(std::fs::read(&cli).unwrap(), installed_before); + let ran = run_bin(); + assert!( + ran.status.success(), + "{}", + npm_e2e_common::output_text(&ran) + ); +} diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs index b254da2a8..213ba7204 100644 --- a/crates/socket-patch-core/src/patch/apply.rs +++ b/crates/socket-patch-core/src/patch/apply.rs @@ -261,8 +261,21 @@ pub async fn verify_file_patch( } }; + // npm installs a CRLF-shebang `bin` with an LF shebang (see + // `npm_bin`): hash the content it came from too, once, when the raw + // bytes match neither side. + let source_hash = if current_hash == file_info.after_hash + || (!is_new_file && current_hash == file_info.before_hash) + { + None + } else { + crate::patch::npm_bin::source_hash(&filepath).await + }; + // Check if already patched - if current_hash == file_info.after_hash { + if current_hash == file_info.after_hash + || source_hash.as_deref() == Some(file_info.after_hash.as_str()) + { return VerifyResult { file: file_name.to_string(), status: VerifyStatus::AlreadyPatched, @@ -286,8 +299,12 @@ pub async fn verify_file_patch( }; } - // Check if matches expected before hash - if current_hash != file_info.before_hash { + // Check if matches expected before hash. A `Ready` whose + // `current_hash` is not the `beforeHash` is npm's form of it; the apply + // engine writes the patched bytes in the same form. + if current_hash != file_info.before_hash + && source_hash.as_deref() != Some(file_info.before_hash.as_str()) + { return VerifyResult { file: file_name.to_string(), status: VerifyStatus::HashMismatch, @@ -1223,14 +1240,30 @@ async fn apply_package_patch_at( } }; + // A bin npm installed with an LF shebang gets the patched bytes in + // that form too (see `npm_bin`). + let (patched_content, written_hash) = match npm_fixed_write( + package_key, + &pkg_path.join(normalized), + verify_result, + file_info, + patched_content, + ) + .await + { + Ok(write) => write, + Err(e) => { + result.error = Some(e.to_string()); + return result; + } + }; + // ONE write site for every source, so a write failure (EACCES on // the stage, ENOSPC, a failed rename) is reported as what it is. // Single copy: // the public `apply_package_patch` wrapper fans out to pnpm // peer-variant copies itself, with per-copy verification. - match apply_file_patch_at(pkg_path, file_name, &patched_content, &file_info.after_hash) - .await - { + match apply_file_patch_at(pkg_path, file_name, &patched_content, &written_hash).await { Ok(warning) => warnings.extend(warning), Err(e) => { // A Windows daemon holding a cached jar open: say which @@ -1302,6 +1335,55 @@ async fn apply_package_patch_at( result } +/// The bytes to write for a patched file, and their hash: the patched +/// content as is, or npm's `fixBin` form of it (see +/// [`npm_bin`](crate::patch::npm_bin)) when the file on disk is a bin npm +/// installed with an LF shebang. That is the case when verify matched +/// npm's form of the original, and, for an npm package, when an +/// overwrite with consent replaces such a file: writing the CRLF shebang +/// back would leave a bin `env` can't run. +async fn npm_fixed_write<'a>( + package_key: &str, + path: &Path, + verify: Option<&VerifyResult>, + file_info: &PatchFileInfo, + patched: Cow<'a, [u8]>, +) -> Result<(Cow<'a, [u8]>, String), std::io::Error> { + use crate::patch::npm_bin; + let raw_before = verify.is_none_or(|v| { + v.current_hash.is_none() || v.current_hash.as_deref() == Some(&file_info.before_hash) + }); + if file_info.before_hash.is_empty() || raw_before { + return Ok((patched, file_info.after_hash.clone())); + } + let installed = match crate::utils::fs::read_regular_to_bytes(path).await { + Ok(bytes) => bytes, + Err(_) => return Ok((patched, file_info.after_hash.clone())), + }; + let npm_installed = npm_bin::has_lf_shebang(&installed) + && (package_key.starts_with("pkg:npm/") + || npm_bin::unfix_bin(&installed) + .is_some_and(|src| compute_git_sha256_from_bytes(&src) == file_info.before_hash)); + let fixed = npm_installed.then(|| npm_bin::fix_bin(&patched)).flatten(); + let Some(fixed) = fixed else { + return Ok((patched, file_info.after_hash.clone())); + }; + // The write below hash-checks what it writes; check the blob here, as + // it would have been checked. + let blob_hash = compute_git_sha256_from_bytes(&patched); + if blob_hash != file_info.after_hash { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!( + "Hash verification failed before patch. Expected: {}, Got: {}", + file_info.after_hash, blob_hash + ), + )); + } + let hash = compute_git_sha256_from_bytes(&fixed); + Ok((Cow::Owned(fixed), hash)) +} + /// Whether `hash` (a file's current content hash) names a blob Socket /// stored: content some Socket patch wrote, not a local edit. async fn is_socket_content(sources: &PatchSources<'_>, hash: Option<&str>) -> bool { @@ -3910,4 +3992,183 @@ mod tests { ); assert!(advisory_warning_codes("All patch files were skipped").is_empty()); } + + /// The tarball bytes of a bin published from Windows (CRLF shebang), + /// and the bytes npm's `bin-links` `fixBin` leaves on disk for it on + /// Linux and macOS: only the first line's `\r` is dropped (#1756). + const CRLF_BIN_BEFORE: &[u8] = b"#!/usr/bin/env node\r\nconsole.log(\"hi\");\r\n"; + const CRLF_BIN_AFTER: &[u8] = + b"#!/usr/bin/env node\r\nconsole.log(\"hi\");\r\n// SOCKET-PATCHED\r\n"; + const NPM_BIN_BEFORE: &[u8] = b"#!/usr/bin/env node\nconsole.log(\"hi\");\r\n"; + const NPM_BIN_AFTER: &[u8] = + b"#!/usr/bin/env node\nconsole.log(\"hi\");\r\n// SOCKET-PATCHED\r\n"; + + fn crlf_bin_files() -> HashMap { + HashMap::from([( + "package/cli.js".to_string(), + PatchFileInfo { + before_hash: compute_git_sha256_from_bytes(CRLF_BIN_BEFORE), + after_hash: compute_git_sha256_from_bytes(CRLF_BIN_AFTER), + }, + )]) + } + + async fn crlf_bin_blobs() -> tempfile::TempDir { + let blobs = tempfile::tempdir().unwrap(); + for bytes in [CRLF_BIN_BEFORE, CRLF_BIN_AFTER] { + tokio::fs::write( + blobs.path().join(compute_git_sha256_from_bytes(bytes)), + bytes, + ) + .await + .unwrap(); + } + blobs + } + + /// Regression (#1756): a pristine npm install of a CRLF-shebang bin is + /// the original, and its patched twin is already patched. + #[tokio::test] + async fn verify_accepts_npm_fixed_crlf_shebang() { + let files = crlf_bin_files(); + let info = &files["package/cli.js"]; + let pkg = tempfile::tempdir().unwrap(); + tokio::fs::write(pkg.path().join("cli.js"), NPM_BIN_BEFORE) + .await + .unwrap(); + let ready = verify_file_patch(pkg.path(), "package/cli.js", info).await; + assert_eq!(ready.status, VerifyStatus::Ready, "{ready:?}"); + assert_eq!(ready.expected_hash, None, "no mismatch warning: {ready:?}"); + + tokio::fs::write(pkg.path().join("cli.js"), NPM_BIN_AFTER) + .await + .unwrap(); + let done = verify_file_patch(pkg.path(), "package/cli.js", info).await; + assert_eq!(done.status, VerifyStatus::AlreadyPatched, "{done:?}"); + } + + /// Regression (#1756): strict apply patches an npm-fixed bin and keeps + /// its shebang LF-terminated, so the bin still runs; a second apply is + /// a no-op. + #[tokio::test] + async fn apply_patches_npm_fixed_bin_and_keeps_it_runnable() { + let files = crlf_bin_files(); + let blobs = crlf_bin_blobs().await; + let pkg = tempfile::tempdir().unwrap(); + tokio::fs::write(pkg.path().join("cli.js"), NPM_BIN_BEFORE) + .await + .unwrap(); + let sources = PatchSources::blobs_only(blobs.path()); + let res = apply_package_patch( + "pkg:npm/crlfbin@1.0.0", + pkg.path(), + &files, + &sources, + false, + MismatchPolicy::Strict, + ) + .await; + assert!(res.success, "{res:?}"); + assert_eq!(res.files_patched, vec!["package/cli.js".to_string()]); + assert_eq!( + tokio::fs::read(pkg.path().join("cli.js")).await.unwrap(), + NPM_BIN_AFTER + ); + + let again = apply_package_patch( + "pkg:npm/crlfbin@1.0.0", + pkg.path(), + &files, + &sources, + false, + MismatchPolicy::Strict, + ) + .await; + assert!(again.success, "{again:?}"); + assert!(again.files_patched.is_empty(), "{again:?}"); + } + + /// Regression (#1756): overwriting a drifted npm bin with consent + /// writes the patched content with npm's LF shebang, never a + /// `#!/usr/bin/env node\r` line `env` can't run. + #[tokio::test] + async fn consent_overwrite_of_npm_bin_keeps_lf_shebang() { + let files = crlf_bin_files(); + let blobs = crlf_bin_blobs().await; + let pkg = tempfile::tempdir().unwrap(); + tokio::fs::write( + pkg.path().join("cli.js"), + b"#!/usr/bin/env node\nconsole.log(\"edited\");\n", + ) + .await + .unwrap(); + let res = apply_package_patch( + "pkg:npm/crlfbin@1.0.0", + pkg.path(), + &files, + &PatchSources::blobs_only(blobs.path()), + false, + MismatchPolicy::Warn, + ) + .await; + assert!(res.success, "{res:?}"); + // Still a disclosed overwrite of unknown content. + assert!(res.files_verified[0].expected_hash.is_some(), "{res:?}"); + assert_eq!( + tokio::fs::read(pkg.path().join("cli.js")).await.unwrap(), + NPM_BIN_AFTER + ); + } + + /// A non-npm package keeps the patch's exact bytes on a consent + /// overwrite: only npm rewrites bin shebangs. + #[tokio::test] + async fn consent_overwrite_outside_npm_writes_exact_bytes() { + let files = crlf_bin_files(); + let blobs = crlf_bin_blobs().await; + let pkg = tempfile::tempdir().unwrap(); + tokio::fs::write(pkg.path().join("cli.js"), b"#!/usr/bin/env node\nedited\n") + .await + .unwrap(); + let res = apply_package_patch( + "pkg:pypi/crlfbin@1.0.0", + pkg.path(), + &files, + &PatchSources::blobs_only(blobs.path()), + false, + MismatchPolicy::Warn, + ) + .await; + assert!(res.success, "{res:?}"); + assert_eq!( + tokio::fs::read(pkg.path().join("cli.js")).await.unwrap(), + CRLF_BIN_AFTER + ); + } + + /// A CRLF-shebang file npm left alone (not a `bin` entry) still + /// matches the raw hashes and gets the patch's exact bytes. + #[tokio::test] + async fn apply_keeps_exact_bytes_for_unfixed_crlf_file() { + let files = crlf_bin_files(); + let blobs = crlf_bin_blobs().await; + let pkg = tempfile::tempdir().unwrap(); + tokio::fs::write(pkg.path().join("cli.js"), CRLF_BIN_BEFORE) + .await + .unwrap(); + let res = apply_package_patch( + "pkg:npm/crlfbin@1.0.0", + pkg.path(), + &files, + &PatchSources::blobs_only(blobs.path()), + false, + MismatchPolicy::Strict, + ) + .await; + assert!(res.success, "{res:?}"); + assert_eq!( + tokio::fs::read(pkg.path().join("cli.js")).await.unwrap(), + CRLF_BIN_AFTER + ); + } } diff --git a/crates/socket-patch-core/src/patch/mod.rs b/crates/socket-patch-core/src/patch/mod.rs index 3f1f45b23..69e0c14de 100644 --- a/crates/socket-patch-core/src/patch/mod.rs +++ b/crates/socket-patch-core/src/patch/mod.rs @@ -4,6 +4,7 @@ pub mod apply_lock; pub mod copy_tree; pub(crate) mod file_hash; pub mod jvm_jar; +pub(crate) mod npm_bin; pub mod package; pub(crate) mod path_safety; pub mod redirect; diff --git a/crates/socket-patch-core/src/patch/npm_bin.rs b/crates/socket-patch-core/src/patch/npm_bin.rs new file mode 100644 index 000000000..6e49eab6b --- /dev/null +++ b/crates/socket-patch-core/src/patch/npm_bin.rs @@ -0,0 +1,167 @@ +//! npm's install-time rewrite of a `bin` file's Windows shebang. +//! +//! On Linux and macOS, npm's `bin-links` runs `fixBin` on every file a +//! package lists in `bin` (`lib/fix-bin.js`): when the first 2048 bytes +//! match `^#![^\n]+\r\n`, it rewrites the file with that first `\r\n` +//! turned into `\n` (`dos2Unix`). pnpm links bins through the same +//! module. So an untouched install of a bin published from Windows +//! differs from the tarball, and from a patch's `beforeHash` / +//! `afterHash`, by exactly one byte (#1756). +//! +//! The hash checks accept that form of the recorded content, and the +//! writers keep it, so the shebang stays one `env` can run. + +use std::path::Path; + +use crate::hash::git_sha256::compute_git_sha256_from_bytes; + +/// The bytes `fixBin` inspects. +const PROBE_LEN: usize = 2048; + +/// What `fixBin` leaves on disk for `bytes`, or `None` when it leaves the +/// file alone (no `#!…\r\n` first line within its 2048-byte probe). +/// +/// `dos2Unix` reads and writes the file as UTF-8, so bytes that are not +/// valid UTF-8 come back as U+FFFD, as they do from Node. +pub(crate) fn fix_bin(bytes: &[u8]) -> Option> { + let cr = first_newline(bytes)?.checked_sub(1)?; + // `[^\n]+` needs one byte between `#!` and the `\r\n`. + if cr < 3 || bytes[cr] != b'\r' { + return None; + } + let mut out = String::from_utf8_lossy(bytes).into_owned().into_bytes(); + // Lossy decoding can only replace bytes after the ASCII `\r`, or bytes + // inside the first line; find the `\r\n` again in the decoded text. + let cr = out.iter().position(|&b| b == b'\n')? - 1; + out.remove(cr); + Some(out) +} + +/// The content `fixBin` turned into `bytes`, or `None` when `bytes` is not +/// something `fixBin` writes (a `#!` first line of at least one byte, +/// ending in `\n` early enough that its `\r\n` original fit the probe). +pub(crate) fn unfix_bin(bytes: &[u8]) -> Option> { + let lf = first_newline(bytes)?; + if lf < 3 || lf + 1 >= PROBE_LEN { + return None; + } + let mut out = Vec::with_capacity(bytes.len() + 1); + out.extend_from_slice(&bytes[..lf]); + out.push(b'\r'); + out.extend_from_slice(&bytes[lf..]); + Some(out) +} + +/// Whether `bytes` starts with a `#!` line that ends in a bare `\n`: the +/// shebang shape npm installs a bin with. +pub(crate) fn has_lf_shebang(bytes: &[u8]) -> bool { + matches!(first_newline(bytes), Some(lf) if lf >= 3 && bytes[lf - 1] != b'\r') +} + +/// The hash of the content npm installed as the file at `path`, when the +/// file is in `fixBin`'s form ([`unfix_bin`]). `None` when it is not, or +/// it can't be read. +pub(crate) async fn source_hash(path: &Path) -> Option { + let bytes = crate::utils::fs::read_regular_to_bytes(path).await.ok()?; + unfix_bin(&bytes).map(|src| compute_git_sha256_from_bytes(&src)) +} + +/// The index of the first `\n` of a file that starts with `#!`, within +/// the probe. +fn first_newline(bytes: &[u8]) -> Option { + if !bytes.starts_with(b"#!") { + return None; + } + let probe = &bytes[..bytes.len().min(PROBE_LEN)]; + probe.iter().position(|&b| b == b'\n') +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn fix_bin_drops_only_the_first_lines_cr() { + assert_eq!( + fix_bin(b"#!/usr/bin/env node\r\na();\r\n").as_deref(), + Some(&b"#!/usr/bin/env node\na();\r\n"[..]) + ); + // The greedy `[^\n]+` keeps an earlier `\r`. + assert_eq!(fix_bin(b"#!x\r\r\n").as_deref(), Some(&b"#!x\r\n"[..])); + } + + #[test] + fn fix_bin_leaves_what_bin_links_leaves() { + for bytes in [ + &b"#!/usr/bin/env node\na();\r\n"[..], + b"#!\r\n", + b"// #!x\r\n", + b"#!/usr/bin/env node", + b"", + ] { + assert_eq!(fix_bin(bytes), None, "{bytes:?}"); + } + // The `\r\n` must fall within the 2048-byte probe. + let mut long = b"#!".to_vec(); + long.extend(vec![b'x'; 2044]); + long.extend_from_slice(b"\r\n"); + assert_eq!(long.len(), 2048); + assert!(fix_bin(&long).is_some()); + long.insert(2, b'x'); + assert_eq!(fix_bin(&long), None); + } + + #[test] + fn fix_bin_rewrites_invalid_utf8_like_node() { + assert_eq!( + fix_bin(b"#!node\r\n\xff\n").as_deref(), + Some("#!node\n\u{fffd}\n".as_bytes()) + ); + } + + #[test] + fn unfix_bin_inverts_fix_bin() { + for src in [ + &b"#!/usr/bin/env node\r\na();\r\n"[..], + b"#!x\r\n", + b"#!x\r\r\n", + ] { + let fixed = fix_bin(src).unwrap(); + assert_eq!(unfix_bin(&fixed).as_deref(), Some(src), "{src:?}"); + } + for bytes in [&b"#!\n"[..], b"a\n", b"#!x"] { + assert_eq!(unfix_bin(bytes), None, "{bytes:?}"); + } + // Its original's `\r\n` would not have fit the probe. + let mut long = b"#!".to_vec(); + long.extend(vec![b'x'; 2045]); + long.push(b'\n'); + assert_eq!(unfix_bin(&long), None); + } + + #[test] + fn has_lf_shebang_wants_a_bare_lf() { + assert!(has_lf_shebang(b"#!/bin/sh\necho\n")); + assert!(!has_lf_shebang(b"#!/bin/sh\r\necho\n")); + assert!(!has_lf_shebang(b"#!\n")); + assert!(!has_lf_shebang(b"echo\n")); + } + + #[tokio::test] + async fn source_hash_is_the_tarball_hash() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("cli.js"); + tokio::fs::write(&path, b"#!/usr/bin/env node\nx\r\n") + .await + .unwrap(); + assert_eq!( + source_hash(&path).await, + Some(compute_git_sha256_from_bytes( + b"#!/usr/bin/env node\r\nx\r\n" + )) + ); + tokio::fs::write(&path, b"x\n").await.unwrap(); + assert_eq!(source_hash(&path).await, None); + assert_eq!(source_hash(&dir.path().join("missing")).await, None); + } +} diff --git a/crates/socket-patch-core/src/patch/rollback.rs b/crates/socket-patch-core/src/patch/rollback.rs index f6b1e52ee..eb61d6178 100644 --- a/crates/socket-patch-core/src/patch/rollback.rs +++ b/crates/socket-patch-core/src/patch/rollback.rs @@ -210,7 +210,18 @@ pub async fn verify_file_rollback( // needs no blob to restore, so a garbage-collected blob must not turn // a finished, no-op rollback into a spurious `MissingBlob` failure // (which would otherwise block the whole package's rollback). - if current_hash == file_info.before_hash { + // npm installs a CRLF-shebang `bin` with an LF shebang (see + // `npm_bin`): hash the content it came from too, once, when the raw + // bytes match neither side. + let source_hash = + if current_hash == file_info.before_hash || current_hash == file_info.after_hash { + None + } else { + crate::patch::npm_bin::source_hash(&filepath).await + }; + if current_hash == file_info.before_hash + || source_hash.as_deref() == Some(file_info.before_hash.as_str()) + { return VerifyRollbackResult { file: file_name.to_string(), status: VerifyRollbackStatus::AlreadyOriginal, @@ -304,8 +315,12 @@ pub async fn verify_file_rollback( } } - // Check if matches expected patched hash (afterHash) - if current_hash != file_info.after_hash { + // Check if matches expected patched hash (afterHash). A `Ready` whose + // `current_hash` is not the `afterHash` is npm's form of it; the + // restore writes the original in the same form. + if current_hash != file_info.after_hash + && source_hash.as_deref() != Some(file_info.after_hash.as_str()) + { return VerifyRollbackResult { file: file_name.to_string(), status: VerifyRollbackStatus::HashMismatch, @@ -435,9 +450,16 @@ async fn holds_this_patch(dir: &Path, files: &HashMap) -> match crate::utils::fs::read_regular_to_bytes(&dir.join(rel)).await { Ok(bytes) => { let hash = crate::hash::git_sha256::compute_git_sha256_from_bytes(&bytes); - if hash == info.after_hash { + // npm's LF-shebang form of a bin counts as the content it + // came from (see `npm_bin`). + let source = || { + crate::patch::npm_bin::unfix_bin(&bytes) + .map(|src| crate::hash::git_sha256::compute_git_sha256_from_bytes(&src)) + }; + let is = |h: &str| hash == h || source().as_deref() == Some(h); + if is(&info.after_hash) { patched = true; - } else if hash != info.before_hash { + } else if !is(&info.before_hash) { return false; } } @@ -736,11 +758,43 @@ async fn rollback_package_patch_at( // file's original mode + uid/gid restored afterward. pnpm store // copies are rolled back by the package-level wrapper, one full // verify each. + // + // A file verify matched in npm's LF-shebang form (its + // `current_hash` is not the `afterHash`) gets the original in that + // form too, the bytes `npm install` leaves (see `npm_bin`). The + // blob is hash-checked first, as the write would have checked it. + let npm_fixed = result + .files_verified + .iter() + .any(|v| { + v.file == *file_name + && v.current_hash + .as_deref() + .is_some_and(|h| h != file_info.after_hash) + }) + .then(|| crate::patch::npm_bin::fix_bin(&original_content)) + .flatten(); + let (original_content, restored_hash) = match npm_fixed { + Some(fixed) => { + let blob_hash = + crate::hash::git_sha256::compute_git_sha256_from_bytes(&original_content); + if blob_hash != file_info.before_hash { + result.error = Some(format!( + "Hash verification failed before patch. Expected: {}, Got: {}", + file_info.before_hash, blob_hash + )); + return result; + } + let hash = crate::hash::git_sha256::compute_git_sha256_from_bytes(&fixed); + (fixed, hash) + } + None => (original_content, file_info.before_hash.clone()), + }; match crate::patch::apply::apply_file_patch_at( pkg_path, file_name, &original_content, - &file_info.before_hash, + &restored_hash, ) .await { @@ -3102,4 +3156,46 @@ mod tests { assert!(!cache.join("mod.cpython-311.pyc").exists()); assert!(cache.join("other.cpython-311.pyc").exists()); } + + /// Regression (#1756): an npm-fixed bin (CRLF shebang in the tarball, + /// LF on disk) at its patched content rolls back to npm's form of the + /// original, and is then already original. + #[tokio::test] + async fn rollback_restores_npm_fixed_bin() { + let before: &[u8] = b"#!/usr/bin/env node\r\nconsole.log(1);\r\n"; + let after: &[u8] = b"#!/usr/bin/env node\r\nconsole.log(2);\r\n"; + let pkg = tempfile::tempdir().unwrap(); + let blobs = tempfile::tempdir().unwrap(); + let before_hash = compute_git_sha256_from_bytes(before); + tokio::fs::write(blobs.path().join(&before_hash), before) + .await + .unwrap(); + tokio::fs::write( + pkg.path().join("cli.js"), + b"#!/usr/bin/env node\nconsole.log(2);\r\n", + ) + .await + .unwrap(); + let files = HashMap::from([( + "package/cli.js".to_string(), + PatchFileInfo { + before_hash, + after_hash: compute_git_sha256_from_bytes(after), + }, + )]); + let res = + rollback_package_patch("pkg:npm/x@1.0.0", pkg.path(), &files, blobs.path(), false) + .await; + assert!(res.success, "{res:?}"); + assert_eq!(res.files_rolled_back, vec!["package/cli.js".to_string()]); + assert_eq!( + tokio::fs::read(pkg.path().join("cli.js")).await.unwrap(), + b"#!/usr/bin/env node\nconsole.log(1);\r\n" + ); + let again = + rollback_package_patch("pkg:npm/x@1.0.0", pkg.path(), &files, blobs.path(), false) + .await; + assert!(again.success, "{again:?}"); + assert!(again.files_rolled_back.is_empty(), "{again:?}"); + } } diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 45b485ba6..15f4828e6 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -783,6 +783,32 @@ mod tests { assert!(out.failed.is_empty()); } + /// Regression (#1756): npm drops the `\r` of a bin's CRLF shebang at + /// install, so a correctly patched hosted install carries the patched + /// bytes in npm's form. VEX attests it instead of omitting it as + /// `hash_mismatch`. + #[tokio::test] + async fn applied_when_npm_fixed_the_patched_bin_shebang() { + let pkg_dir = tempfile::tempdir().unwrap(); + let hash = compute_git_sha256_from_bytes(b"#!/usr/bin/env node\r\npatched();\r\n"); + tokio::fs::write( + pkg_dir.path().join("index.js"), + b"#!/usr/bin/env node\npatched();\r\n", + ) + .await + .unwrap(); + + let mut manifest = PatchManifest::new(); + manifest + .patches + .insert("pkg:npm/x@1.0.0".to_string(), record_with_one_file(&hash)); + let paths = HashMap::from([("pkg:npm/x@1.0.0".to_string(), pkg_dir.path().to_path_buf())]); + + let out = applied_patches(&manifest, &paths).await; + assert_eq!(out.applied, vec!["pkg:npm/x@1.0.0".to_string()], "{out:?}"); + assert!(out.failed.is_empty(), "{out:?}"); + } + /// Two nested copies of one `name@version`: `(dir, bytes)` each. async fn two_copies(a: &[u8], b: &[u8]) -> (tempfile::TempDir, Vec) { let root = tempfile::tempdir().unwrap(); From 7fff1e214b1c482775d5d943cee9d3c521d4004a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 01:44:03 +0000 Subject: [PATCH 3/7] Know npm-installed Socket blobs; probe cheaply A superseding patch may replace content an earlier Socket patch wrote. When npm installed that earlier patch with an LF shebang, its hash named no Socket blob, so the update was refused as a local edit. The check now also tries npm's original form of the file. Hashing npm's original form now reads a file whole only when its first 2 KB start with a shebang line, so a mismatched jar or wheel is not read twice. Refs #1756 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/patch/apply.rs | 49 ++++++++++++++++++- crates/socket-patch-core/src/patch/npm_bin.rs | 11 ++++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs index 213ba7204..c827d178f 100644 --- a/crates/socket-patch-core/src/patch/apply.rs +++ b/crates/socket-patch-core/src/patch/apply.rs @@ -1068,11 +1068,19 @@ async fn apply_package_patch_at( if verify_result.status != VerifyStatus::Ready && verify_result.status != VerifyStatus::AlreadyPatched { + // npm's LF-shebang form of a Socket blob (see `npm_bin`) is + // Socket content too. + let socket_content = || async { + is_socket_content(sources, verify_result.current_hash.as_deref()).await || { + let path = pkg_path.join(normalize_file_path(file_name)); + let source = crate::patch::npm_bin::source_hash(&path).await; + is_socket_content(sources, source.as_deref()).await + } + }; let policy = match policy { MismatchPolicy::SocketContent if verify_result.status == VerifyStatus::HashMismatch - && is_socket_content(sources, verify_result.current_hash.as_deref()) - .await => + && socket_content().await => { MismatchPolicy::Warn } @@ -4120,6 +4128,43 @@ mod tests { ); } + /// Regression (#1756): a superseding patch replaces an earlier Socket + /// patch npm installed in its LF-shebang form, as it replaces the + /// earlier patch's exact bytes. + #[tokio::test] + async fn socket_content_policy_knows_npm_fixed_socket_blob() { + let files = crlf_bin_files(); + let blobs = crlf_bin_blobs().await; + let earlier: &[u8] = b"#!/usr/bin/env node\r\nconsole.log(\"v1\");\r\n"; + tokio::fs::write( + blobs.path().join(compute_git_sha256_from_bytes(earlier)), + earlier, + ) + .await + .unwrap(); + let pkg = tempfile::tempdir().unwrap(); + tokio::fs::write( + pkg.path().join("cli.js"), + b"#!/usr/bin/env node\nconsole.log(\"v1\");\r\n", + ) + .await + .unwrap(); + let res = apply_package_patch( + "pkg:npm/crlfbin@1.0.0", + pkg.path(), + &files, + &PatchSources::blobs_only(blobs.path()), + false, + MismatchPolicy::SocketContent, + ) + .await; + assert!(res.success, "{res:?}"); + assert_eq!( + tokio::fs::read(pkg.path().join("cli.js")).await.unwrap(), + NPM_BIN_AFTER + ); + } + /// A non-npm package keeps the patch's exact bytes on a consent /// overwrite: only npm rewrites bin shebangs. #[tokio::test] diff --git a/crates/socket-patch-core/src/patch/npm_bin.rs b/crates/socket-patch-core/src/patch/npm_bin.rs index 6e49eab6b..ad7d8f29d 100644 --- a/crates/socket-patch-core/src/patch/npm_bin.rs +++ b/crates/socket-patch-core/src/patch/npm_bin.rs @@ -60,8 +60,17 @@ pub(crate) fn has_lf_shebang(bytes: &[u8]) -> bool { /// The hash of the content npm installed as the file at `path`, when the /// file is in `fixBin`'s form ([`unfix_bin`]). `None` when it is not, or -/// it can't be read. +/// it can't be read. Only a file whose first bytes qualify is read whole, +/// so a mismatched jar or wheel is not read twice. pub(crate) async fn source_hash(path: &Path) -> Option { + use tokio::io::AsyncReadExt as _; + let (file, _) = crate::utils::fs::open_regular_file(path).await.ok()?; + let mut prefix = Vec::with_capacity(PROBE_LEN); + file.take(PROBE_LEN as u64) + .read_to_end(&mut prefix) + .await + .ok()?; + unfix_bin(&prefix)?; let bytes = crate::utils::fs::read_regular_to_bytes(path).await.ok()?; unfix_bin(&bytes).map(|src| compute_git_sha256_from_bytes(&src)) } From 1441714ed3b86c1603dbcf09996b3e721e055e63 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 01:55:01 +0000 Subject: [PATCH 4/7] Close npm LF-shebang gaps found in review - Write a bin's recorded bytes when they are not valid UTF-8: npm would replace them with U+FFFD, a form nothing can trace back to the hash. - Roll back a bin the patch added when npm installed it with an LF shebang, as apply and vex already count it patched. - Keep the stale-install probe in step with vex for such bins. - A project rollback now sees a global copy still patched in npm's form and points at `rollback -g`. - On Windows npm leaves bins alone, so an overwrite there writes the patch's exact bytes. - One helper checks the blob and builds npm's form for both writers. Refs #1756 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/rollback.rs | 7 +- crates/socket-patch-core/src/patch/apply.rs | 48 +++++----- crates/socket-patch-core/src/patch/npm_bin.rs | 84 ++++++++++++++---- .../socket-patch-core/src/patch/rollback.rs | 87 ++++++++++++------- crates/socket-patch-core/src/vex/verify.rs | 24 +++++ 5 files changed, 173 insertions(+), 77 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 6f98e9957..39e0cc949 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -334,7 +334,12 @@ async fn patched_in_other_scope( let unverifiable = v.current_hash.is_none() && v.status != VerifyRollbackStatus::AlreadyOriginal && !v.is_absent(); - if unverifiable || v.current_hash.as_deref() == Some(info.after_hash.as_str()) { + // `Ready` also covers npm's LF-shebang form of a patched + // bin, whose hash is not the afterHash. + if unverifiable + || v.status == VerifyRollbackStatus::Ready + || v.current_hash.as_deref() == Some(info.after_hash.as_str()) + { patched.push((purl.clone(), path.clone())); break 'copies; } diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs index c827d178f..31f596578 100644 --- a/crates/socket-patch-core/src/patch/apply.rs +++ b/crates/socket-patch-core/src/patch/apply.rs @@ -1347,9 +1347,10 @@ async fn apply_package_patch_at( /// content as is, or npm's `fixBin` form of it (see /// [`npm_bin`](crate::patch::npm_bin)) when the file on disk is a bin npm /// installed with an LF shebang. That is the case when verify matched -/// npm's form of the original, and, for an npm package, when an -/// overwrite with consent replaces such a file: writing the CRLF shebang -/// back would leave a bin `env` can't run. +/// npm's form of the original, and, for an npm package off Windows (where +/// npm writes cmd-shims and leaves bins alone), when an overwrite with +/// consent replaces such a file: writing the CRLF shebang back would leave +/// a bin `env` can't run. async fn npm_fixed_write<'a>( package_key: &str, path: &Path, @@ -1358,38 +1359,27 @@ async fn npm_fixed_write<'a>( patched: Cow<'a, [u8]>, ) -> Result<(Cow<'a, [u8]>, String), std::io::Error> { use crate::patch::npm_bin; + let as_recorded = |patched| Ok((patched, file_info.after_hash.clone())); let raw_before = verify.is_none_or(|v| { v.current_hash.is_none() || v.current_hash.as_deref() == Some(&file_info.before_hash) }); if file_info.before_hash.is_empty() || raw_before { - return Ok((patched, file_info.after_hash.clone())); + return as_recorded(patched); } - let installed = match crate::utils::fs::read_regular_to_bytes(path).await { - Ok(bytes) => bytes, - Err(_) => return Ok((patched, file_info.after_hash.clone())), + let Ok(installed) = crate::utils::fs::read_regular_to_bytes(path).await else { + return as_recorded(patched); }; let npm_installed = npm_bin::has_lf_shebang(&installed) - && (package_key.starts_with("pkg:npm/") - || npm_bin::unfix_bin(&installed) - .is_some_and(|src| compute_git_sha256_from_bytes(&src) == file_info.before_hash)); - let fixed = npm_installed.then(|| npm_bin::fix_bin(&patched)).flatten(); - let Some(fixed) = fixed else { - return Ok((patched, file_info.after_hash.clone())); - }; - // The write below hash-checks what it writes; check the blob here, as - // it would have been checked. - let blob_hash = compute_git_sha256_from_bytes(&patched); - if blob_hash != file_info.after_hash { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidData, - format!( - "Hash verification failed before patch. Expected: {}, Got: {}", - file_info.after_hash, blob_hash - ), - )); + && ((!cfg!(windows) && package_key.starts_with("pkg:npm/")) + || npm_bin::source_hash_of(&installed).as_deref() + == Some(file_info.before_hash.as_str())); + if !npm_installed { + return as_recorded(patched); + } + match npm_bin::fixed_for_write(&patched, &file_info.after_hash)? { + Some((fixed, hash)) => Ok((Cow::Owned(fixed), hash)), + None => as_recorded(patched), } - let hash = compute_git_sha256_from_bytes(&fixed); - Ok((Cow::Owned(fixed), hash)) } /// Whether `hash` (a file's current content hash) names a blob Socket @@ -4098,7 +4088,9 @@ mod tests { /// Regression (#1756): overwriting a drifted npm bin with consent /// writes the patched content with npm's LF shebang, never a - /// `#!/usr/bin/env node\r` line `env` can't run. + /// `#!/usr/bin/env node\r` line `env` can't run. (On Windows npm + /// leaves bins alone, so the patch's exact bytes are written.) + #[cfg(not(windows))] #[tokio::test] async fn consent_overwrite_of_npm_bin_keeps_lf_shebang() { let files = crlf_bin_files(); diff --git a/crates/socket-patch-core/src/patch/npm_bin.rs b/crates/socket-patch-core/src/patch/npm_bin.rs index ad7d8f29d..024fd1d00 100644 --- a/crates/socket-patch-core/src/patch/npm_bin.rs +++ b/crates/socket-patch-core/src/patch/npm_bin.rs @@ -21,22 +21,44 @@ const PROBE_LEN: usize = 2048; /// What `fixBin` leaves on disk for `bytes`, or `None` when it leaves the /// file alone (no `#!…\r\n` first line within its 2048-byte probe). /// -/// `dos2Unix` reads and writes the file as UTF-8, so bytes that are not -/// valid UTF-8 come back as U+FFFD, as they do from Node. +/// Also `None` for bytes that are not valid UTF-8: `dos2Unix` reads and +/// writes the file as UTF-8, so npm would replace them with U+FFFD, and +/// that form can't be traced back to the recorded hash. Such a file is +/// written as recorded. pub(crate) fn fix_bin(bytes: &[u8]) -> Option> { let cr = first_newline(bytes)?.checked_sub(1)?; // `[^\n]+` needs one byte between `#!` and the `\r\n`. - if cr < 3 || bytes[cr] != b'\r' { + if cr < 3 || bytes[cr] != b'\r' || std::str::from_utf8(bytes).is_err() { return None; } - let mut out = String::from_utf8_lossy(bytes).into_owned().into_bytes(); - // Lossy decoding can only replace bytes after the ASCII `\r`, or bytes - // inside the first line; find the `\r\n` again in the decoded text. - let cr = out.iter().position(|&b| b == b'\n')? - 1; + let mut out = bytes.to_vec(); out.remove(cr); Some(out) } +/// The bytes to write for `content` (verified to hash to `expected_hash` +/// first, as `apply_file_patch_at` would) in `fixBin`'s form, and their +/// hash. `Ok(None)` when [`fix_bin`] leaves `content` alone. +pub(crate) fn fixed_for_write( + content: &[u8], + expected_hash: &str, +) -> Result, String)>, std::io::Error> { + let Some(fixed) = fix_bin(content) else { + return Ok(None); + }; + let content_hash = compute_git_sha256_from_bytes(content); + if content_hash != expected_hash { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!( + "Hash verification failed before patch. Expected: {expected_hash}, Got: {content_hash}" + ), + )); + } + let hash = compute_git_sha256_from_bytes(&fixed); + Ok(Some((fixed, hash))) +} + /// The content `fixBin` turned into `bytes`, or `None` when `bytes` is not /// something `fixBin` writes (a `#!` first line of at least one byte, /// ending in `\n` early enough that its `\r\n` original fit the probe). @@ -63,16 +85,24 @@ pub(crate) fn has_lf_shebang(bytes: &[u8]) -> bool { /// it can't be read. Only a file whose first bytes qualify is read whole, /// so a mismatched jar or wheel is not read twice. pub(crate) async fn source_hash(path: &Path) -> Option { - use tokio::io::AsyncReadExt as _; - let (file, _) = crate::utils::fs::open_regular_file(path).await.ok()?; + let path = path.to_path_buf(); + crate::utils::fs::run_blocking(move || source_hash_sync(&path)).await +} + +/// Blocking twin of [`source_hash`]. +pub(crate) fn source_hash_sync(path: &Path) -> Option { + use std::io::Read as _; + let (file, _) = crate::utils::fs::open_regular_file_sync(path).ok()?; let mut prefix = Vec::with_capacity(PROBE_LEN); - file.take(PROBE_LEN as u64) - .read_to_end(&mut prefix) - .await - .ok()?; + file.take(PROBE_LEN as u64).read_to_end(&mut prefix).ok()?; unfix_bin(&prefix)?; - let bytes = crate::utils::fs::read_regular_to_bytes(path).await.ok()?; - unfix_bin(&bytes).map(|src| compute_git_sha256_from_bytes(&src)) + let bytes = crate::utils::fs::read_regular_to_bytes_sync(path).ok()?; + source_hash_of(&bytes) +} + +/// [`source_hash`] of bytes already read. +pub(crate) fn source_hash_of(bytes: &[u8]) -> Option { + unfix_bin(bytes).map(|src| compute_git_sha256_from_bytes(&src)) } /// The index of the first `\n` of a file that starts with `#!`, within @@ -121,11 +151,27 @@ mod tests { } #[test] - fn fix_bin_rewrites_invalid_utf8_like_node() { - assert_eq!( - fix_bin(b"#!node\r\n\xff\n").as_deref(), - Some("#!node\n\u{fffd}\n".as_bytes()) + fn fix_bin_keeps_bytes_npm_would_mangle() { + // npm would write U+FFFD for the 0xff, which nothing can trace + // back to the recorded hash. + assert_eq!(fix_bin(b"#!node\r\n\xff\n"), None); + } + + #[test] + fn fixed_for_write_checks_the_content_first() { + let content = b"#!/usr/bin/env node\r\nx\r\n"; + let hash = compute_git_sha256_from_bytes(content); + let (fixed, fixed_hash) = fixed_for_write(content, &hash).unwrap().unwrap(); + assert_eq!(fixed, b"#!/usr/bin/env node\nx\r\n"); + assert_eq!(fixed_hash, compute_git_sha256_from_bytes(&fixed)); + let err = fixed_for_write(content, &"0".repeat(64)).unwrap_err(); + assert!( + err.to_string().contains("Hash verification failed"), + "{err}" ); + assert!(fixed_for_write(b"x\r\n", &"0".repeat(64)) + .unwrap() + .is_none()); } #[test] diff --git a/crates/socket-patch-core/src/patch/rollback.rs b/crates/socket-patch-core/src/patch/rollback.rs index eb61d6178..0f7c8487a 100644 --- a/crates/socket-patch-core/src/patch/rollback.rs +++ b/crates/socket-patch-core/src/patch/rollback.rs @@ -157,7 +157,14 @@ pub async fn verify_file_rollback( }; } }; - if current_hash == file_info.after_hash { + // npm's LF-shebang form of a patch-added bin is the patched file + // too (see `npm_bin`). + if current_hash == file_info.after_hash + || crate::patch::npm_bin::source_hash(&filepath) + .await + .as_deref() + == Some(file_info.after_hash.as_str()) + { return VerifyRollbackResult { file: file_name.to_string(), status: VerifyRollbackStatus::Ready, @@ -451,12 +458,15 @@ async fn holds_this_patch(dir: &Path, files: &HashMap) -> Ok(bytes) => { let hash = crate::hash::git_sha256::compute_git_sha256_from_bytes(&bytes); // npm's LF-shebang form of a bin counts as the content it - // came from (see `npm_bin`). - let source = || { - crate::patch::npm_bin::unfix_bin(&bytes) - .map(|src| crate::hash::git_sha256::compute_git_sha256_from_bytes(&src)) + // came from (see `npm_bin`); hashed once, when needed. + let source = std::cell::OnceCell::new(); + let is = |h: &str| { + hash == h + || source + .get_or_init(|| crate::patch::npm_bin::source_hash_of(&bytes)) + .as_deref() + == Some(h) }; - let is = |h: &str| hash == h || source().as_deref() == Some(h); if is(&info.after_hash) { patched = true; } else if !is(&info.before_hash) { @@ -763,33 +773,26 @@ async fn rollback_package_patch_at( // `current_hash` is not the `afterHash`) gets the original in that // form too, the bytes `npm install` leaves (see `npm_bin`). The // blob is hash-checked first, as the write would have checked it. - let npm_fixed = result - .files_verified - .iter() - .any(|v| { - v.file == *file_name - && v.current_hash - .as_deref() - .is_some_and(|h| h != file_info.after_hash) - }) - .then(|| crate::patch::npm_bin::fix_bin(&original_content)) - .flatten(); - let (original_content, restored_hash) = match npm_fixed { - Some(fixed) => { - let blob_hash = - crate::hash::git_sha256::compute_git_sha256_from_bytes(&original_content); - if blob_hash != file_info.before_hash { - result.error = Some(format!( - "Hash verification failed before patch. Expected: {}, Got: {}", - file_info.before_hash, blob_hash - )); + let npm_fixed = result.files_verified.iter().any(|v| { + v.file == *file_name + && v.current_hash + .as_deref() + .is_some_and(|h| h != file_info.after_hash) + }); + let fixed = if npm_fixed { + match crate::patch::npm_bin::fixed_for_write(&original_content, &file_info.before_hash) + { + Ok(fixed) => fixed, + Err(e) => { + result.error = Some(e.to_string()); return result; } - let hash = crate::hash::git_sha256::compute_git_sha256_from_bytes(&fixed); - (fixed, hash) } - None => (original_content, file_info.before_hash.clone()), + } else { + None }; + let (original_content, restored_hash) = + fixed.unwrap_or_else(|| (original_content, file_info.before_hash.clone())); match crate::patch::apply::apply_file_patch_at( pkg_path, file_name, @@ -3198,4 +3201,30 @@ mod tests { assert!(again.success, "{again:?}"); assert!(again.files_rolled_back.is_empty(), "{again:?}"); } + + /// Regression (#1756): a bin the patch added, installed by npm in its + /// LF-shebang form, rolls back (is deleted) like the exact bytes. + #[tokio::test] + async fn rollback_deletes_npm_fixed_added_bin() { + let pkg = tempfile::tempdir().unwrap(); + let blobs = tempfile::tempdir().unwrap(); + tokio::fs::write( + pkg.path().join("cli.js"), + b"#!/usr/bin/env node\nnew();\r\n", + ) + .await + .unwrap(); + let files = HashMap::from([( + "package/cli.js".to_string(), + PatchFileInfo { + before_hash: String::new(), + after_hash: compute_git_sha256_from_bytes(b"#!/usr/bin/env node\r\nnew();\r\n"), + }, + )]); + let res = + rollback_package_patch("pkg:npm/x@1.0.0", pkg.path(), &files, blobs.path(), false) + .await; + assert!(res.success, "{res:?}"); + assert!(!pkg.path().join("cli.js").exists()); + } } diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 15f4828e6..6893c7175 100644 --- a/crates/socket-patch-core/src/vex/verify.rs +++ b/crates/socket-patch-core/src/vex/verify.rs @@ -424,6 +424,12 @@ fn judge_installed_files(pkg_path: &Path, files: &[(String, String)]) -> Install .flatten(); match hashed { Some(hash) if &hash == after_hash => {} + // npm's LF-shebang form of the patched bin (see `npm_bin`), + // which `verify_file_patch` also accepts. + Some(_) + if crate::patch::npm_bin::source_hash_sync(&pkg_path.join(normalized)) + .as_deref() + == Some(after_hash.as_str()) => {} Some(_) => { return InstalledRecordJudgment { patched: false, @@ -809,6 +815,24 @@ mod tests { assert!(out.failed.is_empty(), "{out:?}"); } + /// `judge_installed_record` agrees with `verify_patch_record` on npm's + /// LF-shebang form of a patched bin (#1756): patched, not stale. + #[tokio::test] + async fn judge_installed_record_accepts_npm_fixed_bin() { + let pkg_dir = tempfile::tempdir().unwrap(); + let hash = compute_git_sha256_from_bytes(b"#!/usr/bin/env node\r\npatched();\r\n"); + tokio::fs::write( + pkg_dir.path().join("index.js"), + b"#!/usr/bin/env node\npatched();\r\n", + ) + .await + .unwrap(); + let record = record_with_one_file(&hash); + assert!(verify_patch_record(pkg_dir.path(), &record).await.is_ok()); + let judged = judge_installed_record(pkg_dir.path(), &record).await; + assert!(judged.patched && !judged.stale_evidence, "{judged:?}"); + } + /// Two nested copies of one `name@version`: `(dir, bytes)` each. async fn two_copies(a: &[u8], b: &[u8]) -> (tempfile::TempDir, Vec) { let root = tempfile::tempdir().unwrap(); From aea403ec8d29a4dfa25d5aed2489cdd4ff19413f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 02:48:10 +0000 Subject: [PATCH 5/7] Restore npm's form of a bin on rollback When a patch's own bytes already gave a CRLF-shebang bin an LF shebang, apply wrote them as recorded, and rollback then restored the CRLF tarball original: the bin failed with "env: 'node\r'". Rollback and apply now both decide from the file on disk: off Windows, an npm package's bin with an LF shebang gets npm's form of whatever is written; elsewhere only a file that is provably npm's form of the content it replaces does. Refs #1756 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/patch/apply.rs | 49 +------- crates/socket-patch-core/src/patch/npm_bin.rs | 52 ++++++++- .../socket-patch-core/src/patch/rollback.rs | 108 ++++++++++++++---- 3 files changed, 136 insertions(+), 73 deletions(-) diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs index 31f596578..1ad8175de 100644 --- a/crates/socket-patch-core/src/patch/apply.rs +++ b/crates/socket-patch-core/src/patch/apply.rs @@ -299,9 +299,7 @@ pub async fn verify_file_patch( }; } - // Check if matches expected before hash. A `Ready` whose - // `current_hash` is not the `beforeHash` is npm's form of it; the apply - // engine writes the patched bytes in the same form. + // Check if matches expected before hash (or npm's form of it). if current_hash != file_info.before_hash && source_hash.as_deref() != Some(file_info.before_hash.as_str()) { @@ -1250,12 +1248,12 @@ async fn apply_package_patch_at( // A bin npm installed with an LF shebang gets the patched bytes in // that form too (see `npm_bin`). - let (patched_content, written_hash) = match npm_fixed_write( + let (patched_content, written_hash) = match crate::patch::npm_bin::form_for_write( package_key, &pkg_path.join(normalized), - verify_result, - file_info, + &file_info.before_hash, patched_content, + &file_info.after_hash, ) .await { @@ -1343,45 +1341,6 @@ async fn apply_package_patch_at( result } -/// The bytes to write for a patched file, and their hash: the patched -/// content as is, or npm's `fixBin` form of it (see -/// [`npm_bin`](crate::patch::npm_bin)) when the file on disk is a bin npm -/// installed with an LF shebang. That is the case when verify matched -/// npm's form of the original, and, for an npm package off Windows (where -/// npm writes cmd-shims and leaves bins alone), when an overwrite with -/// consent replaces such a file: writing the CRLF shebang back would leave -/// a bin `env` can't run. -async fn npm_fixed_write<'a>( - package_key: &str, - path: &Path, - verify: Option<&VerifyResult>, - file_info: &PatchFileInfo, - patched: Cow<'a, [u8]>, -) -> Result<(Cow<'a, [u8]>, String), std::io::Error> { - use crate::patch::npm_bin; - let as_recorded = |patched| Ok((patched, file_info.after_hash.clone())); - let raw_before = verify.is_none_or(|v| { - v.current_hash.is_none() || v.current_hash.as_deref() == Some(&file_info.before_hash) - }); - if file_info.before_hash.is_empty() || raw_before { - return as_recorded(patched); - } - let Ok(installed) = crate::utils::fs::read_regular_to_bytes(path).await else { - return as_recorded(patched); - }; - let npm_installed = npm_bin::has_lf_shebang(&installed) - && ((!cfg!(windows) && package_key.starts_with("pkg:npm/")) - || npm_bin::source_hash_of(&installed).as_deref() - == Some(file_info.before_hash.as_str())); - if !npm_installed { - return as_recorded(patched); - } - match npm_bin::fixed_for_write(&patched, &file_info.after_hash)? { - Some((fixed, hash)) => Ok((Cow::Owned(fixed), hash)), - None => as_recorded(patched), - } -} - /// Whether `hash` (a file's current content hash) names a blob Socket /// stored: content some Socket patch wrote, not a local edit. async fn is_socket_content(sources: &PatchSources<'_>, hash: Option<&str>) -> bool { diff --git a/crates/socket-patch-core/src/patch/npm_bin.rs b/crates/socket-patch-core/src/patch/npm_bin.rs index 024fd1d00..6260581c2 100644 --- a/crates/socket-patch-core/src/patch/npm_bin.rs +++ b/crates/socket-patch-core/src/patch/npm_bin.rs @@ -59,6 +59,42 @@ pub(crate) fn fixed_for_write( Ok(Some((fixed, hash))) } +/// The bytes to write over the file at `path` for `content` (recorded as +/// `expected_hash`), and their hash: `content` in `fixBin`'s form when +/// the file now on disk is a bin npm installed with an LF shebang, else +/// `content` as recorded. Writing a CRLF shebang there would leave a bin +/// `env` can't run. +/// +/// The file is taken for an npm-installed bin when it has an LF shebang +/// and either belongs to an npm package (off Windows: npm writes +/// cmd-shims there and leaves bins alone), or hashes, in npm's original +/// form, to `installed_source` (the content this write replaces). +pub(crate) async fn form_for_write<'a>( + package_key: &str, + path: &Path, + installed_source: &str, + content: std::borrow::Cow<'a, [u8]>, + expected_hash: &str, +) -> Result<(std::borrow::Cow<'a, [u8]>, String), std::io::Error> { + let as_recorded = |content| Ok((content, expected_hash.to_string())); + let lf_shebang = read_prefix(path) + .await + .is_some_and(|prefix| has_lf_shebang(&prefix)); + let npm_installed = lf_shebang + && ((!cfg!(windows) + && crate::crawlers::Ecosystem::from_purl(package_key) + == Some(crate::crawlers::Ecosystem::Npm)) + || (!installed_source.is_empty() + && source_hash(path).await.as_deref() == Some(installed_source))); + if !npm_installed { + return as_recorded(content); + } + match fixed_for_write(&content, expected_hash)? { + Some((fixed, hash)) => Ok((std::borrow::Cow::Owned(fixed), hash)), + None => as_recorded(content), + } +} + /// The content `fixBin` turned into `bytes`, or `None` when `bytes` is not /// something `fixBin` writes (a `#!` first line of at least one byte, /// ending in `\n` early enough that its `\r\n` original fit the probe). @@ -91,13 +127,23 @@ pub(crate) async fn source_hash(path: &Path) -> Option { /// Blocking twin of [`source_hash`]. pub(crate) fn source_hash_sync(path: &Path) -> Option { + unfix_bin(&read_prefix_sync(path)?)?; + let bytes = crate::utils::fs::read_regular_to_bytes_sync(path).ok()?; + source_hash_of(&bytes) +} + +/// The first [`PROBE_LEN`] bytes of the regular file at `path`. +async fn read_prefix(path: &Path) -> Option> { + let path = path.to_path_buf(); + crate::utils::fs::run_blocking(move || read_prefix_sync(&path)).await +} + +fn read_prefix_sync(path: &Path) -> Option> { use std::io::Read as _; let (file, _) = crate::utils::fs::open_regular_file_sync(path).ok()?; let mut prefix = Vec::with_capacity(PROBE_LEN); file.take(PROBE_LEN as u64).read_to_end(&mut prefix).ok()?; - unfix_bin(&prefix)?; - let bytes = crate::utils::fs::read_regular_to_bytes_sync(path).ok()?; - source_hash_of(&bytes) + Some(prefix) } /// [`source_hash`] of bytes already read. diff --git a/crates/socket-patch-core/src/patch/rollback.rs b/crates/socket-patch-core/src/patch/rollback.rs index 0f7c8487a..9c5b5735b 100644 --- a/crates/socket-patch-core/src/patch/rollback.rs +++ b/crates/socket-patch-core/src/patch/rollback.rs @@ -322,9 +322,8 @@ pub async fn verify_file_rollback( } } - // Check if matches expected patched hash (afterHash). A `Ready` whose - // `current_hash` is not the `afterHash` is npm's form of it; the - // restore writes the original in the same form. + // Check if matches expected patched hash (afterHash, or npm's form of + // it). if current_hash != file_info.after_hash && source_hash.as_deref() != Some(file_info.after_hash.as_str()) { @@ -769,30 +768,23 @@ async fn rollback_package_patch_at( // copies are rolled back by the package-level wrapper, one full // verify each. // - // A file verify matched in npm's LF-shebang form (its - // `current_hash` is not the `afterHash`) gets the original in that - // form too, the bytes `npm install` leaves (see `npm_bin`). The - // blob is hash-checked first, as the write would have checked it. - let npm_fixed = result.files_verified.iter().any(|v| { - v.file == *file_name - && v.current_hash - .as_deref() - .is_some_and(|h| h != file_info.after_hash) - }); - let fixed = if npm_fixed { - match crate::patch::npm_bin::fixed_for_write(&original_content, &file_info.before_hash) - { - Ok(fixed) => fixed, - Err(e) => { - result.error = Some(e.to_string()); - return result; - } + // A bin npm installed with an LF shebang gets the original in that + // form, the bytes `npm install` leaves (see `npm_bin`). + let (original_content, restored_hash) = match crate::patch::npm_bin::form_for_write( + package_key, + &pkg_path.join(normalize_file_path(file_name)), + &file_info.after_hash, + std::borrow::Cow::Owned(original_content), + &file_info.before_hash, + ) + .await + { + Ok(write) => write, + Err(e) => { + result.error = Some(e.to_string()); + return result; } - } else { - None }; - let (original_content, restored_hash) = - fixed.unwrap_or_else(|| (original_content, file_info.before_hash.clone())); match crate::patch::apply::apply_file_patch_at( pkg_path, file_name, @@ -3227,4 +3219,70 @@ mod tests { assert!(res.success, "{res:?}"); assert!(!pkg.path().join("cli.js").exists()); } + + /// Regression (Bugbot on #1757): a patch whose patched bin already has + /// an LF shebang is applied over npm's form of a CRLF original and + /// written as recorded. Rolling it back restores npm's LF form, not + /// the CRLF tarball bytes `env` can't run. + #[cfg(not(windows))] + #[tokio::test] + async fn rollback_of_lf_patched_bin_restores_npm_form() { + let before: &[u8] = b"#!/usr/bin/env node\r\nconsole.log(1);\r\n"; + let after: &[u8] = b"#!/usr/bin/env node\nconsole.log(2);\n"; + let pkg = tempfile::tempdir().unwrap(); + let blobs = tempfile::tempdir().unwrap(); + let before_hash = compute_git_sha256_from_bytes(before); + tokio::fs::write(blobs.path().join(&before_hash), before) + .await + .unwrap(); + tokio::fs::write(pkg.path().join("cli.js"), after) + .await + .unwrap(); + let files = HashMap::from([( + "package/cli.js".to_string(), + PatchFileInfo { + before_hash, + after_hash: compute_git_sha256_from_bytes(after), + }, + )]); + let res = + rollback_package_patch("pkg:npm/x@1.0.0", pkg.path(), &files, blobs.path(), false) + .await; + assert!(res.success, "{res:?}"); + assert_eq!( + tokio::fs::read(pkg.path().join("cli.js")).await.unwrap(), + b"#!/usr/bin/env node\nconsole.log(1);\r\n" + ); + } + + /// Outside npm, a rollback restores the recorded original exactly. + #[tokio::test] + async fn rollback_outside_npm_restores_exact_bytes() { + let before: &[u8] = b"#!/usr/bin/env python\r\nprint(1)\r\n"; + let after: &[u8] = b"#!/usr/bin/env python\nprint(2)\n"; + let pkg = tempfile::tempdir().unwrap(); + let blobs = tempfile::tempdir().unwrap(); + let before_hash = compute_git_sha256_from_bytes(before); + tokio::fs::write(blobs.path().join(&before_hash), before) + .await + .unwrap(); + tokio::fs::write(pkg.path().join("cli.py"), after) + .await + .unwrap(); + let files = HashMap::from([( + "cli.py".to_string(), + PatchFileInfo { + before_hash, + after_hash: compute_git_sha256_from_bytes(after), + }, + )]); + let res = + rollback_package_patch("pkg:pypi/x@1.0.0", pkg.path(), &files, blobs.path(), false) + .await; + assert!(res.success, "{res:?}"); + assert_eq!( + tokio::fs::read(pkg.path().join("cli.py")).await.unwrap(), + before + ); + } } From 5831b1af6d6bd1052c81817d09ddebea66f67d5b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 05:15:29 +0000 Subject: [PATCH 6/7] Gate npm-fixed SocketContent test off Windows On Windows npm leaves bin files alone, so form_for_write writes the patch's exact CRLF bytes there and this test's NPM_BIN_AFTER assertion fails. Windows unit tests only run in the merge queue, so the failure would evict the PR there. Gate it like its sibling consent_overwrite_of_npm_bin_keeps_lf_shebang. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-core/src/patch/apply.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs index 1ad8175de..b5c4cb18f 100644 --- a/crates/socket-patch-core/src/patch/apply.rs +++ b/crates/socket-patch-core/src/patch/apply.rs @@ -4081,7 +4081,9 @@ mod tests { /// Regression (#1756): a superseding patch replaces an earlier Socket /// patch npm installed in its LF-shebang form, as it replaces the - /// earlier patch's exact bytes. + /// earlier patch's exact bytes. (On Windows npm leaves bins alone, so + /// the patch's exact bytes are written.) + #[cfg(not(windows))] #[tokio::test] async fn socket_content_policy_knows_npm_fixed_socket_blob() { let files = crlf_bin_files(); From 1206981712eab253141e15b9b3920b1f3a59cd39 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 07:03:02 +0000 Subject: [PATCH 7/7] Keep npm's LF shebang form on Windows too npm's bin-links rewrites a bin's CRLF shebang on Windows as well: shim-bin.js runs fixBin right after writing the cmd-shim. The writers assumed npm left bins alone there, so a consent overwrite or a rollback on Windows wrote the recorded CRLF bytes instead of npm's form. Verify accepted both, so nothing broke, but the files no longer matched what npm installs. Drop the Windows exception and the test gates that assumed it. Refs #1756 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/patch/apply.rs | 8 ++------ crates/socket-patch-core/src/patch/npm_bin.rs | 13 ++++++------- crates/socket-patch-core/src/patch/rollback.rs | 1 - 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs index b5c4cb18f..9ab742e01 100644 --- a/crates/socket-patch-core/src/patch/apply.rs +++ b/crates/socket-patch-core/src/patch/apply.rs @@ -4047,9 +4047,7 @@ mod tests { /// Regression (#1756): overwriting a drifted npm bin with consent /// writes the patched content with npm's LF shebang, never a - /// `#!/usr/bin/env node\r` line `env` can't run. (On Windows npm - /// leaves bins alone, so the patch's exact bytes are written.) - #[cfg(not(windows))] + /// `#!/usr/bin/env node\r` line `env` can't run. #[tokio::test] async fn consent_overwrite_of_npm_bin_keeps_lf_shebang() { let files = crlf_bin_files(); @@ -4081,9 +4079,7 @@ mod tests { /// Regression (#1756): a superseding patch replaces an earlier Socket /// patch npm installed in its LF-shebang form, as it replaces the - /// earlier patch's exact bytes. (On Windows npm leaves bins alone, so - /// the patch's exact bytes are written.) - #[cfg(not(windows))] + /// earlier patch's exact bytes. #[tokio::test] async fn socket_content_policy_knows_npm_fixed_socket_blob() { let files = crlf_bin_files(); diff --git a/crates/socket-patch-core/src/patch/npm_bin.rs b/crates/socket-patch-core/src/patch/npm_bin.rs index 6260581c2..35a1f0822 100644 --- a/crates/socket-patch-core/src/patch/npm_bin.rs +++ b/crates/socket-patch-core/src/patch/npm_bin.rs @@ -1,7 +1,8 @@ //! npm's install-time rewrite of a `bin` file's Windows shebang. //! -//! On Linux and macOS, npm's `bin-links` runs `fixBin` on every file a -//! package lists in `bin` (`lib/fix-bin.js`): when the first 2048 bytes +//! npm's `bin-links` runs `fixBin` on every file a package lists in +//! `bin` (`lib/fix-bin.js`), on Windows too, after writing the cmd-shim +//! (`lib/shim-bin.js`): when the first 2048 bytes //! match `^#![^\n]+\r\n`, it rewrites the file with that first `\r\n` //! turned into `\n` (`dos2Unix`). pnpm links bins through the same //! module. So an untouched install of a bin published from Windows @@ -66,8 +67,7 @@ pub(crate) fn fixed_for_write( /// `env` can't run. /// /// The file is taken for an npm-installed bin when it has an LF shebang -/// and either belongs to an npm package (off Windows: npm writes -/// cmd-shims there and leaves bins alone), or hashes, in npm's original +/// and either belongs to an npm package, or hashes, in npm's original /// form, to `installed_source` (the content this write replaces). pub(crate) async fn form_for_write<'a>( package_key: &str, @@ -81,9 +81,8 @@ pub(crate) async fn form_for_write<'a>( .await .is_some_and(|prefix| has_lf_shebang(&prefix)); let npm_installed = lf_shebang - && ((!cfg!(windows) - && crate::crawlers::Ecosystem::from_purl(package_key) - == Some(crate::crawlers::Ecosystem::Npm)) + && (crate::crawlers::Ecosystem::from_purl(package_key) + == Some(crate::crawlers::Ecosystem::Npm) || (!installed_source.is_empty() && source_hash(path).await.as_deref() == Some(installed_source))); if !npm_installed { diff --git a/crates/socket-patch-core/src/patch/rollback.rs b/crates/socket-patch-core/src/patch/rollback.rs index 9c5b5735b..8a21b8ef7 100644 --- a/crates/socket-patch-core/src/patch/rollback.rs +++ b/crates/socket-patch-core/src/patch/rollback.rs @@ -3224,7 +3224,6 @@ mod tests { /// an LF shebang is applied over npm's form of a CRLF original and /// written as recorded. Rolling it back restores npm's LF form, not /// the CRLF tarball bytes `env` can't run. - #[cfg(not(windows))] #[tokio::test] async fn rollback_of_lf_patched_bin_restores_npm_form() { let before: &[u8] = b"#!/usr/bin/env node\r\nconsole.log(1);\r\n";