Skip to content

Commit ec9aabc

Browse files
Fix npm CRLF-shebang bin hash checks (#1756) (#1757)
* Start fix for #1756 Assisted-by: Claude Code:claude-opus-5-5 * 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 * 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 * 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 * 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 * 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) <noreply@anthropic.com> * 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4345294 commit ec9aabc

7 files changed

Lines changed: 914 additions & 16 deletions

File tree

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,12 @@ async fn patched_in_other_scope(
334334
let unverifiable = v.current_hash.is_none()
335335
&& v.status != VerifyRollbackStatus::AlreadyOriginal
336336
&& !v.is_absent();
337-
if unverifiable || v.current_hash.as_deref() == Some(info.after_hash.as_str()) {
337+
// `Ready` also covers npm's LF-shebang form of a patched
338+
// bin, whose hash is not the afterHash.
339+
if unverifiable
340+
|| v.status == VerifyRollbackStatus::Ready
341+
|| v.current_hash.as_deref() == Some(info.after_hash.as_str())
342+
{
338343
patched.push((purl.clone(), path.clone()));
339344
break 'copies;
340345
}

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

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,3 +1160,141 @@ async fn npm_get_ghsa_hosted_narrows_and_installs() {
11601160
let (fresh, installed) = fresh_install_patched(&fx);
11611161
manifestless_tail(&fx, &fresh, installed, &[]);
11621162
}
1163+
1164+
/// Regression (#1756): on Linux and macOS npm's `bin-links` rewrites a
1165+
/// `bin` file's CRLF shebang to LF at install. Agent-mode `apply` patches
1166+
/// that install (it was refused as drift), keeps the bin runnable (a
1167+
/// written-back `#!/usr/bin/env node\r` makes `env` exit 127), re-applies
1168+
/// as a no-op, `vex` attests it, and `rollback` restores npm's form of the
1169+
/// original. Offline: the package is a local tarball.
1170+
#[cfg(unix)]
1171+
#[tokio::test(flavor = "multi_thread")]
1172+
#[ignore = "wall-bound real-npm install; runs on the e2e CI matrix leg"]
1173+
async fn npm_agent_patches_crlf_shebang_bin_npm_rewrote() {
1174+
const SUITE: &str = "npm_agent_patches_crlf_shebang_bin_npm_rewrote";
1175+
if npm_e2e_common::npm_version().is_none() {
1176+
npm_e2e_common::skip(SUITE, "npm not on PATH");
1177+
return;
1178+
}
1179+
let before: &[u8] = b"#!/usr/bin/env node\r\nconsole.log(\"hi\");\r\n";
1180+
let after: &[u8] = b"#!/usr/bin/env node\r\nconsole.log(\"hi\");\r\n// SOCKET-PATCHED\r\n";
1181+
// What npm leaves on disk for each: the first line's `\r` dropped.
1182+
let installed_before: &[u8] = b"#!/usr/bin/env node\nconsole.log(\"hi\");\r\n";
1183+
let installed_after: &[u8] =
1184+
b"#!/usr/bin/env node\nconsole.log(\"hi\");\r\n// SOCKET-PATCHED\r\n";
1185+
1186+
let tmp = tempfile::tempdir().unwrap();
1187+
let pkg = tmp.path().join("pkg");
1188+
std::fs::create_dir_all(&pkg).unwrap();
1189+
std::fs::write(
1190+
pkg.join("package.json"),
1191+
r#"{"name":"crlfbin","version":"1.0.0","bin":{"crlfbin":"cli.js"}}"#,
1192+
)
1193+
.unwrap();
1194+
std::fs::write(pkg.join("cli.js"), before).unwrap();
1195+
std::fs::write(
1196+
tmp.path().join("crlfbin-1.0.0.tgz"),
1197+
npm_e2e_common::npm_pack_like(&pkg),
1198+
)
1199+
.unwrap();
1200+
let proj = tmp.path().join("proj");
1201+
std::fs::create_dir_all(&proj).unwrap();
1202+
std::fs::write(
1203+
proj.join("package.json"),
1204+
r#"{"name":"p","version":"1.0.0","dependencies":{"crlfbin":"file:../crlfbin-1.0.0.tgz"}}"#,
1205+
)
1206+
.unwrap();
1207+
let cache = tmp.path().join("cache");
1208+
let out = npm_e2e_common::npm(
1209+
&proj,
1210+
&[
1211+
"install",
1212+
"--no-audit",
1213+
"--no-fund",
1214+
"--cache",
1215+
cache.to_str().unwrap(),
1216+
],
1217+
);
1218+
assert!(
1219+
out.status.success(),
1220+
"{}",
1221+
npm_e2e_common::output_text(&out)
1222+
);
1223+
let cli = proj.join("node_modules/crlfbin/cli.js");
1224+
assert_eq!(
1225+
std::fs::read(&cli).unwrap(),
1226+
installed_before,
1227+
"npm's fixBin"
1228+
);
1229+
1230+
let (b, a) = (git_sha256(before), git_sha256(after));
1231+
let blobs = proj.join(".socket/blobs");
1232+
std::fs::create_dir_all(&blobs).unwrap();
1233+
std::fs::write(blobs.join(&b), before).unwrap();
1234+
std::fs::write(blobs.join(&a), after).unwrap();
1235+
std::fs::write(
1236+
proj.join(".socket/manifest.json"),
1237+
format!(
1238+
r#"{{"patches":{{"pkg:npm/crlfbin@1.0.0":{{"uuid":"e2f99e5d-3afd-5cbd-be80-6a0102bcd6c2",
1239+
"exportedAt":"2026-01-01T00:00:00Z","files":{{"package/cli.js":{{"beforeHash":"{b}","afterHash":"{a}"}}}},
1240+
"vulnerabilities":{{"GHSA-xxxx-yyyy-zzzz":{{"cves":["CVE-2099-0001"],"summary":"t","severity":"high","description":"d"}}}},
1241+
"description":"d","license":"MIT","tier":"free"}}}}}}"#
1242+
),
1243+
)
1244+
.unwrap();
1245+
1246+
let run_bin = || {
1247+
std::process::Command::new(proj.join("node_modules/.bin/crlfbin"))
1248+
.current_dir(&proj)
1249+
.output()
1250+
.unwrap()
1251+
};
1252+
1253+
let (code, stdout, stderr) = run_socket(&proj, &["apply", "--offline"]);
1254+
assert_eq!(code, 0, "apply\nstdout:\n{stdout}\nstderr:\n{stderr}");
1255+
assert!(
1256+
!stderr.contains("did not match"),
1257+
"no drift warning: {stderr}"
1258+
);
1259+
assert_eq!(std::fs::read(&cli).unwrap(), installed_after);
1260+
let ran = run_bin();
1261+
assert!(
1262+
ran.status.success(),
1263+
"{}",
1264+
npm_e2e_common::output_text(&ran)
1265+
);
1266+
assert_eq!(String::from_utf8_lossy(&ran.stdout).trim(), "hi");
1267+
1268+
let (code, stdout, stderr) = run_socket(&proj, &["apply", "--offline"]);
1269+
assert_eq!(code, 0, "re-apply\nstdout:\n{stdout}\nstderr:\n{stderr}");
1270+
assert_eq!(std::fs::read(&cli).unwrap(), installed_after);
1271+
1272+
let doc = proj.join("vex.json");
1273+
let (code, stdout, stderr) = run_socket(
1274+
&proj,
1275+
&[
1276+
"vex",
1277+
"--offline",
1278+
"-O",
1279+
doc.to_str().unwrap(),
1280+
"--product",
1281+
"pkg:npm/p@1.0.0",
1282+
],
1283+
);
1284+
assert_eq!(code, 0, "vex\nstdout:\n{stdout}\nstderr:\n{stderr}");
1285+
let vex = std::fs::read_to_string(&doc).unwrap();
1286+
assert!(
1287+
vex.contains("CVE-2099-0001") && vex.contains("not_affected"),
1288+
"{vex}"
1289+
);
1290+
1291+
let (code, stdout, stderr) = run_socket(&proj, &["rollback", "--offline"]);
1292+
assert_eq!(code, 0, "rollback\nstdout:\n{stdout}\nstderr:\n{stderr}");
1293+
assert_eq!(std::fs::read(&cli).unwrap(), installed_before);
1294+
let ran = run_bin();
1295+
assert!(
1296+
ran.status.success(),
1297+
"{}",
1298+
npm_e2e_common::output_text(&ran)
1299+
);
1300+
}

0 commit comments

Comments
 (0)