Fix pnpm lock/workspace readers missing a BOM (#903, #904, #905)

crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs

@@ -405,6 +405,78 @@ async fn hosted_trust_edit_reads_the_workspace_yaml_shape() {
     }
 }
 
+/// #903 / #904: a `pnpm-lock.yaml` and `pnpm-workspace.yaml` saved with a
+/// UTF-8 BOM read like their plain twins. The BOM lock gets the
+/// `trustLockfile: true` auto-config (it used to read as unversioned and
+/// skip it), `rollback` unwinds the pin it just wrote (it used to refuse the
+/// lock as "not a pnpm lockfile") byte-exact, BOM included, and a BOM first
+/// `trustLockfile: false` key is the user's explicit opt-out, not a missing
+/// key a duplicate is appended after.
+#[tokio::test]
+#[serial]
+async fn hosted_bom_lock_and_workspace_read_like_their_plain_twins() {
+    let server = MockServer::start().await;
+    mock_discovery(&server).await;
+    mock_reference(&server).await;
+
+    // A BOM lock, no workspace file: the trust scaffold is created and the
+    // rollback restores the lock byte for byte.
+    let tmp = tempfile::tempdir().unwrap();
+    write_pnpm_project(tmp.path());
+    let lock_path = tmp.path().join("pnpm-lock.yaml");
+    let pristine = format!("\u{feff}{}", std::fs::read_to_string(&lock_path).unwrap());
+    std::fs::write(&lock_path, &pristine).unwrap();
+
+    let code = run(hosted_args(tmp.path(), server.uri())).await;
+    assert_eq!(code, 0, "scan --mode hosted should succeed on a BOM lock");
+    let lock = std::fs::read_to_string(&lock_path).unwrap();
+    assert!(lock.starts_with("\u{feff}lockfileVersion:"), "{lock}");
+    assert!(lock.contains(HOSTED_URL), "the BOM lock is redirected: {lock}");
+    let ws_path = tmp.path().join("pnpm-workspace.yaml");
+    assert_eq!(
+        std::fs::read_to_string(&ws_path).ok().as_deref(),
+        Some("packages:\n  - '.'\ntrustLockfile: true\n"),
+        "a BOM v9 lock gets the trustLockfile auto-config"
+    );
+
+    let code = rollback_hosted(tmp.path(), &server).await;
+    assert_eq!(code, 0, "rollback must unwind the pin on a BOM lock");
+    assert_eq!(
+        std::fs::read_to_string(&lock_path).unwrap(),
+        pristine,
+        "rollback restores the BOM lock byte for byte"
+    );
+    assert!(!ws_path.exists(), "the auto-created workspace file goes too");
+
+    // A BOM workspace file whose first key is the user's opt-out: left
+    // byte-identical (no duplicate `trustLockfile`), lock still redirected.
+    // One whose first key is something else gains the key once, BOM kept.
+    for (user_ws, want) in [
+        ("\u{feff}trustLockfile: false\npackages:\n  - '.'\n", None),
+        ("\u{feff}trustLockfile: true\npackages:\n  - '.'\n", None),
+        (
+            "\u{feff}packages:\n  - '.'\n",
+            Some("\u{feff}packages:\n  - '.'\ntrustLockfile: true\n"),
+        ),
+    ] {
+        let tmp = tempfile::tempdir().unwrap();
+        write_pnpm_project(tmp.path());
+        std::fs::write(tmp.path().join("pnpm-workspace.yaml"), user_ws).unwrap();
+
+        let code = run(hosted_args(tmp.path(), server.uri())).await;
+        assert_eq!(code, 0, "scan --mode hosted should succeed for {user_ws:?}");
+        assert!(
+            std::fs::read_to_string(tmp.path().join("pnpm-lock.yaml"))
+                .unwrap()
+                .contains(HOSTED_URL),
+            "the lock is still redirected for {user_ws:?}"
+        );
+        let ws = std::fs::read_to_string(tmp.path().join("pnpm-workspace.yaml")).unwrap();
+        assert_eq!(ws, want.unwrap_or(user_ws), "workspace file for {user_ws:?}");
+        assert_eq!(ws.matches("trustLockfile").count(), 1, "{ws:?}");
+    }
+}
+
 /// `--dry-run` previews: NOTHING lands on disk — no lock rewrite, no
 /// pnpm-workspace.yaml, no ledger — while the envelope still reports both
 /// files as would-be-rewritten (`dryRun: true`).

crates/socket-patch-core/src/crawlers/gradle_cache.rs

@@ -70,8 +70,7 @@ pub fn hash_eq(dir_name: &str, sha1_hex: &str) -> bool {
 /// Whether `bytes` are the pristine download Gradle stored in the hash
 /// directory `dir_name` (their sha1 names it).
 pub fn pristine(dir_name: &str, bytes: &[u8]) -> bool {
-    use sha1::{Digest, Sha1};
-    hash_eq(dir_name, &hex::encode(Sha1::digest(bytes)))
+    hash_eq(dir_name, &crate::utils::digest::sha1_hex_of(bytes))
 }
 
 /// Whether `path` is a version directory of a `files-2.1` tree
@@ -432,8 +431,6 @@ impl DerivedIndex {
     /// The [`DerivedCopies`] of the jar `jar_leaf` whose pristine bytes
     /// hash to `pristine_sha1`.
     pub fn query(&self, jar_leaf: &str, pristine_sha1: &str) -> DerivedCopies {
-        use sha1::{Digest, Sha1};
-
         let instrumented = format!("instrumented-{jar_leaf}");
         let mut out = DerivedCopies {
             incomplete: self.incomplete,
@@ -460,7 +457,9 @@ impl DerivedIndex {
                 out.stale.push(path.clone());
             } else if name == jar_leaf || name == instrumented {
                 match crate::utils::fs::read_regular_to_bytes_sync(path) {
-                    Ok(bytes) if hash_eq(&hex::encode(Sha1::digest(&bytes)), pristine_sha1) => {
+                    Ok(bytes)
+                        if hash_eq(&crate::utils::digest::sha1_hex_of(&bytes), pristine_sha1) =>
+                    {
                         out.stale.push(path.clone())
                     }
                     Ok(_) => out.unknown.push(path.clone()),

crates/socket-patch-core/src/crawlers/npm_crawler.rs

@@ -108,7 +108,7 @@ fn pnpm_modules_dir_setting(start_path: &Path) -> Option<String> {
         .ancestors()
         .find_map(|dir| read(dir.join("pnpm-workspace.yaml")))
         .and_then(|yaml| {
-            crate::utils::serde::strip_bom(&yaml)
+            crate::formats::text::strip_bom(&yaml)
                 .lines()
                 .filter_map(crate::formats::pnpm::workspace::top_level_key)
                 .rfind(|(key, _)| key == "modulesDir")
@@ -367,7 +367,7 @@ fn parse_package_json_identity(content: &str) -> Option<(String, String)> {
     // (Windows-authored packages ship them), but serde_json rejects it —
     // a BOM'd install would be invisible to scan and unpatchable.
     let pkg: PackageJsonPartial =
-        serde_json::from_str(crate::utils::serde::strip_bom(content)).ok()?;
+        serde_json::from_str(crate::formats::text::strip_bom(content)).ok()?;
     let name = pkg.name?;
     let version = pkg.version?;
     if name.is_empty() || version.is_empty() {
@@ -591,7 +591,7 @@ const PNPM_MODULES_YAML: &str = ".modules.yaml";
 /// The `virtualStoreDir` value of a `.modules.yaml`: JSON on pnpm 10+,
 /// YAML before (a top-level `virtualStoreDir:` scalar, maybe quoted).
 fn parse_modules_yaml_virtual_store_dir(text: &str) -> Option<String> {
-    let text = crate::utils::serde::strip_bom(text);
+    let text = crate::formats::text::strip_bom(text);
     if let Ok(value) = serde_json::from_str::<serde_json::Value>(text) {
         return value
             .get("virtualStoreDir")?

crates/socket-patch-core/src/formats/mod.rs

@@ -26,14 +26,15 @@
 //! [`registry()`] is the one table of which project files carry a lock or
 //! its wiring, and in which roles.
 
+pub(crate) mod bun;
 pub mod cargo;
 pub mod composer;
 pub mod gem;
 pub(crate) mod maven;
 pub(crate) mod nuget;
 pub mod pnpm;
-pub(crate) mod bun;
 pub mod registry;
+pub mod text;
 pub mod yarn;
 
 pub use registry::registry;
@@ -81,7 +82,11 @@ mod architecture_tests {
                 .filter(|l| !l.trim_start().starts_with("//"))
                 .collect::<Vec<_>>()
                 .join("\n");
-            let used: Vec<&str> = IMPURE.iter().copied().filter(|n| code.contains(n)).collect();
+            let used: Vec<&str> = IMPURE
+                .iter()
+                .copied()
+                .filter(|n| code.contains(n))
+                .collect();
             assert!(
                 used.is_empty(),
                 "{}: a format model uses {used:?} — models are pure (module docs)",

crates/socket-patch-core/src/formats/pnpm/grammar.rs

@@ -7,10 +7,13 @@
 
 use std::ops::Range;
 
+use crate::formats::text::strip_bom;
+
 /// Early pnpm 1 writes shrinkwrapVersion 3 without a minor version and
 /// unconditionally drops registry tarball URLs on install. Its frozen flag
 /// cannot preserve this redirect (verified with pnpm 1.0.0).
 pub(crate) fn unsupported_early_shrinkwrap(content: &str) -> bool {
+    let content = strip_bom(content);
     let version = content
         .lines()
         .find_map(|line| line.strip_prefix("shrinkwrapVersion:"));
@@ -77,9 +80,11 @@ pub(crate) fn unquote(s: &str) -> &str {
 
 /// Whether `text` is a pnpm lock at all: a column-0 `lockfileVersion:`
 /// (pnpm >= 3) or `shrinkwrapVersion:` (pnpm 1 / 2) line — lockfile
-/// discovery's sniff before it reads any entry.
+/// discovery's sniff before it reads any entry. A leading BOM is encoding,
+/// not key text: pnpm reads a BOM lock like its plain twin (#903).
 pub(crate) fn is_pnpm_lock_text(text: &str) -> bool {
-    text.lines()
+    strip_bom(text)
+        .lines()
         .any(|line| line.starts_with("lockfileVersion:") || line.starts_with("shrinkwrapVersion:"))
 }