From d803c390928cbb07451f92d660a2f3587d4eb359 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 04:24:56 +0000 Subject: [PATCH 1/3] Start fix for #1900 Assisted-by: Claude Code:claude-opus-5-5 From 6d9f5e323b6d9bbffd84a8957d335590b3315ec4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 04:31:44 +0000 Subject: [PATCH 2/3] Read package.json workspaces one way (#1900) Five places parsed a package.json `workspaces` field by hand and disagreed on what a malformed field means: a list with a non-string item, `null`, or the wrong type. Depending on the caller, the same manifest made the Bun member walk read no members, made patch discovery skip the bad item silently, or made the yarn workspace warning and the upgrade hint think workspaces were declared. npm 10.9, yarn 1.22.22 and Bun 1.4.4 all refuse to install a list with a non-string item, and npm and Bun also refuse `null`, a string, and a non-array `packages`. A new formats::package_json::Workspaces reader classifies the field once (absent, patterns, or malformed), and every caller reads it: - governing-root refusals and the npm crawler's extra roots keep reading a malformed field's string items, so they still refuse or crawl on the safe side; - the Bun member walk treats a malformed field as unreadable and falls back to bun.lock; - patch discovery records the malformed manifest as unknown, so a project patch cannot be ruled out silently; - the yarn classic warning and the upgrade hint need at least one pattern string. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 30 ++-- crates/socket-patch-core/src/formats/mod.rs | 1 + .../src/formats/package_json.rs | 156 ++++++++++++++++++ .../src/hosted/governing_root.rs | 23 +-- .../src/patch/user_patches.rs | 21 +-- .../socket-patch-core/src/update/channel.rs | 30 +++- crates/socket-patch-core/src/vendor/mod.rs | 19 +-- 7 files changed, 223 insertions(+), 57 deletions(-) create mode 100644 crates/socket-patch-core/src/formats/package_json.rs diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 2d6acefd5..6cb7a0829 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -915,20 +915,15 @@ fn bun_workspace_pattern_members_sync(root: &Path) -> Option> { 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, + // Bun refuses to install from a malformed field (#1900), so it names + // no member set. + let patterns = match crate::formats::package_json::Workspaces::parse(&text)? { + crate::formats::package_json::Workspaces::None => Vec::new(), + crate::formats::package_json::Workspaces::Patterns(patterns) => patterns, + crate::formats::package_json::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 +8125,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 diff --git a/crates/socket-patch-core/src/formats/mod.rs b/crates/socket-patch-core/src/formats/mod.rs index c738c0be2..3f8fcb874 100644 --- a/crates/socket-patch-core/src/formats/mod.rs +++ b/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; diff --git a/crates/socket-patch-core/src/formats/package_json.rs b/crates/socket-patch-core/src/formats/package_json.rs new file mode 100644 index 000000000..be2cf4561 --- /dev/null +++ b/crates/socket-patch-core/src/formats/package_json.rs @@ -0,0 +1,156 @@ +//! 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. +//! The object form without `packages` installs under the two package +//! managers that write it, so it declares no patterns. + +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), + /// A field the package managers refuse to install from (see the + /// module table), with the string items it does hold. + Malformed(Vec), +} + +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 = 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 { + 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> { + match self { + Workspaces::None => None, + Workspaces::Patterns(list) | Workspaces::Malformed(list) => Some(list), + } + } + + /// 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 { + 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()); + } +} diff --git a/crates/socket-patch-core/src/hosted/governing_root.rs b/crates/socket-patch-core/src/hosted/governing_root.rs index 8d3d37075..dd1e8f54a 100644 --- a/crates/socket-patch-core/src/hosted/governing_root.rs +++ b/crates/socket-patch-core/src/hosted/governing_root.rs @@ -20,6 +20,7 @@ //! directory (its ancestors), so it only runs over the disk; an in-memory //! project is the host's whole file set and has no ancestors. +use crate::formats::package_json::Workspaces; use std::path::{Path, PathBuf}; use crate::constants::npm_family::{BUN_LOCK, BUN_LOCKB, VLT_LOCK}; @@ -1202,25 +1203,13 @@ fn vlt_workspace_patterns(vlt_json: &str) -> Option>> { }) } -/// 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 +/// The `workspaces` patterns of a `package.json` as a widening reader +/// takes them ([`Workspaces::into_listed`]): a root whose field is +/// malformed still governs the members its string items name, so the +/// refusal errs toward naming the root. `None` when no field is declared /// or the manifest does not parse. pub(crate) fn workspace_patterns(package_json: &str) -> Option> { - 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(), - ) + Workspaces::parse(package_json)?.into_listed() } async fn npmrc_lockfile_dir(root: &Path) -> Option { diff --git a/crates/socket-patch-core/src/patch/user_patches.rs b/crates/socket-patch-core/src/patch/user_patches.rs index 0ee7c9d2d..f1d6cdee9 100644 --- a/crates/socket-patch-core/src/patch/user_patches.rs +++ b/crates/socket-patch-core/src/patch/user_patches.rs @@ -4,6 +4,7 @@ //! yet, and need not touch the same file as the Socket fix. Never execute hooks //! or fetch patch URLs to establish ownership. +use crate::formats::package_json::Workspaces; use std::collections::{BTreeSet, VecDeque}; use std::path::Path; use std::sync::LazyLock; @@ -90,17 +91,13 @@ impl UserPatchIndex { } } } - let workspaces = value["workspaces"] - .as_array() - .or_else(|| value["workspaces"]["packages"].as_array()); - if let Some(workspaces) = workspaces { - globs.extend( - workspaces - .iter() - .filter_map(Value::as_str) - .map(str::to_owned), - ); + let workspaces = Workspaces::of(&value); + if matches!(workspaces, Workspaces::Malformed(_)) { + // #1900: the package managers refuse this field, so + // which members install (and patch) cannot be told. + index.unknown("npm", &dir.join("package.json"), "malformed workspaces"); } + globs.extend(workspaces.into_listed().into_iter().flatten()); index.patch_package(&dir, &value).await; } else { index.patch_directory(&dir.join("patches"), false).await; @@ -1207,6 +1204,10 @@ mod tests { r#"{"scripts":{"postinstall":"patch-package --patch-dir $PATCH_DIR"}}"#, r#"{"scripts":{"postinstall":"patch-package --patch-dir missing"}}"#, "{broken json", + // #1900: npm, yarn and Bun all refuse a `workspaces` list with + // a non-string item, and npm and Bun refuse `null`. + r#"{"workspaces": ["a", 1]}"#, + r#"{"workspaces": null}"#, ] { let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "package.json", text); diff --git a/crates/socket-patch-core/src/update/channel.rs b/crates/socket-patch-core/src/update/channel.rs index 7794e6526..d2ecca5d4 100644 --- a/crates/socket-patch-core/src/update/channel.rs +++ b/crates/socket-patch-core/src/update/channel.rs @@ -386,16 +386,10 @@ fn read_package_json(dir: &Path) -> Option { serde_json::from_str(crate::formats::text::strip_bom(&text)).ok() } -/// Whether a `package.json` declares workspaces: a non-empty array, or the -/// `{ "packages": [...] }` form with a non-empty `packages`. +/// Whether a `package.json` declares workspaces: its `workspaces` field +/// names at least one pattern ([`Workspaces::lists_any`](crate::formats::package_json::Workspaces::lists_any)). fn declares_workspaces(pkg: &serde_json::Value) -> bool { - let globs = match pkg.get("workspaces") { - Some(serde_json::Value::Object(obj)) => obj.get("packages"), - other => other, - }; - globs - .and_then(|g| g.as_array()) - .is_some_and(|g| !g.is_empty()) + crate::formats::package_json::Workspaces::of(pkg).lists_any() } /// Whether a `package.json` itself declares socket-patch in any dependency @@ -577,6 +571,24 @@ mod tests { upgrade_hint_for(channel, exe, &ChannelEnv::default()) } + /// #1900: `workspaces` is read through the shared package.json + /// reader, so a field that names no pattern string declares none. + #[test] + fn declares_workspaces_reads_the_shared_field_rule() { + for (text, want) in [ + (r#"{"workspaces":["a"]}"#, true), + (r#"{"workspaces":{"packages":["a"]}}"#, true), + (r#"{"workspaces":["a",1]}"#, true), + (r#"{"workspaces":[1]}"#, false), + (r#"{"workspaces":[]}"#, false), + (r#"{"workspaces":null}"#, false), + (r#"{"name":"x"}"#, false), + ] { + let pkg: serde_json::Value = serde_json::from_str(text).unwrap(); + assert_eq!(declares_workspaces(&pkg), want, "{text}"); + } + } + /// [`channel_label_for`] with no yarn `global-folder` configured. fn label(channel: InstallChannel, exe: &Path) -> &'static str { channel_label_for(channel, exe, &ChannelEnv::default()) diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index 481effdb7..be645cd0d 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -232,15 +232,7 @@ pub fn yarn_classic_workspace_member_risk(project_root: &Path) -> Option Date: Mon, 2 Nov 2026 05:10:26 +0000 Subject: [PATCH 3/3] Keep patch search quiet on bad workspaces Review follow-up for #1900. Patch discovery no longer flags every npm package as possibly user-patched when a package.json carries a malformed `workspaces` field. pnpm ignores the field and yarn 1 installs some malformed shapes, and the string patterns are still walked, so the flag added no safety. Callers now read the shared reader directly instead of going through a wrapper in the hosted module, and the module doc says which readers widen and which treat a malformed field as unreadable. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/npm_crawler.rs | 12 ++--- .../src/formats/package_json.rs | 19 ++++++-- .../src/hosted/governing_root.rs | 36 ++++----------- .../src/patch/redirect/yarnrc.rs | 2 +- .../src/patch/user_patches.rs | 45 ++++++++++++++----- 5 files changed, 65 insertions(+), 49 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/npm_crawler.rs b/crates/socket-patch-core/src/crawlers/npm_crawler.rs index 6cb7a0829..7a24fd60c 100644 --- a/crates/socket-patch-core/src/crawlers/npm_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/npm_crawler.rs @@ -111,7 +111,7 @@ fn skipped_members_of(root: &Path) -> Vec { 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,12 +915,14 @@ fn bun_workspace_pattern_members_sync(root: &Path) -> Option> { Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Some(Vec::new()), Err(_) => 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 crate::formats::package_json::Workspaces::parse(&text)? { - crate::formats::package_json::Workspaces::None => Vec::new(), - crate::formats::package_json::Workspaces::Patterns(patterns) => patterns, - crate::formats::package_json::Workspaces::Malformed(_) => return None, + 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 { diff --git a/crates/socket-patch-core/src/formats/package_json.rs b/crates/socket-patch-core/src/formats/package_json.rs index be2cf4561..dd57cbe03 100644 --- a/crates/socket-patch-core/src/formats/package_json.rs +++ b/crates/socket-patch-core/src/formats/package_json.rs @@ -15,9 +15,16 @@ //! | `{"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. -//! The object form without `packages` installs under the two package -//! managers that write it, so it declares no patterns. +//! [`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; @@ -80,6 +87,12 @@ impl Workspaces { } } + /// [`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> { + 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 { diff --git a/crates/socket-patch-core/src/hosted/governing_root.rs b/crates/socket-patch-core/src/hosted/governing_root.rs index dd1e8f54a..4f7a88cae 100644 --- a/crates/socket-patch-core/src/hosted/governing_root.rs +++ b/crates/socket-patch-core/src/hosted/governing_root.rs @@ -20,11 +20,11 @@ //! directory (its ancestors), so it only runs over the disk; an in-memory //! project is the host's whole file set and has no ancestors. -use crate::formats::package_json::Workspaces; 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; @@ -650,7 +650,7 @@ async fn listing_workspace(base: &Path, dir: &Path) -> Option { 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 @@ -731,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 { @@ -1203,15 +1203,6 @@ fn vlt_workspace_patterns(vlt_json: &str) -> Option>> { }) } -/// The `workspaces` patterns of a `package.json` as a widening reader -/// takes them ([`Workspaces::into_listed`]): a root whose field is -/// malformed still governs the members its string items name, so the -/// refusal errs toward naming the root. `None` when no field is declared -/// or the manifest does not parse. -pub(crate) fn workspace_patterns(package_json: &str) -> Option> { - Workspaces::parse(package_json)?.into_listed() -} - async fn npmrc_lockfile_dir(root: &Path) -> Option { let npmrc = read_regular_to_string(&root.join(".npmrc")).await.ok()?; npmrc_top_level_value(&npmrc, "lockfile-dir") @@ -2542,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 @@ -3482,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] diff --git a/crates/socket-patch-core/src/patch/redirect/yarnrc.rs b/crates/socket-patch-core/src/patch/redirect/yarnrc.rs index 5e40f0f17..2c679323b 100644 --- a/crates/socket-patch-core/src/patch/redirect/yarnrc.rs +++ b/crates/socket-patch-core/src/patch/redirect/yarnrc.rs @@ -667,7 +667,7 @@ pub fn yarn_classic_workspace_members(root: &Path) -> Vec { } 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(); diff --git a/crates/socket-patch-core/src/patch/user_patches.rs b/crates/socket-patch-core/src/patch/user_patches.rs index f1d6cdee9..3bf200a29 100644 --- a/crates/socket-patch-core/src/patch/user_patches.rs +++ b/crates/socket-patch-core/src/patch/user_patches.rs @@ -4,7 +4,6 @@ //! yet, and need not touch the same file as the Socket fix. Never execute hooks //! or fetch patch URLs to establish ownership. -use crate::formats::package_json::Workspaces; use std::collections::{BTreeSet, VecDeque}; use std::path::Path; use std::sync::LazyLock; @@ -12,6 +11,7 @@ use std::sync::LazyLock; use regex::Regex; use serde_json::Value; +use crate::formats::package_json::Workspaces; use crate::formats::text::strip_bom; use crate::utils::fs::read_regular_to_string; use crate::utils::npm_range::npm_range_admits; @@ -91,13 +91,9 @@ impl UserPatchIndex { } } } - let workspaces = Workspaces::of(&value); - if matches!(workspaces, Workspaces::Malformed(_)) { - // #1900: the package managers refuse this field, so - // which members install (and patch) cannot be told. - index.unknown("npm", &dir.join("package.json"), "malformed workspaces"); - } - globs.extend(workspaces.into_listed().into_iter().flatten()); + // A malformed field still names its string items (#1900): + // walking them can only find more patches. + globs.extend(Workspaces::of(&value).into_listed().into_iter().flatten()); index.patch_package(&dir, &value).await; } else { index.patch_directory(&dir.join("patches"), false).await; @@ -1146,6 +1142,35 @@ mod tests { assert!(index.conflicts("pkg:npm/x@1.0.0").is_empty(), "{index:?}"); } + /// #1900: a `workspaces` list with a non-string item is read through + /// the shared package.json reader, which keeps its string patterns, + /// so the members they name are still searched for patches, and + /// unrelated npm packages are not marked as user-patched. + #[tokio::test] + async fn malformed_workspaces_still_walk_their_string_patterns() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "package.json", + r#"{"workspaces":["packages/*", 1]}"#, + ); + write(tmp.path(), "packages/a/package.json", "{}"); + write( + tmp.path(), + "packages/a/patches/one+1.0.0.patch", + "user diff", + ); + let index = UserPatchIndex::discover(tmp.path()).await; + assert!( + !index.conflicts("pkg:npm/one@1.0.0").is_empty(), + "{index:?}" + ); + assert!( + index.conflicts("pkg:npm/other@1.0.0").is_empty(), + "{index:?}" + ); + } + #[tokio::test] async fn dot_dot_workspace_globs_resolve_lexically() { let tmp = tempfile::tempdir().unwrap(); @@ -1204,10 +1229,6 @@ mod tests { r#"{"scripts":{"postinstall":"patch-package --patch-dir $PATCH_DIR"}}"#, r#"{"scripts":{"postinstall":"patch-package --patch-dir missing"}}"#, "{broken json", - // #1900: npm, yarn and Bun all refuse a `workspaces` list with - // a non-string item, and npm and Bun refuse `null`. - r#"{"workspaces": ["a", 1]}"#, - r#"{"workspaces": null}"#, ] { let tmp = tempfile::tempdir().unwrap(); write(tmp.path(), "package.json", text);