fix(pacquet/git-fetcher): bundle transitive deps of bundled dependencies (#12620)

* fix(git-fetcher): bundle transitive deps of bundled dependencies

The packlist `bundleDependencies` pass only spliced in each
directly-bundled package's own files; it never followed that package's
own dependencies, so a bundled dep's transitive (and hoisted)
dependencies were dropped from the published file set.

Replace the per-dependency recursion with npm-bundled's reachability
walk: seed from the root manifest's `bundleDependencies`, then
transitively pull in every reachable package's `dependencies` and
`optionalDependencies`, resolving each via the node module-resolution
walk-up (nested `node_modules/` first, then ancestor `node_modules/`).
The walk-up is what lets a hoisted transitive dep at the root
`node_modules/` be found and spliced in under its real path. A
visited-set keyed on the canonicalised resolved directory keeps a
diamond from being processed twice and stops dependency cycles.

Ports the upstream pnpm test "bundles transitive dependencies of
bundled dependencies (hoisted)" from
releasing/commands/test/publish/pack.ts, plus nested-wins-over-hoisted
and optionalDependencies/devDependencies coverage.

Resolves https://github.com/pnpm/pnpm/issues/12602

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XXHGos5SaEKKr7pUW4FXga

* docs(git-fetcher): fix stale bundling comment and pin test citation

Address review nits on the transitive-bundling change:

- The comment in `collect_own_files` referred to a `bundleDependencies`
  pass "below", but bundling now lives in the separate
  `collect_bundled_files`; describe the current structure instead.
- Pin the ported-test citation to a commit permalink, per
  pacquet/AGENTS.md's rule that code citations link to a specific SHA.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XXHGos5SaEKKr7pUW4FXga

* fix(git-fetcher): refuse bundled deps whose real path escapes the package

A `node_modules/<name>` entry can be a symlink pointing outside the
package (a sibling directory or an absolute host path). The name still
passes `is_safe_bundle_name` because it is a single safe segment, so the
walk-up resolves it and `collect_own_files` walks the symlink target —
splicing host files into the published set. The git/directory fetchers
import untrusted packages, making this an information-disclosure path.

Check each resolved dependency's canonical (symlink-resolved) path
against the canonical package root and refuse anything that escapes,
falling back to a lexical comparison when canonicalization is
unavailable. Reuses the canonicalize call already used for cycle dedup.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XXHGos5SaEKKr7pUW4FXga

* fix(git-fetcher): fail closed when a bundled dep's real path is unverifiable

The symlink-escape guard fell back to a lexical path check whenever
`fs::canonicalize` returned `None` for the dependency. A symlink under
`node_modules/` is lexically inside the package even when its target
escapes, so that fallback was fail-open. When the root canonicalizes but
the dependency does not, refuse the dependency instead: a genuine
dependency always canonicalizes here, since `resolve_bundled_dependency`
already stat'd its `package.json` through the same path. The lexical
fallback now applies only when the root itself cannot be canonicalized.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XXHGos5SaEKKr7pUW4FXga

* fix(git-fetcher): correct the upstream test citation to a real commit

The previous citation pinned SHA cab1c11c69, which does not exist — it
was taken from the issue body without verification. Point instead at the
in-repo copy of the test at pnpm11/releasing/commands/test/publish/pack.ts,
pinned to a commit that actually contains it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XXHGos5SaEKKr7pUW4FXga

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Khải
2026-06-24 18:38:38 +07:00
committed by GitHub
parent e583b04e34
commit 4ec0049f55
2 changed files with 357 additions and 107 deletions

View File

@@ -18,10 +18,16 @@
//! `HISTORY*` / `NOTICE*` at the root, plus the paths declared in
//! `main` / `bin`. These survive `.npmignore` rejection and the
//! `files`-field filter.
//! 4. **`bundleDependencies` recursion**: for each name in
//! 4. **`bundleDependencies` closure**: starting from the names in
//! `manifest.bundleDependencies` (or the legacy
//! `bundledDependencies`), recurse into `node_modules/<name>/` and
//! splice its packlist under that prefix.
//! `bundledDependencies`), transitively include every reachable
//! dependency. A bundled package pulls in its own `dependencies`
//! and `optionalDependencies` too, so the whole closure ships.
//! Each name is resolved with the node module-resolution walk-up
//! (nested `node_modules/` first, then ancestor `node_modules/`),
//! which is what lets a hoisted transitive dep at the root
//! `node_modules/` be found and spliced in under its real path.
//! Port of [`npm-bundled`](https://github.com/npm/npm-bundled).
//!
//! Two intentional divergences from upstream:
//!
@@ -43,17 +49,17 @@ use ignore::{WalkBuilder, gitignore::Gitignore};
use pacquet_package_manifest::safe_read_package_json_from_dir;
use serde_json::Value;
use std::{
collections::{BTreeSet, HashSet},
collections::{BTreeSet, HashSet, VecDeque},
fs,
path::{Path, PathBuf},
};
/// Cap on `bundleDependencies` recursion depth. Real packages bundle
/// Cap on `bundleDependencies` closure depth. Real packages bundle
/// at most a handful of levels (most published packages bundle zero;
/// the rare ones bundle one or two). The cap prevents a runaway
/// recursion if a bundled dep declares its own bundle pointing at
/// itself (or a symlink loop in the source tree slips past
/// `canonicalize`).
/// the rare ones bundle one or two). The visited-set already makes
/// the walk terminate; this cap is belt-and-braces against a
/// pathological tree that keeps resolving fresh canonical paths
/// (e.g. a deep chain of `dependencies` that never repeats).
const MAX_BUNDLE_DEPTH: u32 = 32;
/// Case-insensitive prefix matches for files always-included at the
@@ -88,47 +94,165 @@ const ALWAYS_EXCLUDED_SUFFIXES: &[&str] = &[".orig"];
/// [`fs/packlist/src/index.ts:24-29`](https://github.com/pnpm/pnpm/blob/94240bc046/fs/packlist/src/index.ts#L24-L29)
/// (paths relative to `pkg_dir`, no leading `./`).
pub fn packlist(pkg_dir: &Path, manifest: &Value) -> Result<Vec<String>, PacklistError> {
let mut visited = HashSet::new();
packlist_inner(pkg_dir, manifest, &mut visited, 0)
let mut out: BTreeSet<String> = collect_own_files(pkg_dir, manifest)?;
collect_bundled_files(pkg_dir, manifest, &mut out)?;
Ok(out.into_iter().collect())
}
/// Inner recursive entry point that threads cycle detection and a
/// depth cap through `bundleDependencies` traversals. Each
/// recursion's canonicalised `pkg_dir` is inserted into `visited`
/// so a bundled dep that points back at an ancestor (cycle) gets
/// skipped instead of stack-overflowing. The `depth` counter is a
/// belt-and-braces guard against any cycle the canonical-path check
/// can't see (e.g. filesystem mount tricks).
fn packlist_inner(
pkg_dir: &Path,
manifest: &Value,
visited: &mut HashSet<PathBuf>,
/// One unit of `bundleDependencies`-closure work: resolve `name`
/// starting the node module-resolution walk-up at `from_dir`, then
/// splice the resolved package's files into the output.
struct BundleTask {
name: String,
from_dir: PathBuf,
depth: u32,
) -> Result<Vec<String>, PacklistError> {
// `fs::canonicalize` resolves symlinks, which is precisely what
// we want for cycle detection — a symlink loop in the source
// tree shows up as the same canonical path. Fall back to the
// input path on canonicalisation failure (e.g. permission
// denied); the cycle check then degrades to identity on the
// raw path, which is still enough to catch trivial self-bundles.
let canonical = fs::canonicalize(pkg_dir).unwrap_or_else(|_| pkg_dir.to_path_buf());
if !visited.insert(canonical) {
tracing::warn!(
target: "pacquet::git_fetcher::packlist",
pkg_dir = %pkg_dir.display(),
"bundleDependencies cycle: directory already visited at this canonical path; skipping",
);
return Ok(Vec::new());
}
/// Build the `bundleDependencies` closure for `root` and splice each
/// bundled package's files into `out` under the package's real path
/// relative to `root` (e.g. `node_modules/<name>/...`).
///
/// Mirrors [`npm-bundled`](https://github.com/npm/npm-bundled): seed
/// from the root manifest's bundle list, then transitively pull in
/// every reachable dependency. Once a package is bundled, its own
/// `dependencies` and `optionalDependencies` are bundled too — that
/// is how the closure reaches a hoisted transitive dep sitting at the
/// root `node_modules/`. `devDependencies` are never followed.
///
/// The `visited` set is keyed on the canonicalised resolved directory,
/// so a diamond (two bundled deps sharing a transitive dep) processes
/// the shared package once and a `dependencies` cycle terminates
/// instead of looping forever.
fn collect_bundled_files(
root: &Path,
root_manifest: &Value,
out: &mut BTreeSet<String>,
) -> Result<(), PacklistError> {
// Canonical form of the package root, used to reject any bundled
// dependency whose real path escapes the tree (see the symlink check
// in the loop below). `None` if `root` itself can't be canonicalised,
// in which case the escape check falls back to a lexical comparison.
let canonical_root = fs::canonicalize(root).ok();
let mut visited: HashSet<PathBuf> = HashSet::new();
let mut queue: VecDeque<BundleTask> = root_bundle_dep_names(root_manifest)
.into_iter()
.map(|name| BundleTask { name, from_dir: root.to_path_buf(), depth: 0 })
.collect();
while let Some(task) = queue.pop_front() {
if task.depth > MAX_BUNDLE_DEPTH {
tracing::warn!(
target: "pacquet::git_fetcher::packlist",
bundle_name = %task.name,
depth = task.depth,
"bundleDependencies closure exceeded MAX_BUNDLE_DEPTH; refusing to descend further",
);
continue;
}
// Defense-in-depth: a malicious manifest could carry
// `bundleDependencies: ["../../etc"]` (or an absolute path).
// Reject anything that's not a single safe segment before it
// reaches the join in `resolve_bundled_dependency`.
if !is_safe_bundle_name(&task.name) {
tracing::warn!(
target: "pacquet::git_fetcher::packlist",
bundle_name = %task.name,
"rejecting bundleDependencies entry that is not a single path segment",
);
continue;
}
let Some(dep_dir) = resolve_bundled_dependency(&task.name, &task.from_dir, root) else {
tracing::debug!(
target: "pacquet::git_fetcher::packlist",
bundle_name = %task.name,
from_dir = %task.from_dir.display(),
"bundleDependencies entry not resolvable under node_modules/; skipping",
);
continue;
};
// `fs::canonicalize` resolves symlinks, giving both the dedup
// key (a symlink loop shows up as an already-visited path) and
// the real target for the escape check below. `None` on failure
// (e.g. permission denied); dedup then degrades to the raw path
// and the escape check to a lexical comparison.
let canonical_dep = fs::canonicalize(&dep_dir).ok();
// `is_safe_bundle_name` only screens the name; a
// `node_modules/<name>` symlink pointing at a sibling or an
// absolute host path passes that yet resolves outside the tree.
// Walking it would splice host files into the published set, so
// refuse anything whose real path is not under the root. The
// fetcher imports untrusted git-hosted packages, so this matters.
let escapes = match (&canonical_root, &canonical_dep) {
(Some(root), Some(dep)) => dep.strip_prefix(root).is_err(),
// Root resolved but the dependency's real path didn't: we
// can't prove it stays inside the tree, so fail closed. A
// genuine dependency always canonicalises here —
// `resolve_bundled_dependency` already stat'd its
// `package.json` through the same path.
(Some(_), None) => true,
// Root itself won't canonicalise (pathological): fall back
// to a best-effort lexical check.
_ => dep_dir.strip_prefix(root).is_err(),
};
if escapes {
tracing::warn!(
target: "pacquet::git_fetcher::packlist",
bundle_name = %task.name,
dep_dir = %dep_dir.display(),
"bundled dependency resolves outside the package tree; refusing",
);
continue;
}
if !visited.insert(canonical_dep.unwrap_or_else(|| dep_dir.clone())) {
continue;
}
let prefix = relative_forward_slash(root, &dep_dir);
let dep_manifest = safe_read_package_json_from_dir(&dep_dir)
.ok()
.flatten()
.unwrap_or_else(|| Value::Object(serde_json::Map::new()));
for rel in collect_own_files(&dep_dir, &dep_manifest)? {
out.insert(format!("{prefix}/{rel}"));
}
for name in nested_bundle_dep_names(&dep_manifest) {
queue.push_back(BundleTask { name, from_dir: dep_dir.clone(), depth: task.depth + 1 });
}
}
if depth > MAX_BUNDLE_DEPTH {
tracing::warn!(
target: "pacquet::git_fetcher::packlist",
pkg_dir = %pkg_dir.display(),
depth,
"bundleDependencies recursion exceeded MAX_BUNDLE_DEPTH; refusing to descend further",
);
return Ok(Vec::new());
Ok(())
}
/// Resolve a bundled dependency `name` to its directory using the
/// node module-resolution walk-up: check `from_dir/node_modules/name`,
/// then climb to each ancestor's `node_modules/`, stopping at `root`.
/// Returns the first directory that contains a `package.json`, or
/// `None` if the name resolves nowhere within the package tree.
///
/// Climbing past `root` is refused so a hoisted dep always resolves to
/// the package being packed rather than to a sibling on the host.
fn resolve_bundled_dependency(name: &str, from_dir: &Path, root: &Path) -> Option<PathBuf> {
let mut current = from_dir.to_path_buf();
loop {
let candidate = current.join("node_modules").join(name);
if candidate.join("package.json").is_file() {
return Some(candidate);
}
if current == root {
return None;
}
let parent = current.parent()?;
if parent == current {
return None;
}
current = parent.to_path_buf();
}
}
/// Collect the forward-slash relative paths for a single package's own
/// files — the `.npmignore` / `.gitignore` walk, the `files`-field
/// allowlist, and the always-included / `main` / `bin` force-includes.
/// This is the per-package packlist with no `bundleDependencies`
/// traversal; [`collect_bundled_files`] layers the closure on top.
fn collect_own_files(pkg_dir: &Path, manifest: &Value) -> Result<BTreeSet<String>, PacklistError> {
let files_field = manifest.get("files").and_then(Value::as_array);
let files_matcher: Option<Gitignore> =
files_field.and_then(|arr| build_files_matcher(pkg_dir, arr));
@@ -177,9 +301,9 @@ fn packlist_inner(
if should_always_exclude(&rel) {
continue;
}
// `node_modules/` is handled by the `bundleDependencies`
// pass below — never include its contents via the general
// walk. Without this gate a manifest that publishes a stray
// `node_modules/` contents are bundled by
// `collect_bundled_files`, never via this general walk.
// Without this gate a manifest that publishes a stray
// `node_modules/something` would slip through.
if rel.starts_with("node_modules/") || rel == "node_modules" {
continue;
@@ -241,47 +365,7 @@ fn packlist_inner(
}
}
// Pass 4: recurse into `bundleDependencies` /
// `bundledDependencies`. Each bundled dep gets its own packlist
// pass; the result splices in under `node_modules/<name>/`. Both
// field names are accepted because some published packages use
// one and some the other (npm-packlist tolerates both).
for bundle_name in bundle_dep_names(manifest) {
// Defense-in-depth: a malicious manifest could carry
// `bundleDependencies: ["../../etc"]` (or an absolute path).
// Reject anything that's not a single safe segment before
// building the join path; let `is_safe_bundle_name` log the
// refusal so the gap is observable in install logs.
if !is_safe_bundle_name(&bundle_name) {
tracing::warn!(
target: "pacquet::git_fetcher::packlist",
bundle_name = %bundle_name,
pkg_dir = %pkg_dir.display(),
"rejecting bundleDependencies entry that is not a single path segment",
);
continue;
}
let bundle_pkg_dir = pkg_dir.join("node_modules").join(&bundle_name);
if !bundle_pkg_dir.is_dir() {
tracing::debug!(
target: "pacquet::git_fetcher::packlist",
bundle_name = %bundle_name,
pkg_dir = %pkg_dir.display(),
"bundleDependencies entry not present under node_modules/; skipping",
);
continue;
}
let bundle_manifest = safe_read_package_json_from_dir(&bundle_pkg_dir)
.ok()
.flatten()
.unwrap_or_else(|| Value::Object(serde_json::Map::new()));
let nested = packlist_inner(&bundle_pkg_dir, &bundle_manifest, visited, depth + 1)?;
for rel in nested {
out.insert(format!("node_modules/{bundle_name}/{rel}"));
}
}
Ok(out.into_iter().collect())
Ok(out)
}
/// Compile the `manifest.files` allowlist into a single `Gitignore`
@@ -413,10 +497,11 @@ fn is_safe_bundle_name(name: &str) -> bool {
true
}
/// Collect names from `bundleDependencies` (or the legacy
/// `bundledDependencies`). Both spellings appear in real published
/// packages; npm-packlist accepts either.
fn bundle_dep_names(manifest: &Value) -> Vec<String> {
/// Seed names for the bundle closure: the root manifest's
/// `bundleDependencies` (or the legacy `bundledDependencies`). Both
/// spellings appear in real published packages; npm-packlist accepts
/// either.
fn root_bundle_dep_names(manifest: &Value) -> Vec<String> {
let raw = manifest.get("bundleDependencies").or_else(|| manifest.get("bundledDependencies"));
let Some(raw) = raw else { return Vec::new() };
match raw {
@@ -435,6 +520,23 @@ fn bundle_dep_names(manifest: &Value) -> Vec<String> {
}
}
/// Names a bundled package pulls into the closure: every key in its
/// own `dependencies` and `optionalDependencies`. A bundled package
/// ships its whole runtime closure, so these are followed regardless
/// of whether the nested package declares its own `bundleDependencies`
/// (already-bundled packages don't re-gate their deps). `peer`- and
/// `dev`-dependencies are deliberately excluded — they are not part of
/// the published closure. Mirrors `npm-bundled`'s `getDeps`.
fn nested_bundle_dep_names(manifest: &Value) -> Vec<String> {
let mut names = Vec::new();
for field in ["dependencies", "optionalDependencies"] {
if let Some(map) = manifest.get(field).and_then(Value::as_object) {
names.extend(map.keys().cloned());
}
}
names
}
fn relative_forward_slash(root: &Path, full: &Path) -> String {
let rel = full.strip_prefix(root).unwrap_or(full);
let mut buf = PathBuf::from(rel).into_os_string().to_string_lossy().into_owned();

View File

@@ -4,11 +4,15 @@ use std::{fs, path::Path};
use tempfile::tempdir;
fn touch(root: &Path, rel: &str) {
write(root, rel, "");
}
fn write(root: &Path, rel: &str, contents: &str) {
let path = root.join(rel);
if let Some(parent) = path.parent() {
fs::create_dir_all(parent).unwrap();
}
fs::write(path, "").unwrap();
fs::write(path, contents).unwrap();
}
#[test]
@@ -265,6 +269,115 @@ fn bundle_dependencies_subtree_is_included() {
);
}
#[test]
fn bundle_dependencies_pull_in_hoisted_transitive_deps() {
// Port of pnpm's `pack: bundles transitive dependencies of bundled
// dependencies (hoisted)`
// ([pnpm11/releasing/commands/test/publish/pack.ts](https://github.com/pnpm/pnpm/blob/dd79bdc08e/pnpm11/releasing/commands/test/publish/pack.ts#L161-L191)).
// `top` is bundled and
// declares `dependencies: { nested }`; `nested` is hoisted to the
// root `node_modules`. The closure must follow `top`'s
// dependencies and resolve `nested` via the walk-up to the root,
// splicing it in at `node_modules/nested/`.
let dir = tempdir().unwrap();
let root = dir.path();
touch(root, "package.json");
write(
root,
"node_modules/top/package.json",
r#"{"name":"top","version":"1.0.0","dependencies":{"nested":"1.0.0"}}"#,
);
touch(root, "node_modules/top/index.js");
write(root, "node_modules/nested/package.json", r#"{"name":"nested","version":"1.0.0"}"#);
touch(root, "node_modules/nested/index.js");
let manifest = json!({
"name": "x",
"version": "0.0.0",
"bundledDependencies": ["top"],
});
let out = packlist(root, &manifest).unwrap();
assert!(out.contains(&"node_modules/top/index.js".to_string()), "{out:?}");
assert!(
out.contains(&"node_modules/nested/index.js".to_string()),
"hoisted transitive dep `nested` must be bundled: {out:?}",
);
}
#[test]
fn bundle_dependencies_follow_nested_node_modules_before_hoisted() {
// A bundled dep's own `node_modules/<dep>` wins over a hoisted copy
// at the root, matching node module resolution.
let dir = tempdir().unwrap();
let root = dir.path();
touch(root, "package.json");
write(
root,
"node_modules/top/package.json",
r#"{"name":"top","version":"1.0.0","dependencies":{"nested":"2.0.0"}}"#,
);
write(
root,
"node_modules/top/node_modules/nested/package.json",
r#"{"name":"nested","version":"2.0.0"}"#,
);
touch(root, "node_modules/top/node_modules/nested/nested-v2.js");
write(root, "node_modules/nested/package.json", r#"{"name":"nested","version":"1.0.0"}"#);
touch(root, "node_modules/nested/hoisted-v1.js");
let manifest = json!({
"name": "x",
"version": "0.0.0",
"bundleDependencies": ["top"],
});
let out = packlist(root, &manifest).unwrap();
assert!(
out.contains(&"node_modules/top/node_modules/nested/nested-v2.js".to_string()),
"nested copy of `nested` must be bundled: {out:?}",
);
assert!(
!out.contains(&"node_modules/nested/hoisted-v1.js".to_string()),
"hoisted `nested` is shadowed by the nested copy and must not ship: {out:?}",
);
}
#[test]
fn bundle_dependencies_optional_deps_of_bundled_dep_are_included() {
// `optionalDependencies` are part of a bundled package's runtime
// closure, so they ship; `devDependencies` do not.
let dir = tempdir().unwrap();
let root = dir.path();
touch(root, "package.json");
write(
root,
"node_modules/top/package.json",
r#"{"name":"top","version":"1.0.0","optionalDependencies":{"opt":"1.0.0"},"devDependencies":{"dev":"1.0.0"}}"#,
);
touch(root, "node_modules/top/index.js");
write(root, "node_modules/opt/package.json", r#"{"name":"opt","version":"1.0.0"}"#);
touch(root, "node_modules/opt/index.js");
write(root, "node_modules/dev/package.json", r#"{"name":"dev","version":"1.0.0"}"#);
touch(root, "node_modules/dev/index.js");
let manifest = json!({
"name": "x",
"version": "0.0.0",
"bundleDependencies": ["top"],
});
let out = packlist(root, &manifest).unwrap();
assert!(
out.contains(&"node_modules/opt/index.js".to_string()),
"optionalDependencies of a bundled dep must ship: {out:?}",
);
assert!(
!out.iter().any(|p| p.starts_with("node_modules/dev")),
"devDependencies of a bundled dep must not ship: {out:?}",
);
}
#[test]
fn bundled_dependencies_legacy_spelling_works() {
let dir = tempdir().unwrap();
@@ -352,6 +465,39 @@ fn bundle_dependencies_rejects_path_traversal() {
);
}
#[cfg(unix)]
#[test]
fn bundle_dependency_symlink_escaping_pkg_dir_is_refused() {
// Defense-in-depth: the bundle name is a single safe segment
// (`is_safe_bundle_name` accepts it), but `node_modules/<name>` is
// a symlink pointing outside the package. Resolving and walking it
// would splice host files into the published set. The fetcher
// imports untrusted git-hosted packages, so this must be refused.
let dir = tempdir().unwrap();
let root = dir.path().join("pkg");
fs::create_dir_all(root.join("node_modules")).unwrap();
touch(&root, "package.json");
// A sibling directory outside the package, made to look like a real
// package so the walk-up resolves it.
let escape = dir.path().join("escape");
fs::create_dir_all(&escape).unwrap();
fs::write(escape.join("package.json"), r#"{"name":"evil","version":"1.0.0"}"#).unwrap();
fs::write(escape.join("secret.txt"), "DO NOT EXFIL\n").unwrap();
std::os::unix::fs::symlink(&escape, root.join("node_modules/evil")).unwrap();
let manifest = json!({
"name": "x",
"version": "0.0.0",
"bundleDependencies": ["evil"],
});
let out = packlist(&root, &manifest).unwrap();
assert!(
!out.iter().any(|path| path.contains("secret")),
"a node_modules symlink escaping pkg_dir must not leak host files: {out:?}",
);
}
#[test]
fn always_excluded_dir_segments_only_match_vcs() {
let dir = tempdir().unwrap();
@@ -412,27 +558,29 @@ fn files_field_bare_basename_matches_at_depth() {
#[test]
fn bundle_dependencies_self_cycle_is_caught() {
// Defense-in-depth: a bundled dep whose own manifest declares a
// bundle pointing back at itself (or any cycle reachable through
// the canonical-path chain) must not stack-overflow the fetcher.
// The visited-set + depth cap inside `packlist_inner` stops the
// recursion.
// Defense-in-depth: a bundled dep whose own manifest depends on
// itself (or any cycle reachable through the canonical-path chain)
// must not loop the closure walk forever. The visited-set keyed on
// the canonicalised resolved directory stops the re-entry.
let dir = tempdir().unwrap();
let root = dir.path();
touch(root, "package.json");
touch(root, "node_modules/self/package.json");
touch(root, "node_modules/self/lib.js");
// The bundled dep declares itself as a bundleDependency. Without
// cycle detection this becomes infinite recursion.
// The bundled dep lists itself as a runtime dependency. The
// closure follows `dependencies`, so without cycle detection this
// re-resolves `self` forever.
fs::write(
root.join("node_modules/self/package.json"),
r#"{"name":"self","version":"1.0.0","bundleDependencies":["self"]}"#,
r#"{"name":"self","version":"1.0.0","dependencies":{"self":"1.0.0"}}"#,
)
.unwrap();
// Symlink `node_modules/self/node_modules/self` back to the
// outer `node_modules/self` so the canonical-path check has
// something to catch. (On platforms that can't symlink, the
// depth cap kicks in instead.)
// outer `node_modules/self` so the nested-first walk-up resolves
// the self-dependency to a directory the canonical-path check
// recognises as already visited. (On platforms that can't
// symlink, the walk-up falls back to the same outer directory and
// the visited-set still catches it.)
fs::create_dir_all(root.join("node_modules/self/node_modules")).unwrap();
#[cfg(unix)]
{
@@ -453,8 +601,8 @@ fn bundle_dependencies_self_cycle_is_caught() {
assert!(out.contains(&"package.json".to_string()));
assert!(out.contains(&"node_modules/self/package.json".to_string()));
assert!(out.contains(&"node_modules/self/lib.js".to_string()));
// No deeper paths via the cycle — the visited-set / depth cap
// refused the re-entry.
// No deeper paths via the cycle — the visited-set refused the
// re-entry.
assert!(
!out.iter().any(|p| p.starts_with("node_modules/self/node_modules/")),
"cycle through node_modules/self/node_modules/self/... must be cut: {out:?}",