Skip to content

Yarn classic hosted and vendored rewrites convert LF lines of a mixed CRLF/LF yarn.lock to CRLF, so rollback is not byte-exact #467

Description

[agent] Found by the scheduled Yarn classic (1.x) bug-hunt routine (ledger #304).

Summary

The yarn classic lock writers are built to keep the lock's own line endings: CRLF locks round-trip, bare \r is refused, and the vendored revert restores "in the style of the block it replaces". A yarn.lock that mixes CRLF and LF lines (for example after a merge resolved in an editor on Windows) gets neither treatment:

  • Hosted (rewrite_yarn_classic): any \r sets crlf = true. The lock is LF-normalized, and on output every \n becomes \r\n, including blocks the run never touched. The bare-\r refusal (redirect_yarn_classic_unsupported_line_endings) doesn't fire, because a CRLF+LF mix has no bare \r.
  • Vendored (vendor/yarn_classic_lock.rs): untouched blocks are preserved, but the rewired block is spliced with the file's dominant terminator (detect_eol(&text), line 146), not its own (block_eol). An LF block comes back CRLF. rollback / vendor --revert then restore it with block_eol of the now-CRLF live block, so it stays CRLF.

In both modes rollback is no longer byte-exact. Yarn itself is unaffected (install --frozen-lockfile and check --integrity pass), so the impact is lockfile churn and broken byte-exact revert, not a broken install.

Impact

  • Hosted: a one-package redirect rewrites line endings across the whole lock (unrelated blocks show up in the diff). After rollback, the file is all-CRLF.
  • Vendored: the rewired block's endings change, and rollback doesn't restore the original bytes.
  • This contradicts the in-code contract (redirect/mod.rs:2969: "untouched lines round-trip byte-identically") and the vendored-revert comment at yarn_classic_lock.rs:643-646. The unit test yarn_classic_mixed_line_endings_are_refused only covers bare \r, not the CRLF+LF mix. For comparison, the berry rewriter refuses mixed locks (redirect_yarn_berry_mixed_line_endings).

Repro (Linux, main 2463257, yarn 1.22.22, local mock patch API)

mkdir app && cd app
echo '{"name":"p","version":"1.0.0","dependencies":{"left-pad":"^1.3.0","is-number":"^7.0.0"}}' > package.json
yarn install && rm -rf node_modules
# make the header + is-number block CRLF, leave the left-pad block LF
python3 - <<'EOF'
t=open('yarn.lock').read(); a,b=t.split('left-pad@',1)
open('yarn.lock','w',newline='').write(a.replace('\n','\r\n')+'left-pad@'+b)
EOF
cp yarn.lock orig.lock

# hosted, patching ONLY is-number:
socket-patch scan --package is-number --mode hosted --json
diff <(cat -A orig.lock) <(cat -A yarn.lock)
#   is-number resolved/integrity changed (expected)
#   left-pad block: every line "$" -> "^M$"  (untouched block, line endings rewritten)

# vendored, patching ONLY left-pad (fresh copy of orig.lock):
socket-patch scan --package left-pad --mode vendored --json
#   left-pad block rewired AND converted LF -> CRLF
socket-patch rollback --json      # success
cmp yarn.lock orig.lock           # differs: left-pad block stays CRLF

Reproduced twice in each mode (full two-package runs, then --package-scoped runs).

Expected vs actual

  • Expected: only the rewired entry's resolved / integrity lines change, in the block's own line ending, and every other byte stays (CLI_CONTRACT "Hosted unwind coverage": "only the hosted entries change and every other byte stays the file's own"; vendored table: "byte-stable lock"). Rollback restores the original bytes. Alternatively, the lock is refused with a warning, as berry does.
  • Actual: hosted converts the whole file to CRLF. Vendored converts the rewired LF block to CRLF. Neither rollback is byte-exact. No warning in either mode.
OS yarn hosted untouched blocks vendored rewired block rollback byte-exact
Linux 1.22.22 converted (fail) converted (fail) no (both modes)
Linux uniform CRLF / BOM+CRLF lock (control) preserved preserved yes (apart from the registry host, see below)

The rewrite is pure string handling, independent of OS and yarn release. Note: in the sandbox, hosted rollback needed SOCKET_NPM_REGISTRY pointed at a local passthrough, so restored hosts read registry.npmjs.org. That difference is expected and separate from this bug.

Suspect code

  • crates/socket-patch-core/src/patch/redirect/mod.rs:2975 (let crlf = raw.contains('\r')) and :3115 (out.replace('\n', "\r\n")): treat a CRLF+LF mix as uniform CRLF. Either refuse it like the bare-\r case, or splice per block.
  • crates/socket-patch-core/src/vendor/yarn_classic_lock.rs:146 (let eol = detect_eol(&text), used at :198): should be block_eol(&new_text, block), as the revert path at :646 uses.

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