Skip to content

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

Description

[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:

lock shape mode after rewrite npm ci fresh checkout after undo
CRLF hosted LF (0 CR lines) patched LF, not byte-exact
CRLF vendored LF patched LF, not byte-exact
tabs hosted 2-space patched 2-space, not byte-exact
tabs vendored tabs kept patched byte-exact
BOM hosted untouched, redirect_npm_lock_unparseable, redirected: 0, status success unpatched n/a
BOM vendored refused vendor_lockfile_version_unsupported (rc 1) unpatched n/a

OS × version

npm 10.9.9 npm 12.1.0
Linux (Node 22.22) reproduces (CRLF + tabs cells) reproduces, 2 runs
macOS / Windows not run: the code path is OS-independent. On Windows a core.autocrlf=true checkout produces exactly the CRLF input

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions