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-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..9ab742e01 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,10 @@ 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 (or npm's form of it). + 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, @@ -1051,11 +1066,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 } @@ -1223,14 +1246,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 crate::patch::npm_bin::form_for_write( + package_key, + &pkg_path.join(normalized), + &file_info.before_hash, + patched_content, + &file_info.after_hash, + ) + .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 @@ -3910,4 +3949,220 @@ 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 + ); + } + + /// 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] + 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..35a1f0822 --- /dev/null +++ b/crates/socket-patch-core/src/patch/npm_bin.rs @@ -0,0 +1,267 @@ +//! npm's install-time rewrite of a `bin` file's Windows shebang. +//! +//! 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 +//! 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). +/// +/// 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' || std::str::from_utf8(bytes).is_err() { + return None; + } + 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 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, 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 + && (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). +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. 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 { + 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 { + 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()?; + Some(prefix) +} + +/// [`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 +/// 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_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] + 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..8a21b8ef7 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, @@ -210,7 +217,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 +322,11 @@ 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, or npm's form of + // it). + 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 +456,19 @@ 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`); 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) + }; + if is(&info.after_hash) { patched = true; - } else if hash != info.before_hash { + } else if !is(&info.before_hash) { return false; } } @@ -736,11 +767,29 @@ 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 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; + } + }; match crate::patch::apply::apply_file_patch_at( pkg_path, file_name, &original_content, - &file_info.before_hash, + &restored_hash, ) .await { @@ -3102,4 +3151,137 @@ 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:?}"); + } + + /// 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()); + } + + /// 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. + #[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 + ); + } } diff --git a/crates/socket-patch-core/src/vex/verify.rs b/crates/socket-patch-core/src/vex/verify.rs index 45b485ba6..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, @@ -783,6 +789,50 @@ 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:?}"); + } + + /// `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();