Skip to content

fix(parsers): resolve pnpm workspace deps against ancestor lockfile #340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wakemeup0 merged 1 commit into from
Sep 22, 2026
Merged

fix(parsers): resolve pnpm workspace deps against ancestor lockfile #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 86 additions & 7 deletions src/input.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,12 +466,13 @@ fn scan_virtual_files(
&mut builder,
)?,
LockfileRoute::Manifest { parse, lock_names } => {
// The manifest's declared constraints are superseded by any
// sibling lockfile; the parser still receives the first
// present lockfile so it can skip redundant work.
let lock = lock_names
.iter()
.find_map(|name| files.get(&sibling(path, name)));
// The manifest's declared constraints are superseded by the
// nearest covering lockfile — a sibling, or one in an
// ancestor directory for workspace members whose lockfile
// lives at the workspace root (pnpm workspaces, npm/yarn
// workspaces). The parser still receives the first present
// lockfile so it can skip redundant work.
let lock = covering_lockfile(path, lock_names, &files);
parse(path, bytes, lock, &mut builder)?;
}
}
Expand Down Expand Up @@ -1021,6 +1022,28 @@ fn sibling(path: &str, name: &str) -> String {
.map(|(parent, _)| format!("{parent}/{name}"))
.unwrap_or_else(|| name.to_owned())
}

/// Returns the nearest lockfile covering a manifest at `path`: its sibling
/// first, then ancestor directories outward, so a workspace member's
/// manifest is superseded by the workspace-root lockfile that resolved its
/// dependencies.
fn covering_lockfile<'a>(
path: &str,
lock_names: &[&'static str],
files: &'a BTreeMap<String, Vec<u8>>,
) -> Option<&'a Vec<u8>> {
let mut dir = path.rsplit_once('/').map(|(parent, _)| parent);
while let Some(current) = dir {
if let Some(lock) = lock_names
.iter()
.find_map(|name| files.get(&format!("{current}/{name}")))
{
return Some(lock);
}
dir = current.rsplit_once('/').map(|(parent, _)| parent);
}
lock_names.iter().find_map(|name| files.get(*name))
}
fn base_name(path: &str) -> &str {
path.rsplit('/').next().unwrap_or(path)
}
Expand Down Expand Up @@ -1129,7 +1152,7 @@ fn parse_package_json(
/// Returns the concrete purl version for a specifier, stripping pnpm-style
/// `1.2.3(integrity)` annotations, or `None` for empty values and range
/// constraints (`^1.2`, `~1.2`, `>=1`, `1.*`, `a || b`, `git+https://…`).
fn concrete_version_specifier(version: &str) -> Option<String> {
pub(crate) fn concrete_version_specifier(version: &str) -> Option<String> {
let trimmed = version.trim();
let concrete = trimmed.split('(').next().unwrap_or(trimmed).trim_end();
if concrete.is_empty()
Expand Down Expand Up @@ -2279,6 +2302,62 @@ mod tests {
assert!(inventory.components.values().any(|c| c.name == "a"));
}

#[test]
fn workspace_manifest_deps_are_covered_by_ancestor_lockfile() {
// A workspace member's package.json has no sibling lockfile; the
// workspace-root pnpm-lock.yaml resolved its dependencies, so the
// manifest's declared constraints must not become specifier-versioned
// phantom components (#339).
let dir = tempdir().unwrap();
fs::write(
dir.path().join("package.json"),
r#"{"name":"probe","private":true,"dependencies":{"is-odd":"^3.0.1"}}"#,
)
.unwrap();
let web = dir.path().join("apps/web");
fs::create_dir_all(&web).unwrap();
fs::write(
web.join("package.json"),
r#"{"name":"web","private":true,"dependencies":{"is-even":"^1.0.0"}}"#,
)
.unwrap();
fs::write(
dir.path().join("pnpm-lock.yaml"),
concat!(
"lockfileVersion: '9.0'\n",
"\n",
"importers:\n",
" .:\n",
" dependencies:\n",
" is-odd:\n",
" specifier: ^3.0.1\n",
" version: 3.0.1\n",
" apps/web:\n",
" dependencies:\n",
" is-even:\n",
" specifier: ^1.0.0\n",
" version: 1.0.0\n",
"\n",
"packages:\n",
"\n",
" is-odd@3.0.1:\n",
" resolution: {integrity: sha512-x}\n",
"\n",
" is-even@1.0.0:\n",
" resolution: {integrity: sha512-y}\n",
),
)
.unwrap();
let inventory = scan_path(dir.path(), &config()).unwrap();
let mut components: Vec<(&str, &str)> = inventory
.components
.values()
.map(|c| (c.name.as_str(), c.version.as_str()))
.collect();
components.sort_unstable();
assert_eq!(components, [("is-even", "1.0.0"), ("is-odd", "3.0.1")]);
}

#[test]
fn previously_unbounded_parsers_reject_oversized_lockfiles() {
let config = Config {
Expand Down
135 changes: 131 additions & 4 deletions src/parsers/pnpm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use serde_yaml::Value as Yaml;
use super::npm::npm_scope;
use super::{LockComponents, resolve_lock_component, split_descriptor};
use crate::input::{
InputError, InventoryBuilder, entry_bound, malformed, malformed_msg, utf8,
yaml_expansion_within_budget,
InputError, InventoryBuilder, concrete_version_specifier, entry_bound, malformed,
malformed_msg, utf8, yaml_expansion_within_budget,
};
use crate::model::Scope;

Expand Down Expand Up @@ -88,7 +88,13 @@ pub(crate) fn parse_pnpm_lock(
collect_pnpm_deps(&node, entry, &mut pending);
}
}
let package_names: BTreeSet<String> = entries.keys().map(|(name, _)| name.clone()).collect();
let mut package_versions: BTreeMap<String, BTreeSet<String>> = BTreeMap::new();
for (name, version) in entries.keys() {
package_versions
.entry(name.clone())
.or_default()
.insert(version.clone());
}
if let Some(importers) = importers {
for importer in importers.values() {
for (field, scope) in [
Expand All @@ -104,11 +110,26 @@ pub(crate) fn parse_pnpm_lock(
let Some(node) = pnpm_resolved_parts(dep, spec) else {
continue;
};
if !package_names.contains(node.0.as_str()) {
// pnpm 12 writes workspace-importer dependencies as bare
// specifier strings (`is-even: ^1.0.0`); a specifier is a
// constraint, not a resolved version, so it resolves
// against the locked versions of that package name.
let node = match concrete_version_specifier(&node.1) {
Some(version) if pnpm_concrete_version(&version) => node,
_ => match package_versions.get(node.0.as_str()) {
Some(versions) => (node.0, versions.iter().next().unwrap().to_string()),
None => continue,
},
};
if !package_versions.contains_key(node.0.as_str()) {
// Importer-only dependency absent from `packages:`;
// its scope comes from the importer field through the
// reachability pass below.
entries.insert(node.clone(), None);
package_versions
.entry(node.0.clone())
.or_default()
.insert(node.1.clone());
}
roots.push((node, scope));
}
Expand Down Expand Up @@ -420,6 +441,16 @@ fn pnpm_valid_version(version: &str) -> bool {
!version.contains('@') && !version.contains(':') && !version.contains('/')
}

/// A concrete resolved version starts with a digit and has no `x`/`X`
/// wildcard segments; tags (`latest`) and partial ranges (`1.x`) are
/// specifiers that resolve against the lockfile's recorded versions.
fn pnpm_concrete_version(version: &str) -> bool {
version.chars().next().is_some_and(|c| c.is_ascii_digit())
&& !version
.split('.')
.any(|segment| segment.eq_ignore_ascii_case("x"))
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -878,4 +909,100 @@ mod tests {
Err(InputError::Malformed { .. })
));
}

#[test]
fn pnpm_workspace_importer_specifiers_resolve_to_locked_versions() {
// pnpm 12 writes workspace-importer dependencies as bare specifier
// strings; they must resolve against `packages:` instead of becoming
// specifier-versioned phantom components (#339).
let dir = tempdir().unwrap();
fs::write(
dir.path().join("pnpm-lock.yaml"),
concat!(
"---\n",
"lockfileVersion: '9.0'\n",
"\n",
"importers:\n",
" .:\n",
" dependencies:\n",
" is-odd:\n",
" specifier: ^3.0.1\n",
" version: 3.0.1\n",
" apps/web:\n",
" dependencies:\n",
" is-even: ^1.0.0\n",
" tagged: latest\n",
" ranged: 1.x\n",
" missing: ^9.9.9\n",
" devDependencies:\n",
" vitest: ^4.0.0\n",
"\n",
"---\n",
"lockfileVersion: '9.0'\n",
"\n",
"settings:\n",
" autoInstallPeers: true\n",
"\n",
"packages:\n",
"\n",
" is-odd@3.0.1:\n",
" resolution: {integrity: sha512-x}\n",
"\n",
" is-even@1.0.0:\n",
" resolution: {integrity: sha512-y}\n",
"\n",
" tagged@2.0.0:\n",
" resolution: {integrity: sha512-t}\n",
"\n",
" ranged@1.4.2:\n",
" resolution: {integrity: sha512-r}\n",
"\n",
" vitest@4.1.5:\n",
" resolution: {integrity: sha512-v}\n",
"\n",
"snapshots:\n",
"\n",
" is-odd@3.0.1: {}\n",
"\n",
" is-even@1.0.0: {}\n",
"\n",
" tagged@2.0.0: {}\n",
"\n",
" ranged@1.4.2: {}\n",
"\n",
" vitest@4.1.5: {}\n",
),
)
.unwrap();
let inventory = scan_path(dir.path(), &config()).unwrap();
let mut components: Vec<(&str, &str)> = inventory
.components
.values()
.map(|c| (c.name.as_str(), c.version.as_str()))
.collect();
components.sort_unstable();
// Only resolved versions appear: no specifier text becomes a version,
// and an importer specifier with no locked package yields nothing.
assert_eq!(
components,
[
("is-even", "1.0.0"),
("is-odd", "3.0.1"),
("ranged", "1.4.2"),
("tagged", "2.0.0"),
("vitest", "4.1.5"),
]
);
// Specifier-valued roots still classify their resolved package by the
// importer field that referenced them.
let scope_of = |name: &str| {
inventory
.components
.values()
.find(|c| c.name == name)
.map(|c| c.scope)
};
assert_eq!(scope_of("vitest"), Some(Scope::Development));
assert_eq!(scope_of("is-even"), Some(Scope::Runtime));
}
}
Loading