From c48fa5868c9f24ad9992d45cbccc4c9949d4be72 Mon Sep 17 00:00:00 2001
From: socket-patch-agent <noreply@anthropic.com>
Date: Thu, 29 Oct 2026 01:22:47 +0000
Subject: [PATCH 1/5] Start fix for #1804

Assisted-by: Claude Code:claude-opus-5-5

From 9e8951c8d02e1075a252976ab9ffa645b3d399ea Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Thu, 29 Oct 2026 01:36:24 +0000
Subject: [PATCH 2/5] 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
---
 .../tests/e2e_redirect_npm_build.rs           | 111 +++++--
 crates/socket-patch-core/src/patch/apply.rs   | 105 ++++++-
 crates/socket-patch-core/src/patch/npm_bin.rs | 294 +++++++++++++++---
 .../socket-patch-core/src/patch/rollback.rs   |  36 ++-
 4 files changed, 462 insertions(+), 84 deletions(-)

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 57feb67cd..9529ae9f5 100644
--- a/crates/socket-patch-cli/tests/e2e_redirect_npm_build.rs
+++ b/crates/socket-patch-cli/tests/e2e_redirect_npm_build.rs
@@ -1174,15 +1174,27 @@ const NPM_BIN_AFTER: &[u8] = b"#!/usr/bin/env node\nconsole.log(\"hi\");\r\n// S
 /// `bin`) npm-installed, and an agent manifest patching that bin with
 /// its blobs. Returns the project dir.
 fn crlf_bin_project(tmp: &Path) -> PathBuf {
+    let proj = crlf_bin_project_with(
+        tmp,
+        r#"{"name":"crlfbin","version":"1.0.0","bin":{"crlfbin":"cli.js"}}"#,
+        "cli.js",
+        &[],
+    );
+    let cli = proj.join("node_modules/crlfbin/cli.js");
+    assert_eq!(std::fs::read(&cli).unwrap(), NPM_BIN_BEFORE, "npm's fixBin");
+    proj
+}
+
+/// [`crlf_bin_project`] with the package's `package.json` `manifest`, the
+/// CRLF-shebang file at `rel` in the package, and `flags` added to
+/// `npm install`.
+fn crlf_bin_project_with(tmp: &Path, manifest: &str, rel: &str, flags: &[&str]) -> PathBuf {
     let (before, after) = (CRLF_BIN_BEFORE, CRLF_BIN_AFTER);
     let pkg = tmp.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();
+    let file = pkg.join(rel);
+    std::fs::create_dir_all(file.parent().unwrap()).unwrap();
+    std::fs::write(pkg.join("package.json"), manifest).unwrap();
+    std::fs::write(&file, before).unwrap();
     std::fs::write(
         tmp.join("crlfbin-1.0.0.tgz"),
         npm_e2e_common::npm_pack_like(&pkg),
@@ -1196,23 +1208,20 @@ fn crlf_bin_project(tmp: &Path) -> PathBuf {
     )
     .unwrap();
     let cache = tmp.join("cache");
-    let out = npm_e2e_common::npm(
-        &proj,
-        &[
-            "install",
-            "--no-audit",
-            "--no-fund",
-            "--cache",
-            cache.to_str().unwrap(),
-        ],
-    );
+    let mut args = vec![
+        "install",
+        "--no-audit",
+        "--no-fund",
+        "--cache",
+        cache.to_str().unwrap(),
+    ];
+    args.extend_from_slice(flags);
+    let out = npm_e2e_common::npm(&proj, &args);
     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(), NPM_BIN_BEFORE, "npm's fixBin");
 
     let (b, a) = (git_sha256(before), git_sha256(after));
     let blobs = proj.join(".socket/blobs");
@@ -1223,7 +1232,7 @@ fn crlf_bin_project(tmp: &Path) -> PathBuf {
         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}"}}}},
+"exportedAt":"2026-01-01T00:00:00Z","files":{{"package/{rel}":{{"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"}}}}}}"#
         ),
@@ -1345,3 +1354,65 @@ async fn npm_agent_recovers_crlf_bin_left_by_raw_write() {
     assert_eq!(std::fs::read(&cli).unwrap(), NPM_BIN_BEFORE);
     assert_crlf_bin_runs(&proj, "after rollback");
 }
+
+/// Regression (#1804): npm rewrites the shebang only of bins it links.
+/// With `--no-bin-links`, or for a `directories.bin`-only package
+/// installed from a `file:` tarball (npm doesn't expand that field from a
+/// tarball's `package.json`), the installed bin keeps its CRLF shebang.
+/// `apply` writes the patch's recorded bytes, `apply --check` agrees, and
+/// `rollback` restores what `npm install` wrote, byte for byte.
+#[cfg(unix)]
+#[tokio::test(flavor = "multi_thread")]
+#[ignore = "wall-bound real-npm install; runs on the e2e CI matrix leg"]
+async fn npm_agent_keeps_crlf_shebang_of_bin_npm_did_not_link() {
+    const SUITE: &str = "npm_agent_keeps_crlf_shebang_of_bin_npm_did_not_link";
+    if npm_e2e_common::npm_version().is_none() {
+        npm_e2e_common::skip(SUITE, "npm not on PATH");
+        return;
+    }
+    for (manifest, rel, flags) in [
+        (
+            r#"{"name":"crlfbin","version":"1.0.0","bin":{"crlfbin":"cli.js"}}"#,
+            "cli.js",
+            &["--no-bin-links"][..],
+        ),
+        (
+            r#"{"name":"crlfbin","version":"1.0.0","directories":{"bin":"bin"}}"#,
+            "bin/cli.js",
+            &[][..],
+        ),
+    ] {
+        let tmp = tempfile::tempdir().unwrap();
+        let proj = crlf_bin_project_with(tmp.path(), manifest, rel, flags);
+        let cli = proj.join("node_modules/crlfbin").join(rel);
+        assert_eq!(
+            std::fs::read(&cli).unwrap(),
+            CRLF_BIN_BEFORE,
+            "{rel}: npm left the shebang alone"
+        );
+
+        let (code, stdout, stderr) = run_socket(&proj, &["apply", "--offline"]);
+        assert_eq!(
+            code, 0,
+            "{rel}: apply\nstdout:\n{stdout}\nstderr:\n{stderr}"
+        );
+        assert_eq!(std::fs::read(&cli).unwrap(), CRLF_BIN_AFTER, "{rel}: apply");
+
+        let (code, stdout, stderr) = run_socket(&proj, &["apply", "--check", "--offline"]);
+        assert_eq!(
+            code, 0,
+            "{rel}: check\nstdout:\n{stdout}\nstderr:\n{stderr}"
+        );
+
+        let (code, stdout, stderr) = run_socket(&proj, &["rollback", "--offline"]);
+        assert_eq!(
+            code, 0,
+            "{rel}: rollback\nstdout:\n{stdout}\nstderr:\n{stderr}"
+        );
+        assert_eq!(
+            std::fs::read(&cli).unwrap(),
+            CRLF_BIN_BEFORE,
+            "{rel}: rollback is byte-exact"
+        );
+    }
+}
diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs
index 999651de1..7febda41f 100644
--- a/crates/socket-patch-core/src/patch/apply.rs
+++ b/crates/socket-patch-core/src/patch/apply.rs
@@ -4180,21 +4180,26 @@ mod tests {
     async fn apply_repairs_crlf_bin_left_by_raw_write() {
         let files = crlf_bin_files();
         let blobs = crlf_bin_blobs().await;
-        let pkg = tempfile::tempdir().unwrap();
+        let root = tempfile::tempdir().unwrap();
+        // npm linked the bin, so it ran `fixBin` on it at install.
+        let pkg = root.path().join("node_modules/crlfbin");
+        std::fs::create_dir_all(root.path().join("node_modules/.bin")).unwrap();
+        std::fs::write(root.path().join("node_modules/.bin/crlfbin"), b"shim").unwrap();
+        std::fs::create_dir_all(&pkg).unwrap();
         tokio::fs::write(
-            pkg.path().join("package.json"),
+            pkg.join("package.json"),
             br#"{"name":"crlfbin","version":"1.0.0","bin":{"crlfbin":"./cli.js"}}"#,
         )
         .await
         .unwrap();
-        tokio::fs::write(pkg.path().join("cli.js"), CRLF_BIN_AFTER)
+        tokio::fs::write(pkg.join("cli.js"), CRLF_BIN_AFTER)
             .await
             .unwrap();
         let sources = PatchSources::blobs_only(blobs.path());
         let apply = || {
             apply_package_patch(
                 "pkg:npm/crlfbin@1.0.0",
-                pkg.path(),
+                &pkg,
                 &files,
                 &sources,
                 false,
@@ -4205,7 +4210,7 @@ mod tests {
         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(),
+            tokio::fs::read(pkg.join("cli.js")).await.unwrap(),
             NPM_BIN_AFTER
         );
         let again = apply().await;
@@ -4214,7 +4219,7 @@ mod tests {
         assert!(
             !needs_npm_bin_rewrite(
                 "pkg:npm/crlfbin@1.0.0",
-                pkg.path(),
+                &pkg,
                 "package/cli.js",
                 &files["package/cli.js"]
             )
@@ -4273,6 +4278,94 @@ mod tests {
         }
     }
 
+    /// Regression (#1804): npm rewrites the shebang only of bins it
+    /// links. With `bin-links=false`, or for a `directories.bin` file of a
+    /// `file:` tarball install (npm doesn't expand that field from a
+    /// tarball), the installed bin keeps its CRLF shebang, so apply writes
+    /// the recorded bytes and leaves them alone, and rollback restores the
+    /// installed bytes exactly.
+    #[tokio::test]
+    async fn apply_writes_recorded_bytes_for_bin_npm_did_not_link() {
+        for (manifest, rel, linked) in [
+            // `npm install --no-bin-links`: no `.bin` at all.
+            (
+                &br#"{"name":"crlfbin","version":"1.0.0","bin":{"crlfbin":"cli.js"}}"#[..],
+                "cli.js",
+                None,
+            ),
+            // `directories.bin` from a tarball: `.bin` holds other bins only.
+            (
+                br#"{"name":"crlfbin","version":"1.0.0","directories":{"bin":"bin"}}"#,
+                "bin/cli.js",
+                Some("other"),
+            ),
+        ] {
+            let key = format!("package/{rel}");
+            let key = key.as_str();
+            let mut files = crlf_bin_files();
+            let info = files.remove("package/cli.js").unwrap();
+            files.insert(key.to_string(), info);
+            let blobs = crlf_bin_blobs().await;
+            let root = tempfile::tempdir().unwrap();
+            let pkg = root.path().join("node_modules/crlfbin");
+            std::fs::create_dir_all(pkg.join("bin")).unwrap();
+            if let Some(name) = linked {
+                std::fs::create_dir_all(root.path().join("node_modules/.bin")).unwrap();
+                std::fs::write(root.path().join("node_modules/.bin").join(name), b"shim").unwrap();
+            }
+            tokio::fs::write(pkg.join("package.json"), manifest)
+                .await
+                .unwrap();
+            let cli = pkg.join(rel);
+            tokio::fs::write(&cli, CRLF_BIN_BEFORE).await.unwrap();
+            let sources = PatchSources::blobs_only(blobs.path());
+            let apply = || {
+                apply_package_patch(
+                    "pkg:npm/crlfbin@1.0.0",
+                    &pkg,
+                    &files,
+                    &sources,
+                    false,
+                    MismatchPolicy::Strict,
+                )
+            };
+            let res = apply().await;
+            assert!(res.success, "{res:?}");
+            assert_eq!(res.files_patched, vec![key.to_string()]);
+            assert_eq!(
+                tokio::fs::read(&cli).await.unwrap(),
+                CRLF_BIN_AFTER,
+                "{key}"
+            );
+            assert!(
+                !needs_npm_bin_rewrite("pkg:npm/crlfbin@1.0.0", &pkg, key, &files[key]).await,
+                "{key}"
+            );
+            let again = apply().await;
+            assert!(again.files_patched.is_empty(), "{again:?}");
+            assert_eq!(
+                tokio::fs::read(&cli).await.unwrap(),
+                CRLF_BIN_AFTER,
+                "{key}"
+            );
+
+            let rolled = crate::patch::rollback::rollback_package_patch(
+                "pkg:npm/crlfbin@1.0.0",
+                &pkg,
+                &files,
+                blobs.path(),
+                false,
+            )
+            .await;
+            assert!(rolled.success, "{rolled:?}");
+            assert_eq!(
+                tokio::fs::read(&cli).await.unwrap(),
+                CRLF_BIN_BEFORE,
+                "rollback is byte-exact: {key}"
+            );
+        }
+    }
+
     /// 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 f245b5458..01a2f6ff4 100644
--- a/crates/socket-patch-core/src/patch/npm_bin.rs
+++ b/crates/socket-patch-core/src/patch/npm_bin.rs
@@ -67,9 +67,12 @@ pub(crate) fn fixed_for_write(
 /// would leave a bin `env` can't run.
 ///
 /// The file is taken for an npm-installed bin when its npm package lists
-/// it in `bin` ([`is_listed_bin`]), whatever shebang is on disk now (a
-/// release before #1756 wrote the CRLF one, #1791). Without that
-/// evidence, when the file now on disk has an LF shebang and either
+/// it in `bin` and npm linked it ([`is_linked_bin`]), whatever shebang is
+/// on disk now (a release before #1756 wrote the CRLF one, #1791). A bin
+/// npm didn't link (`bin-links=false`, or a `directories.bin` file of a
+/// non-registry install) keeps its published shebang, so it is written as
+/// recorded (#1804). Without that evidence, when the file now on disk 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>(
@@ -86,7 +89,7 @@ pub(crate) async fn form_for_write<'a>(
         return as_recorded(content);
     }
     let path = pkg_path.join(normalized);
-    let npm_installed = is_listed_bin(package_key, pkg_path, normalized).await || {
+    let npm_installed = is_linked_bin(package_key, pkg_path, normalized).await || {
         let lf_shebang = read_prefix(&path)
             .await
             .is_some_and(|prefix| has_lf_shebang(&prefix));
@@ -109,9 +112,18 @@ fn is_npm(package_key: &str) -> bool {
 }
 
 /// Whether `normalized` (a safe path relative to `pkg_path`) is a file
-/// the npm package `package_key` lists as a bin in its `package.json`:
-/// a file npm's `bin-links` runs `fixBin` on.
-pub(crate) async fn is_listed_bin(package_key: &str, pkg_path: &Path, normalized: &str) -> bool {
+/// the npm package `package_key` lists as a bin in its `package.json` and
+/// npm linked: a file npm's `bin-links` ran `fixBin` on.
+///
+/// npm rewrites only the bins it links, so the listing alone is not
+/// enough: with `bin-links=false`, or for a `directories.bin` file of a
+/// package installed from a tarball or folder (npm expands that field
+/// only in registry packuments), the file keeps its published shebang
+/// (#1804). The link is a `.bin/<name>` entry in a `node_modules` the
+/// package sits under (a symlink must resolve to this file; a cmd or
+/// shell shim is taken as is), a pnpm virtual-store dependent's
+/// `.bin/<name>`, or a global prefix's `bin/<name>` or `<name>.cmd`.
+pub(crate) async fn is_linked_bin(package_key: &str, pkg_path: &Path, normalized: &str) -> bool {
     if !is_npm(package_key) {
         return false;
     }
@@ -120,7 +132,66 @@ pub(crate) async fn is_listed_bin(package_key: &str, pkg_path: &Path, normalized
     else {
         return false;
     };
-    lists_bin(&manifest, normalized)
+    let names = bin_names(&manifest, normalized);
+    if names.is_empty() {
+        return false;
+    }
+    let file = pkg_path.join(normalized);
+    let pkg_path = pkg_path.to_path_buf();
+    crate::utils::fs::run_blocking(move || has_bin_link(&pkg_path, &file, &names)).await
+}
+
+/// Whether an entry named one of `names` links the bin `file` of the
+/// package at `pkg_path`, in the places [`is_linked_bin`] lists.
+fn has_bin_link(pkg_path: &Path, file: &Path, names: &[String]) -> bool {
+    let target = std::fs::canonicalize(file).ok();
+    let links = |entry: &Path| match std::fs::symlink_metadata(entry) {
+        Ok(meta) if meta.file_type().is_symlink() => {
+            target.is_some() && std::fs::canonicalize(entry).ok() == target
+        }
+        Ok(meta) => meta.is_file(),
+        Err(_) => false,
+    };
+    let in_dir = |dir: &Path, suffix: &str| {
+        names
+            .iter()
+            .any(|name| links(&dir.join(format!("{name}{suffix}"))))
+    };
+    for dir in pkg_path.ancestors().skip(1) {
+        match dir.file_name().and_then(|n| n.to_str()) {
+            Some("node_modules") => {
+                if in_dir(&dir.join(".bin"), "") {
+                    return true;
+                }
+                let Some(prefix) = dir.parent() else { continue };
+                // Windows global: `<prefix>/node_modules`, shims in `<prefix>`.
+                if in_dir(prefix, ".cmd") {
+                    return true;
+                }
+                // Unix global: `<prefix>/lib/node_modules`, links in `<prefix>/bin`.
+                if prefix.file_name().is_some_and(|n| n == "lib")
+                    && prefix.parent().is_some_and(|p| in_dir(&p.join("bin"), ""))
+                {
+                    return true;
+                }
+            }
+            // pnpm links a package's bins into each dependent's
+            // `.pnpm/<dependent>/node_modules/.bin`.
+            Some(".pnpm") => {
+                let Ok(entries) = std::fs::read_dir(dir) else {
+                    continue;
+                };
+                if entries
+                    .flatten()
+                    .any(|e| in_dir(&e.path().join("node_modules").join(".bin"), ""))
+                {
+                    return true;
+                }
+            }
+            _ => {}
+        }
+    }
+    false
 }
 
 /// Whether the file at `normalized` holds exactly the recorded content
@@ -144,7 +215,7 @@ pub(crate) async fn needs_fix(
     }) {
         return false;
     }
-    if !is_listed_bin(package_key, pkg_path, normalized).await {
+    if !is_linked_bin(package_key, pkg_path, normalized).await {
         return false;
     }
     crate::utils::fs::read_regular_to_bytes(&path)
@@ -154,49 +225,83 @@ pub(crate) async fn needs_fix(
         })
 }
 
-/// Whether the `package.json` bytes `manifest` list `normalized` as a
-/// bin, as npm's `normalize-package-bin` and `directories.bin` step read
-/// them: a `bin` path (a string, when the package has a `name`, an
-/// object's values, or an array's entries) or, when `bin` is missing or
-/// empty, a non-hidden file under `directories.bin`. Paths are resolved
-/// as npm does (`./` and `..` folded, never above the package).
-fn lists_bin(manifest: &[u8], normalized: &str) -> bool {
+/// The names npm links `normalized` under when the `package.json` bytes
+/// `manifest` list it as a bin, as npm's `normalize-package-bin` and
+/// `directories.bin` step read them: a `bin` path (a string, linked under
+/// the unscoped package `name`, an object's values under their keys, or
+/// an array's entries under their file names) or, when `bin` is missing
+/// or empty, a non-hidden file under `directories.bin`, under its file
+/// name. Paths are resolved as npm does (`./` and `..` folded, never
+/// above the package). Empty when the file is not a listed bin.
+fn bin_names(manifest: &[u8], normalized: &str) -> Vec<String> {
     use serde_json::Value;
     let Ok(json) = serde_json::from_slice::<Value>(manifest) else {
-        return false;
+        return Vec::new();
     };
     let target = clean_path(normalized);
-    if target.is_empty() {
-        return false;
-    }
-    let lists = |bins: &mut dyn Iterator<Item = &Value>| {
-        bins.filter_map(Value::as_str)
-            .any(|bin| clean_path(bin) == target)
+    let Some(file_name) = target.last().map(|n| n.to_string()) else {
+        return Vec::new();
+    };
+    // npm keeps only a key's last segment, `\` and `:` taken for `/`.
+    let link_name = |key: &str| {
+        key.rsplit(['/', '\\', ':'])
+            .next()
+            .filter(|n| !n.is_empty() && *n != "." && *n != "..")
+            .map(str::to_string)
     };
     match json.get("bin") {
-        Some(Value::String(bin)) if !bin.is_empty() => {
-            json.get("name")
-                .and_then(Value::as_str)
-                .is_some_and(|n| !n.is_empty())
-                && clean_path(bin) == target
+        Some(Value::String(bin)) if !bin.is_empty() => json
+            .get("name")
+            .and_then(Value::as_str)
+            .filter(|_| clean_path(bin) == target)
+            .and_then(link_name)
+            .into_iter()
+            .collect(),
+        Some(Value::Object(bins)) => bins
+            .iter()
+            .filter(|(_, bin)| bin.as_str().is_some_and(|bin| clean_path(bin) == target))
+            .filter_map(|(key, _)| link_name(key))
+            .collect(),
+        Some(Value::Array(bins)) => {
+            if bins
+                .iter()
+                .filter_map(Value::as_str)
+                .any(|bin| clean_path(bin) == target)
+            {
+                vec![file_name]
+            } else {
+                Vec::new()
+            }
+        }
+        Some(Value::String(_) | Value::Null | Value::Bool(false)) | None => {
+            let in_dir = json
+                .get("directories")
+                .and_then(|d| d.get("bin"))
+                .and_then(serde_json::Value::as_str)
+                .map(clean_path)
+                .is_some_and(|dir| {
+                    !dir.is_empty()
+                        && target.len() > dir.len()
+                        && target.starts_with(&dir[..])
+                        && !target[dir.len()..].iter().any(|seg| seg.starts_with('.'))
+                });
+            if in_dir {
+                vec![file_name]
+            } else {
+                Vec::new()
+            }
         }
-        Some(Value::Object(bins)) => lists(&mut bins.values()),
-        Some(Value::Array(bins)) => lists(&mut bins.iter()),
-        Some(Value::String(_) | Value::Null | Value::Bool(false)) | None => json
-            .get("directories")
-            .and_then(|d| d.get("bin"))
-            .and_then(serde_json::Value::as_str)
-            .map(clean_path)
-            .is_some_and(|dir| {
-                !dir.is_empty()
-                    && target.len() > dir.len()
-                    && target.starts_with(&dir[..])
-                    && !target[dir.len()..].iter().any(|seg| seg.starts_with('.'))
-            }),
-        Some(_) => false,
+        Some(_) => Vec::new(),
     }
 }
 
+/// Whether the `package.json` bytes `manifest` list `normalized` as a bin
+/// ([`bin_names`]).
+#[cfg(test)]
+fn lists_bin(manifest: &[u8], normalized: &str) -> bool {
+    !bin_names(manifest, normalized).is_empty()
+}
+
 /// `path`'s segments with `.`, empty segments and `..` folded, rooted at
 /// the package as npm's `path.join('/', …)` roots it.
 fn clean_path(path: &str) -> Vec<&str> {
@@ -403,6 +508,111 @@ mod tests {
         assert!(!listed("not json", "cli.js"));
     }
 
+    #[test]
+    fn bin_names_are_npms_link_names() {
+        let names = |manifest: &str, file: &str| bin_names(manifest.as_bytes(), file);
+        assert_eq!(names(r#"{"name":"@s/x","bin":"cli.js"}"#, "cli.js"), ["x"]);
+        assert_eq!(
+            names(
+                r#"{"bin":{"a":"cli.js","b":"other.js","c":"./cli.js"}}"#,
+                "cli.js"
+            ),
+            ["a", "c"]
+        );
+        assert_eq!(names(r#"{"bin":{"../../a":"cli.js"}}"#, "cli.js"), ["a"]);
+        assert_eq!(names(r#"{"bin":["bin/cli.js"]}"#, "bin/cli.js"), ["cli.js"]);
+        assert_eq!(
+            names(r#"{"directories":{"bin":"bin"}}"#, "bin/sub/run"),
+            ["run"]
+        );
+        assert!(names(r#"{"bin":{"a":"x.js"}}"#, "cli.js").is_empty());
+    }
+
+    /// `node_modules/<name>/cli.js` under `root`, its package.json listing
+    /// it as the bin `crlfbin`. Returns the package dir.
+    fn installed_bin(root: &Path, under: &str) -> std::path::PathBuf {
+        let pkg = root.join(under).join("crlfbin");
+        std::fs::create_dir_all(&pkg).unwrap();
+        std::fs::write(
+            pkg.join("package.json"),
+            br#"{"name":"crlfbin","bin":{"crlfbin":"cli.js"}}"#,
+        )
+        .unwrap();
+        std::fs::write(pkg.join("cli.js"), b"#!/usr/bin/env node\r\n").unwrap();
+        pkg
+    }
+
+    fn shim(path: std::path::PathBuf) {
+        std::fs::create_dir_all(path.parent().unwrap()).unwrap();
+        std::fs::write(path, b"#!/bin/sh\n").unwrap();
+    }
+
+    const PURL: &str = "pkg:npm/crlfbin@1.0.0";
+
+    /// Regression (#1804): npm runs `fixBin` only on bins it links, so a
+    /// listed bin without a link (`bin-links=false`) is not npm's form.
+    #[tokio::test]
+    async fn is_linked_bin_wants_npms_link() {
+        let root = tempfile::tempdir().unwrap();
+        let pkg = installed_bin(root.path(), "node_modules");
+        assert!(
+            !is_linked_bin(PURL, &pkg, "cli.js").await,
+            "bin-links=false"
+        );
+        shim(root.path().join("node_modules/.bin/other"));
+        assert!(!is_linked_bin(PURL, &pkg, "cli.js").await, "another bin");
+        shim(root.path().join("node_modules/.bin/crlfbin"));
+        assert!(is_linked_bin(PURL, &pkg, "cli.js").await);
+        assert!(!is_linked_bin("pkg:pypi/crlfbin@1.0.0", &pkg, "cli.js").await);
+    }
+
+    #[cfg(unix)]
+    #[tokio::test]
+    async fn is_linked_bin_follows_a_symlink_to_this_file() {
+        let root = tempfile::tempdir().unwrap();
+        let pkg = installed_bin(root.path(), "node_modules");
+        let bin = root.path().join("node_modules/.bin");
+        std::fs::create_dir_all(&bin).unwrap();
+        std::fs::write(pkg.join("other.js"), b"x").unwrap();
+        std::os::unix::fs::symlink("../crlfbin/other.js", bin.join("crlfbin")).unwrap();
+        assert!(
+            !is_linked_bin(PURL, &pkg, "cli.js").await,
+            "links elsewhere"
+        );
+        std::fs::remove_file(bin.join("crlfbin")).unwrap();
+        std::os::unix::fs::symlink("../crlfbin/cli.js", bin.join("crlfbin")).unwrap();
+        assert!(is_linked_bin(PURL, &pkg, "cli.js").await);
+    }
+
+    #[tokio::test]
+    async fn is_linked_bin_finds_global_and_pnpm_links() {
+        // Unix global prefix.
+        let root = tempfile::tempdir().unwrap();
+        let pkg = installed_bin(root.path(), "lib/node_modules");
+        assert!(!is_linked_bin(PURL, &pkg, "cli.js").await);
+        shim(root.path().join("bin/crlfbin"));
+        assert!(is_linked_bin(PURL, &pkg, "cli.js").await);
+        // Windows global prefix.
+        let root = tempfile::tempdir().unwrap();
+        let pkg = installed_bin(root.path(), "node_modules");
+        shim(root.path().join("crlfbin.cmd"));
+        assert!(is_linked_bin(PURL, &pkg, "cli.js").await);
+        // A pnpm transitive dependency, linked into its dependent's `.bin`.
+        let root = tempfile::tempdir().unwrap();
+        let pkg = installed_bin(root.path(), "node_modules/.pnpm/crlfbin@1.0.0/node_modules");
+        std::fs::create_dir_all(
+            root.path()
+                .join("node_modules/.pnpm/app@1.0.0/node_modules"),
+        )
+        .unwrap();
+        assert!(!is_linked_bin(PURL, &pkg, "cli.js").await);
+        shim(
+            root.path()
+                .join("node_modules/.pnpm/app@1.0.0/node_modules/.bin/crlfbin"),
+        );
+        assert!(is_linked_bin(PURL, &pkg, "cli.js").await);
+    }
+
     #[tokio::test]
     async fn source_hash_is_the_tarball_hash() {
         let dir = tempfile::tempdir().unwrap();
diff --git a/crates/socket-patch-core/src/patch/rollback.rs b/crates/socket-patch-core/src/patch/rollback.rs
index 9ee5a73fe..653c341ff 100644
--- a/crates/socket-patch-core/src/patch/rollback.rs
+++ b/crates/socket-patch-core/src/patch/rollback.rs
@@ -3244,21 +3244,24 @@ mod tests {
     async fn rollback_of_raw_crlf_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\r\nconsole.log(2);\r\n";
-        let pkg = tempfile::tempdir().unwrap();
+        let root = tempfile::tempdir().unwrap();
+        // npm linked the bin, so it ran `fixBin` on it at install.
+        let pkg = root.path().join("node_modules/x");
+        std::fs::create_dir_all(root.path().join("node_modules/.bin")).unwrap();
+        std::fs::write(root.path().join("node_modules/.bin/x"), b"shim").unwrap();
+        std::fs::create_dir_all(&pkg).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("package.json"),
+            pkg.join("package.json"),
             br#"{"name":"x","version":"1.0.0","bin":"cli.js"}"#,
         )
         .await
         .unwrap();
-        tokio::fs::write(pkg.path().join("cli.js"), after)
-            .await
-            .unwrap();
+        tokio::fs::write(pkg.join("cli.js"), after).await.unwrap();
         let files = HashMap::from([(
             "package/cli.js".to_string(),
             PatchFileInfo {
@@ -3267,11 +3270,10 @@ mod tests {
             },
         )]);
         let res =
-            rollback_package_patch("pkg:npm/x@1.0.0", pkg.path(), &files, blobs.path(), false)
-                .await;
+            rollback_package_patch("pkg:npm/x@1.0.0", &pkg, &files, blobs.path(), false).await;
         assert!(res.success, "{res:?}");
         assert_eq!(
-            tokio::fs::read(pkg.path().join("cli.js")).await.unwrap(),
+            tokio::fs::read(pkg.join("cli.js")).await.unwrap(),
             b"#!/usr/bin/env node\nconsole.log(1);\r\n"
         );
     }
@@ -3292,7 +3294,12 @@ mod tests {
             },
         )]);
         for with_blob in [true, false] {
-            let pkg = tempfile::tempdir().unwrap();
+            let root = tempfile::tempdir().unwrap();
+            // npm linked the bin, so it ran `fixBin` on it at install.
+            let pkg = root.path().join("node_modules/x");
+            std::fs::create_dir_all(root.path().join("node_modules/.bin")).unwrap();
+            std::fs::write(root.path().join("node_modules/.bin/x"), b"shim").unwrap();
+            std::fs::create_dir_all(&pkg).unwrap();
             let blobs = tempfile::tempdir().unwrap();
             if with_blob {
                 tokio::fs::write(blobs.path().join(&before_hash), before)
@@ -3300,17 +3307,14 @@ mod tests {
                     .unwrap();
             }
             tokio::fs::write(
-                pkg.path().join("package.json"),
+                pkg.join("package.json"),
                 br#"{"name":"x","version":"1.0.0","bin":"cli.js"}"#,
             )
             .await
             .unwrap();
-            tokio::fs::write(pkg.path().join("cli.js"), before)
-                .await
-                .unwrap();
+            tokio::fs::write(pkg.join("cli.js"), before).await.unwrap();
             let res =
-                rollback_package_patch("pkg:npm/x@1.0.0", pkg.path(), &files, blobs.path(), false)
-                    .await;
+                rollback_package_patch("pkg:npm/x@1.0.0", &pkg, &files, blobs.path(), false).await;
             assert!(res.success, "{with_blob}: {res:?}");
             let expected: &[u8] = if with_blob {
                 b"#!/usr/bin/env node\nconsole.log(1);\r\n"
@@ -3318,7 +3322,7 @@ mod tests {
                 before
             };
             assert_eq!(
-                tokio::fs::read(pkg.path().join("cli.js")).await.unwrap(),
+                tokio::fs::read(pkg.join("cli.js")).await.unwrap(),
                 expected,
                 "{with_blob}"
             );

From 871630f87bd65824c72e15762ea0ab6e5a95f2ed Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Thu, 29 Oct 2026 02:35:31 +0000
Subject: [PATCH 3/5] Look for npm's bin link only where npm puts it

npm links a package's bins into the .bin folder of the node_modules the
package sits in, so a nested copy is no longer taken as linked because
the top-level copy of the same package has a .bin entry. Packages in a
virtual store (pnpm, npm's linked strategy, vlt, Bun) get their bins
linked into each dependent instead, so there the bin listing alone
still counts, as it did before.

Also check the on-disk shebang before the link, which is cheaper, and
read a bin key with a trailing slash the way npm does.

Refs #1804

Assisted-by: Claude Code:claude-opus-5-5
---
 crates/socket-patch-core/src/patch/apply.rs   |  24 +--
 crates/socket-patch-core/src/patch/npm_bin.rs | 147 +++++++++---------
 2 files changed, 87 insertions(+), 84 deletions(-)

diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs
index 7febda41f..5dd071665 100644
--- a/crates/socket-patch-core/src/patch/apply.rs
+++ b/crates/socket-patch-core/src/patch/apply.rs
@@ -4244,25 +4244,25 @@ mod tests {
                 br#"{"name":"crlfbin","bin":"cli.js"}"#,
             ),
         ] {
-            let pkg = tempfile::tempdir().unwrap();
-            tokio::fs::write(pkg.path().join("package.json"), manifest)
+            // npm linked a `crlfbin` bin, but not to this file.
+            let root = tempfile::tempdir().unwrap();
+            let pkg = root.path().join("node_modules/crlfbin");
+            std::fs::create_dir_all(root.path().join("node_modules/.bin")).unwrap();
+            std::fs::write(root.path().join("node_modules/.bin/crlfbin"), b"shim").unwrap();
+            std::fs::create_dir_all(&pkg).unwrap();
+            tokio::fs::write(pkg.join("package.json"), manifest)
                 .await
                 .unwrap();
-            tokio::fs::write(pkg.path().join("cli.js"), CRLF_BIN_AFTER)
+            tokio::fs::write(pkg.join("cli.js"), CRLF_BIN_AFTER)
                 .await
                 .unwrap();
             assert!(
-                !needs_npm_bin_rewrite(
-                    purl,
-                    pkg.path(),
-                    "package/cli.js",
-                    &files["package/cli.js"]
-                )
-                .await
+                !needs_npm_bin_rewrite(purl, &pkg, "package/cli.js", &files["package/cli.js"])
+                    .await
             );
             let res = apply_package_patch(
                 purl,
-                pkg.path(),
+                &pkg,
                 &files,
                 &PatchSources::blobs_only(blobs.path()),
                 false,
@@ -4272,7 +4272,7 @@ mod tests {
             assert!(res.success, "{purl}: {res:?}");
             assert!(res.files_patched.is_empty(), "{purl}: {res:?}");
             assert_eq!(
-                tokio::fs::read(pkg.path().join("cli.js")).await.unwrap(),
+                tokio::fs::read(pkg.join("cli.js")).await.unwrap(),
                 CRLF_BIN_AFTER
             );
         }
diff --git a/crates/socket-patch-core/src/patch/npm_bin.rs b/crates/socket-patch-core/src/patch/npm_bin.rs
index 01a2f6ff4..f50ffb46e 100644
--- a/crates/socket-patch-core/src/patch/npm_bin.rs
+++ b/crates/socket-patch-core/src/patch/npm_bin.rs
@@ -89,15 +89,14 @@ pub(crate) async fn form_for_write<'a>(
         return as_recorded(content);
     }
     let path = pkg_path.join(normalized);
-    let npm_installed = is_linked_bin(package_key, pkg_path, normalized).await || {
-        let lf_shebang = read_prefix(&path)
-            .await
-            .is_some_and(|prefix| has_lf_shebang(&prefix));
-        lf_shebang
-            && (is_npm(package_key)
-                || (!installed_source.is_empty()
-                    && source_hash(&path).await.as_deref() == Some(installed_source)))
-    };
+    let lf_shebang = read_prefix(&path)
+        .await
+        .is_some_and(|prefix| has_lf_shebang(&prefix));
+    let npm_installed = (lf_shebang
+        && (is_npm(package_key)
+            || (!installed_source.is_empty()
+                && source_hash(&path).await.as_deref() == Some(installed_source))))
+        || is_linked_bin(package_key, pkg_path, normalized).await;
     if !npm_installed {
         return as_recorded(content);
     }
@@ -119,10 +118,13 @@ fn is_npm(package_key: &str) -> bool {
 /// enough: with `bin-links=false`, or for a `directories.bin` file of a
 /// package installed from a tarball or folder (npm expands that field
 /// only in registry packuments), the file keeps its published shebang
-/// (#1804). The link is a `.bin/<name>` entry in a `node_modules` the
-/// package sits under (a symlink must resolve to this file; a cmd or
-/// shell shim is taken as is), a pnpm virtual-store dependent's
-/// `.bin/<name>`, or a global prefix's `bin/<name>` or `<name>.cmd`.
+/// (#1804). The link is a `.bin/<name>` entry in the `node_modules` the
+/// package sits in, where npm links it (a symlink must resolve to this
+/// file; a cmd or shell shim is taken as is), or a global prefix's
+/// `bin/<name>` or `<name>.cmd`. A package in a virtual store (pnpm's
+/// `.pnpm`, npm's linked `.store`, vlt, Bun) has its bins linked into
+/// each dependent's `.bin`, wherever that is, so there the listing alone
+/// is taken for the link, as before.
 pub(crate) async fn is_linked_bin(package_key: &str, pkg_path: &Path, normalized: &str) -> bool {
     if !is_npm(package_key) {
         return false;
@@ -157,41 +159,35 @@ fn has_bin_link(pkg_path: &Path, file: &Path, names: &[String]) -> bool {
             .iter()
             .any(|name| links(&dir.join(format!("{name}{suffix}"))))
     };
-    for dir in pkg_path.ancestors().skip(1) {
-        match dir.file_name().and_then(|n| n.to_str()) {
-            Some("node_modules") => {
-                if in_dir(&dir.join(".bin"), "") {
-                    return true;
-                }
-                let Some(prefix) = dir.parent() else { continue };
-                // Windows global: `<prefix>/node_modules`, shims in `<prefix>`.
-                if in_dir(prefix, ".cmd") {
-                    return true;
-                }
-                // Unix global: `<prefix>/lib/node_modules`, links in `<prefix>/bin`.
-                if prefix.file_name().is_some_and(|n| n == "lib")
-                    && prefix.parent().is_some_and(|p| in_dir(&p.join("bin"), ""))
-                {
-                    return true;
-                }
-            }
-            // pnpm links a package's bins into each dependent's
-            // `.pnpm/<dependent>/node_modules/.bin`.
-            Some(".pnpm") => {
-                let Ok(entries) = std::fs::read_dir(dir) else {
-                    continue;
-                };
-                if entries
-                    .flatten()
-                    .any(|e| in_dir(&e.path().join("node_modules").join(".bin"), ""))
-                {
-                    return true;
-                }
-            }
-            _ => {}
-        }
+    let mut ancestors = pkg_path.ancestors().skip(1);
+    let Some(node_modules) = ancestors
+        .by_ref()
+        .find(|dir| dir.file_name().is_some_and(|n| n == "node_modules"))
+    else {
+        return false;
+    };
+    if ancestors.any(|dir| {
+        dir.file_name()
+            .and_then(|n| n.to_str())
+            .is_some_and(|n| n.starts_with('.'))
+            && dir
+                .parent()
+                .and_then(Path::file_name)
+                .is_some_and(|n| n == "node_modules")
+    }) {
+        return true;
+    }
+    if in_dir(&node_modules.join(".bin"), "") {
+        return true;
     }
-    false
+    let Some(prefix) = node_modules.parent() else {
+        return false;
+    };
+    // Windows global: `<prefix>/node_modules`, shims in `<prefix>`.
+    in_dir(prefix, ".cmd")
+        // Unix global: `<prefix>/lib/node_modules`, links in `<prefix>/bin`.
+        || (prefix.file_name().is_some_and(|n| n == "lib")
+            && prefix.parent().is_some_and(|p| in_dir(&p.join("bin"), "")))
 }
 
 /// Whether the file at `normalized` holds exactly the recorded content
@@ -244,7 +240,8 @@ fn bin_names(manifest: &[u8], normalized: &str) -> Vec<String> {
     };
     // npm keeps only a key's last segment, `\` and `:` taken for `/`.
     let link_name = |key: &str| {
-        key.rsplit(['/', '\\', ':'])
+        key.trim_end_matches(['/', '\\', ':'])
+            .rsplit(['/', '\\', ':'])
             .next()
             .filter(|n| !n.is_empty() && *n != "." && *n != "..")
             .map(str::to_string)
@@ -263,18 +260,18 @@ fn bin_names(manifest: &[u8], normalized: &str) -> Vec<String> {
             .filter_map(|(key, _)| link_name(key))
             .collect(),
         Some(Value::Array(bins)) => {
-            if bins
+            let listed = bins
                 .iter()
                 .filter_map(Value::as_str)
-                .any(|bin| clean_path(bin) == target)
-            {
+                .any(|bin| clean_path(bin) == target);
+            if listed {
                 vec![file_name]
             } else {
                 Vec::new()
             }
         }
         Some(Value::String(_) | Value::Null | Value::Bool(false)) | None => {
-            let in_dir = json
+            let listed = json
                 .get("directories")
                 .and_then(|d| d.get("bin"))
                 .and_then(serde_json::Value::as_str)
@@ -285,7 +282,7 @@ fn bin_names(manifest: &[u8], normalized: &str) -> Vec<String> {
                         && target.starts_with(&dir[..])
                         && !target[dir.len()..].iter().any(|seg| seg.starts_with('.'))
                 });
-            if in_dir {
+            if listed {
                 vec![file_name]
             } else {
                 Vec::new()
@@ -295,13 +292,6 @@ fn bin_names(manifest: &[u8], normalized: &str) -> Vec<String> {
     }
 }
 
-/// Whether the `package.json` bytes `manifest` list `normalized` as a bin
-/// ([`bin_names`]).
-#[cfg(test)]
-fn lists_bin(manifest: &[u8], normalized: &str) -> bool {
-    !bin_names(manifest, normalized).is_empty()
-}
-
 /// `path`'s segments with `.`, empty segments and `..` folded, rooted at
 /// the package as npm's `path.join('/', …)` roots it.
 fn clean_path(path: &str) -> Vec<&str> {
@@ -473,7 +463,7 @@ mod tests {
 
     #[test]
     fn lists_bin_reads_every_bin_shape() {
-        let listed = |manifest: &str, file: &str| lists_bin(manifest.as_bytes(), file);
+        let listed = |manifest: &str, file: &str| !bin_names(manifest.as_bytes(), file).is_empty();
         assert!(listed(r#"{"name":"x","bin":"cli.js"}"#, "cli.js"));
         assert!(listed(r#"{"name":"x","bin":"./bin/../cli.js"}"#, "cli.js"));
         assert!(listed(
@@ -520,6 +510,7 @@ mod tests {
             ["a", "c"]
         );
         assert_eq!(names(r#"{"bin":{"../../a":"cli.js"}}"#, "cli.js"), ["a"]);
+        assert_eq!(names(r#"{"bin":{"a/":"cli.js"}}"#, "cli.js"), ["a"]);
         assert_eq!(names(r#"{"bin":["bin/cli.js"]}"#, "bin/cli.js"), ["cli.js"]);
         assert_eq!(
             names(r#"{"directories":{"bin":"bin"}}"#, "bin/sub/run"),
@@ -597,19 +588,31 @@ mod tests {
         let pkg = installed_bin(root.path(), "node_modules");
         shim(root.path().join("crlfbin.cmd"));
         assert!(is_linked_bin(PURL, &pkg, "cli.js").await);
-        // A pnpm transitive dependency, linked into its dependent's `.bin`.
+        // A virtual-store copy, whose bins are linked into its dependents'.
+        for store in [
+            "node_modules/.pnpm/crlfbin@1.0.0/node_modules",
+            "node_modules/.store/crlfbin@1.0.0/node_modules",
+        ] {
+            let root = tempfile::tempdir().unwrap();
+            let pkg = installed_bin(root.path(), store);
+            assert!(is_linked_bin(PURL, &pkg, "cli.js").await, "{store}");
+        }
+    }
+
+    /// npm links a nested copy's bins into its own `node_modules/.bin`, so
+    /// the top-level copy's link says nothing about it.
+    #[tokio::test]
+    async fn is_linked_bin_looks_only_where_npm_links_the_copy() {
         let root = tempfile::tempdir().unwrap();
-        let pkg = installed_bin(root.path(), "node_modules/.pnpm/crlfbin@1.0.0/node_modules");
-        std::fs::create_dir_all(
-            root.path()
-                .join("node_modules/.pnpm/app@1.0.0/node_modules"),
-        )
-        .unwrap();
+        let pkg = installed_bin(root.path(), "node_modules/a/node_modules");
+        shim(root.path().join("node_modules/.bin/crlfbin"));
         assert!(!is_linked_bin(PURL, &pkg, "cli.js").await);
-        shim(
-            root.path()
-                .join("node_modules/.pnpm/app@1.0.0/node_modules/.bin/crlfbin"),
-        );
+        shim(root.path().join("node_modules/a/node_modules/.bin/crlfbin"));
+        assert!(is_linked_bin(PURL, &pkg, "cli.js").await);
+        // Scoped: `node_modules/@s/crlfbin` links into `node_modules/.bin`.
+        let root = tempfile::tempdir().unwrap();
+        let pkg = installed_bin(root.path(), "node_modules/@s");
+        shim(root.path().join("node_modules/.bin/crlfbin"));
         assert!(is_linked_bin(PURL, &pkg, "cli.js").await);
     }
 

From d2387bf318289ebb2bdce01c7a0891097c29953b Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Thu, 29 Oct 2026 03:02:55 +0000
Subject: [PATCH 4/5] Only trust a bin shim that runs this file

A regular file that merely had a bin's name, such as a project's own
<name>.cmd or a lib/../bin script, counted as npm's link, so a bin npm
never linked could still be rewritten with an LF shebang. A shim now
has to name this file's path below node_modules, as npm's and pnpm's
shims do; a symlink still has to resolve to it.

Copies in a store npm doesn't lay out (outside any node_modules, as in
Yarn's pnpmStoreFolder, or in a name@version entry of a relocated pnpm
virtualStoreDir) keep taking the bin listing as the link, as before.

Refs #1804

Assisted-by: Claude Code:claude-opus-5-5
---
 crates/socket-patch-core/src/patch/apply.rs   |  12 +-
 crates/socket-patch-core/src/patch/npm_bin.rs | 147 ++++++++++++++----
 .../socket-patch-core/src/patch/rollback.rs   |  12 +-
 3 files changed, 138 insertions(+), 33 deletions(-)

diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs
index 5dd071665..7901f5043 100644
--- a/crates/socket-patch-core/src/patch/apply.rs
+++ b/crates/socket-patch-core/src/patch/apply.rs
@@ -4184,7 +4184,11 @@ mod tests {
         // npm linked the bin, so it ran `fixBin` on it at install.
         let pkg = root.path().join("node_modules/crlfbin");
         std::fs::create_dir_all(root.path().join("node_modules/.bin")).unwrap();
-        std::fs::write(root.path().join("node_modules/.bin/crlfbin"), b"shim").unwrap();
+        std::fs::write(
+            root.path().join("node_modules/.bin/crlfbin"),
+            b"node \"$basedir/../crlfbin/cli.js\" \"$@\"",
+        )
+        .unwrap();
         std::fs::create_dir_all(&pkg).unwrap();
         tokio::fs::write(
             pkg.join("package.json"),
@@ -4248,7 +4252,11 @@ mod tests {
             let root = tempfile::tempdir().unwrap();
             let pkg = root.path().join("node_modules/crlfbin");
             std::fs::create_dir_all(root.path().join("node_modules/.bin")).unwrap();
-            std::fs::write(root.path().join("node_modules/.bin/crlfbin"), b"shim").unwrap();
+            std::fs::write(
+                root.path().join("node_modules/.bin/crlfbin"),
+                b"node \"$basedir/../crlfbin/cli.js\" \"$@\"",
+            )
+            .unwrap();
             std::fs::create_dir_all(&pkg).unwrap();
             tokio::fs::write(pkg.join("package.json"), manifest)
                 .await
diff --git a/crates/socket-patch-core/src/patch/npm_bin.rs b/crates/socket-patch-core/src/patch/npm_bin.rs
index f50ffb46e..95fd190fd 100644
--- a/crates/socket-patch-core/src/patch/npm_bin.rs
+++ b/crates/socket-patch-core/src/patch/npm_bin.rs
@@ -119,12 +119,13 @@ fn is_npm(package_key: &str) -> bool {
 /// package installed from a tarball or folder (npm expands that field
 /// only in registry packuments), the file keeps its published shebang
 /// (#1804). The link is a `.bin/<name>` entry in the `node_modules` the
-/// package sits in, where npm links it (a symlink must resolve to this
-/// file; a cmd or shell shim is taken as is), or a global prefix's
-/// `bin/<name>` or `<name>.cmd`. A package in a virtual store (pnpm's
-/// `.pnpm`, npm's linked `.store`, vlt, Bun) has its bins linked into
-/// each dependent's `.bin`, wherever that is, so there the listing alone
-/// is taken for the link, as before.
+/// package sits in, where npm links it, or a global prefix's
+/// `bin/<name>` or `<name>.cmd`: a symlink resolving to this file, or a
+/// cmd or shell shim naming it. A package in a virtual store (pnpm's
+/// `.pnpm` or a relocated `virtualStoreDir`, npm's linked `.store`, vlt,
+/// Bun, Yarn's `pnpmStoreFolder`) has its bins linked into each
+/// dependent's `.bin`, wherever that is, so there the listing alone is
+/// taken for the link, as before.
 pub(crate) async fn is_linked_bin(package_key: &str, pkg_path: &Path, normalized: &str) -> bool {
     if !is_npm(package_key) {
         return false;
@@ -146,37 +147,41 @@ pub(crate) async fn is_linked_bin(package_key: &str, pkg_path: &Path, normalized
 /// Whether an entry named one of `names` links the bin `file` of the
 /// package at `pkg_path`, in the places [`is_linked_bin`] lists.
 fn has_bin_link(pkg_path: &Path, file: &Path, names: &[String]) -> bool {
+    let mut ancestors = pkg_path.ancestors().skip(1);
+    // npm installs every package under a `node_modules`; a copy outside
+    // one is in a relocated store.
+    let Some(node_modules) = ancestors
+        .by_ref()
+        .find(|dir| dir.file_name().is_some_and(|n| n == "node_modules"))
+    else {
+        return true;
+    };
+    if in_virtual_store(node_modules) {
+        return true;
+    }
+    // A shim names the file by its path below `node_modules`, from
+    // wherever it is (`..\crlfbin\cli.js`, `node_modules/crlfbin/cli.js`).
+    let Some(below) = file
+        .strip_prefix(node_modules)
+        .ok()
+        .and_then(Path::to_str)
+        .map(|p| p.replace('\\', "/"))
+    else {
+        return false;
+    };
     let target = std::fs::canonicalize(file).ok();
     let links = |entry: &Path| match std::fs::symlink_metadata(entry) {
         Ok(meta) if meta.file_type().is_symlink() => {
             target.is_some() && std::fs::canonicalize(entry).ok() == target
         }
-        Ok(meta) => meta.is_file(),
-        Err(_) => false,
+        Ok(meta) if meta.is_file() => shim_names(entry, &below),
+        _ => false,
     };
     let in_dir = |dir: &Path, suffix: &str| {
         names
             .iter()
             .any(|name| links(&dir.join(format!("{name}{suffix}"))))
     };
-    let mut ancestors = pkg_path.ancestors().skip(1);
-    let Some(node_modules) = ancestors
-        .by_ref()
-        .find(|dir| dir.file_name().is_some_and(|n| n == "node_modules"))
-    else {
-        return false;
-    };
-    if ancestors.any(|dir| {
-        dir.file_name()
-            .and_then(|n| n.to_str())
-            .is_some_and(|n| n.starts_with('.'))
-            && dir
-                .parent()
-                .and_then(Path::file_name)
-                .is_some_and(|n| n == "node_modules")
-    }) {
-        return true;
-    }
     if in_dir(&node_modules.join(".bin"), "") {
         return true;
     }
@@ -190,6 +195,45 @@ fn has_bin_link(pkg_path: &Path, file: &Path, names: &[String]) -> bool {
             && prefix.parent().is_some_and(|p| in_dir(&p.join("bin"), "")))
 }
 
+/// Whether `node_modules` (the one a package sits in) is inside a virtual
+/// store: below a dot folder of a `node_modules` (`.pnpm`, `.store`,
+/// `.vlt`, `.bun`), or in a `<name>@<version>` store entry, which no
+/// project folder is (a relocated pnpm `virtualStoreDir`).
+fn in_virtual_store(node_modules: &Path) -> bool {
+    let entry = node_modules.parent();
+    entry.is_some_and(|entry| {
+        entry
+            .file_name()
+            .and_then(|n| n.to_str())
+            .is_some_and(|n| n.get(1..).is_some_and(|rest| rest.contains('@')))
+            && !entry.join("package.json").exists()
+    }) || node_modules.ancestors().skip(1).any(|dir| {
+        dir.file_name()
+            .and_then(|n| n.to_str())
+            .is_some_and(|n| n.starts_with('.'))
+            && dir
+                .parent()
+                .and_then(Path::file_name)
+                .is_some_and(|n| n == "node_modules")
+    })
+}
+
+/// Whether the shim at `path` (a cmd-shim or shell script) names the
+/// file at `below`, its `/`-separated path below `node_modules`.
+fn shim_names(path: &Path, below: &str) -> bool {
+    use std::io::Read as _;
+    let Ok((file, _)) = crate::utils::fs::open_regular_file_sync(path) else {
+        return false;
+    };
+    let mut text = Vec::new();
+    if file.take(8192).read_to_end(&mut text).is_err() {
+        return false;
+    }
+    String::from_utf8_lossy(&text)
+        .replace('\\', "/")
+        .contains(below)
+}
+
 /// Whether the file at `normalized` holds exactly the recorded content
 /// `expected_hash`, in bytes `fixBin` would change (a CRLF shebang npm
 /// never leaves in a bin), and is a bin of the npm package `package_key`:
@@ -533,9 +577,15 @@ mod tests {
         pkg
     }
 
-    fn shim(path: std::path::PathBuf) {
+    /// A cmd-shim at `path` running `target` (relative to the shim).
+    fn shim_to(path: std::path::PathBuf, target: &str) {
         std::fs::create_dir_all(path.parent().unwrap()).unwrap();
-        std::fs::write(path, b"#!/bin/sh\n").unwrap();
+        let target = target.replace('/', "\\");
+        std::fs::write(path, format!("@\"%dp0%\\{target}\" %*\r\n")).unwrap();
+    }
+
+    fn shim(path: std::path::PathBuf) {
+        shim_to(path, "../crlfbin/cli.js");
     }
 
     const PURL: &str = "pkg:npm/crlfbin@1.0.0";
@@ -612,7 +662,46 @@ mod tests {
         // Scoped: `node_modules/@s/crlfbin` links into `node_modules/.bin`.
         let root = tempfile::tempdir().unwrap();
         let pkg = installed_bin(root.path(), "node_modules/@s");
-        shim(root.path().join("node_modules/.bin/crlfbin"));
+        shim_to(
+            root.path().join("node_modules/.bin/crlfbin"),
+            "../@s/crlfbin/cli.js",
+        );
+        assert!(is_linked_bin(PURL, &pkg, "cli.js").await);
+    }
+
+    /// A file that merely has a bin's name, where a global prefix would
+    /// keep its shim, is not npm's link unless it runs this file.
+    #[tokio::test]
+    async fn is_linked_bin_wants_the_shim_to_run_this_file() {
+        let root = tempfile::tempdir().unwrap();
+        let pkg = installed_bin(root.path(), "node_modules");
+        shim_to(root.path().join("crlfbin.cmd"), "scripts/crlfbin.js");
+        shim_to(
+            root.path().join("node_modules/.bin/crlfbin"),
+            "../other/cli.js",
+        );
+        assert!(!is_linked_bin(PURL, &pkg, "cli.js").await);
+        let root = tempfile::tempdir().unwrap();
+        let pkg = installed_bin(root.path(), "lib/node_modules");
+        shim_to(root.path().join("bin/crlfbin"), "../tools/crlfbin");
+        assert!(!is_linked_bin(PURL, &pkg, "cli.js").await);
+    }
+
+    /// Stores outside a `node_modules` dot folder: a relocated pnpm
+    /// `virtualStoreDir`, and Yarn's `pnpmStoreFolder` (`<slug>/package`).
+    #[tokio::test]
+    async fn is_linked_bin_takes_the_listing_in_relocated_stores() {
+        let root = tempfile::tempdir().unwrap();
+        let pkg = installed_bin(root.path(), "vstore/crlfbin@1.0.0/node_modules");
+        assert!(is_linked_bin(PURL, &pkg, "cli.js").await);
+        let root = tempfile::tempdir().unwrap();
+        let pkg = root.path().join("store/crlfbin-npm-1.0.0-abc/package");
+        std::fs::create_dir_all(&pkg).unwrap();
+        std::fs::write(
+            pkg.join("package.json"),
+            br#"{"name":"crlfbin","bin":{"crlfbin":"cli.js"}}"#,
+        )
+        .unwrap();
         assert!(is_linked_bin(PURL, &pkg, "cli.js").await);
     }
 
diff --git a/crates/socket-patch-core/src/patch/rollback.rs b/crates/socket-patch-core/src/patch/rollback.rs
index 653c341ff..f7cb2928c 100644
--- a/crates/socket-patch-core/src/patch/rollback.rs
+++ b/crates/socket-patch-core/src/patch/rollback.rs
@@ -3248,7 +3248,11 @@ mod tests {
         // npm linked the bin, so it ran `fixBin` on it at install.
         let pkg = root.path().join("node_modules/x");
         std::fs::create_dir_all(root.path().join("node_modules/.bin")).unwrap();
-        std::fs::write(root.path().join("node_modules/.bin/x"), b"shim").unwrap();
+        std::fs::write(
+            root.path().join("node_modules/.bin/x"),
+            b"node \"$basedir/../x/cli.js\" \"$@\"",
+        )
+        .unwrap();
         std::fs::create_dir_all(&pkg).unwrap();
         let blobs = tempfile::tempdir().unwrap();
         let before_hash = compute_git_sha256_from_bytes(before);
@@ -3298,7 +3302,11 @@ mod tests {
             // npm linked the bin, so it ran `fixBin` on it at install.
             let pkg = root.path().join("node_modules/x");
             std::fs::create_dir_all(root.path().join("node_modules/.bin")).unwrap();
-            std::fs::write(root.path().join("node_modules/.bin/x"), b"shim").unwrap();
+            std::fs::write(
+                root.path().join("node_modules/.bin/x"),
+                b"node \"$basedir/../x/cli.js\" \"$@\"",
+            )
+            .unwrap();
             std::fs::create_dir_all(&pkg).unwrap();
             let blobs = tempfile::tempdir().unwrap();
             if with_blob {

From 5789c736a95c7b1c7896b6d13e12bab002fabbef Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Thu, 29 Oct 2026 05:04:22 +0000
Subject: [PATCH 5/5] Match a bin shim's path on whole segments

A shim for another package whose folder name ends the same way, such as
a-crlfbin/cli.js, or a sibling file like cli.json, counted as running
this package's bin. The shim now has to name the path as whole
segments.

Refs #1804

Assisted-by: Claude Code:claude-opus-5-5
---
 crates/socket-patch-core/src/patch/npm_bin.rs | 20 ++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/crates/socket-patch-core/src/patch/npm_bin.rs b/crates/socket-patch-core/src/patch/npm_bin.rs
index 95fd190fd..afdac2868 100644
--- a/crates/socket-patch-core/src/patch/npm_bin.rs
+++ b/crates/socket-patch-core/src/patch/npm_bin.rs
@@ -229,9 +229,13 @@ fn shim_names(path: &Path, below: &str) -> bool {
     if file.take(8192).read_to_end(&mut text).is_err() {
         return false;
     }
-    String::from_utf8_lossy(&text)
-        .replace('\\', "/")
-        .contains(below)
+    let text = String::from_utf8_lossy(&text).replace('\\', "/");
+    // A whole path segment run: `../crlfbin/cli.js`, not
+    // `../a-crlfbin/cli.js` or `../crlfbin/cli.json`.
+    let path_char = |c: char| c.is_alphanumeric() || matches!(c, '.' | '-' | '_' | '/' | '@');
+    text.match_indices(below).any(|(at, _)| {
+        text[..at].ends_with('/') && !text[at + below.len()..].starts_with(path_char)
+    })
 }
 
 /// Whether the file at `normalized` holds exactly the recorded content
@@ -676,6 +680,16 @@ mod tests {
         let root = tempfile::tempdir().unwrap();
         let pkg = installed_bin(root.path(), "node_modules");
         shim_to(root.path().join("crlfbin.cmd"), "scripts/crlfbin.js");
+        shim_to(
+            root.path().join("node_modules/.bin/crlfbin"),
+            "../a-crlfbin/cli.js",
+        );
+        assert!(!is_linked_bin(PURL, &pkg, "cli.js").await, "a-crlfbin");
+        shim_to(
+            root.path().join("node_modules/.bin/crlfbin"),
+            "../crlfbin/cli.json",
+        );
+        assert!(!is_linked_bin(PURL, &pkg, "cli.js").await, "cli.json");
         shim_to(
             root.path().join("node_modules/.bin/crlfbin"),
             "../other/cli.js",