Skip to content

feat(pacquet): port blockExoticSubdeps to reject exotic subdeps#11792

Merged
zkochan merged 1 commit into
mainfrom
exotic-subdeps
May 21, 2026
Merged

feat(pacquet): port blockExoticSubdeps to reject exotic subdeps#11792
zkochan merged 1 commit into
mainfrom
exotic-subdeps

Conversation

@zkochan

@zkochan zkochan commented May 21, 2026

Copy link
Copy Markdown
Member

Summary

Ports pnpm's blockExoticSubdeps setting to pacquet. When on, transitive dependencies resolved via an exotic protocol (git, tarball, file, …) fail the install with ERR_PNPM_EXOTIC_SUBDEP. Direct dependencies remain allowed. Mirrors upstream's gate at installing/deps-resolver/src/resolveDependencies.ts:1420-1434 and the closed NON_EXOTIC_RESOLVED_VIA set.

  • ResolveOptions.block_exotic_subdeps and the per-node check inside resolve_dependency_tree's walker.
  • Config.block_exotic_subdeps (default true, matching v11's config/reader/src/index.ts:187), wired through pnpm-workspace.yaml and the BLOCK_EXOTIC_SUBDEPS env-overlay key.
  • Threaded into install_without_lockfile's ResolveOptions.

Test plan

  • Unit tests in resolving-deps-resolver/src/tests.rs covering the four upstream scenarios (rejects exotic transitive, allows exotic direct, allows registry transitive, gate-off lets exotic through).
  • cargo nextest run -p pacquet-resolving-deps-resolver -p pacquet-config -p pacquet-resolving-resolver-base -p pacquet-package-manager — 516 / 516 pass.
  • cargo clippy --locked --workspace --all-targets -- --deny warnings clean.
  • cargo fmt --check clean.

Written by an agent (Claude Code, claude-opus-4-7).

Summary by CodeRabbit

  • New Features
    • Added a configurable option to block exotic (git/tarball/file) transitive dependencies while allowing direct ones; settable via config, workspace, or environment.
  • Behavior
    • Install/resolution now honor this option and fail with a clear error when an exotic transitive dependency is blocked.
  • Tests
    • Added tests for blocking, allowing, and disabled scenarios.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 35c1f1ea-3170-4dfa-b4c0-3eb0fce5f7cb

📥 Commits

Reviewing files that changed from the base of the PR and between 41b691a and 922fd36.

📒 Files selected for processing (8)
  • pacquet/crates/config/src/env_overlay.rs
  • pacquet/crates/config/src/lib.rs
  • pacquet/crates/config/src/workspace_yaml.rs
  • pacquet/crates/package-manager/src/install_package_from_registry/tests.rs
  • pacquet/crates/package-manager/src/install_without_lockfile.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
  • pacquet/crates/resolving-resolver-base/src/resolve.rs
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Lint and Test (ubuntu-latest)
  • GitHub Check: Code Coverage
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: When porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(), or streamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plain String or &str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only via TryFrom<String> and/or FromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallible From<String> (and From<&str> when convenient).
If upstream occasionally constructs without validation, expose from_str_unchecked as an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization.
Use #[derive(derive_more::From)] and #[derive(derive_more::Into)] for mechanical conversion impls. Fall back to manual impl only when conversion needs custom logic.
String-literal unions should become enums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Path over &PathBuf, &str over &String) when it doesn't force extra copies.
Prefer Arc::clone(&x) / Rc::clone(&x) over x.clone() for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Write use super::{Foo, bar} instead of use super::*;. Two forms stay allowed: external-crate preludes like use rayon::prelude::*; and root-of-module re-...

Files:

  • pacquet/crates/package-manager/src/install_without_lockfile.rs
  • pacquet/crates/config/src/workspace_yaml.rs
  • pacquet/crates/package-manager/src/install_package_from_registry/tests.rs
  • pacquet/crates/config/src/lib.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
  • pacquet/crates/resolving-resolver-base/src/resolve.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/config/src/env_overlay.rs
🧠 Learnings (2)
📚 Learning: 2026-05-20T19:40:55.051Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:40:55.051Z
Learning: In the pacquet Rust code, ensure the semver implementation uses the `node-semver` crate (not `nodejs-semver`). `node-semver`’s public API does not include a `satisfies_with_prerelease`-style method; prerelease-tolerant matching should be implemented inline by first calling `Range::satisfies`, and when it rejects a prerelease version, retry matching against a stripped `MAJOR.MINOR.PATCH` base of the prerelease version.

Applied to files:

  • pacquet/crates/package-manager/src/install_without_lockfile.rs
  • pacquet/crates/config/src/workspace_yaml.rs
  • pacquet/crates/package-manager/src/install_package_from_registry/tests.rs
  • pacquet/crates/config/src/lib.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
  • pacquet/crates/resolving-resolver-base/src/resolve.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/config/src/env_overlay.rs
📚 Learning: 2026-05-20T23:07:58.444Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:07:58.444Z
Learning: When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly. Do not propose review changes that intentionally deviate from pnpm’s documented/observed behavior, even if pnpm appears buggy. If you identify a real bug in pnpm behavior, the review should prioritize fixing it upstream in pnpm first, and avoid implementing a pnpm-behavior workaround here unless the same fix has already landed upstream.

Applied to files:

  • pacquet/crates/package-manager/src/install_without_lockfile.rs
  • pacquet/crates/config/src/workspace_yaml.rs
  • pacquet/crates/package-manager/src/install_package_from_registry/tests.rs
  • pacquet/crates/config/src/lib.rs
  • pacquet/crates/resolving-deps-resolver/src/tests.rs
  • pacquet/crates/resolving-resolver-base/src/resolve.rs
  • pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
  • pacquet/crates/config/src/env_overlay.rs
🔇 Additional comments (8)
pacquet/crates/config/src/lib.rs (1)

495-502: LGTM!

pacquet/crates/config/src/workspace_yaml.rs (1)

151-151: LGTM!

Also applies to: 360-360, 450-450

pacquet/crates/config/src/env_overlay.rs (1)

147-147: LGTM!

pacquet/crates/resolving-resolver-base/src/resolve.rs (1)

218-224: LGTM!

pacquet/crates/package-manager/src/install_without_lockfile.rs (1)

142-145: LGTM!

Also applies to: 340-349, 356-357, 364-365

pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs (1)

40-47: LGTM!

Also applies to: 74-101, 130-130, 149-157, 170-171, 175-185, 198-199, 213-214, 308-319, 423-468, 570-587

pacquet/crates/resolving-deps-resolver/src/tests.rs (1)

116-119: LGTM!

Also applies to: 183-186, 213-216, 228-420, 449-452, 499-502, 547-550, 598-601, 657-660, 679-862

pacquet/crates/package-manager/src/install_package_from_registry/tests.rs (1)

51-51: LGTM!


📝 Walkthrough

Walkthrough

This PR introduces block_exotic_subdeps, a new configuration flag that gates rejection of transitive dependencies resolved via exotic protocols (git, tarball, file) while permitting direct dependencies. The flag flows through configuration, into resolver options, and into dependency tree resolution with post-resolution validation.

Changes

Block Exotic Transitive Dependencies

Layer / File(s) Summary
Configuration field definition and precedence
pacquet/crates/config/src/lib.rs, pacquet/crates/config/src/workspace_yaml.rs, pacquet/crates/config/src/env_overlay.rs
block_exotic_subdeps: bool (default true) is added to Config, as an optional override in WorkspaceSettings, with environment variable parsing and proper precedence handling so workspace YAML overrides global config.
Resolver options contract
pacquet/crates/resolving-resolver-base/src/resolve.rs
ResolveOptions gains block_exotic_subdeps: bool field with documentation explaining that it gates exotic protocol resolutions for transitive subdependencies while allowing direct dependencies.
Wire config into resolver invocation
pacquet/crates/package-manager/src/install_without_lockfile.rs
InstallWithoutLockfile::run passes config.block_exotic_subdeps into the resolver's ResolveOptions.
Blocking logic and error reporting
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
ResolveDependencyTreeError::ExoticSubdep error variant is added; NON_EXOTIC_RESOLVED_VIA allowlist and is_exotic_resolved_via classifier are introduced; post-resolution gate in resolve_node rejects resolutions when block_exotic_subdeps && depth > 0 and resolved_via is exotic.
Test coverage for blocking behavior
pacquet/crates/resolving-deps-resolver/src/tests.rs, pacquet/crates/package-manager/src/install_package_from_registry/tests.rs
New block_exotic_subdeps test module with four tests validating exotic transitive rejection, exotic direct allowance, normal registry allowance, and disabling the gate; test helper create_config updated to set block_exotic_subdeps: false.

Sequence Diagram(s)

sequenceDiagram
  participant Installer as InstallWithoutLockfile::run
  participant Resolver as resolve_importer / resolver chain
  participant DepWalker as resolve_node (dependency walker)
  Installer->>Resolver: call with ResolveOptions { block_exotic_subdeps }
  Resolver->>DepWalker: resolve dependency node (returns ResolveResult with resolved_via)
  DepWalker->>DepWalker: if depth>0 and block_exotic_subdeps then check resolved_via against NON_EXOTIC_RESOLVED_VIA
  alt resolved_via allowed
    DepWalker-->>Resolver: return resolved package
  else exotic resolved_via
    DepWalker-->>Installer: return ExoticSubdep error (specifier, resolved_via)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • pnpm/pnpm#11752: Extends env_overlay env-var parsing mechanism that this PR builds upon for PNPM_CONFIG_BLOCK_EXOTIC_SUBDEPS.
  • pnpm/pnpm#11773: Adds tarball resolver producing resolved_via values (e.g., "url"), related to the exotic-protocol classification this PR enforces.

Poem

🐰 A rabbit's gate now guards the path,
No exotic guests shall pass its wrath,
Direct friends still hop inside,
While git and tarballs must abide,
Safety nibbles on each craft.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(pacquet): port blockExoticSubdeps to reject exotic subdeps' directly and specifically describes the main change: implementing pnpm's blockExoticSubdeps feature to reject exotic subdependencies in pacquet.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch exotic-subdeps

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@zkochan zkochan marked this pull request as ready for review May 21, 2026 00:41
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.04      8.0±0.25ms   540.7 KB/sec    1.00      7.7±0.27ms   560.2 KB/sec

@coderabbitai coderabbitai Bot added area: supply chain security Issues related to minimumReleaseAge, blockExoticSubdeps, build script safety, and trust policies. area: config dependencies Changes related to configDependencies. area: resolution labels May 21, 2026
@codecov-commenter

codecov-commenter commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.39%. Comparing base (667e587) to head (922fd36).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11792      +/-   ##
==========================================
- Coverage   87.41%   87.39%   -0.03%     
==========================================
  Files         198      200       +2     
  Lines       23119    23514     +395     
==========================================
+ Hits        20209    20549     +340     
- Misses       2910     2965      +55     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Integrated-Benchmark Report (Linux)

Scenario: Frozen Lockfile

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.899 ± 0.111 2.795 3.112 1.04 ± 0.05
pacquet@main 2.790 ± 0.058 2.727 2.920 1.00
pnpm 5.756 ± 0.090 5.607 5.942 2.06 ± 0.05
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.8991743531000003,
      "stddev": 0.11128917865175923,
      "median": 2.857300347,
      "user": 3.0333921400000006,
      "system": 4.22339938,
      "min": 2.795270952,
      "max": 3.1116293059999998,
      "times": [
        3.1116293059999998,
        2.811592349,
        2.880357234,
        3.011261246,
        2.860646117,
        2.835657626,
        2.853954577,
        2.795270952,
        2.799847997,
        3.031526127
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.7902462403999997,
      "stddev": 0.05793070086546354,
      "median": 2.7757719925,
      "user": 3.01012724,
      "system": 4.163981980000001,
      "min": 2.727069112,
      "max": 2.920070234,
      "times": [
        2.727069112,
        2.769839168,
        2.773897924,
        2.853433593,
        2.7958284399999997,
        2.777646061,
        2.731354986,
        2.794751768,
        2.758571118,
        2.920070234
      ]
    },
    {
      "command": "pnpm",
      "mean": 5.756288626500002,
      "stddev": 0.09027497155399114,
      "median": 5.759216821000001,
      "user": 9.971017039999998,
      "system": 4.76929708,
      "min": 5.607190512000001,
      "max": 5.942210093000001,
      "times": [
        5.942210093000001,
        5.6737911400000005,
        5.805904561,
        5.775891647000001,
        5.607190512000001,
        5.777640001000001,
        5.741771369,
        5.693643284,
        5.802301663000001,
        5.742541995000001
      ]
    }
  ]
}

Scenario: Frozen Lockfile (Hot Cache)

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 805.8 ± 42.3 775.8 919.3 1.00
pacquet@main 827.9 ± 62.4 780.3 992.8 1.03 ± 0.09
pnpm 3082.3 ± 50.1 3010.4 3149.4 3.82 ± 0.21
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.805834733,
      "stddev": 0.042344210450721564,
      "median": 0.7901867991,
      "user": 0.44442838,
      "system": 1.7494305799999998,
      "min": 0.7757759136,
      "max": 0.9192821766,
      "times": [
        0.9192821766,
        0.7976685796,
        0.7884392266,
        0.7889569056,
        0.8104936476,
        0.8231565436,
        0.7803633676,
        0.7827942766,
        0.7914166926,
        0.7757759136
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.8278628749999999,
      "stddev": 0.06237726239698783,
      "median": 0.8044649126000001,
      "user": 0.44640188,
      "system": 1.7712514799999997,
      "min": 0.7803209086,
      "max": 0.9928375816,
      "times": [
        0.8662117336,
        0.7993886706000001,
        0.8080973086000001,
        0.9928375816,
        0.7942093936,
        0.7803209086,
        0.7988182046,
        0.8203478686,
        0.8008325166,
        0.8175645636000001
      ]
    },
    {
      "command": "pnpm",
      "mean": 3.0822510844,
      "stddev": 0.050139526178859736,
      "median": 3.0870850021000003,
      "user": 3.92905718,
      "system": 2.48433038,
      "min": 3.0104180846000004,
      "max": 3.1494138326,
      "times": [
        3.1205487006,
        3.1410735386,
        3.1494138326,
        3.1055193676000004,
        3.0459596076000004,
        3.0220988296,
        3.0686506366,
        3.0104180846000004,
        3.0451336716000004,
        3.1136945746
      ]
    }
  ]
}

Rejects git/tarball/file resolutions reached transitively from the
importer when `blockExoticSubdeps` is on. Direct deps remain allowed.
Mirrors pnpm's gate at df990fd (`installing/deps-resolver/src/
resolveDependencies.ts:1420-1434`) and the closed `NON_EXOTIC_RESOLVED_VIA`
set (lines 1831-1841). Default is `true`, matching v11's
`config/reader/src/index.ts:187`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: config dependencies Changes related to configDependencies. area: resolution area: supply chain security Issues related to minimumReleaseAge, blockExoticSubdeps, build script safety, and trust policies.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants