Fix package.json workspaces read five ways (#1900)

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

@@ -111,7 +111,7 @@ fn skipped_members_of(root: &Path) -> Vec<PathBuf> {
 
     let Some(patterns) = crate::utils::fs::read_regular_to_string_sync(&root.join("package.json"))
         .ok()
-        .and_then(|text| crate::hosted::governing_root::workspace_patterns(&text))
+        .and_then(|text| crate::formats::package_json::Workspaces::listed_patterns(&text))
     else {
         return Vec::new();
     };
@@ -915,20 +915,17 @@ fn bun_workspace_pattern_members_sync(root: &Path) -> Option<Vec<PathBuf>> {
         Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Some(Vec::new()),
         Err(_) => return None,
     };
-    let text = strip_bom(&text);
-    let doc: serde_json::Value = serde_json::from_str(text).ok()?;
-    let patterns = match doc.get("workspaces") {
-        None | Some(serde_json::Value::Null) => Vec::new(),
-        Some(serde_json::Value::Array(list)) => list.clone(),
-        Some(serde_json::Value::Object(map)) => match map.get("packages") {
-            None => Vec::new(),
-            Some(packages) => packages.as_array()?.clone(),
-        },
-        Some(_) => return None,
+    use crate::formats::package_json::Workspaces;
+
+    // Bun refuses to install from a malformed field (#1900), so it names
+    // no member set.
+    let patterns = match Workspaces::parse(&text)? {
+        Workspaces::None => Vec::new(),
+        Workspaces::Patterns(patterns) => patterns,
+        Workspaces::Malformed(_) => return None,
     };
     let mut members = Vec::new();
     for pattern in &patterns {
-        let pattern = pattern.as_str()?;
         if pattern.starts_with('!') {
             continue;
         }
@@ -8130,6 +8127,19 @@ mod tests {
         )
         .unwrap();
         assert_eq!(bun_workspace_pattern_members_sync(dir.path()), None);
+
+        // #1900: Bun refuses to install from a `null` or non-string
+        // `workspaces`, so the patterns cannot name the member set.
+        for manifest in [r#"{"workspaces":null}"#, r#"{"workspaces":["a",1]}"#] {
+            std::fs::write(dir.path().join("package.json"), manifest).unwrap();
+            assert_eq!(
+                bun_workspace_pattern_members_sync(dir.path()),
+                None,
+                "{manifest}"
+            );
+        }
+        std::fs::write(dir.path().join("package.json"), r#"{"name":"x"}"#).unwrap();
+        assert_eq!(bun_workspace_pattern_members_sync(dir.path()), Some(vec![]));
     }
 
     /// `.yarnrc` `--modules-folder` parsing: bare and quoted keys and

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

@@ -34,6 +34,7 @@ pub mod governing_locks;
 pub(crate) mod json;
 pub(crate) mod maven;
 pub(crate) mod nuget;
+pub(crate) mod package_json;
 pub(crate) mod pipenv;
 pub mod pnpm;
 pub mod registry;

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

@@ -0,0 +1,169 @@
+//! The `workspaces` field of an npm-family `package.json`, read one way
+//! for every caller (#1900).
+//!
+//! The field takes two shapes: the array form (npm, yarn, Bun) and the
+//! object form whose `packages` array holds the patterns (yarn classic's
+//! `nohoist` shape, Bun's catalogs shape). What the package managers do
+//! with anything else was probed on npm 10.9, yarn 1.22.22 and Bun 1.4.4:
+//!
+//! | `workspaces`              | npm    | yarn 1        | Bun    |
+//! |---------------------------|--------|---------------|--------|
+//! | `["a", 1]`                | fails  | fails         | fails  |
+//! | `null`                    | fails  | no workspaces | fails  |
+//! | `"a"`                     | fails  | no workspaces | fails  |
+//! | `{"packages": "a"}`       | fails  | no workspaces | fails  |
+//! | `{"nohoist": []}`         | fails  | no workspaces | no workspaces |
+//!
+//! A list with a non-string item fails every install, so it is
+//! [`Workspaces::Malformed`], as are the other shapes npm and Bun refuse
+//! (yarn 1 installs those as if no field were set). The object form
+//! without `packages` installs under the two package managers that write
+//! it, so it declares no patterns.
+//!
+//! Readers that only widen (a governing-root refusal, an extra crawl
+//! root, a warning, a patch search) take a malformed field's string items
+//! through [`Workspaces::into_listed`]. A reader that must name an exact
+//! member set (the Bun member walk) treats a malformed field as
+//! unreadable.
+
+use serde_json::Value;
+
+/// What a `package.json` `workspaces` field declares.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub(crate) enum Workspaces {
+    /// No field, or the object form without `packages`.
+    None,
+    /// The patterns of a well-formed field, in order (possibly empty).
+    /// A leading `!` is kept: each caller applies negations the way its
+    /// package manager does.
+    Patterns(Vec<String>),
+    /// A field the package managers refuse to install from (see the
+    /// module table), with the string items it does hold.
+    Malformed(Vec<String>),
+}
+
+impl Workspaces {
+    /// The field of a parsed `package.json`.
+    pub(crate) fn of(manifest: &Value) -> Self {
+        let list = match manifest.get("workspaces") {
+            None => return Workspaces::None,
+            Some(Value::Array(list)) => list,
+            Some(Value::Object(map)) => match map.get("packages") {
+                None => return Workspaces::None,
+                Some(Value::Array(list)) => list,
+                Some(_) => return Workspaces::Malformed(Vec::new()),
+            },
+            Some(_) => return Workspaces::Malformed(Vec::new()),
+        };
+        let strings: Vec<String> = list
+            .iter()
+            .filter_map(Value::as_str)
+            .map(str::to_owned)
+            .collect();
+        if strings.len() == list.len() {
+            Workspaces::Patterns(strings)
+        } else {
+            Workspaces::Malformed(strings)
+        }
+    }
+
+    /// The field of a `package.json` text (a leading BOM is ignored);
+    /// `None` when the text is not JSON.
+    pub(crate) fn parse(package_json: &str) -> Option<Self> {
+        serde_json::from_str(super::text::strip_bom(package_json))
+            .ok()
+            .map(|doc| Self::of(&doc))
+    }
+
+    /// The patterns a widening reader takes: every string item of a
+    /// declared field, well-formed or not; `None` when no field is
+    /// declared. A refusal, an extra crawl root or a warning that reads a
+    /// malformed root's patterns errs on the side the package managers'
+    /// failure cannot make unsafe.
+    pub(crate) fn into_listed(self) -> Option<Vec<String>> {
+        match self {
+            Workspaces::None => None,
+            Workspaces::Patterns(list) | Workspaces::Malformed(list) => Some(list),
+        }
+    }
+
+    /// [`Self::parse`] then [`Self::into_listed`]: the patterns a widening
+    /// reader takes from a `package.json` text.
+    pub(crate) fn listed_patterns(package_json: &str) -> Option<Vec<String>> {
+        Self::parse(package_json)?.into_listed()
+    }
+
+    /// Whether the field names at least one pattern ([`Self::into_listed`]
+    /// non-empty).
+    pub(crate) fn lists_any(&self) -> bool {
+        matches!(
+            self,
+            Workspaces::Patterns(list) | Workspaces::Malformed(list) if !list.is_empty()
+        )
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn strings(items: &[&str]) -> Vec<String> {
+        items.iter().map(|s| s.to_string()).collect()
+    }
+
+    #[test]
+    fn classifies_every_shape() {
+        let cases: &[(&str, Workspaces)] = &[
+            (r#"{"name":"x"}"#, Workspaces::None),
+            (r#"{"workspaces":{"nohoist":["**"]}}"#, Workspaces::None),
+            (r#"{"workspaces":[]}"#, Workspaces::Patterns(Vec::new())),
+            (
+                r#"{"workspaces":["a/*","!a/skip","b"]}"#,
+                Workspaces::Patterns(strings(&["a/*", "!a/skip", "b"])),
+            ),
+            (
+                r#"{"workspaces":{"packages":["a/*"],"nohoist":["**"]}}"#,
+                Workspaces::Patterns(strings(&["a/*"])),
+            ),
+            (
+                r#"{"workspaces":["a",1]}"#,
+                Workspaces::Malformed(strings(&["a"])),
+            ),
+            (
+                r#"{"workspaces":{"packages":[null,"b"]}}"#,
+                Workspaces::Malformed(strings(&["b"])),
+            ),
+            (r#"{"workspaces":null}"#, Workspaces::Malformed(Vec::new())),
+            (r#"{"workspaces":"a"}"#, Workspaces::Malformed(Vec::new())),
+            (
+                r#"{"workspaces":{"packages":"a"}}"#,
+                Workspaces::Malformed(Vec::new()),
+            ),
+        ];
+        for (text, want) in cases {
+            assert_eq!(Workspaces::parse(text).as_ref(), Some(want), "{text}");
+        }
+    }
+
+    #[test]
+    fn parse_ignores_a_bom_and_rejects_non_json() {
+        assert_eq!(
+            Workspaces::parse("\u{feff}{\"workspaces\":[\"a\"]}"),
+            Some(Workspaces::Patterns(strings(&["a"])))
+        );
+        assert_eq!(Workspaces::parse("not json"), None);
+    }
+
+    #[test]
+    fn widening_readers_keep_a_malformed_fields_strings() {
+        assert_eq!(Workspaces::None.into_listed(), None);
+        assert_eq!(
+            Workspaces::Malformed(strings(&["a"])).into_listed(),
+            Some(strings(&["a"]))
+        );
+        assert!(Workspaces::Malformed(strings(&["a"])).lists_any());
+        assert!(!Workspaces::Malformed(Vec::new()).lists_any());
+        assert!(!Workspaces::Patterns(Vec::new()).lists_any());
+        assert!(!Workspaces::None.lists_any());
+    }
+}

crates/socket-patch-core/src/hosted/governing_root.rs

@@ -24,6 +24,7 @@ use std::path::{Path, PathBuf};
 
 use crate::constants::npm_family::{BUN_LOCK, BUN_LOCKB, VLT_LOCK};
 use crate::formats::governing_locks::{npm_lock_files, NpmLockFamily};
+use crate::formats::package_json::Workspaces;
 use crate::formats::pnpm::workspace::read_package_globs;
 use crate::formats::text::strip_bom;
 use crate::formats::yarn::berry_gates::YARN_LOCK;
@@ -649,7 +650,7 @@ async fn listing_workspace(base: &Path, dir: &Path) -> Option<PathBuf> {
         let listed = ancestor.join("rush.json").exists()
             || package_json
                 .ok()
-                .and_then(|text| workspace_patterns(&text))
+                .and_then(|text| Workspaces::listed_patterns(&text))
                 .is_some_and(|patterns| lists(&patterns, &rel))
             || read_regular_to_string(&ancestor.join(VLT_JSON))
                 .await
@@ -730,7 +731,7 @@ async fn package_json_workspace_root(root: &Path) -> Option<(PathBuf, Vec<&'stat
         let Ok(text) = read_regular_to_string(&ancestor.join("package.json")).await else {
             continue;
         };
-        let Some(patterns) = workspace_patterns(&text) else {
+        let Some(patterns) = Workspaces::listed_patterns(&text) else {
             continue;
         };
         let Ok(rel) = member.strip_prefix(ancestor) else {
@@ -1202,27 +1203,6 @@ fn vlt_workspace_patterns(vlt_json: &str) -> Option<Vec<Vec<String>>> {
     })
 }
 
-/// The `workspaces` patterns of a `package.json`: the array form (npm,
-/// yarn, Bun) or the object form's `packages` array (yarn classic's
-/// `nohoist` shape, Bun's catalogs shape). `None` when the field is absent
-/// or the manifest does not parse.
-pub(crate) fn workspace_patterns(package_json: &str) -> Option<Vec<String>> {
-    let text = strip_bom(package_json);
-    let doc: serde_json::Value = serde_json::from_str(text).ok()?;
-    let field = doc.get("workspaces")?;
-    let list = match field {
-        serde_json::Value::Array(list) => list,
-        serde_json::Value::Object(map) => map.get("packages")?.as_array()?,
-        _ => return None,
-    };
-    Some(
-        list.iter()
-            .filter_map(|v| v.as_str())
-            .map(str::to_string)
-            .collect(),
-    )
-}
-
 async fn npmrc_lockfile_dir(root: &Path) -> Option<String> {
     let npmrc = read_regular_to_string(&root.join(".npmrc")).await.ok()?;
     npmrc_top_level_value(&npmrc, "lockfile-dir")
@@ -2553,11 +2533,14 @@ mod tests {
                 vlt_workspace_patterns(&text),
                 Some(vec![vec!["a/*".to_string()]])
             );
-            assert_eq!(workspace_patterns(&text), Some(vec!["a/*".to_string()]));
+            assert_eq!(
+                Workspaces::listed_patterns(&text),
+                Some(vec!["a/*".to_string()])
+            );
         }
         let text = format!("\u{feff}\u{feff}{json}");
         assert_eq!(vlt_workspace_patterns(&text), None);
-        assert_eq!(workspace_patterns(&text), None);
+        assert_eq!(Workspaces::listed_patterns(&text), None);
     }
 
     /// #942: vlt's `workspaces` in `vlt.json` is a string, an array or an
@@ -3493,20 +3476,6 @@ mod tests {
         assert!(refusal.message.contains(VLT_LOCK), "{}", refusal.message);
     }
 
-    #[test]
-    fn workspace_patterns_reads_both_field_shapes() {
-        assert_eq!(
-            workspace_patterns(r#"{"workspaces":["a/*","b"]}"#),
-            Some(vec!["a/*".to_string(), "b".to_string()])
-        );
-        assert_eq!(
-            workspace_patterns("\u{feff}{\"workspaces\":{\"packages\":[\"a/*\"]}}"),
-            Some(vec!["a/*".to_string()])
-        );
-        assert_eq!(workspace_patterns(r#"{"name":"x"}"#), None);
-        assert_eq!(workspace_patterns("not json"), None);
-    }
-
     /// #417: a cargo workspace member is refused with the vendored code; the
     /// root and a standalone crate are not.
     #[tokio::test]

crates/socket-patch-core/src/patch/redirect/yarnrc.rs

@@ -667,7 +667,7 @@ pub fn yarn_classic_workspace_members(root: &Path) -> Vec<PathBuf> {
     }
     let Some(patterns) = read_regular_to_string_sync(&root.join("package.json"))
         .ok()
-        .and_then(|text| crate::hosted::governing_root::workspace_patterns(&text))
+        .and_then(|text| crate::formats::package_json::Workspaces::listed_patterns(&text))
         .filter(|patterns| !patterns.is_empty())
     else {
         return Vec::new();