[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Fixes #1791

Summary

Users whose npm bin was broken by v5.0.1 (a CRLF shebang that exits 127) can now recover by upgrading and running apply or rollback. Before this change, both commands reported success and left the bin broken.

• apply rewrites an npm bin that holds the raw patched bytes with a CRLF shebang into npm's fixBin form (LF shebang). The bin runs again, and a second apply is a no-op.
• apply --check reports such a bin as drift (not_applied, exit 1) instead of "Patches are in sync".
• rollback from that state restores npm's runnable form of the original, not the CRLF tarball bytes.
• rollback also rewrites a bin that an older rollback left holding the raw CRLF original, as long as the original's blob is present. Without the blob it stays "already original", as before.

Root cause

npm_bin::form_for_write decided whether a file was a bin npm had installed by looking at the shebang currently on disk: only a file that already had an LF shebang counted. A bin that v5.0.1 overwrote with the raw CRLF afterHash bytes failed that test, so rollback wrote the CRLF original back. Separately, the apply engine returned AlreadyPatched as soon as the raw bytes hashed to afterHash, and the apply --check verifier agreed.

Fix

The bin decision now comes from the package's own package.json, read the way npm's normalize-package-bin and directories.bin step read it:

• bin as a string (only when the package has a name), an object's values, or an array's entries;
• when bin is missing or empty, non-hidden files under directories.bin;
• paths are normalized (./ and .. folded).

The new logic lives in npm_bin::is_listed_bin and npm_bin::needs_fix, which also requires the bytes to be exactly the recorded content. It is used by:

• form_for_write, for both apply and rollback. Content with no CRLF shebang skips all of this I/O.
• mark_crlf_bin_rewrite in the apply engine and in rollback. The shared verify_file_patch is unchanged, because a vendored tarball's staging copy legitimately holds the CRLF bytes.
• apply --check, through the new patch::apply::needs_npm_bin_rewrite, which uses the same afterHash condition as apply so the two always agree.

The earlier LF-shebang heuristic is kept as a fallback for packages with no readable package.json. Vendored npm flows patch tarballs in memory and never reach form_for_write, so they are unaffected.

Per-issue checklist (#1791)

Symptom in #1791	Test
apply says "already patched", bin exits 127	patch::apply::tests::apply_repairs_crlf_bin_left_by_raw_write; e2e npm_agent_recovers_crlf_bin_left_by_raw_write
apply --check says "in sync"	e2e npm_agent_recovers_crlf_bin_left_by_raw_write (exit 1 before apply, 0 after)
rollback writes the CRLF original	patch::rollback::tests::rollback_of_raw_crlf_bin_restores_npm_form; same e2e
Non-bin files, unnamed string bins, non-npm purls stay untouched	apply_keeps_crlf_shebang_npm_does_not_fix, npm_bin::tests::lists_bin_reads_every_bin_shape
Rollback after an old CRLF rollback	rollback_rewrites_raw_crlf_original_bin

Test evidence

Red → green:

• Without the fix, cargo test -p socket-patch-core --lib rollback_of_raw_crlf_bin failed: the file stayed CRLF.
• Without the fix, the e2e npm_agent_recovers_crlf_bin_left_by_raw_write failed at its first assertion: apply --check printed "Patches are in sync (1 patch checked)." and exited 0 (expected 1).
• With the fix, both pass.

Commands run locally (Linux, npm 10.9.4, Node 22):

• cargo fmt --all -- --check and cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test -p socket-patch-core --all-features: 6876 passed, 4 failed. All 4 are read-only-permission tests (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_rolls_back_…). They fail only because this container runs as root, which ignores the read-only bits. They are untouched by this PR and pass on CI's unprivileged runners.
• cargo test -p socket-patch-cli --all-features --lib --bins: 936 passed.
• cargo test -p socket-patch-cli --all-features --test apply --test e2e_redirect_npm_build -- --include-ignored: 157 + 38 passed.
• The full cargo test --workspace --all-features needs more disk than this container allows, so the remaining CLI integration suites are left to CI.
• Wrappers (npm/, pypi/, gem/) only dispatch to the binary; no change was needed there.

Review notes (/code-review high)

Fixed in bb8b488:

• --check now uses apply's afterHash condition.
• Rollback handles a raw CRLF original left by an older rollback.
• bin is read the way npm reads it (arrays, nameless string, empty-bin fallback, hidden files).
• form_for_write checks the content before any I/O.
• The duplicated bin-run test helper is removed.

Not changed:

• vex still attests a bin in the v5.0.1 state. The patched code is on disk byte for byte, so the vulnerability is fixed; the bin just can't start. Making the shared verifier fail it would drop real attestations for every vex and stale-install caller. apply --check is where that drift is reported.
• Installs from managers that don't run fixBin (e.g. yarn classic) now get the LF form for a listed bin. A CRLF shebang never runs under env either way, and every verifier already accepts npm's form of both hashes.
• Extra reads in --check. It only reads a 2 KB prefix per file, and package.json only for a CRLF-shebang candidate.
• A patch that changes bin inside package.json at the same time as the bin. Not seen in practice.
• repair is unchanged. CLI_CONTRACT defines it as blob/artifact repair that never touches installed files. apply is the command that converges installed files, and it now does.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aeQpwwNbXABKDVsjWva

---

Note

Medium Risk
Changes apply, rollback, and check behavior for npm bin files across real installs; scope is limited to listed bins with fixable CRLF shebangs, but incorrect bin detection could rewrite or skip the wrong files.

Overview
Fixes #1791: npm bins that still hold raw patched bytes with a CRLF shebang (from v5.0.1) are treated as broken even when the hash matches afterHash. apply and rollback now rewrite listed npm bin files into npm’s fixBin (LF shebang) form, using package.json bin metadata (with the old LF-on-disk heuristic as fallback) instead of trusting the on-disk shebang alone.

apply --check demotes packages that would still be rewritten to drift (not_applied), aligned with the apply engine via needs_npm_bin_rewrite. Verification still reports AlreadyPatched for tarball staging copies that legitimately keep CRLF bytes.

E2E coverage is refactored around shared CRLF-bin fixtures and adds npm_agent_recovers_crlf_bin_left_by_raw_write for check → apply → rollback recovery.

^{Reviewed by Cursor Bugbot for commit bb8b488. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit bb8b488. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at bb8b488d (burn-down agent).

• CI: 82/82 (32 success, 50 skipped) check runs green or skipped on bb8b488d; no main-wide failures.
• Bugbot: reviewed bb8b488d, no findings; no open review threads.
• Mergeable, no CHANGELOG.md change.
• Slack announcement not sent this run (Slack send unavailable); the next run retries.

---

Generated by Claude Code