feat(pnpr): merge the namespace and the ACL into per-registry packages: maps (RFC pnpm/rfcs#17)#12787
Merged
Conversation
…s: maps Implements the revised model from pnpm/rfcs#17, replacing the interim patterns:-plus-global-ACL shape outright (pre-1.0, no compatibility mode). Every concrete registry now declares one packages: map whose keys are its namespace (the former patterns: list) and whose values are the per-package access/publish/unpublish rules (the former top-level packages: block, scoped to the one registry that serves the name). One declaration routes, filters, and authorizes. The registry-level access: is the default an entry's omitted fields fall back to; publish defaults to $authenticated and unpublish to nobody. Selection is by specificity, not key order: an exact name beats @scope/* beats @*/* beats **. YAML mappings are formally unordered, so a formatter or yq round-trip must not change which rule applies; the restricted pattern language makes the winner unique (at most one matching key per tier), no entry can be dead, and a duplicate key is the only within-registry error. Router sources: stay an ordered list with their unreachable/shadowed-claim validation intact. The removed top-level packages: block is a startup error naming the per-registry replacement — it used to enforce access, so dropping it like an unknown verdaccio key would silently open previously gated packages on upgrade. public: true keeps meaning the upstream fetch (no credential, no headers, no registry-level access default), but per-package access rules are now permitted on a public upstream — they gate who may read the name through pnpr, not how pnpr fetches it. publish/unpublish values on any upstream are rejected: no write can land there. Hosted denials answer by tier, preserving both prior behaviors: a caller the registry-level default denies is masked with 404 for every name (a blanket-private registry never reveals which names exist, even explicitly ruled ones), while an explicit entry denying a caller the registry itself admits rejects loudly — 401 anonymous / 403 authenticated — so clients can prompt for credentials (the registry-mock needs-auth contract). The resolver's route classification resolves path-less fetches through the registry graph — the same dispatch serving uses — and hosted private-access descriptors are registry-qualified (registry NUL package), so the same name@version on two hosted registries can never share a cache key; unqualified descriptors from older builds fail closed and re-resolve. The bundled config.yaml moves the fixture namespace and the old ACL into the local registry's packages: map (YAML anchors keep it readable), and Config::proxy / Config::static_serve carry the registry-mock rules programmatically. Implements the pnpr side of pnpm/rfcs#17 only; no client or lockfile changes.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR replaces pnpr’s global package-policy model with per-registry ChangesRegistry access rules refactor
Benchmark config shape update
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
PR Summary by Qodopnpr: move namespace + ACL into per-registry
AI Description
Diagram
High-Level Assessment
Files changed (17)
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements pnpr’s RFC-17 “revised registries model” by replacing the interim global packages: ACL + per-registry patterns: namespace with a single per-registry packages: map that both (1) declares namespace and (2) defines access/publish/unpublish rules, with rule selection by specificity (order-independent).
Changes:
- Replaced global
PackagePolicies/PackagePolicywith per-registryPackageRules/PackageRule, including specificity-based rule selection and registry-level defaults. - Reworked server read/write authorization and routing to consult the resolved concrete registry’s rules (including registry-qualified private-cache descriptors for hosted registries).
- Updated config parsing + bundled
config.yamland migrated/added tests to cover the new model and invariants.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpr/crates/pnpr/tests/server.rs | Migrates integration tests from global ACL to per-registry rules and adds helpers for rules setup. |
| pnpr/crates/pnpr/tests/policy.rs | Updates YAML-driven integration tests to nest packages: under a registry. |
| pnpr/crates/pnpr/tests/auth_publish.rs | Updates publish/auth integration tests to generate new per-registry packages: YAML. |
| pnpr/crates/pnpr/src/upstream/tests.rs | Updates upstream test fixture construction to include default rules. |
| pnpr/crates/pnpr/src/server/tests.rs | Migrates server tests to use hosted registry rules instead of global policies. |
| pnpr/crates/pnpr/src/server.rs | Rewires authorization/routing to use resolved registry rules; adds hosted gating tiers and write-target plumbing. |
| pnpr/crates/pnpr/src/route/tests.rs | Updates route classification tests for registry-qualified hosted descriptors. |
| pnpr/crates/pnpr/src/route.rs | Makes route classification resolve through the registry graph and consult hosted registry rules; qualifies hosted cache descriptors. |
| pnpr/crates/pnpr/src/resolver/tests.rs | Updates resolver-cache tests for registry-qualified hosted descriptors and per-registry hosted rules. |
| pnpr/crates/pnpr/src/registry.rs | Adds pattern specificity ranking used for order-independent rule selection. |
| pnpr/crates/pnpr/src/policy/tests.rs | Adds/updates tests for specificity selection, defaults, and registry-mock rule behavior. |
| pnpr/crates/pnpr/src/policy.rs | Replaces ordered glob policy list with per-registry PackageRules keyed by PackagePattern + specificity selection. |
| pnpr/crates/pnpr/src/lib.rs | Updates public exports to the new rule types. |
| pnpr/crates/pnpr/src/error.rs | Removes obsolete invalid-policy-pattern error variant tied to wax globs. |
| pnpr/crates/pnpr/src/config/tests.rs | Updates config parsing tests to the new registries+packages shape and adds new validation assertions. |
| pnpr/crates/pnpr/src/config.rs | Moves access rules into per-registry config; rejects top-level packages:; builds rules from per-registry maps. |
| pnpr/crates/pnpr/config.yaml | Updates bundled config to declare fixture namespace + access rules via the per-registry packages: map (with YAML anchors). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1919
to
+1922
| let effective = hosted.rules.for_package(package); | ||
| if effective.access.allows(identity) { | ||
| return HostedGate::Allowed(hosted.org.clone()); | ||
| } |
Member
Author
There was a problem hiding this comment.
Intended, but the doc comment invited this read — sharpened in 0dd24c3. The decision is always the effective per-package access: an explicit entry fully decides its names, including opening one name (access: $all) on an otherwise-private registry (the RFC's field-fallback semantics — the registry-level access: is a default, not an independent AND-gate). Only the shape of a denial is tiered: default-denied callers are masked, explicit-entry denials inside a visible registry are loud.
Comment on lines
+476
to
+484
| let Some(rules) = self.hosted_rules.get(registry) else { | ||
| return RouteClass::Public; | ||
| }; | ||
| let access = rules.for_package(package).access; | ||
| if access.allows(&Identity::Anonymous) { | ||
| return RouteClass::Public; | ||
| } | ||
| if access.allows(identity) { | ||
| RouteClass::Hosted { policy_id: package.to_string() } | ||
| RouteClass::Hosted { policy_id: hosted_policy_id(registry, package) } |
Member
Author
There was a problem hiding this comment.
Classification admits with the same effective-access lookup the serving gate does (rules.for_package(package).access.allows(identity)), so they cannot disagree about whether a caller may read a name — the serving tiers only vary the denial's shape (mask vs 401/403), and every denial classifies as an anonymous public fetch the endpoint fails closed on. Added a comment stating the invariant in 0dd24c3.
Comment on lines
+261
to
+265
| let winner = self | ||
| .rules | ||
| .iter() | ||
| .filter(|rule| rule.pattern.matches(package)) | ||
| .max_by_key(|rule| rule.pattern.specificity()); |
Member
Author
There was a problem hiding this comment.
Done in 0dd24c3: for_package now resolves the winner through a per-tier index (exact/scope maps plus any-scoped/all slots) — the specificity chain means each tier holds at most one candidate, so the lookup is a couple of map probes instead of a scan.
Comment on lines
+531
to
+535
| Some((registry, package)) => self | ||
| .hosted_rules | ||
| .get(registry) | ||
| .is_some_and(|rules| rules.for_package(package).access.allows(identity)), | ||
| None => false, |
Member
Author
There was a problem hiding this comment.
Same as the classification note: the registry-level default is a field fallback inside for_package, not an independent gate, so this check is already consistent with hosted_gate's Allowed branch. The denial tiers only affect response shape on the serving path.
Code Review by Qodo
1.
|
Comment on lines
+379
to
+397
| return match self.registries.resolve_default(package) { | ||
| Resolved::Concrete { registry, kind: ConcreteKind::Hosted } => { | ||
| self.classify_hosted(identity, registry, Some(package)) | ||
| } | ||
| Resolved::Concrete { registry, kind: ConcreteKind::Upstream } => { | ||
| match self | ||
| .aliases | ||
| .iter() | ||
| .find(|alias| alias.name == registry && alias.access.allows(identity)) | ||
| { | ||
| Some(alias) => RouteClass::Proxied { | ||
| alias: alias.name.clone(), | ||
| credential_digest: alias.credential_digest.clone(), | ||
| }, | ||
| // A public upstream source (no alias) is an anonymous | ||
| // public fetch; an unauthorized caller falls through | ||
| // to one the endpoint fails closed on. | ||
| None => RouteClass::Public, | ||
| } |
There was a problem hiding this comment.
1. Upstream access misclassified 🐞 Bug ⛨ Security
RouteContext::classify returns RouteClass::Public for upstream registries that have no access-bearing alias, without consulting that upstream registry’s per-package packages: access rules, so restricted upstream packages can be treated as “public” for resolver footprinting/caching. This can allow resolution-cache reuse across identities even when upstream.rules denies anonymous access for the package.
Agent Prompt
### Issue description
`RouteContext::classify()` currently treats upstream routes with no access-bearing alias as `RouteClass::Public`, but upstream registries now have per-package `rules` (from `registries.<name>.packages`) that can deny anonymous access. When such a route is classified as `Public`, `RouteHook::metadata_scope()` returns `MetadataCacheScope::Public` and `RouteHook::authorize()` records no private descriptor, so the resolver’s footprint can remain empty and the resulting resolution can be cached/replayed as globally shareable.
### Issue Context
- Upstream per-package access can deny anonymous reads (`upstream.rules.for_package(..).access`).
- Resolver cache privacy relies on route classification: `RouteClass::Public` => public metadata scope and no footprint entry.
- The bug affects both `/~<registry>/...` fetches and path-less fetches that resolve to an upstream.
### Fix Focus Areas
- pnpr/crates/pnpr/src/route.rs[342-402]
### What to change
1. Extend `RouteContext` to retain upstream rule sets (e.g. `upstream_rules: IndexMap<String, PackageRules>`) similar to `hosted_rules`.
2. In `classify()`, when a fetch resolves to an upstream registry (either `/~<name>/` or via `registries.resolve_default(package)`):
- Look up that upstream’s rules and compute effective access for `package`.
- If anonymous is allowed, keep `RouteClass::Public`.
- If anonymous is denied but `identity` is allowed, return a **private** classification that records a private descriptor (so metadata scope becomes `Private` and the footprint is non-empty).
- If `identity` is denied, fall back to the existing “fail closed by letting the endpoint reject” behavior.
3. Add a descriptor type that can be re-validated on cache hits (either:
- a new `PrivateAccessDescriptor`/`RouteClass` variant for upstream-rule-gated routes, or
- generalize the existing `Hosted` descriptor handling to include upstream registries too).
4. Update `allows_descriptor()` to re-check upstream rules for this descriptor, so cached private resolutions are only reused when the caller is still authorized.
5. Add tests covering: public upstream with per-package access rule => authorized caller produces a private footprint + private metadata scope; anonymous caller does not.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Member
Author
There was a problem hiding this comment.
Fixed in 715dedd: alias selection is now per-package-aware on every classification path (direct origin, /~<name>/, and the path-less graph resolution) — a caller the upstream's effective access denies for a name is never handed the credential, so a fresh resolve fails closed exactly where the serving endpoint would deny the read, and cannot warm a cache entry for that name. One deliberate boundary, now documented at the descriptor gate: cache replay granularity stays registry-scoped (the alias descriptor names no package), shared among callers the registry-level access: admits — per-package refinements bound fresh resolves and every serving read.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
pnpr/crates/pnpr/src/server.rs (1)
3100-3167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated
resolve_write_target+authorizepattern.The sequence
resolve_write_target(...)→ buildRegistrySource::Hosted(target.source.clone())→ loop/callauthorize(..., action)is duplicated near-identically inupdate_packument(Lines 3111-3121),delete_package(Lines 3333-3346),delete_tarball(Lines 3382-3395),update_dist_tag(Lines 3501-3514), andvalidate_publish_doc(Lines 2725-2733). A helper folding both steps (accepting one or moreActions) would remove five near-identical blocks and reduce the risk of a future write handler forgetting a required action check.♻️ Proposed consolidation
+fn resolve_authorized_write_target( + state: &AppState, + identity: &Identity, + registry: Option<&str>, + name: &PackageName, + actions: &[Action], +) -> Result<WriteTarget, Box<Response>> { + let target = resolve_write_target(state, identity, registry, name)?; + let source = RegistrySource::Hosted(target.source.clone()); + for action in actions { + authorize(state, identity, &source, name.as_str(), *action) + .map_err(|err| Box::new(error_response(&err)))?; + } + Ok(target) +}Call sites then collapse to e.g.
let target = resolve_authorized_write_target(state, identity, registry, &name, &[Action::Publish, Action::Unpublish])?;.Also applies to: 3323-3409, 3483-3576, 3588-3610
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pnpr/crates/pnpr/src/server.rs` around lines 3100 - 3167, The repeated write-target resolution and authorization flow should be extracted into a shared helper to avoid duplication across update_packument and the other write handlers. Create a helper that combines resolve_write_target, building RegistrySource::Hosted(target.source.clone()), and the authorize checks for one or more Action values, then update update_packument, delete_package, delete_tarball, update_dist_tag, and validate_publish_doc to call it. Keep the existing behavior and error propagation unchanged, just centralize the pattern under a clear name like resolve_authorized_write_target.pnpr/crates/pnpr/tests/auth_publish.rs (1)
31-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate YAML-splicing helper vs.
tests/policy.rs.This helper and
config_from_yamlinpnpr/crates/pnpr/tests/policy.rs(Lines 25-53) both re-indent a caller-suppliedpackages:block and splice it underregistries.local.packages, but with subtly different rules (blank-line handling,strip_prefixcontract, catch-all appending). Consider factoring a single shared test helper (e.g. in a common test-support module) that both integration test files call, so the splicing/indentation contract only needs to be correct — and updated — in one place.As per path instructions, "Focus your added depth on: ... (3) missed reuse or duplication across the monorepo."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pnpr/crates/pnpr/tests/auth_publish.rs` around lines 31 - 61, The test helper for YAML package-block splicing is duplicated between static_config_with_packages and config_from_yaml, with slightly different indentation/blank-line/catch-all behavior. Extract this logic into a shared test-support helper used by both auth_publish.rs and policy.rs, and keep the contract for re-indenting under registries.local.packages in one place so future changes only need to be made once.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pnpr/crates/pnpr/src/route.rs`:
- Around line 383-402: The resolve footprint for package-level upstreams is
still being classified as public even when upstream ACL rules should make the
fetch private. Update the RouteClass selection in route.rs, especially the
Resolved::Concrete { kind: ConcreteKind::Upstream } branch and
RouteContext/route keying, so upstream package rules are consulted when building
the context and read-gated upstreams get a private cache key instead of the
shared public one. Keep anonymous public fetches as RouteClass::Public, but
thread the upstream access rules into the resolve path used by the route
context.
In `@pnpr/crates/pnpr/tests/server.rs`:
- Line 2934: Fix the flagged spelling in the comment near the merged model/ACL
discussion by changing “mis-order” to “misorder” in the test commentary within
server.rs; no code logic changes are needed, just update the wording in the
nearby comment text so Rust CI spell check passes.
---
Nitpick comments:
In `@pnpr/crates/pnpr/src/server.rs`:
- Around line 3100-3167: The repeated write-target resolution and authorization
flow should be extracted into a shared helper to avoid duplication across
update_packument and the other write handlers. Create a helper that combines
resolve_write_target, building RegistrySource::Hosted(target.source.clone()),
and the authorize checks for one or more Action values, then update
update_packument, delete_package, delete_tarball, update_dist_tag, and
validate_publish_doc to call it. Keep the existing behavior and error
propagation unchanged, just centralize the pattern under a clear name like
resolve_authorized_write_target.
In `@pnpr/crates/pnpr/tests/auth_publish.rs`:
- Around line 31-61: The test helper for YAML package-block splicing is
duplicated between static_config_with_packages and config_from_yaml, with
slightly different indentation/blank-line/catch-all behavior. Extract this logic
into a shared test-support helper used by both auth_publish.rs and policy.rs,
and keep the contract for re-indenting under registries.local.packages in one
place so future changes only need to be made once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 33ef93e8-fc24-4172-ad93-26fcaab1823e
📒 Files selected for processing (17)
pnpr/crates/pnpr/config.yamlpnpr/crates/pnpr/src/config.rspnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/src/error.rspnpr/crates/pnpr/src/lib.rspnpr/crates/pnpr/src/policy.rspnpr/crates/pnpr/src/policy/tests.rspnpr/crates/pnpr/src/registry.rspnpr/crates/pnpr/src/resolver/tests.rspnpr/crates/pnpr/src/route.rspnpr/crates/pnpr/src/route/tests.rspnpr/crates/pnpr/src/server.rspnpr/crates/pnpr/src/server/tests.rspnpr/crates/pnpr/src/upstream/tests.rspnpr/crates/pnpr/tests/auth_publish.rspnpr/crates/pnpr/tests/policy.rspnpr/crates/pnpr/tests/server.rs
💤 Files with no reviewable changes (1)
- pnpr/crates/pnpr/src/error.rs
|
Code review by qodo was updated up to the latest commit 522bdfb |
- pacquet-pnpr-client's integration test builds an UpstreamConfig literal; add the new rules field (this compile error was failing the CI test jobs and coverage). - Index PackageRules::for_package by specificity tier (exact/scope maps + any-scoped/all slots) instead of scanning every rule: the lookup runs on every read, write, search hit, and route classification, and the tier chain makes the winner a map lookup. - Sharpen the hosted_gate and classify_hosted docs: the effective per-package access decides — an explicit entry fully decides its names, including opening one name on an otherwise-private registry — and only the denial's *shape* varies by tier (default denial masks, explicit-entry denial is loud). Classification admits with the same lookup serving does, so the two cannot diverge. - Appease dylint (single-letter closure params, intra-doc link), clippy (trailing comma), and typos (mis-order).
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
Contributor
Micro-Benchmark ResultsLinux |
…t path Route classification now consults the upstream registry's packages: rules per name: a caller the effective access denies is never handed the server-owned credential, so a fresh resolve fails closed exactly where the serving endpoint would deny the read. Cache replay stays registry-scoped by design (the alias descriptor names no package) — documented at the descriptor gate. Search skips a hosted registry outright when no rule of it could admit the caller, restoring the pre-merge fast path: a blanket mask must not become an enumeration or scan-timing primitive.
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12787 +/- ##
==========================================
+ Coverage 85.59% 85.62% +0.02%
==========================================
Files 413 413
Lines 64125 64311 +186
==========================================
+ Hits 54888 55065 +177
- Misses 9237 9246 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Code review by qodo was updated up to the latest commit 715dedd |
The cold-mock config carried three routing shapes so one file could drive any benchmarked pnpr revision, on the premise that every server ignores the blocks it doesn't recognize. HEAD broke that premise on purpose: a top-level packages: block is now a startup error, so the pnpr@HEAD revision mock refused to start and the benchmark job failed. Keep the two shapes that still coexist (registries:/defaultRegistry: and mounts:/defaultTarget:) and document that a pre-mount pnpr can no longer share a config file with current ones.
|
Code review by qodo was updated up to the latest commit 7403adb |
|
Code review by qodo was updated up to the latest commit 7403adb |
Option<IgnoredAny> maps a present-but-null packages: (a bare key, or ~) to None, slipping past the loud rejection. Detect presence through a custom deserializer that consumes any value — including null — so every spelling of the removed key fails startup identically.
Contributor
Integrated-Benchmark Report (Linux)Commit: Each scenario reports direct installs and pnpr installs. Bencher consumes pacquet@HEAD and pnpr@HEAD. Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.018140557320001,
"stddev": 0.10039456693647687,
"median": 3.9854780939200003,
"user": 3.8487341,
"system": 3.4992582399999996,
"min": 3.92705808442,
"max": 4.23132079242,
"times": [
4.04234923742,
3.92705808442,
3.95821687842,
4.15438093642,
3.92844858642,
4.01270347442,
3.9765743164200003,
3.9943818714200003,
4.23132079242,
3.95597139542
]
},
{
"command": "pacquet@main",
"mean": 4.01514801362,
"stddev": 0.12417226788066171,
"median": 3.96475723042,
"user": 3.8699114000000003,
"system": 3.46761074,
"min": 3.8806904874200003,
"max": 4.29366291842,
"times": [
3.95836019442,
3.9422018854200003,
4.29366291842,
3.9655925304200004,
4.12366687042,
3.92336138342,
3.96392193042,
3.99143810742,
4.1085838284200005,
3.8806904874200003
]
},
{
"command": "pnpr@HEAD",
"mean": 2.10060477712,
"stddev": 0.10948574706385154,
"median": 2.07458233492,
"user": 2.659427,
"system": 2.99546154,
"min": 1.97957946242,
"max": 2.2785783034200002,
"times": [
2.2785783034200002,
2.0106268804200003,
2.07463359842,
2.07453107142,
2.0752831134200003,
1.97957946242,
2.0280885684200003,
2.2429739784200002,
2.23295101642,
2.00880177842
]
},
{
"command": "pnpr@main",
"mean": 2.1611873879199996,
"stddev": 0.10927019275365787,
"median": 2.15917986492,
"user": 2.6271036,
"system": 3.00990774,
"min": 1.98055361042,
"max": 2.32873525742,
"times": [
2.15276659842,
2.11873715942,
2.23063180942,
2.32873525742,
2.08633651942,
1.98055361042,
2.16559313142,
2.23866247142,
2.27400987442,
2.03584744742
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.63348431038,
"stddev": 0.011809711790930304,
"median": 0.6351319211800001,
"user": 0.3705574800000001,
"system": 1.32630516,
"min": 0.6167603071800001,
"max": 0.65617657518,
"times": [
0.63099406418,
0.63941268918,
0.63853514918,
0.6167603071800001,
0.61988293518,
0.6373785011800001,
0.6328853411800001,
0.65617657518,
0.64101600418,
0.62180153718
]
},
{
"command": "pacquet@main",
"mean": 0.66420539258,
"stddev": 0.07804366533794395,
"median": 0.64202782018,
"user": 0.36657128000000005,
"system": 1.3470020599999997,
"min": 0.62063131618,
"max": 0.88397855818,
"times": [
0.64370527918,
0.62063131618,
0.64190312618,
0.6421525141800001,
0.6285042671800001,
0.63305710118,
0.66181055018,
0.64938838318,
0.6369228301800001,
0.88397855818
]
},
{
"command": "pnpr@HEAD",
"mean": 0.72676939488,
"stddev": 0.0927484848449469,
"median": 0.69919339818,
"user": 0.39236827999999996,
"system": 1.3641371599999998,
"min": 0.67232387618,
"max": 0.98545938718,
"times": [
0.6989174971800001,
0.67232387618,
0.71119111918,
0.68090208318,
0.6938629321800001,
0.6998546841800001,
0.68571047018,
0.69946929918,
0.98545938718,
0.7400026001800001
]
},
{
"command": "pnpr@main",
"mean": 0.7563722883799999,
"stddev": 0.08002526520944013,
"median": 0.72917760318,
"user": 0.39235968000000004,
"system": 1.38667626,
"min": 0.69335756618,
"max": 0.9499379721800001,
"times": [
0.79897180918,
0.69335756618,
0.69590638218,
0.81124151318,
0.73152080418,
0.7026964961800001,
0.69855877818,
0.9499379721800001,
0.7268344021800001,
0.75469716018
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.21057094792,
"stddev": 0.023391391822376255,
"median": 4.200291248320001,
"user": 3.7352734399999994,
"system": 3.38196384,
"min": 4.17853999932,
"max": 4.24162402632,
"times": [
4.23134272532,
4.24162402632,
4.2003686493200005,
4.19133407432,
4.19412575332,
4.23469811432,
4.200213847320001,
4.17853999932,
4.23901116332,
4.194451126320001
]
},
{
"command": "pacquet@main",
"mean": 4.21222148962,
"stddev": 0.02970439001268445,
"median": 4.20081348632,
"user": 3.73557774,
"system": 3.3717686400000004,
"min": 4.175752274320001,
"max": 4.253875846320001,
"times": [
4.253875846320001,
4.18295392832,
4.175752274320001,
4.20217520332,
4.19945176932,
4.23821435232,
4.25094684932,
4.18610595932,
4.19558749832,
4.23715121532
]
},
{
"command": "pnpr@HEAD",
"mean": 2.1418604980200002,
"stddev": 0.10845013190591293,
"median": 2.11970891232,
"user": 2.48229754,
"system": 2.91761374,
"min": 2.0421416533200003,
"max": 2.3966652013200003,
"times": [
2.10138475232,
2.1380330723200003,
2.2436339643200003,
2.1616001223200003,
2.04225880732,
2.08425679732,
2.3966652013200003,
2.13984840632,
2.0421416533200003,
2.06878220332
]
},
{
"command": "pnpr@main",
"mean": 2.2165111428200004,
"stddev": 0.12564788285738857,
"median": 2.23227594932,
"user": 2.46721794,
"system": 2.92573884,
"min": 1.98319294432,
"max": 2.42897991432,
"times": [
2.21667054132,
2.42897991432,
2.12583074032,
2.2630622433200003,
1.98319294432,
2.1180820863200003,
2.33160480632,
2.2883715273200003,
2.2478813573200003,
2.1614352673200004
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.3417539080399998,
"stddev": 0.013036564430902797,
"median": 1.3433600705400002,
"user": 1.31754006,
"system": 1.68699896,
"min": 1.32191355254,
"max": 1.35812235854,
"times": [
1.35812235854,
1.34823781154,
1.33993298454,
1.35275352454,
1.32191355254,
1.32325035154,
1.33264861254,
1.34678715654,
1.33701889954,
1.3568738285400002
]
},
{
"command": "pacquet@main",
"mean": 1.4066138695400001,
"stddev": 0.0715925857597112,
"median": 1.39059505954,
"user": 1.3364731600000002,
"system": 1.7551601599999997,
"min": 1.35593881254,
"max": 1.60316791854,
"times": [
1.39502800954,
1.3604703825400002,
1.60316791854,
1.4032570155400002,
1.3722182975400001,
1.40871534154,
1.38616210954,
1.40693143354,
1.37424937454,
1.35593881254
]
},
{
"command": "pnpr@HEAD",
"mean": 0.66676623124,
"stddev": 0.03308375617872055,
"median": 0.65594723904,
"user": 0.33600776,
"system": 1.2990068599999998,
"min": 0.6456536845399999,
"max": 0.75612153054,
"times": [
0.64853838954,
0.75612153054,
0.67695678954,
0.65874214054,
0.64709289554,
0.6722777975399999,
0.65636023354,
0.65553424454,
0.65038460654,
0.6456536845399999
]
},
{
"command": "pnpr@main",
"mean": 0.6765368492399999,
"stddev": 0.09370474337042041,
"median": 0.64656093754,
"user": 0.33299776,
"system": 1.2920636600000002,
"min": 0.6409560935399999,
"max": 0.94302316854,
"times": [
0.65329017654,
0.64526533654,
0.64728255154,
0.64583932354,
0.64833175054,
0.65210720054,
0.6409560935399999,
0.64494276554,
0.64433012554,
0.94302316854
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 3.0128920316800003,
"stddev": 0.05162303238297894,
"median": 2.99622050208,
"user": 1.7844675000000003,
"system": 1.9609562399999998,
"min": 2.96419977108,
"max": 3.15070494808,
"times": [
2.99125439408,
3.0050940110799997,
2.99979748608,
2.96419977108,
2.99047424308,
3.01564787808,
2.98743674408,
2.99264351808,
3.15070494808,
3.0316673230799998
]
},
{
"command": "pacquet@main",
"mean": 3.0157537883800005,
"stddev": 0.03789501850431374,
"median": 3.00995607208,
"user": 1.7972662,
"system": 1.97431144,
"min": 2.98150009608,
"max": 3.11723741708,
"times": [
3.01830975808,
3.00946336408,
2.99726016608,
3.00793009208,
2.98150009608,
2.98459859508,
3.01403909908,
3.01044878008,
3.11723741708,
3.01675051608
]
},
{
"command": "pnpr@HEAD",
"mean": 0.68198339008,
"stddev": 0.007246548275964527,
"median": 0.6831774750800002,
"user": 0.33759429999999996,
"system": 1.3432887399999998,
"min": 0.6712437610800001,
"max": 0.6935887460800001,
"times": [
0.6712437610800001,
0.6841944130800001,
0.6821564320800001,
0.6753176990800001,
0.6853049840800001,
0.6717809300800001,
0.6851884290800001,
0.6821605370800001,
0.6935887460800001,
0.6888979690800001
]
},
{
"command": "pnpr@main",
"mean": 0.65538075478,
"stddev": 0.007543499425344166,
"median": 0.65418612458,
"user": 0.32299770000000005,
"system": 1.2904883399999998,
"min": 0.6423239890800001,
"max": 0.6676932040800001,
"times": [
0.6629601170800001,
0.6515722100800001,
0.6423239890800001,
0.65485053808,
0.66117931908,
0.6598306770800001,
0.6490158530800001,
0.6676932040800001,
0.6508599290800001,
0.6535217110800001
]
}
]
}Scenario: Isolated linker: fresh restore, cold cache + cold store + cold pnpr
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 6.91076838868,
"stddev": 0.1902083518358758,
"median": 6.849158878280001,
"user": 4.01540584,
"system": 3.7789683800000007,
"min": 6.647473675780001,
"max": 7.29928241278,
"times": [
6.932701846780001,
6.830538999780001,
7.29928241278,
6.79069801678,
6.86777875678,
7.1668374987800005,
6.93071207778,
6.82365942878,
6.81800117278,
6.647473675780001
]
},
{
"command": "pacquet@main",
"mean": 6.855683388979999,
"stddev": 0.1566863838474555,
"median": 6.81479851628,
"user": 4.0336269399999995,
"system": 3.7845370799999998,
"min": 6.63666390778,
"max": 7.17137910178,
"times": [
6.77639461378,
6.9521960617800005,
7.17137910178,
6.96706589778,
6.71900002278,
6.8532024187800005,
6.95374544278,
6.76854067078,
6.63666390778,
6.7586457517800005
]
},
{
"command": "pnpr@HEAD",
"mean": 4.954338699279999,
"stddev": 0.12799288213924553,
"median": 4.93651372728,
"user": 2.75993944,
"system": 3.22496268,
"min": 4.79694728978,
"max": 5.27703530378,
"times": [
4.991598795780001,
5.27703530378,
4.92716597478,
4.947442623780001,
4.9458614797800005,
4.85410580878,
4.9122244557800006,
4.79694728978,
4.991935170780001,
4.89907008978
]
},
{
"command": "pnpr@main",
"mean": 4.97969829168,
"stddev": 0.09528845991846709,
"median": 4.9602803052799995,
"user": 2.756878339999999,
"system": 3.24735368,
"min": 4.85332706578,
"max": 5.17634656278,
"times": [
4.97646704978,
5.04017315978,
5.040141691780001,
4.89345110878,
5.03561230778,
4.85332706578,
4.9341753097800005,
5.17634656278,
4.94409356078,
4.9031950997800005
]
}
]
} |
Contributor
Bencher Report
Contributor
|
| Branch | pr/12787 |
| Testbed | pnpr |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | Latency | milliseconds (ms) |
|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot | 2,141.86 ms |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot | 681.98 ms |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot | 666.77 ms |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot | 2,100.60 ms |
| isolated-linker.fresh-restore.cold-cache.cold-store.cold-pnpr | 📈 view plot | 4,954.34 ms |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot | 726.77 ms |
Comment on lines
+1784
to
+1792
| // A private upstream (registry-level `access:`) is caller-gated for | ||
| // every name; a public one can still gate individual names through a | ||
| // per-package `access` rule. | ||
| RegistrySource::Upstream(source) => { | ||
| state.inner.config.upstreams.get(&source).is_some_and(|upstream| { | ||
| upstream.access.is_some() | ||
| || !upstream.rules.for_package(package).access.allows(&Identity::Anonymous) | ||
| }) | ||
| } |
Member
Author
There was a problem hiding this comment.
The disjunction is deliberate, and deriving privacy solely from effective per-package access would under-mark here: unlike a hosted registry, an upstream's registry-level access: is enforced independently at serving — authorized_upstream runs before per-package rules on every upstream read — so a per-package access: $all entry cannot open a name on a private upstream, and access.is_some() alone already means the response varies by caller (403 vs 200). Marking such a response shared-cacheable would let a shared HTTP cache replay an authenticated response to an anonymous caller. Documented the asymmetry at the check in 53fed93.
|
Code review by qodo was updated up to the latest commit 5a5c797 |
|
Code review by qodo was updated up to the latest commit 5a5c797 |
Cache replay was registry-scoped for alias descriptors, so a caller passing the registry-level gate but denied by a per-package upstream access refinement could replay a cached resolution a fresh resolve would refuse them. The alias descriptor now carries the package name — only when the upstream's rules explicitly refine that name's access — and replay re-checks the refinement through the same per-package-aware alias selection a fresh resolve uses. Unrefined names keep the plain registry-scoped descriptor, so the common footprint stays one descriptor per alias. The refined metadata mirror namespaces per package for the same reason. Also document why resolves_to_private_source treats every name on an access-bearing upstream as caller-gated: unlike hosted registries, the upstream registry-level gate is enforced independently at serving (authorized_upstream runs before per-package rules), so a per-package 'access: $all' entry cannot open a name on a private upstream.
|
Code review by qodo was updated up to the latest commit 53fed93 |
1 similar comment
|
Code review by qodo was updated up to the latest commit 53fed93 |
zkochan
added a commit
to pnpm/pnpm.io
that referenced
this pull request
Jul 3, 2026
…maps (#834) Documents the redesigned model from pnpm/pnpm#12773, pnpm/pnpm#12778, and pnpm/pnpm#12787: - mounts:/defaultTarget: -> registries:/defaultRegistry: - routers collapse to ordered sources: lists; a package resolves to the first listed source whose declared packages claim it - the namespace and ACL merge into per-registry packages: maps, with most-specific-key selection and namespace enforcement on every path - the registry: config toggle is gone; the npm-registry surface is served iff at least one registry is declared, minus --disable-registry - the account (login/token) endpoints are always served on every tier
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the revised registries model from pnpm/rfcs#17 as a full replacement of the interim
patterns:-plus-global-ACL shape (pre-1.0, no compatibility mode).One declaration: the per-registry
packages:mapEvery concrete registry (hosted or upstream) declares one
packages:map whose keys are its namespace (replacing thepatterns:list) and whose values are the per-packageaccess/publish/unpublishrules (replacing the top-levelpackages:ACL, scoped to the one registry that serves the name). One declaration routes, filters, and authorizes. The registry-levelaccess:is the default an entry's omitted fields fall back to;publishdefaults to$authenticatedandunpublishto nobody.Specificity selection — key order carries no meaning
The most specific matching key wins: an exact name beats
@scope/*beats@*/*beats**. YAML mappings are formally unordered, so a formatter oryqround-trip must not change which access rule applies; the restricted pattern language makes the winner unique (at most one matching key per tier), no entry can be dead, and a duplicate key is the only within-registry error. Routersources:stay an ordered list with their unreachable-source/shadowed-claim validation intact.The removed global ACL fails loudly
A config still containing a top-level
packages:block is a startup error naming the per-registry replacement. It used to enforce access, so dropping it like an unknown verdaccio key would silently open previously gated packages on upgrade.Upstream rules
public: truekeeps describing the upstream fetch (no credential, no headers, no registry-levelaccess:default), but per-packageaccessrules are now permitted on a public upstream — they gate who may read the name through pnpr, not how pnpr fetches it.publish/unpublishvalues on any upstream are rejected on every tier: no write can land there.Hosted denials answer by tier
The decision is always the effective per-package access — an explicit entry fully decides its names, including opening one name (
access: $all) on an otherwise-private registry. Only the shape of a denial is tiered, preserving both prior behaviors through one mechanism: a caller the registry-level default also denies is masked with404— a blanket-private registry never confirms which names exist, even explicitly ruled ones — while an explicit entry denying a caller the default admits rejects loudly (401anonymous /403authenticated), so clients can prompt for credentials (the registry-mockneeds-authcontract the pnpm e2e suite relies on).Resolver classification follows the graph
Path-less fetches classify through the registry graph — the same dispatch serving uses — so a fetch resolving to an upstream source classifies through that upstream's alias, and hosted private-access descriptors are registry-qualified (
registry\0package): the samename@versionon two hosted registries can never share a cache key. Unqualified descriptors from older builds fail closed and re-resolve.Review-round additions
for_packageresolves the winner via per-tier maps instead of scanning every rule.Config surface
The bundled
config.yamlmoves the fixture namespace and old ACL into thelocalregistry's map (YAML anchors keep the per-keyunpublishcontract readable);Config::proxy/Config::static_servecarry the registry-mock rules programmatically; registry-mock keeps working with no task or fixture-seed changes.Checklist
packages:rejection, upstream write-rule rejection, public-upstream per-package access, upstream namespace bounds, tiered hosted denials; existing suites migrated to the map shape. 585 pnpr tests pass; clippy, rustfmt, and rustdoc clean; workspace check clean.config.yamldocuments the map shape).Written by an agent (Claude Code, claude-fable-5).
Summary by CodeRabbit
packagesrules now drive access, publish, and unpublish for both hosted and upstream registries.packagesentries.