[Mikola Lysenko (mikolalysenko)]

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

Fixes #1756

Summary

npm's bin-links (fixBin, which pnpm also uses) rewrites, on every OS, every bin file whose first line is #!…\r\n so that line ends in \n. A bin published from Windows is therefore one byte different on disk from the tarball and from the patch's beforeHash/afterHash. socket-patch treated that byte as user drift:

• agent apply/scan --mode agent failed with apply_failed ("File hash does not match expected value") on a pristine install.
• agent + consent (--allow-patch-conflicts/--force, and the v5.0.1 default) wrote the CRLF shebang back, so /usr/bin/env: 'node\r' exited 127 while socket-patch reported success.
• hosted vex omitted a correctly patched install (hash_mismatch, then no_applicable_patches).
• vendored vex warned vendored_tree_out_of_sync on a tree that was in sync.

Root cause

Every hash check compared the raw installed bytes with the tarball hashes. None of them modelled npm's own deterministic install transform.

Fix

• New patch::npm_bin models fixBin exactly: a ^#[^\n]+\r\n match within the first 2048 bytes, and only the first \r is dropped. It also has the inverse (unfix_bin) and a cheap source_hash(path), which reads a whole file only when its 2 KB prefix qualifies.
• verify_file_patch (used by agent apply, hosted vex, vlt, and vendored vex's live-tree in-sync check) now also accepts npm's form of afterHash (already patched) and of beforeHash (ready). This is checked only when the raw hash matches neither.
• Apply and rollback both write through npm_bin::form_for_write, which looks at the file on disk. An npm package's file with an LF shebang gets npm's form of whatever is written, whether patched bytes, a consent overwrite or a restored original, so the bin stays runnable. In other ecosystems this happens only when the file is provably npm's form of the content it replaces. The blob is hash-checked before the transform, as apply_file_patch_at would check it.
• Rollback (verify_file_rollback, including patch-added files, and the store-copy ownership probe) accepts npm's form the same way, so an agent apply→rollback round trip restores npm's form of the original. The CLI's other-scope probe for rollback also counts a copy in npm's form as patched.
• Under MismatchPolicy::SocketContent, npm's form of an earlier Socket blob still counts as Socket content, so superseding updates work.
• judge_installed_record (the gem/python stale-install probe) stays in step with verify_patch_record, as its contract test requires.
• A blob that is not valid UTF-8 is written as recorded. npm would replace those bytes with U+FFFD, and nothing can trace that form back to the hash.

The fallback in verify is not limited to npm purls (verify_file_patch takes no ecosystem). It only matches content that differs from the recorded bytes by exactly that one shebang \r, so it can't mask a code change.

The wrappers under npm/, pypi/ and gem/ only dispatch to the binary and don't need changes.

Test evidence

Red on main (the fix files reverted, the new tests kept):

test patch::apply::tests::apply_patches_npm_fixed_bin_and_keeps_it_runnable ... FAILED
test patch::apply::tests::verify_accepts_npm_fixed_crlf_shebang ... FAILED
test patch::apply::tests::consent_overwrite_of_npm_bin_keeps_lf_shebang ... FAILED
test patch::rollback::tests::rollback_restores_npm_fixed_bin ... FAILED
test vex::verify::tests::applied_when_npm_fixed_the_patched_bin_shebang ... FAILED
e2e npm_agent_patches_crlf_shebang_bin_npm_rewrote: apply exit 1 "File hash does not match expected value"

Green on this branch:

• cargo test -p socket-patch-core --all-features --lib -- npm_fixed npm_bin crlf_shebang unfixed_crlf consent_overwrite socket_content judge_installed rollback_of_lf rollback_outside: 21 passed
• cargo test -p socket-patch-cli --all-features --test e2e_redirect_npm_build -- --ignored npm_agent_patches_crlf: passed with real npm 10.9.4. It covers npm install of a local CRLF-bin tarball, agent apply --offline, the bin printing hi, a no-op re-apply, vex attesting not_affected, and rollback restoring npm's form with the bin still runnable.
• cargo fmt --all -- --check, cargo clippy --workspace --all-features -- -D warnings: clean
• cargo test --workspace --all-features --no-fail-fast: 228 of 240 test binaries passed. The 12 failing tests are environmental and outside this diff. Nine are write-failure tests (partial_lockfile_write_failure_exits_1_and_writes_no_ledger, relax_loop_must_not_traverse_symlinked_root, an_unremovable_hidden_lock_keeps_every_store_entry, redirect_json_mode_write_failures_emit_error_envelope, two repair_*, wire_write_failure_maps_error_and_leaves_lock_untouched, wire_failure_rolls_back_already_written_files, scan_redirect_vlt_heal_invalidation_failure_warns) that can't induce EACCES because this sandbox runs as root. Three are e2e_bun_lockb tests: one needs the live patches-api.socket.dev, which the sandbox proxy refuses, and two depend on the CI-pinned bun (local bun is 1.4.4). CI is the authority for these.
• node --test npm/socket-patch/bin/socket-patch.test.mjs: 24 passed (wrappers untouched)

Per-issue checklist (#1756)

• Agent default fails on a pristine install: verify_accepts_npm_fixed_crlf_shebang, apply_patches_npm_fixed_bin_and_keeps_it_runnable, e2e
• Agent + consent breaks the bin: consent_overwrite_of_npm_bin_keeps_lf_shebang (and consent_overwrite_outside_npm_writes_exact_bytes as the guard)
• Hosted vex omits the patch: applied_when_npm_fixed_the_patched_bin_shebang (the same verify_patch_record_for path the hosted verifier uses)
• Vendored vex vendored_tree_out_of_sync: the live-tree in-sync probe is the same verify_patch_record_for / verify_file_patch call, covered by the test above
• Rollback round trip (not in the report, same cause): rollback_restores_npm_fixed_bin, rollback_deletes_npm_fixed_added_bin, e2e

/code-review high and Bugbot

Every /code-review high finding is fixed: lossy UTF-8 round trip, the new-file rollback branch, the other-scope rollback probe, the judge_installed_record contract, Windows gating, the duplicated blob check, and the repeated hashing. Bugbot's first review found that rolling back a patch whose own bytes gave the bin an LF shebang restored the CRLF original. It is fixed in aea403e, which also removes the "a Ready whose current_hash is not the expected hash" inference that /code-review had flagged. Both writers now decide from the file on disk. Regression tests: rollback_of_lf_patched_bin_restores_npm_form and rollback_outside_npm_restores_exact_bytes.

Follow-ups

• Windows: npm runs fixBin there too, right after writing the cmd-shim (bin-links 5.0.0 lib/shim-bin.js:78), as the final review pointed out. 1206981 drops the Windows exception and the test gates that assumed it. Before that, a consent overwrite or a rollback on Windows wrote the recorded CRLF bytes; verify accepted them, so nothing broke. No real-npm Windows e2e covers this; the unit tests now run there.
• From the final review, not done: (2) a copy in the other scope held in npm's form whose before-blob was garbage-collected is not counted as patched (MissingBlob); (3) the write side treats any LF-shebang file of an npm package as a bin, without reading package.json bin.

https://claude.ai/code/session_011f9aeS6yNpFr3Yme2w9kfw

---

Note

Medium Risk
Changes core hash verification and file writes for npm bins across apply, rollback, and VEX; logic is narrow (one-byte shebang transform) but touches security-sensitive patch paths.

Overview
Fixes #1756: npm bin-links drops the \r from a Windows-style #!…\r\n shebang on install, so pristine bins no longer match tarball beforeHash/afterHash and patch apply, VEX, and rollback misbehaved.

Adds patch::npm_bin to mirror fixBin (and invert it for hashing). Verify paths now treat npm’s on-disk LF-shebang form as matching recorded before/after hashes via source_hash. Apply and rollback write through form_for_write so npm bins stay LF-shebang and remain runnable instead of writing CRLF back. VEX / stale-install checks and SocketContent supersede logic follow the same rules; the CLI rollback “other scope still patched” probe also counts npm-form patched bins (Ready).

Coverage is mostly new unit tests plus an ignored Unix e2e that runs a real npm install through apply, vex, and rollback.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review at aea403e.

• CI: 31/31 non-skipped checks green on the head (19 skipped).
• Bugbot: reviewed aea403e; its one earlier thread on patch/rollback.rs is resolved and outdated, no open findings.
• Look at: patch/npm_bin.rs (accepting npm bin-links' CRLF-shebang rewrite in hash checks) and its use in apply, rollback and vex verify.
• Slack announcement not sent this run (Slack send tool unavailable); a later run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Ready for review at aea403ec8d29a4dfa25d5aed2489cdd4ff19413f (burn-down agent label sweep).

• CI: 50/50 check runs green or skipped on this head; mergeable, no conflicts.
• Bugbot: reviewed aea403e, no unresolved findings.
• Slack announcement not sent this run (Slack send unavailable); a later run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review: one blocking issue, fixed in 5831b1a.

socket_content_policy_knows_npm_fixed_socket_blob (crates/socket-patch-core/src/patch/apply.rs) wasn't gated off Windows. On Windows form_for_write skips npm's LF form, because npm leaves bins alone there. The fallback compares against before_hash, but the disk holds the earlier v1 blob, so the exact CRLF bytes are written and the NPM_BIN_AFTER assertion fails. I reproduced this by forcing the Windows branch locally. The Windows unit-test leg is skipped on PR pushes, so this would only have shown up as a merge-queue eviction. The fix adds #[cfg(not(windows))], matching the sibling consent_overwrite_of_npm_bin_keeps_lf_shebang. Production code is unchanged.

Checked locally: rustfmt --check passes on the file and cargo test -p socket-patch-core --lib npm passes 928/928. clippy --all-targets reports only the existing type_complexity on shared_store_fixture, which is also on main.

I removed Ready for review so the burn-down re-labels it once CI is green. A full review brief follows on the next pass.

bugbot run

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Ready for review at 5831b1af6d6bd1052c81817d09ddebea66f67d5b (burn-down agent label sweep).

• CI: 31/31 non-skipped checks green on this head (19 skipped); mergeable, no conflicts.
• Bugbot: Cursor Bugbot passed on 5831b1a with no new findings; the earlier patch/rollback.rs thread is from an older head and already addressed.
• Look at: the Windows gate the final reviewer added in 5831b1a on socket_content_policy_knows_npm_fixed_socket_blob (patch/apply.rs).
• Slack announcement not sent from this run (send tool unavailable); the next run should retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Ready for review again at 5831b1af (burn-down agent label sweep).

• CI: 49/49 check runs green or skipped on this head (30 success, 19 skipped); mergeable, no conflicts.
• Bugbot: reviewed 5831b1a (check success); its only earlier thread on patch/rollback.rs is resolved and outdated.
• Changed since the last label at aea403e: the final reviewer's 5831b1a gates socket_content_policy_knows_npm_fixed_socket_blob off Windows, where form_for_write skips npm's LF form.
• Slack announcement not sent this run (Slack send unavailable); next run retries.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. npm's bin-links fixBin changes a bin's #!…\r\n shebang line to \n at install time, so an untouched install no longer matches the patch's before/after hashes (#1756). This PR adds patch/npm_bin.rs, which models that rewrite. Verify, rollback, VEX and the other-scope check now also accept the npm form of the recorded content. On writes, a file npm installed with an LF shebang is written in npm's form, so the bin still runs.

Risk: low.

• The extra acceptance only forgives a one-byte CR difference on line 1 of a #! file, and only after the raw hash has failed.
• Writes still hash-check the recorded content before deriving the bytes.
• No path is built from package.json.
• File reads go through the existing safe open for regular files.

Look here

• npm_bin.rs:28 fix_bin, :72-96 form_for_write (with its Windows gate), and :101 unfix_bin
• apply.rs:264: verify accepts the npm form; apply.rs:1249: the write path
• rollback.rs:770: restore in npm form
• cli rollback.rs:337: the other-scope "still patched" check

Verified

• I read the full diff and compared fix_bin/unfix_bin with bin-links 5.0.0 lib/fix-bin.js: the 2048-byte probe, first match only, and the #!x\r\r\n case all agree.
• I traced apply → verify → re-apply → rollback → verify. Every form that gets written is accepted on the next verify.
• The unit tests fail without the change (e.g. verify_accepts_npm_fixed_crlf_shebang gets HashMismatch), and the e2e_redirect_npm_build leg is green.
• CHANGELOG.md is untouched and there is no leftover debug code.
• CI on the head: 31 passed, 19 skipped, 0 failed. Bugbot is clean.

Changes I made: none.

Open questions (none block merging; could go in a follow-up):

1. The Windows rationale is wrong. In bin-links, shim-bin.js:78 runs cmdShim(...).then(() => fixBin(absFrom)), so npm on Windows rewrites the shebang too. The comments at npm_bin.rs:3 and :69-70 and the !cfg!(windows) gate at :84 assume it doesn't. The effect is that on Windows a consent overwrite or a rollback writes the raw CRLF bytes, not npm's form. That is harmless (verify accepts both, and node ignores the shebang), but the gate and the #[cfg(not(windows))] test gates could be dropped.
2. In patched_in_other_scope (cli rollback.rs:337-342), if a copy in the other scope holds npm's LF form of the patched bytes and the before-blob has been GC'd, the status is MissingBlob and the copy isn't counted as patched. Comparing npm_bin::source_hash there too would close the gap. This needs a GC'd blob to happen.
3. The read-side acceptance applies to every ecosystem, and the write side treats any npm-package file with an LF shebang as a bin, without checking package.json bin. The worst outcome is a one-byte deviation for non-bin files.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Final-review open question 1 confirmed: npm's bundled bin-links 5.0.0 runs fixBin after cmdShim on Windows (shim-bin.js:78). 1206981 drops the !cfg!(windows) exception in npm_bin::form_for_write, fixes the comments, and removes the three #[cfg(not(windows))] test gates. Checked locally: fmt, clippy -D warnings, 985 core unit tests and the real-npm e2e all pass. Questions 2 and 3 stay as listed in the PR body's follow-ups.

---

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 1206981. Configure here.}

[Mikola Lysenko (mikolalysenko)]

[agent] Final reviewer: disarmed auto-merge. The head moved to 1206981 ("Keep npm's LF shebang form on Windows too", a code change) after the brief on 5831b1a, so the brief no longer covers the head. I'll re-review and re-brief the new head; auto-merge gets re-armed with the new brief.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (updated for 1206981)

What it does. npm's bin-links fixBin changes a bin's #!…\r\n shebang line to \n at install time, so an untouched install no longer matched the patch's hashes (#1756). This PR models that rewrite in patch/npm_bin.rs. Verify, rollback and VEX now accept the npm form, and writes keep npm's LF form so the bin still runs. The new head commit drops the Windows exception: bin-links runs fixBin on Windows too (shim-bin.js:78). That was open question 1 in the previous brief.

Risk: low. The extra acceptance only forgives one CR on line 1 of a #! file, and only after the raw hash has already failed. Writes still hash-check the recorded content first.

Look here

• npm_bin.rs:72-96 form_for_write: the Windows gate is gone
• apply.rs:1251: write path; rollback.rs:773: restore path
• npm_bin.rs:1-12: module doc, now right about Windows

Verified

• I reviewed the delta 5831b1a..1206981 (3 files, +8/−14). The rest of the PR is unchanged since the previous brief, which covered the full diff.
• The new delta removes the !cfg!(windows) gate and three #[cfg(not(windows))] test gates. No stale "npm leaves bins alone" comments remain.
• cargo test -p socket-patch-core --lib -- npm_bin shebang passes locally (10/10).
• CI on the head: 31/31 passed (the rest are skipped). Bugbot is clean. CHANGELOG.md is untouched.

Changes I made: none.

Open questions (not blocking):

1. The re-enabled tests only run on Windows in the nightly or release legs, because the PR CI skips Windows. Nothing in them looks platform-specific.
2. Two carry-overs from the previous brief: the other-scope check when a before-blob has been GC'd (cli rollback.rs), and the write side not checking package.json bin. Each has a worst case of a one-byte deviation or a missed "still patched" count.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code