• Add full token-record lookup to preserve readonly and CIDR restrictions.
• Enforce bearer-token restrictions in middleware before buffering large write bodies.
• Add CIDR parsing/matching and end-to-end tests for enforcement and edge cases.

graph TD
  A["HTTP request"] --> B(["Axum middleware: token gate"]) --> C["Route handler"]
  B --> D["TokenBackend.lookup_record"] --> E[("Token store")]
  D --> F{"Readonly?"} --> G{"CIDR allowed?"} --> C
  F --> H["Reject 403"]
  G --> H

  subgraph Legend
    direction LR
    _req["Request/Handler"] ~~~ _mw(["Middleware"]) ~~~ _dec{"Decision"} ~~~ _db[("Store")
  end

Loading

The following are alternative approaches to this PR:

1. Enforce restrictions inside identify()/authorization logic

• ➕ Centralizes all auth decisions in one place (identity + access policy).
• ➕ Avoids adding a separate middleware stage.
• ➖ Harder to ensure early rejection before large request bodies are buffered.
• ➖ Likely requires refactoring identity types to carry restrictions everywhere.

2. Use a CIDR library (e.g., ipnet / cidr / ipnetwork)

• ➕ Less bespoke parsing and bitmask logic to audit/maintain.
• ➕ Potentially broader coverage and fewer edge-case bugs.
• ➖ Adds a new dependency in a security-critical path.
• ➖ Must still define fail-closed behavior and IPv4-mapped IPv6 handling.

3. Have TokenBackend::lookup return TokenRecord instead of username

• ➕ Eliminates dual lookup APIs and the risk of dropping restrictions again.
• ➕ Makes restriction data available wherever bearer auth is used.
• ➖ Breaking trait/API change with broader churn across backends and call sites.
• ➖ More invasive than adding a default lookup_record() method.

Recommendation: The PR’s approach is the best tradeoff for a vuln fix: add a default TokenBackend::lookup_record() (minimizing backend churn) and enforce in middleware to fail fast before large bodies are read. The in-house CIDR matcher is acceptable here because it’s small, covered by targeted tests (IPv4/IPv6, boundary prefixes, malformed entries), and explicitly fail-closed; a third-party CIDR crate was the main alternative worth considering.

Bug fix (2) +195 / -7

auth.rsAdd TokenBackend::lookup_record to return full TokenRecord +10/-0

Add TokenBackend::lookup_record to return full TokenRecord

• Introduces a default trait method that resolves a raw token to a full TokenRecord (including readonly and cidr_whitelist). The implementation reuses find_by_key(sha256(raw)) to avoid per-backend changes while preserving restriction data.

pnpr/crates/pnpr/src/auth.rs

server.rsEnforce token restrictions via middleware using real peer address +185/-7

Enforce token restrictions via middleware using real peer address

• Wires ConnectInfo into axum::serve via a PeerAddr newtype and Connected impl for the custom listener wrapper. Adds an axum middleware that (1) extracts Bearer tokens, (2) loads TokenRecord via lookup_record, (3) blocks mutating methods for readonly tokens, and (4) checks CIDR whitelists against the socket peer IP with fail-closed behavior; includes in-house IPv4/IPv6 CIDR matching helpers.

pnpr/crates/pnpr/src/server.rs

Tests (2) +298 / -3

tests.rsTest that lookup_record preserves readonly/CIDR restrictions +29/-2

Test that lookup_record preserves readonly/CIDR restrictions

• Adds a tokio test seeding an in-memory TokenStore with a restricted TokenRecord and asserting lookup_record returns the restriction fields intact. Also verifies unknown tokens return None.

pnpr/crates/pnpr/src/auth/tests.rs

tests.rsAdd CIDR/method/header unit tests and middleware enforcement tests +269/-1

Add CIDR/method/header unit tests and middleware enforcement tests

• Adds unit tests for CIDR matching (IPv4/IPv6, /0 and host prefixes, malformed entries, IPv4-mapped IPv6 normalization) and for write-method/header parsing. Adds end-to-end middleware tests using a one-token backend to validate readonly and CIDR enforcement, peer-address requirements, and that X-Forwarded-For cannot bypass CIDR pinning.

pnpr/crates/pnpr/src/server/tests.rs

Walkthrough

Adds a lookup_record method to the TokenBackend trait that resolves a raw bearer token to its full TokenRecord. Introduces a PeerAddr type for non-spoofable socket peer IP via Axum's ConnectInfo. Implements a new authenticate Axum middleware that enforces read-only and CIDR-whitelist token restrictions before request handlers execute, backed by CIDR/IP parsing helpers. Refactors all route handlers and service helpers across the API (read, publish, account, dist-tags) to consume middleware-resolved Identity instead of reading authorization headers directly. Validates all behavior with CIDR/IP unit tests, helper unit tests, and comprehensive integration tests.

Changes

Bearer Token Self-Restrictions Enforcement

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Listener as TCP Listener
  participant Middleware as authenticate middleware
  participant TokenBackend
  participant Handler as Route Handler
  participant Client2

  Listener->>Middleware: accept peer, inject ConnectInfo<PeerAddr>
  Client->>Middleware: HTTP request + Authorization: Bearer <token>
  Middleware->>TokenBackend: lookup_record(raw_token)
  alt store error
    TokenBackend-->>Middleware: Err
    Middleware-->>Client: 500 Internal Server Error
  else token not found
    TokenBackend-->>Middleware: Ok(None)
    Middleware->>Handler: forward (anonymous identity)
    Handler-->>Client: response
  else token found with TokenRecord
    TokenBackend-->>Middleware: Ok(Some(TokenRecord))
    alt readonly=true AND is_write_method(http_method)
      Middleware-->>Client: 403 Forbidden
    else cidr_whitelist non-empty AND peer IP outside range
      Middleware-->>Client: 403 Forbidden
    else all checks pass
      Middleware->>Handler: forward with Identity in extensions
      Handler-->>Client2: response
    end
  end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Possibly related PRs

• pnpm/pnpm#12206: The main PR's new TokenBackend::lookup_record and server-side auth middleware wiring directly extend this PR's async TokenBackend trait and identity-resolution refactor.
• pnpm/pnpm#12011: This PR's new token-record lookup and middleware-resolved identity support enables the retrieved PR's whoami/profile/token CRUD handlers that require non-header-based caller resolution and token revocation by raw token.

Suggested labels

product: pnpr

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

• X
• Mastodon
• Reddit
• LinkedIn

Description

AuthedCaller extraction uses .cloned() on the value stored in request extensions, which clones
Identity and therefore clones the username: String on every request that hits a registry
handler. This adds avoidable per-request heap work on a pnpr hot path that the middleware otherwise
optimized to a single auth-backend lookup.

Code

pnpr/crates/pnpr/src/server.rs[R2033-2047]

+impl<RouterState: Send + Sync> FromRequestParts<RouterState> for AuthedCaller {
+    type Rejection = Response;
+
+    async fn from_request_parts(
+        parts: &mut Parts,
+        _state: &RouterState,
+    ) -> Result<Self, Self::Rejection> {
+        // The middleware runs on every route, so the context is always
+        // present; a miss means a wiring bug, surfaced as a 5xx.
+        parts.extensions.get::<AuthedCaller>().cloned().ok_or_else(|| {
+            error_response(&RegistryError::Internal {
+                reason: "authentication middleware did not run".to_string(),
+            })
+        })
+    }

Evidence

The extractor clones the stored AuthedCaller, and Identity::User contains an owned String, so
this becomes a per-request string clone/allocation on the handler path.

pnpr/crates/pnpr/src/server.rs[2033-2047]
pnpr/crates/pnpr/src/policy.rs[81-90]

Agent prompt

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

## Issue description
`AuthedCaller` currently clones the stored identity from `request.extensions`, which clones the underlying `String` username per request.
### Issue Context
The middleware already computed and stored the identity once per request; handlers should be able to retrieve it without cloning allocations.
### Fix Focus Areas
- pnpr/crates/pnpr/src/server.rs[2033-2047]
### Implementation notes
- Prefer moving the value out of extensions:
- In `FromRequestParts`, use `parts.extensions.remove::<AuthedCaller>()` instead of `get(...).cloned()`.
- This avoids cloning `Identity`/`String` while still failing with a 5xx if the middleware didn’t run.
- Alternative if multiple extractions are expected: store `Arc<Identity>` (or `Arc<str>` for username) in extensions so cloning is cheap.

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

Description

CIDR-pinned token enforcement reparses each whitelist entry (IpAddr parse + prefix/mask
computation) on every request, making authorization cost scale with whitelist length. This is
avoidable work on a hot path and can become significant for long whitelists or high RPS.

Code

pnpr/crates/pnpr/src/server.rs[R2191-2197]

+/// Whether `peer` falls inside any range of a token's CIDR whitelist. An
+/// IPv4-mapped IPv6 peer is normalized to its IPv4 form first, so a
+/// dual-stack listener still matches plain IPv4 ranges.
+fn cidr_whitelist_allows(whitelist: &[String], peer: SocketAddr) -> bool {
+    let peer = canonical_ip(peer.ip());
+    whitelist.iter().any(|entry| cidr_contains(entry.trim(), peer))
+}

Evidence

When CIDR restrictions are present, check_token_restrictions calls cidr_whitelist_allows, which
iterates the string whitelist and parses each entry via cidr_contains on every request.

pnpr/crates/pnpr/src/server.rs[2127-2137]
pnpr/crates/pnpr/src/server.rs[2191-2236]

Agent prompt

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

## Issue description
`cidr_whitelist_allows()` walks the whitelist and calls `cidr_contains()`, which parses the CIDR string every time a request is authorized. That repeats identical parsing work across requests using the same token.
### Issue Context
This runs inside `check_token_restrictions()` during bearer-token auth, so it’s on the authorization hot path.
### Fix Focus Areas
- pnpr/crates/pnpr/src/server.rs[2115-2137]
- pnpr/crates/pnpr/src/server.rs[2191-2257]
### Suggested fix approach
- Introduce a small bounded cache keyed by token key/hash (e.g., SHA-256 hex) that stores a pre-parsed whitelist representation (e.g., `Vec<(IpAddr, u8)>` or `Vec<CidrRange>`).
- Populate the cache when a token record is first resolved, and reuse it for subsequent requests using the same token.
- Ensure the cache is size-bounded (to avoid memory growth) and that malformed entries still fail closed (cache should store only successfully parsed entries; any parse failure should keep the token denied or mark the cache entry as "deny").

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

Description

The new enforce_token_restrictions middleware is applied to the entire router, so any request that
includes an Authorization: Bearer … header will trigger tokens.lookup_record() even on
/-/pnpr/* resolver and /-/ping routes that explicitly don’t consult auth. This expands an
attacker-controlled availability/perf risk (spam resolver/ping with random Bearer values to force DB
lookups) and adds avoidable latency/load on libsql/sqlx backends.

Code

pnpr/crates/pnpr/src/server.rs[R316-320]

+        // Enforce bearer-token read-only / CIDR restrictions ahead of
+        // every handler, so a restricted token is rejected before a write
+        // handler buffers its (up to 100 MiB) request body. Inside the
+        // trace layer below, so a rejection is still one access record.
+        .layer(axum::middleware::from_fn_with_state(state.clone(), enforce_token_restrictions))

Evidence

enforce_token_restrictions is installed as a router-wide layer, while the same router mounts
/-/ping and the resolver /-/pnpr/* endpoints. The resolver handler explicitly documents that it
has no access gate, so the new Bearer-triggered token lookup is extra work on routes that previously
avoided auth-store reads.

pnpr/crates/pnpr/src/server.rs[260-323]
pnpr/crates/pnpr/src/server.rs[2620-2631]

Agent prompt

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

## Issue description
`enforce_token_restrictions` is layered on the top-level router, so Bearer-present requests to endpoints that do not use auth (e.g. `/-/pnpr/v0/resolve`) still incur an auth-backend lookup. This increases unnecessary auth DB traffic and expands the DoS surface (attacker can force DB reads by sending random Bearer tokens to resolver/ping).
### Issue Context
The resolver handler explicitly states it has no per-package read gate and uses forwarded credentials for private packages; it should not require/trigger token backend reads unless the registry surface is actually being authorized.
### Fix Focus Areas
- pnpr/crates/pnpr/src/server.rs[260-323]
- pnpr/crates/pnpr/src/server.rs[2620-2631]
### Suggested fix direction
- Apply the `enforce_token_restrictions` layer only to the registry sub-router (i.e., wrap only the registry routes before merging into the main router), **or**
- Add a cheap path-based early return in `enforce_token_restrictions` to skip enforcement for `/-/pnpr` (and optionally `/-/ping`) routes, so resolver traffic can’t be used to amplify auth-backend load.

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

Description

is_write_method() treats PATCH as a mutating method even though the router only wires write
surfaces as PUT/DELETE, and the comment above claims writes are only PUT/DELETE. This
mismatch can create surprising readonly-token rejections if a non-mutating PATCH endpoint is added
later, or can mislead future changes/tests.

Code

pnpr/crates/pnpr/src/server.rs[R2183-2189]

+/// Whether `method` mutates registry state. Every write surface (publish,
+/// unpublish, dist-tag add/remove, adduser, logout, token revoke) is a
+/// PUT or DELETE; reads and the resolver POSTs are not. A read-only token
+/// is confined to the non-mutating methods.
+fn is_write_method(method: &Method) -> bool {
+    matches!(*method, Method::PUT | Method::DELETE | Method::PATCH)
+}

Evidence

The helper explicitly includes Method::PATCH, while the routing setup only attaches write handlers
using .put(...) and .delete(...) (no .patch(...) routes).

pnpr/crates/pnpr/src/server.rs[2183-2189]
pnpr/crates/pnpr/src/server.rs[294-313]

Agent prompt

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

## Issue description
`is_write_method` includes `PATCH` but surrounding documentation claims all mutating surfaces are `PUT`/`DELETE`, and current routing shows only `PUT`/`DELETE` for writes.
### Issue Context
This is a minor drift issue today, but it can trip future work (unexpected 403s for readonly tokens on PATCH endpoints, or confusion about intended behavior).
### Fix Focus Areas
- pnpr/crates/pnpr/src/server.rs[2183-2189]
- pnpr/crates/pnpr/src/server.rs[294-313]
### Implementation notes
Choose one:
- If PATCH is not intended: remove `Method::PATCH` from `is_write_method`.
- If PATCH is intended as a write-class method: update the comment to include PATCH and add/adjust tests accordingly.

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

Description

authenticate() copies the full Authorization header into an owned String for every request,
adding an avoidable heap allocation/copy on the request hot path. This extra work is paid even when
credentials are invalid and will be rejected, reducing throughput under load.

Code

pnpr/crates/pnpr/src/server.rs[R2067-2073]

+    let header = request
+        .headers()
+        .get(header::AUTHORIZATION)
+        .and_then(|value| value.to_str().ok())
+        .map(str::to_owned);
+    let method = request.method().clone();
+    let peer = request.extensions().get::<ConnectInfo<PeerAddr>>().map(|info| info.0.0);

Evidence

The middleware allocates a new String from the header value (map(str::to_owned)), and it is
installed as a router layer so the allocation occurs on every request that reaches the router.

pnpr/crates/pnpr/src/server.rs[2063-2073]
pnpr/crates/pnpr/src/server.rs[315-323]

Agent prompt

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

## Issue description
`authenticate()` currently does `to_str().ok().map(str::to_owned)`, which allocates and copies the whole `Authorization` header value on every request before any auth decision is made.
### Issue Context
This middleware is installed as a router layer, so it runs on the hot path for all requests.
### Fix Focus Areas
- pnpr/crates/pnpr/src/server.rs[2063-2081]
- pnpr/crates/pnpr/src/server.rs[2090-2109]
### Suggested fix approach
- In `authenticate()`, clone the header as a `HeaderValue` (cheap) rather than allocating a `String`:
- `let header = request.headers().get(header::AUTHORIZATION).cloned();`
- Update `resolve_caller` to accept `Option<HeaderValue>` (or similar owned type).
- For the Bearer path, parse/extract credentials and compute the SHA-256 digest immediately, then call `tokens.find_by_key(&digest)` to avoid retaining/allocating the full raw token across an `await`.
- For the Basic path, parse/decode from the owned `HeaderValue` bytes and pass only the minimal owned pieces (username/password) into any awaited backend calls.

ⓘ 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.