Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
341 changes: 341 additions & 0 deletions crates/turborepo-lockfiles/src/npm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,168 @@ impl NpmLockfile {
pruned.insert(new_key, pkg);
}
}

// Promoting the package changed its position in the tree, so any of
// its transitive deps that were resolved through workspace-nested
// siblings (e.g. `apps/app-a/node_modules/mime`) are no longer
// reachable from the new hoisted position. Walk the promoted
// package's dependency closure and relocate any stranded versions.
let mut visited = std::collections::HashSet::new();
Self::relocate_stranded_closure(
pruned,
original_packages,
&nested_key,
&hoisted_key,
&mut visited,
);
}
}

/// After a workspace-nested package has been promoted to the hoisted
/// position, its transitive dependencies that previously resolved through
/// workspace-nested siblings can become unreachable: Node's resolution only
/// walks upward, so a package now at `node_modules/send` cannot reach
/// `apps/app-a/node_modules/mime`.
///
/// This walks the promoted package's dependency closure (using the original
/// lockfile as the source of truth for which version each dependency must
/// resolve to) and, for every dependency that no longer resolves to the
/// correct version, copies that version from the original lockfile to a
/// position the promoted package can reach — hoisted to the root slot when
/// it's free, otherwise nested directly under the promoted package.
/// See https://github.com/vercel/turborepo/issues/13109
fn relocate_stranded_closure(
pruned: &mut HashMap<String, NpmPackage>,
original: &HashMap<String, NpmPackage>,
original_key: &str,
new_key: &str,
visited: &mut std::collections::HashSet<String>,
) {
if !visited.insert(new_key.to_string()) {
return;
}

// The authoritative dependency list comes from the original lockfile.
let Some(pkg) = original.get(original_key) else {
return;
};
let dep_names: Vec<String> = pkg.dep_keys().cloned().collect();

for dep in dep_names {
// The version this dependency resolved to in the original tree.
let Some((orig_dep_key, Some(orig_version))) =
Self::resolve_in_map(original, original_key, &dep)
else {
// No concrete resolution (missing/optional dep or a workspace
// link without a version) — nothing to relocate.
continue;
};

// What it currently resolves to from the new (pruned) position.
if let Some((pruned_dep_key, Some(pruned_version))) =
Self::resolve_in_map(pruned, new_key, &dep)
&& pruned_version == orig_version
{
// Already reachable and correct — descend to validate the
// dependency's own closure.
Self::relocate_stranded_closure(
pruned,
original,
&orig_dep_key,
&pruned_dep_key,
visited,
);
continue;
}

// Missing or wrong version. Place the correct version where the
// promoted package can reach it: hoist to the root slot if free,
// otherwise nest directly under the promoted package.
let hoisted = format!("node_modules/{dep}");
let placement = if pruned.contains_key(&hoisted) {
format!("{new_key}/node_modules/{dep}")
Comment thread
vercel[bot] marked this conversation as resolved.
} else {
hoisted
};

// Copy the dependency and its nested subtree from the original
// lockfile to the new placement.
if let Some(entry) = original.get(&orig_dep_key) {
pruned.insert(placement.clone(), entry.clone());
}
let orig_sub_prefix = format!("{orig_dep_key}/node_modules/");
let new_sub_prefix = format!("{placement}/node_modules/");
for (k, v) in original.iter() {
if let Some(rest) = k.strip_prefix(&orig_sub_prefix) {
pruned.insert(format!("{new_sub_prefix}{rest}"), v.clone());
}
}

// Remove the original nested location only if no remaining package
// still resolves to it. Sibling consumers in the same workspace can
// share that nested copy even after this package is promoted.
if orig_dep_key != placement
&& !Self::is_resolved_by_any_consumer(pruned, &orig_dep_key)
{
pruned.remove(&orig_dep_key);
let strand_prefix = format!("{orig_dep_key}/node_modules/");
let strays: Vec<String> = pruned
.keys()
.filter(|k| k.starts_with(&strand_prefix))
.cloned()
.collect();
for s in strays {
pruned.remove(&s);
}
}

// Descend into the relocated dependency.
Self::relocate_stranded_closure(pruned, original, &orig_dep_key, &placement, visited);
}
}

fn is_resolved_by_any_consumer(packages: &HashMap<String, NpmPackage>, dep_key: &str) -> bool {
let Some(dep_name) = dep_key.rsplit_once("node_modules/").map(|(_, name)| name) else {
return false;
};

packages.iter().any(|(consumer_key, pkg)| {
pkg.dep_keys().any(|dep| {
dep == dep_name
&& Self::resolve_in_map(packages, consumer_key, dep)
.is_some_and(|(resolved_key, _)| resolved_key == dep_key)
})
})
}

/// Resolve a dependency name within an arbitrary packages map by walking up
/// the node_modules hierarchy from `key`, mirroring Node's resolution.
/// Returns the resolved key and its version (if the entry has one).
fn resolve_in_map(
packages: &HashMap<String, NpmPackage>,
key: &str,
dep: &str,
) -> Option<(String, Option<String>)> {
// First candidate: nested directly under the current package.
let nested = format!("{key}/node_modules/{dep}");
if let Some(entry) = packages.get(&nested) {
return Some((nested, entry.version.clone()));
}

// Walk up the node_modules hierarchy.
let mut curr = Some(key);
while let Some(k) = curr {
let parent = Self::npm_path_parent(k);
let candidate = match parent {
Some(p) => format!("{p}node_modules/{dep}"),
None => format!("node_modules/{dep}"),
};
if let Some(entry) = packages.get(&candidate) {
return Some((candidate, entry.version.clone()));
}
curr = parent;
}
None
}

/// Resolve a dependency name by walking up the node_modules hierarchy,
Expand Down Expand Up @@ -826,6 +987,186 @@ mod test {
);
}

// Regression test for https://github.com/vercel/turborepo/issues/13109
//
// When the full monorepo has two incompatible versions of a shared
// transitive dep — one hoisted to root, the other workspace-nested — and
// the workspace-nested package gets promoted to root during pruning, its
// workspace-nested transitive deps (siblings under the workspace's
// node_modules, not under the package itself) must follow it so they remain
// reachable.
//
// Here app-1 forces send@1.2.1 + mime@3.0.0 (root devDep) to root, while
// app-2 needs send@0.17.2 which requires mime@1.6.0. mime@1.6.0 lives at
// `apps/app-2/node_modules/mime`. Pruning to app-2 drops send@1.2.1 and
// promotes send@0.17.2 to `node_modules/send`; mime@1.6.0 must move to a
// position where the promoted send can resolve it (here nested under send,
// since `node_modules/mime` is still occupied by the root devDep 3.0.0).
// The original nested copy must remain when another app-2 dependency still
// resolves to it.
#[test]
fn test_subgraph_relocates_stranded_promoted_deps() {
let json = r#"{
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "monorepo",
"workspaces": ["apps/*"],
"devDependencies": {
"legacy": "^2.0.0",
"mime": "^3.0.0"
}
},
"node_modules/app-1": {
"resolved": "apps/app-1",
"link": true
},
"node_modules/app-2": {
"resolved": "apps/app-2",
"link": true
},
"node_modules/destroy": {
"version": "1.0.4"
},
"node_modules/mime": {
"version": "3.0.0"
},
"node_modules/legacy": {
"version": "2.0.0"
},
"node_modules/send": {
"version": "1.2.1",
"dependencies": { "encodeurl": "^2.0.0" }
},
"node_modules/send/node_modules/encodeurl": {
"version": "2.0.0"
},
"node_modules/encodeurl": {
"version": "1.0.2"
},
"apps/app-1": {
"version": "1.0.0",
"dependencies": { "send": "^1.0.0" }
},
"apps/app-2": {
"version": "1.0.0",
"dependencies": {
"legacy": "^1.0.0",
"send": "^0.17.0"
}
},
"apps/app-2/node_modules/legacy": {
"version": "1.0.0",
"dependencies": { "mime": "1.6.0" }
},
"apps/app-2/node_modules/mime": {
"version": "1.6.0"
},
"apps/app-2/node_modules/send": {
"version": "0.17.2",
"dependencies": {
"destroy": "~1.0.4",
"encodeurl": "~1.0.2",
"mime": "1.6.0"
}
}
}
}"#;

let lockfile = NpmLockfile::load(json.as_bytes()).unwrap();

// Pruning to app-2: the root workspace keeps its mime@3.0.0 devDep and
// app-2 pulls in send@0.17.2's closure (mime@1.6.0 nested, destroy and
// encodeurl@1.0.2 hoisted).
let workspace_packages = vec!["apps/app-2".to_string()];
let packages = vec![
"node_modules/destroy".to_string(),
"node_modules/legacy".to_string(),
"node_modules/mime".to_string(),
"node_modules/encodeurl".to_string(),
"apps/app-2/node_modules/legacy".to_string(),
"apps/app-2/node_modules/mime".to_string(),
"apps/app-2/node_modules/send".to_string(),
];

let pruned = lockfile.subgraph(&workspace_packages, &packages).unwrap();
let encoded = pruned.encode().unwrap();
let reparsed: NpmLockfile = NpmLockfile::load(&encoded).unwrap();

// send@0.17.2 was promoted to the hoisted slot (the root version 1.2.1
// was only needed by the now-pruned app-1).
assert_eq!(
reparsed
.packages
.get("node_modules/send")
.and_then(|p| p.version.as_deref()),
Some("0.17.2"),
"send@0.17.2 should be promoted to node_modules/send"
);

// mime@1.6.0 must be reachable from the promoted send. node_modules/mime
// is still occupied by the root devDep (3.0.0), so it must be nested
// directly under send rather than stranded under apps/app-2.
assert_eq!(
reparsed
.packages
.get("node_modules/send/node_modules/mime")
.and_then(|p| p.version.as_deref()),
Some("1.6.0"),
"send@0.17.2's mime@1.6.0 must be relocated where send can resolve it"
);
assert_eq!(
reparsed
.packages
.get("apps/app-2/node_modules/mime")
.and_then(|p| p.version.as_deref()),
Some("1.6.0"),
"the shared apps/app-2/node_modules/mime copy must remain for sibling consumers"
);
assert_eq!(
reparsed
.packages
.get("apps/app-2/node_modules/legacy")
.and_then(|p| p.version.as_deref()),
Some("1.0.0"),
"legacy@1.0.0 should remain nested and resolve apps/app-2/node_modules/mime"
);

// The root devDependency mime@3.0.0 must be preserved untouched.
assert_eq!(
reparsed
.packages
.get("node_modules/mime")
.and_then(|p| p.version.as_deref()),
Some("3.0.0"),
"root devDep mime@3.0.0 should be preserved"
);

// Hoisted deps that already resolve correctly must stay put.
assert!(
reparsed.packages.contains_key("node_modules/destroy"),
"hoisted destroy@1.0.4 should be reachable from send"
);

// No part of the old root send@1.2.1 subtree should linger.
assert!(
!reparsed
.packages
.contains_key("node_modules/send/node_modules/encodeurl"),
"old send@1.2.1's nested encodeurl@2.0.0 should be gone"
);
// send@0.17.2 wants encodeurl@~1.0.2, which is the hoisted 1.0.2.
assert_eq!(
reparsed
.packages
.get("node_modules/encodeurl")
.and_then(|p| p.version.as_deref()),
Some("1.0.2"),
"hoisted encodeurl@1.0.2 (what send@0.17.2 needs) should remain"
);
}

#[test]
fn test_turbo_version_rejects_non_semver() {
// Malicious version strings that could be used for RCE via npx should be
Expand Down
9 changes: 8 additions & 1 deletion lockfile-tests/check-lockfiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ interface FixtureMeta {
pruneTargets?: string[];
/** Run `turbo prune --docker` and validate `out/json`. */
docker?: boolean;
/**
* Additionally assert that every package in the pruned lockfile can resolve
* its declared dependencies (via `npm ls`). Catches stranded transitive deps
* that `npm ci --dry-run` silently accepts. npm fixtures only.
*/
validateResolution?: boolean;
/** Workspace names where pruning or lockfile validation is known to fail. */
expectedFailures?: string[];
}
Expand Down Expand Up @@ -211,7 +217,8 @@ function buildTestCases(
filepath: fixture.dir,
packageManager: fixture.meta.packageManager,
lockfileName: fixture.meta.lockfileName,
packageManagerVersion: fixture.meta.packageManagerVersion
packageManagerVersion: fixture.meta.packageManagerVersion,
validateResolution: fixture.meta.validateResolution
},
targetWorkspace: { name: target },
label,
Expand Down
6 changes: 6 additions & 0 deletions lockfile-tests/fixtures/issue-13109/apps/app-1/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "app-1",
"dependencies": {
"send": "^1.0.0"
}
}
6 changes: 6 additions & 0 deletions lockfile-tests/fixtures/issue-13109/apps/app-2/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "app-2",
"dependencies": {
"send": "^0.17.0"
}
}
Loading
Loading