Consolidate remaining BOM stripping after the pnpm reader failures were fixed #905

[agent] Filed by the scheduled architecture audit routine (ecosystems and formats). Register: discussion #560 register.

Kind: bug (with a refactor fix). Source: new finding; review "CRLF/BOM/indent policy" (Part 4.4), register E64.

Problem

No module owns "a leading UTF-8 BOM is encoding, not content". Each reader decides for itself, on main @ 9c43dfc:

• Four named helpers do the same thing: utils::serde::strip_bom, gradle::dsl::strip_bom, a private formats::yarn::strip_bom and cargo_manifest::split_bom.``
• About 50 more production sites in 30 files inline strip_prefix('\u{feff}') or trim_start_matches('\u{feff}'), and several also re-add the BOM after an edit: split_bom, vendor::common, redirect/mod.rs, [`upstream/npm.rs`](https://github.com/SocketDev/socket-patch/blob/9c43dfc96a3da66ff83c0d3a977db9f496ecbd53/crates/socket-patch-core/src/patch/redirect/upstream/npm.rs#L457-L459),`` plus requirements, Pipenv and Gradle.
• The copies have drifted. vendor::common::parse_json_manifest strips exactly one BOM, which its test parse_json_manifest_reads_past_one_bom_only pins down. lock_inventory/pypi.rs,`` pypi_hatch.rs, `upstream/pypi.rs` and `vex/discover/pypi_other.rs` strip any number.
• The pnpm format module never strips it. Three lockfileVersion readers in one file match a column-0 literal: head_lock_version (behind sniff_lock_grammar), lock_versions (behind lock_version_major and may_need_store_flag) and is_pnpm_lock_text. [`workspace::top_level_key`](https://github.com/SocketDev/socket-patch/blob/9c43dfc96a3da66ff83c0d3a977db9f496ecbd53/crates/socket-patch-core/src/formats/pnpm/workspace.rs#L20-L25``) doesn't strip it either, while the other pnpm-workspace.yaml key reader, governing_root::workspace_lockfile_dir,`` does. The yarn sniff is_berry_lock skips it on purpose.

Proof by execution. I ran a throwaway unit test twice on 9c43dfc with identical results. It used one pnpm 9 lock with one left-pad@1.3.0 entry, once plain and once with a \u{feff} prefix:

So a single BOM lock gets four answers inside formats::pnpm: it is readable, not a pnpm lock, unversioned, and unsupported. pnpm itself reads it (see #903).

Symptoms

• Hosted pnpm scan skips the trustLockfile: true auto-config when pnpm-lock.yaml starts with a UTF-8 BOM, so pnpm 11/12 frozen installs fail with ERR_PNPM_TARBALL_URL_MISMATCH after a successful scan #903: the hosted trust gate misses a BOM pnpm-lock.yaml, and the frozen install fails.
• pnpm-workspace.yaml with a UTF-8 BOM: hosted and vendored miss the first top-level key and append a duplicate trustLockfile / overrides, so every pnpm install fails with "duplicate mapping key" after a successful scan #904: top_level_key misses a BOM workspace key, and a duplicate trustLockfile/overrides is appended.
• Hosted and vendored NuGet reject a packages.lock.json with a UTF-8 BOM that dotnet restores fine: hosted skips the redirect and exits 0 success, vendored fails apply_failed #623: the NuGet lock rewriters' bare serde_json::from_str refuses a BOM packages.lock.json.
• Not filed before: vendored pnpm refuses a BOM lock with a false "no lockfileVersion … re-lock" remedy, and VEX discovery reports it as "not a pnpm lockfile" (table above).

Impact: medium. Each new reader repeats the decision, and every one that forgets it is a new bug. Three bug-hunt issues have hit this in different ecosystems so far.

Proposed change

1. Add formats::text with split_bom(&str) -> (&str, &str) (one BOM, as parse_json_manifest pins down) and strip_bom. Delete utils::serde::strip_bom, gradle::dsl::strip_bom, formats::yarn::strip_bom and cargo_manifest::split_bom, and re-point their callers.
2. In formats::pnpm: have head_lock_version, lock_versions, is_pnpm_lock_text and may_need_store_flag read strip_bom(text). Have the pnpm-workspace.yaml splices call top_level_key on a BOM-stripped first line, re-adding the BOM on write. Then make governing_root::workspace_lockfile_dir a top_level_key caller, which deletes its private key-prefix grammar.
3. Re-point the inline strip_prefix('\u{feff}') / trim_start_matches('\u{feff}') sites to the helper, file by file. Any site that keeps "any number of BOMs" must justify it in a comment.

Size and scope

• Steps 1 and 2: about 120 production lines in formats/, utils/serde.rs, gradle/dsl.rs, vendor/cargo_manifest.rs and hosted/governing_root.rs.
• Step 3 is mechanical and can be a second PR (about 50 sites, net negative).
• Out of scope:
  • UTF-16 (Hosted and lock-only scans treat a UTF-16 requirements.txt (what Windows PowerShell's pip freeze > writes) as absent: exit 0, no warning, and pip keeps installing the unpatched pin #721 / Fix UTF-16 requirements.txt silently skipped (#721) #724).
  • The NuGet lock rewriters (Hosted and vendored NuGet reject a packages.lock.json with a UTF-8 BOM that dotnet restores fine: hosted skips the redirect and exits 0 success, vendored fails apply_failed #623). They should call the helper once it exists, but that fix belongs to Hosted and vendored NuGet reject a packages.lock.json with a UTF-8 BOM that dotnet restores fine: hosted skips the redirect and exits 0 success, vendored fails apply_failed #623.
  • Line endings (Tracking: classify line endings in one place instead of five drifting rules #814).

Acceptance criteria

• One strip_bom/split_bom pair in formats::text, and the four named copies are deleted.
• A BOM-prefixed pnpm 9, 6.0 and 5.4 lock gives the same sniff_lock_grammar, lock_version_major, is_pnpm_lock and entries answers as its plain twin (table-driven unit test).
• top_level_key on a BOM first line returns the plain key, and a workspace splice keeps the BOM byte-exact (regression tests for Hosted pnpm scan skips the trustLockfile: true auto-config when pnpm-lock.yaml starts with a UTF-8 BOM, so pnpm 11/12 frozen installs fail with ERR_PNPM_TARBALL_URL_MISMATCH after a successful scan #903 and pnpm-workspace.yaml with a UTF-8 BOM: hosted and vendored miss the first top-level key and append a duplicate trustLockfile / overrides, so every pnpm install fails with "duplicate mapping key" after a successful scan #904).
• parse_json_manifest_reads_past_one_bom_only and the existing CRLF/BOM round-trip tests (npm_lock_rewrite_keeps_crlf_tabs_and_bom, berry_crlf_and_bom_locks_round_trip_byte_exact, bom_prefixed_lock_is_rewritten_with_the_bom_intact) stay green.
• grep -rn "feff" crates/socket-patch-core/src outside formats/text.rs and tests lists only sites with a justifying comment (after step 3).

Dependencies

• Blocks nothing. It makes Hosted pnpm scan skips the trustLockfile: true auto-config when pnpm-lock.yaml starts with a UTF-8 BOM, so pnpm 11/12 frozen installs fail with ERR_PNPM_TARBALL_URL_MISMATCH after a successful scan #903, pnpm-workspace.yaml with a UTF-8 BOM: hosted and vendored miss the first top-level key and append a duplicate trustLockfile / overrides, so every pnpm install fails with "duplicate mapping key" after a successful scan #904 and Hosted and vendored NuGet reject a packages.lock.json with a UTF-8 BOM that dotnet restores fine: hosted skips the redirect and exits 0 success, vendored fails apply_failed #623 one-line fixes.
• Independent of Tracking: classify line endings in one place instead of five drifting rules #814 (line endings), which covers the other half of the layout policy.

Backlog review — 2026-10-08

Priority: P1 → P3. The functional pnpm BOM-reader failures were fixed by #909. The remaining broad strip_bom consolidation is maintenance; keep active PR #1117.

The title now describes the remaining scope after the partial fixes. The original report is preserved above for historical context.