GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — npm lock rewrites drop CRLF (and hosted drops tab indent), so rollback and vendor --revert are not byte-exact; BOM locks are refused · Issue #324 · SocketDev/socket-patch

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

New issue
Copy link
New issue
Copy link
Closed
#357
Closed
npm lock rewrites drop CRLF (and hosted drops tab indent), so rollback and vendor --revert are not byte-exact; BOM locks are refused#324
#357
Copy link
Labels
agent:triagedbugSomething isn't workingSomething isn't workingbughuntFound by a scheduled package-manager bug-hunt agentFound by a scheduled package-manager bug-hunt agentpm:npmnpmnpmpriority:p1

Description

@mikolalysenko
Mikola Lysenko (mikolalysenko)
opened on Sep 30, 2026
Issue body actions

[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 [email protected]), 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

  1. Mikola Lysenko (mikolalysenko)
    added
    bugSomething isn't workingSomething isn't working
    bughuntFound by a scheduled package-manager bug-hunt agentFound by a scheduled package-manager bug-hunt agent
    pm:npmnpmnpm
    agent:triaged
    priority:p1
    on Sep 30, 2026
  2. mikolalysenko commented on Sep 30, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Sep 30, 2026
    CollaboratorAuthor
    More actions

    [agent] Triaged: priority:p1 (npm). This isn't a duplicate, and I found no existing fix PR. Its cause, the npm lock writers ignoring vendor::common::JsonLayout, is separate from the other open npm reports (#325, #326).


    Generated by Claude Code

  3. mikolalysenko commented on Sep 30, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Sep 30, 2026
    CollaboratorAuthor
    More actions

    [agent] Shares root cause with #351: JSON manifest and lock writers re-serialize through detect_indent + serialize_json (or a fixed 2-space serializer) rather than the layout-preserving vendor::common::JsonLayout. That drops CRLF, BOM and indent. Will be fixed together.


    Generated by Claude Code

  4. mikolalysenko commented on Sep 30, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Sep 30, 2026
    CollaboratorAuthor
    More actions

    [agent] Claiming this issue (with #351; shared root cause: JSON writers ignore the file's layout instead of using JsonLayout). Branch: agent/fix-json-writers-keep-layout. Claim-ID: 2026-09-30T18:20:39Z-005fbf


    Generated by Claude Code

  5. Mikola Lysenko (mikolalysenko)
    added
    agent:claimed
    on Sep 30, 2026
  6. mikolalysenko commented on Sep 30, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Sep 30, 2026
    CollaboratorAuthor
    More actions

    [agent] Draft PR: #357


    Generated by Claude Code

  7. Mikola Lysenko (mikolalysenko)
    added 4 commits that reference this issue on Sep 30, 2026

    Add tests for JSON writers losing file layout

    80389b3

    Keep npm lock CRLF, tabs and BOM on rewrite

    6a79bd2

    Add real-npm e2e for a CRLF + BOM package-lock

    0dbe6cc

    Merge main into fix-json-writers-keep-layout to keep #324 on v5

    0644c00
  8. mikolalysenko commented on Oct 1, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 1, 2026
    CollaboratorAuthor
    More actions

    [agent] Re-checked on the v5 main 2463257 (#277), which hasn't merged #357: this still reproduces with the new default hosted mode, and every CRLF line is lost.

    Linux, npm 10.9.4 / Node 22.22, [email protected]. I converted package-lock.json to CRLF after npm install, then ran a bare socket-patch scan (hosted) followed by socket-patch rollback. I ran it twice in fresh dirs.

    scan rc=0 crlf_lines=0/22      # the hosted rewrite leaves 0 of 22 lines with CRLF
    rb rc=0
    NOT byte-exact crlf=0          # rollback writes LF, so cmp against the original lock fails
    

    Generated by Claude Code

  9. Mikola Lysenko (mikolalysenko)
    closed this as completedin c9485bcon Oct 1, 2026
  10. Mikola Lysenko (mikolalysenko)
    removed
    agent:claimed
    on Oct 1, 2026
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

    agent:triagedbugSomething isn't workingSomething isn't workingbughuntFound by a scheduled package-manager bug-hunt agentFound by a scheduled package-manager bug-hunt agentpm:npmnpmnpmpriority:p1

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions

      You are visitor 01402503 — GeezerVison does not store the pages you look up.
      Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.