Skip to content

Commit 9e8951c

Browse files
committed
Keep CRLF shebang on bins npm didn't link
Since #1792, apply and rollback treated every file a package lists as a bin as one npm had rewritten to an LF shebang at install. npm only does that for bins it links. With bin-links=false (--no-bin-links), or for a directories.bin file of a package installed from a tarball or folder, the installed file keeps its CRLF shebang, so apply wrote bytes that matched neither the patch nor a fresh install, and rollback no longer restored the installed file byte for byte. A listed bin now counts as npm's form only when npm linked it: a .bin/<name> entry in a node_modules the package sits under (a symlink must point at this file), a pnpm dependent's .bin entry, or a global prefix bin or .cmd shim. Bins npm did link, including ones an older release left with a CRLF shebang (#1791), are still repaired. Fixes #1804 Assisted-by: Claude Code:claude-opus-5-5
1 parent c48fa58 commit 9e8951c

4 files changed

Lines changed: 462 additions & 84 deletions

File tree

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

Lines changed: 91 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1174,15 +1174,27 @@ const NPM_BIN_AFTER: &[u8] = b"#!/usr/bin/env node\nconsole.log(\"hi\");\r\n// S
11741174
/// `bin`) npm-installed, and an agent manifest patching that bin with
11751175
/// its blobs. Returns the project dir.
11761176
fn crlf_bin_project(tmp: &Path) -> PathBuf {
1177+
let proj = crlf_bin_project_with(
1178+
tmp,
1179+
r#"{"name":"crlfbin","version":"1.0.0","bin":{"crlfbin":"cli.js"}}"#,
1180+
"cli.js",
1181+
&[],
1182+
);
1183+
let cli = proj.join("node_modules/crlfbin/cli.js");
1184+
assert_eq!(std::fs::read(&cli).unwrap(), NPM_BIN_BEFORE, "npm's fixBin");
1185+
proj
1186+
}
1187+
1188+
/// [`crlf_bin_project`] with the package's `package.json` `manifest`, the
1189+
/// CRLF-shebang file at `rel` in the package, and `flags` added to
1190+
/// `npm install`.
1191+
fn crlf_bin_project_with(tmp: &Path, manifest: &str, rel: &str, flags: &[&str]) -> PathBuf {
11771192
let (before, after) = (CRLF_BIN_BEFORE, CRLF_BIN_AFTER);
11781193
let pkg = tmp.join("pkg");
1179-
std::fs::create_dir_all(&pkg).unwrap();
1180-
std::fs::write(
1181-
pkg.join("package.json"),
1182-
r#"{"name":"crlfbin","version":"1.0.0","bin":{"crlfbin":"cli.js"}}"#,
1183-
)
1184-
.unwrap();
1185-
std::fs::write(pkg.join("cli.js"), before).unwrap();
1194+
let file = pkg.join(rel);
1195+
std::fs::create_dir_all(file.parent().unwrap()).unwrap();
1196+
std::fs::write(pkg.join("package.json"), manifest).unwrap();
1197+
std::fs::write(&file, before).unwrap();
11861198
std::fs::write(
11871199
tmp.join("crlfbin-1.0.0.tgz"),
11881200
npm_e2e_common::npm_pack_like(&pkg),
@@ -1196,23 +1208,20 @@ fn crlf_bin_project(tmp: &Path) -> PathBuf {
11961208
)
11971209
.unwrap();
11981210
let cache = tmp.join("cache");
1199-
let out = npm_e2e_common::npm(
1200-
&proj,
1201-
&[
1202-
"install",
1203-
"--no-audit",
1204-
"--no-fund",
1205-
"--cache",
1206-
cache.to_str().unwrap(),
1207-
],
1208-
);
1211+
let mut args = vec![
1212+
"install",
1213+
"--no-audit",
1214+
"--no-fund",
1215+
"--cache",
1216+
cache.to_str().unwrap(),
1217+
];
1218+
args.extend_from_slice(flags);
1219+
let out = npm_e2e_common::npm(&proj, &args);
12091220
assert!(
12101221
out.status.success(),
12111222
"{}",
12121223
npm_e2e_common::output_text(&out)
12131224
);
1214-
let cli = proj.join("node_modules/crlfbin/cli.js");
1215-
assert_eq!(std::fs::read(&cli).unwrap(), NPM_BIN_BEFORE, "npm's fixBin");
12161225

12171226
let (b, a) = (git_sha256(before), git_sha256(after));
12181227
let blobs = proj.join(".socket/blobs");
@@ -1223,7 +1232,7 @@ fn crlf_bin_project(tmp: &Path) -> PathBuf {
12231232
proj.join(".socket/manifest.json"),
12241233
format!(
12251234
r#"{{"patches":{{"pkg:npm/crlfbin@1.0.0":{{"uuid":"e2f99e5d-3afd-5cbd-be80-6a0102bcd6c2",
1226-
"exportedAt":"2026-01-01T00:00:00Z","files":{{"package/cli.js":{{"beforeHash":"{b}","afterHash":"{a}"}}}},
1235+
"exportedAt":"2026-01-01T00:00:00Z","files":{{"package/{rel}":{{"beforeHash":"{b}","afterHash":"{a}"}}}},
12271236
"vulnerabilities":{{"GHSA-xxxx-yyyy-zzzz":{{"cves":["CVE-2099-0001"],"summary":"t","severity":"high","description":"d"}}}},
12281237
"description":"d","license":"MIT","tier":"free"}}}}}}"#
12291238
),
@@ -1345,3 +1354,65 @@ async fn npm_agent_recovers_crlf_bin_left_by_raw_write() {
13451354
assert_eq!(std::fs::read(&cli).unwrap(), NPM_BIN_BEFORE);
13461355
assert_crlf_bin_runs(&proj, "after rollback");
13471356
}
1357+
1358+
/// Regression (#1804): npm rewrites the shebang only of bins it links.
1359+
/// With `--no-bin-links`, or for a `directories.bin`-only package
1360+
/// installed from a `file:` tarball (npm doesn't expand that field from a
1361+
/// tarball's `package.json`), the installed bin keeps its CRLF shebang.
1362+
/// `apply` writes the patch's recorded bytes, `apply --check` agrees, and
1363+
/// `rollback` restores what `npm install` wrote, byte for byte.
1364+
#[cfg(unix)]
1365+
#[tokio::test(flavor = "multi_thread")]
1366+
#[ignore = "wall-bound real-npm install; runs on the e2e CI matrix leg"]
1367+
async fn npm_agent_keeps_crlf_shebang_of_bin_npm_did_not_link() {
1368+
const SUITE: &str = "npm_agent_keeps_crlf_shebang_of_bin_npm_did_not_link";
1369+
if npm_e2e_common::npm_version().is_none() {
1370+
npm_e2e_common::skip(SUITE, "npm not on PATH");
1371+
return;
1372+
}
1373+
for (manifest, rel, flags) in [
1374+
(
1375+
r#"{"name":"crlfbin","version":"1.0.0","bin":{"crlfbin":"cli.js"}}"#,
1376+
"cli.js",
1377+
&["--no-bin-links"][..],
1378+
),
1379+
(
1380+
r#"{"name":"crlfbin","version":"1.0.0","directories":{"bin":"bin"}}"#,
1381+
"bin/cli.js",
1382+
&[][..],
1383+
),
1384+
] {
1385+
let tmp = tempfile::tempdir().unwrap();
1386+
let proj = crlf_bin_project_with(tmp.path(), manifest, rel, flags);
1387+
let cli = proj.join("node_modules/crlfbin").join(rel);
1388+
assert_eq!(
1389+
std::fs::read(&cli).unwrap(),
1390+
CRLF_BIN_BEFORE,
1391+
"{rel}: npm left the shebang alone"
1392+
);
1393+
1394+
let (code, stdout, stderr) = run_socket(&proj, &["apply", "--offline"]);
1395+
assert_eq!(
1396+
code, 0,
1397+
"{rel}: apply\nstdout:\n{stdout}\nstderr:\n{stderr}"
1398+
);
1399+
assert_eq!(std::fs::read(&cli).unwrap(), CRLF_BIN_AFTER, "{rel}: apply");
1400+
1401+
let (code, stdout, stderr) = run_socket(&proj, &["apply", "--check", "--offline"]);
1402+
assert_eq!(
1403+
code, 0,
1404+
"{rel}: check\nstdout:\n{stdout}\nstderr:\n{stderr}"
1405+
);
1406+
1407+
let (code, stdout, stderr) = run_socket(&proj, &["rollback", "--offline"]);
1408+
assert_eq!(
1409+
code, 0,
1410+
"{rel}: rollback\nstdout:\n{stdout}\nstderr:\n{stderr}"
1411+
);
1412+
assert_eq!(
1413+
std::fs::read(&cli).unwrap(),
1414+
CRLF_BIN_BEFORE,
1415+
"{rel}: rollback is byte-exact"
1416+
);
1417+
}
1418+
}

‎crates/socket-patch-core/src/patch/apply.rs‎

Lines changed: 99 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4180,21 +4180,26 @@ mod tests {
41804180
async fn apply_repairs_crlf_bin_left_by_raw_write() {
41814181
let files = crlf_bin_files();
41824182
let blobs = crlf_bin_blobs().await;
4183-
let pkg = tempfile::tempdir().unwrap();
4183+
let root = tempfile::tempdir().unwrap();
4184+
// npm linked the bin, so it ran `fixBin` on it at install.
4185+
let pkg = root.path().join("node_modules/crlfbin");
4186+
std::fs::create_dir_all(root.path().join("node_modules/.bin")).unwrap();
4187+
std::fs::write(root.path().join("node_modules/.bin/crlfbin"), b"shim").unwrap();
4188+
std::fs::create_dir_all(&pkg).unwrap();
41844189
tokio::fs::write(
4185-
pkg.path().join("package.json"),
4190+
pkg.join("package.json"),
41864191
br#"{"name":"crlfbin","version":"1.0.0","bin":{"crlfbin":"./cli.js"}}"#,
41874192
)
41884193
.await
41894194
.unwrap();
4190-
tokio::fs::write(pkg.path().join("cli.js"), CRLF_BIN_AFTER)
4195+
tokio::fs::write(pkg.join("cli.js"), CRLF_BIN_AFTER)
41914196
.await
41924197
.unwrap();
41934198
let sources = PatchSources::blobs_only(blobs.path());
41944199
let apply = || {
41954200
apply_package_patch(
41964201
"pkg:npm/crlfbin@1.0.0",
4197-
pkg.path(),
4202+
&pkg,
41984203
&files,
41994204
&sources,
42004205
false,
@@ -4205,7 +4210,7 @@ mod tests {
42054210
assert!(res.success, "{res:?}");
42064211
assert_eq!(res.files_patched, vec!["package/cli.js".to_string()]);
42074212
assert_eq!(
4208-
tokio::fs::read(pkg.path().join("cli.js")).await.unwrap(),
4213+
tokio::fs::read(pkg.join("cli.js")).await.unwrap(),
42094214
NPM_BIN_AFTER
42104215
);
42114216
let again = apply().await;
@@ -4214,7 +4219,7 @@ mod tests {
42144219
assert!(
42154220
!needs_npm_bin_rewrite(
42164221
"pkg:npm/crlfbin@1.0.0",
4217-
pkg.path(),
4222+
&pkg,
42184223
"package/cli.js",
42194224
&files["package/cli.js"]
42204225
)
@@ -4273,6 +4278,94 @@ mod tests {
42734278
}
42744279
}
42754280

4281+
/// Regression (#1804): npm rewrites the shebang only of bins it
4282+
/// links. With `bin-links=false`, or for a `directories.bin` file of a
4283+
/// `file:` tarball install (npm doesn't expand that field from a
4284+
/// tarball), the installed bin keeps its CRLF shebang, so apply writes
4285+
/// the recorded bytes and leaves them alone, and rollback restores the
4286+
/// installed bytes exactly.
4287+
#[tokio::test]
4288+
async fn apply_writes_recorded_bytes_for_bin_npm_did_not_link() {
4289+
for (manifest, rel, linked) in [
4290+
// `npm install --no-bin-links`: no `.bin` at all.
4291+
(
4292+
&br#"{"name":"crlfbin","version":"1.0.0","bin":{"crlfbin":"cli.js"}}"#[..],
4293+
"cli.js",
4294+
None,
4295+
),
4296+
// `directories.bin` from a tarball: `.bin` holds other bins only.
4297+
(
4298+
br#"{"name":"crlfbin","version":"1.0.0","directories":{"bin":"bin"}}"#,
4299+
"bin/cli.js",
4300+
Some("other"),
4301+
),
4302+
] {
4303+
let key = format!("package/{rel}");
4304+
let key = key.as_str();
4305+
let mut files = crlf_bin_files();
4306+
let info = files.remove("package/cli.js").unwrap();
4307+
files.insert(key.to_string(), info);
4308+
let blobs = crlf_bin_blobs().await;
4309+
let root = tempfile::tempdir().unwrap();
4310+
let pkg = root.path().join("node_modules/crlfbin");
4311+
std::fs::create_dir_all(pkg.join("bin")).unwrap();
4312+
if let Some(name) = linked {
4313+
std::fs::create_dir_all(root.path().join("node_modules/.bin")).unwrap();
4314+
std::fs::write(root.path().join("node_modules/.bin").join(name), b"shim").unwrap();
4315+
}
4316+
tokio::fs::write(pkg.join("package.json"), manifest)
4317+
.await
4318+
.unwrap();
4319+
let cli = pkg.join(rel);
4320+
tokio::fs::write(&cli, CRLF_BIN_BEFORE).await.unwrap();
4321+
let sources = PatchSources::blobs_only(blobs.path());
4322+
let apply = || {
4323+
apply_package_patch(
4324+
"pkg:npm/crlfbin@1.0.0",
4325+
&pkg,
4326+
&files,
4327+
&sources,
4328+
false,
4329+
MismatchPolicy::Strict,
4330+
)
4331+
};
4332+
let res = apply().await;
4333+
assert!(res.success, "{res:?}");
4334+
assert_eq!(res.files_patched, vec![key.to_string()]);
4335+
assert_eq!(
4336+
tokio::fs::read(&cli).await.unwrap(),
4337+
CRLF_BIN_AFTER,
4338+
"{key}"
4339+
);
4340+
assert!(
4341+
!needs_npm_bin_rewrite("pkg:npm/crlfbin@1.0.0", &pkg, key, &files[key]).await,
4342+
"{key}"
4343+
);
4344+
let again = apply().await;
4345+
assert!(again.files_patched.is_empty(), "{again:?}");
4346+
assert_eq!(
4347+
tokio::fs::read(&cli).await.unwrap(),
4348+
CRLF_BIN_AFTER,
4349+
"{key}"
4350+
);
4351+
4352+
let rolled = crate::patch::rollback::rollback_package_patch(
4353+
"pkg:npm/crlfbin@1.0.0",
4354+
&pkg,
4355+
&files,
4356+
blobs.path(),
4357+
false,
4358+
)
4359+
.await;
4360+
assert!(rolled.success, "{rolled:?}");
4361+
assert_eq!(
4362+
tokio::fs::read(&cli).await.unwrap(),
4363+
CRLF_BIN_BEFORE,
4364+
"rollback is byte-exact: {key}"
4365+
);
4366+
}
4367+
}
4368+
42764369
/// A non-npm package keeps the patch's exact bytes on a consent
42774370
/// overwrite: only npm rewrites bin shebangs.
42784371
#[tokio::test]

0 commit comments

Comments
 (0)