npm lock rewrites drop CRLF (and hosted drops tab indent), so rollback and vendor --revert are not byte-exact; BOM locks are refused #324

[agent] Found by the scheduled npm bug-hunt routine (ledger #302).

Summary

The npm lockfile writers throw away the file's layout. The hosted rewrite turns a CRLF package-lock.json into LF and re-indents a tab-indented one to 2 spaces. The vendored rewrite keeps the indent but also turns CRLF into LF. The undo commands (rollback for hosted, vendor --revert for vendored) re-serialize the same way, so they don't put the original bytes back. A lock with a UTF-8 BOM, which npm itself installs from, is refused outright: hosted skips it as "not valid JSON" and vendored reports the misleading vendor_lockfile_version_unsupported.

Impact

• A repository that commits its lock with CRLF (Windows teams with .gitattributes eol=crlf / -text, or any CRLF working tree) or with tab indentation gets a whole-file diff from scan --mode hosted / scan --mode vendored, not a two-line edit. That makes the security change hard to review.
• The documented undo guarantee breaks. README says vendor --revert "restores the original lockfile byte-for-byte" (README.md:450, :789, :822), remove says "the lockfile is restored byte-for-byte" (README.md:1107), and docs/testing/npm-compatibility.md lists "byte-exact revert" for e2e_vendor_npm_build. After an undo, the lock is still fully reformatted.
• BOM-prefixed locks (npm accepts them; npm ci installs fine) can't be patched in hosted or vendored mode. Vendored mode reports it under a wrong error code.

Installs aren't broken: every rewritten lock still installs the patched bytes with npm ci. This is a fidelity and contract problem, not a security failure.

Repro

Real npm 12.1.0 lock (npm install left-pad@1.3.0), then converted in place. The patch source is a local mock of the patch API modelled on the wiremock fixtures in crates/socket-patch-cli/tests/e2e_redirect_npm_build.rs: batch, by-package, /patches/package grant and /patches/view, serving a tarball whose index.js has a marker prepended.

mkdir app && cd app
echo '{"name":"app","version":"1.0.0","private":true,"dependencies":{"left-pad":"1.3.0"}}' > package.json
npm install
# pick one:
sed -i 's/$/\r/' package-lock.json                                            # CRLF
# node -e 'const f="package-lock.json";require("fs").writeFileSync(f,JSON.stringify(JSON.parse(require("fs").readFileSync(f)),null,"\t")+"\n")'   # tabs
# printf '\xef\xbb\xbf' | cat - package-lock.json > l && mv l package-lock.json  # BOM
cp package-lock.json lock.orig
npm ci                                   # npm accepts all three shapes

socket-patch scan --mode hosted --yes --json --api-url $MOCK --org test-org --api-token fake
grep -c $'\r' package-lock.json          # CRLF input: 0 (was 22)
socket-patch rollback --yes
cmp lock.orig package-lock.json          # differs

# vendored twin
socket-patch scan --mode vendored --yes --json ...; socket-patch vendor --revert --yes
cmp lock.orig package-lock.json          # differs for CRLF input

Expected vs actual

Expected: an edit changes only the rewired resolved / integrity values, and the undo restores the original bytes, as README documents for vendor --revert / remove and as the pnpm contract states ("LF/CRLF and unrelated lock bytes are preserved", CLI_CONTRACT.md:97). A BOM lock is read like npm reads it (npm strips the BOM).

Actual, on main f6b7fb9e, run twice with identical results:

OS × version

Released 4.0.0 behaves the same (CRLF and tabs cells checked). 3.3.0 has no --mode. So this is long-standing, not a regression.

Suspect code

• crates/socket-patch-core/src/patch/redirect/mod.rs:965: rewrite_one_npm_lock writes serialize_json(&lock), the fixed 2-space / LF serializer defined at :269.
• crates/socket-patch-core/src/patch/redirect/mod.rs:819: serde_json::from_str rejects a BOM, so the lock counts as unparseable.
• crates/socket-patch-core/src/vendor/npm_lock.rs:309-310 (also :268, :724): detect_indent + serialize_json keep the indent but always emit LF.
• crates/socket-patch-core/src/vendor/npm_lock.rs:510: a BOM parse failure is mapped to vendor_lockfile_version_unsupported.
• crates/socket-patch-core/src/vendor/common.rs:185 already has JsonLayout (BOM + indent + EOL + trailer, "so a vendor edit and its revert change nothing but the edited keys"), and parse_json_manifest (:169) strips the BOM. Both are used for package.json and berry, but not for the npm locks.

Not checked on release/v5-prerelease.