• Derive npm-registry surface enablement from mounts: and --disable-registry.
• Reject removed registry: config key to avoid fail-open exposure.
• Always mount account/token endpoints so resolver-only tiers can authenticate.

graph TD
  A["config.yaml (docs)"] --> B["Config loader (config.rs)"] --> C["Feature flags"] --> D["Axum router (server.rs)"] --> E["Registry routes"]
  D --> F["Resolver routes"]
  D --> G["Account routes"]
  H["Tests"] --> B --> D

Loading

The following are alternative approaches to this PR:

1. Soft-deprecate `registry:` (warn + ignore) instead of erroring

• ➕ Smoother upgrades for existing configs that still carry registry:
• ➕ Avoids startup failures when operators forget to remove the key
• ➖ Risk of silent semantic drift (especially registry: { enabled: false })
• ➖ Harder to guarantee fail-closed behavior for security-sensitive toggles

2. Keep `registry.enabled` but enforce consistency with `mounts:`

• ➕ Preserves an explicit operator toggle for registry exposure
• ➕ Can emit a targeted error when mounts exist but registry.enabled=false
• ➖ Continues duplicating config intent across two fields
• ➖ Still requires defining precedence rules and documenting them

3. Gate account endpoints behind resolver/registry enablement

• ➕ Smaller always-exposed API surface area
• ➕ Keeps all auth-adjacent routes grouped under a surface conceptually
• ➖ Breaks resolver-only tiers that need to mint/manage tokens
• ➖ Encourages coupling auth workflows to registry-serving replicas

Recommendation: The PR’s approach is the best trade-off: deriving registry enablement from mounts: eliminates contradictory configuration, and rejecting the removed registry: key avoids a fail-open scenario for previously disabling configs. Always-mounting account/token endpoints is a pragmatic separation of concerns that enables resolver-only tiers to be operationally independent while keeping registry routes truly absent when disabled.

Enhancement (2) +237 / -135

config.rsDerive registry enablement from mounts and reject removed 'registry:' key +58/-30

Derive registry enablement from mounts and reject removed 'registry:' key

• Replaces the YAML-configured registry toggle with derived enablement ('!mounts.is_empty()'), still honoring '--disable-registry'. Adds a custom deserializer that errors on any 'registry:' key presence with migration guidance, and updates the startup invariant error to a clearer "nothing to serve" message.

pnpr/crates/pnpr/src/config.rs

server.rsAlways mount account/token endpoints and decouple them from registry surface +179/-105

Always mount account/token endpoints and decouple them from registry surface

• Moves login/whoami/profile/token list+revoke/logout onto dedicated always-mounted routes (also under '/{prefix}/...' constrained to '~'-prefixed paths). Removes legacy handling of these endpoints from the registry catch-all segment handlers and updates startup/auth loading commentary to match the new routing model.

pnpr/crates/pnpr/src/server.rs

Tests (2) +98 / -67

tests.rsUpdate config unit tests for derived registry surface and key rejection +45/-67

Update config unit tests for derived registry surface and key rejection

• Reworks tests to assert registry enablement is determined by mount presence and that resolver toggles still parse as before. Adds coverage ensuring any 'registry:' key form fails config load, and updates the misconfiguration test to the new "nothing to serve" invariant.

pnpr/crates/pnpr/src/config/tests.rs

server.rsExtend resolver-only integration test to cover account endpoints and token use +53/-0

Extend resolver-only integration test to cover account endpoints and token use

• Enhances the resolver-only scenario to allow user creation/login and to validate whoami/profile/token-list endpoints work without registry routes. Verifies a minted token successfully authenticates to the resolver endpoint, ensuring resolver-only tiers can manage credentials independently.

pnpr/crates/pnpr/tests/server.rs

Other (2) +8 / -7

config.yamlUpdate bundled config docs to explain derived registry surface +6/-5

Update bundled config docs to explain derived registry surface

• Removes the documented 'registry:' toggle and explains that the registry surface is enabled when 'mounts:' are declared, with '--disable-registry' as a per-tier override. Clarifies that account endpoints are always served alongside '/-/ping'.

pnpr/crates/pnpr/config.yaml

main.rsClarify '--disable-registry' CLI semantics under derived enablement +2/-2

Clarify '--disable-registry' CLI semantics under derived enablement

• Updates the flag help text to reflect that registry routes are served whenever mounts exist, unless explicitly disabled per tier via '--disable-registry'.

pnpr/crates/pnpr/src/main.rs

Walkthrough

This PR derives registry-surface enablement from mounts, moves account endpoints to dedicated always-on routes, removes account handling from generic dispatch, and updates related config, CLI, logging, and tests.

Changes

Registry surface derivation and account routing

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ConfigFile
  participant ConfigLoader
  participant Router
  participant AccountRoutes

  ConfigFile->>ConfigLoader: parse mounts and resolver settings
  ConfigLoader->>Router: build tier routes from derived enablement
  Router->>AccountRoutes: mount whoami/login/logout/profile/token routes
  AccountRoutes-->>Router: always-on account responses

Assessment against linked issues

Possibly related PRs

• pnpm/pnpm#12563: Changes pnpr surface enablement behavior in config and server code.
• pnpm/pnpm#12574: Touches the same auth/account route handling in server.rs.
• pnpm/pnpm#12747: Shares the mount-derived routing foundation used by this change.

Suggested labels: product: pnpr

Suggested reviewers: juanpicado

• X
• Mastodon
• Reddit
• LinkedIn

Description

The startup guard now errors when mounts exist but --disable-registry is set and the resolver is
disabled, because it checks effective registry.enabled rather than the mounts-derived registry
capability. This violates the compliance requirement that “nothing to serve” should only be
triggered when no mounts are declared and the resolver is disabled.

Code

pnpr/crates/pnpr/src/config.rs[R1551-1565]

+    /// At least one top-level surface must be served; a server with no
+    /// registry surface (no mounts declared, or `--disable-registry`) and
+    /// the resolver disabled would answer only `/-/ping` and the account
+    /// endpoints. Checked at config load and again in the serve/router
+    /// entry points for programmatically built configs.
  pub fn ensure_a_feature_is_enabled(&self) -> Result<(), RegistryError> {
      if self.registry.enabled || self.resolver.enabled {
          Ok(())
      } else {
          Err(RegistryError::InvalidConfig {
-                reason: "at least one of `registry` or `resolver` must be enabled".to_string(),
+                reason: "nothing to serve: the npm-registry surface is off (no `mounts:` \
+                         declared, or `--disable-registry`) and the resolver is disabled"
+                    .to_string(),
          })
      }

Evidence

PR Compliance ID 5 requires that the startup error only occurs when there are no mounts declared and
the resolver is disabled. The current implementation uses self.registry.enabled (which is false
when --disable-registry is set even if mounts exist), so it can incorrectly throw the “nothing to
serve” error in a mounts-present configuration.

Update “nothing to serve” startup check to reflect mounts-derived registry surface
pnpr/crates/pnpr/src/config.rs[1551-1565]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The “nothing to serve” startup check currently fails when `--disable-registry` disables the registry surface even if mounts exist, because it checks `self.registry.enabled || self.resolver.enabled`.
## Issue Context
Per PR Compliance ID 5, startup should only error when **no mounts are declared** AND the resolver is disabled (via config and/or CLI). With mounts present, the config is valid even if the registry is effectively disabled by CLI.
## Fix Focus Areas
- pnpr/crates/pnpr/src/config.rs[1502-1565]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The newly always-mounted logout route DELETE /-/user/token/{token} places the raw bearer token in
the URL path, while the global access TraceLayer logs request.uri() verbatim; this will write
reusable credentials into logs on resolver-only tiers too. Any log reader (or downstream log export)
can replay leaked tokens until revoked.

Code

pnpr/crates/pnpr/src/server.rs[R298-305]

+        .route("/-/user/token/{token}", delete(delete_session_token))
+        .route("/{prefix}/-/user/token/{token}", delete(delete_session_token_prefixed))
+        .route("/-/npm/v1/user", get(get_profile))
+        .route("/{prefix}/-/npm/v1/user", get(get_profile_prefixed))
+        .route("/-/npm/v1/tokens", get(get_token_list))
+        .route("/{prefix}/-/npm/v1/tokens", get(get_token_list_prefixed))
+        .route("/-/npm/v1/tokens/token/{key}", delete(delete_token_by_key))
+        .route("/{prefix}/-/npm/v1/tokens/token/{key}", delete(delete_token_by_key_prefixed));

Evidence

The router now mounts a logout endpoint whose URL contains the raw bearer token, and the access
logger records the full URI; therefore the token will be present in logs for these requests.

pnpr/crates/pnpr/src/server.rs[273-305]
pnpr/crates/pnpr/src/server.rs[409-417]
pnpr/crates/pnpr/src/server.rs[2190-2204]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The access log span includes `uri = %request.uri()`, which will include sensitive path segments for endpoints like `DELETE /-/user/token/{token}` where `{token}` is the raw bearer token. With account endpoints now mounted on every tier, this becomes a practical credential-leak vector for resolver-only deployments.
### Issue Context
- The router now always mounts `/-/user/token/{token}` and its prefixed twin.
- The access TraceLayer logs the full request URI.
- The logout handler explicitly documents that the token is the raw bearer token in the URL.
### Fix Focus Areas
- pnpr/crates/pnpr/src/server.rs[293-305]
- pnpr/crates/pnpr/src/server.rs[409-421]
- pnpr/crates/pnpr/src/server.rs[2190-2204]
### Suggested fix approach
1. Add a small helper to produce a redacted URI string for logging (fast path: return the original for most routes).
2. In `TraceLayer::make_span_with`, log the redacted URI instead of `request.uri()`.
- Redact at least:
- `/-/user/token/<...>` and `/<prefix>/-/user/token/<...>`
- (Optionally) `/-/npm/v1/tokens/token/<...>` and prefixed variant, if you consider token keys sensitive in your environment.
3. Consider also capping/validating the `{token}` path parameter length in the logout handler to reduce worst-case CPU (hashing/lookup) on intentionally huge path segments, but the primary fix is log redaction.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The config loader remains verdaccio-lenient, so because ConfigFile no longer declares a registry
field and does not deny unknown YAML keys, a removed top-level registry: key will be silently
dropped instead of failing config load. This violates the requirement that registry: is no longer
accepted and can cause a fail-open where legacy configs like registry: { enabled: false } still
expose the registry surface whenever mounts: is non-empty (unless overridden by the CLI).

Code

pnpr/crates/pnpr/src/config.rs[R1012-1018]

+    /// pnpr-only feature toggle for the resolver surface. On unless
+    /// explicitly disabled; absent on a stock verdaccio config, so it
+    /// stays enabled there. `Option` so a bare `resolver:` (which YAML
+    /// parses as null) is accepted as "default" rather than failing to
+    /// deserialize into the struct.
#[serde(default)]
resolver: Option<FeatureFile>,

Evidence

Compliance requires that the top-level registry: key is no longer accepted by the schema, but
ConfigFile deserialization is intentionally lenient (no deny_unknown_fields) and the struct no
longer contains a registry field, which means any YAML registry: entry will be treated as an
unknown top-level key and silently ignored rather than producing a config error. Additionally, the
config load logic now derives registry.enabled from whether mounts is non-empty (and a CLI
disable override), so a legacy registry.enabled: false setting cannot actually disable the
registry surface and will be ignored, creating unexpected fail-open exposure when mounts exist.

Remove top-level registry: config field and derive registry surface from mounts:
pnpr/crates/pnpr/src/config.rs[985-1051]
pnpr/crates/pnpr/src/config.rs[1478-1484]
pnpr/crates/pnpr/src/config.rs[1471-1549]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR removes the `registry:` field from `ConfigFile`, but because config deserialization is intentionally verdaccio-lenient about unknown top-level keys, a `registry:` key will now be silently ignored instead of rejected with migration guidance. This violates the compliance requirement that `registry:` is no longer accepted and, since `registry.enabled` is now derived from `mounts:` (plus a CLI override), legacy configs like `registry: { enabled: false }` can unintentionally leave the registry surface enabled whenever mounts are present.
## Issue Context
Compliance requires the YAML/Config schema to fail-closed when a top-level `registry:` key is present (with clear migration guidance). Today `ConfigFile` does not deny unknown fields for verdaccio compatibility, so removing the `registry` field causes `registry:` to be treated as unknown and dropped; separately, `registry.enabled` is computed solely from `!mounts.is_empty()` and `--disable-registry`, so legacy `registry.enabled: false` has no effect.
Suggested implementation approaches:
1. Preserve lenient parsing *but* explicitly detect a top-level `registry` key during config load.
- Option A (preferred): add a `#[serde(flatten)] extra: IndexMap<String, serde_saphyr::Value>` (or equivalent value type) to `ConfigFile`, and after deserialization error if `extra.contains_key("registry")`.
- Option B: parse once into a generic YAML value/map, check for `registry` presence, then deserialize into `ConfigFile`.
2. Return `RegistryError::InvalidConfig` (or equivalent) with clear migration guidance (e.g. “remove `registry:`; registry surface is now derived from `mounts:`; use `--disable-registry` per tier”).
3. Add tests covering at least `registry:` (null), `registry: { enabled: false }`, and `registry: false` (scalar), ensuring each fails config load with the guidance string.
## Fix Focus Areas
- pnpr/crates/pnpr/src/config.rs[985-1051]
- pnpr/crates/pnpr/src/config.rs[1471-1549]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

put_login/put_login_prefixed are now mounted on every tier and return a freshly minted bearer
token via add_user, but the response uses Cache-Control: no-cache, which does not forbid
intermediaries from storing the token-bearing 201 response. This increases the risk of token
retention/exposure in shared caching layers (proxies/CDNs) compared to an explicit no-store
policy.

Code

pnpr/crates/pnpr/src/server.rs[R1012-1020]

+async fn put_login(
+    State(state): State<AppState>,
+    Path(user): Path<String>,
+    body: axum::body::Bytes,
+) -> Response {
+    match user.strip_prefix("org.couchdb.user:") {
+        Some(name) => add_user(&state, name, &body).await,
+        None => not_found(),
+    }

Evidence

The router now mounts login on every tier and put_login forwards to add_user, which returns a
JSON body containing token while setting Cache-Control: no-cache (not no-store). The codebase
already defines private_no_cache for sensitive identity endpoints using private, no-store,
showing the intended caching posture for credentialed data.

pnpr/crates/pnpr/src/server.rs[280-318]
pnpr/crates/pnpr/src/server.rs[1010-1032]
pnpr/crates/pnpr/src/server.rs[2090-2135]
pnpr/crates/pnpr/src/server.rs[2339-2352]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The login/adduser endpoint returns a bearer token but sets `Cache-Control: no-cache`, which does not explicitly prevent storage of the token-bearing response.
### Issue Context
This PR mounts the login/adduser route on every tier (`router_with_auth_and_osv`), so token issuance is reachable in more deployment topologies (including ones fronted by shared proxies).
### Fix Focus Areas
- pnpr/crates/pnpr/src/server.rs[300-310]
- pnpr/crates/pnpr/src/server.rs[2096-2135]
- pnpr/crates/pnpr/src/server.rs[2339-2352]
### What to change
- In `add_user`, change the response caching headers to explicitly forbid storage, e.g. `Cache-Control: private, no-store` (or at minimum `no-store`).
- Consider reusing `private_no_cache(...)` or adding a small helper for token-bearing responses (note `private_no_cache` also sets `Vary: Authorization`; for login it’s optional but harmless).
- Add/extend a test to assert the login response carries `no-store` (similar to existing auth-endpoint cache header tests).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

router_with_auth_and_osv now mounts the unauthenticated adduser/login endpoint on every tier
(including resolver-only), and each request can trigger expensive bcrypt hashing/verification via
add_user → auth.users.add_or_login. This allows unauthenticated callers to consume significant
CPU with repeated login attempts, degrading availability of the tier’s resolver/registry surfaces.

Code

pnpr/crates/pnpr/src/server.rs[R286-310]

+    // The account endpoints — adduser/login, whoami, profile, token
+    // listing/revocation, logout — are pnpr account management, not
+    // npm-registry functionality: they mint and manage the tokens every
+    // authenticated surface demands, so they ride every tier alongside
+    // `/-/ping`. A resolver-only tier can then issue its own credentials
+    // (`pnpm login --registry https://<resolver-host>/`) instead of
+    // depending on a registry-serving replica that shares the auth backend.
+    //
+    // Each endpoint also answers under any `/~<prefix>/`, so a client whose
+    // registry URL is a mount endpoint can log in against it. The identity
+    // endpoints are global and consult no mount state; a mount-table lookup
+    // would gate nothing while turning the 401-vs-404 split into an
+    // existence oracle for private mount names that the content handlers
+    // carefully mask.
+    router = router
+        .route("/-/whoami", get(get_whoami))
+        .route("/{prefix}/-/whoami", get(get_whoami_prefixed))
+        .route(
+            "/-/user/{user}",
+            put(put_login).route_layer(DefaultBodyLimit::max(MAX_LOGIN_BODY_BYTES)),
+        )
+        .route(
+            "/{prefix}/-/user/{user}",
+            put(put_login_prefixed).route_layer(DefaultBodyLimit::max(MAX_LOGIN_BODY_BYTES)),
+        )

Evidence

The router now mounts login routes unconditionally on every tier, making them reachable even when
the registry surface is off. Those routes call add_user, which delegates to
auth.users.add_or_login; the auth implementation uses bcrypt hashing/verification, documented as
~50–100ms cost, making repeated unauthenticated login attempts a viable CPU exhaustion vector.

pnpr/crates/pnpr/src/server.rs[280-318]
pnpr/crates/pnpr/src/server.rs[2088-2134]
pnpr/crates/pnpr/src/auth.rs[761-795]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The login/adduser endpoint is now always mounted (even on resolver-only tiers) and is reachable without prior authentication. Each request may invoke bcrypt hashing/verification, which is intentionally expensive; repeated requests can cause CPU exhaustion and deny service.
## Issue Context
The PR intentionally moved account endpoints (including login) out of the registry-gated surface so resolver-only tiers can mint/manage tokens. The existing 64KiB body cap mitigates memory amplification, but CPU amplification via bcrypt remains.
## Fix Focus Areas
Add an explicit throttle on the login route (or the underlying auth path) to bound bcrypt work under abuse.
Practical options:
- Apply a `ConcurrencyLimitLayer` (or similar) specifically to the two login routes (`/-/user/{user}` and `/{prefix}/-/user/{user}`), returning 429/503 when saturated.
- Or add a dedicated `tokio::sync::Semaphore` in shared state to cap concurrent login attempts, acquiring a permit at the start of `put_login` / `put_login_prefixed` and failing fast if unavailable.
- If you already have a preferred rate-limiter middleware in the repo, reuse it for per-IP or global rate limiting.
### References
- pnpr/crates/pnpr/src/server.rs[280-318]
- pnpr/crates/pnpr/src/server.rs[2088-2134]
- pnpr/crates/pnpr/src/auth.rs[761-795]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

loggable_uri() returns a newly allocated String and is called for every request when building
the access-log span, so even the common non-redaction case (None => uri.to_string()) adds
per-request heap allocation and formatting overhead. This is on pnpr’s hottest path (every HTTP
request), so it can measurably increase CPU/allocator pressure under high request rates (e.g.
install fan-out).

Code

pnpr/crates/pnpr/src/server.rs[R426-432]

                   target: "pnpr::access",
                   "request",
                   method = %request.method(),
-                        uri = %request.uri(),
+                        uri = %loggable_uri(request.uri()),
                   // Filled in by `record_cache_status` for packument
                   // reads (e.g. `cache=hit`); stays absent otherwise.
                   cache = tracing::field::Empty,

Evidence

The access TraceLayer calls loggable_uri(request.uri()) while creating a span for each request,
and loggable_uri() allocates a String even when no token marker is present via
uri.to_string(). This makes the allocation unconditional on the request hot path.

pnpr/crates/pnpr/src/server.rs[422-463]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The access log span currently uses `uri = %loggable_uri(request.uri())`, and `loggable_uri()` returns a `String` (`uri.to_string()` in the common case). This introduces an unconditional heap allocation for *every* HTTP request.
## Issue Context
The redaction goal is correct (never log the raw bearer token in `/-/user/token/{token}`), but the implementation can preserve that security property without allocating in the non-redaction case.
## Fix Focus Areas
- pnpr/crates/pnpr/src/server.rs[423-463]
## Suggested fix approach
- Replace `fn loggable_uri(...) -> String` with a small wrapper type implementing `std::fmt::Display`, e.g. `struct LoggableUri<'a>(&'a Uri);`.
- In `Display::fmt`, scan `self.0.path()` for `"/-/user/token/"`:
- If found, write the prefix and `"<redacted>"` directly to the formatter.
- If not found, write `path_and_query` (or the `Uri` itself) directly, avoiding `to_string()`.
- Change the span field to `uri = %LoggableUri(request.uri())` so `tracing` can format without allocating.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

• ❄️ 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.